this repo has no description
1# OAuth update plan for CoreATProtocol
2
3Goal: keep OAuthenticator as the OAuth 2.1 transport, but tighten the AT-Proto-specific layer above it. Borrow the few ideas AtprotoOAuth (MIT, germ-network) does well that genuinely improve correctness, testability, or readability. No library swap. Each step leaves the package building and the existing test suite green.
4
5## Status (2026-04-29)
6
7**The plan is fully landed.** Steps 1–7 are all done.
8
9- Per-origin DPoP nonce caching, shared `URLOrigin.normalized` helper, extracted `TokenValidator`, `ATProtoIdentityResolver` injection seam, file split, and deprecated-symbol removal are all live.
10- CoreATProtocol has 71 tests (4 live-network ones opt-in via `CORE_ATPROTOCOL_LIVE_TESTS=1`); the deterministic suite runs in ~50ms.
11- bskyKit and EffemKit rebuild cleanly against the local checkout. Atprosphere + effem-iOS verified by source inspection — no consumer call sites broke.
12- Auto-memory updated: stale Plume entries dropped, four new memory files added covering the per-origin nonce design, consumer OAuth modes, the singleton testing constraint, and the live-tests env var.
13
14The only obvious next milestone is unrelated to this plan: **make `ATProtoSession` instance-based.** See "What didn't make this plan" near the bottom.
15
16See the "Notes from..." sections near the bottom for deviations from the original plan at each step.
17
18## Inventory of what we have today
19
20Files (in `Sources/CoreATProtocol/`):
21- `OAuth/ATProtoOAuth.swift` — 1016 lines. Authorize / refresh / DPoP JWT generation / token persistence. Single class doing everything.
22- `OAuth/AuthProxy.swift` — 294 lines. `URLResponseProvider` interceptor that re-routes PAR + token requests through an auth proxy, with bypass on transport failure.
23- `OAuth/IdentityResolver.swift` — 366 lines. Handle ↔ DID ↔ PDS ↔ auth server resolution + bidirectional handle verification.
24- `Networking.swift` — `APRouterDelegate` that signs each authenticated XRPC request with a DPoP proof, watches for `use_dpop_nonce`, refreshes on 401/403, harvests clock skew.
25- `APEnvironment.swift` — `ATProtoSession.shared` singleton holding tokens + DPoP key + JWT key collection + nonce store + clock skew store.
26- `DPoPNonceStore.swift` — single-slot actor for the most recent server nonce.
27- `ClockSkewStore.swift` — observed offset between local and server clocks.
28- `TokenRefreshCoordinator.swift` — coalesces concurrent refresh attempts.
29- `Models/` — `Session`, `AtError`, etc.
30
31Consumers:
32- `bskyKit` re-exports `ATProtoOAuth` as `BskyOAuth` (via `@_exported import CoreATProtocol`).
33- `Atprosphere` calls `ATProtoOAuth(config:storage:)` with `clientAuthMethod: .privateKeyJWT` + `authProxyBaseURL`.
34- `effem` calls `ATProtoOAuth(config:storage:)` with `authProxyBaseURL` only (default `.none`).
35- `ATProtoCLI` does not use OAuth.
36
37Tests: 1012 lines across 9 files, including `OAuthTests.swift` (446 lines) covering proxy request encoding, key ID storage, DPoP key persistence, and identity resolution.
38
39## What's worth borrowing from AtprotoOAuth
40
411. **Per-origin DPoP nonce cache.** AtprotoOAuth's `OAuth.DPoP.State.nonceCache` is keyed by origin (capped at 25 entries). RFC 9449 lets each server issue its own nonce; the auth server's nonce ≠ the PDS's nonce. Our single-slot store causes spurious `use_dpop_nonce` retries when the most recent nonce is from the wrong origin.
422. **Consolidated DPoP signer type.** AtprotoOAuth bundles `signingKey` + `nonceCache` + `decoder` into one value (`OAuth.DPoP.State`). We have these scattered across `ATProtoSession` (`dpopPrivateKey`, `dpopKeys`, `dpopNonceStore`) and re-thread them through `Networking.swift` and `ATProtoOAuth.swift`. A single `DPoPSigner` actor would hold the lot.
433. **`htu` canonicalization helper.** We strip query+fragment in two places (`Networking.swift` line 92-96 and `ATProtoOAuth.swift` line 561). One small free function.
444. **Pluggable resolver protocol.** Right now `IdentityResolver` is concrete. AtprotoOAuth has `Atproto.Resolver` as a protocol with a default `verifiedResolve` and a `FallbackResolver` that tries primary then secondary. Useful for tests (inject a fake resolver) and for adding Slingshot/community DoH fallback later.
455. **Token validator closure.** AtprotoOAuth pulls token validation (sub matches expected DID, issuer matches expected auth server, DPoP token type, scope contains "atproto") out of the loginProvider into a single throwing closure. Easier to unit test in isolation. We already do this work — just inline.
46
47What we should *not* borrow:
48- `AtprotoOAuthAgent`/`AuthPDSAgent` — couples to germ's XRPC abstraction.
49- `OAuth.SessionState.Archive` — our `Login`/`storage` callbacks already do the same job.
50- AtprotoTypes `Atproto.DID`/`Handle`/`DIDDocument` — we have our own types, no need to swap.
51- The `AsyncStream` save model — a callback closure (what we have) is simpler and equally correct.
52
53## Plan
54
55Each step ends with `swift test` passing in `CoreATProtocol/`, then a build of `bskyKit`, Atprosphere, and effem against the local CoreATProtocol checkout. Treat the build of all three apps as the integration gate.
56
57### Step 1 — Add `DPoPSigner` (the consolidated signer actor) — DONE
58
59**Why first:** the per-origin nonce fix lands here and is the highest-value correctness improvement. Doing it before the file split keeps later diffs small.
60
61Create `Sources/CoreATProtocol/OAuth/DPoPSigner.swift`:
62
63```swift
64import Foundation
65import Crypto
66import JWTKit
67
68/// Owns the DPoP signing key, the per-origin nonce cache, and JWT generation.
69///
70/// One `DPoPSigner` per logical session. Nonces are keyed by origin
71/// (`scheme://host[:port]`) per RFC 9449 — the auth server and the PDS have
72/// independent nonce streams, and using the wrong one causes a spurious
73/// `use_dpop_nonce` challenge on the next request.
74public actor DPoPSigner {
75 public struct ProofParameters: Sendable {
76 public let httpMethod: String
77 public let url: URL
78 /// Access token to bind via `ath`. Pass `nil` for unauthenticated
79 /// requests (PAR, token exchange).
80 public let accessToken: String?
81 }
82
83 private let privateKey: ES256PrivateKey
84 private let keys: JWTKeyCollection
85 private var noncesByOrigin: [String: String] = [:]
86 private let nonceCacheLimit = 25
87 private let clockSkew: ClockSkewStore
88
89 public init(privateKey: ES256PrivateKey, clockSkew: ClockSkewStore) async {
90 self.privateKey = privateKey
91 let keys = JWTKeyCollection()
92 await keys.add(ecdsa: privateKey)
93 self.keys = keys
94 self.clockSkew = clockSkew
95 }
96
97 public convenience init(clockSkew: ClockSkewStore) async {
98 await self.init(privateKey: ES256PrivateKey(), clockSkew: clockSkew)
99 }
100
101 public var pemRepresentation: String { privateKey.pemRepresentation }
102
103 public func sign(_ params: ProofParameters) async throws -> String {
104 let htu = Self.canonicalHTU(params.url)
105 let origin = Self.origin(for: params.url)
106 let nonce = origin.flatMap { noncesByOrigin[$0] }
107 let issuedAt = await clockSkew.serverAdjustedNow()
108 let ath = params.accessToken.map(Self.athClaim)
109
110 var header = JWTHeader()
111 header.typ = "dpop+jwt"
112 header.alg = "ES256"
113 if let p = privateKey.parameters {
114 header.jwk = [
115 "kty": .string("EC"), "crv": .string("P-256"),
116 "x": .string(p.x.base64URLEncoded()),
117 "y": .string(p.y.base64URLEncoded()),
118 ]
119 }
120
121 let payload = DPoPProofPayload(
122 htm: params.httpMethod, htu: htu,
123 iat: .init(value: issuedAt),
124 jti: .init(value: UUID().uuidString),
125 nonce: nonce, ath: ath
126 )
127 return try await keys.sign(payload, header: header)
128 }
129
130 /// Records a nonce against the origin of `url`, evicting the oldest entry
131 /// if the cache exceeds its limit.
132 public func cacheNonce(_ nonce: String, from url: URL) {
133 guard let origin = Self.origin(for: url) else { return }
134 if noncesByOrigin.count >= nonceCacheLimit, noncesByOrigin[origin] == nil {
135 // Drop one arbitrary entry. A true LRU is overkill — bounded growth is the goal.
136 if let key = noncesByOrigin.keys.first { noncesByOrigin.removeValue(forKey: key) }
137 }
138 noncesByOrigin[origin] = nonce
139 }
140
141 public func clearNonces() { noncesByOrigin.removeAll() }
142
143 // MARK: - Helpers
144
145 static func canonicalHTU(_ url: URL) -> String {
146 var components = URLComponents(url: url, resolvingAgainstBaseURL: false)
147 components?.query = nil
148 components?.fragment = nil
149 return components?.url?.absoluteString ?? url.absoluteString
150 }
151
152 static func origin(for url: URL) -> String? {
153 guard let scheme = url.scheme?.lowercased(),
154 let host = url.host?.lowercased() else { return nil }
155 let port = url.port.map { ":\($0)" } ?? ""
156 return "\(scheme)://\(host)\(port)"
157 }
158
159 static func athClaim(for accessToken: String) -> String {
160 let hash = SHA256.hash(data: Data(accessToken.utf8))
161 return Data(hash).base64URLEncodedString()
162 }
163}
164
165private struct DPoPProofPayload: JWTPayload {
166 let htm: String
167 let htu: String
168 let iat: IssuedAtClaim
169 let jti: IDClaim
170 let nonce: String?
171 let ath: String?
172 func verify(using key: some JWTAlgorithm) throws {}
173}
174```
175
176Tests to add (Swift Testing):
177- `DPoPSignerTests.signEmitsDpopJwtType` — verify header `typ=dpop+jwt`, `alg=ES256`, JWK has `kty/crv/x/y`.
178- `DPoPSignerTests.htuStripsQueryAndFragment` — `https://x/y?z#a` → `https://x/y`.
179- `DPoPSignerTests.athPresentOnlyWhenAccessTokenProvided` — round-trip JWT, decode payload, check `ath`.
180- `DPoPSignerTests.nonceCachedPerOrigin` — cache nonces for two origins, verify each origin gets its own nonce in the next signed proof.
181- `DPoPSignerTests.nonceCacheBounded` — push 30 origins, assert ≤ 25 entries.
182- `DPoPSignerTests.iatRespectsClockSkew` — set `ClockSkewStore` offset to +600, sign, decode `iat`, assert ≈ now + 600.
183
184Don't wire it into `Networking.swift` or `ATProtoOAuth.swift` yet — that's Step 2.
185
186### Step 2 — Replace ad-hoc DPoP signing in `Networking.swift` and `ATProtoOAuth.swift` — DONE
187
188`APRouterDelegate.intercept` currently reads `dpopPrivateKey`, `dpopKeys`, `dpopNonceStore` from `ATProtoSession.shared` and builds the proof inline (Networking.swift:55-129). Replace with a call to a `DPoPSigner` held on the session.
189
190Changes:
191- Add `var dpopSigner: DPoPSigner?` to `ATProtoSession`.
192- Update `setDPoPPrivateKey(pem:)` in `CoreATProtocol.swift` to instantiate `DPoPSigner` instead of populating `dpopPrivateKey` + `dpopKeys`.
193- Replace `APRouterDelegate.generateDPoPProof` body with `try await signer.sign(.init(httpMethod: method, url: url, accessToken: accessToken))`.
194- Replace `APRouterDelegate.didReceiveErrorResponse`'s `dpopNonceStore.update(headerNonce)` with `signer.cacheNonce(headerNonce, from: response.url ?? request.url)`. The response URL is the right origin to key on.
195- In `ATProtoOAuth.generateJWT` (the auth-server-side signer at line 528), build a `DPoPSigner.ProofParameters` from `params.requestEndpoint` + `params.httpMethod` + `nil` access token. Delete the local `stripQueryAndFragment` helper (line 561).
196- Keep `DPoPNonceStore` for now — it backs the deprecated `dpopNonceStore` accessor on the session. Mark it `@available(*, deprecated, message: "Use DPoPSigner.cacheNonce instead.")` and migrate callers in Step 6.
197- Keep the existing `dpopPrivateKey` / `dpopKeys` accessors on `ATProtoSession`, but mark them deprecated. They're public, so removing them is a major-version change.
198
199After this, `Networking.swift` shrinks by ~50 lines. The whole DPoP proof construction lives in one file.
200
201Tests to update:
202- `DPoPStoreTests.swift` continues to test `DPoPNonceStore` for now. The new `DPoPSignerTests` covers the new path.
203- Add an integration test in `OAuthTests.swift` that drives a fake `URLResponseProvider`, sees a `DPoP-Nonce` header, and confirms the next outbound proof carries the nonce.
204
205### Step 3 — Centralize URL canonicalization + origin helpers — DONE
206
207The `htu` and origin logic now lives in `DPoPSigner` (steps 1–2). Two more places use it:
208- `ATProtoOAuth.normalizedOrigin(_:)` (line 901) — for issuer comparison.
209- `IdentityResolver.normalizedOrigin(from:)` (line 287) — for auth-server-vs-PDS check.
210
211Both are private and trivially identical. Lift them into a small file `Sources/CoreATProtocol/OAuth/URLOrigin.swift`:
212
213```swift
214enum URLOrigin {
215 static func normalized(_ url: URL) -> String? {
216 guard let scheme = url.scheme?.lowercased(),
217 let host = url.host?.lowercased() else { return nil }
218 let port = url.port.map { ":\($0)" } ?? ""
219 return "\(scheme)://\(host)\(port)"
220 }
221}
222```
223
224Update three call sites (`DPoPSigner.origin`, `ATProtoOAuth.normalizedOrigin`, `IdentityResolver.normalizedOrigin`) to call `URLOrigin.normalized`. Internal helper, no public API change.
225
226### Step 4 — Extract the token validator closure — DONE
227
228`ATProtoOAuth.loginProvider` (line 759) inlines all the token-response validation: DPoP token type, `atproto` scope, issuer matches expected auth server, sub matches expected DID. Same checks duplicate in `refreshProvider` (line 834).
229
230Extract to a private helper:
231
232```swift
233private struct TokenValidator: Sendable {
234 let expectedAuthorizationServer: String
235 let expectedSubjectDID: String?
236
237 func validate(
238 response: OAuthTokenResponse,
239 actualIssuer: String
240 ) throws {
241 guard response.tokenType == "DPoP" else {
242 throw AuthenticatorError.dpopTokenExpected(response.tokenType)
243 }
244 guard response.scopes.contains("atproto") else {
245 throw ATProtoOAuthError.missingRequiredScope("atproto")
246 }
247 try ATProtoOAuth.validateIssuer(actualIssuer, matches: expectedAuthorizationServer)
248 if let expectedSubjectDID, response.subject != expectedSubjectDID {
249 throw ATProtoOAuthError.subjectMismatch(
250 expected: expectedSubjectDID,
251 actual: response.subject
252 )
253 }
254 }
255}
256```
257
258Both providers call `validator.validate(response:actualIssuer:)`. Worth ~30 lines saved and the validator becomes directly unit-testable.
259
260Tests: add `TokenValidatorTests` covering the four failure modes (wrong token type, missing scope, issuer mismatch, sub mismatch) and the success path.
261
262### Step 5 — Make `IdentityResolver` a protocol — DONE
263
264Today `IdentityResolver` is a concrete struct. Tests have to hit live PDS endpoints (see `OAuthTests.swift` line 11-25 — calls `atproto.com`). That's fine for a smoke test, terrible for CI determinism.
265
266Introduce:
267
268```swift
269public protocol ATProtoIdentityResolver: Sendable {
270 func resolve(identifier: String) async throws -> IdentityResolver.ResolvedIdentity
271 func isAuthorizationServer(_ authorizationServer: String, validFor pdsEndpoint: String) async throws -> Bool
272}
273
274extension IdentityResolver: ATProtoIdentityResolver {}
275```
276
277Add an injection point on `ATProtoOAuth.init`:
278
279```swift
280public init(
281 config: ATProtoOAuthConfig,
282 storage: ATProtoAuthStorage,
283 identityResolver: ATProtoIdentityResolver = IdentityResolver()
284) async { ... }
285```
286
287Don't add a "fallback resolver" yet — YAGNI until you actually need a Slingshot/community DoH backup. Just open the seam.
288
289Tests: replace the live-network identity tests in `OAuthTests.swift` with a fake `ATProtoIdentityResolver` that returns canned `ResolvedIdentity` values. Keep one live integration test, but mark it with `@Test(.disabled(if: ...))` or a tag so it's opt-in.
290
291### Step 6 — Optional: split `ATProtoOAuth.swift` along seams — DONE
292
293Only do this if Steps 1–5 still leave the file feeling unwieldy. The seams are clear:
294
295- `ATProtoOAuth.swift` (~400 lines) — `ATProtoOAuth` class, public config, public errors.
296- `OAuth/TokenHandling+ATProto.swift` (~250 lines) — `buildTokenHandling`, `authorizationURLProvider`, `loginProvider`, `refreshProvider` extracted as private extensions on `ATProtoOAuth`.
297- `OAuth/TokenModels.swift` (~150 lines) — `OAuthTokenRequest`, `OAuthRefreshTokenRequest`, `OAuthTokenResponse`.
298
299Don't change behavior in this step — pure file move + access-control tightening.
300
301Once the split lands, drop the deprecated `ATProtoSession.dpopPrivateKey` / `dpopKeys` / `dpopNonceStore` accessors (Step 2 deprecated them). Remove `DPoPNonceStore.swift`. Bump CoreATProtocol's tag — this is a SemVer-major change because the deprecated symbols are public.
302
303### Step 7 — Update auto-memory — DONE
304
305After all of this lands, update `~/.claude/projects/-Users-rademaker-Developer-SparrowTek-AtProto/memory/MEMORY.md`:
306- Note that DPoP signing is consolidated in `OAuth/DPoPSigner.swift`.
307- Note the per-origin nonce cache (so future-Claude doesn't try to re-introduce a single-slot store).
308- Note that Atprosphere uses `clientAuthMethod: .privateKeyJWT`, effem uses `.none`, and the auth proxy path is exercised in production by both.
309- Drop the stale Plume entries — that directory no longer exists in this checkout.
310
311## What we're explicitly NOT doing
312
313- **Not switching to AtprotoOAuth.** Their README says it's not ready, they don't support `private_key_jwt`, and adopting them means adopting AtprotoTypes/AtprotoClient too.
314- **Not removing `clientAuthMethod` or `AuthProxy.swift`.** Atprosphere's production OAuth depends on the confidential-client + auth-proxy path.
315- **Not changing the public API surface in Atprosphere/effem.** Steps 1–5 are additive or internal. Only Step 6's deprecation removal is breaking, and that's gated behind a SemVer bump.
316- **Not introducing new dependencies.** `swift-crypto` is already pulled in transitively via `jwt-kit`; everything else is Foundation.
317
318## Risk + rollback
319
320| Step | Risk | Rollback |
321| --- | --- | --- |
322| 1 (add DPoPSigner) | Low — new file, not wired in. | Delete the file. |
323| 2 (wire in DPoPSigner) | Medium — touches every authenticated request. | Revert; deprecated accessors still work. |
324| 3 (URL canonicalization) | Low — pure refactor. | Revert. |
325| 4 (extract validator) | Low — pure refactor. | Revert. |
326| 5 (resolver protocol) | Low — additive. | Revert. |
327| 6 (file split + deprecation removal) | Medium — public API change. | SemVer-major bump means consumers opt in. |
328
329## Order of work, in PR-size chunks
330
3311. **PR 1**: Step 1 — `DPoPSigner` + tests. Standalone, no consumer changes. **DONE**, bundled with PR 2.
3322. **PR 2**: Step 2 — wire `DPoPSigner` into `Networking.swift` + `ATProtoOAuth.swift`. Largest diff. Verify against Atprosphere + effem before merge. **DONE**.
3333. **PR 3**: Steps 3 + 4 — URL canonicalization + token validator. Small refactor PR. **DONE**.
3344. **PR 4**: Step 5 — resolver protocol + offline tests. **DONE**.
3355. **PR 5** (SemVer-major): Step 6 — file split and deprecation removal. **DONE**.
3366. **PR 6** (housekeeping): Step 7 — auto-memory refresh. **DONE**.
337
338Steps 1–5 are non-breaking. Step 6 is breaking and can wait until you have another reason to bump the major version.
339
340## Notes from the Steps 1+2 implementation
341
342- **Renamed the new actor to `DPoPProofSigner`.** OAuthenticator already exports `public final class DPoPSigner` (its `JWTGenerator` typealias is the entry point we hand to `TokenHandling`), so the plan's name would have collided. The new file is `Sources/CoreATProtocol/OAuth/DPoPProofSigner.swift`; everywhere the plan says `DPoPSigner` for our new actor, the code says `DPoPProofSigner`. OAuthenticator's `DPoPSigner` keeps its name and role (per-flow nonce holder for `Authenticator`).
343- **Added an `explicitNonce` parameter to `ProofParameters`.** The auth-server flow runs through `OAuthenticator.DPoPSigner`, which already tracks the auth-server nonce for the exchange and passes it via `JWTParameters.nonce`. We forward that as `explicitNonce` so the call doesn't consult the per-origin cache (which is the XRPC layer's). The XRPC layer in `Networking.swift` calls `sign(_:)` without an `explicitNonce` and the cache is consulted normally.
344- **Did not deprecate `dpopPrivateKey` / `dpopKeys` / `dpopNonceStore` yet.** The plan calls for `@available(*, deprecated, ...)` on those public properties, but applying it now would require `@available`-aware suppression at the write sites in `setDPoPPrivateKey(pem:)`. Cleaner to defer until Step 6, which removes them outright. The new `dpopProofSigner` accessor is the production read path; the legacy fields are dual-written in `setDPoPPrivateKey(pem:)` purely for source compatibility with external readers.
345- **Dropped the integration test from Step 2.** The plan suggested an end-to-end test that drives `APRouterDelegate.didReceiveErrorResponse` then `intercept` and verifies the cached nonce flows through. Implementation revealed that any test touching `ATProtoSession.shared` races with other suites that call `await ATProtoSession.shared.reset()` (`NonceDetectionTests`, `RefreshLoginTests`, `DPoPTests`). Swift Testing's `.serialized` trait only orders within a suite, not across suites. Two paths are open here: (1) globally serialize all session-touching suites, or (2) make the session instance-based so tests can spin up isolated sessions. (2) is the long-term fix and is closer to what's contemplated in `APEnvironment.swift`'s "future major release will make it instance-based" comment. Until then, the per-origin caching is fully covered by the unit tests in `DPoPProofSignerTests`, and the wiring through `Networking.swift` is straightforward delegation (the kind of code production traffic exercises immediately).
346- **Verified consumers.** bskyKit and EffemKit were rebuilt against the local CoreATProtocol checkout via a temporary `.package(path:)` swap — both succeeded. Atprosphere and the effem iOS app are `.xcodeproj` consumers; their call sites were inspected and only touch preserved public APIs (`setDPoPPrivateKey(pem:)`, `ATProtoOAuth(config:storage:)`, `ATProtoOAuthConfig`, `ATProtoOAuthError`, `ATProtoSession.shared.host`, `ATProtoSession.shared.routerDelegate`).
347- **Updated test:** `nonceCacheBounded` was rewritten — the original draft asserted "≥ 1 of the 5 oldest origins was evicted," which was probabilistic since Swift dictionaries don't guarantee key ordering. The new assertion is the deterministic invariant: after 30 insertions into a 25-slot cache, exactly 25 entries survive.
348
349## Notes from the Steps 3+4 implementation
350
351- **Deleted the `normalizedOrigin` wrappers entirely** rather than having them delegate. `DPoPProofSigner.origin(for:)`, `ATProtoOAuth.normalizedOrigin(_:)`, and `IdentityResolver.normalizedOrigin(from:)` are gone; their five call sites now invoke `URLOrigin.normalized` directly. Wrappers that only forward to a one-liner are noise.
352- **`TokenValidator` is in its own file** — `Sources/CoreATProtocol/OAuth/TokenValidator.swift`. The plan placed it as a private nested type in `ATProtoOAuth.swift`, but the file is already 970+ lines and the validator is unit-tested directly (`TokenValidatorTests`), so a separate file is cleaner. The struct is `internal` (not `private`) so tests reach it via `@testable import CoreATProtocol`.
353- **`OAuthTokenResponse` was bumped from `private` to `internal` (and `Sendable`).** Tests construct it via the synthesized memberwise init — much simpler than building JSON and round-tripping through `Codable`. `OAuthTokenRequest` and `OAuthRefreshTokenRequest` stay `private`; nothing outside `ATProtoOAuth.swift` needs them.
354- **Issuer validation stays inline in `loginProvider`, not in `TokenValidator`.** The plan implied both providers shared an issuer check, but they don't: the auth-callback flow validates the `iss` query parameter (which the validator never sees), and the refresh flow has no `iss` to validate against — the auth-server URL is fixed by the time refresh runs. The validator handles only the response-payload checks (token type, scope, sub) that genuinely duplicated. Net change: ~10 lines of duplicated guards collapsed to two `validator.validate(_:)` calls.
355- **Tests added:** `TokenValidatorTests` covers the success path, wrong token type, missing `atproto` scope, sub mismatch, sub passthrough when expected DID is nil, and a multi-scope success case. Six tests, all sub-millisecond. Total suite is now 72 tests.
356
357## Notes from the Step 5 implementation
358
359- **Protocol shape matches the existing concrete API.** `ATProtoIdentityResolver` exposes only the two methods `ATProtoOAuth` actually calls — `resolve(identifier:)` and `isAuthorizationServer(_:validFor:)`. The concrete `IdentityResolver`'s extras (`resolve(handle:)`, `resolve(did:)`) stay off the protocol; tests that need them keep using the concrete type.
360- **Optional parameter, not an autoclosure default.** Plan's signature was `identityResolver: ATProtoIdentityResolver = IdentityResolver()`. Default expressions on a global-actor-isolated `init` get murky in Swift 6 (the expression is evaluated relative to the function's isolation, but `IdentityResolver()` is itself `@APActor`-isolated). Sidestepped with `identityResolver: (any ATProtoIdentityResolver)? = nil` and `self.identityResolver = identityResolver ?? IdentityResolver()` in the body. Same caller ergonomics, no isolation puzzle.
361- **`ResolvedIdentity` stays nested in `IdentityResolver`.** Lifting it to a top-level `ATProtoResolvedIdentity` would be cleaner but renames a public type. Deferred to Step 6 (which is already a SemVer-major bump). The protocol references `IdentityResolver.ResolvedIdentity` directly.
362- **Live tests are opt-in via env var, not removed.** Plan suggested `.disabled(if:)` or a tag. Used a single env-var gate: `CORE_ATPROTOCOL_LIVE_TESTS=1` runs the four live network tests (`testResolveHandle`, `testHandleCleaning`, `testServerMetadataLoad`, `testClientMetadataLoad`); without it they're skipped and the deterministic suite stays at ~80ms. The smoke-test value of those tests is preserved for local debugging without polluting CI.
363- **Three new offline tests in `IdentityResolverProtocolTests`.** A `StubIdentityResolver` round-trips canned values through both protocol methods, plus one test verifies `ATProtoOAuth.init(config:storage:identityResolver:)` accepts the injected resolver. End-to-end behavior with the fake can't be tested offline because `authenticate(...)` still goes to live network for client/server metadata — that's a future seam.
364- **No consumer changes.** The new `identityResolver:` parameter has a default, so all existing `await ATProtoOAuth(config: ..., storage: ...)` and `try await ATProtoOAuth(config: ..., storage: ..., privateKeyPEM: ...)` call sites compile unchanged. bskyKit, EffemKit, Atprosphere, and effem-iOS all good.
365
366## Notes from the Steps 6+7 implementation
367
368- **Removed more than just the DPoP-deprecated surface.** Plan Step 6 listed `ATProtoSession.dpopPrivateKey` / `dpopKeys` / `dpopNonceStore` and the `DPoPNonceStore` type as the things to delete. While we were already breaking, also deleted the orphan `APEnvironment` typealias and the `ATProtoSession.current` accessor (both `@available(*, deprecated, renamed:)` shims from a previous rename). Confirmed by grep that no consumer references either symbol.
369- **`DPoPNonceStoreTests` deleted; the test file was renamed.** The old `Tests/CoreATProtocolTests/DPoPStoreTests.swift` only had `ClockSkewStoreTests` left after removing the nonce-store tests, so it became `ClockSkewStoreTests.swift`. Test count dropped from 75 → 71 (lost 4 tests for the deleted store; gained nothing).
370- **File split kept `DPoPRequestActor` in the main file.** Plan didn't specify where to put it; moving it to the extension file would have required bumping its access level. It's a small infra type used by `refreshLoginIfNeeded`, so it stays alongside its only caller in `ATProtoOAuth.swift`.
371- **`validateIssuer` bumped from `private` to `internal`.** Cross-file extension access for `private` doesn't work in Swift — the `loginProvider` (now in `ATProtoOAuth+TokenHandling.swift`) needs to call `Self.validateIssuer(...)`, and the source-of-truth method lives in the main file (also called from `authenticate` and `refreshLoginIfNeeded` there). `internal` keeps it module-scoped.
372- **Final layout:** `ATProtoOAuth.swift` (720 lines — public types + class + auth orchestration + `DPoPRequestActor` + `validateIssuer`), `ATProtoOAuth+TokenHandling.swift` (186 lines — `buildTokenHandling`, the three providers, `decodeTokenResponse`, and `URLSession.defaultProvider`), `TokenModels.swift` (73 lines — the three Codable structs). The plan's "~400 lines" estimate for `ATProtoOAuth.swift` was light; the public types + orchestration logic are unavoidable.
373- **Auto-memory restructured to match the rules in the system prompt.** The previous `MEMORY.md` had inline content (the auto-memory rules require it to be a flat index pointing at typed memory files). Dropped six stale Plume-app entries (Plume isn't in this checkout), kept the generic `feedback_review_struct_vs_class.md`, added four new files: `feedback_dpop_per_origin_nonce.md`, `project_consumer_oauth_modes.md`, `feedback_atproto_session_singleton.md`, `reference_live_tests_env_var.md`.
374- **Stale `.docc` reference noted but not fixed.** `effem/EffemKit/Sources/EffemKit/EffemKit.docc/Authentication.md` still references `APEnvironment.current.dpopPrivateKey` (an API that's been gone for a while now). It's documentation, not code, so the build is fine. Worth a follow-up cleanup in the effem repo when someone touches that doc file.
375
376## What didn't make this plan
377
378The most obvious payoff still on the table is **making `ATProtoSession` instance-based** (already flagged in `APEnvironment.swift`'s docstring as "a future major release"):
379
380- The singleton is what blocked the integration test from Step 2 (cross-suite `reset()` calls clobber state).
381- A scoped session would make the routing layer testable end-to-end without env-var gates.
382- Consumers would need updates: `setup()`, `updateTokens()`, `setDPoPPrivateKey(pem:)`, `setDelegate()`, `setTokenRefreshHandler()`, `update(hostURL:)` would all become methods on a session instance, and `APRouterDelegate` would need to be initialized with a session reference rather than reading `ATProtoSession.shared` directly.
383- It's a bigger lift than any single step in this plan — touches every consumer and the entire networking layer.
384
385Other smaller follow-ups:
386- Lift `IdentityResolver.ResolvedIdentity` to a top-level `ATProtoResolvedIdentity` (renames a public nested type; pairs naturally with the next major bump).
387- The `effem` `EffemKit.docc` references stale `APEnvironment` symbols (see Steps 6+7 notes).
388- Add a Slingshot/community DoH fallback resolver — the `ATProtoIdentityResolver` seam exists for exactly this; not built yet because nobody needs it.
389- Consider extracting `authenticate` and `refreshLoginIfNeeded` into their own extension file. They're ~200 lines together; the main file would shrink to ~520. Pure organizational; only worth doing if `ATProtoOAuth.swift` starts feeling unwieldy again.