Commit `809306694d`

809306694d1c3b59347ee798932f9d85b59e065d

parent: 9e8931e4c5

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-20 22:21 UTC

auth: document the removal-hook bypass in expire and activate

removeAccountState skips willRemoveAccount; expire and activate's
stale-token branch both call it directly, for different reasons.
Comment each so a future caller has to make the same call on purpose
instead of pattern-matching the omission silently, and add a test
that fails if expire starts running the hook.

Ref krz/gitbay#89

Layout: unified · split

gitbay/Auth/SessionStore.swift +20

	@@ -96,6 +96,9 @@ final class SessionStore {
96	@discardableResult	96	@discardableResult
97	func activate(_ account: Account) -> Bool {	97	func activate(_ account: Account) -> Bool {
98	guard let token = store.token(for: account.id) else {	98	guard let token = store.token(for: account.id) else {
		99	// Bypasses willRemoveAccount deliberately: store.token(for:)
		100	// just returned nil, so there is no credential left to
		101	// deregister a device with.
99	removeAccountState(account)	102	removeAccountState(account)
100	return false	103	return false
101	}	104	}
	@@ -130,6 +133,14 @@ final class SessionStore {
130	removeAccountState(account)	133	removeAccountState(account)
131	}	134	}
132		135
		136	/// The state change behind `remove`, minus the hook. `remove` is the
		137	/// only path that awaits `willRemoveAccount` — this does NOT, on
		138	/// purpose, and every other caller of this method is an intentional
		139	/// exemption from that hook, commented at the call site with why a
		140	/// deregistration attempt there could not succeed anyway. Do not
		141	/// call this from a new site without asking the same question: does
		142	/// the account's token still work here? If it might, route through
		143	/// `remove` instead so the hook gets a chance to run.
133	private func removeAccountState(_ account: Account) {	144	private func removeAccountState(_ account: Account) {
134	store.deleteToken(for: account.id)	145	store.deleteToken(for: account.id)
135	accounts.removeAll { $0.id == account.id }	146	accounts.removeAll { $0.id == account.id }
	@@ -144,6 +155,15 @@ final class SessionStore {
144		155
145	/// The server said 401: the token is expired or revoked. Sign the	156	/// The server said 401: the token is expired or revoked. Sign the
146	/// account out cleanly and say why — never crash, never loop.	157	/// account out cleanly and say why — never crash, never loop.
		158	///
		159	/// Bypasses willRemoveAccount deliberately: expire only runs after
		160	/// the server has already rejected this token with a 401 (via
		161	/// watchForRevocation or handle's requiresReauthentication check),
		162	/// so a deregistration call carrying that same token would be
		163	/// rejected the same way. The device row this account's push
		164	/// registration left on the server survives this and is not
		165	/// cleaned up here — the user clears it from the web settings page,
		166	/// same as any other stale device.
147	func expire(_ account: Account) {	167	func expire(_ account: Account) {
148	removeAccountState(account)	168	removeAccountState(account)
149	if current == nil {	169	if current == nil {

gitbayTests/SessionClientTests.swift +18

	@@ -57,3 +57,21 @@ private func twoAccounts() async throws -> (SessionStore, StubProtocol.Box) {
57	#expect(sawTokenDuringHook == true)	57	#expect(sawTokenDuringHook == true)
58	#expect(session.client(for: victim) == nil)	58	#expect(session.client(for: victim) == nil)
59	}	59	}
		60
		61	// expire is reached only after the server has already rejected the
		62	// account's token with a 401 (revocation, or handle's
		63	// requiresReauthentication path), so a deregistration call carrying
		64	// that token would fail the same way. It deliberately does not run
		65	// the hook; the stale device row is cleared from the web settings
		66	// page instead. See the comment on expire(_:) in SessionStore.
		67	@Test @MainActor func expireDoesNotRunTheRemovalHook() async throws {
		68	let (session, _) = try await twoAccounts()
		69	let account = try #require(session.current)
		70	var hookRan = false
		71	session.willRemoveAccount = { _ in hookRan = true }
		72
		73	session.handle(GitbayError.unauthorized("invalid or expired token"))
		74
		75	#expect(hookRan == false)
		76	#expect(session.accounts.contains(account) == false)
		77	}

Commit 809306694d

Commit `809306694d`