fix(auth): GitHub-only accounts go 30 days after GitHub's end, on the register alone, one failure holding back none

The reading of the full reading's fixes found that a member, which keeps
no addresses, would delete everyone it holds with a GitHub ID as if
GitHub were their only way in (D1); that each deletion delivered every
pending notice, so with a member hanging the daily run grew quadratic,
and one failing person aborted it, forgetting no GitHub ID (D2); and that
a GITHUB_SIGN_IN_ENDS set wrong deleted data at the next daily run.

forgetGitHubAccounts now does nothing on a member and waits 30 days after
the end (gitHubAccountsKeptAfterEnd), keeping the accounts and IDs while a
wrong date can be put right; deletes each GitHub-only account on its own,
a failure logged by type, that person's GitHub ID kept for the next run;
and tells the members once, after all of them (deletePerson's tellMembers).

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

packages/halkyon_base_server/lib/src/auth/account.dart

@@ -20,8 +20,14 @@ import 'package:serverpod_auth_idp_server/core.dart';
 /// transaction holds, what the host's deletions left for afterwards runs.
 ///
 /// This deletes here alone: on a member, the account itself goes through the
-/// register (`AccountRegister.deleteAccount`).
-Future<void> deletePerson(Session session, UuidValue authUserId) async {
+/// register (`AccountRegister.deleteAccount`). Without [tellMembers], the
+/// notices are kept but not delivered yet, for a caller deleting many to
+/// deliver once after all of them (`deliverDeletionNotices`).
+Future<void> deletePerson(
+  Session session,
+  UuidValue authUserId, {
+  bool tellMembers = true,
+}) async {
   final base = HalkyonBase.of(session.serverpod);
   final deletions = base.deletePersonData;
   final members = switch (base.accounts) {
@@ -137,7 +143,7 @@ Future<void> deletePerson(Session session, UuidValue authUserId) async {
   for (final after in afterwards) {
     await after();
   }
-  if (noticesKept) {
+  if (noticesKept && tellMembers) {
     await deliverDeletionNotices(session);
   }
 }

packages/halkyon_base_server/lib/src/auth/github.dart

@@ -3,8 +3,10 @@ import 'dart:io';
 import 'package:clock/clock.dart';
 import 'package:halkyon_base_server/src/auth/account.dart';
 import 'package:halkyon_base_server/src/auth/identified.dart';
+import 'package:halkyon_base_server/src/auth/membership.dart';
 import 'package:halkyon_base_server/src/generated/protocol.dart';
 import 'package:halkyon_base_server/src/password/password_config.dart';
+import 'package:halkyon_base_server/src/register/deletion_notices.dart';
 import 'package:halkyon_base_server/src/register/people.dart';
 import 'package:serverpod/serverpod.dart';
 import 'package:serverpod_auth_idp_server/core.dart';
@@ -69,23 +71,37 @@ void useGitHubSignInEnds(Serverpod pod, DateTime? ends) => _endsOf[pod] = ends;
 
 final _endsOf = Expando<DateTime>('GitHub sign-in ends');
 
+/// How long after GitHub sign-in ended ([gitHubSignInEnds]) the accounts
+/// that signed in with it alone and every GitHub ID are kept before they
+/// are deleted ([forgetGitHubAccounts]): a date the operator set wrong can
+/// be put right meanwhile, before anything is lost.
+const gitHubAccountsKeptAfterEnd = Duration(days: 30);
+
 /// Deletes every GitHub ID the instance keeps once GitHub sign-in has ended
-/// at [now] ([gitHubSignInEnds]), and returns how many: nobody signs in with
-/// them any more, so nothing is kept of them. Daily, with the pruning; on the
-/// register, which keeps them (ADR 0016).
+/// ([gitHubSignInEnds]) and [gitHubAccountsKeptAfterEnd] passed at [now],
+/// and returns how many: nobody signs in with them any more, so nothing is
+/// kept of them. Daily, with the pruning; on the register, which keeps them
+/// (ADR 0016): a member does nothing (the reading of the fixes' D1), as it
+/// keeps no addresses, so everyone there would look as if GitHub were their
+/// only way in.
 ///
 /// First the accounts that sign in with GitHub alone (no address, so never
 /// a password) go, with everything they have, every member told
 /// (`deletePerson`): no way in is left to them, and an account nobody can
 /// enter would stay for good, or be claimed by whoever holds a session of
 /// it (the full reading's M1 and L6). The app tells them for the three
-/// months before (ADR 0017, D2).
+/// months before (ADR 0017, D2). One whose deletion fails holds back nobody
+/// else and keeps its GitHub ID, so the next day's run tries it again; the
+/// members are told once, after all of them (the reading of the fixes' D2).
 Future<int> forgetGitHubAccounts(
   Session session, {
   required DateTime now,
 }) async {
-  final ends = gitHubSignInEnds(session.serverpod);
-  if (ends == null || now.isBefore(ends)) {
+  final pod = session.serverpod;
+  final ends = gitHubSignInEnds(pod);
+  if (isMember(pod) ||
+      ends == null ||
+      now.isBefore(ends.add(gitHubAccountsKeptAfterEnd))) {
     return 0;
   }
   // SELECT * FROM "serverpod_auth_idp_github_account"
@@ -93,6 +109,7 @@ Future<int> forgetGitHubAccounts(
   final gitHubPeople = {
     for (final account in gitHubAccounts) account.authUserId,
   };
+  final failed = <UuidValue>{};
   if (gitHubPeople.isNotEmpty) {
     // SELECT * FROM "email_identity" WHERE "authUserId" = ANY($1)
     final withAddress = {
@@ -104,20 +121,35 @@ Future<int> forgetGitHubAccounts(
     };
     final gitHubOnly = gitHubPeople.difference(withAddress);
     for (final person in gitHubOnly) {
-      await deletePerson(session, person);
+      try {
+        await deletePerson(session, person, tellMembers: false);
+      } on Object catch (error) {
+        // Its type only: whose stays out of the log.
+        session.log(
+          'Deleting an account that signed in with GitHub alone failed: '
+          '${error.runtimeType}; tried again tomorrow.',
+          level: LogLevel.warning,
+        );
+        failed.add(person);
+      }
     }
     session.log(
-      'Deleted ${gitHubOnly.length} accounts that signed in with GitHub '
-      'alone, at its end.',
+      'Deleted ${gitHubOnly.length - failed.length} accounts that signed in '
+      'with GitHub alone, at its end.',
     );
+    await deliverDeletionNotices(session);
   }
   // DELETE FROM "serverpod_auth_idp_github_account"
+  //   WHERE "authUserId" <> ALL($1)
   await GitHubAccount.db.deleteWhere(
     session,
-    where: (t) => t.id.notEquals(null),
+    where: (t) =>
+        failed.isEmpty ? t.id.notEquals(null) : t.authUserId.notInSet(failed),
   );
   // Those deleted with their accounts counted too.
-  return gitHubAccounts.length;
+  return gitHubAccounts
+      .where((account) => !failed.contains(account.authUserId))
+      .length;
 }
 
 /// The numeric GitHub ID that [code], from the app's GitHub sign-in, proves.

packages/halkyon_base_server/test/integration/auth/github_test.dart

@@ -232,13 +232,102 @@ void main() {
             throwsA(_refusal(GitHubSignInRefusal.ended)),
           );
         });
-        expect(await forgetGitHubAccounts(session, now: ends), 1);
+        // For 30 days after, nothing goes: a date set wrong can be put
+        // right meanwhile.
+        expect(
+          await forgetGitHubAccounts(
+            session,
+            now: ends.add(
+              gitHubAccountsKeptAfterEnd - const Duration(minutes: 1),
+            ),
+          ),
+          0,
+        );
+        expect(await AuthUser.db.findById(session, ben), isNotNull);
+        expect(await GitHubAccount.db.count(session), 1);
+        expect(
+          await forgetGitHubAccounts(
+            session,
+            now: ends.add(gitHubAccountsKeptAfterEnd),
+          ),
+          1,
+        );
         expect(await GitHubAccount.db.count(session), 0);
         // Ben signed in with GitHub alone and never set a password up: his
         // account goes with it (the full reading's M1), no way in left.
         expect(await AuthUser.db.findById(session, ben), isNull);
       });
 
+      test('a member forgets nothing and deletes nobody: it keeps no '
+          "addresses, so everyone there would look GitHub's alone (the "
+          "reading of the fixes' D1)", () async {
+        useBase(
+          sessionBuilder,
+          accounts: MemberOfRegister(Uri.parse('http://learn.test:8082')),
+        );
+        final ends = DateTime.utc(2027, 1, 22);
+        useGitHubSignInEnds(session.serverpod, ends);
+
+        expect(
+          await forgetGitHubAccounts(
+            session,
+            now: ends.add(gitHubAccountsKeptAfterEnd),
+          ),
+          0,
+        );
+        expect(await AuthUser.db.findById(session, ben), isNotNull);
+        expect(await GitHubAccount.db.count(session), 1);
+      });
+
+      test('a deletion that fails holds back nobody else, keeps that '
+          "person's GitHub ID for the next run, and members are tried once "
+          'for each account, not once per account deleted (the reading of '
+          "the fixes' D2)", () async {
+        final notifier = _HangingMembers();
+        useBase(
+          sessionBuilder,
+          accounts: HoldsRegister(members: [Uri.parse('http://todo.test')]),
+          deletePersonData: [
+            (session, person, transaction) async {
+              if (person == ben) {
+                throw StateError('the host failed');
+              }
+              return null;
+            },
+          ],
+        );
+        MemberNotifier.use(session.serverpod, notifier);
+        addTearDown(() => MemberNotifier.use(session.serverpod, null));
+        final dana = (await AuthServices.instance.authUsers.create(
+          session,
+          scopes: {identified},
+        )).id;
+        final emil = (await AuthServices.instance.authUsers.create(
+          session,
+          scopes: {identified},
+        )).id;
+        await GitHubAccount.db.insert(session, [
+          GitHubAccount(userIdentifier: '4004', authUserId: dana),
+          GitHubAccount(userIdentifier: '5005', authUserId: emil),
+        ]);
+        final ends = DateTime.utc(2027, 1, 22);
+        useGitHubSignInEnds(session.serverpod, ends);
+
+        await forgetGitHubAccounts(
+          session,
+          now: ends.add(gitHubAccountsKeptAfterEnd),
+        );
+
+        expect(await AuthUser.db.findById(session, dana), isNull);
+        expect(await AuthUser.db.findById(session, emil), isNull);
+        expect(await AuthUser.db.findById(session, ben), isNotNull);
+        expect(await _gitHubAccount(session, '1001'), isNotNull);
+        expect(await GitHubAccount.db.count(session), 1);
+        // Dana's notice and Emil's, each tried once.
+        expect(notifier.told, hasLength(2));
+        expect(notifier.told.toSet(), {dana, emil});
+      });
+
       test('at its end keeps the account of someone with an address too, '
           'forgetting only the GitHub ID', () async {
         final carl = (await AuthServices.instance.authUsers.create(
@@ -259,7 +348,13 @@ void main() {
         );
         final ends = DateTime.utc(2027, 1, 22);
         useGitHubSignInEnds(session.serverpod, ends);
-        expect(await forgetGitHubAccounts(session, now: ends), 2);
+        expect(
+          await forgetGitHubAccounts(
+            session,
+            now: ends.add(gitHubAccountsKeptAfterEnd),
+          ),
+          2,
+        );
         expect(await AuthUser.db.findById(session, carl), isNotNull);
         expect(await AuthUser.db.findById(session, ben), isNull);
         expect(await GitHubAccount.db.count(session), 0);
@@ -268,6 +363,25 @@ void main() {
   });
 }
 
+/// Members that never answer, noting each account they were told is gone.
+final class _HangingMembers implements MemberNotifier {
+  final told = <UuidValue>[];
+
+  @override
+  Future<bool> accountGone(Uri member, UuidValue account) async {
+    told.add(account);
+    return false;
+  }
+
+  @override
+  Future<bool> sessionsEnded(
+    Uri member,
+    UuidValue account, {
+    required DateTime endedBefore,
+    UuidValue? keepSession,
+  }) async => false;
+}
+
 /// The loopback callback the desktop apps use.
 const _redirectUri = 'http://127.0.0.1:43823/github-callback';