fix(auth): a password set up for good stays, through resets too
`password.set` replaced a password whenever the account had no confirmed authenticator, which is also the state an authenticator's or the account's reset leaves behind; and in that state a code by mail or GitHub signed in again. So whoever got a session another way could replace the password and the keys, skipping the reset's 72 hours. An account had a confirmed authenticator while it has one or a reset of it stands (`hadConfirmedAuthenticator`: the reset's row stays until a new authenticator is confirmed). Then `password.set` is refused (`passwordExists`), and the account signs in with its password alone: codes by mail and GitHub are refused (`usePassword`), the latter through `SignInWays.passwordOnly`, which members read from the register. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>
This commit is contained in:
parent
892fba97e8
commit
4e425cb17f
9 changed files with 167 additions and 19 deletions
|
|
@ -20,14 +20,17 @@ abstract class SignInWays
|
|||
required this.email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
}) : password = password ?? false,
|
||||
authenticator = authenticator ?? false;
|
||||
authenticator = authenticator ?? false,
|
||||
passwordOnly = passwordOnly ?? false;
|
||||
|
||||
factory SignInWays({
|
||||
required bool gitHub,
|
||||
required bool email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
}) = _SignInWaysImpl;
|
||||
|
||||
factory SignInWays.fromJson(Map<String, dynamic> jsonSerialization) {
|
||||
|
|
@ -40,6 +43,9 @@ abstract class SignInWays
|
|||
authenticator: jsonSerialization['authenticator'] == null
|
||||
? null
|
||||
: _isc.BoolJsonExtension.fromJson(jsonSerialization['authenticator']),
|
||||
passwordOnly: jsonSerialization['passwordOnly'] == null
|
||||
? null
|
||||
: _isc.BoolJsonExtension.fromJson(jsonSerialization['passwordOnly']),
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -53,6 +59,11 @@ abstract class SignInWays
|
|||
/// A confirmed authenticator: the password signs in only with one.
|
||||
bool authenticator;
|
||||
|
||||
/// The account signs in with its password alone: it had a confirmed
|
||||
/// authenticator (a reset may have left it without one since), so
|
||||
/// neither a code by mail nor GitHub signs into it (ADR 0017).
|
||||
bool passwordOnly;
|
||||
|
||||
/// Returns a shallow copy of this [SignInWays]
|
||||
/// with some or all fields replaced by the given arguments.
|
||||
@_isc.useResult
|
||||
|
|
@ -61,6 +72,7 @@ abstract class SignInWays
|
|||
bool? email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
});
|
||||
@override
|
||||
Map<String, dynamic> toJson() {
|
||||
|
|
@ -70,6 +82,7 @@ abstract class SignInWays
|
|||
'email': email,
|
||||
'password': password,
|
||||
'authenticator': authenticator,
|
||||
'passwordOnly': passwordOnly,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -81,6 +94,7 @@ abstract class SignInWays
|
|||
'email': email,
|
||||
'password': password,
|
||||
'authenticator': authenticator,
|
||||
'passwordOnly': passwordOnly,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -96,11 +110,13 @@ class _SignInWaysImpl extends SignInWays {
|
|||
required bool email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
}) : super._(
|
||||
gitHub: gitHub,
|
||||
email: email,
|
||||
password: password,
|
||||
authenticator: authenticator,
|
||||
passwordOnly: passwordOnly,
|
||||
);
|
||||
|
||||
/// Returns a shallow copy of this [SignInWays]
|
||||
|
|
@ -112,12 +128,14 @@ class _SignInWaysImpl extends SignInWays {
|
|||
bool? email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
}) {
|
||||
return SignInWays(
|
||||
gitHub: gitHub ?? this.gitHub,
|
||||
email: email ?? this.email,
|
||||
password: password ?? this.password,
|
||||
authenticator: authenticator ?? this.authenticator,
|
||||
passwordOnly: passwordOnly ?? this.passwordOnly,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -179,7 +179,9 @@ Built on the three parts above, on the register (members forward every step thro
|
|||
each) and both wrappings (24-byte nonce, 112 bytes). It needs an address: the
|
||||
caller's, or the one a mailed code proves, which becomes theirs as email sign-in makes
|
||||
it (anonymous → `identified`, same ID). It signs in once an authenticator is
|
||||
confirmed; until then a new setup replaces it.
|
||||
confirmed; until then a new setup replaces it, never once one was, even while a reset
|
||||
leaves the account without one (a new password comes through the recovery phrase or
|
||||
the account's reset alone).
|
||||
- **Signing in:** `begin(address)` answers salt, costs and B; `complete(challenge, A, M1,
|
||||
totpCode | recoveryCode)` answers M2, the public keys, the password's wrapping and a
|
||||
session. SRP's identity I is the challenge's ID. An address nobody has (or whose
|
||||
|
|
@ -203,9 +205,9 @@ Built on the three parts above, on the register (members forward every step thro
|
|||
pass (a mail at the start, a sign-in calls it off), a new code carries it out: new
|
||||
password, new keys, no authenticators. Either change of password ends the account's
|
||||
sessions on the register's server.
|
||||
- **Older ways:** an account that signs in with a password (and a confirmed
|
||||
authenticator) no longer signs in by a mailed code (`usePassword`) or GitHub
|
||||
(`usePassword`); where passwords are offered, no new GitHub account is linked
|
||||
- **Older ways:** an account that signs in with a password (it had a confirmed
|
||||
authenticator, which a reset may have taken since: `passwordOnly` in its ways in) no
|
||||
longer signs in by a mailed code (`usePassword`) or GitHub (`usePassword`); where passwords are offered, no new GitHub account is linked
|
||||
(`deprecated`); from `GITHUB_SIGN_IN_ENDS` on GitHub signs nobody in (`ended`) and
|
||||
`forgetGitHubAccounts` deletes every GitHub ID.
|
||||
|
||||
|
|
|
|||
|
|
@ -31,7 +31,8 @@ class GitHubSignInEndpoint extends Endpoint {
|
|||
/// The account the GitHub account [gitHubUserId] signs into, as the
|
||||
/// instance's register knows it; an account it doesn't know is refused
|
||||
/// ([GitHubSignInRefusal.unknownAccount]), and so is one that signs in with
|
||||
/// a password ([GitHubSignInRefusal.usePassword], ADR 0017). GitHub is
|
||||
/// a password ([GitHubSignInRefusal.usePassword], ADR 0017: it had a
|
||||
/// confirmed authenticator, `SignInWays.passwordOnly`). GitHub is
|
||||
/// deprecated: a person signed in so without a password sets an address, a
|
||||
/// password and an authenticator next, as the app reads from their ways in
|
||||
/// (`account.signInWays`).
|
||||
|
|
@ -45,8 +46,7 @@ Future<UuidValue> gitHubAccountToSignIn(
|
|||
(throw GitHubSignInRefusedException(
|
||||
reason: GitHubSignInRefusal.unknownAccount,
|
||||
));
|
||||
final ways = await register.signInWaysOf(session, account);
|
||||
if (ways.password && ways.authenticator) {
|
||||
if ((await register.signInWaysOf(session, account)).passwordOnly) {
|
||||
throw GitHubSignInRefusedException(
|
||||
reason: GitHubSignInRefusal.usePassword,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -7,3 +7,7 @@ fields:
|
|||
password: bool, default=false
|
||||
### A confirmed authenticator: the password signs in only with one.
|
||||
authenticator: bool, default=false
|
||||
### The account signs in with its password alone: it had a confirmed
|
||||
### authenticator (a reset may have left it without one since), so
|
||||
### neither a code by mail nor GitHub signs into it (ADR 0017).
|
||||
passwordOnly: bool, default=false
|
||||
|
|
|
|||
|
|
@ -20,14 +20,17 @@ abstract class SignInWays
|
|||
required this.email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
}) : password = password ?? false,
|
||||
authenticator = authenticator ?? false;
|
||||
authenticator = authenticator ?? false,
|
||||
passwordOnly = passwordOnly ?? false;
|
||||
|
||||
factory SignInWays({
|
||||
required bool gitHub,
|
||||
required bool email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
}) = _SignInWaysImpl;
|
||||
|
||||
factory SignInWays.fromJson(Map<String, dynamic> jsonSerialization) {
|
||||
|
|
@ -40,6 +43,9 @@ abstract class SignInWays
|
|||
authenticator: jsonSerialization['authenticator'] == null
|
||||
? null
|
||||
: _is.BoolJsonExtension.fromJson(jsonSerialization['authenticator']),
|
||||
passwordOnly: jsonSerialization['passwordOnly'] == null
|
||||
? null
|
||||
: _is.BoolJsonExtension.fromJson(jsonSerialization['passwordOnly']),
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -53,6 +59,11 @@ abstract class SignInWays
|
|||
/// A confirmed authenticator: the password signs in only with one.
|
||||
bool authenticator;
|
||||
|
||||
/// The account signs in with its password alone: it had a confirmed
|
||||
/// authenticator (a reset may have left it without one since), so
|
||||
/// neither a code by mail nor GitHub signs into it (ADR 0017).
|
||||
bool passwordOnly;
|
||||
|
||||
/// Returns a shallow copy of this [SignInWays]
|
||||
/// with some or all fields replaced by the given arguments.
|
||||
@_is.useResult
|
||||
|
|
@ -61,6 +72,7 @@ abstract class SignInWays
|
|||
bool? email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
});
|
||||
@override
|
||||
Map<String, dynamic> toJson() {
|
||||
|
|
@ -70,6 +82,7 @@ abstract class SignInWays
|
|||
'email': email,
|
||||
'password': password,
|
||||
'authenticator': authenticator,
|
||||
'passwordOnly': passwordOnly,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -81,6 +94,7 @@ abstract class SignInWays
|
|||
'email': email,
|
||||
'password': password,
|
||||
'authenticator': authenticator,
|
||||
'passwordOnly': passwordOnly,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -96,11 +110,13 @@ class _SignInWaysImpl extends SignInWays {
|
|||
required bool email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
}) : super._(
|
||||
gitHub: gitHub,
|
||||
email: email,
|
||||
password: password,
|
||||
authenticator: authenticator,
|
||||
passwordOnly: passwordOnly,
|
||||
);
|
||||
|
||||
/// Returns a shallow copy of this [SignInWays]
|
||||
|
|
@ -112,12 +128,14 @@ class _SignInWaysImpl extends SignInWays {
|
|||
bool? email,
|
||||
bool? password,
|
||||
bool? authenticator,
|
||||
bool? passwordOnly,
|
||||
}) {
|
||||
return SignInWays(
|
||||
gitHub: gitHub ?? this.gitHub,
|
||||
email: email ?? this.email,
|
||||
password: password ?? this.password,
|
||||
authenticator: authenticator ?? this.authenticator,
|
||||
passwordOnly: passwordOnly ?? this.passwordOnly,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -120,9 +120,10 @@ PasswordKeys requirePasswordKeys(Serverpod pod) => passwordSignInAvailable(pod)
|
|||
? passwordKeysOf(pod)!
|
||||
: throw refusedFor(PasswordRefusal.unavailable);
|
||||
|
||||
/// Whether [person] signs in with a password: they have one and a confirmed
|
||||
/// authenticator (ADR 0017). Then neither a code by mail nor GitHub signs
|
||||
/// into their account any more.
|
||||
/// Whether [person] signs in with a password: they have one, and an
|
||||
/// authenticator was confirmed ([hadConfirmedAuthenticator]; ADR 0017).
|
||||
/// Then neither a code by mail nor GitHub signs into their account any
|
||||
/// more, also while a reset leaves it without one.
|
||||
Future<bool> signsInWithPassword(
|
||||
Session session,
|
||||
UuidValue person, {
|
||||
|
|
@ -135,7 +136,31 @@ Future<bool> signsInWithPassword(
|
|||
transaction: transaction,
|
||||
) >
|
||||
0 &&
|
||||
await hasConfirmedAuthenticator(session, person, transaction: transaction);
|
||||
await hadConfirmedAuthenticator(session, person, transaction: transaction);
|
||||
|
||||
/// Whether [person]'s account had a confirmed authenticator: it has one, or
|
||||
/// a reset of it or its authenticators stands (waiting, or carried out with
|
||||
/// none confirmed since, which leaves an `account_reset` row until one is).
|
||||
/// Such an account's password is set up for good: a session alone never
|
||||
/// replaces it ([setPassword]), as the 72 hours of a reset would otherwise
|
||||
/// be skipped by whoever signs in another way.
|
||||
Future<bool> hadConfirmedAuthenticator(
|
||||
Session session,
|
||||
UuidValue person, {
|
||||
Transaction? transaction,
|
||||
}) async =>
|
||||
await hasConfirmedAuthenticator(
|
||||
session,
|
||||
person,
|
||||
transaction: transaction,
|
||||
) ||
|
||||
// SELECT count(*) FROM "account_reset" WHERE "authUserId" = $1
|
||||
await AccountReset.db.count(
|
||||
session,
|
||||
where: (t) => t.authUserId.equals(person),
|
||||
transaction: transaction,
|
||||
) >
|
||||
0;
|
||||
|
||||
/// Whether [person] has an authenticator a code from was accepted.
|
||||
Future<bool> hasConfirmedAuthenticator(
|
||||
|
|
@ -223,14 +248,16 @@ AccountPassword passwordRowOf(
|
|||
/// sign-in makes it (they become `identified` and stop being anonymous,
|
||||
/// keeping their ID and everything they have) in the same transaction. The
|
||||
/// account signs in with the password only once an authenticator is
|
||||
/// confirmed; until then a new setup replaces it.
|
||||
/// confirmed; until then a new setup replaces it, but never once one was
|
||||
/// ([hadConfirmedAuthenticator]: after a reset, a new password comes through
|
||||
/// the recovery phrase or the account's reset alone).
|
||||
///
|
||||
/// Refused ([PasswordRefusedException]) where the instance has no password
|
||||
/// sign-in, for a setup out of range, without an address, for an address
|
||||
/// another account has, and once the account has a password and a confirmed
|
||||
/// authenticator; a code's refusals as `proveAddressCode`'s, and a second
|
||||
/// address as email sign-in refuses it ([EmailSignInRefusedException]).
|
||||
/// Nothing changes then.
|
||||
/// another account has, and once the account has a password and had a
|
||||
/// confirmed authenticator ([PasswordRefusal.passwordExists]); a code's
|
||||
/// refusals as `proveAddressCode`'s, and a second address as email sign-in
|
||||
/// refuses it ([EmailSignInRefusedException]). Nothing changes then.
|
||||
Future<void> setPassword(
|
||||
Session session, {
|
||||
required UuidValue person,
|
||||
|
|
@ -312,7 +339,7 @@ Future<void> setPassword(
|
|||
);
|
||||
return;
|
||||
}
|
||||
if (await hasConfirmedAuthenticator(
|
||||
if (await hadConfirmedAuthenticator(
|
||||
session,
|
||||
person,
|
||||
transaction: transaction,
|
||||
|
|
|
|||
|
|
@ -403,6 +403,7 @@ final class LocalRegister implements AccountRegister {
|
|||
session,
|
||||
person,
|
||||
),
|
||||
passwordOnly: await passwords.signsInWithPassword(session, person),
|
||||
);
|
||||
|
||||
@override
|
||||
|
|
|
|||
|
|
@ -1248,6 +1248,67 @@ void main() {
|
|||
);
|
||||
});
|
||||
|
||||
test('once carried out, leaves the password to no session alone, and '
|
||||
'the account to its password, until one is confirmed '
|
||||
'again', () async {
|
||||
await annaComplete();
|
||||
await GitHubAccount.db.insertRow(
|
||||
session,
|
||||
GitHubAccount(userIdentifier: '1001', authUserId: anna),
|
||||
);
|
||||
await resetByMail();
|
||||
now = now.add(resetDelay);
|
||||
expect((await signIn()).needsAuthenticator, isTrue);
|
||||
expect(await TotpAuthenticator.db.count(session), 0);
|
||||
|
||||
final other = await PasswordDevice.make(seed: 99);
|
||||
await expectLater(
|
||||
endpoints.password.set(asAnna(), setup: other.setup()),
|
||||
refused(PasswordRefusal.passwordExists),
|
||||
);
|
||||
expect(
|
||||
Uint8List.sublistView(
|
||||
(await AccountPassword.db.findFirstRow(
|
||||
session,
|
||||
where: (t) => t.authUserId.equals(anna),
|
||||
))!.verifier,
|
||||
),
|
||||
Uint8List.sublistView(device.verifier),
|
||||
);
|
||||
// Nor does a code by mail or GitHub sign in in its place.
|
||||
expect(await signsInWithPassword(session, anna), isTrue);
|
||||
final ben = await newAnonymousPerson(session);
|
||||
final asBen = sessionBuilder.copyWith(
|
||||
authentication: AuthenticationOverride.authenticationInfo(
|
||||
ben.uuid,
|
||||
{},
|
||||
),
|
||||
);
|
||||
final request = await endpoints.emailCode.sendCode(
|
||||
asBen,
|
||||
address: 'anna@example.org',
|
||||
language: 'en',
|
||||
);
|
||||
await expectLater(
|
||||
endpoints.emailCode.verifyCode(
|
||||
asBen,
|
||||
request: request,
|
||||
code: mailbox.codeFor('anna@example.org'),
|
||||
),
|
||||
refusedAsEmail(EmailSignInRefusal.usePassword),
|
||||
);
|
||||
await expectLater(
|
||||
gitHubAccountToSignIn(session, '1001'),
|
||||
throwsA(
|
||||
isA<GitHubSignInRefusedException>().having(
|
||||
(error) => error.reason,
|
||||
'reason',
|
||||
GitHubSignInRefusal.usePassword,
|
||||
),
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('is called off by signing in with a code meanwhile', () async {
|
||||
final complete = await annaComplete();
|
||||
await resetByMail();
|
||||
|
|
@ -1659,6 +1720,21 @@ void main() {
|
|||
fresh.signingPublicKey,
|
||||
);
|
||||
await expectLater(finish(fresh), refused(PasswordRefusal.noReset));
|
||||
// A session alone replaces neither, before an authenticator is
|
||||
// confirmed again.
|
||||
await expectLater(
|
||||
endpoints.password.set(asAnna(), setup: device.setup()),
|
||||
refused(PasswordRefusal.passwordExists),
|
||||
);
|
||||
expect(
|
||||
Uint8List.sublistView(
|
||||
(await AccountPassword.db.findFirstRow(
|
||||
session,
|
||||
where: (t) => t.authUserId.equals(anna),
|
||||
))!.verifier,
|
||||
),
|
||||
Uint8List.sublistView(fresh.verifier),
|
||||
);
|
||||
});
|
||||
|
||||
test('is called off by signing in with the password', () async {
|
||||
|
|
|
|||
|
|
@ -219,6 +219,8 @@ final class _Register implements AccountRegister {
|
|||
email: addresses.containsValue(person),
|
||||
password: passwords.contains(person),
|
||||
authenticator: authenticated.contains(person),
|
||||
passwordOnly:
|
||||
passwords.contains(person) && authenticated.contains(person),
|
||||
),
|
||||
);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue