Skip to content

Commit af16058

Browse files
wolpertclaude
andcommitted
tier 2: adapter parity & SPI consistency
#9 — Dropwizard auto-wires OTP/MagicLink/BackupCode via AltFlowsModule + new bundle ctor #10 — shared BackupCodesCountResponse / EmailVerificationResult in pk-auth-admin-api #11 — shared AdminRequests records in pk-auth-admin-api; all three adapters consume them #12 — AdminErrorBody + test deleted; DESIGN.md error envelope aligned with mappers #14 — new OtpPepperResolver in pk-auth-otp; Spring + Micronaut adapters share it #16 — Micronaut credential-id path template renamed to {credentialId} #17 — Spring toResponse collapsed into single entrypoint auto-detecting null → 204 #19 — RelyingParty defaults dropped in Spring and Micronaut; fail fast #20 — JWT defaults dropped; new JwtSecretResolver in pk-auth-jwt; Spring random-key fallback removed #21 — typed boolean devMode added to PkAuthProperties / PkAuthConfiguration #26 — ChallengeStore.put Javadoc + TTL validation; all three impls enforce #27 — BackupCodeRepository.replaceAll abstract; DynamoDB impl uses TransactWriteItems #29 — UserLookup methods renamed; USERNAMELESS_KEY constant added (#36 bundled) #30 — AssertionResult.Success convenience constructor removed Blocked: #13, #15, #22, #23, #24, #25, #31 — deferred per user pause Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 23e3e27 commit af16058

63 files changed

Lines changed: 1846 additions & 587 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

DESIGN.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,15 @@ framework-specific on the wire.
166166
`ChallengeId` so adapters using Jackson 3 get the right shape
167167
automatically. The Dropwizard adapter still rides Jackson 2 and uses
168168
a small bridge module (`PkAuthJacksonBridge`) for the same effect.
169-
- **Errors**: `4xx` with a JSON body `{ "error": "<kind>", "detail":
170-
"..." }`. Common kinds: `validation_failed`, `origin_mismatch`,
171-
`counter_regression`, `challenge_expired`, `conflict`,
172-
`rate_limited`.
169+
- **Errors**: `4xx` with a JSON body `{ "outcome": "<kind>", "error":
170+
"<kind>", "detail": "..." }`. `outcome` and `error` both carry the
171+
same machine-readable tag so clients keyed off either field keep
172+
working; `detail` is present only when the result variant carried
173+
one. Common kinds: `validation_failed`, `origin_mismatch`,
174+
`counter_regression`, `challenge_expired`, `conflict`, `forbidden`,
175+
`not_found`, `rate_limited`. `rate_limited` is paired with a
176+
`Retry-After` response header. The adapter `PkAuthAdminResultMapper`
177+
classes are the source of truth.
173178
- **Idempotence**: `finish` endpoints are *not* idempotent — challenges
174179
are single-use (`ChallengeStore.takeOnce`).
175180

TODO.md

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ These pre-answer questions that would otherwise come up mid-implementation:
146146

147147
## Tier 2 — Adapter parity & API/SPI consistency (high-impact public surface)
148148

149-
### 9. Dropwizard adapter does not auto-wire OTP / MagicLink / BackupCode services
149+
### ✅ 9. Dropwizard adapter does not auto-wire OTP / MagicLink / BackupCode services
150+
**Completed:** 2026-05-16 — added new AltFlowsModule + PkAuthFullComponent; PkAuthBundle gets an alt-flow constructor; demo updated.
150151
- Severity: **High**[API #1]
151152
- Files: `pk-auth-dropwizard/.../dagger/PkAuthModule.java`; `PkAuthBundle.java:72`.
152153
- Issue: Spring and Micronaut auto-wire all three; Dropwizard hosts must
@@ -156,7 +157,8 @@ These pre-answer questions that would otherwise come up mid-implementation:
156157
`PkAuthModule`, and a `PkAuthBundle` constructor that constructs
157158
`DefaultAdminService` internally.
158159

159-
### 10. Unify admin response body shape across adapters
160+
### ✅ 10. Unify admin response body shape across adapters
161+
**Completed:** 2026-05-16 — promoted `BackupCodesCountResponse` and `EmailVerificationResult` into pk-auth-admin-api; all three adapters now emit identical JSON.
160162
- Severity: **High**[API #3]
161163
- Files: Spring `PkAuthAdminController.java:85-94, 105-115` vs Dropwizard
162164
`:100-104` vs Micronaut `:91-95`.
@@ -168,7 +170,8 @@ These pre-answer questions that would otherwise come up mid-implementation:
168170
(`BackupCodesCountResponse(int remaining)`, `EmailVerificationResult(String userHandle)`).
169171
Spring's `Map.of(...)` shape is the right shape.
170172

171-
### 11. Promote admin request-body DTOs into `pk-auth-admin-api`
173+
### ✅ 11. Promote admin request-body DTOs into `pk-auth-admin-api`
174+
**Completed:** 2026-05-16 — new `AdminRequests` class with five nested records; Spring/Dropwizard/Micronaut adapters use them.
172175
- Severity: **High**[API #2] and [Maint #19] (same finding, two agents)
173176
- Files: Spring `:144-157` (`RenameBody`, `EmailBody`, ...); Dropwizard `:35-47`
174177
(`RenameRequest`, `EmailStartRequest`, ...); Micronaut `:170-178`
@@ -177,7 +180,8 @@ These pre-answer questions that would otherwise come up mid-implementation:
177180
silently breaks parity.
178181
- Fix: move records into `pk-auth-admin-api` as `AdminRequests.RenameCredential` etc.
179182

180-
### 12. Delete `AdminErrorBody` and align the documented schema with what adapters emit
183+
### ✅ 12. Delete `AdminErrorBody` and align the documented schema with what adapters emit
184+
**Completed:** 2026-05-16 — AdminErrorBody.java + test deleted; DESIGN.md error-envelope description rewritten to match adapter mappers.
181185
- Severity: **High**[API #4] + [DeadCode #1] (conflict **resolved: delete**)
182186
- Files: `pk-auth-admin-api/.../AdminErrorBody.java` and its test
183187
`AdminErrorBodyTest.java`; `*AdminResultMapper.java` in all three adapters.
@@ -188,7 +192,8 @@ These pre-answer questions that would otherwise come up mid-implementation:
188192
- Follow-up: after deletion, sweep any docs that referenced the documented
189193
`{error, detail}` schema so they match what the mappers emit.
190194

191-
### 13. Extract `CeremonyOrchestrator` / `AdminFlow` to remove ~600 lines of triplicated adapter code
195+
### ⚠️ 13. Extract `CeremonyOrchestrator` / `AdminFlow` to remove ~600 lines of triplicated adapter code
196+
**Blocked:** not attempted in this run — deferred per user request to pause after Tier 2.
192197
- Severity: **High**[Maint #1, #3, #7]
193198
- Files: ceremony controllers (`PkAuthCeremonyController` x3 + Dropwizard's
194199
resource) and admin controllers / `PkAuthAdminResultMapper` x3 / inline
@@ -201,14 +206,16 @@ These pre-answer questions that would otherwise come up mid-implementation:
201206
- Related: [API #29] (errorEnvelope duplication), [Maint #19] (request DTOs),
202207
[Maint #20] (rate-limiter classes), [Maint #18] (`currentUser()` patterns).
203208

204-
### 14. De-duplicate `resolveOtpPepper` between Spring and Micronaut adapters
209+
### ✅ 14. De-duplicate `resolveOtpPepper` between Spring and Micronaut adapters
210+
**Completed:** 2026-05-16 — new `OtpPepperResolver` in pk-auth-otp; Spring + Micronaut adapters now call it.
205211
- Severity: **High**[Maint #2]
206212
- Files: Spring `PkAuthAutoConfiguration.java:298-332`,
207213
Micronaut `PkAuthFactory.java:180-214` — 35 lines verbatim copy.
208214
- Fix: extract `OtpPepperResolver.resolve(Supplier<String> configuredPepper, BooleanSupplier devMode)`
209215
to `pk-auth-otp`. Adapters become a two-line call.
210216

211-
### 15. Decompose oversized `finishRegistration` and `finishAuthentication`
217+
### ⚠️ 15. Decompose oversized `finishRegistration` and `finishAuthentication`
218+
**Blocked:** worktree merge conflicted heavily with Tier 1 changes to DefaultPasskeyAuthenticationService (rate limiter, enum leak fix). Agent work exists on branch `worktree-agent-a082f9cbecf7dd83a` (now removed). Re-do as a focused PR.
212219
- Severity: **High**[Maint #4]
213220
- File: `pk-auth-core/.../internal/DefaultPasskeyAuthenticationService.java:193-302` (110 lines), `:354-461` (108 lines).
214221
- Issue: each method does ~seven things and contradicts its own Javadoc's
@@ -218,14 +225,16 @@ These pre-answer questions that would otherwise come up mid-implementation:
218225
`persistRegistration(...)` so `finish…` becomes a ~20-line orchestration.
219226
- Related: [Maint #24] (narrow the `RuntimeException` catch when extracting).
220227

221-
### 16. Normalize credential-id URL path-template name across adapters
228+
### ✅ 16. Normalize credential-id URL path-template name across adapters
229+
**Completed:** 2026-05-16 — Micronaut path template renamed from `{credentialIdB64}` to `{credentialId}`.
222230
- Severity: **High**[API #7]
223231
- Files: Spring `:60` & Dropwizard `:71` use `{credentialId}`; Micronaut `:66`
224232
uses `{credentialIdB64}`. Runtime URL is the same; OpenAPI / SDK tooling
225233
isn't.
226234
- Fix: rename Micronaut to `{credentialId}`.
227235

228-
### 17. Eliminate the Spring `toEmptyResponse` foot-gun
236+
### ✅ 17. Eliminate the Spring `toEmptyResponse` foot-gun
237+
**Completed:** 2026-05-16 — collapsed into single `toResponse` that auto-detects null value → 204.
229238
- Severity: **High**[API #8]
230239
- File: `pk-auth-spring-boot-starter/.../PkAuthAdminResultMapper.java:25-60`.
231240
- Issue: caller has to choose `toResponse` vs `toEmptyResponse` for
@@ -241,7 +250,8 @@ These pre-answer questions that would otherwise come up mid-implementation:
241250
- Fix: declare `DuplicateCredentialException extends PkAuthPersistenceException`
242251
(combines naturally with #8) and update all three impls.
243252

244-
### 19. Drop RelyingParty defaults in Spring and Micronaut — adopt Dropwizard's fail-fast
253+
### ✅ 19. Drop RelyingParty defaults in Spring and Micronaut — adopt Dropwizard's fail-fast
254+
**Completed:** 2026-05-16 — Spring `localhost` and Micronaut `example.com` defaults removed; missing values now fail fast.
245255
- Severity: **Med**[API #18]
246256
- Files: Spring `PkAuthProperties.java:48-51` (`localhost`), Dropwizard
247257
`PkAuthConfig.java:23-29` (required), Micronaut `PkAuthConfiguration.java:75-77`
@@ -250,7 +260,8 @@ These pre-answer questions that would otherwise come up mid-implementation:
250260
framework binding fail fast when `pkauth.relying-party.{id,name,origins}`
251261
is missing.
252262

253-
### 20. Drop JWT defaults; throw on missing secret in all three adapters
263+
### ✅ 20. Drop JWT defaults; throw on missing secret in all three adapters
264+
**Completed:** 2026-05-16 — Spring's silent random-key fallback gone; new `JwtSecretResolver` in pk-auth-jwt centralizes fail-fast policy.
254265
- Severity: **Med**[API #19] and [Maint #23]
255266
- Files: Spring `PkAuthProperties.java:66-69` (`issuer="pk-auth"`,
256267
`audience="pk-auth-clients"`), Dropwizard required, Micronaut
@@ -261,14 +272,16 @@ These pre-answer questions that would otherwise come up mid-implementation:
261272
defaults. Centralise the resolver in `pk-auth-jwt` so all three adapters
262273
share one fail-fast policy.
263274

264-
### 21. Add typed `dev-mode` property to Spring and Micronaut configs
275+
### ✅ 21. Add typed `dev-mode` property to Spring and Micronaut configs
276+
**Completed:** 2026-05-16 — `boolean devMode` field added and bound on both adapters; string-property reads removed.
265277
- Severity: **Med**[API #20]
266278
- Issue: `dev-mode` is consumed via `@ConditionalOnProperty` / `Environment.getProperty`
267279
but never bound on `PkAuthProperties` / `PkAuthConfiguration`. IDE / metadata
268280
tooling can't help hosts find it.
269281
- Fix: add `boolean devMode` field and bind it; remove string-property reads.
270282

271-
### 22. Stop using `byte[]` in result records — use `CredentialId` / `UserHandle`
283+
### ⚠️ 22. Stop using `byte[]` in result records — use `CredentialId` / `UserHandle`
284+
**Blocked:** not attempted in this run — deferred per user pause.
272285
- Severity: **Med**[API #10], [API #11], [API #17]
273286
- Files: `AssertionResult.java:34, 86`; `RegistrationResult.java:48`;
274287
`CredentialSummary.java:25-27`; `UserHandle.java`; `CeremonyWireMapper.java:75, 121`.
@@ -281,7 +294,8 @@ These pre-answer questions that would otherwise come up mid-implementation:
281294
(base64url string) in `PkAuthObjectMappers`; switch the record fields to the
282295
value classes; delete the manual encodings.
283296

284-
### 23. Standardize on `start*/finish*` across every feature service
297+
### ⚠️ 23. Standardize on `start*/finish*` across every feature service
298+
**Blocked:** not attempted in this run — deferred per user pause.
285299
- Severity: **Med**[API #9], [API #13]
286300
- Files: ceremony (already `start*`/`finish*`), OTP (`send`/`verify`),
287301
magic-link (`send*`/`consume`), admin (`start*`/`complete*`),
@@ -301,29 +315,33 @@ These pre-answer questions that would otherwise come up mid-implementation:
301315
- Result-type names align too: `*Result.Success` / `Invalid` /
302316
`AlreadyFinished` across the board.
303317

304-
### 24. Unify service-construction patterns (single `create(Dependencies, Config)` factory)
318+
### ⚠️ 24. Unify service-construction patterns (single `create(Dependencies, Config)` factory)
319+
**Blocked:** worktree branch off c7064d1 conflicts heavily with Tier 1 SPI changes (MagicLinkService gained ConsumedJtiStore, BackupCodeRepository `consume` returns boolean, etc.). Agent work exists on branch `worktree-agent-a40954b7fe3a473f2`. Re-do as a focused PR after Tier 1 baseline settles.
305320
- Severity: **Med**[API #12] and [Maint #6]
306321
- Files: `OtpService` (2 ctors), `MagicLinkService` (3), `BackupCodeService` (4),
307322
`DefaultAdminService` (3 statics + `Dependencies`), `PasskeyAuthenticationServices`
308323
(Builder).
309324
- Fix: adopt `DefaultAdminService`'s "deps + optional config" idiom uniformly.
310325

311-
### 25. Generalize `EmailSender` and `SmsSender` SPIs
326+
### ⚠️ 25. Generalize `EmailSender` and `SmsSender` SPIs
327+
**Blocked:** worktree branch off c7064d1 conflicts with Tier 1. Agent work exists on branch `worktree-agent-a54b3074c291a9d8f`. Re-do as a focused PR.
312328
- Severity: **Med**[API #14]
313329
- Files: `EmailSender.java:11` (`sendMagicLink(to, subject, body)`),
314330
`SmsSender.java:12` (`sendOtp(to, message)`).
315331
- Fix: rename to `send(String to, String subject, String body)` /
316332
`send(String to, String body)`; move message construction (including the
317333
hard-coded OTP body string in `OtpService.send`) into a `MessageFormatter` SPI.
318334

319-
### 26. Document and constrain `ChallengeStore.put` semantics
335+
### ✅ 26. Document and constrain `ChallengeStore.put` semantics
336+
**Completed:** 2026-05-16 — Javadoc specifies overwrite-on-duplicate-key and rejects zero/negative TTL; all three impls validate.
320337
- Severity: **Med**[API #15]
321338
- File: `pk-auth-core/.../spi/ChallengeStore.java:15`
322339
- Issue: `put` has no Javadoc; `takeOnce` does. Overwrite-on-retry behaviour is
323340
unspecified — replay-protection-relevant.
324341
- Fix: document overwrite vs reject; specify zero/negative TTL behaviour.
325342

326-
### 27. Tighten `BackupCodeRepository.replaceAll` — make it abstract
343+
### ✅ 27. Tighten `BackupCodeRepository.replaceAll` — make it abstract
344+
**Completed:** 2026-05-16 — default removed; method is abstract; DynamoDB impl now uses TransactWriteItems for atomicity.
327345
- Severity: **Med**[API #16]
328346
- Issue: default does non-transactional delete-then-loop-save with "implementers
329347
SHOULD override" — silent foot-gun for new SPI implementers.
@@ -337,17 +355,20 @@ These pre-answer questions that would otherwise come up mid-implementation:
337355
the `>` comparator (#1) this is the same bug at the SPI level.
338356
- Fix: change to `OptionalInt` and require the caller to handle "no row".
339357

340-
### 29. Rename `UserLookup` methods for parallelism and intent
358+
### ✅ 29. Rename `UserLookup` methods for parallelism and intent
359+
**Completed:** 2026-05-16 — renamed to `findHandleByUsername`/`findViewByHandle`/`getOrCreateHandle`; promoted `__usernameless__` to `USERNAMELESS_KEY` constant (item #36 bundled).
341360
- Severity: **Med**[API #22]
342361
- Fix: `findHandleByUsername`, `findViewByHandle`, `getOrCreateHandle`. Also
343362
promote the magic string `"__usernameless__"` to a constant (see #36).
344363

345-
### 30. Drop `AssertionResult.Success` convenience constructor (or move to static)
364+
### ✅ 30. Drop `AssertionResult.Success` convenience constructor (or move to static)
365+
**Completed:** 2026-05-16 — convenience 3-arg constructor removed; callers must supply `counterStatus` explicitly.
346366
- Severity: **Med**[API #24]
347367
- Fix: force callers to supply `counterStatus` explicitly, or expose
348368
`Success.ok(...)` as a static factory.
349369

350-
### 31. Stamp `@since 0.9.1` on every new/modified public element this sweep
370+
### ⚠️ 31. Stamp `@since 0.9.1` on every new/modified public element this sweep
371+
**Blocked:** stamps were applied to every element each prior agent touched, but a final sweep + CONTRIBUTING.md update was not run. Deferred per user pause.
351372
- Severity: **Med**[API #21]
352373
- Issue: zero `@since` tags across `pk-auth-core`, `pk-auth-admin-api`, `pk-auth-jwt`,
353374
`pk-auth-otp`, `pk-auth-magic-link`, `pk-auth-backup-codes`.
@@ -377,7 +398,8 @@ These pre-answer questions that would otherwise come up mid-implementation:
377398
- Severity: **Med**[Maint #12]
378399
- Files: `MagicLinkService.java:310-316`, `FakeAuthenticator.java:225`.
379400

380-
### 36. Promote the `"__usernameless__"` magic string to a constant; rename `stored2`
401+
### ✅ 36. Promote the `"__usernameless__"` magic string to a constant; rename `stored2`
402+
**Completed:** 2026-05-16 — bundled with #29; magic string is now `UserLookup.USERNAMELESS_KEY`. (`stored2` rename not done — defer.)
381403
- Severity: **Med**[Maint #10]
382404

383405
### 37. Replace metric-tag string concatenation with a `Ceremony` enum

0 commit comments

Comments
 (0)