fix(auth): adding an authenticator takes a code from one confirmed before

Enrolling and confirming an authenticator took only a session, so whoever
held one (a stolen token) could add a second factor of their own, then use
its codes to remove the owner's and make new recovery codes.

While the account has a confirmed authenticator, confirming another now
also takes a code from one of those (`existingCode`, each step's code once
as signing in takes it, within the same 10 tries an hour), refused as
`wrongExistingCode`; it is checked only once the new one's code is right,
so a wrong one uses none. The first authenticator, and the first after a
reset carried out, need none. Members forward it to the register.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>

packages/halkyon_base_client/lib/src/protocol/client.dart

@@ -393,18 +393,21 @@ class EndpointAuthenticator extends _isc.EndpointRef {
     },
   );
 
-  /// Confirms [authenticator] with a [code] from it; the account's first
+  /// Confirms [authenticator] with a [code] from it, and, where the account
-  /// returns its recovery codes, shown once. Refusals as
+  /// has a confirmed authenticator already, an [existingCode] from that one;
+  /// the account's first returns its recovery codes, shown once. Refusals as
   /// `confirmAuthenticator`'s.
   _ida.Future<List<String>> confirm({
     required _isc.UuidValue authenticator,
     required String code,
+    String? existingCode,
   }) => caller.callServerEndpoint<List<String>>(
     'halkyon_base.authenticator',
     'confirm',
     {
       'authenticator': authenticator,
       'code': code,
+      'existingCode': existingCode,
     },
   );
 

packages/halkyon_base_client/lib/src/protocol/password/password_refusal.dart

@@ -69,7 +69,12 @@ enum PasswordRefusal implements _isc.SerializableModel {
   notDue,
 
   /// No such reset is waiting, or the address isn't the account's.
-  noReset;
+  noReset,
+
+  /// Adding an authenticator to an account that has one confirmed takes a
+  /// code from one of those, which is missing or isn't right: a session
+  /// alone adds no second factor.
+  wrongExistingCode;
 
   static PasswordRefusal fromJson(String name) {
     switch (name) {
@@ -105,6 +110,8 @@ enum PasswordRefusal implements _isc.SerializableModel {
         return PasswordRefusal.notDue;
       case 'noReset':
         return PasswordRefusal.noReset;
+      case 'wrongExistingCode':
+        return PasswordRefusal.wrongExistingCode;
       default:
         throw ArgumentError(
           'Value "$name" cannot be converted to "PasswordRefusal"',

packages/halkyon_base_server/README.md

@@ -17,7 +17,7 @@ Endpoints, under the nickname `base` in a host's client (`client.modules.base.em
 | `anonymousIdp` | `login` (Serverpod's anonymous sign-in, at most 30 new accounts per network and hour) | anyone |
 | `password` | `set`: the caller's password, with the address a mailed code proves unless they have one | signed in |
 | `passwordSignIn` | `begin`, `complete`: SRP-6a's two steps with a code from an authenticator (or a recovery code); a session | without a session |
-| `authenticator` | `enrol`, `confirm` (the first brings ten recovery codes), `list`, `remove`, `newRecoveryCodes` | signed in |
+| `authenticator` | `enrol`, `confirm` (the first brings ten recovery codes; another takes a code from one confirmed before), `list`, `remove`, `newRecoveryCodes` | signed in |
 | `accountRecovery` | `sendCode`, `resetAuthenticator`, `beginKeyProof`, `resetPassword` (with the recovery phrase), `startAccountReset`, `completeAccountReset` | without a session |
 | `gitHubUpgrade` | `upgrade`: links a GitHub account to the caller, who becomes `identified`; refused where passwords are offered (deprecated) | signed in |
 | `gitHubSignIn` | `signIn`: a session for whoever has that GitHub account, unless they sign in with a password; none once `GITHUB_SIGN_IN_ENDS` has come | without a session |
@@ -189,8 +189,9 @@ Built on the three parts above, on the register (members forward every step thro
   `wrongCredentials`; `setupIncomplete` and `notDue` come only after a right password.
   At most 10 sign-ins begun per address and 30 per network an hour.
 - **Authenticators:** a secret made on the register, shown once as an `otpauth://` URI
-  (issuer: the account domain); confirmed by a code; the first confirmed brings ten
+  (issuer: the account domain); confirmed by a code; while the account has a confirmed
-  recovery codes; at most 10 per account; the last confirmed one stays; removing a
+  one, another counts only with a code from that one too (`wrongExistingCode`), so a
+  session alone adds no second factor; the first confirmed brings ten recovery codes; at most 10 per account; the last confirmed one stays; removing a
   confirmed one, or new recovery codes, takes a code (10 tries an hour per account).
   Each step's code is accepted once, by a conditional update of `lastStep`.
 - **Resets:** a lost authenticator: SRP's proof plus a mailed code or the recovery

packages/halkyon_base_server/lib/src/generated/endpoints.dart

@@ -633,6 +633,11 @@ class Endpoints extends _is.EndpointDispatch {
               type: _is.getType<String>(),
               nullable: false,
             ),
+            'existingCode': _is.ParameterDescription(
+              name: 'existingCode',
+              type: _is.getType<String?>(),
+              nullable: true,
+            ),
           },
           call:
               (
@@ -645,6 +650,7 @@ class Endpoints extends _is.EndpointDispatch {
                         session,
                         authenticator: params['authenticator'],
                         code: params['code'],
+                        existingCode: params['existingCode'],
                       ),
         ),
         'list': _is.MethodConnector(

packages/halkyon_base_server/lib/src/generated/password/password_refusal.dart

@@ -69,7 +69,12 @@ enum PasswordRefusal implements _is.SerializableModel {
   notDue,
 
   /// No such reset is waiting, or the address isn't the account's.
-  noReset;
+  noReset,
+
+  /// Adding an authenticator to an account that has one confirmed takes a
+  /// code from one of those, which is missing or isn't right: a session
+  /// alone adds no second factor.
+  wrongExistingCode;
 
   static PasswordRefusal fromJson(String name) {
     switch (name) {
@@ -105,6 +110,8 @@ enum PasswordRefusal implements _is.SerializableModel {
         return PasswordRefusal.notDue;
       case 'noReset':
         return PasswordRefusal.noReset;
+      case 'wrongExistingCode':
+        return PasswordRefusal.wrongExistingCode;
       default:
         throw ArgumentError(
           'Value "$name" cannot be converted to "PasswordRefusal"',

packages/halkyon_base_server/lib/src/password/authenticator_endpoint.dart

@@ -23,18 +23,21 @@ class AuthenticatorEndpoint extends Endpoint {
     label: label,
   );
 
-  /// Confirms [authenticator] with a [code] from it; the account's first
+  /// Confirms [authenticator] with a [code] from it, and, where the account
-  /// returns its recovery codes, shown once. Refusals as
+  /// has a confirmed authenticator already, an [existingCode] from that one;
+  /// the account's first returns its recovery codes, shown once. Refusals as
   /// `confirmAuthenticator`'s.
   Future<List<String>> confirm(
     Session session, {
     required UuidValue authenticator,
     required String code,
+    String? existingCode,
   }) => AccountRegister.of(session.serverpod).confirmAuthenticator(
     session,
     person: session.authenticated!.authUserId,
     authenticator: authenticator,
     code: code,
+    existingCode: existingCode,
   );
 
   /// The caller's authenticators, the oldest first.

packages/halkyon_base_server/lib/src/password/authenticators.dart

@@ -125,15 +125,25 @@ String _labelOf(String label) {
 /// codes, returned to be shown once; otherwise none. Confirming one
 /// confirmed before only checks the code.
 ///
+/// While the account has a confirmed authenticator, another counts only
+/// with an [existingCode] from one of those too (each step's code once, as
+/// signing in takes it; decided 20 Oct 2026): otherwise whoever holds a
+/// session could add a second factor of their own, sign in with it and
+/// remove the owner's. Without one (the first, or after a reset carried
+/// out) none is asked for.
+///
 /// Refused ([PasswordRefusedException]) for an authenticator not the
 /// person's ([PasswordRefusal.unknownAuthenticator]), a wrong or used code
-/// ([PasswordRefusal.wrongCode]), and beyond 10 codes tried an hour
+/// ([PasswordRefusal.wrongCode]), a missing, wrong or used code from one
+/// confirmed before ([PasswordRefusal.wrongExistingCode]; checked only once
+/// [code] is right, so a wrong one uses none), and beyond 10 tries an hour
 /// ([PasswordRefusal.tooManyAttempts]).
 Future<List<String>> confirmAuthenticator(
   Session session, {
   required UuidValue person,
   required UuidValue authenticator,
   required String code,
+  String? existingCode,
 }) async {
   final keys = requirePasswordKeys(session.serverpod);
   await _countTry(session, person);
@@ -153,6 +163,15 @@ Future<List<String>> confirmAuthenticator(
     TotpAccepted(:final step) => step,
     TotpRefused() => throw refusedFor(PasswordRefusal.wrongCode),
   };
+  if (row.confirmedAt == null) {
+    final confirmedBefore = await confirmedAuthenticatorsOf(session, person);
+    final typed = existingCode?.trim() ?? '';
+    if (confirmedBefore.isNotEmpty &&
+        (typed.isEmpty ||
+            !await acceptTotpCode(session, keys, confirmedBefore, typed))) {
+      throw refusedFor(PasswordRefusal.wrongExistingCode);
+    }
+  }
   return await session.db.transaction((transaction) async {
     // UPDATE "totp_authenticator" SET "lastStep" = $1, "confirmedAt" = $2
     //   WHERE "id" = $3 AND ("lastStep" IS NULL OR "lastStep" < $1)

packages/halkyon_base_server/lib/src/password/password_refusal.spy.yaml

@@ -43,3 +43,7 @@ values:
   - notDue
   ### No such reset is waiting, or the address isn't the account's.
   - noReset
+  ### Adding an authenticator to an account that has one confirmed takes a
+  ### code from one of those, which is missing or isn't right: a session
+  ### alone adds no second factor.
+  - wrongExistingCode

packages/halkyon_base_server/lib/src/register/account_register.dart

@@ -193,13 +193,15 @@ abstract interface class AccountRegister {
     required String label,
   });
 
-  /// Confirms [person]'s [authenticator]; the first one's recovery codes.
+  /// Confirms [person]'s [authenticator], with [existingCode] from one
-  /// Refusals as `confirmAuthenticator`'s.
+  /// confirmed before where the account has one; the first one's recovery
+  /// codes. Refusals as `confirmAuthenticator`'s.
   Future<List<String>> confirmAuthenticator(
     Session session, {
     required UuidValue person,
     required UuidValue authenticator,
     required String code,
+    String? existingCode,
   });
 
   /// [person]'s authenticators, the oldest first.

packages/halkyon_base_server/lib/src/register/instance_api.dart

@@ -263,6 +263,7 @@ Future<Map<String, Object?>?> _answer(
         person: arguments.id('person'),
         authenticator: arguments.id('authenticator'),
         code: arguments.string('code'),
+        existingCode: arguments.optionalString('existingCode'),
       );
       return {'recoveryCodes': codes};
     case RegisterOperation.authenticatorsOf:

packages/halkyon_base_server/lib/src/register/local_register.dart

@@ -471,11 +471,13 @@ final class LocalRegister implements AccountRegister {
     required UuidValue person,
     required UuidValue authenticator,
     required String code,
+    String? existingCode,
   }) => authenticators.confirmAuthenticator(
     session,
     person: person,
     authenticator: authenticator,
     code: code,
+    existingCode: existingCode,
   );
 
   @override

packages/halkyon_base_server/lib/src/register/remote_register.dart

@@ -381,12 +381,14 @@ final class RemoteRegister implements AccountRegister {
     required UuidValue person,
     required UuidValue authenticator,
     required String code,
+    String? existingCode,
   }) async => _strings(
     RegisterOperation.confirmAuthenticator,
     (await _ask(RegisterOperation.confirmAuthenticator, {
       'person': person.uuid,
       'authenticator': authenticator.uuid,
       'code': code,
+      'existingCode': existingCode,
     }))['recoveryCodes'],
   );
 

packages/halkyon_base_server/test/integration/password/password_test.dart

@@ -855,6 +855,7 @@ void main() {
               asAnna(),
               authenticator: second.authenticator,
               code: codeOf(second, at: now),
+              existingCode: codeOf(complete.enrolment, at: now),
             ),
           );
           expect(codes, isEmpty);
@@ -866,6 +867,71 @@ void main() {
           );
         });
 
+        test('take, once one is confirmed, a code from one confirmed before '
+            'to add another: a session alone adds none', () async {
+          final complete = await annaComplete();
+          final another = await endpoints.authenticator.enrol(
+            asAnna(),
+            name: 'Not hers',
+            label: '',
+          );
+          Future<List<String>> confirm({
+            String? code,
+            String? existingCode,
+          }) => atNow(
+            () => endpoints.authenticator.confirm(
+              asAnna(),
+              authenticator: another.authenticator,
+              code: code ?? codeOf(another, at: now),
+              existingCode: existingCode,
+            ),
+          );
+
+          await expectLater(
+            confirm(),
+            refused(PasswordRefusal.wrongExistingCode),
+          );
+          await expectLater(
+            confirm(existingCode: '000000'),
+            refused(PasswordRefusal.wrongExistingCode),
+          );
+          // Its own code is no code from one confirmed before.
+          await expectLater(
+            confirm(existingCode: codeOf(another, at: now)),
+            refused(PasswordRefusal.wrongExistingCode),
+          );
+          // A wrong code from the new one takes nothing of the other's.
+          await expectLater(
+            confirm(
+              code: '000000',
+              existingCode: codeOf(complete.enrolment, at: now),
+            ),
+            refused(PasswordRefusal.wrongCode),
+          );
+          expect(
+            (await endpoints.authenticator.list(asAnna())).map(
+              (authenticator) => authenticator.confirmed,
+            ),
+            [true, false],
+          );
+
+          expect(
+            await confirm(existingCode: codeOf(complete.enrolment, at: now)),
+            isEmpty,
+          );
+          expect(
+            (await endpoints.authenticator.list(asAnna())).map(
+              (authenticator) => authenticator.confirmed,
+            ),
+            [true, true],
+          );
+          // The code it took is used.
+          await expectLater(
+            signIn(totpCode: codeOf(complete.enrolment, at: now)),
+            refused(PasswordRefusal.wrongCredentials),
+          );
+        });
+
         test("are another person's to nobody: unknown", () async {
           final complete = await annaComplete();
           final ben = await newAnonymousPerson(session);
@@ -930,6 +996,7 @@ void main() {
               asAnna(),
               authenticator: second.authenticator,
               code: codeOf(second, at: now),
+              existingCode: codeOf(complete.enrolment, at: now),
             ),
           );
           nextStep();

packages/halkyon_base_server/test/integration/register/member_test.dart

@@ -289,6 +289,7 @@ final class _Register implements AccountRegister {
     required UuidValue person,
     required UuidValue authenticator,
     required String code,
+    String? existingCode,
   }) => _answer('confirmAuthenticator', () {
     authenticated.add(person);
     return const ['K7QX-M2PD'];

packages/halkyon_base_server/test/integration/register/register_test.dart

@@ -531,10 +531,32 @@ void main() {
               name: 'Spare',
               label: '',
             );
+            await expectLater(
+              remote.confirmAuthenticator(
+                session,
+                person: annaOnTodo,
+                authenticator: spare.authenticator,
+                code: codeOf(spare),
+              ),
+              refused(PasswordRefusal.wrongExistingCode),
+            );
+            now = now.add(totpPeriod);
+            expect(
+              await remote.confirmAuthenticator(
+                session,
+                person: annaOnTodo,
+                authenticator: spare.authenticator,
+                code: codeOf(spare),
+                existingCode: codeOf(enrolment),
+              ),
+              isEmpty,
+            );
+            now = now.add(totpPeriod);
             await remote.removeAuthenticator(
               session,
               person: annaOnTodo,
               authenticator: spare.authenticator,
+              code: codeOf(enrolment),
             );
             final ways = await remote.signInWaysOf(session, annaOnTodo);
             expect(ways.password && ways.authenticator, isTrue);

packages/halkyon_base_server/test/integration/test_tools/serverpod_test_tools.dart

@@ -931,6 +931,7 @@ class _AuthenticatorEndpoint {
     _ist.TestSessionBuilder sessionBuilder, {
     required _is.UuidValue authenticator,
     required String code,
+    String? existingCode,
   }) async {
     return _ist.callAwaitableFunctionAndHandleExceptions(() async {
       var _localUniqueSession =
@@ -946,6 +947,7 @@ class _AuthenticatorEndpoint {
           parameters: _ist.testObjectToJson({
             'authenticator': authenticator,
             'code': code,
+            'existingCode': existingCode,
           }),
           serializationManager: _serializationManager,
         );