diff --git a/userfront/lib/core/services/oidc_redirect_guard.dart b/userfront/lib/core/services/oidc_redirect_guard.dart index b0db4c79..93cc3b9c 100644 --- a/userfront/lib/core/services/oidc_redirect_guard.dart +++ b/userfront/lib/core/services/oidc_redirect_guard.dart @@ -3,19 +3,90 @@ class OidcRedirectCheckResult { final bool isValid; final String reason; final int length; + final String scheme; final String host; final String path; + final int queryParamCount; + final List queryKeys; final bool hasLoginVerifier; + final int loginVerifierLength; + final bool hasState; + final int stateLength; + final bool hasClientId; + final String clientId; + final bool hasCodeChallenge; + final int codeChallengeLength; + final String codeChallengeMethod; + final bool hasRedirectUri; + final int redirectUriLength; + final String redirectUriScheme; + final String redirectUriHost; + final int redirectUriPort; + final String redirectUriPath; + final String responseType; + final int scopeCount; + final bool isOidcAuthPath; const OidcRedirectCheckResult({ required this.uri, required this.isValid, required this.reason, required this.length, + required this.scheme, required this.host, required this.path, + required this.queryParamCount, + required this.queryKeys, required this.hasLoginVerifier, + required this.loginVerifierLength, + required this.hasState, + required this.stateLength, + required this.hasClientId, + required this.clientId, + required this.hasCodeChallenge, + required this.codeChallengeLength, + required this.codeChallengeMethod, + required this.hasRedirectUri, + required this.redirectUriLength, + required this.redirectUriScheme, + required this.redirectUriHost, + required this.redirectUriPort, + required this.redirectUriPath, + required this.responseType, + required this.scopeCount, + required this.isOidcAuthPath, }); + + Map toDiagnostics() { + return { + 'is_valid': isValid, + 'reason': reason, + 'length': length, + 'scheme': scheme, + 'host': host, + 'path': path, + 'is_oidc_auth_path': isOidcAuthPath, + 'query_param_count': queryParamCount, + 'query_keys': queryKeys, + 'has_login_verifier': hasLoginVerifier, + 'login_verifier_len': loginVerifierLength, + 'has_state': hasState, + 'state_len': stateLength, + 'has_client_id': hasClientId, + 'client_id': clientId, + 'has_code_challenge': hasCodeChallenge, + 'code_challenge_len': codeChallengeLength, + 'code_challenge_method': codeChallengeMethod, + 'has_redirect_uri': hasRedirectUri, + 'redirect_uri_len': redirectUriLength, + 'redirect_uri_scheme': redirectUriScheme, + 'redirect_uri_host': redirectUriHost, + 'redirect_uri_port': redirectUriPort, + 'redirect_uri_path': redirectUriPath, + 'response_type': responseType, + 'scope_count': scopeCount, + }; + } } OidcRedirectCheckResult validateOidcRedirectTarget(String redirectTo) { @@ -26,9 +97,29 @@ OidcRedirectCheckResult validateOidcRedirectTarget(String redirectTo) { isValid: false, reason: 'empty', length: 0, + scheme: '', host: '', path: '', + queryParamCount: 0, + queryKeys: [], hasLoginVerifier: false, + loginVerifierLength: 0, + hasState: false, + stateLength: 0, + hasClientId: false, + clientId: '', + hasCodeChallenge: false, + codeChallengeLength: 0, + codeChallengeMethod: '', + hasRedirectUri: false, + redirectUriLength: 0, + redirectUriScheme: '', + redirectUriHost: '', + redirectUriPort: 0, + redirectUriPath: '', + responseType: '', + scopeCount: 0, + isOidcAuthPath: false, ); } @@ -41,9 +132,29 @@ OidcRedirectCheckResult validateOidcRedirectTarget(String redirectTo) { isValid: false, reason: 'parse_error', length: trimmed.length, + scheme: '', host: '', path: '', + queryParamCount: 0, + queryKeys: [], hasLoginVerifier: false, + loginVerifierLength: 0, + hasState: false, + stateLength: 0, + hasClientId: false, + clientId: '', + hasCodeChallenge: false, + codeChallengeLength: 0, + codeChallengeMethod: '', + hasRedirectUri: false, + redirectUriLength: 0, + redirectUriScheme: '', + redirectUriHost: '', + redirectUriPort: 0, + redirectUriPath: '', + responseType: '', + scopeCount: 0, + isOidcAuthPath: false, ); } @@ -51,6 +162,27 @@ OidcRedirectCheckResult validateOidcRedirectTarget(String redirectTo) { final isHttpScheme = scheme == 'http' || scheme == 'https'; final isAbsolute = parsed.hasScheme && parsed.host.isNotEmpty; final isValid = isHttpScheme && isAbsolute; + final query = parsed.queryParameters; + final queryKeys = query.keys.toList()..sort(); + final loginVerifier = query['login_verifier'] ?? ''; + final state = query['state'] ?? ''; + final clientId = query['client_id'] ?? ''; + final codeChallenge = query['code_challenge'] ?? ''; + final codeChallengeMethod = query['code_challenge_method'] ?? ''; + final redirectUriValue = query['redirect_uri'] ?? query['redirect_url'] ?? ''; + final responseType = query['response_type'] ?? ''; + final scope = query['scope'] ?? ''; + + final Uri? redirectUriParsed = redirectUriValue.isEmpty + ? null + : Uri.tryParse(redirectUriValue); + final redirectUriScheme = redirectUriParsed?.scheme ?? ''; + final redirectUriHost = redirectUriParsed?.host ?? ''; + final redirectUriPort = redirectUriParsed?.port ?? 0; + final redirectUriPath = redirectUriParsed?.path ?? ''; + final scopeCount = scope.isEmpty + ? 0 + : scope.split(RegExp(r'\s+')).where((s) => s.isNotEmpty).length; final reason = isValid ? 'ok' @@ -61,8 +193,28 @@ OidcRedirectCheckResult validateOidcRedirectTarget(String redirectTo) { isValid: isValid, reason: reason, length: trimmed.length, + scheme: scheme, host: parsed.host, path: parsed.path, - hasLoginVerifier: parsed.queryParameters.containsKey('login_verifier'), + queryParamCount: query.length, + queryKeys: queryKeys, + hasLoginVerifier: loginVerifier.isNotEmpty, + loginVerifierLength: loginVerifier.length, + hasState: state.isNotEmpty, + stateLength: state.length, + hasClientId: clientId.isNotEmpty, + clientId: clientId, + hasCodeChallenge: codeChallenge.isNotEmpty, + codeChallengeLength: codeChallenge.length, + codeChallengeMethod: codeChallengeMethod, + hasRedirectUri: redirectUriValue.isNotEmpty, + redirectUriLength: redirectUriValue.length, + redirectUriScheme: redirectUriScheme, + redirectUriHost: redirectUriHost, + redirectUriPort: redirectUriPort, + redirectUriPath: redirectUriPath, + responseType: responseType, + scopeCount: scopeCount, + isOidcAuthPath: parsed.path == '/oidc/oauth2/auth', ); } diff --git a/userfront/lib/features/auth/domain/password_login_flow_policy.dart b/userfront/lib/features/auth/domain/password_login_flow_policy.dart new file mode 100644 index 00000000..801d4c6c --- /dev/null +++ b/userfront/lib/features/auth/domain/password_login_flow_policy.dart @@ -0,0 +1,23 @@ +enum PasswordLoginNextAction { redirectToOidc, acceptOidc, localLogin, invalid } + +PasswordLoginNextAction decidePasswordLoginNextAction({ + required bool hasLoginChallenge, + required String? redirectTo, + required String? jwt, +}) { + final hasRedirectTo = redirectTo != null && redirectTo.isNotEmpty; + if (hasRedirectTo) { + return PasswordLoginNextAction.redirectToOidc; + } + + if (hasLoginChallenge) { + return PasswordLoginNextAction.acceptOidc; + } + + final hasJwt = jwt != null && jwt.isNotEmpty; + if (hasJwt) { + return PasswordLoginNextAction.localLogin; + } + + return PasswordLoginNextAction.invalid; +} diff --git a/userfront/lib/features/auth/presentation/login_screen.dart b/userfront/lib/features/auth/presentation/login_screen.dart index 530ffef2..a0b4ffcd 100644 --- a/userfront/lib/features/auth/presentation/login_screen.dart +++ b/userfront/lib/features/auth/presentation/login_screen.dart @@ -11,6 +11,7 @@ import '../../../core/services/auth_proxy_service.dart'; import '../../../core/services/auth_token_store.dart'; import '../../../core/services/oidc_redirect_guard.dart'; import '../../../core/notifiers/auth_notifier.dart'; +import '../domain/password_login_flow_policy.dart'; import '../../profile/domain/notifiers/profile_notifier.dart'; import '../../../core/services/web_window.dart'; @@ -167,11 +168,15 @@ class _LoginScreenState extends ConsumerState Future _onCookieLoginSuccess(String provider) async { debugPrint("[Auth] Cookie-based login success. Provider: $provider"); AuthNotifier.instance.notify(); - if (_loginChallenge != null && _loginChallenge!.isNotEmpty) { + if (_hasLoginChallenge) { final accepted = await _acceptOidcLoginAndRedirect(); if (accepted) { return; } + if (mounted) { + _showError(tr('msg.userfront.login.oidc_failed')); + } + return; } final token = AuthTokenStore.getToken(); @@ -238,6 +243,7 @@ class _LoginScreenState extends ConsumerState bool _redirectToOidcTarget(String redirectTo, {required String source}) { final checked = validateOidcRedirectTarget(redirectTo); + _logOidcRedirectDiagnostics(source: source, checked: checked); debugPrint( "[Auth] OIDC redirect check ($source): valid=${checked.isValid}, reason=${checked.reason}, len=${checked.length}, host=${checked.host}, path=${checked.path}, has_login_verifier=${checked.hasLoginVerifier}", ); @@ -249,8 +255,42 @@ class _LoginScreenState extends ConsumerState return false; } - webWindow.redirectTo(checked.uri.toString()); - return true; + try { + debugPrint( + "[Auth] OIDC redirect execute ($source): host=${checked.host}, path=${checked.path}, redirect_uri_host=${checked.redirectUriHost}, redirect_uri_port=${checked.redirectUriPort}, state_len=${checked.stateLength}, login_verifier_len=${checked.loginVerifierLength}", + ); + webWindow.redirectTo(checked.uri.toString()); + return true; + } catch (e) { + debugPrint("[Auth] OIDC redirect failed ($source): $e"); + if (mounted) { + _showError(tr('msg.userfront.login.oidc_failed')); + } + return false; + } + } + + bool get _hasLoginChallenge => + _loginChallenge != null && _loginChallenge!.isNotEmpty; + + void _logOidcRedirectDiagnostics({ + required String source, + required OidcRedirectCheckResult checked, + }) { + final current = Uri.base; + final currentQueryKeys = current.queryParameters.keys.toList()..sort(); + + final payload = { + 'source': source, + 'current_path': current.path, + 'current_query_param_count': current.queryParameters.length, + 'current_query_keys': currentQueryKeys, + 'has_login_challenge': _hasLoginChallenge, + 'login_challenge_len': _loginChallenge?.length ?? 0, + ...checked.toDiagnostics(), + }; + + debugPrint("[Auth] OIDC redirect diagnostics: ${jsonEncode(payload)}"); } void _resetLinkLoginState() { @@ -829,17 +869,44 @@ class _LoginScreenState extends ConsumerState password, loginChallenge: _loginChallenge, ); - final jwt = res['sessionJwt'] ?? res['sessionToken'] ?? res['token']; + final jwtRaw = res['sessionJwt'] ?? res['sessionToken'] ?? res['token']; + final jwt = jwtRaw?.toString(); final provider = res['provider'] as String?; final redirectTo = res['redirectTo'] as String?; + final hasJwt = jwt != null && jwt.isNotEmpty; + final nextAction = decidePasswordLoginNextAction( + hasLoginChallenge: _hasLoginChallenge, + redirectTo: redirectTo, + jwt: jwt, + ); - if (redirectTo != null && redirectTo.isNotEmpty) { - _redirectToOidcTarget(redirectTo, source: 'password_login'); - return; - } + debugPrint( + "[Auth] Password login outcome: has_login_challenge=$_hasLoginChallenge, next_action=$nextAction, has_jwt=$hasJwt", + ); - if (jwt != null) { - _onLoginSuccess(jwt, provider: provider); + switch (nextAction) { + case PasswordLoginNextAction.redirectToOidc: + _redirectToOidcTarget(redirectTo!, source: 'password_login'); + return; + case PasswordLoginNextAction.acceptOidc: + final accepted = await _acceptOidcLoginAndRedirect( + token: hasJwt ? jwt : null, + ); + if (accepted) { + return; + } + if (mounted) { + _showError(tr('msg.userfront.login.oidc_failed')); + } + return; + case PasswordLoginNextAction.localLogin: + _onLoginSuccess(jwt!, provider: provider); + return; + case PasswordLoginNextAction.invalid: + if (mounted) { + _showError(tr('msg.userfront.login.password.failed')); + } + return; } } catch (e) { if (e.toString().contains("User not registered")) { @@ -1080,20 +1147,16 @@ class _LoginScreenState extends ConsumerState debugPrint("[Auth] Failed to pre-fetch profile: $e"); } - if (_loginChallenge != null && _loginChallenge!.isNotEmpty) { + if (_hasLoginChallenge) { try { - final res = await AuthProxyService.acceptOidcLogin( - _loginChallenge!, - token: token, - ); - final redirectTo = res['redirectTo'] as String?; - if (redirectTo != null && redirectTo.isNotEmpty) { - _redirectToOidcTarget( - redirectTo, - source: 'on_login_success_accept_oidc', - ); + final accepted = await _acceptOidcLoginAndRedirect(token: token); + if (accepted) { return; } + if (mounted) { + _showError(tr('msg.userfront.login.oidc_failed')); + } + return; } catch (e) { _showError(tr('msg.userfront.login.oidc_failed')); return; diff --git a/userfront/test/oidc_redirect_guard_test.dart b/userfront/test/oidc_redirect_guard_test.dart index 70e8f868..10cb6a0c 100644 --- a/userfront/test/oidc_redirect_guard_test.dart +++ b/userfront/test/oidc_redirect_guard_test.dart @@ -5,13 +5,36 @@ void main() { group('oidc_redirect_guard', () { test('http/https 절대 URL만 허용', () { final ok = validateOidcRedirectTarget( - 'https://sso-test.hmac.kr/oidc/oauth2/auth?client_id=devfront&login_verifier=abc', + 'https://sso-test.hmac.kr/oidc/oauth2/auth?client_id=devfront&login_verifier=abc&state=xyz&code_challenge=ccc&code_challenge_method=S256&response_type=code&scope=openid%20profile&redirect_uri=http%3A%2F%2Flocalhost%3A5174%2Fcallback', ); expect(ok.isValid, isTrue); expect(ok.reason, 'ok'); + expect(ok.scheme, 'https'); expect(ok.host, 'sso-test.hmac.kr'); expect(ok.path, '/oidc/oauth2/auth'); + expect(ok.isOidcAuthPath, isTrue); + expect(ok.queryParamCount, 8); + expect( + ok.queryKeys, + containsAll(['client_id', 'login_verifier', 'state']), + ); expect(ok.hasLoginVerifier, isTrue); + expect(ok.loginVerifierLength, 3); + expect(ok.hasState, isTrue); + expect(ok.stateLength, 3); + expect(ok.hasClientId, isTrue); + expect(ok.clientId, 'devfront'); + expect(ok.hasCodeChallenge, isTrue); + expect(ok.codeChallengeLength, 3); + expect(ok.codeChallengeMethod, 'S256'); + expect(ok.hasRedirectUri, isTrue); + expect(ok.redirectUriScheme, 'http'); + expect(ok.redirectUriHost, 'localhost'); + expect(ok.redirectUriPort, 5174); + expect(ok.redirectUriPath, '/callback'); + expect(ok.responseType, 'code'); + expect(ok.scopeCount, 2); + expect(ok.toDiagnostics()['client_id'], 'devfront'); final relative = validateOidcRedirectTarget('/oidc/oauth2/auth'); expect(relative.isValid, isFalse); @@ -27,10 +50,13 @@ void main() { expect(empty.isValid, isFalse); expect(empty.reason, 'empty'); expect(empty.length, 0); + expect(empty.queryParamCount, 0); + expect(empty.hasRedirectUri, isFalse); final malformed = validateOidcRedirectTarget('https://[broken'); expect(malformed.isValid, isFalse); expect(malformed.reason, 'parse_error'); + expect(malformed.queryParamCount, 0); }); }); } diff --git a/userfront/test/password_login_flow_policy_test.dart b/userfront/test/password_login_flow_policy_test.dart new file mode 100644 index 00000000..426c5eba --- /dev/null +++ b/userfront/test/password_login_flow_policy_test.dart @@ -0,0 +1,47 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:userfront/features/auth/domain/password_login_flow_policy.dart'; + +void main() { + group('password_login_flow_policy', () { + test('redirectTo가 있으면 OIDC redirect를 우선한다', () { + final action = decidePasswordLoginNextAction( + hasLoginChallenge: true, + redirectTo: + 'https://sso-test.hmac.kr/oidc/oauth2/auth?login_verifier=a', + jwt: 'jwt-token', + ); + + expect(action, PasswordLoginNextAction.redirectToOidc); + }); + + test('OIDC challenge가 있고 redirectTo가 없으면 accept를 시도한다', () { + final action = decidePasswordLoginNextAction( + hasLoginChallenge: true, + redirectTo: null, + jwt: 'jwt-token', + ); + + expect(action, PasswordLoginNextAction.acceptOidc); + }); + + test('OIDC challenge가 없고 jwt가 있으면 로컬 로그인 완료로 진행한다', () { + final action = decidePasswordLoginNextAction( + hasLoginChallenge: false, + redirectTo: null, + jwt: 'jwt-token', + ); + + expect(action, PasswordLoginNextAction.localLogin); + }); + + test('redirectTo/jwt 모두 없으면 invalid로 처리한다', () { + final action = decidePasswordLoginNextAction( + hasLoginChallenge: false, + redirectTo: null, + jwt: null, + ); + + expect(action, PasswordLoginNextAction.invalid); + }); + }); +}