diff --git a/app/lib/features/auth/presentation/auth_failure_texts.dart b/app/lib/features/auth/presentation/auth_failure_texts.dart index 5471f57..7379891 100644 --- a/app/lib/features/auth/presentation/auth_failure_texts.dart +++ b/app/lib/features/auth/presentation/auth_failure_texts.dart @@ -36,3 +36,8 @@ String weakPasswordMessage( }; return texts.isEmpty ? l10n.authErrorWeakPasswordCharacters : texts.join(' '); } + +/// Failures the user can fix by simply trying again (offline, unexpected). +extension AuthFailureRetry on AuthFailure { + bool get isRetryable => this is NetworkFailure || this is Unexpected; +} diff --git a/app/lib/features/auth/presentation/login/login_controller.dart b/app/lib/features/auth/presentation/login/login_controller.dart index abd79f4..1ec8b93 100644 --- a/app/lib/features/auth/presentation/login/login_controller.dart +++ b/app/lib/features/auth/presentation/login/login_controller.dart @@ -60,15 +60,17 @@ class LoginController extends _$LoginController { final result = await ref .read(authRepositoryProvider) .signIn(email, state.password); - if (!ref.mounted) { - return null; - } - final failure = result.errorOrNull; if (failure == null) { - state = state.copyWith(status: LoginStatus.editing); + // Success: the target is returned even if the screen is already gone + // (the router redirect can remove it before this future completes), so + // the caller can still finish the autofill context. The status stays + // `submitting`, the form is not reused after a successful sign-in. return safeFromPath(from) ?? AppRoutes.home; } + if (!ref.mounted) { + return null; + } state = state.copyWith( status: LoginStatus.editing, bannerFailure: failure, diff --git a/app/lib/features/auth/presentation/login/login_controller.g.dart b/app/lib/features/auth/presentation/login/login_controller.g.dart index 09313c7..17285c1 100644 --- a/app/lib/features/auth/presentation/login/login_controller.g.dart +++ b/app/lib/features/auth/presentation/login/login_controller.g.dart @@ -44,7 +44,7 @@ final class LoginControllerProvider } } -String _$loginControllerHash() => r'f8c37f2b7b942c8cfffbf8b61f3296872b5bcb70'; +String _$loginControllerHash() => r'693066c260db8abe8f70d04b88b216b2e0411579'; /// Logic of the sign-in form (KTO-01 Kriterien 5, 6 und 7). diff --git a/app/lib/features/auth/presentation/login/login_screen.dart b/app/lib/features/auth/presentation/login/login_screen.dart index f5d9e2c..ef4189b 100644 --- a/app/lib/features/auth/presentation/login/login_screen.dart +++ b/app/lib/features/auth/presentation/login/login_screen.dart @@ -67,10 +67,15 @@ class _LoginScreenState extends ConsumerState { final target = await ref .read(loginControllerProvider.notifier) .submit(from: widget.from); - if (!mounted || target == null) { + if (target == null) { return; } + // Also when the router redirect (status signedIn) already removed this + // screen: the password manager must still learn that sign-in worked. TextInput.finishAutofillContext(); + if (!mounted) { + return; + } context.go(target); } @@ -136,13 +141,16 @@ class _LoginScreenState extends ConsumerState { message: l10n.authCheckEmailResent, ), ], - if (state.showsNotConfirmed && resend.failure != null) ...[ + if ((state.showsNotConfirmed, resend.failure) case ( + true, + final resendFailure?, + )) ...[ const SizedBox(height: AppSpacing.md), AppBanner( key: const Key('login_resend_error_banner'), type: AppBannerType.error, typeLabel: l10n.authBannerTypeError, - message: authFailureMessage(l10n, resend.failure!), + message: authFailureMessage(l10n, resendFailure), ), ], const SizedBox(height: AppSpacing.xl), @@ -226,7 +234,7 @@ class _FailureBanner extends StatelessWidget { @override Widget build(BuildContext context) { final l10n = context.l10n; - final retryable = failure is NetworkFailure || failure is Unexpected; + final retryable = failure.isRetryable; return AppBanner( type: AppBannerType.error, typeLabel: l10n.authBannerTypeError, diff --git a/app/lib/features/auth/presentation/login/login_state.dart b/app/lib/features/auth/presentation/login/login_state.dart index 2f42ed1..3a83202 100644 --- a/app/lib/features/auth/presentation/login/login_state.dart +++ b/app/lib/features/auth/presentation/login/login_state.dart @@ -37,6 +37,12 @@ abstract class LoginState with _$LoginState { const LoginState._(); + /// No credentials in logs or error reports. + @override + String toString() => + 'LoginState(email: , password: , status: $status, ' + 'bannerFailure: $bannerFailure)'; + bool get isSubmitting => status == LoginStatus.submitting; EmailError? get emailError => EmailFormat.validate(email); diff --git a/app/lib/features/auth/presentation/login/login_state.freezed.dart b/app/lib/features/auth/presentation/login/login_state.freezed.dart index dd0f531..3376d37 100644 --- a/app/lib/features/auth/presentation/login/login_state.freezed.dart +++ b/app/lib/features/auth/presentation/login/login_state.freezed.dart @@ -42,11 +42,6 @@ int get hashCode { return Object.hash(runtimeType,_this.email,_this.password,const DeepCollectionEquality().hash(_this.visibleFields),_this.status,_this.bannerFailure,_this.unconfirmedEmail,_this.focusField,_this.focusRequest); } -@override -String toString() { - final _this = this as LoginState; - return 'LoginState(email: ${_this.email}, password: ${_this.password}, visibleFields: ${_this.visibleFields}, status: ${_this.status}, bannerFailure: ${_this.bannerFailure}, unconfirmedEmail: ${_this.unconfirmedEmail}, focusField: ${_this.focusField}, focusRequest: ${_this.focusRequest})'; -} } @@ -278,10 +273,6 @@ int get hashCode { return Object.hash(runtimeType,email,password,const DeepCollectionEquality().hash(_visibleFields),status,bannerFailure,unconfirmedEmail,focusField,focusRequest); } -@override -String toString() { - return 'LoginState(email: $email, password: $password, visibleFields: $visibleFields, status: $status, bannerFailure: $bannerFailure, unconfirmedEmail: $unconfirmedEmail, focusField: $focusField, focusRequest: $focusRequest)'; -} } diff --git a/app/lib/features/auth/presentation/providers/auth_redirect.dart b/app/lib/features/auth/presentation/providers/auth_redirect.dart index e318d83..e79d840 100644 --- a/app/lib/features/auth/presentation/providers/auth_redirect.dart +++ b/app/lib/features/auth/presentation/providers/auth_redirect.dart @@ -69,10 +69,21 @@ String? safeFromPath(String? from) { if (from.startsWith('//') || from.startsWith(r'/\')) { return null; } + // Browsers drop tabs and line breaks inside URLs (`/\t/evil` -> `//evil`), + // so control characters are never allowed. + if (RegExp(r'[\x00-\x1f\x7f]').hasMatch(from)) { + return null; + } final uri = Uri.tryParse(from); if (uri == null || uri.hasScheme || uri.hasAuthority) { return null; } + // Percent-encoded variants (`/%2F%2Fevil`, `/%5Cevil`) must not decode to a + // protocol-relative path either. + final decoded = Uri.decodeFull(uri.path); + if (decoded.startsWith('//') || decoded.startsWith(r'/\')) { + return null; + } // `/login/` is the same page as `/login` (go_router ignores the trailing // slash), so it is not allowed either. final path = uri.path.length > 1 diff --git a/app/lib/features/auth/presentation/register/register_screen.dart b/app/lib/features/auth/presentation/register/register_screen.dart index 6b36515..032a0b9 100644 --- a/app/lib/features/auth/presentation/register/register_screen.dart +++ b/app/lib/features/auth/presentation/register/register_screen.dart @@ -319,7 +319,7 @@ class _FailureBanner extends StatelessWidget { @override Widget build(BuildContext context) { final l10n = context.l10n; - final retryable = failure is NetworkFailure || failure is Unexpected; + final retryable = failure.isRetryable; return AppBanner( type: AppBannerType.error, typeLabel: l10n.authBannerTypeError, diff --git a/app/lib/features/auth/presentation/register/register_state.dart b/app/lib/features/auth/presentation/register/register_state.dart index 0005b68..697fe3f 100644 --- a/app/lib/features/auth/presentation/register/register_state.dart +++ b/app/lib/features/auth/presentation/register/register_state.dart @@ -56,6 +56,13 @@ abstract class RegisterState with _$RegisterState { const RegisterState._(); + /// No credentials or personal data in logs or error reports. + @override + String toString() => + 'RegisterState(email: , password: , ' + 'displayName: , status: $status, ' + 'bannerFailure: $bannerFailure)'; + bool get isSubmitting => status == RegisterStatus.submitting; EmailError? get emailError => EmailFormat.validate(email); diff --git a/app/lib/features/auth/presentation/register/register_state.freezed.dart b/app/lib/features/auth/presentation/register/register_state.freezed.dart index 3051f2a..f6f37a1 100644 --- a/app/lib/features/auth/presentation/register/register_state.freezed.dart +++ b/app/lib/features/auth/presentation/register/register_state.freezed.dart @@ -44,11 +44,6 @@ int get hashCode { return Object.hash(runtimeType,_this.email,_this.password,_this.displayName,_this.ageConfirmed,_this.termsAccepted,const DeepCollectionEquality().hash(_this.visibleFields),_this.status,_this.commonPasswords,_this.bannerFailure,const DeepCollectionEquality().hash(_this.serverPasswordReasons),_this.serverEmailInvalid,_this.focusField,_this.focusRequest); } -@override -String toString() { - final _this = this as RegisterState; - return 'RegisterState(email: ${_this.email}, password: ${_this.password}, displayName: ${_this.displayName}, ageConfirmed: ${_this.ageConfirmed}, termsAccepted: ${_this.termsAccepted}, visibleFields: ${_this.visibleFields}, status: ${_this.status}, commonPasswords: ${_this.commonPasswords}, bannerFailure: ${_this.bannerFailure}, serverPasswordReasons: ${_this.serverPasswordReasons}, serverEmailInvalid: ${_this.serverEmailInvalid}, focusField: ${_this.focusField}, focusRequest: ${_this.focusRequest})'; -} } @@ -301,10 +296,6 @@ int get hashCode { return Object.hash(runtimeType,email,password,displayName,ageConfirmed,termsAccepted,const DeepCollectionEquality().hash(_visibleFields),status,commonPasswords,bannerFailure,const DeepCollectionEquality().hash(_serverPasswordReasons),serverEmailInvalid,focusField,focusRequest); } -@override -String toString() { - return 'RegisterState(email: $email, password: $password, displayName: $displayName, ageConfirmed: $ageConfirmed, termsAccepted: $termsAccepted, visibleFields: $visibleFields, status: $status, commonPasswords: $commonPasswords, bannerFailure: $bannerFailure, serverPasswordReasons: $serverPasswordReasons, serverEmailInvalid: $serverEmailInvalid, focusField: $focusField, focusRequest: $focusRequest)'; -} } diff --git a/app/test/features/auth/presentation/auth_redirect_test.dart b/app/test/features/auth/presentation/auth_redirect_test.dart index 671b194..f248b5a 100644 --- a/app/test/features/auth/presentation/auth_redirect_test.dart +++ b/app/test/features/auth/presentation/auth_redirect_test.dart @@ -150,5 +150,13 @@ void main() { expect(safeFromPath(''), isNull); expect(safeFromPath('javascript:alert(1)'), isNull); }); + + test('rejects encoded and control-character tricks', () { + expect(safeFromPath('/%2F%2Fevil.example'), isNull); + expect(safeFromPath('%2F%2Fevil'), isNull); + expect(safeFromPath('/%5Cevil.example'), isNull); + expect(safeFromPath('/\t/evil.example'), isNull); + expect(safeFromPath('/a\nb'), isNull); + }); }); } diff --git a/app/test/features/auth/presentation/login/login_controller_test.dart b/app/test/features/auth/presentation/login/login_controller_test.dart index 79c21f0..23b9be7 100644 --- a/app/test/features/auth/presentation/login/login_controller_test.dart +++ b/app/test/features/auth/presentation/login/login_controller_test.dart @@ -47,7 +47,8 @@ void main() { verify(() => repository.signIn('Anna@Example.com', 'pw')).called(1); expect(target, '/'); - expect(state().isSubmitting, isFalse); + // The form is left after success; it is not unlocked again. + expect(state().isSubmitting, isTrue); expect(state().bannerFailure, isNull); }); @@ -68,6 +69,10 @@ void main() { '/register', '/reset-password', '/auth/confirm?token_hash=x', + '/%2F%2Fevil.example', + '%2F%2Fevil', + '/\t/evil.example', + '/%5Cevil.example', '', ]) { test('ignores the unsafe from "$unsafe"', () async { @@ -123,6 +128,18 @@ void main() { }); }); + group('no credentials in logs', () { + test('toString masks email and password', () { + fill('anna@example.com', 'geheim-und-lang'); + + final text = state().toString(); + + expect(text, isNot(contains('anna@example.com'))); + expect(text, isNot(contains('geheim-und-lang'))); + expect(text, contains('')); + }); + }); + group('KTO-01 Kriterium 7: failures', () { test('invalid credentials keep the input and show the failure', () async { stubSignIn(failed(const AuthFailure.invalidCredentials())); diff --git a/app/test/features/auth/presentation/login/login_screen_test.dart b/app/test/features/auth/presentation/login/login_screen_test.dart index bbe62e8..54fe2ce 100644 --- a/app/test/features/auth/presentation/login/login_screen_test.dart +++ b/app/test/features/auth/presentation/login/login_screen_test.dart @@ -243,9 +243,13 @@ void main() { testWidgets( 'wrong email and wrong password show exactly the same screen text', (tester) async { - // The repository maps both cases to invalidCredentials (Spec 4.3); the - // screen must not add anything that tells them apart. + // Screen level only: the repository mock returns invalidCredentials + // for both cases, so this proves the screen adds nothing that tells + // them apart. The real proof that an unknown address and a wrong + // password yield the same `invalid_credentials` answer is the API + // E2E test (T3). stubSignInFailure(const AuthFailure.invalidCredentials()); + final handle = tester.ensureSemantics(); await open(tester); await type(tester, 'login_email', 'unknown@example.com'); await type(tester, 'login_password', 'irgendein-passwort'); @@ -258,22 +262,16 @@ void main() { final wrongPassword = visibleTexts(tester); expect(wrongEmail, wrongPassword); - expect(wrongEmail, contains(_de.authErrorInvalidCredentials)); - verify(() => repository.signIn(any(), any())).called(2); + // Fixed wording, independent of the l10n getter. + const fixed = 'E-Mail-Adresse oder Passwort ist nicht korrekt.'; + expect(wrongEmail, contains(fixed)); + expect(_de.authErrorInvalidCredentials, fixed); + expect(find.text(fixed), findsOneWidget); + expect(find.bySemanticsLabel('Fehler: $fixed'), findsOneWidget); + handle.dispose(); }, ); - testWidgets('english text of the generic failure', (tester) async { - stubSignInFailure(const AuthFailure.invalidCredentials()); - await open(tester, locale: const Locale('en')); - await fill(tester); - - await submit(tester); - - expect(find.text(_en.authErrorInvalidCredentials), findsOneWidget); - expect(find.text(_de.authErrorInvalidCredentials), findsNothing); - }); - testWidgets('throttling (429) shows the throttle hint', (tester) async { stubSignInFailure(const AuthFailure.rateLimited()); await open(tester); @@ -378,6 +376,99 @@ void main() { }); }); + group('KTO-01 Kriterium 5: autofill context', () { + // Only calls that save the credentials (`shouldSave: true`). + int finishCalls(WidgetTester tester) => tester.testTextInput.log + .where( + (call) => + call.method == 'TextInput.finishAutofillContext' && + call.arguments == true, + ) + .length; + + testWidgets('is finished after a successful sign-in', (tester) async { + await open(tester); + await fill(tester); + tester.testTextInput.log.clear(); + + await submit(tester); + + // Our explicit call, plus the AutofillGroup committing on disposal + // when the screen is replaced. + expect(finishCalls(tester), greaterThanOrEqualTo(1)); + }); + + testWidgets('is finished even if the router already removed the screen', ( + tester, + ) async { + final completer = Completer>(); + when(() => repository.signIn(any(), any())) + .thenAnswer((_) => completer.future); + final (:container, :router) = await open(tester); + await fill(tester); + await tapKey(tester, 'login_submit'); + // The status change makes the router redirect away before the + // sign-in future completes. + router.go('/private'); + await tester.pumpAndSettle(); + expect(find.byKey(const Key('login_screen')), findsNothing); + tester.testTextInput.log.clear(); + + completer.complete(ok()); + await tester.pumpAndSettle(); + + expect(finishCalls(tester), 1); + expect(currentPath(router), '/private'); + }); + + testWidgets('is not finished after a failed sign-in', (tester) async { + stubSignInFailure(const AuthFailure.invalidCredentials()); + await open(tester); + await fill(tester); + tester.testTextInput.log.clear(); + + await submit(tester); + + expect(finishCalls(tester), 0); + }); + }); + + group('Zustände Kriterium 3: banners and focus', () { + testWidgets('unexpected error shows the banner with retry', (tester) async { + stubSignInFailure(const AuthFailure.unexpected()); + await open(tester); + await fill(tester); + + await submit(tester); + + expect(find.text(_de.authErrorUnexpected), findsOneWidget); + expect(find.text(_de.authActionRetry), findsOneWidget); + stubSignInFailure(const AuthFailure.unexpected()); + await tester.tap(find.text(_de.authActionRetry)); + await tester.pumpAndSettle(); + verify(() => repository.signIn(any(), any())).called(2); + }); + + testWidgets('a failed submit focuses the first invalid field', ( + tester, + ) async { + await open(tester); + + await submit(tester); + expect( + tester.widget(fieldOf('login_email')).focusNode!.hasFocus, + isTrue, + ); + + await type(tester, 'login_email', 'anna@example.com'); + await submit(tester); + expect( + tester.widget(fieldOf('login_password')).focusNode!.hasFocus, + isTrue, + ); + }); + }); + group('QUE-05 Kriterium 2: de and en', () { testWidgets('english texts', (tester) async { await open(tester, locale: const Locale('en')); @@ -405,6 +496,23 @@ void main() { }); } + for (final mode in [ThemeMode.light, ThemeMode.dark]) { + testWidgets('error banner meets the contrast guideline ($mode)', ( + tester, + ) async { + final handle = tester.ensureSemantics(); + stubSignInFailure(const AuthFailure.invalidCredentials()); + await open(tester, themeMode: mode); + await fill(tester); + await submit(tester); + expect(find.byKey(const Key('login_banner')), findsOneWidget); + + await expectLater(tester, meetsGuideline(textContrastGuideline)); + await expectLater(tester, meetsGuideline(androidTapTargetGuideline)); + handle.dispose(); + }); + } + testWidgets('the title is a heading and the banner a live region', ( tester, ) async { @@ -416,7 +524,7 @@ void main() { expect( find.bySemanticsLabel( - '${_de.authBannerTypeError}: ${_de.authErrorInvalidCredentials}', + 'Fehler: E-Mail-Adresse oder Passwort ist nicht korrekt.', ), findsOneWidget, ); diff --git a/app/test/features/auth/presentation/register/register_controller_test.dart b/app/test/features/auth/presentation/register/register_controller_test.dart index ddc32e0..1eb6ed0 100644 --- a/app/test/features/auth/presentation/register/register_controller_test.dart +++ b/app/test/features/auth/presentation/register/register_controller_test.dart @@ -71,6 +71,24 @@ void main() { ..termsAcceptedChanged(value: true); } + group('no personal data in logs', () { + test('toString masks email, password and display name', () { + final container = makeContainer(); + fillValid( + container.read(registerControllerProvider.notifier), + email: 'anna@example.com', + name: 'Anna Geheim', + ); + + final text = container.read(registerControllerProvider).toString(); + + expect(text, isNot(contains('anna@example.com'))); + expect(text, isNot(contains('geheim-und-lang'))); + expect(text, isNot(contains('Anna Geheim'))); + expect(text, contains('')); + }); + }); + group('KTO-01 Kriterium 1: submit', () { test('KTO-01 Kriterium 11 (Client): sends all data, terms version and locale to signUp', () async { final container = makeContainer(locale: const Locale('en'));