fix(auth): address review findings for password reset
Finish the recovery guard only after the password change, mark the change in the repository so a late userUpdated event is harmless, keep the form locked after success, carry from through the reset check-email page, share the password error texts and focus the continue button. Refs #1 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
3b2995463e
commit
7c2f684098
22 changed files with 383 additions and 97 deletions
|
|
@ -17,6 +17,7 @@ class AppPrimaryButton extends StatelessWidget {
|
|||
this.loadingLabel,
|
||||
this.icon,
|
||||
this.expand = true,
|
||||
this.autofocus = false,
|
||||
});
|
||||
|
||||
final String label;
|
||||
|
|
@ -26,6 +27,10 @@ class AppPrimaryButton extends StatelessWidget {
|
|||
final IconData? icon;
|
||||
final bool expand;
|
||||
|
||||
/// Takes the focus when it appears (e.g. after a view switch removed the
|
||||
/// previously focused control).
|
||||
final bool autofocus;
|
||||
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
final enabled = onPressed != null && !isLoading;
|
||||
|
|
@ -43,6 +48,7 @@ class AppPrimaryButton extends StatelessWidget {
|
|||
: null;
|
||||
|
||||
final button = FilledButton(
|
||||
autofocus: autofocus,
|
||||
onPressed: enabled ? onPressed : null,
|
||||
child: Row(
|
||||
mainAxisSize: expand ? MainAxisSize.max : MainAxisSize.min,
|
||||
|
|
|
|||
|
|
@ -59,9 +59,24 @@ class AuthStatusTracker {
|
|||
return statusFor(session);
|
||||
}
|
||||
|
||||
/// Ends the recovery after the user confirmed the success message.
|
||||
/// The password was set during a running recovery (successful
|
||||
/// `updateUser`). Idempotent to the later `userUpdated` event, which gotrue
|
||||
/// delivers asynchronously, possibly after `updateUser` returned. Without a
|
||||
/// running recovery nothing changes.
|
||||
AuthStatus markPasswordChanged(Session? session) {
|
||||
if (_recovery) {
|
||||
_passwordChanged = true;
|
||||
}
|
||||
return statusFor(session);
|
||||
}
|
||||
|
||||
/// Ends the recovery after the user confirmed the success message. Only
|
||||
/// possible once the password is changed; before that the status stays
|
||||
/// unchanged (the guard cannot be skipped).
|
||||
AuthStatus finishRecovery(Session? session) {
|
||||
_end();
|
||||
if (_recovery && _passwordChanged) {
|
||||
_end();
|
||||
}
|
||||
return statusFor(session);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -123,14 +123,18 @@ class SupabaseAuthRepository implements AuthRepository {
|
|||
});
|
||||
|
||||
@override
|
||||
Future<Result<void, AuthFailure>> setNewPassword(String password) => _guard(
|
||||
AuthOperation.updatePassword,
|
||||
() => _auth.updateUser(UserAttributes(password: password)),
|
||||
);
|
||||
Future<Result<void, AuthFailure>> setNewPassword(String password) =>
|
||||
_guard(AuthOperation.updatePassword, () async {
|
||||
await _auth.updateUser(UserAttributes(password: password));
|
||||
// gotrue delivers `userUpdated` asynchronously, possibly after this
|
||||
// call returned. Mark the change here as well (idempotent), so the
|
||||
// status never lags behind the successful result.
|
||||
_publish(_tracker.markPasswordChanged(_auth.currentSession));
|
||||
});
|
||||
|
||||
@override
|
||||
void finishPasswordRecovery() {
|
||||
if (_lastStatus is PasswordRecovery) {
|
||||
if (_lastStatus case PasswordRecovery(passwordChanged: true)) {
|
||||
_publish(_tracker.finishRecovery(_auth.currentSession));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -29,8 +29,9 @@ abstract interface class AuthRepository {
|
|||
|
||||
Future<Result<void, AuthFailure>> setNewPassword(String password);
|
||||
|
||||
/// Beendet den Recovery-Guard nach erfolgreich gesetztem Passwort (die
|
||||
/// Sitzung bleibt bestehen). Ohne laufende Recovery ohne Wirkung.
|
||||
/// Beendet den Recovery-Guard, nachdem das neue Passwort gesetzt ist (die
|
||||
/// Sitzung bleibt bestehen). Ohne laufende Recovery oder vor dem
|
||||
/// Passwortwechsel ohne Wirkung: Der Guard lässt sich nicht überspringen.
|
||||
void finishPasswordRecovery();
|
||||
|
||||
/// Meldet lokal ab (u. a. beim Abbruch der Recovery). Fehler werden nur
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import 'package:shuffle/core/l10n/l10n.dart';
|
||||
import 'package:shuffle/features/auth/domain/auth_failure.dart';
|
||||
import 'package:shuffle/features/auth/domain/validation/password_policy.dart';
|
||||
|
||||
/// User-facing text of an [AuthFailure] (Spec #1 § 4.4, texts from
|
||||
/// `docs/design/auth-screens.md`).
|
||||
|
|
@ -41,3 +42,43 @@ String weakPasswordMessage(
|
|||
extension AuthFailureRetry on AuthFailure {
|
||||
bool get isRetryable => this is NetworkFailure || this is Unexpected;
|
||||
}
|
||||
|
||||
/// Text of the password field error, shared by registration and reset.
|
||||
///
|
||||
/// Order: server answer, `same_password`, "required", then the local policy
|
||||
/// [error] (only passed when it should be shown).
|
||||
String? passwordFieldError(
|
||||
AppLocalizations l10n, {
|
||||
required PasswordError? error,
|
||||
required bool required,
|
||||
List<WeakPasswordReason>? serverReasons,
|
||||
bool serverSamePassword = false,
|
||||
}) {
|
||||
if (serverReasons != null) {
|
||||
return weakPasswordMessage(l10n, serverReasons);
|
||||
}
|
||||
if (serverSamePassword) {
|
||||
return l10n.authErrorSamePassword;
|
||||
}
|
||||
if (required) {
|
||||
return l10n.authPasswordRequired;
|
||||
}
|
||||
return switch (error) {
|
||||
PasswordTooShort(:final missing) => l10n.authPasswordTooShort(missing),
|
||||
PasswordCommon() => l10n.authPasswordCommon,
|
||||
null => null,
|
||||
};
|
||||
}
|
||||
|
||||
/// The only message shown is the "n missing" count, which changes with every
|
||||
/// keystroke: screens announce it only when the field is left.
|
||||
bool isPasswordLengthHintOnly({
|
||||
required PasswordError? error,
|
||||
required bool required,
|
||||
List<WeakPasswordReason>? serverReasons,
|
||||
bool serverSamePassword = false,
|
||||
}) =>
|
||||
serverReasons == null &&
|
||||
!serverSamePassword &&
|
||||
!required &&
|
||||
error is PasswordTooShort;
|
||||
|
|
|
|||
|
|
@ -41,8 +41,10 @@ final List<RouteBase> authRoutes = [
|
|||
GoRoute(
|
||||
path: 'check-email',
|
||||
// The mode is part of the path, so it survives a web reload.
|
||||
builder: (context, state) =>
|
||||
const CheckEmailScreen(mode: CheckEmailMode.reset),
|
||||
builder: (context, state) => CheckEmailScreen(
|
||||
mode: CheckEmailMode.reset,
|
||||
from: state.uri.queryParameters[AppRoutes.fromParam],
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import 'package:shuffle/core/widgets/app_text_field.dart';
|
|||
import 'package:shuffle/features/auth/domain/resend_origin.dart';
|
||||
import 'package:shuffle/features/auth/presentation/auth_failure_texts.dart';
|
||||
import 'package:shuffle/features/auth/presentation/check_email/resend_controller.dart';
|
||||
import 'package:shuffle/features/auth/presentation/providers/auth_redirect.dart';
|
||||
import 'package:shuffle/features/auth/presentation/widgets/auth_screen_title.dart';
|
||||
|
||||
/// Which flow the "check your inbox" page belongs to. The mode comes from the
|
||||
|
|
@ -35,10 +36,14 @@ class CheckEmailScreen extends ConsumerStatefulWidget {
|
|||
const CheckEmailScreen({
|
||||
this.email,
|
||||
this.mode = CheckEmailMode.confirmation,
|
||||
this.from,
|
||||
super.key,
|
||||
});
|
||||
|
||||
final String? email;
|
||||
|
||||
/// Unchecked `from` query parameter, carried back to the sign-in page.
|
||||
final String? from;
|
||||
final CheckEmailMode mode;
|
||||
|
||||
@override
|
||||
|
|
@ -138,7 +143,9 @@ class _CheckEmailScreenState extends ConsumerState<CheckEmailScreen> {
|
|||
if (_isReset)
|
||||
TextButton(
|
||||
key: const Key('check_email_different_address'),
|
||||
onPressed: () => context.go(AppRoutes.forgotPassword),
|
||||
onPressed: () => context.go(
|
||||
authLocation(AppRoutes.forgotPassword, from: widget.from),
|
||||
),
|
||||
child: Text(l10n.authCheckEmailDifferentAddress),
|
||||
)
|
||||
else
|
||||
|
|
@ -152,7 +159,8 @@ class _CheckEmailScreenState extends ConsumerState<CheckEmailScreen> {
|
|||
const SizedBox(height: AppSpacing.sm),
|
||||
TextButton(
|
||||
key: const Key('check_email_back_to_login'),
|
||||
onPressed: () => context.go(AppRoutes.login),
|
||||
onPressed: () =>
|
||||
context.go(authLocation(AppRoutes.login, from: widget.from)),
|
||||
child: Text(l10n.authBackToLogin),
|
||||
),
|
||||
],
|
||||
|
|
|
|||
|
|
@ -57,7 +57,9 @@ class _ForgotPasswordScreenState extends ConsumerState<ForgotPasswordScreen> {
|
|||
if (!succeeded || !mounted) {
|
||||
return;
|
||||
}
|
||||
context.go(AppRoutes.forgotPasswordCheckEmail);
|
||||
context.go(
|
||||
authLocation(AppRoutes.forgotPasswordCheckEmail, from: widget.from),
|
||||
);
|
||||
}
|
||||
|
||||
@override
|
||||
|
|
|
|||
|
|
@ -17,7 +17,12 @@ const _guestOnlyPaths = {
|
|||
};
|
||||
|
||||
/// Pfade, die nie als `from`-Ziel zugelassen sind (Schleifenschutz).
|
||||
const _neverFromPaths = {..._guestOnlyPaths, AppRoutes.resetPassword};
|
||||
const _neverFromPaths = {
|
||||
..._guestOnlyPaths,
|
||||
AppRoutes.resetPassword,
|
||||
AppRoutes.registerCheckEmail,
|
||||
AppRoutes.forgotPasswordCheckEmail,
|
||||
};
|
||||
|
||||
/// Redirect-Regeln der Auth-Story (Spec #1 § 5.3), rein und ohne Zustand.
|
||||
///
|
||||
|
|
|
|||
|
|
@ -16,7 +16,6 @@ import 'package:shuffle/core/widgets/app_text_field.dart';
|
|||
import 'package:shuffle/features/auth/domain/auth_failure.dart';
|
||||
import 'package:shuffle/features/auth/domain/validation/display_name_rule.dart';
|
||||
import 'package:shuffle/features/auth/domain/validation/email_format.dart';
|
||||
import 'package:shuffle/features/auth/domain/validation/password_policy.dart';
|
||||
import 'package:shuffle/features/auth/presentation/auth_failure_texts.dart';
|
||||
import 'package:shuffle/features/auth/presentation/providers/auth_redirect.dart';
|
||||
import 'package:shuffle/features/auth/presentation/register/register_controller.dart';
|
||||
|
|
@ -281,26 +280,19 @@ class _RegisterScreenState extends ConsumerState<RegisterScreen> {
|
|||
return null;
|
||||
}
|
||||
|
||||
/// The only password message shown is the "n missing" count (no server
|
||||
/// answer, not "required").
|
||||
bool _isLengthHintOnly(RegisterState state) =>
|
||||
state.serverPasswordReasons == null &&
|
||||
!state.shownPasswordRequired &&
|
||||
state.shownPasswordError is PasswordTooShort;
|
||||
bool _isLengthHintOnly(RegisterState state) => isPasswordLengthHintOnly(
|
||||
error: state.shownPasswordError,
|
||||
required: state.shownPasswordRequired,
|
||||
serverReasons: state.serverPasswordReasons,
|
||||
);
|
||||
|
||||
String? _passwordError(AppLocalizations l10n, RegisterState state) {
|
||||
if (state.serverPasswordReasons case final reasons?) {
|
||||
return weakPasswordMessage(l10n, reasons);
|
||||
}
|
||||
if (state.shownPasswordRequired) {
|
||||
return l10n.authPasswordRequired;
|
||||
}
|
||||
return switch (state.shownPasswordError) {
|
||||
PasswordTooShort(:final missing) => l10n.authPasswordTooShort(missing),
|
||||
PasswordCommon() => l10n.authPasswordCommon,
|
||||
null => null,
|
||||
};
|
||||
}
|
||||
String? _passwordError(AppLocalizations l10n, RegisterState state) =>
|
||||
passwordFieldError(
|
||||
l10n,
|
||||
error: state.shownPasswordError,
|
||||
required: state.shownPasswordRequired,
|
||||
serverReasons: state.serverPasswordReasons,
|
||||
);
|
||||
|
||||
String? _displayNameError(AppLocalizations l10n, RegisterState state) =>
|
||||
switch (state.shownDisplayNameError) {
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ class ResetPasswordController extends _$ResetPasswordController {
|
|||
@override
|
||||
ResetPasswordState build() => const ResetPasswordState();
|
||||
|
||||
void passwordChanged(String value) {
|
||||
void onPasswordInput(String value) {
|
||||
// The password is checked while typing (Kriterium 3).
|
||||
state = state.copyWith(
|
||||
password: value,
|
||||
|
|
@ -99,11 +99,14 @@ class ResetPasswordController extends _$ResetPasswordController {
|
|||
if (!ref.mounted) {
|
||||
return false;
|
||||
}
|
||||
state = state.copyWith(status: ResetPasswordStatus.editing);
|
||||
final failure = result.errorOrNull;
|
||||
if (failure == null) {
|
||||
// The form stays locked: the auth status moves to "password changed"
|
||||
// and the screen switches to the success view. No window in which the
|
||||
// form could be used again.
|
||||
return true;
|
||||
}
|
||||
state = state.copyWith(status: ResetPasswordStatus.editing);
|
||||
switch (failure) {
|
||||
case WeakPassword(:final reasons):
|
||||
state = state.copyWith(
|
||||
|
|
|
|||
|
|
@ -101,7 +101,7 @@ final class ResetPasswordControllerProvider
|
|||
}
|
||||
|
||||
String _$resetPasswordControllerHash() =>
|
||||
r'ab273bab468ed730052be4877484943ad49f6e15';
|
||||
r'ba0b2b07e134ce80fd75c1b8d7c4f66034bb73c3';
|
||||
|
||||
/// Logic of the "new password" form (KTO-01 Kriterium 3, 8 und 9).
|
||||
///
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ import 'package:shuffle/core/widgets/app_banner.dart';
|
|||
import 'package:shuffle/core/widgets/app_form_scaffold.dart';
|
||||
import 'package:shuffle/core/widgets/app_password_field.dart';
|
||||
import 'package:shuffle/core/widgets/app_primary_button.dart';
|
||||
import 'package:shuffle/features/auth/domain/validation/password_policy.dart';
|
||||
import 'package:shuffle/features/auth/presentation/auth_failure_texts.dart';
|
||||
import 'package:shuffle/features/auth/presentation/reset_password/reset_password_controller.dart';
|
||||
import 'package:shuffle/features/auth/presentation/reset_password/reset_password_state.dart';
|
||||
|
|
@ -108,6 +107,7 @@ class _ResetPasswordScreenState extends ConsumerState<ResetPasswordScreen> {
|
|||
const SizedBox(height: AppSpacing.xl),
|
||||
AppPrimaryButton(
|
||||
key: const Key('reset_password_continue'),
|
||||
autofocus: true,
|
||||
label: l10n.authConfirmContinue,
|
||||
onPressed: _continue,
|
||||
),
|
||||
|
|
@ -156,7 +156,7 @@ class _ResetPasswordScreenState extends ConsumerState<ResetPasswordScreen> {
|
|||
if (!_typing) {
|
||||
setState(() => _typing = true);
|
||||
}
|
||||
controller.passwordChanged(value);
|
||||
controller.onPasswordInput(value);
|
||||
},
|
||||
onSubmitted: (_) {
|
||||
if (!busy) {
|
||||
|
|
@ -182,26 +182,19 @@ class _ResetPasswordScreenState extends ConsumerState<ResetPasswordScreen> {
|
|||
];
|
||||
}
|
||||
|
||||
bool _isLengthHintOnly(ResetPasswordState state) =>
|
||||
state.serverPasswordReasons == null &&
|
||||
!state.serverSamePassword &&
|
||||
!state.shownPasswordRequired &&
|
||||
state.shownPasswordError is PasswordTooShort;
|
||||
bool _isLengthHintOnly(ResetPasswordState state) => isPasswordLengthHintOnly(
|
||||
error: state.shownPasswordError,
|
||||
required: state.shownPasswordRequired,
|
||||
serverReasons: state.serverPasswordReasons,
|
||||
serverSamePassword: state.serverSamePassword,
|
||||
);
|
||||
|
||||
String? _passwordError(AppLocalizations l10n, ResetPasswordState state) {
|
||||
if (state.serverPasswordReasons case final reasons?) {
|
||||
return weakPasswordMessage(l10n, reasons);
|
||||
}
|
||||
if (state.serverSamePassword) {
|
||||
return l10n.authErrorSamePassword;
|
||||
}
|
||||
if (state.shownPasswordRequired) {
|
||||
return l10n.authPasswordRequired;
|
||||
}
|
||||
return switch (state.shownPasswordError) {
|
||||
PasswordTooShort(:final missing) => l10n.authPasswordTooShort(missing),
|
||||
PasswordCommon() => l10n.authPasswordCommon,
|
||||
null => null,
|
||||
};
|
||||
}
|
||||
String? _passwordError(AppLocalizations l10n, ResetPasswordState state) =>
|
||||
passwordFieldError(
|
||||
l10n,
|
||||
error: state.shownPasswordError,
|
||||
required: state.shownPasswordRequired,
|
||||
serverReasons: state.serverPasswordReasons,
|
||||
serverSamePassword: state.serverSamePassword,
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -113,6 +113,43 @@ void main() {
|
|||
expect(tracker.statusFor(testSession()), isA<SignedIn>());
|
||||
});
|
||||
|
||||
test('finishRecovery before the password change keeps the guard', () {
|
||||
final tracker = AuthStatusTracker()
|
||||
..handle(AuthChangeEvent.passwordRecovery, testSession());
|
||||
final status = tracker.finishRecovery(testSession());
|
||||
expect(status, isA<PasswordRecovery>());
|
||||
expect((status as PasswordRecovery).passwordChanged, isFalse);
|
||||
expect(tracker.statusFor(testSession()), isA<PasswordRecovery>());
|
||||
});
|
||||
|
||||
test('markPasswordChanged marks a running recovery (no event needed)', () {
|
||||
final tracker = AuthStatusTracker()
|
||||
..handle(AuthChangeEvent.passwordRecovery, testSession());
|
||||
expect(
|
||||
tracker.markPasswordChanged(testSession()),
|
||||
isA<PasswordRecovery>().having(
|
||||
(s) => s.passwordChanged,
|
||||
'changed',
|
||||
true,
|
||||
),
|
||||
);
|
||||
// A late userUpdated is idempotent.
|
||||
expect(
|
||||
tracker.handle(AuthChangeEvent.userUpdated, testSession()),
|
||||
isA<PasswordRecovery>().having(
|
||||
(s) => s.passwordChanged,
|
||||
'changed',
|
||||
true,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('markPasswordChanged without a recovery changes nothing', () {
|
||||
final tracker = AuthStatusTracker()
|
||||
..handle(AuthChangeEvent.signedIn, testSession());
|
||||
expect(tracker.markPasswordChanged(testSession()), isA<SignedIn>());
|
||||
});
|
||||
|
||||
test('a new recovery resets the passwordChanged mark', () {
|
||||
final tracker = AuthStatusTracker()
|
||||
..handle(AuthChangeEvent.passwordRecovery, testSession())
|
||||
|
|
|
|||
|
|
@ -365,6 +365,78 @@ void main() {
|
|||
expect(repository.currentStatus, isA<SignedIn>());
|
||||
});
|
||||
|
||||
test(
|
||||
'finishPasswordRecovery before the password change keeps the guard',
|
||||
() {
|
||||
currentSession = testSession();
|
||||
events.add(AuthState(AuthChangeEvent.passwordRecovery, testSession()));
|
||||
|
||||
repository.finishPasswordRecovery();
|
||||
|
||||
expect(
|
||||
repository.currentStatus,
|
||||
isA<PasswordRecovery>().having(
|
||||
(s) => s.passwordChanged,
|
||||
'changed',
|
||||
false,
|
||||
),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'setNewPassword marks the change even if userUpdated comes late',
|
||||
() async {
|
||||
currentSession = testSession();
|
||||
events.add(AuthState(AuthChangeEvent.passwordRecovery, testSession()));
|
||||
when(
|
||||
() => auth.updateUser(any()),
|
||||
).thenAnswer((_) async => UserResponse.fromJson(testUser().toJson()));
|
||||
|
||||
final result = await repository.setNewPassword('neues-passwort');
|
||||
|
||||
// No userUpdated event has arrived yet.
|
||||
expect(result.isSuccess, isTrue);
|
||||
expect(
|
||||
repository.currentStatus,
|
||||
isA<PasswordRecovery>().having(
|
||||
(s) => s.passwordChanged,
|
||||
'changed',
|
||||
true,
|
||||
),
|
||||
);
|
||||
// The late event changes nothing.
|
||||
events.add(AuthState(AuthChangeEvent.userUpdated, testSession()));
|
||||
expect(
|
||||
repository.currentStatus,
|
||||
isA<PasswordRecovery>().having(
|
||||
(s) => s.passwordChanged,
|
||||
'changed',
|
||||
true,
|
||||
),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
test('a failed setNewPassword does not mark the change', () async {
|
||||
currentSession = testSession();
|
||||
events.add(AuthState(AuthChangeEvent.passwordRecovery, testSession()));
|
||||
when(() => auth.updateUser(any())).thenThrow(
|
||||
const AuthApiException('x', statusCode: '422', code: 'same_password'),
|
||||
);
|
||||
|
||||
await repository.setNewPassword('neues-passwort');
|
||||
|
||||
expect(
|
||||
repository.currentStatus,
|
||||
isA<PasswordRecovery>().having(
|
||||
(s) => s.passwordChanged,
|
||||
'changed',
|
||||
false,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('finishPasswordRecovery without a recovery changes nothing', () {
|
||||
currentSession = testSession();
|
||||
events.add(AuthState(AuthChangeEvent.signedIn, testSession()));
|
||||
|
|
|
|||
|
|
@ -76,6 +76,8 @@ void main() {
|
|||
'private',
|
||||
'/login',
|
||||
'/reset-password',
|
||||
'/register/check-email',
|
||||
'/forgot-password/check-email',
|
||||
'/auth/confirm',
|
||||
]) {
|
||||
expect(
|
||||
|
|
|
|||
|
|
@ -222,7 +222,7 @@ void main() {
|
|||
});
|
||||
|
||||
group('KTO-01 Kriterium 8, 9: password reset flow through the router', () {
|
||||
void stubGoTrue() {
|
||||
void stubGoTrue({bool lateUserUpdated = false}) {
|
||||
registerFallbackValue(OtpType.recovery);
|
||||
registerFallbackValue(UserAttributes());
|
||||
when(
|
||||
|
|
@ -236,8 +236,17 @@ void main() {
|
|||
return AuthResponse(session: testSession());
|
||||
});
|
||||
when(() => mocks.auth.updateUser(any())).thenAnswer((_) async {
|
||||
// gotrue emits userUpdated before updateUser returns.
|
||||
emit(AuthChangeEvent.userUpdated, withSession: testSession());
|
||||
if (lateUserUpdated) {
|
||||
// gotrue delivers the event asynchronously, after the return.
|
||||
unawaited(
|
||||
Future.microtask(
|
||||
() =>
|
||||
emit(AuthChangeEvent.userUpdated, withSession: testSession()),
|
||||
),
|
||||
);
|
||||
} else {
|
||||
emit(AuthChangeEvent.userUpdated, withSession: testSession());
|
||||
}
|
||||
return UserResponse.fromJson(testUser().toJson());
|
||||
});
|
||||
when(() => mocks.auth.signOut()).thenAnswer((_) async {
|
||||
|
|
@ -308,6 +317,31 @@ void main() {
|
|||
verifyNever(() => mocks.auth.signOut());
|
||||
});
|
||||
|
||||
testWidgets(
|
||||
'userUpdated after the return: same result, form never reusable',
|
||||
(tester) async {
|
||||
stubGoTrue(lateUserUpdated: true);
|
||||
final (container: _, :router) = await start(tester);
|
||||
await redeemRecoveryLink(tester, router);
|
||||
|
||||
await savePassword(tester, 'ein-langes-passwort');
|
||||
|
||||
expect(currentPath(router), '/reset-password');
|
||||
expect(
|
||||
find.byKey(const Key('reset_password_success_banner')),
|
||||
findsOneWidget,
|
||||
);
|
||||
expect(find.byKey(const Key('reset_password_submit')), findsNothing);
|
||||
router.go('/');
|
||||
await tester.pumpAndSettle();
|
||||
expect(currentPath(router), '/reset-password');
|
||||
|
||||
await tester.tap(find.byKey(const Key('reset_password_continue')));
|
||||
await tester.pumpAndSettle();
|
||||
expect(currentPath(router), '/');
|
||||
},
|
||||
);
|
||||
|
||||
testWidgets('cancel signs out and leads to /login', (tester) async {
|
||||
stubGoTrue();
|
||||
final (container: _, :router) = await start(tester);
|
||||
|
|
|
|||
|
|
@ -210,6 +210,33 @@ void main() {
|
|||
expect(currentUri(router).toString(), '/login?from=%2Fprivate%3Fa%3D1');
|
||||
});
|
||||
|
||||
testWidgets('from survives the check-your-inbox page and comes back', (
|
||||
tester,
|
||||
) async {
|
||||
final (:container, :router) = await open(
|
||||
tester,
|
||||
location: '/forgot-password?from=%2Fprivate',
|
||||
);
|
||||
|
||||
await send(tester, 'anna@example.com');
|
||||
expect(
|
||||
currentUri(router).toString(),
|
||||
'/forgot-password/check-email?from=%2Fprivate',
|
||||
);
|
||||
|
||||
await tester.tap(find.byKey(const Key('check_email_different_address')));
|
||||
await tester.pumpAndSettle();
|
||||
expect(currentUri(router).toString(), '/forgot-password?from=%2Fprivate');
|
||||
|
||||
await send(tester, 'anna@example.com');
|
||||
await tester.ensureVisible(
|
||||
find.byKey(const Key('check_email_back_to_login')),
|
||||
);
|
||||
await tester.tap(find.byKey(const Key('check_email_back_to_login')));
|
||||
await tester.pumpAndSettle();
|
||||
expect(currentUri(router).toString(), '/login?from=%2Fprivate');
|
||||
});
|
||||
|
||||
testWidgets('an unsafe from is dropped', (tester) async {
|
||||
final (:container, :router) = await open(
|
||||
tester,
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ void main() {
|
|||
|
||||
group('KTO-01 Kriterium 3: password policy', () {
|
||||
test('too short is rejected locally and shows the missing count', () async {
|
||||
notifier().passwordChanged('kurz');
|
||||
notifier().onPasswordInput('kurz');
|
||||
|
||||
expect(await notifier().submit(), isFalse);
|
||||
|
||||
|
|
@ -56,7 +56,7 @@ void main() {
|
|||
|
||||
test('exactly 10 characters pass', () async {
|
||||
stubSet(ok());
|
||||
notifier().passwordChanged('1234567890');
|
||||
notifier().onPasswordInput('1234567890');
|
||||
|
||||
expect(await notifier().submit(), isTrue);
|
||||
|
||||
|
|
@ -66,7 +66,7 @@ void main() {
|
|||
test(
|
||||
'a common password is rejected (the list loads before submit)',
|
||||
() async {
|
||||
notifier().passwordChanged('Passwort1234');
|
||||
notifier().onPasswordInput('Passwort1234');
|
||||
|
||||
expect(await notifier().submit(), isFalse);
|
||||
|
||||
|
|
@ -83,13 +83,13 @@ void main() {
|
|||
|
||||
test('the error shows while typing and after blur', () {
|
||||
expect(state().shownPasswordError, isNull);
|
||||
notifier().passwordChanged('abc');
|
||||
notifier().onPasswordInput('abc');
|
||||
expect(state().shownPasswordError, isNotNull);
|
||||
});
|
||||
|
||||
test('the password is sent unchanged (no trimming)', () async {
|
||||
stubSet(ok());
|
||||
notifier().passwordChanged(' zehn zeichen ');
|
||||
notifier().onPasswordInput(' zehn zeichen ');
|
||||
|
||||
await notifier().submit();
|
||||
|
||||
|
|
@ -102,7 +102,7 @@ void main() {
|
|||
'success returns true and keeps the form locked from re-submits',
|
||||
() async {
|
||||
stubSet(ok());
|
||||
notifier().passwordChanged('ein-langes-passwort');
|
||||
notifier().onPasswordInput('ein-langes-passwort');
|
||||
|
||||
expect(await notifier().submit(), isTrue);
|
||||
|
||||
|
|
@ -115,25 +115,25 @@ void main() {
|
|||
stubSet(
|
||||
failed(const AuthFailure.weakPassword([WeakPasswordReason.length])),
|
||||
);
|
||||
notifier().passwordChanged('ein-langes-passwort');
|
||||
notifier().onPasswordInput('ein-langes-passwort');
|
||||
|
||||
expect(await notifier().submit(), isFalse);
|
||||
|
||||
expect(state().serverPasswordReasons, [WeakPasswordReason.length]);
|
||||
expect(state().bannerFailure, isNull);
|
||||
notifier().passwordChanged('ein-anderes-passwort');
|
||||
notifier().onPasswordInput('ein-anderes-passwort');
|
||||
expect(state().serverPasswordReasons, isNull);
|
||||
});
|
||||
|
||||
test('same_password is shown at the field', () async {
|
||||
stubSet(failed(const AuthFailure.samePassword()));
|
||||
notifier().passwordChanged('ein-langes-passwort');
|
||||
notifier().onPasswordInput('ein-langes-passwort');
|
||||
|
||||
expect(await notifier().submit(), isFalse);
|
||||
|
||||
expect(state().serverSamePassword, isTrue);
|
||||
expect(state().bannerFailure, isNull);
|
||||
notifier().passwordChanged('ein-anderes-passwort');
|
||||
notifier().onPasswordInput('ein-anderes-passwort');
|
||||
expect(state().serverSamePassword, isFalse);
|
||||
});
|
||||
|
||||
|
|
@ -144,7 +144,7 @@ void main() {
|
|||
]) {
|
||||
test('${failure.runtimeType} is a banner and keeps the input', () async {
|
||||
stubSet(failed(failure));
|
||||
notifier().passwordChanged('ein-langes-passwort');
|
||||
notifier().onPasswordInput('ein-langes-passwort');
|
||||
|
||||
expect(await notifier().submit(), isFalse);
|
||||
|
||||
|
|
@ -158,7 +158,7 @@ void main() {
|
|||
final completer = Completer<Result<void, AuthFailure>>();
|
||||
when(() => repository.setNewPassword(any()))
|
||||
.thenAnswer((_) => completer.future);
|
||||
notifier().passwordChanged('ein-langes-passwort');
|
||||
notifier().onPasswordInput('ein-langes-passwort');
|
||||
|
||||
final first = notifier().submit();
|
||||
expect(await notifier().submit(), isFalse);
|
||||
|
|
@ -208,7 +208,7 @@ void main() {
|
|||
});
|
||||
|
||||
test('toString masks the password', () {
|
||||
notifier().passwordChanged('geheimes-passwort');
|
||||
notifier().onPasswordInput('geheimes-passwort');
|
||||
|
||||
final text = state().toString();
|
||||
|
||||
|
|
|
|||
|
|
@ -33,12 +33,20 @@ void main() {
|
|||
|
||||
setUpAll(registerAuthFallbacks);
|
||||
|
||||
/// What the real repository does on a successful `updateUser`: the status
|
||||
/// moves to "password changed".
|
||||
Future<Result<void, AuthFailure>> succeed() async {
|
||||
when(() => repository.currentStatus).thenReturn(_changed);
|
||||
status.add(_changed);
|
||||
return ok();
|
||||
}
|
||||
|
||||
setUp(() {
|
||||
repository = MockAuthRepository();
|
||||
status = StreamController<AuthStatus>.broadcast();
|
||||
when(() => repository.currentStatus).thenReturn(_recovery);
|
||||
when(() => repository.watchStatus()).thenAnswer((_) => status.stream);
|
||||
when(() => repository.setNewPassword(any())).thenAnswer((_) async => ok());
|
||||
when(() => repository.setNewPassword(any())).thenAnswer((_) => succeed());
|
||||
when(() => repository.signOut()).thenAnswer((_) async {});
|
||||
});
|
||||
|
||||
|
|
@ -85,12 +93,6 @@ void main() {
|
|||
await tester.pumpAndSettle();
|
||||
}
|
||||
|
||||
/// What the real repository does on a successful `updateUser`.
|
||||
void passwordChangedOnServer() {
|
||||
when(() => repository.currentStatus).thenReturn(_changed);
|
||||
status.add(_changed);
|
||||
}
|
||||
|
||||
group('KTO-01 Kriterium 3: policy at the field', () {
|
||||
testWidgets('shows the rule hint and the missing count while typing', (
|
||||
tester,
|
||||
|
|
@ -171,8 +173,6 @@ void main() {
|
|||
await type(tester, 'ein-langes-passwort');
|
||||
|
||||
await save(tester);
|
||||
passwordChangedOnServer();
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
verify(() => repository.setNewPassword('ein-langes-passwort')).called(1);
|
||||
expect(find.text(_de.authPasswordChanged), findsOneWidget);
|
||||
|
|
@ -207,6 +207,49 @@ void main() {
|
|||
expect(currentPath(router), '/');
|
||||
});
|
||||
|
||||
testWidgets('the form stays locked after success (no window to reuse it)', (
|
||||
tester,
|
||||
) async {
|
||||
final completer = Completer<Result<void, AuthFailure>>();
|
||||
when(() => repository.setNewPassword(any()))
|
||||
.thenAnswer((_) => completer.future);
|
||||
await open(tester);
|
||||
await type(tester, 'ein-langes-passwort');
|
||||
await tester.tap(find.byKey(const Key('reset_password_submit')));
|
||||
await tester.pump();
|
||||
|
||||
// Success arrives, the status event is still on its way.
|
||||
completer.complete(ok());
|
||||
await tester.pump();
|
||||
|
||||
expect(find.byType(CircularProgressIndicator), findsOneWidget);
|
||||
expect(
|
||||
tester.widget<TextField>(fieldOf('reset_password_password')).enabled,
|
||||
isFalse,
|
||||
);
|
||||
status.add(_changed);
|
||||
when(() => repository.currentStatus).thenReturn(_changed);
|
||||
await tester.pumpAndSettle();
|
||||
expect(find.text(_de.authPasswordChanged), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('the success view puts the focus on "Weiter"', (tester) async {
|
||||
await open(tester);
|
||||
await type(tester, 'ein-langes-passwort');
|
||||
|
||||
await save(tester);
|
||||
|
||||
final focused = FocusManager.instance.primaryFocus;
|
||||
expect(focused, isNotNull);
|
||||
expect(
|
||||
find.ancestor(
|
||||
of: find.byElementPredicate((e) => e == focused!.context),
|
||||
matching: find.byKey(const Key('reset_password_continue')),
|
||||
),
|
||||
findsOneWidget,
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('the success view survives a rebuilt screen', (tester) async {
|
||||
when(() => repository.currentStatus).thenReturn(_changed);
|
||||
|
||||
|
|
@ -246,7 +289,7 @@ void main() {
|
|||
.onPressed,
|
||||
isNull,
|
||||
);
|
||||
completer.complete(ok());
|
||||
completer.complete(await succeed());
|
||||
await tester.pumpAndSettle();
|
||||
verify(() => repository.setNewPassword(any())).called(1);
|
||||
});
|
||||
|
|
@ -269,8 +312,7 @@ void main() {
|
|||
.text,
|
||||
'ein-langes-passwort',
|
||||
);
|
||||
when(() => repository.setNewPassword(any()))
|
||||
.thenAnswer((_) async => ok());
|
||||
when(() => repository.setNewPassword(any())).thenAnswer((_) => succeed());
|
||||
await tester.tap(find.text(_de.authActionRetry));
|
||||
await tester.pumpAndSettle();
|
||||
verify(() => repository.setNewPassword(any())).called(2);
|
||||
|
|
|
|||
|
|
@ -144,11 +144,11 @@ Passwort geändert. (Hinweis authPasswordChanged)
|
|||
|
||||
Umsetzungsentscheidungen (01c):
|
||||
|
||||
- **Der Redirect nach `userUpdated` wäre ohne Eingriff ein Fehler:** GoTrue sendet `userUpdated`, bevor `updateUser`
|
||||
zurückkehrt; die Regel „angemeldet auf `/reset-password` → `/`“ hätte die Seite entfernt, bevor die Erfolgsmeldung
|
||||
- **Der Redirect nach `userUpdated` wäre ohne Eingriff ein Fehler:** gotrue liefert `userUpdated` asynchron (auch nach der Rückkehr von `updateUser`); die Regel „angemeldet auf `/reset-password` → `/`“ hätte die Seite entfernt, bevor die Erfolgsmeldung
|
||||
(Krit. 9) sichtbar ist. Lösung: `userUpdated` während der Recovery beendet den Guard **nicht**, sondern setzt
|
||||
`AuthStatus.passwordRecovery(passwordChanged: true)`. Erst „Weiter“ (`AuthRepository.finishPasswordRecovery()`) beendet den
|
||||
Guard; der Router leitet dann nach `/`. Die Erfolgsansicht leitet sich aus dem Status ab (`passwordRecoveryCompleted`),
|
||||
`AuthStatus.passwordRecovery(passwordChanged: true)`. Das Repository markiert die Änderung nach erfolgreichem `updateUser` zusätzlich selbst (idempotent), das Formular bleibt
|
||||
bis zur Erfolgsansicht gesperrt. Erst „Weiter“ (`AuthRepository.finishPasswordRecovery()`, vor dem Wechsel ohne Wirkung) beendet den
|
||||
Guard; der Fokus liegt beim Wechsel auf „Weiter“; der Router leitet dann nach `/`. Die Erfolgsansicht leitet sich aus dem Status ab (`passwordRecoveryCompleted`),
|
||||
überlebt also einen Neuaufbau der Seite. Bewusst **kein** zeitgesteuerter Sprung (Screenreader, Lesezeit). Spec § 5.5
|
||||
ist entsprechend ergänzt.
|
||||
- „Abbrechen“ gibt es nur im Formular, nicht in der Erfolgsansicht (es würde eine gültige Sitzung abmelden).
|
||||
|
|
|
|||
|
|
@ -292,9 +292,9 @@ Alle Formulare: Eingaben bleiben bei jedem Fehler erhalten (Querschnitt Zuständ
|
|||
### 5.5 Provider / Notifier
|
||||
|
||||
- `supabaseClientProvider` (Fundament), `authRepositoryProvider`, `commonPasswordListProvider` (FutureProvider, lazy).
|
||||
- `authStatusProvider` (Stream) mit Abbildung der `AuthChangeEvent`s: `signedIn`/`tokenRefreshed`/`userUpdated` → `signedIn`, `passwordRecovery` → `passwordRecovery`, `signedOut` → `signedOut`.
|
||||
- **Abweichung (umgesetzt in `AuthStatusTracker`, bewusst):** Während einer laufenden Recovery heben `tokenRefreshed` und `signedIn` den Status `passwordRecovery` **nicht** auf. Der Guard endet nur mit `userUpdated` (Passwort geändert), `signedOut` oder dem Wegfall der Sitzung. Grund: Ein Token-Refresh (oder ein erneutes `signedIn` beim Wiederherstellen der Sitzung) im offenen Reset-Screen würde den Nutzer sonst ohne neues Passwort aus dem Guard entlassen. Der Zustand lebt weiter nur im Prozess (5.4, R12).
|
||||
- **Ergänzung 01c (bewusst):** Auch `userUpdated` beendet den Guard nicht mehr. GoTrue sendet es, bevor `updateUser` zurückkehrt; der Redirect „angemeldet auf `/reset-password` → `/`“ hätte den Screen entfernt, bevor die Erfolgsmeldung (Krit. 9) sichtbar ist. Der Tracker setzt stattdessen `AuthStatus.passwordRecovery(passwordChanged: true)`, der Router hält den Nutzer auf `/reset-password` (Erfolgsansicht mit Knopf „Weiter“), und erst `AuthRepository.finishPasswordRecovery()` beendet den Guard. `signedOut` (Abbrechen, Refresh-Fehler) beendet ihn weiterhin sofort. Der Zustand lebt weiter nur im Prozess (R12).
|
||||
- `authStatusProvider` (Stream) mit Abbildung der `AuthChangeEvent`s: `signedIn`/`tokenRefreshed`/`userUpdated` → `signedIn`, `passwordRecovery` → `passwordRecovery`, `signedOut` → `signedOut`; **Ausnahmen während einer laufenden Recovery siehe die beiden folgenden Punkte (Abweichung, Ergänzung 01c).**
|
||||
- **Abweichung (umgesetzt in `AuthStatusTracker`, bewusst):** Während einer laufenden Recovery heben `tokenRefreshed` und `signedIn` den Status `passwordRecovery` **nicht** auf. Der Guard endet nur mit `AuthRepository.finishPasswordRecovery()` (nach dem Passwortwechsel, siehe „Ergänzung 01c“), `signedOut` oder dem Wegfall der Sitzung. Grund: Ein Token-Refresh (oder ein erneutes `signedIn` beim Wiederherstellen der Sitzung) im offenen Reset-Screen würde den Nutzer sonst ohne neues Passwort aus dem Guard entlassen. Der Zustand lebt weiter nur im Prozess (5.4, R12).
|
||||
- **Ergänzung 01c (bewusst):** Auch `userUpdated` beendet den Guard nicht mehr. GoTrue sendet es asynchron, unabhängig vom Zeitpunkt der Rückkehr von `updateUser` (möglicherweise danach); ein Redirect „angemeldet auf `/reset-password` → `/`“ hätte den Screen entfernt, bevor die Erfolgsmeldung (Krit. 9) sichtbar ist. Der Tracker setzt stattdessen `AuthStatus.passwordRecovery(passwordChanged: true)` (bei `userUpdated` **und** direkt nach erfolgreichem `setNewPassword`, idempotent; das Formular bleibt bis dahin gesperrt), der Router hält den Nutzer auf `/reset-password` (Erfolgsansicht mit Knopf „Weiter“), und erst `AuthRepository.finishPasswordRecovery()` beendet den Guard (ohne gesetzte `passwordChanged`-Markierung ohne Wirkung). `signedOut` (Abbrechen, Refresh-Fehler) beendet ihn weiterhin sofort. Der Zustand lebt weiter nur im Prozess (R12).
|
||||
- Je Screen ein `@riverpod`-Notifier (`RegisterController` usw.) mit Zustand `AsyncValue<void>` bzw. freezed-Formzustand. Keine Logik in Widgets.
|
||||
|
||||
### 5.6 Liste häufiger Passwörter (Krit. 3)
|
||||
|
|
@ -511,7 +511,7 @@ Nach der Freigabe am 2026-11-02 eingearbeitet. Die Texte der Akzeptanzkriterien
|
|||
|
||||
### 2026-11-03 – Umsetzung 01c (Branch `feature/1-01c-passwort-reset`), Abweichungen zur Freigabe durch Chris
|
||||
|
||||
1. **Guard endet nicht mit `userUpdated` (5.5, 5.4):** `AuthStatus.passwordRecovery` bekommt `passwordChanged`; `AuthRepository.finishPasswordRecovery()` beendet den Guard, Knopf „Weiter“ statt automatischer Weiterleitung. Grund: GoTrue sendet `userUpdated`, bevor `updateUser` zurückkehrt; der Redirect hätte die Erfolgsmeldung (Krit. 9) verhindert.
|
||||
1. **Guard endet nicht mit `userUpdated` (5.5, 5.4):** `AuthStatus.passwordRecovery` bekommt `passwordChanged`; `AuthRepository.finishPasswordRecovery()` beendet den Guard, Knopf „Weiter“ statt automatischer Weiterleitung. Grund: Der Redirect hätte die Erfolgsmeldung (Krit. 9) verhindert; gotrue liefert `userUpdated` asynchron, deshalb markiert das Repository die Änderung zusätzlich selbst.
|
||||
2. **Neue Route `/forgot-password/check-email` (5.3):** Der Modus (Bestätigung/Reset) steht im Pfad statt im `extra`, damit er einen Web-Reload übersteht. Die Adresse wird im Modus Reset nicht übergeben.
|
||||
3. **Kein Resend im Modus Reset** (wie 5.4): „Andere E-Mail-Adresse eingeben“ statt „E-Mail erneut senden“. `ResendOrigin.checkEmail` gilt weiter nur für den Modus Bestätigung.
|
||||
4. **Erfolgstext Krit. 9** unverändert (kein „sofort“, ehrlich zu F1), dazu der Knopf „Weiter“; `AppBanner` um `actionEnabled` erweitert (Resend im LoginScreen bleibt beim Senden sichtbar, aber deaktiviert); `from` wird zwischen Login, Registrierung und „Passwort vergessen“ weitergegeben.
|
||||
|
|
|
|||
Loading…
Reference in a new issue