fix(auth)!: the first authenticator takes the password, and a reset ends every session
The fourth reading's second finding: a stolen session outlived the reset of the authenticators (carrying it out ended no session) and confirmed the account's first authenticator on its own code alone, ahead of the owner, so the owner's confirmation and sign-in were refused after. - Carrying out a reset of the authenticators ends every session of the account, on the register at once and on every member by notice, as the other resets do; the sign-in that carries it out gets its session after that moment (PasswordProof.sessionsEndedBefore, issueSessionAfter). - Confirming the first authenticator takes the password's proof for a change's challenge, without a code (proveOwnersPassword), counted per session as the owner's proofs are: a session of a lost phone, or of a setup never finished, claims no second factor. The app sends the password with every confirmation: the sign-up's, a reset's and the account section's (which asks for it), and a sign-in's own unlock where the session is still held back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>
This commit is contained in:
parent
f0413531ff
commit
d7855162c7
16 changed files with 548 additions and 95 deletions
|
|
@ -220,5 +220,6 @@
|
|||
"passwordSignInWithGitHubAgain": "Wieder mit GitHub anmelden",
|
||||
"passwordOfferTitle": "Richte ein Passwort ein",
|
||||
"passwordOfferBody": "Die Anmeldung mit GitHub endet. Richte ein Passwort und eine Authenticator-App ein, um dein Konto zu behalten.",
|
||||
"passwordOfferLater": "Später"
|
||||
"passwordOfferLater": "Später",
|
||||
"passwordAuthenticatorPasswordIntro": "Bestätige zum Hinzufügen mit deinem Passwort, dass du es bist."
|
||||
}
|
||||
|
|
|
|||
|
|
@ -467,5 +467,6 @@
|
|||
"passwordSignInWithGitHubAgain": "Sign in with GitHub again",
|
||||
"passwordOfferTitle": "Set up a password",
|
||||
"passwordOfferBody": "Sign-in with GitHub is ending. Set up a password and an authenticator app to keep your account.",
|
||||
"passwordOfferLater": "Later"
|
||||
"passwordOfferLater": "Later",
|
||||
"passwordAuthenticatorPasswordIntro": "To add it, confirm it is you with your password."
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1428,6 +1428,12 @@ abstract class HalkyonAppLocalizations {
|
|||
/// In en, this message translates to:
|
||||
/// **'Later'**
|
||||
String get passwordOfferLater;
|
||||
|
||||
/// No description provided for @passwordAuthenticatorPasswordIntro.
|
||||
///
|
||||
/// In en, this message translates to:
|
||||
/// **'To add it, confirm it is you with your password.'**
|
||||
String get passwordAuthenticatorPasswordIntro;
|
||||
}
|
||||
|
||||
class _HalkyonAppLocalizationsDelegate
|
||||
|
|
|
|||
|
|
@ -828,4 +828,8 @@ class HalkyonAppLocalizationsDe extends HalkyonAppLocalizations {
|
|||
|
||||
@override
|
||||
String get passwordOfferLater => 'Später';
|
||||
|
||||
@override
|
||||
String get passwordAuthenticatorPasswordIntro =>
|
||||
'Bestätige zum Hinzufügen mit deinem Passwort, dass du es bist.';
|
||||
}
|
||||
|
|
|
|||
|
|
@ -811,6 +811,10 @@ class HalkyonAppLocalizationsEn extends HalkyonAppLocalizations {
|
|||
|
||||
@override
|
||||
String get passwordOfferLater => 'Later';
|
||||
|
||||
@override
|
||||
String get passwordAuthenticatorPasswordIntro =>
|
||||
'To add it, confirm it is you with your password.';
|
||||
}
|
||||
|
||||
/// The translations for English, as used in the United Kingdom (`en_GB`).
|
||||
|
|
|
|||
|
|
@ -119,8 +119,8 @@ final class const AddAuthenticatorState({
|
|||
}
|
||||
|
||||
/// Adds an authenticator to the signed-in person's account: enrolled as it
|
||||
/// opens, confirmed with its code (and, once one is confirmed, the owner's
|
||||
/// proof: the password and a code from one of those, [needsExistingCode]),
|
||||
/// opens, confirmed with its code and the owner's proof (the password, and
|
||||
/// once one is confirmed a code from one of those, [needsExistingCode]),
|
||||
/// the first one's recovery codes shown.
|
||||
class AddAuthenticatorCubit(
|
||||
final PasswordRepository _passwords, {
|
||||
|
|
@ -147,7 +147,7 @@ class AddAuthenticatorCubit(
|
|||
}
|
||||
}
|
||||
|
||||
/// Confirms it with its [code] and, where needed, the [password] and
|
||||
/// Confirms it with its [code], the [password] and, where needed,
|
||||
/// [existingCode].
|
||||
Future<void> codeEntered(
|
||||
String code, {
|
||||
|
|
@ -166,7 +166,7 @@ class AddAuthenticatorCubit(
|
|||
final codes = await _passwords.confirm(
|
||||
setup.id,
|
||||
code,
|
||||
password: needsExistingCode ? password : null,
|
||||
password: password,
|
||||
existingCode: needsExistingCode ? existingCode : null,
|
||||
);
|
||||
if (!isClosed) {
|
||||
|
|
|
|||
|
|
@ -145,6 +145,10 @@ class AccountSetupCubit(
|
|||
String? _request;
|
||||
PendingSignIn? _pending;
|
||||
|
||||
/// The password typed: the first authenticator takes it (the fourth
|
||||
/// reading).
|
||||
String? _password;
|
||||
|
||||
/// Whether the steps end in a session to put in place.
|
||||
bool get _entersSession => _mode != AccountSetupMode.setPassword;
|
||||
|
||||
|
|
@ -176,6 +180,7 @@ class AccountSetupCubit(
|
|||
return;
|
||||
}
|
||||
emit(state.copyWith(passwordsDiffer: false));
|
||||
_password = password;
|
||||
await _run(() async {
|
||||
final phrase = _phrase;
|
||||
final material = phrase == null
|
||||
|
|
@ -301,6 +306,7 @@ class AccountSetupCubit(
|
|||
final codes = await _passwords.confirm(
|
||||
state.authenticator!.id,
|
||||
code,
|
||||
password: _password,
|
||||
pending: _pending,
|
||||
);
|
||||
emit(
|
||||
|
|
|
|||
|
|
@ -223,6 +223,8 @@ class PasswordSignInCubit(
|
|||
final codes = await _passwords.confirm(
|
||||
state.authenticator!.id,
|
||||
code,
|
||||
// The first takes the password too (the fourth reading).
|
||||
unlocked: _unlocked,
|
||||
pending: _pending,
|
||||
);
|
||||
if (codes.isEmpty) {
|
||||
|
|
|
|||
|
|
@ -121,7 +121,7 @@ abstract interface class PasswordGateway() {
|
|||
Future<PasswordWays> ways();
|
||||
|
||||
/// A change of password's first step.
|
||||
Future<protocol.PasswordChallenge> beginChange();
|
||||
Future<protocol.PasswordChallenge> beginChange({SessionGrant? as});
|
||||
|
||||
/// Deletes the signed-in person's account, which has a password: [answer]
|
||||
/// to [challenge] ([beginChange]) proves it, with a [totpCode] or
|
||||
|
|
|
|||
|
|
@ -267,24 +267,31 @@ class PasswordRepository(
|
|||
PendingSignIn? pending,
|
||||
}) => _gateway.enrol(name: name, label: label, as: pending?.grant);
|
||||
|
||||
/// Confirms authenticator [id] with its [code], and, where the account
|
||||
/// has one confirmed already, the person's [password] and [existingCode]
|
||||
/// from one of those (the owner's proof: a session alone adds no second
|
||||
/// factor); the recovery codes the first brings (shown once), none for
|
||||
/// another.
|
||||
/// Confirms authenticator [id] with its [code] and the owner's proof (a
|
||||
/// session alone adds no second factor): the person's [password] (or
|
||||
/// [unlocked], a sign-in's), and, where the account has one confirmed
|
||||
/// already, [existingCode] from one of those; the first takes the
|
||||
/// password alone (the fourth reading). The recovery codes the first
|
||||
/// brings (shown once), none for another.
|
||||
Future<List<String>> confirm(
|
||||
String id,
|
||||
String code, {
|
||||
String? password,
|
||||
PasswordUnlocked? unlocked,
|
||||
String? existingCode,
|
||||
PendingSignIn? pending,
|
||||
}) async {
|
||||
final codes = await _gateway.confirm(
|
||||
id,
|
||||
code.replaceAll(' ', ''),
|
||||
owner: password == null
|
||||
owner: password == null && unlocked == null
|
||||
? null
|
||||
: await _ownerAnswer(password, totpCode: existingCode),
|
||||
: await _ownerAnswer(
|
||||
password,
|
||||
unlocked: unlocked,
|
||||
totpCode: existingCode,
|
||||
as: pending?.grant,
|
||||
),
|
||||
as: pending?.grant,
|
||||
);
|
||||
_changed();
|
||||
|
|
@ -317,19 +324,28 @@ class PasswordRepository(
|
|||
await _ownerAnswer(password, totpCode: code),
|
||||
);
|
||||
|
||||
/// The owner's proof (`OwnerProof`): [password] answering a change's
|
||||
/// challenge, with a [totpCode] or [recoveryCode].
|
||||
/// The owner's proof (`OwnerProof`): [password] (or [unlocked], unlocked
|
||||
/// at a sign-in already) answering a change's challenge, of the signed-in
|
||||
/// session or [as], with a [totpCode] or [recoveryCode] where given.
|
||||
Future<OwnerAnswer> _ownerAnswer(
|
||||
String password, {
|
||||
String? password, {
|
||||
PasswordUnlocked? unlocked,
|
||||
String? totpCode,
|
||||
String? recoveryCode,
|
||||
SessionGrant? as,
|
||||
}) async {
|
||||
final challenge = await _gateway.beginChange();
|
||||
final unlock = await _crypto.unlock(
|
||||
password,
|
||||
salt: Uint8List.sublistView(challenge.salt),
|
||||
costs: challenge.costs,
|
||||
);
|
||||
final challenge = await _gateway.beginChange(as: as);
|
||||
final salt = Uint8List.sublistView(challenge.salt);
|
||||
final unlock = switch (unlocked?._unlock) {
|
||||
final kept? when kept.fits(salt, challenge.costs) => kept,
|
||||
_ when password != null => await _crypto.unlock(
|
||||
password,
|
||||
salt: salt,
|
||||
costs: challenge.costs,
|
||||
),
|
||||
// Its password changed since: begin again.
|
||||
_ => throw const PasswordFailed(PasswordProblem.challengeExpired),
|
||||
};
|
||||
return (
|
||||
challenge: challenge,
|
||||
answer: await _crypto.answer(unlock, challenge, host: _gateway.host),
|
||||
|
|
|
|||
|
|
@ -209,8 +209,8 @@ class ServerpodPasswordGateway(
|
|||
});
|
||||
|
||||
@override
|
||||
Future<protocol.PasswordChallenge> beginChange() =>
|
||||
_failing(_base.password.beginChange);
|
||||
Future<protocol.PasswordChallenge> beginChange({SessionGrant? as}) =>
|
||||
_as(as, (base) => base.password.beginChange());
|
||||
|
||||
@override
|
||||
Future<void> deleteAccount(
|
||||
|
|
|
|||
|
|
@ -462,7 +462,9 @@ class _AddAuthenticatorState() extends State<_AddAuthenticator> {
|
|||
AddAuthenticatorStep.code => AuthenticatorSetupView(
|
||||
setup: state.setup!,
|
||||
code: _code,
|
||||
password: widget.needsExistingCode ? _password : null,
|
||||
// The owner's password always: a session alone adds no
|
||||
// authenticator, the first included (the fourth reading).
|
||||
password: _password,
|
||||
existingCode: widget.needsExistingCode ? _existing : null,
|
||||
enabled: !state.busy,
|
||||
// The new one's own code under it; the owner's proof (the
|
||||
|
|
|
|||
|
|
@ -482,8 +482,10 @@ class _QrPainter(final QrImage _code) extends CustomPainter {
|
|||
|
||||
/// What setting up an authenticator shows: the QR code of its URI, the key
|
||||
/// to type by hand, on phones a button opening the authenticator app, and
|
||||
/// the fields for its code (and, once one is confirmed, the password and a
|
||||
/// code from an authenticator added before: the owner's proof).
|
||||
/// the fields for its code and the owner's proof where the account's own
|
||||
/// session sets it up ([password], and once one is confirmed [existingCode]
|
||||
/// from an authenticator added before; [existingErrorText] under the last
|
||||
/// of them).
|
||||
class const AuthenticatorSetupView({
|
||||
required final AuthenticatorSetup setup,
|
||||
required final TextEditingController code,
|
||||
|
|
@ -563,19 +565,30 @@ class const AuthenticatorSetupView({
|
|||
kind: CodeKind.totp,
|
||||
enabled: enabled,
|
||||
errorText: errorText,
|
||||
textInputAction: existingCode == null
|
||||
textInputAction: password == null && existingCode == null
|
||||
? TextInputAction.done
|
||||
: TextInputAction.next,
|
||||
onSubmitted: existingCode == null ? (_) => onDone?.call() : null,
|
||||
onSubmitted: password == null && existingCode == null
|
||||
? (_) => onDone?.call()
|
||||
: null,
|
||||
),
|
||||
if (password case final password?) ...[
|
||||
const SizedBox(height: 16),
|
||||
Text(l10n.passwordAuthenticatorOwnerIntro),
|
||||
Text(
|
||||
existingCode == null
|
||||
? l10n.passwordAuthenticatorPasswordIntro
|
||||
: l10n.passwordAuthenticatorOwnerIntro,
|
||||
),
|
||||
const SizedBox(height: 8),
|
||||
PasswordField(
|
||||
controller: password,
|
||||
label: l10n.passwordCurrent,
|
||||
enabled: enabled,
|
||||
errorText: existingCode == null ? existingErrorText : null,
|
||||
textInputAction: existingCode == null
|
||||
? TextInputAction.done
|
||||
: TextInputAction.next,
|
||||
onSubmitted: existingCode == null ? (_) => onDone?.call() : null,
|
||||
),
|
||||
],
|
||||
if (existingCode case final existing?) ...[
|
||||
|
|
|
|||
|
|
@ -480,13 +480,21 @@ class FakePasswordGateway(final AuthGateway _auth) implements PasswordGateway {
|
|||
throw const PasswordFailed(PasswordProblem.wrongCode);
|
||||
}
|
||||
final first = !account.finished;
|
||||
if (owner == null) {
|
||||
throw const PasswordFailed(PasswordProblem.wrongCredentials);
|
||||
}
|
||||
if (!first) {
|
||||
if (owner == null) {
|
||||
_ownerProved(owner);
|
||||
} else {
|
||||
// The first takes the password alone (the fourth reading).
|
||||
final proved = _proved(owner.challenge, owner.answer);
|
||||
if (proved.person != account.person) {
|
||||
throw const PasswordFailed(PasswordProblem.wrongCredentials);
|
||||
}
|
||||
_ownerProved(owner);
|
||||
} else if (owner != null) {
|
||||
throw const PasswordFailed(PasswordProblem.invalidSetup);
|
||||
if ((owner.totpCode ?? '').isNotEmpty ||
|
||||
(owner.recoveryCode ?? '').isNotEmpty) {
|
||||
throw const PasswordFailed(PasswordProblem.invalidSetup);
|
||||
}
|
||||
}
|
||||
final added = account.authenticators[index];
|
||||
account.authenticators[index] = AccountAuthenticator(
|
||||
|
|
@ -562,13 +570,14 @@ class FakePasswordGateway(final AuthGateway _auth) implements PasswordGateway {
|
|||
});
|
||||
|
||||
@override
|
||||
Future<protocol.PasswordChallenge> beginChange() => _call('beginChange', () {
|
||||
final account = _accountAs(null);
|
||||
if (account.password == null) {
|
||||
throw const PasswordFailed(PasswordProblem.noPassword);
|
||||
}
|
||||
return _challengeFor(account.address);
|
||||
});
|
||||
Future<protocol.PasswordChallenge> beginChange({SessionGrant? as}) =>
|
||||
_call('beginChange', () {
|
||||
final account = _accountAs(as);
|
||||
if (account.password == null) {
|
||||
throw const PasswordFailed(PasswordProblem.noPassword);
|
||||
}
|
||||
return _challengeFor(account.address);
|
||||
});
|
||||
|
||||
@override
|
||||
Future<void> deleteAccount(
|
||||
|
|
|
|||
|
|
@ -207,8 +207,8 @@ void main() {
|
|||
expect(anna.authenticators.where((each) => each.confirmed), hasLength(2));
|
||||
});
|
||||
|
||||
test("the first after a reset brings recovery codes, without another's "
|
||||
'code', () async {
|
||||
test('the first after a reset brings recovery codes, with the password '
|
||||
"alone (the fourth reading), without another's code", () async {
|
||||
anna.authenticators.clear();
|
||||
final cubit = AddAuthenticatorCubit(
|
||||
passwords,
|
||||
|
|
@ -219,7 +219,23 @@ void main() {
|
|||
addTearDown(cubit.close);
|
||||
await cubit.start();
|
||||
|
||||
await cubit.codeEntered(fakeTotpCode, existingCode: 'ignored');
|
||||
await cubit.codeEntered(
|
||||
fakeTotpCode,
|
||||
password: 'wrong horse',
|
||||
existingCode: 'ignored',
|
||||
);
|
||||
expect(
|
||||
cubit.state,
|
||||
at(
|
||||
AddAuthenticatorStep.code,
|
||||
problem: PasswordProblem.wrongCredentials,
|
||||
),
|
||||
);
|
||||
await cubit.codeEntered(
|
||||
fakeTotpCode,
|
||||
password: 'correct horse',
|
||||
existingCode: 'ignored',
|
||||
);
|
||||
|
||||
expect(cubit.state, at(AddAuthenticatorStep.recoveryCodes));
|
||||
expect(cubit.state.recoveryCodes, hasLength(10));
|
||||
|
|
|
|||
|
|
@ -154,9 +154,15 @@ void main() {
|
|||
label: 'carla@example.org',
|
||||
pending: pending,
|
||||
);
|
||||
// The pending session alone confirms none (the fourth reading).
|
||||
await expectLater(
|
||||
passwords.confirm(setup.id, '123 456', pending: pending),
|
||||
throwsA(const PasswordFailed(PasswordProblem.wrongCredentials)),
|
||||
);
|
||||
final codes = await passwords.confirm(
|
||||
setup.id,
|
||||
'123 456',
|
||||
unlocked: unlocked,
|
||||
pending: pending,
|
||||
);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue