Skip to content

Commit d489a0b

Browse files
wolpertclaude
andcommitted
security: resolve MED-severity review findings (TODO #18-#33, #35-#36)
WebAuthn / core - #18 finishAuthentication now binds the asserted credential to the challenge-record userHandle and to the response.userHandle when present; mismatches reject as UnknownCredential before WebAuthn4J verification. - #33 Extract ChallengeValidator + sealed ChallengeValidation from the 586-line DefaultPasskeyAuthenticationService; ceremony methods now follow validate -> verify -> map -> emit and the validator is unit-tested in isolation. OTP / alt-flows - #19 Mask phone numbers (+1***1234) in OtpService logs; drop message/body from LoggingSmsSender and LoggingEmailSender. - #21 OtpRepository.incrementAttempts now returns the new count; OtpService rejects when the post-increment count exceeds maxAttempts, closing the TOCTOU window. - #28 Replace Argon2id with HMAC-SHA256+server-side-pepper for OTP code storage (proportionate to 10^6 search space) and drop the argon2 dep from the OTP module. - #20 sendLoginEmail collapses UserNotFound into Sent to remove the account-enumeration shape/timing side channel. - #22 InMemoryRateLimiter now logs an INFO warning at construction and Javadocs the per-replica multiplier; production deployments must swap in a shared limiter. JWT - #24 Add RevocationCheck SPI (default no-op) consulted by PkAuthJwtValidator after signature/claims checks; new Revoked variant on JwtVerificationResult. Persistence - #25 JdbiUserLookup.createOrGetUserHandle is now a single atomic INSERT ... ON CONFLICT DO UPDATE ... RETURNING, removing the re-SELECT race window. - #26 DynamoDB CredentialItem gains @DynamoDbVersionAttribute; updateLabel and updateSignCount retry up to 3 times on ConditionalCheckFailed. - #27 New Flyway V6 migration adds revoked_at / revoked_reason to backup_codes + credentials and creates pkauth_audit_events. Backup-code consume and credential delete are now soft-deletes with audit rows; active-row queries filter on revoked_at IS NULL. Adapters / API - #29 Micronaut ceremony + admin controllers annotated @ExecuteOn(TaskExecutors.BLOCKING); Javadoc clarifies the blocking-SPI design. - #31 New PkAuthCeremonyJwt.mintForAssertion helper shared by all three adapters; standardizes amr=["pkauth","webauthn"] and eliminates the Spring re-fetch. - #35 Rename adapter classes to the PkAuth* prefix (Dropwizard Passkey*, Micronaut PkAuthJwtFilter, Spring admin controllers / result mappers / JwtAuthenticationToken). - #36 Delete the unused Spring PkAuthAuthenticationProvider + PreAuthenticatedJwtToken (filter writes SecurityContextHolder directly). - #30 PkAuthHttpError parses JSON body on construction and exposes error/detail/outcome; new CeremonyResult discriminated union mirrors the Java sealed result hierarchies. - #32 Delete examples/spring-boot-demo DemoConfiguration; relying-party config now lives under pkauth.relying-party.* in application.yml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 177b440 commit d489a0b

69 files changed

Lines changed: 1590 additions & 717 deletions

File tree

Some content is hidden

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

clients/passkeys-browser/src/http.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,36 @@ import type { ClientOptions } from "./types";
55
export class PkAuthHttpError extends Error {
66
readonly status: number;
77
readonly body: string;
8+
readonly data: Record<string, unknown> | undefined;
89

910
constructor(status: number, body: string) {
1011
super(`HTTP ${status}: ${body}`);
1112
this.name = "PkAuthHttpError";
1213
this.status = status;
1314
this.body = body;
15+
try {
16+
const parsed = JSON.parse(body);
17+
this.data = parsed !== null && typeof parsed === "object" && !Array.isArray(parsed)
18+
? (parsed as Record<string, unknown>)
19+
: undefined;
20+
} catch {
21+
this.data = undefined;
22+
}
23+
}
24+
25+
/** The `error` field from the parsed response body, if present. */
26+
get error(): string | undefined {
27+
return typeof this.data?.["error"] === "string" ? (this.data["error"] as string) : undefined;
28+
}
29+
30+
/** The `detail` field from the parsed response body, if present. */
31+
get detail(): string | undefined {
32+
return typeof this.data?.["detail"] === "string" ? (this.data["detail"] as string) : undefined;
33+
}
34+
35+
/** The `outcome` field from the parsed response body, if present. */
36+
get outcome(): string | undefined {
37+
return typeof this.data?.["outcome"] === "string" ? (this.data["outcome"] as string) : undefined;
1438
}
1539
}
1640

clients/passkeys-browser/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
export * as base64url from "./base64url";
44
export { PkAuthHttpError } from "./http";
5+
export type { CeremonyResult } from "./results";
6+
export { isCeremonySuccess, isCeremonyFailure } from "./results";
57
export {
68
PkAuthCeremonyClient,
79
decodeCreationOptions,
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// SPDX-License-Identifier: MIT
2+
3+
/**
4+
* Discriminated union mirroring the Java sealed result hierarchies
5+
* (AuthenticationResult / RegistrationResult) as returned by the pk-auth HTTP adapters.
6+
*
7+
* Authentication outcomes: success | challenge_invalid | origin_mismatch |
8+
* signature_invalid | unknown_credential | failed
9+
*
10+
* Registration outcomes: success | challenge_invalid | origin_mismatch |
11+
* attestation_rejected | duplicate_credential | invalid_payload | failed
12+
*/
13+
export type CeremonyResult =
14+
| { outcome: 'success'; token: string; credentialId?: string }
15+
| { outcome: 'challenge_invalid'; detail?: string }
16+
| { outcome: 'origin_mismatch'; detail?: string }
17+
| { outcome: 'signature_invalid'; detail?: string }
18+
| { outcome: 'unknown_credential'; detail?: string }
19+
| { outcome: 'attestation_rejected'; detail?: string }
20+
| { outcome: 'duplicate_credential'; detail?: string }
21+
| { outcome: 'invalid_payload'; detail?: string }
22+
| { outcome: 'failed'; detail?: string };
23+
24+
/** Type guard: narrows a CeremonyResult to the success variant. */
25+
export function isCeremonySuccess(
26+
result: CeremonyResult,
27+
): result is { outcome: 'success'; token: string; credentialId?: string } {
28+
return result.outcome === 'success';
29+
}
30+
31+
/** Type guard: narrows a CeremonyResult to any failure variant. */
32+
export function isCeremonyFailure(
33+
result: CeremonyResult,
34+
): result is Exclude<CeremonyResult, { outcome: 'success' }> {
35+
return result.outcome !== 'success';
36+
}

clients/passkeys-browser/test/http.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,29 @@ describe("request()", () => {
9797
expect(e.message).toContain("500");
9898
expect(e.message).toContain("boom");
9999
});
100+
101+
it("PkAuthHttpError parses JSON body and exposes error/detail/outcome getters", () => {
102+
const body = JSON.stringify({ error: "invalid_token", detail: "expired", outcome: "failed" });
103+
const e = new PkAuthHttpError(400, body);
104+
expect(e.data).toMatchObject({ error: "invalid_token", detail: "expired", outcome: "failed" });
105+
expect(e.error).toBe("invalid_token");
106+
expect(e.detail).toBe("expired");
107+
expect(e.outcome).toBe("failed");
108+
// raw body still preserved
109+
expect(e.body).toBe(body);
110+
expect(e.status).toBe(400);
111+
});
112+
113+
it("PkAuthHttpError leaves data/error/detail/outcome undefined for non-JSON body", () => {
114+
const e = new PkAuthHttpError(503, "Service Unavailable");
115+
expect(e.data).toBeUndefined();
116+
expect(e.error).toBeUndefined();
117+
expect(e.detail).toBeUndefined();
118+
expect(e.outcome).toBeUndefined();
119+
});
120+
121+
it("PkAuthHttpError leaves data undefined when JSON body is not an object", () => {
122+
const e = new PkAuthHttpError(400, JSON.stringify([1, 2, 3]));
123+
expect(e.data).toBeUndefined();
124+
});
100125
});

examples/dropwizard-demo/src/main/java/com/codeheadsystems/pkauth/demo/dropwizard/DemoApplication.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,11 @@ private AdminService buildAdminService(DemoPersistence persistence) {
114114
persistence.userLookup(),
115115
clock,
116116
"http://localhost:8080");
117+
byte[] otpPepper = new byte[32];
118+
new java.security.SecureRandom().nextBytes(otpPepper);
117119
OtpService otp =
118-
new OtpService(persistence.bindings().otpRepository(), new LoggingSmsSender(), clock);
120+
new OtpService(
121+
persistence.bindings().otpRepository(), new LoggingSmsSender(), clock, otpPepper);
119122
return DefaultAdminService.builder()
120123
.credentialRepository(persistence.bindings().credentialRepository())
121124
.userLookup(persistence.userLookup())

examples/spring-boot-demo/src/main/java/com/codeheadsystems/pkauth/examples/spring/DemoConfiguration.java

Lines changed: 0 additions & 47 deletions
This file was deleted.

examples/spring-boot-demo/src/main/java/com/codeheadsystems/pkauth/examples/spring/SpringBootDemoApplication.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,17 @@
55
import org.springframework.boot.autoconfigure.SpringBootApplication;
66

77
/**
8-
* Runnable entry point for the pk-auth Spring Boot demo. The auto-configuration in pk-auth-spring-
9-
* boot-starter wires every ceremony and admin endpoint; this class only provides the {@code
10-
* main(String[])} and a profile-conditional override for the {@code RelyingPartyConfig} (see {@link
11-
* DemoConfiguration}). To run:
8+
* Runnable entry point for the pk-auth Spring Boot demo. The auto-configuration in
9+
* pk-auth-spring-boot-starter wires every ceremony and admin endpoint; relying-party identity is
10+
* configured via {@code pkauth.relying-party.*} in {@code application.yml} — no custom
11+
* {@code @Bean @Primary RelyingPartyConfig} override is needed. To run:
1212
*
1313
* <pre>
14-
* ./gradlew :examples:spring-boot-demo:bootRun # JDBI persistence (Postgres if local)
15-
* ./gradlew :examples:spring-boot-demo:bootRun \
16-
* --args="--persistence=dynamodb" # DynamoDB persistence
14+
* ./gradlew :examples:spring-boot-demo:bootRun
1715
* </pre>
1816
*
19-
* <p>Without an explicit {@code --persistence=} flag the demo ships with the testkit's in-memory
20-
* SPIs (the autoconfigure defaults), so reviewers can boot the demo with no external services.
17+
* <p>The demo ships with the testkit's in-memory SPIs ({@code pkauth.dev-mode=true}), so reviewers
18+
* can boot the demo with no external services.
2119
*/
2220
@SpringBootApplication
2321
public class SpringBootDemoApplication {

examples/spring-boot-demo/src/main/resources/application.yml

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,6 @@ spring:
66
threads:
77
virtual:
88
enabled: true # brief §11 — virtual threads in every adapter's HTTP server
9-
demo:
10-
rp-id: localhost
11-
rp-name: pk-auth Spring Boot demo
12-
origins:
13-
- http://localhost:8080
14-
# `memory` (default) uses the testkit's InMemory SPIs — boots with no external services.
15-
# `jdbi` requires a running Postgres + flyway-migrated schema (see docker-compose.yml).
16-
# `dynamodb` requires a running DynamoDB Local + bootstrapped tables.
17-
persistence: memory
189
pkauth:
1910
# Demo runs against the testkit's in-memory SPIs by default. In production, do NOT set
2011
# `dev-mode: true` — supply real CredentialRepository / UserLookup / ChallengeStore /

examples/spring-boot-demo/src/test/resources/application.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,3 @@
1-
demo:
2-
rp-id: example.com
3-
rp-name: pk-auth Spring Boot demo (test)
4-
origins:
5-
- https://example.com
6-
persistence: memory
71
pkauth:
82
dev-mode: true
93
relying-party:

pk-auth-admin-api/src/test/java/com/codeheadsystems/pkauth/admin/DefaultAdminServiceTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ void setUp() {
5858
alice = users.register("alice", "Alice");
5959

6060
Argon2 argon2 = Argon2Factory.create(Argon2Factory.Argon2Types.ARGON2id);
61+
byte[] otpPepper = new byte[32];
62+
new SecureRandom().nextBytes(otpPepper);
6163
backupCodeService =
6264
new BackupCodeService(backupCodes, CLOCK, new SecureRandom(), argon2, 1, 1024, 1, 5);
6365
otpService =
@@ -66,7 +68,7 @@ void setUp() {
6668
new LoggingSmsSender(),
6769
CLOCK,
6870
new SecureRandom(),
69-
argon2,
71+
otpPepper,
7072
Duration.ofMinutes(5),
7173
3,
7274
3,

0 commit comments

Comments
 (0)