From ac3fd1cc4107ae67064267e53818bcd94881d489 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 06:59:56 -0700 Subject: [PATCH 01/12] fix(dynamodb): only treat freshness-condition failures as race-lost rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rotateAtomically previously caught every TransactionCanceledException and returned false, signalling "race lost" to the caller — which scorches the token family. A transient cancellation (throughput, transaction conflict, validation) would therefore be misread as a replay and silently revoke a legitimate family. Now inspect cancellationReasons(): return false only when the parent's freshness condition (index 0) is ConditionalCheckFailed, and rethrow anything else so it maps to a retryable 5xx. Fail closed when the reason is undeterminable. Replay detection is unchanged (a genuinely used/revoked/expired parent still surfaces as ConditionalCheckFailed). Also guard the two full-table scans (refresh deleteExpiredBefore, access-token deleteExpiredBefore) with a server-side begins_with(pk,...) filter so non-matching rows of the shared table stay off the wire, replace the brittle regionMatches(...,3) prefix test with startsWith, route used/ revoked instant fields through the central encode helpers, and remove dead code (UNUSED_CONDITION, an unused local, snake_case helper name). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../dynamodb/DynamoDbAccessTokenStore.java | 21 ++++++- .../DynamoDbRefreshTokenRepository.java | 62 +++++++++++++++---- 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbAccessTokenStore.java b/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbAccessTokenStore.java index 71694e0..99a000b 100644 --- a/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbAccessTokenStore.java +++ b/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbAccessTokenStore.java @@ -9,9 +9,12 @@ import java.util.Optional; import software.amazon.awssdk.enhanced.dynamodb.DynamoDbEnhancedClient; import software.amazon.awssdk.enhanced.dynamodb.DynamoDbTable; +import software.amazon.awssdk.enhanced.dynamodb.Expression; import software.amazon.awssdk.enhanced.dynamodb.Key; import software.amazon.awssdk.enhanced.dynamodb.TableSchema; import software.amazon.awssdk.enhanced.dynamodb.model.QueryConditional; +import software.amazon.awssdk.enhanced.dynamodb.model.ScanEnhancedRequest; +import software.amazon.awssdk.services.dynamodb.model.AttributeValue; /** * {@link AccessTokenStore} backed by the {@code PkAuthCore} single table. Each issued JTI is @@ -184,8 +187,14 @@ public int deleteExpiredBefore(Instant before) { () -> { long beforeEpoch = before.getEpochSecond(); int[] removed = {0}; - table.scan().items().stream() - .filter(item -> DynamoKeys.AT.regionMatches(0, item.getPk(), 0, 3)) + // The server-side begins_with filter keeps non-AT# rows of the shared table off the wire; + // a scan still consumes read capacity proportional to table size, so operators should + // prefer native TTL and treat this as a test/maintenance path. + table + .scan(ScanEnhancedRequest.builder().filterExpression(primaryItemsOnly()).build()) + .items() + .stream() + .filter(item -> item.getPk() != null && item.getPk().startsWith(DynamoKeys.AT)) .filter(item -> item.getPk().equals(item.getSk())) // primary items only .filter(item -> item.getTtl() != null && item.getTtl() < beforeEpoch) .forEach( @@ -231,4 +240,12 @@ private static AccessTokenItem buildItem( item.setTtl(ttl); return item; } + + /** Server-side filter restricting a table scan to AT# primary items (pk begins with AT#). */ + private static Expression primaryItemsOnly() { + return Expression.builder() + .expression("begins_with(pk, :atPrefix)") + .putExpressionValue(":atPrefix", AttributeValue.fromS(DynamoKeys.AT)) + .build(); + } } diff --git a/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepository.java b/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepository.java index badc3e2..477825b 100644 --- a/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepository.java +++ b/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepository.java @@ -10,7 +10,6 @@ import com.codeheadsystems.pkauth.refresh.spi.RefreshTokenRepository; import java.time.Duration; import java.time.Instant; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -21,14 +20,15 @@ import software.amazon.awssdk.enhanced.dynamodb.Expression; import software.amazon.awssdk.enhanced.dynamodb.Key; import software.amazon.awssdk.enhanced.dynamodb.TableSchema; -import software.amazon.awssdk.enhanced.dynamodb.model.ConditionCheck; import software.amazon.awssdk.enhanced.dynamodb.model.IgnoreNullsMode; import software.amazon.awssdk.enhanced.dynamodb.model.PutItemEnhancedRequest; import software.amazon.awssdk.enhanced.dynamodb.model.QueryConditional; +import software.amazon.awssdk.enhanced.dynamodb.model.ScanEnhancedRequest; import software.amazon.awssdk.enhanced.dynamodb.model.TransactPutItemEnhancedRequest; import software.amazon.awssdk.enhanced.dynamodb.model.TransactUpdateItemEnhancedRequest; import software.amazon.awssdk.enhanced.dynamodb.model.TransactWriteItemsEnhancedRequest; import software.amazon.awssdk.services.dynamodb.model.AttributeValue; +import software.amazon.awssdk.services.dynamodb.model.CancellationReason; import software.amazon.awssdk.services.dynamodb.model.ConditionalCheckFailedException; import software.amazon.awssdk.services.dynamodb.model.TransactionCanceledException; @@ -57,6 +57,8 @@ public final class DynamoDbRefreshTokenRepository implements RefreshTokenReposit // named separately because they play distinct roles in the single-table layout. private static final String PRIMARY_PK_PREFIX = DynamoKeys.RT; private static final String INDEX_SK_PREFIX = DynamoKeys.RT; + // DynamoDB's code() for a cancellation reason whose ConditionExpression evaluated false. + private static final String CONDITIONAL_CHECK_FAILED = "ConditionalCheckFailed"; private final DynamoDbEnhancedClient enhanced; private final DynamoDbTable table; @@ -158,7 +160,7 @@ public boolean rotateAtomically( RefreshTokenItem successorUser = toItem( successor, - USER_PK_PREFIX + successor_userB64(successor), + USER_PK_PREFIX + successorUserB64(successor), INDEX_SK_PREFIX + successor.refreshId()); RefreshTokenItem successorFamily = toItem( @@ -188,11 +190,37 @@ public boolean rotateAtomically( .build()); return true; } catch (TransactionCanceledException cancelled) { - return false; + // Distinguish a genuine freshness-condition failure (the parent was already used, + // revoked, or expired — a real replay/race that SHOULD return false and let the caller + // scorch the family) from a transient cancellation (throughput, transaction conflict, + // validation). Only the former is "race lost". Surfacing the latter as false would let + // a momentary throughput blip be misread as a replay and silently revoke a legitimate + // token family; rethrowing instead maps it (via DynamoDbSupport.wrap) to a 5xx the + // client can retry. When the reason is undeterminable, fail closed by rethrowing. + if (isParentFreshnessFailure(cancelled)) { + return false; + } + throw cancelled; } }); } + /** + * True only when the {@code rotateAtomically} transaction was cancelled because the parent's + * freshness condition failed — the legitimate replay/race signal. The parent's conditional + * {@code UpdateItem} is the first action added to the transaction, so its reason is at index 0; + * a {@code ConditionalCheckFailed} code there means the parent was already used, revoked, or + * expired. Any other cancellation reason (throughput, transaction conflict, validation) is + * transient and returns false here so the caller rethrows rather than scorching the family. + */ + private static boolean isParentFreshnessFailure(TransactionCanceledException cancelled) { + List reasons = cancelled.cancellationReasons(); + if (reasons == null || reasons.isEmpty()) { + return false; + } + return CONDITIONAL_CHECK_FAILED.equals(reasons.get(0).code()); + } + @Override public int revokeFamily(String familyId, Instant now, RevokeReason reason) { return DynamoDbSupport.wrap( @@ -366,8 +394,13 @@ public int deleteExpiredBefore(Instant cutoff) { int[] removed = {0}; // Scan only primary items (pk and sk both start with RT#) and filter by the // retention predicate that mirrors the JDBI cleanup SQL (expires_at < cutoff). This uses - // `expiresAtEpoch`, NOT the retention-extended `ttl` attribute. - table.scan().items().stream() + // `expiresAtEpoch`, NOT the retention-extended `ttl` attribute. The server-side + // begins_with filter keeps non-RT# rows of the shared table off the wire; note a scan + // still consumes read capacity proportional to table size, so operators should prefer + // native TTL and treat this as a test/maintenance path. + table.scan(ScanEnhancedRequest.builder().filterExpression(primaryItemsOnly()).build()) + .items() + .stream() .filter(item -> item.getPk() != null && item.getPk().startsWith(PRIMARY_PK_PREFIX)) .filter(item -> item.getPk().equals(item.getSk())) // primary only .filter( @@ -437,7 +470,7 @@ private void deleteAllItems(RefreshTokenItem primary) { } } - private static String successor_userB64(RefreshTokenRecord r) { + private static String successorUserB64(RefreshTokenRecord r) { return Base64Url.encode(r.userHandle().value()); } @@ -454,8 +487,8 @@ private RefreshTokenItem toItem(RefreshTokenRecord r, String pk, String sk) { item.setParentRefreshId(r.parentRefreshId().orElse(null)); item.setIssuedAtIso(DynamoDbSupport.encodeInstant(r.issuedAt())); item.setExpiresAtIso(DynamoDbSupport.encodeInstant(r.expiresAt())); - item.setUsedAtIso(r.usedAt().map(Instant::toString).orElse(null)); - item.setRevokedAtIso(r.revokedAt().map(Instant::toString).orElse(null)); + item.setUsedAtIso(r.usedAt().map(DynamoDbSupport::encodeInstant).orElse(null)); + item.setRevokedAtIso(r.revokedAt().map(DynamoDbSupport::encodeInstant).orElse(null)); item.setRevokedReason(r.revokedReason().map(Enum::name).orElse(null)); item.setAmr(Amr.encode(r.amr())); item.setExpiresAtEpoch(r.expiresAt().getEpochSecond()); @@ -464,7 +497,6 @@ private RefreshTokenItem toItem(RefreshTokenRecord r, String pk, String sk) { } private static RefreshTokenRecord toRecord(RefreshTokenItem item) { - Map ignored = new HashMap<>(); byte[] hash = Base64Url.decode(item.getTokenHashB64u()); return new RefreshTokenRecord( item.getRefreshId(), @@ -482,7 +514,11 @@ private static RefreshTokenRecord toRecord(RefreshTokenItem item) { Amr.decode(item.getAmr())); } - // Defensive: silence unused-import warnings on classes pulled in for the TransactWrite path. - @SuppressWarnings("unused") - private static final ConditionCheck UNUSED_CONDITION = null; + /** Server-side filter restricting a table scan to RT# primary items (pk begins with RT#). */ + private static Expression primaryItemsOnly() { + return Expression.builder() + .expression("begins_with(pk, :rtPrefix)") + .putExpressionValue(":rtPrefix", AttributeValue.fromS(PRIMARY_PK_PREFIX)) + .build(); + } } From aa5c325449b59b730ad2d65ae59fddd68803a713 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:00:11 -0700 Subject: [PATCH 02/12] fix(otp): drop misleading constant-time self-compare on no-active-OTP branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The no-active-OTP branch computed a throwaway HMAC and then ran MessageDigest.isEqual(dummy, dummy) — comparing a value to itself and discarding the result. The self-compare equalizes nothing the HMAC doesn't already dominate and reads as dead code. Keep the throwaway hmacHash(candidate) (the dominant cost that hides whether an OTP exists for the user), drop the self-compare. Mirrors the dummy-Argon2 verify in BackupCodeService. Timing resistance is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../com/codeheadsystems/pkauth/otp/OtpService.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pk-auth-otp/src/main/java/com/codeheadsystems/pkauth/otp/OtpService.java b/pk-auth-otp/src/main/java/com/codeheadsystems/pkauth/otp/OtpService.java index 36dd960..14b5e8c 100644 --- a/pk-auth-otp/src/main/java/com/codeheadsystems/pkauth/otp/OtpService.java +++ b/pk-auth-otp/src/main/java/com/codeheadsystems/pkauth/otp/OtpService.java @@ -183,12 +183,12 @@ public VerifyResult finishVerification(UserHandle user, String phoneE164, String Optional activeOpt = repository.findLatestActive(user, phoneE164); if (activeOpt.isEmpty()) { - // Run a throwaway HMAC + constant-time compare so the no-active-OTP branch takes - // approximately the same wall-clock time as the matching/mismatching branches below. - // Mirrors the dummy-Argon2 pattern in BackupCodeService.verify. - String dummyHash = hmacHash(candidate); - MessageDigest.isEqual( - dummyHash.getBytes(StandardCharsets.UTF_8), dummyHash.getBytes(StandardCharsets.UTF_8)); + // Run a throwaway HMAC so the no-active-OTP branch takes approximately the same wall-clock + // time as the matching/mismatching branches below — the HMAC dominates the cost, so the + // response can't leak (via timing) whether an OTP exists for (user, phone). Mirrors the + // dummy-Argon2 verify in BackupCodeService.verify. The subsequent constant-time compare is + // negligible by comparison and deliberately omitted here. + hmacHash(candidate); return new VerifyResult.NoActiveOtp(); } StoredOtp active = activeOpt.get(); From e29ab8d3d1ed659c84c55b5fedccdbb33d4c4f85 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:00:11 -0700 Subject: [PATCH 03/12] ci: drop residual SonarQube scan and guard against skipped integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0018 removed SonarQube; delete the leftover scan step from the browser job. Add a guard to the build job that fails the gate if PKAUTH_SKIP_TESTCONTAINERS=1 leaks into CI — otherwise the persistence integration and concurrency suites self-skip and the build goes green with zero real-DB coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1b67fea..c236d1c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,6 +33,17 @@ jobs: - name: Set up Gradle uses: gradle/actions/setup-gradle@5e2ebd065dc2488b7a6ad670704656cbbe1e8f60 # v6 + # The persistence integration suites self-skip when PKAUTH_SKIP_TESTCONTAINERS=1. The CI + # gate must exercise them against real Postgres/DynamoDB Local, so fail loudly if that + # escape hatch leaks into the gating pipeline (otherwise the build can go green with zero + # persistence and concurrency coverage). + - name: Guard against skipped integration tests + run: | + if [ "${PKAUTH_SKIP_TESTCONTAINERS:-}" = "1" ]; then + echo "PKAUTH_SKIP_TESTCONTAINERS=1 must not be set in the CI gate — it would silently skip all persistence integration tests." >&2 + exit 1 + fi + - name: Build and test run: ./gradlew clean build test --stacktrace @@ -64,15 +75,6 @@ jobs: run: npx vitest run --coverage working-directory: clients/passkeys-browser - - name: SonarQube scan - # SONAR_TOKEN is unavailable to PRs from forks; skip there to avoid a hard failure. - if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository - uses: sonarsource/sonarqube-scan-action@713881670b6b3676cda39549040e2d88c70d582e # v8.2.0 - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - with: - projectBaseDir: clients/passkeys-browser - - name: Run Stryker mutation tests run: npm exec --no-install -- stryker run working-directory: clients/passkeys-browser From 29b00090b3268cd07ebfd5810f17e5c09f82dcc8 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:00:11 -0700 Subject: [PATCH 04/12] docs: fix drift and close security-facing documentation gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - OTP hashing is HMAC-SHA256 with a server-side pepper, not Argon2id (DESIGN, GETTING_STARTED, threat-model); rewrite the OTP DoS mitigation to credit the attempt cap + rate limiter rather than hash cost. Argon2id retained only for backup codes. - Document the userVerification=REQUIRED default and the risk of relaxing it (threat-model Spoofing row + operator-guide UV subsection). - Correct the OTP pepper minimum length (>=16 bytes decoded, 32+ recommended) and dev-mode throwaway behavior. - Bump schema version 9 -> 10 and add V10 (amr column) in DESIGN §8. - Give the DESIGN §6 SPI table a Module column and reconcile the §2 prose list with the real owning modules. - Describe the magic-link token as a signed single-use HS256 JWT. - Replace CONTRIBUTING's stale per-module-README claim with a pointer to GETTING_STARTED; de-hardcode the CLAUDE.md @since target. - Add docs/adr/README.md index; bring the micronaut demo to parity with a docker-compose.yml + README persistence note. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 2 +- CONTRIBUTING.md | 2 +- DESIGN.md | 64 +++++++++++++--------- GETTING_STARTED.md | 2 +- docs/adr/README.md | 28 ++++++++++ docs/operator-guide.md | 17 +++++- docs/threat-model.md | 6 +- examples/micronaut-demo/README.md | 7 +++ examples/micronaut-demo/docker-compose.yml | 16 ++++++ 9 files changed, 112 insertions(+), 32 deletions(-) create mode 100644 docs/adr/README.md create mode 100644 examples/micronaut-demo/docker-compose.yml diff --git a/CLAUDE.md b/CLAUDE.md index 95d6cd1..bea2d18 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,7 +49,7 @@ Feature modules (`pk-auth-backup-codes`, `pk-auth-magic-link`, `pk-auth-otp`, `p ## Conventions (enforced — `./gradlew check` fails otherwise) - **SPDX header** `// SPDX-License-Identifier: MIT` on every Java file (Spotless adds/checks it). -- **`@since X.Y.Z`** Javadoc on every new/modified public API element across the library modules (current in-flight target `@since 1.3.0`-line; check `gradle.properties` for the active version). Full policy in `CONTRIBUTING.md` §7. +- **`@since X.Y.Z`** Javadoc on every new/modified public API element across the library modules (the in-flight target is the active version in `gradle.properties` with the `-SNAPSHOT` suffix dropped — always check `gradle.properties` rather than trusting a hardcoded number here). Full policy in `CONTRIBUTING.md` §7. - **JSpecify null discipline** — `@NonNull`/`@Nullable` on every public param and return; Error Prone enforces it. Compilation runs `-Xlint:all` with strict Error Prone. - **Records for DTOs/configs/result variants; sealed interfaces for closed sums.** Public API sealed via `module-info.java` exports. - **Conventional commits** (`feat(core):`, `fix(jdbi):`, `docs(adr):`, `build:`, `ci:`). Small, atomic. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6eca713..25ed41c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -24,4 +24,4 @@ JDK 21 required (Gradle toolchain will fetch one if needed). ## Running locally -Module-level READMEs (added in each phase) document the "5-minute integration" snippet and per-module specifics. +See [`GETTING_STARTED.md`](./GETTING_STARTED.md) for the "5-minute integration" snippet and per-module specifics. diff --git a/DESIGN.md b/DESIGN.md index 17d35f4..956a2b1 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -91,10 +91,17 @@ The whole project is structured around three concentric rings: WebAuthn, JWT, and the wire contract. Has no dependencies on Spring, Dropwizard, Micronaut, JDBC, DynamoDB, HTTP, or any servlet API. Exposes services and SPIs. -2. **SPIs (ports).** Interfaces the host implements: - `CredentialRepository`, `UserLookup`, `ChallengeStore`, - `BackupCodeRepository`, `OtpRepository`, `EmailSender`, `SmsSender`, - `AttestationTrustPolicy`, `OriginValidator`, `ClockProvider`. +2. **SPIs (ports).** Interfaces the host implements. The required and + core-resident ones live in `pk-auth-core/spi` (`CredentialRepository`, + `UserLookup`, `ChallengeStore`, plus the optional `AttestationTrustPolicy`, + `OriginValidator`, `ClockProvider`, `ConsumedJtiStore`, + `CeremonyRateLimiter`); `UserDeletionListener` is in `pk-auth-core/lifecycle`. + The remaining ports ship with the feature modules that own them — + `BackupCodeRepository` (`pk-auth-backup-codes`), `OtpRepository` / + `SmsSender` (`pk-auth-otp`), `EmailSender` (`pk-auth-magic-link`), + `AccessTokenStore` / `TokenTtlPolicy` / `RevocationCheck` (`pk-auth-jwt`), + and `RefreshTokenRepository` (`pk-auth-refresh-tokens`). The §6 table below + lists the owning module for each. 3. **Adapters.** Three of them — Spring Boot, Dropwizard, Micronaut. Each adapter mounts the same JSON contract under `/auth/**` and delegates to the core. @@ -111,7 +118,7 @@ modules implement SPIs declared in core and are wired in by the host. | `pk-auth-jwt` | HS256 JWT mint (`PkAuthJwtIssuer`) + validate (`PkAuthJwtValidator`). Nimbus JOSE+JWT under the hood. Hosts the `TokenTtlPolicy` SPI for per-audience access-token TTL dispatch ([ADR 0014](./docs/adr/0014-per-audience-ttl-policy.md)) and the `AccessTokenStore` SPI for stateful (server-revocable) access tokens ([ADR 0015](./docs/adr/0015-stateful-access-tokens.md)). | | `pk-auth-backup-codes` | Alt flow: generate, hash (Argon2id), and atomically claim view-once backup codes. | | `pk-auth-magic-link` | Alt flow: random-token magic links over the host's email dispatcher. | -| `pk-auth-otp` | Alt flow: 6-digit OTPs over the host's SMS dispatcher; Argon2id-hashed and atomic-claim. | +| `pk-auth-otp` | Alt flow: 6-digit OTPs over the host's SMS dispatcher; hashed with HMAC-SHA256 (server-side pepper) and atomic-claim. | | `pk-auth-refresh-tokens` | Rotating refresh tokens with family-based replay defense. `RefreshTokenService` + `RefreshTokenRepository` SPI; `RefreshHandler` is the framework-neutral `POST /auth/refresh` composer ([ADR 0013](./docs/adr/0013-refresh-tokens-family-rotation.md)). | | `pk-auth-admin-api` | `AdminService` exposes account/credential/backup-code/email/phone operations. Result-typed (`AdminResult` sealed sum). | | `pk-auth-persistence-jdbi` | SPI impls on JDBI + Postgres + Flyway. Migrations at `src/main/resources/db/migration/`. | @@ -207,23 +214,23 @@ Worth knowing for any non-trivial integration: If you adopt pk-auth, the SPIs are your only mandatory contact surface. They are intentionally narrow. -| SPI | Required? | Notes | -|---|---|---| -| `UserLookup` | **Yes** | Maps `(username/email) ↔ UserHandle`. Atomic find-or-create on first registration. | -| `CredentialRepository` | **Yes** | Insert / list-by-user / update / delete / find-by-id. | -| `ChallengeStore` | **Yes** | `create(...)`, `takeOnce(challengeId)` — atomic single-use. | -| `BackupCodeRepository` | Only if backup codes are enabled | Hashed-storage CRUD + atomic claim. | -| `OtpRepository` | Only if phone OTP is enabled | Same shape as backup codes. | -| `EmailSender` | Only if magic-link is enabled | `send(to, subject, body)`. | -| `SmsSender` | Only if phone OTP is enabled | `send(phoneE164, body)`. | -| `AccessTokenStore` | Optional (paved road for revocability) | Stateful access tokens. Issuer calls `record` on issue, validator calls `exists` on every validate. Default `AccessTokenStore.noop()` preserves stateless behaviour. JDBI + DynamoDB implementations ship in-tree. See [ADR 0015](./docs/adr/0015-stateful-access-tokens.md). | -| `TokenTtlPolicy` | Optional | Per-audience access-token TTL dispatch. Static factories `TokenTtlPolicy.single(ttl)` and `TokenTtlPolicy.fixed(default, overrides)` cover the common cases. See [ADR 0014](./docs/adr/0014-per-audience-ttl-policy.md). | -| `RefreshTokenRepository` | Only if `pk-auth-refresh-tokens` is wired | Storage SPI for the rotating refresh-token primitive. Load-bearing `rotateAtomically` atomically marks the parent used and inserts the successor. JDBI, DynamoDB, and in-memory impls ship; the contract is enforced by a parity test suite that includes the 8-thread concurrent-rotation race test. See [ADR 0013](./docs/adr/0013-refresh-tokens-family-rotation.md). | -| `UserDeletionListener` | Optional (extension point) | Hook for the `UserDeletionService` fan-out. The library auto-registers listeners for credentials, backup codes, OTPs, access tokens, and refresh tokens; hosts add their own to clean up host-owned tables on user delete. See [ADR 0016](./docs/adr/0016-user-deletion-fan-out.md). | -| `RevocationCheck` | Optional | In-process deny-list for hosts that want fast invalidation of a small set of JTIs without persisting every issued token. Orthogonal to `AccessTokenStore`. | -| `AttestationTrustPolicy` | Optional | Default policy is `none`. Override to enforce MDS3 / specific AAGUID lists. | -| `OriginValidator` | Optional | Default is config-driven exact-match. Override for tenancy-aware origins. | -| `ClockProvider` | Optional | Default is `Clock.systemUTC()`. Override in tests. | +| SPI | Module | Required? | Notes | +|---|---|---|---| +| `UserLookup` | `pk-auth-core` | **Yes** | Maps `(username/email) ↔ UserHandle`. Atomic find-or-create on first registration. | +| `CredentialRepository` | `pk-auth-core` | **Yes** | Insert / list-by-user / update / delete / find-by-id. | +| `ChallengeStore` | `pk-auth-core` | **Yes** | `create(...)`, `takeOnce(challengeId)` — atomic single-use. | +| `BackupCodeRepository` | `pk-auth-backup-codes` | Only if backup codes are enabled | Hashed-storage CRUD + atomic claim. | +| `OtpRepository` | `pk-auth-otp` | Only if phone OTP is enabled | Same shape as backup codes. | +| `EmailSender` | `pk-auth-magic-link` | Only if magic-link is enabled | `send(to, subject, body)`. | +| `SmsSender` | `pk-auth-otp` | Only if phone OTP is enabled | `send(phoneE164, body)`. | +| `AccessTokenStore` | `pk-auth-jwt` | Optional (paved road for revocability) | Stateful access tokens. Issuer calls `record` on issue, validator calls `exists` on every validate. Default `AccessTokenStore.noop()` preserves stateless behaviour. JDBI + DynamoDB implementations ship in-tree. See [ADR 0015](./docs/adr/0015-stateful-access-tokens.md). | +| `TokenTtlPolicy` | `pk-auth-jwt` | Optional | Per-audience access-token TTL dispatch. Static factories `TokenTtlPolicy.single(ttl)` and `TokenTtlPolicy.fixed(default, overrides)` cover the common cases. See [ADR 0014](./docs/adr/0014-per-audience-ttl-policy.md). | +| `RevocationCheck` | `pk-auth-jwt` | Optional | In-process deny-list for hosts that want fast invalidation of a small set of JTIs without persisting every issued token. Orthogonal to `AccessTokenStore`. | +| `RefreshTokenRepository` | `pk-auth-refresh-tokens` (`…refresh.spi`) | Only if `pk-auth-refresh-tokens` is wired | Storage SPI for the rotating refresh-token primitive. Load-bearing `rotateAtomically` atomically marks the parent used and inserts the successor. JDBI, DynamoDB, and in-memory impls ship; the contract is enforced by a parity test suite that includes the 8-thread concurrent-rotation race test. See [ADR 0013](./docs/adr/0013-refresh-tokens-family-rotation.md). | +| `UserDeletionListener` | `pk-auth-core` (`…lifecycle`, not `spi`) | Optional (extension point) | Hook for the `UserDeletionService` fan-out. The library auto-registers listeners for credentials, backup codes, OTPs, access tokens, and refresh tokens; hosts add their own to clean up host-owned tables on user delete. See [ADR 0016](./docs/adr/0016-user-deletion-fan-out.md). | +| `AttestationTrustPolicy` | `pk-auth-core` | Optional | Default policy is `none`. Override to enforce MDS3 / specific AAGUID lists. | +| `OriginValidator` | `pk-auth-core` | Optional | Default is config-driven exact-match. Override for tenancy-aware origins. | +| `ClockProvider` | `pk-auth-core` | Optional | Default is `Clock.systemUTC()`. Override in tests. | For a fresh project, the testkit's in-memory implementations let you boot end-to-end without writing any SPI. For Phase-12-style @@ -303,8 +310,10 @@ SPIs. (V1–V5, no `pkauth_` prefix), plus the append-only `pkauth_audit_events` table from V6. `V8__create_access_tokens.sql` and `V9__create_refresh_tokens.sql` add the stateful-access-token and - refresh-token tables for the 1.1.0 SPIs. - `PkAuthJdbiSchema.CURRENT_SCHEMA_VERSION` is `"9"`. Magic-link tokens are + refresh-token tables for the 1.1.0 SPIs; `V10__refresh_tokens_amr.sql` + adds the `amr` (RFC 8176 authentication-method-reference) column to + `refresh_tokens`. `PkAuthJdbiSchema.CURRENT_SCHEMA_VERSION` is `"10"`. + Magic-link tokens are not persisted — the JWT itself is the credential; consumed JTIs live in a `ConsumedJtiStore` (in-memory by default, swap in a shared backend for multi-replica deployments). @@ -431,8 +440,11 @@ Highlights: for sites where synced (counter-0) passkeys dominate, at the cost of weakening the clone-detection signal. - **Challenges are single-use** and TTL-bounded (5 min default). -- **Backup codes and OTPs are Argon2id-hashed** server-side. Plaintext - is returned only at regeneration time (view-once). +- **Backup codes are Argon2id-hashed** server-side; **OTPs are hashed + with HMAC-SHA256 using a server-side pepper** (the 10^6 search space + makes a CPU-heavy hash pointless — the per-attempt cap and rate + limiter are the brute-force defence). Backup-code plaintext is + returned only at regeneration time (view-once). - **Last-credential guard**: `DELETE /credentials/{id}` returns 409 if it would leave the user with zero passkeys. Backup codes are the intended recovery path; encourage users to add a second passkey diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md index 72ea055..2c7f46c 100644 --- a/GETTING_STARTED.md +++ b/GETTING_STARTED.md @@ -107,7 +107,7 @@ Skip them and the feature isn't exposed. |---|---|---| | `pk-auth-backup-codes` | View-once Argon2id-hashed backup codes for account recovery. | Users will lose their phones; backup codes are the documented recovery path before "talk to support." | | `pk-auth-magic-link` | Single-use email magic-link tokens. JWTs on the wire; consumed-JTI tracking via a swappable SPI. | Email verification, or a passwordless login alternative for users without a passkey-capable device. | -| `pk-auth-otp` | 6-digit SMS OTPs with attempt caps and Argon2id-hashed storage. | Phone verification. | +| `pk-auth-otp` | 6-digit SMS OTPs with attempt caps; codes are hashed with HMAC-SHA256 using a server-side pepper. | Phone verification. | | `pk-auth-refresh-tokens` | Rotating refresh tokens with family-based replay defense. Adds `POST /auth/refresh`; on success returns a new refresh token + a fresh access JWT, on replay scorches the entire token family. Requires a `RefreshTokenRepository` SPI (JDBI / DynamoDB impls ship). See [ADR 0013](./docs/adr/0013-refresh-tokens-family-rotation.md). | When you need sessions longer than the access-token TTL without re-running a WebAuthn ceremony. | | `pk-auth-admin-api` | Adds the `/auth/admin/**` endpoints (rename / delete passkeys, regenerate backup codes, account summary, email & phone verification). | Almost always — without it the UI can't manage credentials. | diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..84174e0 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,28 @@ +# Architecture Decision Records + +This directory holds pk-auth's Architecture Decision Records in +[Nygard format](https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions), +numbered sequentially. ADRs are append-only: when a decision is reversed, a new +ADR is added and the prior one is marked `Superseded by NNNN`. See +[ADR 0001](./0001-record-architecture-decisions.md) for the format itself. + +| # | Status | Date | Title | +|---|---|---|---| +| [0001](./0001-record-architecture-decisions.md) | Accepted | 2026-05-12 | Record architecture decisions | +| [0002](./0002-webauthn4j-over-yubico.md) | Accepted | 2026-05-13 | WebAuthn4J over Yubico java-webauthn-server | +| [0003](./0003-jdbi-over-jpa.md) | Accepted | 2026-05-14 | JDBI over JPA | +| [0004](./0004-dagger-for-dropwizard.md) | Accepted | 2026-05-14 | Use Dagger 2 for the Dropwizard adapter's DI | +| [0005](./0005-stateless-jwt-default.md) | Accepted | 2026-05-14 | Stateless JWT as the default post-ceremony credential | +| [0006](./0006-userlookup-spi-not-owned.md) | Accepted | 2026-05-15 | `UserLookup` is an SPI, not an owned table | +| [0007](./0007-dynamodb-local-vs-localstack.md) | Accepted | 2026-05-14 | DynamoDB Local over LocalStack for integration tests | +| [0008](./0008-dynamodb-single-table-design.md) | Accepted | 2026-05-14 | DynamoDB single-table design for auth items, separate users table | +| [0009](./0009-jackson-3-over-jackson-2.md) | Accepted | 2026-05-13 | Jackson 3 (`tools.jackson`) over Jackson 2 (`com.fasterxml.jackson`) | +| [0010](./0010-dropwizard-track-latest.md) | Accepted | 2026-05-15 | Track latest Dropwizard rather than pin to 4.x | +| [0011](./0011-spring-boot-4.md) | Accepted | 2026-05-16 | Spring Boot 4 / Spring Security 7 for the Spring starter | +| [0012](./0012-micronaut-4.md) | Accepted | 2026-05-16 | Micronaut 4 for the Micronaut adapter | +| [0013](./0013-refresh-tokens-family-rotation.md) | Accepted | 2026-05-16 | Rotating refresh tokens with family-based replay detection | +| [0014](./0014-per-audience-ttl-policy.md) | Accepted | 2026-05-16 | Per-audience JWT TTL via a TokenTtlPolicy SPI | +| [0015](./0015-stateful-access-tokens.md) | Accepted | 2026-05-16 | Stateful access tokens via AccessTokenStore SPI | +| [0016](./0016-user-deletion-fan-out.md) | Accepted | 2026-05-16 | User deletion fan-out is sequential and best-effort | +| [0017](./0017-sonarqube-cloud-static-analysis.md) | Superseded by [0018](./0018-remove-sonarqube-cloud.md) | 2026-06-11 | SonarQube Cloud for static analysis and coverage tracking | +| [0018](./0018-remove-sonarqube-cloud.md) | Accepted | 2026-06-11 | Remove SonarQube Cloud; enforce coverage with native JaCoCo line + branch gates | diff --git a/docs/operator-guide.md b/docs/operator-guide.md index 02867d7..9345dc7 100644 --- a/docs/operator-guide.md +++ b/docs/operator-guide.md @@ -26,11 +26,26 @@ adapters all consume the same core. A typical production deployment needs: | `pkauth.jwt.secret` (HS256) | 32 bytes | Hard fail at boot if shorter. Rotate by issuing a fresh secret and tolerating a grace window (issue + verify in parallel — pk-auth itself does not rotate; the host shoulds run two issuers behind a load balancer until tokens expire). | | `pkauth.relying-party.id` | n/a | The eTLD+1 (e.g. `example.com`, NOT `auth.example.com`). Cross-subdomain passkeys all bind to this. Once a credential is registered against an RP ID, it cannot be re-registered against a different one without a fresh enrollment. | | `pkauth.relying-party.origins` | n/a | Strict allow-list of `https://` origins. WebAuthn rejects mismatches; expand the list as you add subdomains. | -| OTP pepper (`pkauth.otp.pepper`) | n/a | Per-deployment pepper for OTP hashes only — OTP codes are hashed with HMAC-SHA256(pepper, code), not Argon2id. (Backup codes use Argon2id with no pepper.) Treat as a long-lived secret; rotating it invalidates every existing OTP hash. | +| OTP pepper (`pkauth.otp.pepper`) | 16 bytes decoded (32+ recommended) | Base64-encoded per-deployment pepper for OTP hashes only — OTP codes are hashed with HMAC-SHA256(pepper, code), not Argon2id. (Backup codes use Argon2id with no pepper.) Hard fail at boot if the value is not valid Base64 or decodes to fewer than 16 bytes. If unset, the adapter auto-generates a throwaway per-startup pepper **only when `pkauth.dev-mode=true`** (dev only — it invalidates outstanding OTPs across restarts and across cluster instances); with dev-mode off, an unset pepper is a hard boot failure. Treat as a long-lived secret; rotating it invalidates every existing OTP hash. | Recommended: stash secrets in a KMS/Secrets Manager and inject as environment variables (`PKAUTH_JWT_SECRET`, `PKAUTH_OTP_PEPPER`). The adapters bind both. +### User Verification (UV) + +`CeremonyConfig.userVerification` defaults to **`REQUIRED`**. With this default +WebAuthn4J enforces the asserted `flagUV` on every registration and +authentication, so each ceremony must carry a per-ceremony biometric or PIN — +this is what makes a passkey a genuine factor (something you *have* **and** +something you *are*/*know*). + +Relaxing it to `PREFERRED` or `DISCOURAGED` accepts a present-but-unverified +authenticator (mere user-presence, no biometric/PIN). A passkey then degrades to +"something you have" alone, which materially weakens the factor for every user. +**Generally do not relax UV.** The only standard reason to opt out is supporting +UV-incapable roaming hardware security keys; if you do, scope it deliberately and +understand the trade-off. + ## 3. Persistence migrations ### JDBI / Postgres diff --git a/docs/threat-model.md b/docs/threat-model.md index 198feff..4d1c151 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -44,6 +44,7 @@ Boundaries cross-checked in this model: | Attacker forges a JWT | HS256 signature with ≥ 32-byte secret. JWT verification enforces issuer, audience, expiry. | | Attacker spoofs the relying party | RP ID and origin are checked server-side on every `finish` call — config-driven allow-list (`pkauth.relying-party.origins`). Cross-origin attempts are rejected with `origin_mismatch`. | | Attacker reuses a backup code | Backup codes are Argon2id-hashed, single-use, and atomically claimed inside the SPI (`atomic claim or fail`). | +| Attacker presents a passkey the legitimate user never consciously authorized (lost/borrowed device with a key already enrolled, or a key with no user-presence factor) | User Verification defaults to `REQUIRED` (`CeremonyConfig.defaults()`), so WebAuthn4J enforces the asserted `flagUV` on every ceremony — the authenticator must prove a per-ceremony biometric or PIN, making the passkey a genuine "something you have **and** something you are/know" factor. Relaxing UV to `PREFERRED` or `DISCOURAGED` removes that guarantee: a present-but-unverified authenticator (mere user-presence, no biometric/PIN) is accepted, so a passkey degrades to "something you have" alone. Do not relax UV unless a deployment specifically needs UV-incapable hardware security keys, and understand it weakens the factor for everyone. | ### Tampering @@ -66,7 +67,7 @@ Boundaries cross-checked in this model: |---|---| | Disclose a credential's public key | Public keys are public. Their disclosure leaks no authentication material. | | Disclose backup codes | Backup codes are returned plaintext **exactly once** at regeneration time. The server only retains Argon2id hashes. | -| Disclose magic-link tokens | Magic-link tokens are random 32-byte values stored hashed; the plaintext only exists on the dispatcher path. Use a real email sender in production. | +| Disclose magic-link tokens | A magic-link token is a signed single-use HS256 JWT (it carries a `pkauth.purpose` claim and is bound by the HS256 signature — not a random value stored hashed). It is never persisted: single-use is enforced by recording its JTI in a `ConsumedJtiStore` after redemption (JWT TTL 15m, consumed-JTI TTL 30m). The token only exists in the outbound email on the dispatcher path. Use a real email sender in production, and inject a shared `ConsumedJtiStore` for multi-replica deployments so a token can't be replayed across replicas. | | Disclose JWT secret via logs | The starter never logs `pkauth.jwt.secret`. Other secrets (Argon2 pepper, RP origins) are bound only via env vars; do not echo them in `--debug` traces. | | Enumerate usernames | `/auth/passkeys/registration/start` returns the same shape for any username (a fresh user handle is created); `/auth/passkeys/authentication/start` does not leak existence — it returns an `allowCredentials` list that an unknown user would receive empty. Avoid mounting routes that surface user existence (e.g. "/users/exists"). | @@ -75,7 +76,8 @@ Boundaries cross-checked in this model: | Threat | Mitigation | |---|---| | Flood challenges | Challenges expire in 5 minutes by default and are stored by `ChallengeId`. pk-auth ships a `CeremonyRateLimiter` SPI (with an in-memory Caffeine-backed default) keyed on client IP and username; the `start*` / `finish*` endpoints short-circuit with `429 Retry-After` when the limiter denies. For multi-replica deployments, replace the default with a shared-store implementation. Heavy floods should still be filtered at the host's WAF / API gateway upstream of the adapter. | -| Hash-burn via repeated OTP / backup-code attempts | Argon2id is intentionally CPU-heavy. The SPIs ship per-credential attempt counters; the magic-link and backup-code modules also ship per-user sliding-window rate limiters (in-memory defaults, override for multi-replica). | +| Hash-burn via repeated backup-code attempts | Backup codes are Argon2id-hashed (intentionally CPU-heavy), so a wrong code costs the attacker as much as a right one. The SPIs ship per-credential attempt counters; the magic-link and backup-code modules also ship per-user sliding-window rate limiters (in-memory defaults, override for multi-replica). | +| Brute-force / flood of OTP attempts | OTP codes are hashed with cheap HMAC-SHA256, **not** a CPU-heavy KDF — the 10^6 code space makes hash cost pointless, so the hash burn argument does **not** apply here. The defence is instead the per-credential attempt cap (default 5 verifies per code, enforced atomically in the repository) plus the per-(user, phone) send rate limiter (default 3 codes per 15-minute window). A wrong code is cheap to check but exhausts the cap quickly, and the rate limiter bounds how many fresh codes an attacker can mint. | | Exhaust DB connections | Connection pooling is the host's responsibility. Recommend HikariCP for JDBI, the AWS SDK's default for DynamoDB. | ### Elevation of Privilege diff --git a/examples/micronaut-demo/README.md b/examples/micronaut-demo/README.md index db4b092..9c0fd1e 100644 --- a/examples/micronaut-demo/README.md +++ b/examples/micronaut-demo/README.md @@ -35,6 +35,13 @@ persistence: Replace the `InMemoryPersistenceFactory` with a factory that surfaces those repos. +For hands-on poking at those backends, [`docker-compose.yml`](./docker-compose.yml) ships a +Postgres 16 + DynamoDB Local stack (none of which the default in-memory profile needs): + +```bash +docker compose up -d +``` + ## Frontend The demo's SPA lives in `src/main/resources/public/index.html` and `demo.js`, both of diff --git a/examples/micronaut-demo/docker-compose.yml b/examples/micronaut-demo/docker-compose.yml new file mode 100644 index 0000000..465b59e --- /dev/null +++ b/examples/micronaut-demo/docker-compose.yml @@ -0,0 +1,16 @@ +# Local infrastructure for hands-on poking at the Micronaut demo with non-default persistence. +# Not needed for the default in-memory profile — the demo runs out of the box without these. +services: + postgres: + image: postgres:16 + environment: + POSTGRES_USER: pkauth + POSTGRES_PASSWORD: pkauth + POSTGRES_DB: pkauth + ports: + - "5432:5432" + dynamodb-local: + image: amazon/dynamodb-local:latest + command: ["-jar", "DynamoDBLocal.jar", "-sharedDb"] + ports: + - "8000:8000" From 7c126e323256d60b86c1a97d44c2be424412bdf4 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:03:31 -0700 Subject: [PATCH 05/12] feat(otp,backup-codes,magic-link): validate Config record ranges The feature-service Config records validated the pepper/baseUrl/non-null fields but not the numeric/duration ranges. A zero or negative maxAttempts, ttl, rateLimit, codeCount, or Argon2 parameter would silently produce never-verifiable or instantly-expired codes. Reject non-positive values in the compact constructors, with tests covering each branch. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../pkauth/backupcodes/BackupCodeService.java | 17 ++++++++- .../backupcodes/BackupCodeServiceTest.java | 22 ++++++++++++ .../pkauth/magiclink/MagicLinkService.java | 6 ++++ .../magiclink/MagicLinkServiceTest.java | 14 ++++++++ .../pkauth/otp/OtpService.java | 14 +++++++- .../pkauth/otp/OtpServiceTest.java | 35 +++++++++++++++++++ 6 files changed, 106 insertions(+), 2 deletions(-) diff --git a/pk-auth-backup-codes/src/main/java/com/codeheadsystems/pkauth/backupcodes/BackupCodeService.java b/pk-auth-backup-codes/src/main/java/com/codeheadsystems/pkauth/backupcodes/BackupCodeService.java index 5a2b2b7..b883da2 100644 --- a/pk-auth-backup-codes/src/main/java/com/codeheadsystems/pkauth/backupcodes/BackupCodeService.java +++ b/pk-auth-backup-codes/src/main/java/com/codeheadsystems/pkauth/backupcodes/BackupCodeService.java @@ -356,10 +356,25 @@ public record Config( int parallelism, int codeCount, int rateLimit) { - /** Compact constructor — enforces non-null on object-typed fields. */ + /** Compact constructor — enforces non-null on object-typed fields and positive ranges. */ public Config { Objects.requireNonNull(random, "random"); Objects.requireNonNull(argon2, "argon2"); + if (iterations < 1) { + throw new IllegalArgumentException("iterations must be at least 1"); + } + if (memory < 1) { + throw new IllegalArgumentException("memory must be at least 1"); + } + if (parallelism < 1) { + throw new IllegalArgumentException("parallelism must be at least 1"); + } + if (codeCount < 1) { + throw new IllegalArgumentException("codeCount must be at least 1"); + } + if (rateLimit < 1) { + throw new IllegalArgumentException("rateLimit must be at least 1"); + } } /** diff --git a/pk-auth-backup-codes/src/test/java/com/codeheadsystems/pkauth/backupcodes/BackupCodeServiceTest.java b/pk-auth-backup-codes/src/test/java/com/codeheadsystems/pkauth/backupcodes/BackupCodeServiceTest.java index 5cba0cd..14193e6 100644 --- a/pk-auth-backup-codes/src/test/java/com/codeheadsystems/pkauth/backupcodes/BackupCodeServiceTest.java +++ b/pk-auth-backup-codes/src/test/java/com/codeheadsystems/pkauth/backupcodes/BackupCodeServiceTest.java @@ -2,6 +2,7 @@ package com.codeheadsystems.pkauth.backupcodes; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.codeheadsystems.pkauth.api.UserHandle; import com.codeheadsystems.pkauth.backupcodes.BackupCodeService.InMemoryBackupCodeRateLimiter; @@ -46,6 +47,27 @@ void setUp() { /* rateLimit */ BackupCodeService.DEFAULT_RATE_LIMIT)); } + @Test + void configRejectsNonPositiveRanges() { + SecureRandom rng = new SecureRandom(); + Argon2 argon2 = Argon2Factory.create(Argon2Factory.Argon2Types.ARGON2id); + assertThatThrownBy(() -> new BackupCodeService.Config(rng, argon2, 0, 1024, 1, 5, 5)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("iterations"); + assertThatThrownBy(() -> new BackupCodeService.Config(rng, argon2, 1, 0, 1, 5, 5)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("memory"); + assertThatThrownBy(() -> new BackupCodeService.Config(rng, argon2, 1, 1024, 0, 5, 5)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("parallelism"); + assertThatThrownBy(() -> new BackupCodeService.Config(rng, argon2, 1, 1024, 1, 0, 5)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("codeCount"); + assertThatThrownBy(() -> new BackupCodeService.Config(rng, argon2, 1, 1024, 1, 5, 0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("rateLimit"); + } + @Test void generateProducesNDistinctTenCharCodes() { UserHandle user = UserHandle.of(new byte[] {1, 2, 3}); diff --git a/pk-auth-magic-link/src/main/java/com/codeheadsystems/pkauth/magiclink/MagicLinkService.java b/pk-auth-magic-link/src/main/java/com/codeheadsystems/pkauth/magiclink/MagicLinkService.java index 8fb885c..e41547f 100644 --- a/pk-auth-magic-link/src/main/java/com/codeheadsystems/pkauth/magiclink/MagicLinkService.java +++ b/pk-auth-magic-link/src/main/java/com/codeheadsystems/pkauth/magiclink/MagicLinkService.java @@ -398,6 +398,12 @@ public record Config( Objects.requireNonNull(rateLimiter, "rateLimiter"); Objects.requireNonNull(consumedJtiTtl, "consumedJtiTtl"); validateBaseUrl(baseUrl); + if (rateLimit < 1) { + throw new IllegalArgumentException("rateLimit must be at least 1"); + } + if (consumedJtiTtl.isZero() || consumedJtiTtl.isNegative()) { + throw new IllegalArgumentException("consumedJtiTtl must be strictly positive"); + } } private static void validateBaseUrl(String baseUrl) { diff --git a/pk-auth-magic-link/src/test/java/com/codeheadsystems/pkauth/magiclink/MagicLinkServiceTest.java b/pk-auth-magic-link/src/test/java/com/codeheadsystems/pkauth/magiclink/MagicLinkServiceTest.java index 7dd3cd0..12087d4 100644 --- a/pk-auth-magic-link/src/test/java/com/codeheadsystems/pkauth/magiclink/MagicLinkServiceTest.java +++ b/pk-auth-magic-link/src/test/java/com/codeheadsystems/pkauth/magiclink/MagicLinkServiceTest.java @@ -2,6 +2,7 @@ package com.codeheadsystems.pkauth.magiclink; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.codeheadsystems.pkauth.api.UserHandle; import com.codeheadsystems.pkauth.jwt.JwtConfig; @@ -55,6 +56,19 @@ void setUp() { MagicLinkService.DEFAULT_CONSUMED_JTI_TTL)); } + @Test + void configRejectsNonPositiveRanges() { + MagicLinkService.InMemoryRateLimiter limiter = + new MagicLinkService.InMemoryRateLimiter(Duration.ofHours(1)); + assertThatThrownBy( + () -> new MagicLinkService.Config(BASE_URL, 0, limiter, Duration.ofMinutes(30))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("rateLimit"); + assertThatThrownBy(() -> new MagicLinkService.Config(BASE_URL, 5, limiter, Duration.ZERO)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("consumedJtiTtl"); + } + @Test void verificationEmailDispatchesAndTokenConsumesOnce() { UserHandle user = UserHandle.random(); diff --git a/pk-auth-otp/src/main/java/com/codeheadsystems/pkauth/otp/OtpService.java b/pk-auth-otp/src/main/java/com/codeheadsystems/pkauth/otp/OtpService.java index 14b5e8c..fa4da34 100644 --- a/pk-auth-otp/src/main/java/com/codeheadsystems/pkauth/otp/OtpService.java +++ b/pk-auth-otp/src/main/java/com/codeheadsystems/pkauth/otp/OtpService.java @@ -338,7 +338,7 @@ public record Config( int maxAttempts, int rateLimit, Duration rateWindow) { - /** Compact constructor — validates pepper length and non-null fields. */ + /** Compact constructor — validates pepper length, field ranges, and non-null fields. */ public Config { Objects.requireNonNull(random, "random"); Objects.requireNonNull(pepper, "pepper"); @@ -347,6 +347,18 @@ public record Config( if (pepper.length < 16) { throw new IllegalArgumentException("pepper must be at least 16 bytes"); } + if (ttl.isZero() || ttl.isNegative()) { + throw new IllegalArgumentException("ttl must be strictly positive"); + } + if (rateWindow.isZero() || rateWindow.isNegative()) { + throw new IllegalArgumentException("rateWindow must be strictly positive"); + } + if (maxAttempts < 1) { + throw new IllegalArgumentException("maxAttempts must be at least 1"); + } + if (rateLimit < 1) { + throw new IllegalArgumentException("rateLimit must be at least 1"); + } } /** diff --git a/pk-auth-otp/src/test/java/com/codeheadsystems/pkauth/otp/OtpServiceTest.java b/pk-auth-otp/src/test/java/com/codeheadsystems/pkauth/otp/OtpServiceTest.java index a70a019..70dbddc 100644 --- a/pk-auth-otp/src/test/java/com/codeheadsystems/pkauth/otp/OtpServiceTest.java +++ b/pk-auth-otp/src/test/java/com/codeheadsystems/pkauth/otp/OtpServiceTest.java @@ -2,6 +2,7 @@ package com.codeheadsystems.pkauth.otp; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.codeheadsystems.pkauth.api.UserHandle; import com.codeheadsystems.pkauth.spi.ClockProvider; @@ -133,6 +134,40 @@ void maskPhoneKeepsCountryPrefixAndLast4() { assertThat(OtpService.maskPhone("abc")).isEqualTo("+***"); } + @Test + void configRejectsNonPositiveRangesAndShortPepper() { + SecureRandom rng = new SecureRandom(); + assertThatThrownBy( + () -> + new OtpService.Config( + rng, new byte[8], Duration.ofMinutes(5), 3, 3, Duration.ofMinutes(15))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("pepper"); + assertThatThrownBy( + () -> + new OtpService.Config( + rng, TEST_PEPPER, Duration.ZERO, 3, 3, Duration.ofMinutes(15))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("ttl"); + assertThatThrownBy( + () -> + new OtpService.Config( + rng, TEST_PEPPER, Duration.ofMinutes(5), 0, 3, Duration.ofMinutes(15))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("maxAttempts"); + assertThatThrownBy( + () -> + new OtpService.Config( + rng, TEST_PEPPER, Duration.ofMinutes(5), 3, 0, Duration.ofMinutes(15))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("rateLimit"); + assertThatThrownBy( + () -> + new OtpService.Config(rng, TEST_PEPPER, Duration.ofMinutes(5), 3, 3, Duration.ZERO)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("rateWindow"); + } + /** Captures sends so tests can pluck the dispatched code out of the message body. */ private static final class RecordingSmsSender implements SmsSender { private final List messages = new ArrayList<>(); From bde5f0ac143e741f2e7b05c66d99e68739e979ee Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:03:31 -0700 Subject: [PATCH 06/12] refactor(admin): replace SendResult downcast with exhaustive sealed switch startPhoneVerification cast SendResult to Sent after only ruling out RateLimited. A future SendResult variant would throw ClassCastException across the AdminResult boundary instead of failing at compile time. Switch exhaustively over the sealed sum, matching finishPhoneVerification. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../pkauth/admin/DefaultAdminService.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pk-auth-admin-api/src/main/java/com/codeheadsystems/pkauth/admin/DefaultAdminService.java b/pk-auth-admin-api/src/main/java/com/codeheadsystems/pkauth/admin/DefaultAdminService.java index 776ec2a..52b1c9c 100644 --- a/pk-auth-admin-api/src/main/java/com/codeheadsystems/pkauth/admin/DefaultAdminService.java +++ b/pk-auth-admin-api/src/main/java/com/codeheadsystems/pkauth/admin/DefaultAdminService.java @@ -215,11 +215,14 @@ public AdminResult startPhoneVerification( } if (otpService == null) return notConfigured("phone verification"); OtpService.SendResult send = otpService.startVerification(target, phoneE164); - if (send instanceof OtpService.SendResult.RateLimited) { - return new AdminResult.RateLimited<>(Duration.ofMinutes(15)); - } - return new AdminResult.Success<>( - new OtpDispatchResult(((OtpService.SendResult.Sent) send).otpId())); + // Exhaustive switch over the sealed SendResult — a new variant becomes a compile error here + // rather than a ClassCastException across the AdminResult boundary. + return switch (send) { + case OtpService.SendResult.Sent sent -> + new AdminResult.Success<>(new OtpDispatchResult(sent.otpId())); + case OtpService.SendResult.RateLimited rateLimited -> + new AdminResult.RateLimited<>(Duration.ofMinutes(15)); + }; } @Override From 13254b6baa775b3ba29f4b2a4c2177a09626ff75 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:05:56 -0700 Subject: [PATCH 07/12] refactor(core,adapters): centralize the persistence-failure response envelope The three adapters each hardcoded the 503 status and the {"error":"persistence_failure","operation":...} body in byte-identical LinkedHashMap blocks. Add PkAuthPersistenceResponse to core spi (next to the exception it renders) as the single source of the status and body, and have each adapter bind it while keeping its framework-specific response type and logging. Guarantees a DB outage can't drift the envelope between adapters or leak a framework-default 500. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../pkauth/spi/PkAuthPersistenceResponse.java | 43 +++++++++++++++++++ .../spi/PkAuthPersistenceResponseTest.java | 26 +++++++++++ .../PkAuthPersistenceExceptionMapper.java | 10 ++--- .../PkAuthPersistenceExceptionHandler.java | 9 ++-- .../spring/web/PkAuthExceptionHandler.java | 8 ++-- 5 files changed, 79 insertions(+), 17 deletions(-) create mode 100644 pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponse.java create mode 100644 pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponseTest.java diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponse.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponse.java new file mode 100644 index 0000000..e24e0e7 --- /dev/null +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponse.java @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: MIT +package com.codeheadsystems.pkauth.spi; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Shared producer for the stable wire envelope every adapter returns when a {@link + * PkAuthPersistenceException} escapes an SPI call: HTTP {@value #STATUS} with a {@code + * {"error":"persistence_failure","operation":"..."}} body. + * + *

Centralizing the status and body here keeps the three adapters' exception handlers from + * drifting and guarantees a host-side DB outage surfaces as the same sanitized {@code 503} from + * every adapter, instead of one accidentally leaking a framework-default 500 with a stack trace. + * Each adapter keeps only its framework-specific glue (response type, logging). + * + * @since 1.3.1 + */ +public final class PkAuthPersistenceResponse { + + /** HTTP status returned for a persistence failure: {@code 503 Service Unavailable}. */ + public static final int STATUS = 503; + + /** Stable machine-readable error code carried in the response body. */ + public static final String ERROR_CODE = "persistence_failure"; + + private PkAuthPersistenceResponse() {} + + /** + * Builds the neutral response body for {@code exception}. Adapters wrap this map in their own + * framework response type at {@link #STATUS}. + * + * @param exception the persistence failure to render. + * @return an insertion-ordered map with {@code error} and {@code operation} keys. + * @since 1.3.1 + */ + public static Map body(PkAuthPersistenceException exception) { + Map body = new LinkedHashMap<>(); + body.put("error", ERROR_CODE); + body.put("operation", exception.operation()); + return body; + } +} diff --git a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponseTest.java b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponseTest.java new file mode 100644 index 0000000..6528750 --- /dev/null +++ b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponseTest.java @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: MIT +package com.codeheadsystems.pkauth.spi; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class PkAuthPersistenceResponseTest { + + @Test + void statusIs503() { + assertThat(PkAuthPersistenceResponse.STATUS).isEqualTo(503); + } + + @Test + void bodyCarriesStableErrorCodeAndOperationInOrder() { + PkAuthPersistenceException e = + new PkAuthPersistenceException("credentials.save", "boom", new RuntimeException("boom")); + + var body = PkAuthPersistenceResponse.body(e); + + assertThat(body).containsExactly( + org.assertj.core.api.Assertions.entry("error", "persistence_failure"), + org.assertj.core.api.Assertions.entry("operation", "credentials.save")); + } +} diff --git a/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/resource/PkAuthPersistenceExceptionMapper.java b/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/resource/PkAuthPersistenceExceptionMapper.java index 0c2dabc..da7ea01 100644 --- a/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/resource/PkAuthPersistenceExceptionMapper.java +++ b/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/resource/PkAuthPersistenceExceptionMapper.java @@ -2,11 +2,10 @@ package com.codeheadsystems.pkauth.dropwizard.resource; import com.codeheadsystems.pkauth.spi.PkAuthPersistenceException; +import com.codeheadsystems.pkauth.spi.PkAuthPersistenceResponse; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.ext.ExceptionMapper; import jakarta.ws.rs.ext.Provider; -import java.util.LinkedHashMap; -import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,9 +28,8 @@ public Response toResponse(PkAuthPersistenceException exception) { exception.operation(), exception.getMessage(), exception); - Map body = new LinkedHashMap<>(); - body.put("error", "persistence_failure"); - body.put("operation", exception.operation()); - return Response.status(503).entity(body).build(); + return Response.status(PkAuthPersistenceResponse.STATUS) + .entity(PkAuthPersistenceResponse.body(exception)) + .build(); } } diff --git a/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthPersistenceExceptionHandler.java b/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthPersistenceExceptionHandler.java index 057c6b4..b702afb 100644 --- a/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthPersistenceExceptionHandler.java +++ b/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthPersistenceExceptionHandler.java @@ -2,6 +2,7 @@ package com.codeheadsystems.pkauth.micronaut; import com.codeheadsystems.pkauth.spi.PkAuthPersistenceException; +import com.codeheadsystems.pkauth.spi.PkAuthPersistenceResponse; import io.micronaut.context.annotation.Requires; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; @@ -9,8 +10,6 @@ import io.micronaut.http.annotation.Produces; import io.micronaut.http.server.exceptions.ExceptionHandler; import jakarta.inject.Singleton; -import java.util.LinkedHashMap; -import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,9 +36,7 @@ public HttpResponse handle(HttpRequest request, PkAuthPersistenceException ex exception.operation(), exception.getMessage(), exception); - Map body = new LinkedHashMap<>(); - body.put("error", "persistence_failure"); - body.put("operation", exception.operation()); - return HttpResponse.status(HttpStatus.SERVICE_UNAVAILABLE).body(body); + return HttpResponse.status(HttpStatus.valueOf(PkAuthPersistenceResponse.STATUS)) + .body(PkAuthPersistenceResponse.body(exception)); } } diff --git a/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/web/PkAuthExceptionHandler.java b/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/web/PkAuthExceptionHandler.java index a41efc0..d00591e 100644 --- a/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/web/PkAuthExceptionHandler.java +++ b/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/web/PkAuthExceptionHandler.java @@ -2,7 +2,7 @@ package com.codeheadsystems.pkauth.spring.web; import com.codeheadsystems.pkauth.spi.PkAuthPersistenceException; -import java.util.LinkedHashMap; +import com.codeheadsystems.pkauth.spi.PkAuthPersistenceResponse; import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -27,9 +27,7 @@ public class PkAuthExceptionHandler { public ResponseEntity> handlePersistence(PkAuthPersistenceException e) { LOG.warn( "pkauth.persistence.failure operation={} message={}", e.operation(), e.getMessage(), e); - Map body = new LinkedHashMap<>(); - body.put("error", "persistence_failure"); - body.put("operation", e.operation()); - return ResponseEntity.status(503).body(body); + return ResponseEntity.status(PkAuthPersistenceResponse.STATUS) + .body(PkAuthPersistenceResponse.body(e)); } } From 32a93f680a96364dcad1f129c7aa2c06880b70b9 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:10:40 -0700 Subject: [PATCH 08/12] refactor(core,adapters): centralize relying-party and ceremony config translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the host->domain config centralization begun in be10176. Add RelyingPartyConfig.from(id,name,origins) carrying the shared required-field validation and canonical "pkauth.relying-party.* are required" message, and CeremonyConfig.from(...) which coalesces each null knob to the conservative core default (UV=REQUIRED, counter=REJECT) — a null never weakens a knob. All three adapters now delegate instead of rebuilding the records field by field; the security-relevant RP/origin/UV validation lives in one place. Dropwizard gains the friendly required-field check it previously lacked. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../pkauth/config/CeremonyConfig.java | 31 +++++++++++++++++ .../pkauth/config/RelyingPartyConfig.java | 29 ++++++++++++++++ .../pkauth/config/ConfigTest.java | 33 +++++++++++++++++++ .../spi/PkAuthPersistenceResponseTest.java | 7 ++-- .../dropwizard/dagger/PkAuthModule.java | 15 +++------ .../pkauth/micronaut/PkAuthFactory.java | 27 +++------------ .../PkAuthAutoConfiguration.java | 22 +++++-------- 7 files changed, 114 insertions(+), 50 deletions(-) diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/config/CeremonyConfig.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/config/CeremonyConfig.java index 4530f48..9cd8231 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/config/CeremonyConfig.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/config/CeremonyConfig.java @@ -6,6 +6,7 @@ import com.codeheadsystems.pkauth.api.UserVerificationRequirement; import java.time.Duration; import java.util.Objects; +import org.jspecify.annotations.Nullable; /** * Ceremony-level policy knobs. Brief §7 documents the security-relevant defaults. @@ -48,4 +49,34 @@ public static CeremonyConfig defaults() { AttestationConveyance.NONE, CounterRegressionPolicy.REJECT); } + + /** + * Builds a {@link CeremonyConfig} from raw host configuration where any knob the host left unset + * is {@code null}. Each null field falls back to the conservative value from {@link #defaults()} + * (e.g. {@code userVerification=REQUIRED}, {@code counterRegression=REJECT}); a {@code null} + * never weakens a knob — a host must pass a non-null value to relax a default. Centralizes the + * per-field default-coalescing every adapter previously performed by hand. + * + * @param challengeTtl challenge TTL, or null for the default. + * @param userVerification UV requirement, or null for the default. + * @param residentKey resident-key requirement, or null for the default. + * @param attestationConveyance attestation conveyance, or null for the default. + * @param counterRegression counter-regression policy, or null for the default. + * @return the resolved ceremony config. + * @since 1.3.1 + */ + public static CeremonyConfig from( + @Nullable Duration challengeTtl, + @Nullable UserVerificationRequirement userVerification, + @Nullable ResidentKeyRequirement residentKey, + @Nullable AttestationConveyance attestationConveyance, + @Nullable CounterRegressionPolicy counterRegression) { + CeremonyConfig d = defaults(); + return new CeremonyConfig( + challengeTtl == null ? d.challengeTtl() : challengeTtl, + userVerification == null ? d.userVerification() : userVerification, + residentKey == null ? d.residentKey() : residentKey, + attestationConveyance == null ? d.attestationConveyance() : attestationConveyance, + counterRegression == null ? d.counterRegression() : counterRegression); + } } diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/config/RelyingPartyConfig.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/config/RelyingPartyConfig.java index 58e20e9..4f1f767 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/config/RelyingPartyConfig.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/config/RelyingPartyConfig.java @@ -1,8 +1,10 @@ // SPDX-License-Identifier: MIT package com.codeheadsystems.pkauth.config; +import java.util.Collection; import java.util.Objects; import java.util.Set; +import org.jspecify.annotations.Nullable; /** * Relying-party identity used when issuing WebAuthn options. @@ -29,4 +31,31 @@ public record RelyingPartyConfig(String id, String name, Set origins) { } origins = Set.copyOf(origins); } + + /** + * Builds a {@link RelyingPartyConfig} from raw host configuration, applying the validation and + * the canonical "required — no defaults" error message every adapter shares. RP id, name, and + * origins are mandatory (there is deliberately no default); a missing or blank value raises an + * {@link IllegalStateException} naming the {@code pkauth.relying-party.*} configuration keys. + * + * @param id the RP ID (eTLD+1), or null/blank if unset. + * @param name human-readable RP name, or null/blank if unset. + * @param origins acceptable client-reported origins, or null/empty if unset; copied defensively. + * @return the validated relying-party config. + * @since 1.3.1 + */ + public static RelyingPartyConfig from( + @Nullable String id, @Nullable String name, @Nullable Collection origins) { + if (id == null + || id.isBlank() + || name == null + || name.isBlank() + || origins == null + || origins.isEmpty()) { + throw new IllegalStateException( + "pkauth.relying-party.{id,name,origins} are required. Set them explicitly in" + + " configuration — there are no defaults."); + } + return new RelyingPartyConfig(id, name, Set.copyOf(origins)); + } } diff --git a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/config/ConfigTest.java b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/config/ConfigTest.java index 984dacd..cee1a3c 100644 --- a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/config/ConfigTest.java +++ b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/config/ConfigTest.java @@ -54,4 +54,37 @@ void ceremonyConfigRejectsNonPositiveTtl() { CounterRegressionPolicy.REJECT)) .isInstanceOf(IllegalArgumentException.class); } + + @Test + void relyingPartyConfigFromValidatesAndCopies() { + RelyingPartyConfig rp = + RelyingPartyConfig.from("example.com", "Example", java.util.List.of("https://example.com")); + assertThat(rp.origins()).containsExactly("https://example.com"); + } + + @Test + void relyingPartyConfigFromRejectsMissingFieldsWithCanonicalMessage() { + assertThatThrownBy(() -> RelyingPartyConfig.from(null, "n", Set.of("https://x"))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("pkauth.relying-party"); + assertThatThrownBy(() -> RelyingPartyConfig.from("x", " ", Set.of("https://x"))) + .isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> RelyingPartyConfig.from("x", "n", Set.of())) + .isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> RelyingPartyConfig.from("x", "n", null)) + .isInstanceOf(IllegalStateException.class); + } + + @Test + void ceremonyConfigFromCoalescesNullsToDefaultsAndNeverWeakens() { + // All null -> every knob takes the conservative default. + CeremonyConfig allDefaults = CeremonyConfig.from(null, null, null, null, null); + assertThat(allDefaults).isEqualTo(CeremonyConfig.defaults()); + + // A supplied value overrides only that knob; the rest stay at the secure defaults. + CeremonyConfig overridden = CeremonyConfig.from(Duration.ofMinutes(2), null, null, null, null); + assertThat(overridden.challengeTtl()).isEqualTo(Duration.ofMinutes(2)); + assertThat(overridden.userVerification()).isEqualTo(UserVerificationRequirement.REQUIRED); + assertThat(overridden.counterRegression()).isEqualTo(CounterRegressionPolicy.REJECT); + } } diff --git a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponseTest.java b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponseTest.java index 6528750..d08919e 100644 --- a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponseTest.java +++ b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponseTest.java @@ -19,8 +19,9 @@ void bodyCarriesStableErrorCodeAndOperationInOrder() { var body = PkAuthPersistenceResponse.body(e); - assertThat(body).containsExactly( - org.assertj.core.api.Assertions.entry("error", "persistence_failure"), - org.assertj.core.api.Assertions.entry("operation", "credentials.save")); + assertThat(body) + .containsExactly( + org.assertj.core.api.Assertions.entry("error", "persistence_failure"), + org.assertj.core.api.Assertions.entry("operation", "credentials.save")); } } diff --git a/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/dagger/PkAuthModule.java b/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/dagger/PkAuthModule.java index c8ddba4..89e36b4 100644 --- a/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/dagger/PkAuthModule.java +++ b/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/dagger/PkAuthModule.java @@ -98,22 +98,15 @@ ChallengeStore provideChallengeStore() { @Singleton RelyingPartyConfig provideRelyingPartyConfig(PkAuthConfig cfg) { PkAuthConfig.RelyingParty rp = cfg.relyingParty(); - return new RelyingPartyConfig(rp.id(), rp.name(), Set.copyOf(rp.origins())); + return RelyingPartyConfig.from(rp.id(), rp.name(), rp.origins()); } @Provides @Singleton CeremonyConfig provideCeremonyConfig(PkAuthConfig cfg) { - CeremonyConfig defaults = CeremonyConfig.defaults(); - if (cfg.ceremony().challengeTtl() == null) { - return defaults; - } - return new CeremonyConfig( - cfg.ceremony().challengeTtl(), - defaults.userVerification(), - defaults.residentKey(), - defaults.attestationConveyance(), - defaults.counterRegression()); + // Only challengeTtl is host-settable here; the remaining knobs take the conservative core + // defaults (UV=REQUIRED, counter=REJECT) via the null fallbacks. + return CeremonyConfig.from(cfg.ceremony().challengeTtl(), null, null, null, null); } /** diff --git a/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthFactory.java b/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthFactory.java index 8730ab3..ca696fa 100644 --- a/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthFactory.java +++ b/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthFactory.java @@ -43,8 +43,6 @@ import jakarta.inject.Singleton; import java.util.ArrayList; import java.util.Collection; -import java.util.List; -import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,31 +60,14 @@ public class PkAuthFactory { @Singleton RelyingPartyConfig relyingPartyConfig(PkAuthConfiguration config) { PkAuthConfiguration.RelyingParty rp = config.getRelyingParty(); - String id = rp.getId(); - String name = rp.getName(); - List origins = rp.getOrigins(); - if (id == null - || id.isBlank() - || name == null - || name.isBlank() - || origins == null - || origins.isEmpty()) { - throw new IllegalStateException( - "pkauth.relying-party.{id,name,origins} are required. Set them explicitly in" - + " configuration — there are no defaults."); - } - return new RelyingPartyConfig(id, name, Set.copyOf(origins)); + return RelyingPartyConfig.from(rp.getId(), rp.getName(), rp.getOrigins()); } @Singleton CeremonyConfig ceremonyConfig(PkAuthConfiguration config) { - CeremonyConfig defaults = CeremonyConfig.defaults(); - return new CeremonyConfig( - config.getCeremony().getChallengeTtl(), - defaults.userVerification(), - defaults.residentKey(), - defaults.attestationConveyance(), - defaults.counterRegression()); + // Only challengeTtl is host-settable here; the remaining knobs take the conservative core + // defaults (UV=REQUIRED, counter=REJECT) via the null fallbacks. + return CeremonyConfig.from(config.getCeremony().getChallengeTtl(), null, null, null, null); } @Singleton diff --git a/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/autoconfigure/PkAuthAutoConfiguration.java b/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/autoconfigure/PkAuthAutoConfiguration.java index b6009ad..f530e60 100644 --- a/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/autoconfigure/PkAuthAutoConfiguration.java +++ b/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/autoconfigure/PkAuthAutoConfiguration.java @@ -102,27 +102,23 @@ public ClockProvider pkAuthClockProvider() { @ConditionalOnMissingBean public RelyingPartyConfig pkAuthRelyingPartyConfig(PkAuthProperties props) { PkAuthProperties.RelyingParty rp = props.relyingParty(); - if (rp == null) { - throw new IllegalStateException( - "pkauth.relying-party.{id,name,origins} are required. Set them explicitly in" - + " configuration — there are no defaults."); - } - return new RelyingPartyConfig(rp.id(), rp.name(), rp.origins()); + return rp == null + ? RelyingPartyConfig.from(null, null, null) + : RelyingPartyConfig.from(rp.id(), rp.name(), rp.origins()); } @Bean @ConditionalOnMissingBean public CeremonyConfig pkAuthCeremonyConfig(PkAuthProperties props) { PkAuthProperties.Ceremony c = props.ceremony(); - CeremonyConfig defaults = CeremonyConfig.defaults(); // Every knob falls back to the framework-neutral core defaults (UV=REQUIRED, counter=REJECT), // not weaker adapter-local values — a host must opt in explicitly to relax any of them. - return new CeremonyConfig( - c.challengeTtl() == null ? defaults.challengeTtl() : c.challengeTtl(), - c.userVerification() == null ? defaults.userVerification() : c.userVerification(), - c.residentKey() == null ? defaults.residentKey() : c.residentKey(), - c.attestation() == null ? defaults.attestationConveyance() : c.attestation(), - c.counterRegression() == null ? defaults.counterRegression() : c.counterRegression()); + return CeremonyConfig.from( + c.challengeTtl(), + c.userVerification(), + c.residentKey(), + c.attestation(), + c.counterRegression()); } /** From d89a20ef0a691c97482fdb897d2dd858cc497e16 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:28:08 -0700 Subject: [PATCH 09/12] feat(core,adapters)!: model start* ceremony rate-limiting as a sealed result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit start* ceremonies threw CeremonyRateLimitedException across the adapter boundary — the one place the otherwise-uniform "results, not exceptions" contract was broken, and a trap for any new adapter that forgot the handler. Introduce sealed StartRegistrationResult (Started | RateLimited) and StartAuthenticationResult, mirroring the finish* result sums: - PasskeyAuthenticationService.start* now return the sealed result; the internal limiter check returns the denied bucket instead of throwing, still evaluated before any ChallengeStore interaction (throttle-before-challenge preserved). - Each adapter controller pattern-matches the sum (Started -> 200 with the options envelope, RateLimited -> 429), so a new variant is a compile error in every adapter rather than a runtime 500. The per-adapter exception handlers are removed. - Delete CeremonyRateLimitedException; update CeremonyRateLimiter and CeremonyWireMapper docs. responseOrThrow() is a caller-side convenience for embedded/test callers that never configure a limiter. Also correct a long-standing doc error: all three adapters mount identical /auth/passkeys/... paths (verified in code); fix the false "Dropwizard mounts one segment shorter" claim in CLAUDE.md, DESIGN.md, and the browser SDK README (whose override example would have broken against the real Dropwizard adapter). Includes a google-java-format reflow of the DynamoDB refresh-token repo. BREAKING CHANGE: PasskeyAuthenticationService.startRegistration/ startAuthentication now return StartRegistrationResult/StartAuthenticationResult instead of the bare response envelope, and CeremonyRateLimitedException is removed. The /auth/** wire contract (200/429 bodies) is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 2 +- DESIGN.md | 9 +-- clients/passkeys-browser/README.md | 16 +++-- .../pkauth/api/CeremonyWireMapper.java | 9 +-- .../pkauth/api/StartAuthenticationResult.java | 64 +++++++++++++++++++ .../pkauth/api/StartRegistrationResult.java | 64 +++++++++++++++++++ .../PasskeyAuthenticationService.java | 23 +++---- .../DefaultPasskeyAuthenticationService.java | 35 ++++++---- .../spi/CeremonyRateLimitedException.java | 44 ------------- .../pkauth/spi/CeremonyRateLimiter.java | 7 +- .../pkauth/api/StartResultTest.java | 43 +++++++++++++ ...keyAuthenticationServiceRateLimitTest.java | 35 +++++----- ...faultPasskeyAuthenticationServiceTest.java | 22 +++++-- .../resource/PkAuthCeremonyResource.java | 31 ++++----- .../pkauth/jwt/CeremonyOrchestrator.java | 15 +++-- .../pkauth/jwt/CeremonyOrchestratorTest.java | 8 ++- .../micronaut/PkAuthCeremonyController.java | 38 ++++++----- .../PkAuthIntrospectionsCoverageTest.java | 9 ++- .../DynamoDbRefreshTokenRepository.java | 13 ++-- .../spring/web/PkAuthCeremonyController.java | 38 ++++++----- .../pkauth/testkit/CeremonyScenarios.java | 22 +++++-- 21 files changed, 354 insertions(+), 193 deletions(-) create mode 100644 pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/StartAuthenticationResult.java create mode 100644 pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/StartRegistrationResult.java delete mode 100644 pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimitedException.java create mode 100644 pk-auth-core/src/test/java/com/codeheadsystems/pkauth/api/StartResultTest.java diff --git a/CLAUDE.md b/CLAUDE.md index bea2d18..0d56b6d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,7 +41,7 @@ Feature modules (`pk-auth-backup-codes`, `pk-auth-magic-link`, `pk-auth-otp`, `p - **Sealed result sums, not exceptions.** Ceremony and admin operations return sealed interfaces — `AdminResult` (`Success | NotFound | Forbidden | ValidationFailed | Conflict | RateLimited`), `RegistrationResult`, `AssertionResult`, `RotateResult`, `JwtVerificationResult`. Adapters map these to HTTP; never throw across that boundary. When you add a variant, every adapter's `*ResultMapper` must handle it. - **Wire bytes are base64url, no padding** (RFC 4648 §5). Jackson 3 adapters get this from `PkAuthObjectMappers.pkAuthModule()`; the Dropwizard adapter is still on Jackson 2 and uses the `PkAuthJacksonBridge`. - **`finish` endpoints are not idempotent** — challenges are single-use via `ChallengeStore.takeOnce`. There is **no shared transaction across SPIs**: `takeOnce` is consumed before `CredentialRepository.save`; a failed save forces a ceremony restart. This is intentional — see [`docs/transactional-semantics.md`](./docs/transactional-semantics.md). -- **Dropwizard mounts paths one segment shorter** (`/auth/registration/start` vs `/auth/passkeys/registration/start`); the TS SDK handles this via a per-client path override. +- **All three adapters mount identical `/auth/**` paths** (`/auth/passkeys/**`, `/auth/refresh`, `/auth/admin/**`) — Dropwizard's Jersey resources use the same `@Path("/auth/passkeys")` etc. as Spring/Micronaut, so the TS SDK targets one path scheme everywhere (no per-client path override). - **DI annotations differ by adapter on purpose** (`CONTRIBUTING.md` §9): Spring and Micronaut auto-detect the single constructor — do **not** add `@Autowired`/`@Inject`. Dropwizard's Dagger 2 wiring **requires** `@Inject` on the injected constructor. Match the module you're in. - **Atomic-claim operations return `boolean`** so the caller can detect a race-lost claim — JDBI uses conditional `UPDATE ... WHERE consumed_at IS NULL`; DynamoDB uses `ConditionExpression` (failed condition → `ConditionalCheckFailedException` mapped to `Conflict`/`Expired`). - **DynamoDB is single-table** (`DynamoDbTable` per item type; refresh tokens write 3 items per token for the jti/user/family indexes). **JDBI uses Flyway** migrations at `pk-auth-persistence-jdbi/src/main/resources/db/migration/` — bump `PkAuthJdbiSchema.CURRENT_SCHEMA_VERSION` when adding one. diff --git a/DESIGN.md b/DESIGN.md index 956a2b1..b870cf3 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -147,10 +147,11 @@ framework-specific on the wire. | `POST` | `/auth/passkeys/authentication/finish` | Mints a JWT; returns `{token}` | | `POST` | `/auth/refresh` | Rotates a refresh token; returns `{refresh, access}` on success, `401 {detail}` on any failure. Only mounted when `pk-auth-refresh-tokens` is on the classpath and a `RefreshTokenRepository` SPI is bound. | -> The Dropwizard adapter mounts these one segment shorter -> (`/auth/registration/start`, etc.) because Dropwizard's bundle root -> path convention differs. The TypeScript SDK handles this via a -> per-client path override; see `clients/passkeys-browser/README.md`. +> All three adapters mount these paths identically — the Dropwizard +> Jersey resources use the same `@Path("/auth/passkeys")` (and +> `/auth/refresh`, `/auth/admin`) roots as the Spring and Micronaut +> controllers, so the TypeScript SDK targets one path scheme everywhere +> with no per-client path override. ### Admin endpoints (require `Authorization: Bearer `) diff --git a/clients/passkeys-browser/README.md b/clients/passkeys-browser/README.md index 0972bde..c1c97ae 100644 --- a/clients/passkeys-browser/README.md +++ b/clients/passkeys-browser/README.md @@ -49,18 +49,20 @@ await pk.ceremonies.authenticate({ conditional: true }); Wires `mediation: "conditional"` into the underlying `navigator.credentials.get` call, so the browser can offer passkeys via autofill UI before the user clicks "Sign in." -### Adapter path differences +### Overriding ceremony paths -The default ceremony paths target Spring Boot / Micronaut (`/auth/passkeys/...`). The -Dropwizard adapter omits the `/passkeys/` segment; pass `paths` to override: +All three adapters (Spring Boot, Dropwizard, Micronaut) mount the ceremony endpoints at the +same `/auth/passkeys/...` paths, which are the SDK defaults — no per-adapter override is +needed. The `paths` option remains an escape hatch for hosts that remount the endpoints under +a custom prefix: ```ts new PkAuthCeremonyClient(options, { paths: { - startReg: "/auth/registration/start", - finishReg: "/auth/registration/finish", - startAuth: "/auth/authentication/start", - finishAuth: "/auth/authentication/finish", + startReg: "/api/auth/passkeys/registration/start", + finishReg: "/api/auth/passkeys/registration/finish", + startAuth: "/api/auth/passkeys/authentication/start", + finishAuth: "/api/auth/passkeys/authentication/finish", }, }); ``` diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/CeremonyWireMapper.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/CeremonyWireMapper.java index 8c56637..af18254 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/CeremonyWireMapper.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/CeremonyWireMapper.java @@ -26,8 +26,9 @@ *

  • Assertion success → 200; {@code UnknownCredential} → 404; {@code CounterRegression} → * 409; {@code UserVerificationRequired} / {@code InvalidSignature} → 401; {@code * RateLimited} → 429; the rest → 400. - *
  • Start ceremony rate-limit refusal (a {@code CeremonyRateLimitedException} escaping the - * service) → 429; adapters call {@link #rateLimited()} to shape the response. + *
  • Start ceremony rate-limit refusal (the {@code RateLimited} variant of {@code + * StartRegistrationResult} / {@code StartAuthenticationResult}) → 429; adapters call {@link + * #rateLimited()} to shape the response. * * * @since 0.9.0 @@ -74,8 +75,8 @@ public static CeremonyResponse forRegistration(RegistrationResult result) { /** * Canonical 429 response shape for {@code start*} ceremony rate-limit refusals. Adapter - * controllers use this when {@link com.codeheadsystems.pkauth.spi.CeremonyRateLimitedException} - * escapes the service. + * controllers use this for the {@code RateLimited} variant of {@code StartRegistrationResult} / + * {@code StartAuthenticationResult}. * * @return canonical rate-limited response (HTTP 429, body {@code {"outcome": "rate_limited"}}) * @since 0.9.1 diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/StartAuthenticationResult.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/StartAuthenticationResult.java new file mode 100644 index 0000000..47584b7 --- /dev/null +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/StartAuthenticationResult.java @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: MIT +package com.codeheadsystems.pkauth.api; + +import java.util.Objects; + +/** + * Sealed result of {@code PasskeyAuthenticationService.startAuthentication}. Mirrors the {@code + * finish*} ceremonies' result-sum discipline so a rate-limit refusal is a value, not an exception + * thrown across the adapter boundary. + * + *
      + *
    • {@link Started} — the limiter allowed the call; carries the {@link + * StartAuthenticationResponse} envelope the browser consumes. + *
    • {@link RateLimited} — the configured {@code CeremonyRateLimiter} refused the call before + * any challenge was created; adapters map it to HTTP {@code 429}. + *
    + * + * @since 1.3.1 + */ +public sealed interface StartAuthenticationResult + permits StartAuthenticationResult.Started, StartAuthenticationResult.RateLimited { + + /** + * The limiter allowed the ceremony; {@code response} carries the WebAuthn request options. + * + * @param response the start-authentication envelope. + * @since 1.3.1 + */ + record Started(StartAuthenticationResponse response) implements StartAuthenticationResult { + public Started { + Objects.requireNonNull(response, "response"); + } + } + + /** + * The configured rate limiter refused the call before any challenge was created. + * + * @param bucket which limiter bucket denied the call ({@code "ip"} or {@code "username"}). + * @since 1.3.1 + */ + record RateLimited(String bucket) implements StartAuthenticationResult { + public RateLimited { + Objects.requireNonNull(bucket, "bucket"); + } + } + + /** + * Convenience for embedded/test callers that do not configure a rate limiter and therefore never + * expect a refusal. Returns the {@link StartAuthenticationResponse} on {@link Started}; throws on + * {@link RateLimited}. Adapter controllers must NOT use this — they pattern-match the sum so the + * {@code 429} path is handled explicitly. + * + * @return the start-authentication envelope. + * @throws IllegalStateException if this result is {@link RateLimited}. + * @since 1.3.1 + */ + default StartAuthenticationResponse responseOrThrow() { + if (this instanceof Started started) { + return started.response(); + } + throw new IllegalStateException( + "ceremony was rate-limited (bucket=" + ((RateLimited) this).bucket() + ")"); + } +} diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/StartRegistrationResult.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/StartRegistrationResult.java new file mode 100644 index 0000000..b36a3f8 --- /dev/null +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/api/StartRegistrationResult.java @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: MIT +package com.codeheadsystems.pkauth.api; + +import java.util.Objects; + +/** + * Sealed result of {@code PasskeyAuthenticationService.startRegistration}. Mirrors the {@code + * finish*} ceremonies' result-sum discipline so a rate-limit refusal is a value, not an exception + * thrown across the adapter boundary. + * + *
      + *
    • {@link Started} — the limiter allowed the call; carries the {@link + * StartRegistrationResponse} envelope the browser consumes. + *
    • {@link RateLimited} — the configured {@code CeremonyRateLimiter} refused the call before + * any challenge was created; adapters map it to HTTP {@code 429}. + *
    + * + * @since 1.3.1 + */ +public sealed interface StartRegistrationResult + permits StartRegistrationResult.Started, StartRegistrationResult.RateLimited { + + /** + * The limiter allowed the ceremony; {@code response} carries the WebAuthn creation options. + * + * @param response the start-registration envelope. + * @since 1.3.1 + */ + record Started(StartRegistrationResponse response) implements StartRegistrationResult { + public Started { + Objects.requireNonNull(response, "response"); + } + } + + /** + * The configured rate limiter refused the call before any challenge was created. + * + * @param bucket which limiter bucket denied the call ({@code "ip"} or {@code "username"}). + * @since 1.3.1 + */ + record RateLimited(String bucket) implements StartRegistrationResult { + public RateLimited { + Objects.requireNonNull(bucket, "bucket"); + } + } + + /** + * Convenience for embedded/test callers that do not configure a rate limiter and therefore never + * expect a refusal. Returns the {@link StartRegistrationResponse} on {@link Started}; throws on + * {@link RateLimited}. Adapter controllers must NOT use this — they pattern-match the sum so the + * {@code 429} path is handled explicitly. + * + * @return the start-registration envelope. + * @throws IllegalStateException if this result is {@link RateLimited}. + * @since 1.3.1 + */ + default StartRegistrationResponse responseOrThrow() { + if (this instanceof Started started) { + return started.response(); + } + throw new IllegalStateException( + "ceremony was rate-limited (bucket=" + ((RateLimited) this).bucket() + ")"); + } +} diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/ceremony/PasskeyAuthenticationService.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/ceremony/PasskeyAuthenticationService.java index 4baa21c..8e01aab 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/ceremony/PasskeyAuthenticationService.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/ceremony/PasskeyAuthenticationService.java @@ -6,10 +6,9 @@ import com.codeheadsystems.pkauth.api.FinishRegistrationRequest; import com.codeheadsystems.pkauth.api.RegistrationResult; import com.codeheadsystems.pkauth.api.StartAuthenticationRequest; -import com.codeheadsystems.pkauth.api.StartAuthenticationResponse; +import com.codeheadsystems.pkauth.api.StartAuthenticationResult; import com.codeheadsystems.pkauth.api.StartRegistrationRequest; -import com.codeheadsystems.pkauth.api.StartRegistrationResponse; -import com.codeheadsystems.pkauth.spi.CeremonyRateLimitedException; +import com.codeheadsystems.pkauth.api.StartRegistrationResult; import org.jspecify.annotations.Nullable; /** @@ -30,7 +29,7 @@ public interface PasskeyAuthenticationService { * both the WebAuthn options the browser consumes and the {@code ChallengeId} the client must * round-trip in {@code finishRegistration}. */ - default StartRegistrationResponse startRegistration(StartRegistrationRequest req) { + default StartRegistrationResult startRegistration(StartRegistrationRequest req) { return startRegistration(req, null); } @@ -43,11 +42,12 @@ default StartRegistrationResponse startRegistration(StartRegistrationRequest req * @param req start request * @param clientIp source IP address of the HTTP request, or {@code null} when the host cannot * determine one — the limiter implementation decides how to handle the null case - * @return start-registration envelope - * @throws CeremonyRateLimitedException when the configured limiter refuses the call + * @return start-registration result; {@link StartRegistrationResult.RateLimited} when the limiter + * refuses the call (before any challenge is created), otherwise {@link + * StartRegistrationResult.Started} * @since 0.9.1 */ - StartRegistrationResponse startRegistration( + StartRegistrationResult startRegistration( StartRegistrationRequest req, @Nullable String clientIp); /** Verify a registration response and produce a persistable credential record. */ @@ -75,7 +75,7 @@ default RegistrationResult finishRegistration(FinishRegistrationRequest req) { * WebAuthn options the browser consumes and the {@code ChallengeId} the client must round-trip in * {@code finishAuthentication}. */ - default StartAuthenticationResponse startAuthentication(StartAuthenticationRequest req) { + default StartAuthenticationResult startAuthentication(StartAuthenticationRequest req) { return startAuthentication(req, null); } @@ -86,11 +86,12 @@ default StartAuthenticationResponse startAuthentication(StartAuthenticationReque * @param req start request * @param clientIp source IP address of the HTTP request, or {@code null} when the host cannot * determine one - * @return start-authentication envelope - * @throws CeremonyRateLimitedException when the configured limiter refuses the call + * @return start-authentication result; {@link StartAuthenticationResult.RateLimited} when the + * limiter refuses the call (before any challenge is created), otherwise {@link + * StartAuthenticationResult.Started} * @since 0.9.1 */ - StartAuthenticationResponse startAuthentication( + StartAuthenticationResult startAuthentication( StartAuthenticationRequest req, @Nullable String clientIp); /** Verify an authentication response. */ diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationService.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationService.java index f63589e..e5adacb 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationService.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationService.java @@ -16,8 +16,10 @@ import com.codeheadsystems.pkauth.api.RegistrationResult; import com.codeheadsystems.pkauth.api.StartAuthenticationRequest; import com.codeheadsystems.pkauth.api.StartAuthenticationResponse; +import com.codeheadsystems.pkauth.api.StartAuthenticationResult; import com.codeheadsystems.pkauth.api.StartRegistrationRequest; import com.codeheadsystems.pkauth.api.StartRegistrationResponse; +import com.codeheadsystems.pkauth.api.StartRegistrationResult; import com.codeheadsystems.pkauth.api.Transport; import com.codeheadsystems.pkauth.api.UserHandle; import com.codeheadsystems.pkauth.api.UserVerificationRequirement; @@ -29,7 +31,6 @@ import com.codeheadsystems.pkauth.credential.CredentialRecord; import com.codeheadsystems.pkauth.metrics.Metrics; import com.codeheadsystems.pkauth.spi.AttestationTrustPolicy; -import com.codeheadsystems.pkauth.spi.CeremonyRateLimitedException; import com.codeheadsystems.pkauth.spi.CeremonyRateLimiter; import com.codeheadsystems.pkauth.spi.ChallengeRecord; import com.codeheadsystems.pkauth.spi.ChallengeStore; @@ -216,10 +217,13 @@ public boolean tryAcquireForUsername(String username) { * @since 0.9.1 */ @Override - public StartRegistrationResponse startRegistration( + public StartRegistrationResult startRegistration( StartRegistrationRequest req, @Nullable String clientIp) { Objects.requireNonNull(req, "req"); - enforceRateLimit("registration.start", clientIp, req.username()); + String deniedBucket = rateLimitedBucket("registration.start", clientIp, req.username()); + if (deniedBucket != null) { + return new StartRegistrationResult.RateLimited(deniedBucket); + } UserHandle userHandle = userLookup.getOrCreateHandle(req.username()); byte[] challenge = challengeGenerator.generate(); @@ -266,7 +270,7 @@ public StartRegistrationResponse startRegistration( metrics.incrementCounter("pkauth.registration.start", "rp", rpConfig.id()); LOG.info("registration.start userHandle={} challengeId={}", userHandle, challengeId.value()); - return new StartRegistrationResponse(challengeId, options); + return new StartRegistrationResult.Started(new StartRegistrationResponse(challengeId, options)); } @Override @@ -447,10 +451,13 @@ private RegistrationResult persistRegistration( * @since 0.9.1 */ @Override - public StartAuthenticationResponse startAuthentication( + public StartAuthenticationResult startAuthentication( StartAuthenticationRequest req, @Nullable String clientIp) { Objects.requireNonNull(req, "req"); - enforceRateLimit("authentication.start", clientIp, req.username()); + String deniedBucket = rateLimitedBucket("authentication.start", clientIp, req.username()); + if (deniedBucket != null) { + return new StartAuthenticationResult.RateLimited(deniedBucket); + } @Nullable UserHandle resolvedHandle = null; // Always non-null: unknown usernames and the usernameless flow both yield an empty list @@ -490,7 +497,8 @@ public StartAuthenticationResponse startAuthentication( metrics.incrementCounter("pkauth.authentication.start", "rp", rpConfig.id()); LOG.info( "authentication.start username={} challengeId={}", req.username(), challengeId.value()); - return new StartAuthenticationResponse(challengeId, options); + return new StartAuthenticationResult.Started( + new StartAuthenticationResponse(challengeId, options)); } @Override @@ -850,18 +858,21 @@ private AssertionResult outcomeAssertion(AssertionResult result, long start) { /** * Consults the configured {@link CeremonyRateLimiter} for both the per-IP and per-username - * buckets on a {@code start*} call. Throws {@link CeremonyRateLimitedException} when either - * bucket denies; the adapter controller catches this and emits {@code 429 Too Many Requests}. + * buckets on a {@code start*} call. Returns the name of the bucket that denied the call ({@code + * "ip"} or {@code "username"}), or {@code null} when both buckets allow it. The caller surfaces a + * non-null result as the {@code RateLimited} variant of the relevant start-result sum — and MUST + * do so before creating any challenge, so a throttled caller never touches the ChallengeStore. */ - private void enforceRateLimit( + private @Nullable String rateLimitedBucket( String phase, @Nullable String clientIp, @Nullable String username) { if (!rateLimiter.tryAcquireForIp(clientIp)) { LOG.info("{} rate-limited ip-bucket clientIp={}", phase, clientIp); - throw new CeremonyRateLimitedException("ip"); + return "ip"; } if (username != null && !rateLimiter.tryAcquireForUsername(username)) { LOG.info("{} rate-limited username-bucket username={}", phase, username); - throw new CeremonyRateLimitedException("username"); + return "username"; } + return null; } } diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimitedException.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimitedException.java deleted file mode 100644 index de11bb8..0000000 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimitedException.java +++ /dev/null @@ -1,44 +0,0 @@ -// SPDX-License-Identifier: MIT -package com.codeheadsystems.pkauth.spi; - -/** - * Thrown by {@code PasskeyAuthenticationService.startRegistration} and {@code startAuthentication} - * when the configured {@link CeremonyRateLimiter} refuses the call. Adapters map this exception to - * HTTP {@code 429 Too Many Requests}. - * - *

    This exception type is reserved for the {@code start*} ceremony entrypoints because their - * response envelopes ({@code StartRegistrationResponse}, {@code StartAuthenticationResponse}) are - * not sealed result sums and therefore cannot grow a {@code RateLimited} variant without a - * disruptive API rewrite. The {@code finish*} entrypoints surface the same refusal through the - * sealed-result variants {@code RegistrationResult.RateLimited} and {@code - * AssertionResult.RateLimited}. - * - * @since 0.9.1 - */ -public final class CeremonyRateLimitedException extends RuntimeException { - - private static final long serialVersionUID = 1L; - - private final String bucket; - - /** - * Constructs an exception for the given limiter bucket. - * - * @param bucket which limiter bucket denied the call ({@code "ip"} or {@code "username"}) - * @since 0.9.1 - */ - public CeremonyRateLimitedException(String bucket) { - super("ceremony rate-limited (bucket=" + bucket + ")"); - this.bucket = bucket; - } - - /** - * Returns the limiter bucket that denied the call ({@code "ip"} or {@code "username"}). - * - * @return bucket name - * @since 0.9.1 - */ - public String bucket() { - return bucket; - } -} diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimiter.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimiter.java index 6733411..d46fce3 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimiter.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimiter.java @@ -27,9 +27,10 @@ * * *

    When the limiter denies a call, the ceremony service short-circuits before generating / - * consulting a challenge and surfaces the refusal through the existing sealed result types ({@code - * AssertionResult.RateLimited}, {@code RegistrationResult.RateLimited}) for {@code finish*} and - * through {@link CeremonyRateLimitedException} for {@code start*}. + * consulting a challenge and surfaces the refusal through sealed result types: {@code + * AssertionResult.RateLimited} / {@code RegistrationResult.RateLimited} for {@code finish*}, and + * {@code StartAuthenticationResult.RateLimited} / {@code StartRegistrationResult.RateLimited} for + * {@code start*}. * *

    Production deployments with more than one replica MUST override this SPI with a * shared, race-safe limiter (Redis token-bucket, etc.). The default implementation supplied by diff --git a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/api/StartResultTest.java b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/api/StartResultTest.java new file mode 100644 index 0000000..0c6e705 --- /dev/null +++ b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/api/StartResultTest.java @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: MIT +package com.codeheadsystems.pkauth.api; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; + +import org.junit.jupiter.api.Test; + +class StartResultTest { + + @Test + void registrationStartedUnwrapsResponse() { + StartRegistrationResponse response = mock(StartRegistrationResponse.class); + StartRegistrationResult result = new StartRegistrationResult.Started(response); + assertThat(result.responseOrThrow()).isSameAs(response); + } + + @Test + void registrationRateLimitedThrowsAndCarriesBucket() { + StartRegistrationResult result = new StartRegistrationResult.RateLimited("ip"); + assertThat(((StartRegistrationResult.RateLimited) result).bucket()).isEqualTo("ip"); + assertThatThrownBy(result::responseOrThrow) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("ip"); + } + + @Test + void authenticationStartedUnwrapsResponse() { + StartAuthenticationResponse response = mock(StartAuthenticationResponse.class); + StartAuthenticationResult result = new StartAuthenticationResult.Started(response); + assertThat(result.responseOrThrow()).isSameAs(response); + } + + @Test + void authenticationRateLimitedThrowsAndCarriesBucket() { + StartAuthenticationResult result = new StartAuthenticationResult.RateLimited("username"); + assertThat(((StartAuthenticationResult.RateLimited) result).bucket()).isEqualTo("username"); + assertThatThrownBy(result::responseOrThrow) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("username"); + } +} diff --git a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceRateLimitTest.java b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceRateLimitTest.java index 552db8c..e139e44 100644 --- a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceRateLimitTest.java +++ b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceRateLimitTest.java @@ -2,7 +2,6 @@ package com.codeheadsystems.pkauth.internal; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; @@ -21,7 +20,9 @@ import com.codeheadsystems.pkauth.api.RegistrationResult; import com.codeheadsystems.pkauth.api.ResidentKeyRequirement; import com.codeheadsystems.pkauth.api.StartAuthenticationRequest; +import com.codeheadsystems.pkauth.api.StartAuthenticationResult; import com.codeheadsystems.pkauth.api.StartRegistrationRequest; +import com.codeheadsystems.pkauth.api.StartRegistrationResult; import com.codeheadsystems.pkauth.api.UserHandle; import com.codeheadsystems.pkauth.api.UserVerificationRequirement; import com.codeheadsystems.pkauth.config.CeremonyConfig; @@ -29,7 +30,6 @@ import com.codeheadsystems.pkauth.config.RelyingPartyConfig; import com.codeheadsystems.pkauth.metrics.Metrics; import com.codeheadsystems.pkauth.spi.AttestationTrustPolicy; -import com.codeheadsystems.pkauth.spi.CeremonyRateLimitedException; import com.codeheadsystems.pkauth.spi.CeremonyRateLimiter; import com.codeheadsystems.pkauth.spi.ChallengeStore; import com.codeheadsystems.pkauth.spi.ClockProvider; @@ -128,13 +128,12 @@ public void nextBytes(byte[] bytes) { void startRegistrationRefusedWhenIpBucketDenies() { limiter.denyIp = true; - assertThatThrownBy( - () -> - service.startRegistration( - new StartRegistrationRequest("alice", "Alice", null, null), "1.2.3.4")) - .isInstanceOf(CeremonyRateLimitedException.class) - .satisfies(t -> assertThat(((CeremonyRateLimitedException) t).bucket()).isEqualTo("ip")); + StartRegistrationResult result = + service.startRegistration( + new StartRegistrationRequest("alice", "Alice", null, null), "1.2.3.4"); + assertThat(result).isInstanceOf(StartRegistrationResult.RateLimited.class); + assertThat(((StartRegistrationResult.RateLimited) result).bucket()).isEqualTo("ip"); assertThat(limiter.ipCalls).containsExactly("1.2.3.4"); assertThat(limiter.usernameCalls).isEmpty(); verify(challengeStore, never()).put(any(), any(), any()); @@ -144,14 +143,12 @@ void startRegistrationRefusedWhenIpBucketDenies() { void startRegistrationRefusedWhenUsernameBucketDenies() { limiter.denyUsername = true; - assertThatThrownBy( - () -> - service.startRegistration( - new StartRegistrationRequest("alice", "Alice", null, null), "1.2.3.4")) - .isInstanceOf(CeremonyRateLimitedException.class) - .satisfies( - t -> assertThat(((CeremonyRateLimitedException) t).bucket()).isEqualTo("username")); + StartRegistrationResult result = + service.startRegistration( + new StartRegistrationRequest("alice", "Alice", null, null), "1.2.3.4"); + assertThat(result).isInstanceOf(StartRegistrationResult.RateLimited.class); + assertThat(((StartRegistrationResult.RateLimited) result).bucket()).isEqualTo("username"); assertThat(limiter.ipCalls).containsExactly("1.2.3.4"); assertThat(limiter.usernameCalls).containsExactly("alice"); verify(challengeStore, never()).put(any(), any(), any()); @@ -161,12 +158,10 @@ void startRegistrationRefusedWhenUsernameBucketDenies() { void startAuthenticationRefusedWhenIpBucketDenies() { limiter.denyIp = true; - assertThatThrownBy( - () -> - service.startAuthentication( - new StartAuthenticationRequest("alice", null), "9.9.9.9")) - .isInstanceOf(CeremonyRateLimitedException.class); + StartAuthenticationResult result = + service.startAuthentication(new StartAuthenticationRequest("alice", null), "9.9.9.9"); + assertThat(result).isInstanceOf(StartAuthenticationResult.RateLimited.class); verify(challengeStore, never()).put(any(), any(), any()); } diff --git a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceTest.java b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceTest.java index 54655b8..91d603b 100644 --- a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceTest.java +++ b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceTest.java @@ -150,7 +150,9 @@ void startRegistrationPersistsChallengeAndReturnsEnvelope() { when(credentialRepository.findByUserHandle(USER_HANDLE)).thenReturn(List.of()); StartRegistrationResponse resp = - service.startRegistration(new StartRegistrationRequest("alice", "Alice", null, null)); + service + .startRegistration(new StartRegistrationRequest("alice", "Alice", null, null)) + .responseOrThrow(); // The challengeId is now an opaque random handle, independent of the challenge bytes. Assert it // is well-formed and that the SAME id is what gets persisted (the binding to the bytes is @@ -181,7 +183,9 @@ void startRegistrationSerializesEmptyExcludeCredentialsAsJsonArray() { when(credentialRepository.findByUserHandle(USER_HANDLE)).thenReturn(List.of()); StartRegistrationResponse resp = - service.startRegistration(new StartRegistrationRequest("brand-new", null, null, null)); + service + .startRegistration(new StartRegistrationRequest("brand-new", null, null, null)) + .responseOrThrow(); String json = jsonMapper.writeValueAsString(resp.publicKey()); assertThat(json).contains("\"excludeCredentials\":[]"); } @@ -193,7 +197,9 @@ void startAuthenticationIncludesAllowCredentialsForKnownUser() { .thenReturn(List.of(stubStoredCredential())); StartAuthenticationResponse resp = - service.startAuthentication(new StartAuthenticationRequest("alice", null)); + service + .startAuthentication(new StartAuthenticationRequest("alice", null)) + .responseOrThrow(); assertThat(resp.publicKey().rpId()).isEqualTo("example.com"); assertThat(resp.publicKey().allowCredentials()).hasSize(1); @@ -208,7 +214,7 @@ void startAuthenticationReturnsEmptyAllowCredentialsForUsernamelessFlow() { // empty list so // it is wire-indistinguishable from an unknown-username request. StartAuthenticationResponse resp = - service.startAuthentication(new StartAuthenticationRequest(null, null)); + service.startAuthentication(new StartAuthenticationRequest(null, null)).responseOrThrow(); assertThat(resp.publicKey().allowCredentials()).isNotNull().isEmpty(); } @@ -221,7 +227,9 @@ void startAuthenticationReturnsEmptyAllowCredentialsForUnknownUsername() { when(userLookup.findHandleByUsername("ghost")).thenReturn(Optional.empty()); StartAuthenticationResponse resp = - service.startAuthentication(new StartAuthenticationRequest("ghost", null)); + service + .startAuthentication(new StartAuthenticationRequest("ghost", null)) + .responseOrThrow(); assertThat(resp.publicKey().allowCredentials()).isNotNull().isEmpty(); } @@ -232,7 +240,9 @@ void startAuthenticationSerializesEmptyAllowCredentialsAsJsonArray() { // "allowCredentials":[] field present in the JSON, not an omitted field. when(userLookup.findHandleByUsername("ghost")).thenReturn(Optional.empty()); StartAuthenticationResponse resp = - service.startAuthentication(new StartAuthenticationRequest("ghost", null)); + service + .startAuthentication(new StartAuthenticationRequest("ghost", null)) + .responseOrThrow(); String json = jsonMapper.writeValueAsString(resp.publicKey()); assertThat(json).contains("\"allowCredentials\":[]"); } diff --git a/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/resource/PkAuthCeremonyResource.java b/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/resource/PkAuthCeremonyResource.java index d77341d..0ed2987 100644 --- a/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/resource/PkAuthCeremonyResource.java +++ b/pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/resource/PkAuthCeremonyResource.java @@ -5,11 +5,10 @@ import com.codeheadsystems.pkauth.api.FinishAuthenticationRequest; import com.codeheadsystems.pkauth.api.FinishRegistrationRequest; import com.codeheadsystems.pkauth.api.StartAuthenticationRequest; -import com.codeheadsystems.pkauth.api.StartAuthenticationResponse; +import com.codeheadsystems.pkauth.api.StartAuthenticationResult; import com.codeheadsystems.pkauth.api.StartRegistrationRequest; -import com.codeheadsystems.pkauth.api.StartRegistrationResponse; +import com.codeheadsystems.pkauth.api.StartRegistrationResult; import com.codeheadsystems.pkauth.jwt.CeremonyOrchestrator; -import com.codeheadsystems.pkauth.spi.CeremonyRateLimitedException; import jakarta.inject.Inject; import jakarta.servlet.http.HttpServletRequest; import jakarta.ws.rs.Consumes; @@ -46,13 +45,10 @@ public PkAuthCeremonyResource(CeremonyOrchestrator orchestrator) { @Path("/registration/start") public Response startRegistration( StartRegistrationRequest request, @Context HttpServletRequest httpRequest) { - try { - StartRegistrationResponse body = - orchestrator.startRegistration(request, clientIp(httpRequest)); - return Response.ok(body).build(); - } catch (CeremonyRateLimitedException ex) { - return rateLimitedResponse(ex); - } + return switch (orchestrator.startRegistration(request, clientIp(httpRequest))) { + case StartRegistrationResult.Started started -> Response.ok(started.response()).build(); + case StartRegistrationResult.RateLimited rl -> rateLimitedResponse(rl.bucket()); + }; } @POST @@ -66,13 +62,10 @@ public Response finishRegistration( @Path("/authentication/start") public Response startAuthentication( StartAuthenticationRequest request, @Context HttpServletRequest httpRequest) { - try { - StartAuthenticationResponse body = - orchestrator.startAuthentication(request, clientIp(httpRequest)); - return Response.ok(body).build(); - } catch (CeremonyRateLimitedException ex) { - return rateLimitedResponse(ex); - } + return switch (orchestrator.startAuthentication(request, clientIp(httpRequest))) { + case StartAuthenticationResult.Started started -> Response.ok(started.response()).build(); + case StartAuthenticationResult.RateLimited rl -> rateLimitedResponse(rl.bucket()); + }; } @POST @@ -90,8 +83,8 @@ private static String clientIp(HttpServletRequest httpRequest) { return httpRequest == null ? null : httpRequest.getRemoteAddr(); } - private Response rateLimitedResponse(CeremonyRateLimitedException ex) { - LOG.info("auth.ceremony rate-limited bucket={}", ex.bucket()); + private Response rateLimitedResponse(String bucket) { + LOG.info("auth.ceremony rate-limited bucket={}", bucket); return toResponse(orchestrator.rateLimited()); } } diff --git a/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/CeremonyOrchestrator.java b/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/CeremonyOrchestrator.java index d246a41..871be6f 100644 --- a/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/CeremonyOrchestrator.java +++ b/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/CeremonyOrchestrator.java @@ -8,9 +8,9 @@ import com.codeheadsystems.pkauth.api.FinishRegistrationRequest; import com.codeheadsystems.pkauth.api.RegistrationResult; import com.codeheadsystems.pkauth.api.StartAuthenticationRequest; -import com.codeheadsystems.pkauth.api.StartAuthenticationResponse; +import com.codeheadsystems.pkauth.api.StartAuthenticationResult; import com.codeheadsystems.pkauth.api.StartRegistrationRequest; -import com.codeheadsystems.pkauth.api.StartRegistrationResponse; +import com.codeheadsystems.pkauth.api.StartRegistrationResult; import com.codeheadsystems.pkauth.ceremony.PasskeyAuthenticationService; import com.codeheadsystems.pkauth.credential.CredentialRecord; import com.codeheadsystems.pkauth.spi.CredentialRepository; @@ -54,13 +54,13 @@ public CeremonyOrchestrator( } /** Delegates to {@link PasskeyAuthenticationService#startRegistration}. */ - public StartRegistrationResponse startRegistration( + public StartRegistrationResult startRegistration( StartRegistrationRequest request, @Nullable String clientIp) { return service.startRegistration(request, clientIp); } /** Delegates to {@link PasskeyAuthenticationService#startAuthentication}. */ - public StartAuthenticationResponse startAuthentication( + public StartAuthenticationResult startAuthentication( StartAuthenticationRequest request, @Nullable String clientIp) { return service.startAuthentication(request, clientIp); } @@ -109,9 +109,10 @@ public CeremonyResponse finishAuthentication( } /** - * Canonical wire shape for a {@code start*} ceremony rate-limit refusal. Adapter exception - * handlers convert {@link com.codeheadsystems.pkauth.spi.CeremonyRateLimitedException} into a - * {@link CeremonyResponse} via this helper. + * Canonical wire shape (HTTP {@code 429}) for a {@code start*} ceremony rate-limit refusal. + * Adapter controllers call this for the {@code RateLimited} variant of {@link + * com.codeheadsystems.pkauth.api.StartRegistrationResult} / {@link + * com.codeheadsystems.pkauth.api.StartAuthenticationResult}. */ public CeremonyResponse rateLimited() { return CeremonyWireMapper.rateLimited(); diff --git a/pk-auth-jwt/src/test/java/com/codeheadsystems/pkauth/jwt/CeremonyOrchestratorTest.java b/pk-auth-jwt/src/test/java/com/codeheadsystems/pkauth/jwt/CeremonyOrchestratorTest.java index c2c52e2..2b6b00b 100644 --- a/pk-auth-jwt/src/test/java/com/codeheadsystems/pkauth/jwt/CeremonyOrchestratorTest.java +++ b/pk-auth-jwt/src/test/java/com/codeheadsystems/pkauth/jwt/CeremonyOrchestratorTest.java @@ -15,8 +15,10 @@ import com.codeheadsystems.pkauth.api.RegistrationResult; import com.codeheadsystems.pkauth.api.StartAuthenticationRequest; import com.codeheadsystems.pkauth.api.StartAuthenticationResponse; +import com.codeheadsystems.pkauth.api.StartAuthenticationResult; import com.codeheadsystems.pkauth.api.StartRegistrationRequest; import com.codeheadsystems.pkauth.api.StartRegistrationResponse; +import com.codeheadsystems.pkauth.api.StartRegistrationResult; import com.codeheadsystems.pkauth.api.Transport; import com.codeheadsystems.pkauth.api.UserHandle; import com.codeheadsystems.pkauth.ceremony.PasskeyAuthenticationService; @@ -60,7 +62,8 @@ void constructorRejectsNulls() { @Test void startRegistrationDelegates() { StartRegistrationRequest req = mock(StartRegistrationRequest.class); - StartRegistrationResponse resp = mock(StartRegistrationResponse.class); + StartRegistrationResult resp = + new StartRegistrationResult.Started(mock(StartRegistrationResponse.class)); when(service.startRegistration(req, "1.2.3.4")).thenReturn(resp); assertThat(orchestrator.startRegistration(req, "1.2.3.4")).isSameAs(resp); } @@ -68,7 +71,8 @@ void startRegistrationDelegates() { @Test void startAuthenticationDelegates() { StartAuthenticationRequest req = mock(StartAuthenticationRequest.class); - StartAuthenticationResponse resp = mock(StartAuthenticationResponse.class); + StartAuthenticationResult resp = + new StartAuthenticationResult.Started(mock(StartAuthenticationResponse.class)); when(service.startAuthentication(req, null)).thenReturn(resp); assertThat(orchestrator.startAuthentication(req, null)).isSameAs(resp); } diff --git a/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthCeremonyController.java b/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthCeremonyController.java index 3fc09b0..2669220 100644 --- a/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthCeremonyController.java +++ b/pk-auth-micronaut/src/main/java/com/codeheadsystems/pkauth/micronaut/PkAuthCeremonyController.java @@ -5,18 +5,16 @@ import com.codeheadsystems.pkauth.api.FinishAuthenticationRequest; import com.codeheadsystems.pkauth.api.FinishRegistrationRequest; import com.codeheadsystems.pkauth.api.StartAuthenticationRequest; -import com.codeheadsystems.pkauth.api.StartAuthenticationResponse; +import com.codeheadsystems.pkauth.api.StartAuthenticationResult; import com.codeheadsystems.pkauth.api.StartRegistrationRequest; -import com.codeheadsystems.pkauth.api.StartRegistrationResponse; +import com.codeheadsystems.pkauth.api.StartRegistrationResult; import com.codeheadsystems.pkauth.jwt.CeremonyOrchestrator; -import com.codeheadsystems.pkauth.spi.CeremonyRateLimitedException; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; import io.micronaut.http.MediaType; import io.micronaut.http.annotation.Body; import io.micronaut.http.annotation.Controller; -import io.micronaut.http.annotation.Error; import io.micronaut.http.annotation.Post; import io.micronaut.http.annotation.Produces; import io.micronaut.scheduling.TaskExecutors; @@ -50,9 +48,15 @@ public PkAuthCeremonyController(CeremonyOrchestrator orchestrator) { } @Post("/registration/start") - public HttpResponse startRegistration( + public HttpResponse startRegistration( @Body StartRegistrationRequest req, HttpRequest httpRequest) { - return HttpResponse.ok(orchestrator.startRegistration(req, clientIp(httpRequest))); + return switch (orchestrator.startRegistration(req, clientIp(httpRequest))) { + case StartRegistrationResult.Started started -> HttpResponse.ok(started.response()); + case StartRegistrationResult.RateLimited rl -> { + LOG.info("auth.registration.start rate-limited bucket={}", rl.bucket()); + yield toResponse(orchestrator.rateLimited()); + } + }; } @Post("/registration/finish") @@ -62,9 +66,15 @@ public HttpResponse> finishRegistration( } @Post("/authentication/start") - public HttpResponse startAuthentication( + public HttpResponse startAuthentication( @Body StartAuthenticationRequest req, HttpRequest httpRequest) { - return HttpResponse.ok(orchestrator.startAuthentication(req, clientIp(httpRequest))); + return switch (orchestrator.startAuthentication(req, clientIp(httpRequest))) { + case StartAuthenticationResult.Started started -> HttpResponse.ok(started.response()); + case StartAuthenticationResult.RateLimited rl -> { + LOG.info("auth.authentication.start rate-limited bucket={}", rl.bucket()); + yield toResponse(orchestrator.rateLimited()); + } + }; } @Post("/authentication/finish") @@ -73,18 +83,6 @@ public HttpResponse> finishAuthentication( return toResponse(orchestrator.finishAuthentication(req, clientIp(httpRequest))); } - /** - * Maps a {@link CeremonyRateLimitedException} thrown by {@code startRegistration} / {@code - * startAuthentication} to the canonical {@code 429} wire shape. - * - * @since 0.9.1 - */ - @Error(exception = CeremonyRateLimitedException.class) - public HttpResponse> handleRateLimited(CeremonyRateLimitedException ex) { - LOG.info("auth.ceremony rate-limited bucket={}", ex.bucket()); - return toResponse(orchestrator.rateLimited()); - } - private static HttpResponse> toResponse(CeremonyResponse wire) { return HttpResponse.>status(HttpStatus.valueOf(wire.status())) .body(wire.body()); diff --git a/pk-auth-micronaut/src/test/java/com/codeheadsystems/pkauth/micronaut/PkAuthIntrospectionsCoverageTest.java b/pk-auth-micronaut/src/test/java/com/codeheadsystems/pkauth/micronaut/PkAuthIntrospectionsCoverageTest.java index 0d5521e..cc775f6 100644 --- a/pk-auth-micronaut/src/test/java/com/codeheadsystems/pkauth/micronaut/PkAuthIntrospectionsCoverageTest.java +++ b/pk-auth-micronaut/src/test/java/com/codeheadsystems/pkauth/micronaut/PkAuthIntrospectionsCoverageTest.java @@ -44,7 +44,14 @@ class PkAuthIntrospectionsCoverageTest { // Internal mapper return shapes — the adapter unwraps these into framework // response objects, the records themselves are never JSON-serialized. "com.codeheadsystems.pkauth.api.CeremonyWireMapper$CeremonyResponse", - "com.codeheadsystems.pkauth.admin.AdminResponseMapper$AdminResponse"); + "com.codeheadsystems.pkauth.admin.AdminResponseMapper$AdminResponse", + // start*-ceremony result sums — the controller serializes the Started variant's wrapped + // response (already introspected) or maps RateLimited to the rate_limited body; the + // result records themselves never cross the serialization boundary. + "com.codeheadsystems.pkauth.api.StartRegistrationResult$Started", + "com.codeheadsystems.pkauth.api.StartRegistrationResult$RateLimited", + "com.codeheadsystems.pkauth.api.StartAuthenticationResult$Started", + "com.codeheadsystems.pkauth.api.StartAuthenticationResult$RateLimited"); @Test void everyWireRecordIsIntrospected() throws Exception { diff --git a/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepository.java b/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepository.java index 477825b..7cef66e 100644 --- a/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepository.java +++ b/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepository.java @@ -207,11 +207,11 @@ public boolean rotateAtomically( /** * True only when the {@code rotateAtomically} transaction was cancelled because the parent's - * freshness condition failed — the legitimate replay/race signal. The parent's conditional - * {@code UpdateItem} is the first action added to the transaction, so its reason is at index 0; - * a {@code ConditionalCheckFailed} code there means the parent was already used, revoked, or - * expired. Any other cancellation reason (throughput, transaction conflict, validation) is - * transient and returns false here so the caller rethrows rather than scorching the family. + * freshness condition failed — the legitimate replay/race signal. The parent's conditional {@code + * UpdateItem} is the first action added to the transaction, so its reason is at index 0; a {@code + * ConditionalCheckFailed} code there means the parent was already used, revoked, or expired. Any + * other cancellation reason (throughput, transaction conflict, validation) is transient and + * returns false here so the caller rethrows rather than scorching the family. */ private static boolean isParentFreshnessFailure(TransactionCanceledException cancelled) { List reasons = cancelled.cancellationReasons(); @@ -398,7 +398,8 @@ public int deleteExpiredBefore(Instant cutoff) { // begins_with filter keeps non-RT# rows of the shared table off the wire; note a scan // still consumes read capacity proportional to table size, so operators should prefer // native TTL and treat this as a test/maintenance path. - table.scan(ScanEnhancedRequest.builder().filterExpression(primaryItemsOnly()).build()) + table + .scan(ScanEnhancedRequest.builder().filterExpression(primaryItemsOnly()).build()) .items() .stream() .filter(item -> item.getPk() != null && item.getPk().startsWith(PRIMARY_PK_PREFIX)) diff --git a/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/web/PkAuthCeremonyController.java b/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/web/PkAuthCeremonyController.java index 26eaea4..2e13dae 100644 --- a/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/web/PkAuthCeremonyController.java +++ b/pk-auth-spring-boot-starter/src/main/java/com/codeheadsystems/pkauth/spring/web/PkAuthCeremonyController.java @@ -5,17 +5,15 @@ import com.codeheadsystems.pkauth.api.FinishAuthenticationRequest; import com.codeheadsystems.pkauth.api.FinishRegistrationRequest; import com.codeheadsystems.pkauth.api.StartAuthenticationRequest; -import com.codeheadsystems.pkauth.api.StartAuthenticationResponse; +import com.codeheadsystems.pkauth.api.StartAuthenticationResult; import com.codeheadsystems.pkauth.api.StartRegistrationRequest; -import com.codeheadsystems.pkauth.api.StartRegistrationResponse; +import com.codeheadsystems.pkauth.api.StartRegistrationResult; import com.codeheadsystems.pkauth.jwt.CeremonyOrchestrator; -import com.codeheadsystems.pkauth.spi.CeremonyRateLimitedException; import jakarta.servlet.http.HttpServletRequest; import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.ResponseEntity; -import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -39,10 +37,16 @@ public PkAuthCeremonyController(CeremonyOrchestrator orchestrator) { } @PostMapping("/registration/start") - public StartRegistrationResponse startRegistration( + public ResponseEntity startRegistration( @RequestBody StartRegistrationRequest req, HttpServletRequest httpRequest) { LOG.debug("auth.registration.start username={}", req.username()); - return orchestrator.startRegistration(req, clientIp(httpRequest)); + return switch (orchestrator.startRegistration(req, clientIp(httpRequest))) { + case StartRegistrationResult.Started started -> ResponseEntity.ok(started.response()); + case StartRegistrationResult.RateLimited rl -> { + LOG.info("auth.registration.start rate-limited bucket={}", rl.bucket()); + yield toResponseEntity(orchestrator.rateLimited()); + } + }; } @PostMapping("/registration/finish") @@ -52,10 +56,16 @@ public ResponseEntity finishRegistration( } @PostMapping("/authentication/start") - public StartAuthenticationResponse startAuthentication( + public ResponseEntity startAuthentication( @RequestBody StartAuthenticationRequest req, HttpServletRequest httpRequest) { LOG.debug("auth.authentication.start username={}", req.username()); - return orchestrator.startAuthentication(req, clientIp(httpRequest)); + return switch (orchestrator.startAuthentication(req, clientIp(httpRequest))) { + case StartAuthenticationResult.Started started -> ResponseEntity.ok(started.response()); + case StartAuthenticationResult.RateLimited rl -> { + LOG.info("auth.authentication.start rate-limited bucket={}", rl.bucket()); + yield toResponseEntity(orchestrator.rateLimited()); + } + }; } @PostMapping("/authentication/finish") @@ -64,18 +74,6 @@ public ResponseEntity finishAuthentication( return toResponseEntity(orchestrator.finishAuthentication(req, clientIp(httpRequest))); } - /** - * Maps a {@link CeremonyRateLimitedException} thrown by {@code startRegistration} / {@code - * startAuthentication} to a canonical {@code 429} response. - * - * @since 0.9.1 - */ - @ExceptionHandler(CeremonyRateLimitedException.class) - public ResponseEntity handleRateLimited(CeremonyRateLimitedException ex) { - LOG.info("auth.ceremony rate-limited bucket={}", ex.bucket()); - return toResponseEntity(orchestrator.rateLimited()); - } - private static ResponseEntity toResponseEntity(CeremonyResponse wire) { return ResponseEntity.status(wire.status()).body(wire.body()); } diff --git a/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/CeremonyScenarios.java b/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/CeremonyScenarios.java index cd2aa9c..26cfc72 100644 --- a/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/CeremonyScenarios.java +++ b/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/CeremonyScenarios.java @@ -91,7 +91,7 @@ public void usernamelessFlowSucceedsWithSingleCredential() { UserHandle handle = register(); StartAuthenticationResponse start = - service.startAuthentication(new StartAuthenticationRequest(null, null)); + service.startAuthentication(new StartAuthenticationRequest(null, null)).responseOrThrow(); assertThat(start.publicKey().allowCredentials()).isNotNull().isEmpty(); AuthenticationResponseJson resp = authenticator.createAssertionResponse(start, handle); @@ -103,7 +103,9 @@ public void usernamelessFlowSucceedsWithSingleCredential() { /** Registers a single credential and returns the issued user handle. */ public UserHandle register() { StartRegistrationResponse start = - service.startRegistration(new StartRegistrationRequest(USERNAME, DISPLAY_NAME, null, null)); + service + .startRegistration(new StartRegistrationRequest(USERNAME, DISPLAY_NAME, null, null)) + .responseOrThrow(); RegistrationResponseJson resp = authenticator.createRegistrationResponse(start); RegistrationResult result = service.finishRegistration( @@ -123,7 +125,9 @@ public void tamperedAssertionSignatureIsRejected() { long before = singleCredentialFor(handle).orElseThrow().signCount(); StartAuthenticationResponse start = - service.startAuthentication(new StartAuthenticationRequest(USERNAME, null)); + service + .startAuthentication(new StartAuthenticationRequest(USERNAME, null)) + .responseOrThrow(); AuthenticationResponseJson resp = authenticator.createAssertionResponse(start, handle); AssertionResult result = service.finishAuthentication( @@ -140,7 +144,9 @@ public void tamperedAssertionSignatureIsRejected() { */ public void tamperedRegistrationPayloadIsRejected() { StartRegistrationResponse start = - service.startRegistration(new StartRegistrationRequest(USERNAME, DISPLAY_NAME, null, null)); + service + .startRegistration(new StartRegistrationRequest(USERNAME, DISPLAY_NAME, null, null)) + .responseOrThrow(); RegistrationResponseJson resp = authenticator.createRegistrationResponse(start); RegistrationResult result = service.finishRegistration( @@ -162,7 +168,9 @@ public void replayedAssertionChallengeIsRejected() { UserHandle handle = register(); StartAuthenticationResponse start = - service.startAuthentication(new StartAuthenticationRequest(USERNAME, null)); + service + .startAuthentication(new StartAuthenticationRequest(USERNAME, null)) + .responseOrThrow(); AuthenticationResponseJson resp = authenticator.createAssertionResponse(start, handle); AssertionResult first = @@ -210,7 +218,9 @@ private static RegistrationResponseJson tamperAttestation(RegistrationResponseJs /** Runs a single assertion for the registered username. */ public AssertionResult assertOnce(UserHandle handle) { StartAuthenticationResponse start = - service.startAuthentication(new StartAuthenticationRequest(USERNAME, null)); + service + .startAuthentication(new StartAuthenticationRequest(USERNAME, null)) + .responseOrThrow(); AuthenticationResponseJson resp = authenticator.createAssertionResponse(start, handle); return service.finishAuthentication(new FinishAuthenticationRequest(start.challengeId(), resp)); } From 4972ab5efd1207058070032c5904b53df33de67b Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:32:16 -0700 Subject: [PATCH 10/12] test(persistence): cover concurrent consume-once for OTP and backup codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The atomic single-use race was proven only for refresh tokens; OTP and backup-code consume were tested single-threaded, so a regression from an atomic conditional UPDATE to a read-modify-write in either backend would let two callers redeem the same code (OTP / backup-code reuse) undetected. Add OtpRepositoryScenarios and BackupCodeRepositoryScenarios to the testkit — each fires 8 threads at consume() on one freshly-saved row and asserts exactly one winner — mirroring RefreshTokenScenarios. Run both against real Postgres (JDBI) and DynamoDB Local in the AltFlows integration suites. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../DynamoDbAltFlowsIntegrationTest.java | 12 +++ .../jdbi/JdbiAltFlowsIntegrationTest.java | 12 +++ .../BackupCodeRepositoryScenarios.java | 83 ++++++++++++++++++ .../testkit/OtpRepositoryScenarios.java | 86 +++++++++++++++++++ 4 files changed, 193 insertions(+) create mode 100644 pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/BackupCodeRepositoryScenarios.java create mode 100644 pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/OtpRepositoryScenarios.java diff --git a/pk-auth-persistence-dynamodb/src/test/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbAltFlowsIntegrationTest.java b/pk-auth-persistence-dynamodb/src/test/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbAltFlowsIntegrationTest.java index 256b04a..5a1a4c3 100644 --- a/pk-auth-persistence-dynamodb/src/test/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbAltFlowsIntegrationTest.java +++ b/pk-auth-persistence-dynamodb/src/test/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbAltFlowsIntegrationTest.java @@ -6,6 +6,8 @@ import com.codeheadsystems.pkauth.api.UserHandle; import com.codeheadsystems.pkauth.spi.BackupCodeRepository; import com.codeheadsystems.pkauth.spi.OtpRepository; +import com.codeheadsystems.pkauth.testkit.BackupCodeRepositoryScenarios; +import com.codeheadsystems.pkauth.testkit.OtpRepositoryScenarios; import java.time.Instant; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; @@ -126,4 +128,14 @@ void otpRoundTripAndCountSince() { assertThat(otp.findLatestActive(user, "+15551234567")) .hasValueSatisfying(o -> assertThat(o.otpId()).isEqualTo("o1")); } + + @Test + void concurrentOtpConsumeYieldsExactlyOneWinner() throws Exception { + new OtpRepositoryScenarios(otp).concurrentConsumeYieldsExactlyOneWinner(); + } + + @Test + void concurrentBackupCodeConsumeYieldsExactlyOneWinner() throws Exception { + new BackupCodeRepositoryScenarios(backupCodes).concurrentConsumeYieldsExactlyOneWinner(); + } } diff --git a/pk-auth-persistence-jdbi/src/test/java/com/codeheadsystems/pkauth/persistence/jdbi/JdbiAltFlowsIntegrationTest.java b/pk-auth-persistence-jdbi/src/test/java/com/codeheadsystems/pkauth/persistence/jdbi/JdbiAltFlowsIntegrationTest.java index 1dd9bb0..0e08eda 100644 --- a/pk-auth-persistence-jdbi/src/test/java/com/codeheadsystems/pkauth/persistence/jdbi/JdbiAltFlowsIntegrationTest.java +++ b/pk-auth-persistence-jdbi/src/test/java/com/codeheadsystems/pkauth/persistence/jdbi/JdbiAltFlowsIntegrationTest.java @@ -6,6 +6,8 @@ import com.codeheadsystems.pkauth.api.UserHandle; import com.codeheadsystems.pkauth.spi.BackupCodeRepository; import com.codeheadsystems.pkauth.spi.OtpRepository; +import com.codeheadsystems.pkauth.testkit.BackupCodeRepositoryScenarios; +import com.codeheadsystems.pkauth.testkit.OtpRepositoryScenarios; import java.time.Instant; import org.jdbi.v3.core.Jdbi; import org.junit.jupiter.api.BeforeEach; @@ -171,4 +173,14 @@ void otpRoundTripAndCountSince() { var noActive = otp.findLatestActive(user, "+15551234567"); assertThat(noActive).hasValueSatisfying(o -> assertThat(o.otpId()).isEqualTo("o1")); } + + @Test + void concurrentOtpConsumeYieldsExactlyOneWinner() throws Exception { + new OtpRepositoryScenarios(otp).concurrentConsumeYieldsExactlyOneWinner(); + } + + @Test + void concurrentBackupCodeConsumeYieldsExactlyOneWinner() throws Exception { + new BackupCodeRepositoryScenarios(backupCodes).concurrentConsumeYieldsExactlyOneWinner(); + } } diff --git a/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/BackupCodeRepositoryScenarios.java b/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/BackupCodeRepositoryScenarios.java new file mode 100644 index 0000000..ed3b606 --- /dev/null +++ b/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/BackupCodeRepositoryScenarios.java @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: MIT +package com.codeheadsystems.pkauth.testkit; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.codeheadsystems.pkauth.api.UserHandle; +import com.codeheadsystems.pkauth.spi.BackupCodeRepository; +import com.codeheadsystems.pkauth.spi.BackupCodeRepository.StoredBackupCode; +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Backend-agnostic acceptance scenarios for {@link BackupCodeRepository}, focused on the atomic + * single-use ({@code consume}) contract. Run against the in-memory testkit repo and every real + * backend (JDBI, DynamoDB) so the double-spend guarantee — a recovery code can be redeemed at most + * once — cannot regress in one implementation while passing in another. Mirrors {@link + * RefreshTokenScenarios}'s concurrent rotation race for the refresh-token path. + * + * @since 1.3.1 + */ +public final class BackupCodeRepositoryScenarios { + + private final BackupCodeRepository repository; + + public BackupCodeRepositoryScenarios(BackupCodeRepository repository) { + this.repository = repository; + } + + /** + * The consume-once race: eight threads call {@link BackupCodeRepository#consume} on the same + * freshly-saved code. Exactly one must win (return {@code true}); the rest must observe {@code + * false}, and a follow-up consume must also be {@code false}. A regression from an atomic + * conditional update to a read-modify-write would let two threads both redeem the same recovery + * code. + * + * @throws Exception if a worker thread is interrupted. + * @since 1.3.1 + */ + public void concurrentConsumeYieldsExactlyOneWinner() throws Exception { + UserHandle user = UserHandle.random(); + Instant now = Instant.parse("2026-05-14T12:00:00Z"); + String codeId = "tck-concurrent-backup-code"; + repository.save(new StoredBackupCode(codeId, user, "hash", false, now, null)); + + int threads = 8; + CountDownLatch ready = new CountDownLatch(threads); + CountDownLatch fire = new CountDownLatch(1); + AtomicInteger winners = new AtomicInteger(); + try (ExecutorService pool = Executors.newFixedThreadPool(threads)) { + List> futures = new ArrayList<>(); + for (int i = 0; i < threads; i++) { + futures.add( + pool.submit( + () -> { + ready.countDown(); + try { + fire.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(e); + } + if (repository.consume(user, codeId, now)) { + winners.incrementAndGet(); + } + })); + } + assertThat(ready.await(5, TimeUnit.SECONDS)).isTrue(); + fire.countDown(); + for (Future f : futures) { + f.get(10, TimeUnit.SECONDS); + } + assertThat(winners.get()).as("exactly one thread consumes the backup code").isEqualTo(1); + assertThat(repository.consume(user, codeId, now)).as("nothing left after the race").isFalse(); + } + } +} diff --git a/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/OtpRepositoryScenarios.java b/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/OtpRepositoryScenarios.java new file mode 100644 index 0000000..4bcf696 --- /dev/null +++ b/pk-auth-testkit/src/main/java/com/codeheadsystems/pkauth/testkit/OtpRepositoryScenarios.java @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: MIT +package com.codeheadsystems.pkauth.testkit; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.codeheadsystems.pkauth.api.UserHandle; +import com.codeheadsystems.pkauth.spi.OtpRepository; +import com.codeheadsystems.pkauth.spi.OtpRepository.StoredOtp; +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Backend-agnostic acceptance scenarios for {@link OtpRepository}, focused on the atomic single-use + * ({@code consume}) contract. Run against the in-memory testkit repo and every real backend (JDBI, + * DynamoDB) so the double-spend guarantee — an OTP can be verified at most once — cannot regress in + * one implementation while passing in another. Mirrors {@link RefreshTokenScenarios}'s concurrent + * rotation race for the refresh-token path. + * + * @since 1.3.1 + */ +public final class OtpRepositoryScenarios { + + private static final String PHONE = "+15551230000"; + + private final OtpRepository repository; + + public OtpRepositoryScenarios(OtpRepository repository) { + this.repository = repository; + } + + /** + * The consume-once race: eight threads call {@link OtpRepository#consume} on the same + * freshly-saved OTP. Exactly one must win (return {@code true}); the rest must observe {@code + * false}, and a follow-up consume must also be {@code false}. A regression from an atomic + * conditional update to a read-modify-write would let two threads both "consume" the same code, + * enabling OTP reuse. + * + * @throws Exception if a worker thread is interrupted. + * @since 1.3.1 + */ + public void concurrentConsumeYieldsExactlyOneWinner() throws Exception { + UserHandle user = UserHandle.random(); + Instant now = Instant.parse("2026-05-14T12:00:00Z"); + String otpId = "tck-concurrent-otp"; + repository.save( + new StoredOtp(otpId, user, PHONE, "hash", 0, 5, false, now, now.plusSeconds(300))); + + int threads = 8; + CountDownLatch ready = new CountDownLatch(threads); + CountDownLatch fire = new CountDownLatch(1); + AtomicInteger winners = new AtomicInteger(); + try (ExecutorService pool = Executors.newFixedThreadPool(threads)) { + List> futures = new ArrayList<>(); + for (int i = 0; i < threads; i++) { + futures.add( + pool.submit( + () -> { + ready.countDown(); + try { + fire.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(e); + } + if (repository.consume(user, otpId)) { + winners.incrementAndGet(); + } + })); + } + assertThat(ready.await(5, TimeUnit.SECONDS)).isTrue(); + fire.countDown(); + for (Future f : futures) { + f.get(10, TimeUnit.SECONDS); + } + assertThat(winners.get()).as("exactly one thread consumes the OTP").isEqualTo(1); + assertThat(repository.consume(user, otpId)).as("nothing left after the race").isFalse(); + } + } +} From 7a52556a9330862867ad199924676a72da052b0a Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:35:07 -0700 Subject: [PATCH 11/12] docs(spi): backfill contract Javadoc and @since on required and optional SPIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The required CredentialRepository CRUD methods (save/findByCredentialId/ findByUserHandle/updateSignCount) carried no Javadoc — document return-on-miss, the DuplicateCredentialException-on-duplicate contract, and the last-writer-wins / no-CAS semantics of updateSignCount so the SPI is implementable without reading source. Add the missing method-level @since and brief contracts to OriginValidator.isAllowed, AttestationTrustPolicy.evaluate, ClockProvider.now, RevocationCheck (class + isRevoked), and the JwtVerificationResult sealed interface. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../pkauth/spi/AttestationTrustPolicy.java | 7 ++++ .../pkauth/spi/ClockProvider.java | 7 ++++ .../pkauth/spi/CredentialRepository.java | 35 ++++++++++++++++++- .../pkauth/spi/OriginValidator.java | 9 +++++ .../pkauth/jwt/JwtVerificationResult.java | 6 +++- .../pkauth/jwt/RevocationCheck.java | 3 ++ 6 files changed, 65 insertions(+), 2 deletions(-) diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/AttestationTrustPolicy.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/AttestationTrustPolicy.java index a4ad804..136f308 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/AttestationTrustPolicy.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/AttestationTrustPolicy.java @@ -25,6 +25,13 @@ */ public interface AttestationTrustPolicy { + /** + * Evaluates the attestation presented during registration and decides whether to trust it. + * + * @param data the parsed attestation material for the registering credential. + * @return a {@link Decision} — trusted, or rejected with a reason. + * @since 0.9.0 + */ Decision evaluate(AttestationData data); /** Policy that accepts any attestation. */ diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/ClockProvider.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/ClockProvider.java index abdefe2..a1629f1 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/ClockProvider.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/ClockProvider.java @@ -13,6 +13,13 @@ @FunctionalInterface public interface ClockProvider { + /** + * Returns the current instant. Injecting this (rather than calling {@link Instant#now()} + * directly) lets tests drive ceremony/expiry timing deterministically. + * + * @return the current instant per this provider's clock. + * @since 0.9.0 + */ Instant now(); /** Default provider backed by the system UTC clock. */ diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CredentialRepository.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CredentialRepository.java index be8e116..ef2d23c 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CredentialRepository.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CredentialRepository.java @@ -16,13 +16,46 @@ */ public interface CredentialRepository { - /** Inserts a new credential record. Implementations must reject duplicates on credentialId. */ + /** + * Inserts a new credential record. Implementations MUST reject a duplicate {@code credentialId} + * by throwing {@link DuplicateCredentialException} rather than overwriting the existing row — a + * duplicate at registration-finish is a replay/clobber attempt, not an update. + * + * @param record the credential to persist. + * @throws DuplicateCredentialException if a credential with the same {@code credentialId} exists. + * @since 0.9.0 + */ void save(CredentialRecord record); + /** + * Looks up a credential by its (globally unique) {@code credentialId}. + * + * @param credentialId the credential to find. + * @return the credential, or {@link Optional#empty()} if no row matches. + * @since 0.9.0 + */ Optional findByCredentialId(CredentialId credentialId); + /** + * Lists every credential owned by the supplied user, in unspecified order. + * + * @param userHandle the owner. + * @return the user's credentials, or an empty list if the user has none (never {@code null}). + * @since 0.9.0 + */ List findByUserHandle(UserHandle userHandle); + /** + * Updates the stored signature counter and last-used timestamp after a successful assertion. This + * is a last-writer-wins overwrite of the two fields (no compare-and-set); the WebAuthn4J counter + * verification that authorizes the new value runs in the service before this call. A missing row + * is a silent no-op. + * + * @param credentialId the credential that just authenticated. + * @param newCount the new signature counter to store. + * @param lastUsedAt when the assertion occurred. + * @since 0.9.0 + */ void updateSignCount(CredentialId credentialId, long newCount, Instant lastUsedAt); /** diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/OriginValidator.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/OriginValidator.java index 9734fd8..e590ef7 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/OriginValidator.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/OriginValidator.java @@ -14,6 +14,15 @@ @FunctionalInterface public interface OriginValidator { + /** + * Decides whether a client-reported WebAuthn origin is acceptable for this relying party. + * + * @param origin the origin string from the authenticator's client data (e.g. {@code + * https://example.com}). + * @return {@code true} if the origin is allowed; {@code false} (including for a {@code null} + * origin) rejects the ceremony. + * @since 0.9.0 + */ boolean isAllowed(String origin); /** Strict allow-list validator backed by the configured set of origins. */ diff --git a/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/JwtVerificationResult.java b/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/JwtVerificationResult.java index 31ba99f..7386fb6 100644 --- a/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/JwtVerificationResult.java +++ b/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/JwtVerificationResult.java @@ -4,7 +4,11 @@ import java.time.Instant; import java.util.Objects; -/** Closed sum of outcomes from {@link PkAuthJwtValidator#validate(String)}. */ +/** + * Closed sum of outcomes from {@link PkAuthJwtValidator#validate(String)}. + * + * @since 1.1.0 + */ public sealed interface JwtVerificationResult { /** Token signature verified and all standard claims pass. */ diff --git a/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/RevocationCheck.java b/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/RevocationCheck.java index 76161e9..bc84794 100644 --- a/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/RevocationCheck.java +++ b/pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/RevocationCheck.java @@ -9,6 +9,8 @@ *

    Register a custom implementation by passing it to the {@link * PkAuthJwtValidator#PkAuthJwtValidator(JwtConfig, JwtKeyset, * com.codeheadsystems.pkauth.spi.ClockProvider, RevocationCheck)} constructor. + * + * @since 1.1.0 */ @FunctionalInterface public interface RevocationCheck { @@ -29,6 +31,7 @@ public interface RevocationCheck { * @param jti the JWT ID claim ({@code jti}), may be {@code null} if absent from the token * @param subject the subject claim ({@code sub}), always non-null when called from the validator * @return {@code true} to reject the token; {@code false} to allow it + * @since 1.1.0 */ boolean isRevoked(String jti, String subject); From 67f10bd41990884e5667d9025889df6310711842 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Fri, 12 Jun 2026 07:43:33 -0700 Subject: [PATCH 12/12] ci: restore the SonarQube scan for the browser SDK (JavaScript only) The earlier "drop residual SonarQube scan" change over-reached: that step was the TypeScript/browser-SDK scan (projectBaseDir: clients/passkeys-browser), not a JVM scan. SonarQube stays removed for the Java modules (ADR 0018 -> native JaCoCo gates) but is wanted for the JS SDK. Restore the step; sonar-project.properties was never removed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c236d1c..3b1b015 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,6 +75,17 @@ jobs: run: npx vitest run --coverage working-directory: clients/passkeys-browser + - name: SonarQube scan + # JavaScript/browser-SDK static analysis only. SonarQube was dropped for the JVM modules + # (ADR 0018, native JaCoCo gates) but retained here for the TypeScript SDK. + # SONAR_TOKEN is unavailable to PRs from forks; skip there to avoid a hard failure. + if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository + uses: sonarsource/sonarqube-scan-action@713881670b6b3676cda39549040e2d88c70d582e # v8.2.0 + env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + with: + projectBaseDir: clients/passkeys-browser + - name: Run Stryker mutation tests run: npm exec --no-install -- stryker run working-directory: clients/passkeys-browser