diff --git a/README.md b/README.md index f843ce0..baebd08 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ All modules share the same version and `com.codeheadsystems` group id. | `pk-auth-otp` | [![Maven Central: pk-auth-otp](https://img.shields.io/maven-central/v/com.codeheadsystems/pk-auth-otp?label=pk-auth-otp)](https://central.sonatype.com/artifact/com.codeheadsystems/pk-auth-otp) | 6-digit SMS OTP codes for phone verification. | | `pk-auth-refresh-tokens` | [![Maven Central: pk-auth-refresh-tokens](https://img.shields.io/maven-central/v/com.codeheadsystems/pk-auth-refresh-tokens?label=pk-auth-refresh-tokens)](https://central.sonatype.com/artifact/com.codeheadsystems/pk-auth-refresh-tokens) | Rotating refresh tokens with family-based replay defense. *(1.1.0)* | | `pk-auth-persistence-jdbi` | [![Maven Central: pk-auth-persistence-jdbi](https://img.shields.io/maven-central/v/com.codeheadsystems/pk-auth-persistence-jdbi?label=pk-auth-persistence-jdbi)](https://central.sonatype.com/artifact/com.codeheadsystems/pk-auth-persistence-jdbi) | JDBI 3 + Flyway + Postgres SPI implementations. | -| `pk-auth-persistence-dynamodb` | [![Maven Central: pk-auth-persistence-dynamodb](https://img.shields.io/maven-central/v/com.codeheadsystems/pk-auth-persistence-dynamodb?label=pk-auth-persistence-dynamodb)](https://central.sonatype.com/artifact/com.codeheadsystems/pk-auth-persistence-dynamodb) | AWS SDK v2 DynamoDB Enhanced SPI implementations (single-table). | +| `pk-auth-persistence-dynamodb` | [![Maven Central: pk-auth-persistence-dynamodb](https://img.shields.io/maven-central/v/com.codeheadsystems/pk-auth-persistence-dynamodb?label=pk-auth-persistence-dynamodb)](https://central.sonatype.com/artifact/com.codeheadsystems/pk-auth-persistence-dynamodb) | AWS SDK v2 DynamoDB Enhanced SPI implementations (single-table core + separate users table). | | `pk-auth-testkit` | [![Maven Central: pk-auth-testkit](https://img.shields.io/maven-central/v/com.codeheadsystems/pk-auth-testkit?label=pk-auth-testkit)](https://central.sonatype.com/artifact/com.codeheadsystems/pk-auth-testkit) | `FakeAuthenticator`, in-memory SPIs, and fixtures for tests. | | `pk-auth-spring-boot-starter` | [![Maven Central: pk-auth-spring-boot-starter](https://img.shields.io/maven-central/v/com.codeheadsystems/pk-auth-spring-boot-starter?label=pk-auth-spring-boot-starter)](https://central.sonatype.com/artifact/com.codeheadsystems/pk-auth-spring-boot-starter) | Spring Boot 4 / Spring Security 7 auto-configure. | | `pk-auth-dropwizard` | [![Maven Central: pk-auth-dropwizard](https://img.shields.io/maven-central/v/com.codeheadsystems/pk-auth-dropwizard?label=pk-auth-dropwizard)](https://central.sonatype.com/artifact/com.codeheadsystems/pk-auth-dropwizard) | Dropwizard 5 `ConfiguredBundle` + Dagger 2 wiring. | diff --git a/docs/adr/0003-jdbi-over-jpa.md b/docs/adr/0003-jdbi-over-jpa.md index d5f5d9c..bc2620e 100644 --- a/docs/adr/0003-jdbi-over-jpa.md +++ b/docs/adr/0003-jdbi-over-jpa.md @@ -24,7 +24,7 @@ The build brief is explicit (§3): "JDBI 3 + Flyway against PostgreSQL. **No Hib ## Consequences - **Positive — predictability.** Every query is visible. No magic SQL emitted by an ORM. The audit-friendliest stance for an auth-layer module. -- **Positive — minimal runtime surface.** JDBI 3 + the Postgres JDBC driver are the only runtime dependencies pk-auth-persistence-jdbi adds (Flyway is implementation-only and used at startup). +- **Positive — minimal runtime surface.** pk-auth-persistence-jdbi adds only JDBI 3, the Postgres JDBC driver, HikariCP (connection pool), and slf4j-api as runtime dependencies; Flyway is implementation-scoped and used at startup. - **Positive — no JPA-style entity-manager threading caveats.** Repository methods are stateless; the brief's "no reflection in hot paths" stance (§11) holds. - **Negative — no automatic schema generation.** Every change to a `CredentialRecord` / `ChallengeRecord` field needs a new Flyway migration. We accept this — auth-data migrations should be deliberate. - **Negative — no entity-graph fetch.** pk-auth's data is shallow (no relationships across repositories), so this is fine for now. If a future feature needs a join, we write it. diff --git a/docs/adr/0004-dagger-for-dropwizard.md b/docs/adr/0004-dagger-for-dropwizard.md index 2c697d2..b63a125 100644 --- a/docs/adr/0004-dagger-for-dropwizard.md +++ b/docs/adr/0004-dagger-for-dropwizard.md @@ -34,20 +34,24 @@ This ADR records the rationale. Use Dagger 2 (`com.google.dagger:dagger:2.59.x` with `dagger-compiler` on the `annotationProcessor` configuration) to wire the Dropwizard adapter's object graph. The bundle's public API does not leak Dagger types: host applications hand the bundle a `PersistenceBindings` -record and an optional `AdminService`, and the bundle internally builds a `PkAuthComponent` whose -provision methods Jersey resources consume. +object and an optional `AdminService`, and the bundle internally builds a `PkAuthComponent` (or the +larger `PkAuthFullComponent` when the alt-flow modules are auto-wired) whose provision methods +Jersey resources consume. The Dagger module structure is intentionally small: - `PkAuthModule` — provides `PasskeyAuthenticationService`, `JwtConfig` / `JwtKeyset` / - `PkAuthJwtIssuer` / `PkAuthJwtValidator`, `PasskeyAuthenticator`, and the - `PasskeyCeremonyResource`. Bound to the runtime `PkAuthConfig` block via constructor injection + `PkAuthJwtIssuer` / `PkAuthJwtValidator`, `PkAuthDropwizardAuthenticator`, and the + `PkAuthCeremonyResource`. Bound to the runtime `PkAuthConfig` block via constructor injection on the module itself. -- `PkAuthComponent` — the `@Component` whose four provision methods expose what the bundle hands - to Jersey (`ceremonyResource()`, `passkeyAuthenticator()`, `jwtIssuer()`, `jwtValidator()`). -- The optional admin path (when `pk-auth-admin-api` is on the classpath) is not Dagger-wired — - the bundle instantiates `AdminResource` directly from the host-supplied `AdminService` so the - admin module's compile-time dependency stays optional. +- `PkAuthComponent` — the `@Component` whose provision methods expose what the bundle hands to + Jersey: `ceremonyResource()`, `passkeyAuthenticator()`, `jwtIssuer()`, `jwtValidator()`, + `userDeletionService()`, and `refreshHandler()`. +- The optional admin path (when `pk-auth-admin-api` is on the classpath): when the alt-flow modules + are auto-wired the bundle builds `PkAuthFullComponent` (`PkAuthModule` + `AltFlowsModule`), which + adds `adminResource()`; when a host supplies its own `AdminService` the bundle instantiates + `PkAuthAdminResource` directly. Either way the admin module's compile-time dependency stays + optional. Generated classes live in `com.codeheadsystems.pkauth.dropwizard.dagger` and are excluded from the JaCoCo coverage report (they're auto-generated boilerplate that should not skew the diff --git a/docs/adr/0005-stateless-jwt-default.md b/docs/adr/0005-stateless-jwt-default.md index 6069481..8072cbd 100644 --- a/docs/adr/0005-stateless-jwt-default.md +++ b/docs/adr/0005-stateless-jwt-default.md @@ -61,9 +61,13 @@ every shipped demo, however, is stateless JWT. - **Negative — revocation is harder.** A short TTL (default 1 hour) bounds the blast radius but doesn't eliminate it. A future denylist SPI on the `ChallengeStore` (or a sibling store) can plug this if we need true logout; the brief calls it out as a non-goal for v0.x. + *(Shipped in 1.1.0 as the opt-in `AccessTokenStore` SPI — stateful, revocable-before-`exp` + access tokens; the default stays the stateless no-op. See ADR 0015.)* - **Negative — refresh-token machinery is not provided.** Hosts that need long-lived sessions must either accept re-authentication on TTL expiry or layer their own refresh flow on top. pk-auth's scope is the credential layer, not session management. + *(Shipped in 1.1.0 as the `pk-auth-refresh-tokens` module — rotating refresh tokens with + family-based replay defense. See ADR 0013.)* - **Neutral — key rotation is a separate concern.** `JwtKeyset` already supports a current signing key plus a list of retired verification keys. Rotating the signing key invalidates no outstanding tokens; rotating it AND removing the retired key invalidates everything @@ -71,8 +75,14 @@ every shipped demo, however, is stateless JWT. ## Open follow-ups -- A token-revocation SPI is deferred. If a downstream consumer needs it, the natural shape - is a `RevokedTokenStore` with `revoke(jti, expiresAt)` and `isRevoked(jti)`. The Spring - filter would consult it after signature validation. Out of scope for v0.x. -- Refresh-token issuance is out of scope. Hosts that want it are expected to wrap - `PkAuthJwtIssuer` in their own service. +> **Resolved in 1.1.0.** Both deferred items below have since shipped. The stateless-JWT +> default in this ADR still stands — the new capabilities are opt-in and leave the default +> behavior unchanged. + +- ~~A token-revocation SPI is deferred.~~ Shipped as `AccessTokenStore` (ADR 0015) — an + allow-list (`record` / `exists` / `delete` / `deleteAllForUser` / `deleteExpiredBefore`) + rather than the originally-sketched `RevokedTokenStore` deny-list. The validator consults + it after signature validation; the default no-op store preserves stateless behavior. +- ~~Refresh-token issuance is out of scope.~~ Shipped as the `pk-auth-refresh-tokens` module + (ADR 0013): rotating refresh tokens with family-based replay defense, exposed via + `POST /auth/refresh`. diff --git a/docs/adr/0008-dynamodb-single-table-design.md b/docs/adr/0008-dynamodb-single-table-design.md index 4b15d29..b13d42a 100644 --- a/docs/adr/0008-dynamodb-single-table-design.md +++ b/docs/adr/0008-dynamodb-single-table-design.md @@ -63,7 +63,7 @@ The minor size overhead (~33%) is acceptable for pk-auth's data sizes — creden - **Positive — cheap user-scoped queries.** Listing a user's credentials, backup codes, or OTP codes is a single `Query` on the same partition. - **Positive — host-app independence.** A host app that already runs DynamoDB can point `DynamoDbUserLookup` at its own existing users table by reusing the `UserItem` schema (or implementing `UserLookup` against an entirely different schema). -- **Positive — schema simplicity.** Three attribute definitions for the core table (`pk`, `sk`, `gsi1pk`, `gsi1sk`); no migrations were required when Phase 6 item types were added. +- **Positive — schema simplicity.** Four attribute definitions for the core table (`pk`, `sk`, `gsi1pk`, `gsi1sk`); no migrations were required when Phase 6 item types were added. - **Negative — hot-partition risk on a single user with thousands of credentials.** Not a realistic scenario for consumer passkeys, but worth flagging. - **Negative — GSI projection.** `gsi1-credential-by-id` projects ALL attributes; that's twice the storage cost on credential rows. We accept it because the assertion path needs the full credential to verify, and avoiding a second `GetItem` per assertion is the right trade for the latency floor. diff --git a/docs/adr/0009-jackson-3-over-jackson-2.md b/docs/adr/0009-jackson-3-over-jackson-2.md index 9b3be19..0bd5198 100644 --- a/docs/adr/0009-jackson-3-over-jackson-2.md +++ b/docs/adr/0009-jackson-3-over-jackson-2.md @@ -23,7 +23,7 @@ The constraints that made (3) the right call: ## Decision -`pk-auth-core` standardizes on **Jackson 3** (`tools.jackson.databind 3.1.3`). The shared `ObjectMapper` factory uses the Jackson 3 `JsonMapper.builder()` flow, `ValueSerializer` / `ValueDeserializer`, and `changeDefaultPropertyInclusion(...)` for the NON_NULL output policy. We continue to use the classical `com.fasterxml.jackson.core:jackson-annotations 2.21` artifact for annotations. +`pk-auth-core` standardizes on **Jackson 3** (`tools.jackson.databind 3.1.4`). The shared `ObjectMapper` factory uses the Jackson 3 `JsonMapper.builder()` flow, `ValueSerializer` / `ValueDeserializer`, and `changeDefaultPropertyInclusion(...)` for the NON_NULL output policy. We continue to use the classical `com.fasterxml.jackson.core:jackson-annotations 2.22` artifact for annotations. Jackson 3's `tools.jackson.databind` bundles `java.time` and `Jdk8` datatype support directly into databind, so the separate `jackson-datatype-jsr310` and `jackson-datatype-jdk8` dependencies are no longer required. diff --git a/docs/adr/0010-dropwizard-track-latest.md b/docs/adr/0010-dropwizard-track-latest.md index 4de1f1d..222d62c 100644 --- a/docs/adr/0010-dropwizard-track-latest.md +++ b/docs/adr/0010-dropwizard-track-latest.md @@ -39,7 +39,7 @@ Dropwizard major. ## Decision The `pk-auth-dropwizard` adapter tracks the latest released Dropwizard. The -catalog pin is the current latest (5.0.1 at this time). The Dependabot +catalog pin is the current latest (5.0.2 at this time). The Dependabot `io.dropwizard:* version-update:semver-major` ignore rule has been removed — future majors will surface as PRs the same way every other dependency does. diff --git a/docs/adr/0012-micronaut-4.md b/docs/adr/0012-micronaut-4.md index d04f10f..b3683f6 100644 --- a/docs/adr/0012-micronaut-4.md +++ b/docs/adr/0012-micronaut-4.md @@ -9,7 +9,7 @@ Accepted. ## Context `pk-auth-micronaut` was written directly against Micronaut 4.x (the build -catalog currently pins `micronaut = "4.10.23"`). The decision to skip +catalog currently pins `micronaut = "4.10.25"`). The decision to skip Micronaut 3 entirely was made at module creation: Micronaut 3 reached end of maintenance during pk-auth's first development cycle, and adopters starting new projects in 2026 are expected on Micronaut 4. diff --git a/docs/adr/0015-stateful-access-tokens.md b/docs/adr/0015-stateful-access-tokens.md index 04c51b1..ab49e35 100644 --- a/docs/adr/0015-stateful-access-tokens.md +++ b/docs/adr/0015-stateful-access-tokens.md @@ -46,7 +46,7 @@ public interface AccessTokenStore { void record(String jti, UserHandle, String audience, Optional deviceId, Instant issuedAt, Instant expiresAt); boolean exists(String jti); - boolean delete(String jti); + boolean delete(UserHandle userHandle, String jti); int deleteAllForUser(UserHandle userHandle); int deleteExpiredBefore(Instant before); static AccessTokenStore noop(); @@ -86,7 +86,7 @@ Adapter wiring: - **Pro**: Server-side logout works end-to-end with no host-side code beyond binding the JDBI/Dynamo `AccessTokenStore` bean and calling - `store.delete(jti)` from the logout endpoint. + `store.delete(userHandle, jti)` from the logout endpoint. - **Pro**: User deletion (ADR 0016) becomes a one-call operation: `UserDeletionService.deleteUser(handle)` fans out to every listener including the access-token cleanup. diff --git a/docs/operator-guide.md b/docs/operator-guide.md index dd827a1..02867d7 100644 --- a/docs/operator-guide.md +++ b/docs/operator-guide.md @@ -11,8 +11,10 @@ adapters all consume the same core. A typical production deployment needs: - **JDK 21** (records, sealed types, virtual threads). Earlier JDKs will not compile. - **Postgres 16+** (when using `pk-auth-persistence-jdbi`) — Flyway migrations run at startup, no manual schema work. -- **DynamoDB** (when using `pk-auth-persistence-dynamodb`) — single table, schema - per item type. See ADR 0008 for the table layout. +- **DynamoDB** (when using `pk-auth-persistence-dynamodb`) — two tables: a + single-table `PkAuthCore` carrying every pk-auth auth item plus a separate + `PkAuthUsers` table for the host-app user records the `UserLookup` SPI reads. + See ADR 0008 for the table layout. - **At least one trusted dispatcher** for magic links + OTP if you enable those flows. The testkit's `LoggingEmailSender` / `LoggingSmsSender` log secrets to stdout; never use them in production. @@ -24,7 +26,7 @@ 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. | -| Argon2id pepper | n/a | Per-deployment pepper for OTP hashes only (backup codes use Argon2id without a pepper). Treat as a long-lived secret; rotating it invalidates every existing OTP hash. | +| 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. | Recommended: stash secrets in a KMS/Secrets Manager and inject as environment variables (`PKAUTH_JWT_SECRET`, `PKAUTH_OTP_PEPPER`). The adapters bind both. @@ -43,6 +45,10 @@ variables (`PKAUTH_JWT_SECRET`, `PKAUTH_OTP_PEPPER`). The adapters bind both. `V7__credentials_hard_delete.sql` drops the `revoked_at` / `revoked_reason` columns on `credentials` — credential delete is a hard delete, with the audit record captured as a structured log event (`pkauth.credential.deleted`). + `V8__create_access_tokens.sql` and `V9__create_refresh_tokens.sql` add the + 1.1.0 `access_tokens` and `refresh_tokens` tables; `V10__refresh_tokens_amr.sql` + adds the `amr` (RFC 8176 authentication-method-reference) column to + `refresh_tokens`. - Magic-link tokens are not persisted: the JWT is the credential, and the consumed-JTI store is in-memory by default (see `ConsumedJtiStore` SPI for a multi-replica override). @@ -51,14 +57,20 @@ variables (`PKAUTH_JWT_SECRET`, `PKAUTH_OTP_PEPPER`). The adapters bind both. ### DynamoDB -- Single physical table (see ADR 0008). Provision it before the app starts; the - adapter does not create it. -- TTL attribute `expiresAt` is honored on `Challenge` / `OneTimePasscode` / - `MagicLink` items — enable it on the table. -- 1.1.0 adds `access_tokens` and `refresh_tokens` items on the same table (ADR - 0015, 0013). Both set the DynamoDB-native `ttl` attribute to the row's - `expiresAt` epoch second so background pruning is automatic — TTL must be - enabled on the table for this to work. +- Two physical tables (see ADR 0008): `PkAuthCore` holds every pk-auth auth item + (credentials, challenges, backup codes, OTP codes, and the 1.1.0 token rows), + and `PkAuthUsers` holds the host-app user records the `UserLookup` SPI reads. + Provision both before the app starts; the adapter does not create them. +- The DynamoDB-native TTL attribute is `ttl` (epoch seconds) — enable TTL on the + `ttl` attribute of the `PkAuthCore` table. It is set on `Challenge` and + `OneTimePasscode` items so DynamoDB evicts them after expiry. (Magic-link tokens + are never persisted, in any backend.) +- 1.1.0 adds `access_tokens` and `refresh_tokens` items on the same `PkAuthCore` + table (ADR 0015, 0013), both pruned by the native `ttl` attribute. Access-token + rows set `ttl` to their `expiresAt` epoch second; refresh-token rows set it to + `expiresAt + cleanupRetention` (default 30 days) so used/revoked rows survive the + forensic-retention window before the background sweep removes them — matching the + JDBI cleanup semantics. TTL must be enabled on the table for this to work. - Capacity-mode: on-demand is recommended for steady reads but bursty registration; provisioned only makes sense once you have a stable signing/verification baseline. diff --git a/docs/transactional-semantics.md b/docs/transactional-semantics.md index 31eec2e..c222007 100644 --- a/docs/transactional-semantics.md +++ b/docs/transactional-semantics.md @@ -63,14 +63,17 @@ write, and a failure leaves the user in a recoverable (if inconvenient) state. ## Where pk-auth *does* require atomicity inside an SPI The general "no shared transaction across SPI calls" stance does not extend to -operations that pk-auth declares atomic inside a single SPI call. Two surfaces -require the implementation to commit a multi-step change atomically; the -parity-test suite enforces this on every shipped backend. +operations that pk-auth declares atomic inside a single SPI call. Three surfaces +require the implementation to commit a multi-step change atomically. The +parity-test suite exercises this on every shipped backend — directly under +concurrency for `takeOnce` and `rotateAtomically` (multi-thread / `CountDownLatch` +races), and via round-trip parity tests for the backup-code and OTP `consume` +paths. | SPI method | Atomicity contract | |---|---| | `ChallengeStore.takeOnce(challengeId)` | Read + delete (or read + mark consumed) commit as one step. A second concurrent caller for the same challenge must receive `Optional.empty()`. | -| `BackupCodeRepository.consume` / `OtpRepository.consume` | Hash compare + mark-used commit as one step; exactly one of N concurrent callers wins. | +| `BackupCodeRepository.consume` / `OtpRepository.consume` | The id-keyed mark-used commits as one atomic step, so exactly one of N concurrent callers wins. (The Argon2 / HMAC hash compare runs earlier in the service layer and is deliberately racy; the `consume` return value is the single source of truth for which caller won.) | | `RefreshTokenRepository.rotateAtomically` *(1.1.0)* | Mark the parent refresh row used AND insert the successor row in a single atomic operation — JDBI transaction, DynamoDB `TransactWriteItems`, or in-memory `ConcurrentHashMap.compute`. A non-atomic implementation has a window where a concurrent rotator's family-scorch can miss the freshly-inserted successor; the contract forbids this and the `concurrentRotationExactlyOneSucceedsFamilyRevoked` parity test (8 threads + `CountDownLatch`) enforces it against in-memory, real Postgres, and DynamoDB Local. See [ADR 0013](./adr/0013-refresh-tokens-family-rotation.md). | Cross-SPI atomicity is still **not** required; the `UserDeletionService` 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 4d55199..cca9ead 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 @@ -3,10 +3,12 @@ import com.codeheadsystems.pkauth.api.UserHandle; import com.codeheadsystems.pkauth.json.Base64Url; +import com.codeheadsystems.pkauth.refresh.RefreshTokenConfig; import com.codeheadsystems.pkauth.refresh.RefreshTokenRecord; import com.codeheadsystems.pkauth.refresh.RevokeReason; import com.codeheadsystems.pkauth.refresh.spi.RefreshTokenRepository; import com.codeheadsystems.pkauth.spi.PkAuthPersistenceException; +import java.time.Duration; import java.time.Instant; import java.util.HashMap; import java.util.LinkedHashMap; @@ -57,11 +59,28 @@ public final class DynamoDbRefreshTokenRepository implements RefreshTokenReposit private final DynamoDbEnhancedClient enhanced; private final DynamoDbTable table; + private final Duration cleanupRetention; + /** + * Uses the {@linkplain RefreshTokenConfig#DEFAULT_CLEANUP_RETENTION default 30-day} forensic + * retention window for the native-TTL prune timestamp. + */ public DynamoDbRefreshTokenRepository( DynamoDbEnhancedClient enhanced, PkAuthDynamoTables tables) { + this(enhanced, tables, RefreshTokenConfig.DEFAULT_CLEANUP_RETENTION); + } + + /** + * @param cleanupRetention how long past {@code expiresAt} the native {@code ttl} attribute keeps + * a row before DynamoDB prunes it — must match the {@link + * RefreshTokenConfig#cleanupRetention()} the host runs the service with, so the background + * sweep honors the same forensic window the JDBI backend does. + */ + public DynamoDbRefreshTokenRepository( + DynamoDbEnhancedClient enhanced, PkAuthDynamoTables tables, Duration cleanupRetention) { this.enhanced = Objects.requireNonNull(enhanced, "enhanced"); Objects.requireNonNull(tables, "tables"); + this.cleanupRetention = Objects.requireNonNull(cleanupRetention, "cleanupRetention"); this.table = enhanced.table(tables.core(), TableSchema.fromBean(RefreshTokenItem.class)); } @@ -108,11 +127,12 @@ public boolean rotateAtomically( // // The condition is the authoritative freshness check: the parent must exist // (attribute_exists(pk) — UpdateItem would otherwise upsert a brand-new row) and be - // unused, unrevoked, and unexpired. Expiry compares the numeric epoch-second `ttl` - // attribute (== expiresAt.epochSecond), NOT the ISO string: Instant.toString() is - // variable-precision (it drops the fractional-seconds field when zero), so a - // lexicographic ">" sorts "...:00Z" after "...:00.000001Z" and would treat an expired - // token as still fresh. + // unused, unrevoked, and unexpired. Expiry compares the numeric epoch-second + // `expiresAtEpoch` attribute (== expiresAt.epochSecond), NOT the ISO string: + // Instant.toString() is variable-precision (it drops the fractional-seconds field when + // zero), so a lexicographic ">" sorts "...:00Z" after "...:00.000001Z" and would treat an + // expired token as still fresh. The separate `ttl` attribute carries + // expiresAt + cleanupRetention for native pruning and must NOT be used for freshness. RefreshTokenItem parentMark = new RefreshTokenItem(); parentMark.setPk(PRIMARY_PK_PREFIX + parentRefreshId); parentMark.setSk(PRIMARY_PK_PREFIX + parentRefreshId); @@ -122,8 +142,8 @@ public boolean rotateAtomically( Expression.builder() .expression( "attribute_exists(pk) AND attribute_not_exists(usedAtIso)" - + " AND attribute_not_exists(revokedAtIso) AND #ttl > :nowEpoch") - .putExpressionName("#ttl", "ttl") + + " AND attribute_not_exists(revokedAtIso) AND #expEpoch > :nowEpoch") + .putExpressionName("#expEpoch", "expiresAtEpoch") .putExpressionValue( ":nowEpoch", AttributeValue.fromN(Long.toString(now.getEpochSecond()))) .build(); @@ -344,11 +364,14 @@ public int deleteExpiredBefore(Instant cutoff) { long cutoffEpoch = cutoff.getEpochSecond(); 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. + // retention predicate that mirrors the JDBI cleanup SQL (expires_at < cutoff). This uses + // `expiresAtEpoch`, NOT the retention-extended `ttl` attribute. table.scan().items().stream() .filter(item -> item.getPk() != null && item.getPk().startsWith(PRIMARY_PK_PREFIX)) .filter(item -> item.getPk().equals(item.getSk())) // primary only - .filter(item -> item.getTtl() != null && item.getTtl() < cutoffEpoch) + .filter( + item -> + item.getExpiresAtEpoch() != null && item.getExpiresAtEpoch() < cutoffEpoch) .filter( item -> (item.getUsedAtIso() != null @@ -416,7 +439,7 @@ private static String successor_userB64(RefreshTokenRecord r) { return Base64Url.encode(r.userHandle().value()); } - private static RefreshTokenItem toItem(RefreshTokenRecord r, String pk, String sk) { + private RefreshTokenItem toItem(RefreshTokenRecord r, String pk, String sk) { RefreshTokenItem item = new RefreshTokenItem(); item.setPk(pk); item.setSk(sk); @@ -433,7 +456,8 @@ private static RefreshTokenItem toItem(RefreshTokenRecord r, String pk, String s item.setRevokedAtIso(r.revokedAt().map(Instant::toString).orElse(null)); item.setRevokedReason(r.revokedReason().map(Enum::name).orElse(null)); item.setAmr(String.join(",", r.amr())); - item.setTtl(r.expiresAt().getEpochSecond()); + item.setExpiresAtEpoch(r.expiresAt().getEpochSecond()); + item.setTtl(r.expiresAt().plus(cleanupRetention).getEpochSecond()); return item; } diff --git a/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/RefreshTokenItem.java b/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/RefreshTokenItem.java index b3e06c5..f4979fc 100644 --- a/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/RefreshTokenItem.java +++ b/pk-auth-persistence-dynamodb/src/main/java/com/codeheadsystems/pkauth/persistence/dynamodb/RefreshTokenItem.java @@ -18,9 +18,13 @@ * every member of a family. * * - *

The {@code ttl} attribute is set to {@code expiresAt + cleanupRetention}.epochSecond so - * DynamoDB's native TTL eventually prunes used/revoked/expired rows. Synchronous cleanup via {@code - * deleteExpiredBefore} stays available for tests and operator-driven retention. + *

Two epoch-second attributes drive the lifecycle. {@code expiresAtEpoch} is the token's hard + * expiry and is what the {@code rotateAtomically} freshness condition compares against — a numeric + * compare avoids the variable-precision pitfall of comparing {@code Instant.toString()} ISO + * strings. {@code ttl} is {@code expiresAt + cleanupRetention}.epochSecond and is the attribute + * DynamoDB's native TTL prunes on, so used / revoked / expired rows survive the forensic-retention + * window before being swept. Synchronous cleanup via {@code deleteExpiredBefore} stays available + * for tests and operator-driven retention. * * @since 1.1.0 */ @@ -42,6 +46,7 @@ public class RefreshTokenItem { private String revokedAtIso; private String revokedReason; private String amr; + private Long expiresAtEpoch; private Long ttl; public RefreshTokenItem() {} @@ -169,6 +174,17 @@ public void setAmr(String amr) { this.amr = amr; } + /** + * Hard expiry as an epoch second; drives the numeric freshness check in {@code rotateAtomically}. + */ + public Long getExpiresAtEpoch() { + return expiresAtEpoch; + } + + public void setExpiresAtEpoch(Long expiresAtEpoch) { + this.expiresAtEpoch = expiresAtEpoch; + } + public Long getTtl() { return ttl; } diff --git a/pk-auth-persistence-dynamodb/src/test/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepositoryIntegrationTest.java b/pk-auth-persistence-dynamodb/src/test/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepositoryIntegrationTest.java index 065e8ff..2a037d6 100644 --- a/pk-auth-persistence-dynamodb/src/test/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepositoryIntegrationTest.java +++ b/pk-auth-persistence-dynamodb/src/test/java/com/codeheadsystems/pkauth/persistence/dynamodb/DynamoDbRefreshTokenRepositoryIntegrationTest.java @@ -1,7 +1,16 @@ // SPDX-License-Identifier: MIT package com.codeheadsystems.pkauth.persistence.dynamodb; +import static org.assertj.core.api.Assertions.assertThat; + +import com.codeheadsystems.pkauth.api.UserHandle; +import com.codeheadsystems.pkauth.refresh.RefreshTokenRecord; import com.codeheadsystems.pkauth.testkit.RefreshTokenScenarios; +import java.time.Duration; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -9,24 +18,80 @@ import org.testcontainers.junit.jupiter.Testcontainers; import software.amazon.awssdk.enhanced.dynamodb.DynamoDbEnhancedClient; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; +import software.amazon.awssdk.services.dynamodb.model.AttributeValue; +import software.amazon.awssdk.services.dynamodb.model.GetItemRequest; @Testcontainers @DisabledIfEnvironmentVariable(named = "PKAUTH_SKIP_TESTCONTAINERS", matches = "1") class DynamoDbRefreshTokenRepositoryIntegrationTest { private DynamoDbRefreshTokenRepository repository; + private DynamoDbClient client; + private DynamoDbEnhancedClient enhanced; + private String coreTable; @BeforeEach void setUp() { - DynamoDbClient client = DynamoDbLocalFixture.client(); - DynamoDbEnhancedClient enhanced = DynamoDbLocalFixture.enhanced(); + client = DynamoDbLocalFixture.client(); + enhanced = DynamoDbLocalFixture.enhanced(); String suffix = UUID.randomUUID().toString().substring(0, 8); - PkAuthDynamoTables tables = - new PkAuthDynamoTables("PkAuthCore_" + suffix, "PkAuthUsers_" + suffix); + coreTable = "PkAuthCore_" + suffix; + PkAuthDynamoTables tables = new PkAuthDynamoTables(coreTable, "PkAuthUsers_" + suffix); new DynamoDbSchemaBootstrapper(client, tables).bootstrap(); repository = new DynamoDbRefreshTokenRepository(enhanced, tables); } + /** + * The native-TTL {@code ttl} attribute must carry {@code expiresAt + cleanupRetention} so the + * background sweep keeps used/revoked rows through the forensic window (parity with JDBI), while + * {@code expiresAtEpoch} carries the unextended hard expiry used by the freshness check. + */ + @Test + void nativeTtlExtendsExpiryByCleanupRetention() { + String suffix = UUID.randomUUID().toString().substring(0, 8); + PkAuthDynamoTables tables = new PkAuthDynamoTables(coreTable, "PkAuthUsers_" + suffix); + Duration retention = Duration.ofDays(7); + DynamoDbRefreshTokenRepository retentionRepo = + new DynamoDbRefreshTokenRepository(enhanced, tables, retention); + + Instant expiresAt = Instant.parse("2030-01-01T00:00:00Z"); + String refreshId = "rid-" + suffix; + RefreshTokenRecord record = + new RefreshTokenRecord( + refreshId, + new byte[32], + UserHandle.of(new byte[] {1, 2, 3, 4}), + "web", + Optional.empty(), + refreshId, + Optional.empty(), + Instant.parse("2029-12-18T00:00:00Z"), + expiresAt, + Optional.empty(), + Optional.empty(), + Optional.empty(), + List.of("pkauth", "webauthn")); + retentionRepo.create(record); + + Map item = + client + .getItem( + GetItemRequest.builder() + .tableName(coreTable) + .key( + Map.of( + "pk", AttributeValue.fromS("RT#" + refreshId), + "sk", AttributeValue.fromS("RT#" + refreshId))) + .build()) + .item(); + + assertThat(item).isNotEmpty(); + assertThat(Long.parseLong(item.get("expiresAtEpoch").n())) + .isEqualTo(expiresAt.getEpochSecond()); + assertThat(Long.parseLong(item.get("ttl").n())) + .isEqualTo(expiresAt.plus(retention).getEpochSecond()); + } + @Test void issueRotateRevokeHappyPath() { new RefreshTokenScenarios(repository).issueRotateRevokeHappyPath();