diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1b67fea..3b1b015 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 @@ -65,6 +76,8 @@ jobs: 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 diff --git a/CLAUDE.md b/CLAUDE.md index 95d6cd1..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. @@ -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..b870cf3 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/`. | @@ -140,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 `) @@ -207,23 +215,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 +311,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 +441,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/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/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" 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 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-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/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/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/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/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/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-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/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/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/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-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..d08919e --- /dev/null +++ b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/PkAuthPersistenceResponseTest.java @@ -0,0 +1,27 @@ +// 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/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-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-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-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/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); 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-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-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/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-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-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-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..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 @@ -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(); @@ -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<>(); 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..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 @@ -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,14 @@ 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 +471,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 +488,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 +498,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 +515,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(); + } } 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-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()); } /** 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-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)); } } 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/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)); } 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(); + } + } +}