Commit f0267e5ff1
f0267e5ff1f4ec8c6f74d945004377c7d8ce3973
parent: d5499b57f6
Verified · cmc
cmc <hello@cleberg.net> · 2026-09-20 23:57 UTC
push: register an account that signs in while the app runs
The spec names three registration events: the APNs token arriving, an
account signing in, and launch. Only two were wired, so an account
added while the app ran had no device row until the next cold launch.
SessionStore gains didAddAccount beside willRemoveAccount, fired at
the end of a successful signIn and pointed at the registrar in the
same place the removal hook is.
deviceToken is private rather than private(set): the registrar is in
the environment, so a readable token was one any view could put on a
screen.
Ref krz/gitbay#89
Layout: unified · split
gitbay/Auth/SessionStore.swift
+7
| @@ -76,6 +76,7 @@ final class SessionStore { |
| 76 | currentAccountID = account.id |
76 | currentAccountID = account.id |
| 77 | signedOutMessage = nil |
77 | signedOutMessage = nil |
| 78 | watchForRevocation(candidate, account: account) |
78 | watchForRevocation(candidate, account: account) |
| |
79 | await didAddAccount?(account) |
| 79 | } |
80 | } |
| 80 | |
81 | |
| 81 | /// A client for any signed-in account, active or not. Push |
82 | /// A client for any signed-in account, active or not. Push |
| @@ -91,6 +92,12 @@ final class SessionStore { |
| 91 | /// nothing about push beyond that something wants to run first. |
92 | /// nothing about push beyond that something wants to run first. |
| 92 | var willRemoveAccount: ((Account) async -> Void)? |
93 | var willRemoveAccount: ((Account) async -> Void)? |
| 93 | |
94 | |
| |
95 | /// Runs after an account signs in, with its token stored. The |
| |
96 | /// counterpart to `willRemoveAccount`: an account added while the |
| |
97 | /// app runs has no push device row until something registers it, |
| |
98 | /// and nothing else would until the next cold launch. |
| |
99 | var didAddAccount: ((Account) async -> Void)? |
| |
100 | |
| 94 | /// Switch to another stored account. Returns false if its token is |
101 | /// Switch to another stored account. Returns false if its token is |
| 95 | /// gone from the Keychain, in which case the account is dropped too. |
102 | /// gone from the Keychain, in which case the account is dropped too. |
| 96 | @discardableResult |
103 | @discardableResult |
gitbay/Push/PushRegistrar.swift
+15 −2
| @@ -17,7 +17,10 @@ final class PushRegistrar { |
| 17 | private let defaults: UserDefaults |
17 | private let defaults: UserDefaults |
| 18 | |
18 | |
| 19 | /// The APNs token as lowercase hex, once iOS has handed it over. |
19 | /// The APNs token as lowercase hex, once iOS has handed it over. |
| 20 | private(set) var deviceToken: String? |
20 | /// Private rather than `private(set)`: this object is in the |
| |
21 | /// environment, so a readable token would be one any view could |
| |
22 | /// put on a screen. |
| |
23 | private var deviceToken: String? |
| 21 | |
24 | |
| 22 | init(session: SessionStore, defaults: UserDefaults = .standard) { |
25 | init(session: SessionStore, defaults: UserDefaults = .standard) { |
| 23 | self.session = session |
26 | self.session = session |
| @@ -39,6 +42,14 @@ final class PushRegistrar { |
| 39 | } |
42 | } |
| 40 | } |
43 | } |
| 41 | |
44 | |
| |
45 | /// Registers one account, for one that signs in while the app is |
| |
46 | /// running. Does nothing until APNs has handed over a token; the |
| |
47 | /// account is picked up by `registerAll` when it does. |
| |
48 | func register(_ account: Account) async { |
| |
49 | guard let token = deviceToken else { return } |
| |
50 | await register(account, token: token) |
| |
51 | } |
| |
52 | |
| 42 | private func register(_ account: Account, token: String) async { |
53 | private func register(_ account: Account, token: String) async { |
| 43 | guard let client = session.client(for: account) else { return } |
54 | guard let client = session.client(for: account) else { return } |
| 44 | nonisolated struct Registered: Decodable, Sendable { let id: Int64 } |
55 | nonisolated struct Registered: Decodable, Sendable { let id: Int64 } |
| @@ -91,7 +102,9 @@ final class PushRegistrar { |
| 91 | await UNUserNotificationCenter.current().notificationSettings().authorizationStatus |
102 | await UNUserNotificationCenter.current().notificationSettings().authorizationStatus |
| 92 | } |
103 | } |
| 93 | |
104 | |
| 94 | /// Re-registers on launch when permission is already granted. |
105 | /// Asks iOS for a token when permission is already granted: on |
| |
106 | /// launch, and when the user grants it in Settings rather than |
| |
107 | /// through the toggle. Idempotent — `device add` upserts. |
| 95 | func registerIfAlreadyAuthorized() async { |
108 | func registerIfAlreadyAuthorized() async { |
| 96 | if await authorizationStatus() == .authorized { |
109 | if await authorizationStatus() == .authorized { |
| 97 | UIApplication.shared.registerForRemoteNotifications() |
110 | UIApplication.shared.registerForRemoteNotifications() |
gitbay/gitbayApp.swift
+3
| @@ -40,6 +40,9 @@ struct gitbayApp: App { |
| 40 | session.willRemoveAccount = { [weak registrar] account in |
40 | session.willRemoveAccount = { [weak registrar] account in |
| 41 | await registrar?.deregister(account) |
41 | await registrar?.deregister(account) |
| 42 | } |
42 | } |
| |
43 | session.didAddAccount = { [weak registrar] account in |
| |
44 | await registrar?.register(account) |
| |
45 | } |
| 43 | // A tap that launched the app reaches the delegate |
46 | // A tap that launched the app reaches the delegate |
| 44 | // before this runs; it parked the payload. |
47 | // before this runs; it parked the payload. |
| 45 | appDelegate.deliverPendingNotification() |
48 | appDelegate.deliverPendingNotification() |
gitbayTests/PushRegistrarTests.swift
+31
| @@ -85,6 +85,37 @@ struct PushRegistrarTests { |
| 85 | #expect(argv(of: removes[0]).last == "7") |
85 | #expect(argv(of: removes[0]).last == "7") |
| 86 | } |
86 | } |
| 87 | |
87 | |
| |
88 | // What SessionStore.didAddAccount calls: the account that just |
| |
89 | // signed in gets a row, and no other account is touched. |
| |
90 | @Test func registrarRegistersOneAccountOnDemand() async throws { |
| |
91 | let (session, box) = try await twoAccounts() |
| |
92 | box.enqueue(.init(status: 200, json: registeredJSON)) |
| |
93 | box.enqueue(.init(status: 200, json: registeredJSON)) |
| |
94 | let registrar = PushRegistrar(session: session, defaults: scratchDefaults()) |
| |
95 | await registrar.deviceTokenArrived(Data([0x01])) |
| |
96 | let before = box.seen.count |
| |
97 | |
| |
98 | box.enqueue(.init(status: 200, json: registeredJSON)) |
| |
99 | await registrar.register(try #require(session.accounts.last)) |
| |
100 | |
| |
101 | let adds = box.seen.dropFirst(before).filter { |
| |
102 | argv(of: $0).starts(with: ["notifications", "device", "add"]) |
| |
103 | } |
| |
104 | #expect(adds.count == 1) |
| |
105 | } |
| |
106 | |
| |
107 | // There is nothing to register with before APNs answers; the |
| |
108 | // token's arrival then registers every account, this one included. |
| |
109 | @Test func registrarRegistersNothingBeforeTheTokenArrives() async throws { |
| |
110 | let (session, box) = try await twoAccounts() |
| |
111 | let registrar = PushRegistrar(session: session, defaults: scratchDefaults()) |
| |
112 | let before = box.seen.count |
| |
113 | |
| |
114 | await registrar.register(try #require(session.accounts.first)) |
| |
115 | |
| |
116 | #expect(box.seen.count == before) |
| |
117 | } |
| |
118 | |
| 88 | // Registration is a side channel: a failure leaves the account working |
119 | // Registration is a side channel: a failure leaves the account working |
| 89 | // and is retried on the next launch, not surfaced. |
120 | // and is retried on the next launch, not surfaced. |
| 90 | @Test func registrarSurvivesAFailedRegistration() async throws { |
121 | @Test func registrarSurvivesAFailedRegistration() async throws { |
gitbayTests/SessionClientTests.swift
+29
| @@ -58,6 +58,35 @@ private func twoAccounts() async throws -> (SessionStore, StubProtocol.Box) { |
| 58 | #expect(session.client(for: victim) == nil) |
58 | #expect(session.client(for: victim) == nil) |
| 59 | } |
59 | } |
| 60 | |
60 | |
| |
61 | // The counterpart to the removal hook: an account added while the app |
| |
62 | // runs has no push device row until something registers it, and |
| |
63 | // nothing else does before the next cold launch. |
| |
64 | @Test @MainActor func signInRunsTheAddHook() async throws { |
| |
65 | let (session, box) = makeSession() |
| |
66 | var added: [String] = [] |
| |
67 | session.didAddAccount = { added.append($0.id) } |
| |
68 | |
| |
69 | box.enqueue(.init(status: 200, json: whoami)) |
| |
70 | try await session.signIn(instanceURL: "https://gitbay.org", token: "t1") |
| |
71 | |
| |
72 | #expect(added == [try #require(session.current).id]) |
| |
73 | } |
| |
74 | |
| |
75 | // A sign-in that stored nothing must not announce an account. |
| |
76 | @Test @MainActor func aFailedSignInDoesNotRunTheAddHook() async throws { |
| |
77 | let (session, box) = makeSession() |
| |
78 | var hookRan = false |
| |
79 | session.didAddAccount = { _ in hookRan = true } |
| |
80 | |
| |
81 | box.enqueue(.init(status: 401, json: #"{"protocol_version":1,"error":"nope","exit_code":4}"#)) |
| |
82 | await #expect(throws: (any Error).self) { |
| |
83 | try await session.signIn(instanceURL: "https://gitbay.org", token: "bad") |
| |
84 | } |
| |
85 | |
| |
86 | #expect(hookRan == false) |
| |
87 | #expect(session.accounts.isEmpty) |
| |
88 | } |
| |
89 | |
| 61 | // expire is reached only after the server has already rejected the |
90 | // expire is reached only after the server has already rejected the |
| 62 | // account's token with a 401 (revocation, or handle's |
91 | // account's token with a 401 (revocation, or handle's |
| 63 | // requiresReauthentication path), so a deregistration call carrying |
92 | // requiresReauthentication path), so a deregistration call carrying |