fix: resolve security & correctness findings from review

.github/dependabot.yml

@@ -17,12 +17,6 @@ updates:
         patterns:
           - "*"
     ignore:
-      # Spotless 8.x's google-java-format integration intermittently fails to load Guava / GJF
-      # classes (NoClassDefFoundError on Newlines / ImmutableList$*) on Gradle 9 / JDK 21. Pin
-      # to 7.x until upstream fixes. The Gradle plugin marker artifact uses the plugin id, not
-      # the implementation artifact id, so this is the right Dependabot dependency-name.
-      - dependency-name: "com.diffplug.spotless:com.diffplug.spotless.gradle.plugin"
-        versions: [">=8"]
       # Micronaut 5.x requires JVM 25; the project targets Java 21 (brief §3). Stay on the 4.x
       # line until the JDK baseline moves.
       - dependency-name: "io.micronaut:*"

gradle/libs.versions.toml

@@ -10,9 +10,11 @@
 java = "21"
 
 # Build / quality
-# Spotless pinned to the 7.x line: Spotless 8's google-java-format integration intermittently
-# fails class loading on Gradle 9 / JDK 21. Dependabot ignore in .github/dependabot.yml keeps
-# this from getting bumped back. Revisit when Spotless 8 ships a fix.
+# Spotless tracks the 8.x line (currently 8.7.0) and is kept current by Dependabot's
+# dev-dependencies group. Historical note: Spotless 8's google-java-format integration could
+# intermittently fail class loading on Gradle 9 / JDK 21 under a *cached* build, which is why the
+# Gradle build cache stays disabled (see gradle.properties) — `./gradlew check` itself runs clean
+# on 8.x. The earlier 7.x pin (and its dependabot ignore) has been retired.
 spotless = "8.7.0"
 errorprone-plugin = "5.1.0"
 errorprone-core = "2.50.0"

pk-auth-core/src/main/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationService.java

@@ -229,12 +229,20 @@ public StartRegistrationResult startRegistration(
     byte[] challenge = challengeGenerator.generate();
     ChallengeId challengeId = ChallengeId.random();
 
+    // Resolve the user-verification requirement (per-request override, else ceremony default) and
+    // persist it on the challenge record so finishRegistration can enforce it server-side. Without
+    // this, a per-request REQUIRED (e.g. step-up) is advisory only — finish would fall back to the
+    // global config and accept flagUV=false.
+    UserVerificationRequirement uv =
+        req.userVerification() == null ? ceremonyConfig.userVerification() : req.userVerification();
+
     challengeStore.put(
         challengeId,
         new ChallengeRecord(
             challenge,
             ChallengeRecord.Purpose.REGISTRATION,
             userHandle,
+            uv,
             clockProvider.now().plus(ceremonyConfig.challengeTtl())),
         ceremonyConfig.challengeTtl());
 
@@ -244,9 +252,6 @@ public StartRegistrationResult startRegistration(
     List<PublicKeyCredentialDescriptor> excludeCredentials =
         credentialRepository.findByUserHandle(userHandle).stream().map(this::toDescriptor).toList();
 
-    UserVerificationRequirement uv =
-        req.userVerification() == null ? ceremonyConfig.userVerification() : req.userVerification();
-
     AuthenticatorSelectionCriteria selection =
         new AuthenticatorSelectionCriteria(null, ceremonyConfig.residentKey(), null, uv);
 
@@ -351,13 +356,31 @@ private RegistrationData verifyRegistrationWithW4j(
         new RegistrationParameters(
             serverProperty,
             WebAuthn4JConverters.pubKeyCredParams(ceremonyConfig.acceptedAlgorithms()),
-            WebAuthn4JConverters.userVerificationRequired(ceremonyConfig.userVerification()),
+            effectiveUserVerificationRequired(challengeRecord),
             /* userPresenceRequired */ true);
     // Attestation signature verification is intentionally non-strict in the default manager;
     // BadSignatureException is not thrown here (see finding #41 / #3 for strict-mode opt-in).
     return webAuthnManager.verify(w4jRequest, w4jParams);
   }
 
+  /**
+   * Effective user-verification requirement for a finish step: {@code true} (UV required) if EITHER
+   * the global {@link CeremonyConfig#userVerification()} OR the per-request requirement resolved at
+   * start (persisted on {@link ChallengeRecord#userVerification()}) is {@code REQUIRED} — i.e. the
+   * stricter of the two. This enforces a per-request step-up {@code REQUIRED} server-side even when
+   * the global default is relaxed to {@code PREFERRED}/{@code DISCOURAGED}, and never weakens the
+   * global config. A {@code null} recorded requirement (legacy record) contributes nothing, so the
+   * global config still applies.
+   */
+  private boolean effectiveUserVerificationRequired(ChallengeRecord challengeRecord) {
+    boolean configRequired =
+        WebAuthn4JConverters.userVerificationRequired(ceremonyConfig.userVerification());
+    boolean perRequestRequired =
+        challengeRecord.userVerification() != null
+            && WebAuthn4JConverters.userVerificationRequired(challengeRecord.userVerification());
+    return configRequired || perRequestRequired;
+  }
+
   /**
    * Validates the attested credential data against the configured {@link AttestationTrustPolicy}
    * and rejects duplicates already stored. Returns the failure result to emit, or {@code null} when
@@ -399,7 +422,13 @@ private RegistrationResult persistRegistration(
 
     var authData = data.getAttestationObject().getAuthenticatorData();
     byte[] coseBytes = WebAuthn4JConverters.serializeCoseKey(acd.getCOSEKey(), objectConverter);
-    UUID aaguidUuid = AAGUID.ZERO.equals(acd.getAaguid()) ? null : acd.getAaguid().getValue();
+    // Null-guard the AAGUID to match evaluateAttestation: AAGUID.ZERO.equals(null) is false, so
+    // without the explicit null check a null AAGUID would NPE on getValue() and escape as a 500
+    // instead of a sealed RegistrationResult.
+    UUID aaguidUuid =
+        acd.getAaguid() == null || AAGUID.ZERO.equals(acd.getAaguid())
+            ? null
+            : acd.getAaguid().getValue();
 
     Set<Transport> transports = EnumSet.noneOf(Transport.class);
     if (data.getTransports() != null) {
@@ -480,18 +509,21 @@ public StartAuthenticationResult startAuthentication(
     byte[] challenge = challengeGenerator.generate();
     ChallengeId challengeId = ChallengeId.random();
 
+    // Resolve UV (per-request override, else ceremony default) and persist it on the record so
+    // finishAuthentication enforces it server-side (see startRegistration for rationale).
+    UserVerificationRequirement uv =
+        req.userVerification() == null ? ceremonyConfig.userVerification() : req.userVerification();
+
     challengeStore.put(
         challengeId,
         new ChallengeRecord(
             challenge,
             ChallengeRecord.Purpose.AUTHENTICATION,
             resolvedHandle,
+            uv,
             clockProvider.now().plus(ceremonyConfig.challengeTtl())),
         ceremonyConfig.challengeTtl());
 
-    UserVerificationRequirement uv =
-        req.userVerification() == null ? ceremonyConfig.userVerification() : req.userVerification();
-
     PublicKeyCredentialRequestOptionsJson options =
         new PublicKeyCredentialRequestOptionsJson(
             challenge, DEFAULT_TIMEOUT_MS, rpConfig.id(), allowCredentials, uv, null);
@@ -648,7 +680,7 @@ private AuthenticationData verifyAssertionWithW4j(
             serverProperty,
             w4jCred,
             /* allowCredentials */ null,
-            WebAuthn4JConverters.userVerificationRequired(ceremonyConfig.userVerification()),
+            effectiveUserVerificationRequired(challengeRecord),
             /* userPresenceRequired */ true);
     return webAuthnManager.verify(w4jRequest, w4jParams);
   }

pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/ChallengeRecord.java

@@ -2,6 +2,7 @@
 package com.codeheadsystems.pkauth.spi;
 
 import com.codeheadsystems.pkauth.api.UserHandle;
+import com.codeheadsystems.pkauth.api.UserVerificationRequirement;
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.Objects;
@@ -14,11 +15,21 @@
  * @param purpose whether this challenge belongs to a registration or authentication ceremony
  * @param userHandle the user this challenge is bound to; nullable for usernameless flows where the
  *     user is only known at finish
+ * @param userVerification the user-verification requirement resolved at start (the per-request
+ *     override if the caller supplied one, otherwise the ceremony default); persisted so the finish
+ *     step can enforce it server-side rather than letting a per-request {@code REQUIRED} be
+ *     silently downgraded. {@code null} means "no resolved requirement recorded" (legacy records /
+ *     direct constructions), in which case the finish step enforces only the global ceremony
+ *     config.
  * @param expiresAt absolute expiration; consumers should treat past-due records as missing
  * @since 0.9.0
  */
 public record ChallengeRecord(
-    byte[] challenge, Purpose purpose, @Nullable UserHandle userHandle, Instant expiresAt) {
+    byte[] challenge,
+    Purpose purpose,
+    @Nullable UserHandle userHandle,
+    @Nullable UserVerificationRequirement userVerification,
+    Instant expiresAt) {
 
   public ChallengeRecord {
     Objects.requireNonNull(challenge, "challenge");
@@ -30,6 +41,17 @@ public record ChallengeRecord(
     challenge = challenge.clone();
   }
 
+  /**
+   * Back-compatible constructor for callers that don't carry a resolved user-verification
+   * requirement; equivalent to passing {@code null} for {@code userVerification}.
+   *
+   * @since 2.1.0
+   */
+  public ChallengeRecord(
+      byte[] challenge, Purpose purpose, @Nullable UserHandle userHandle, Instant expiresAt) {
+    this(challenge, purpose, userHandle, null, expiresAt);
+  }
+
   @Override
   public byte[] challenge() {
     return challenge.clone();
@@ -41,12 +63,14 @@ public boolean equals(Object o) {
         && Arrays.equals(this.challenge, other.challenge)
         && this.purpose == other.purpose
         && Objects.equals(this.userHandle, other.userHandle)
+        && this.userVerification == other.userVerification
         && this.expiresAt.equals(other.expiresAt);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(Arrays.hashCode(challenge), purpose, userHandle, expiresAt);
+    return Objects.hash(
+        Arrays.hashCode(challenge), purpose, userHandle, userVerification, expiresAt);
   }
 
   /**

pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/OtpRepository.java

@@ -57,10 +57,16 @@ record StoredOtp(
   void save(StoredOtp otp);
 
   /**
-   * Returns the most recently issued, non-consumed, non-expired OTP for the given user + phone, if
-   * any. Used by {@code OtpService.verify} as the candidate for matching.
+   * Returns the most recently issued, non-consumed, and non-expired ({@code expiresAt > now}) OTP
+   * for the given user + phone, if any. Used by {@code OtpService.verify} as the candidate for
+   * matching.
+   *
+   * @param userHandle owning user
+   * @param phoneE164 destination phone in E.164 format
+   * @param now the caller's current instant (from the host ClockProvider)
+   * @since 2.1.0
    */
-  Optional<StoredOtp> findLatestActive(UserHandle userHandle, String phoneE164);
+  Optional<StoredOtp> findLatestActive(UserHandle userHandle, String phoneE164, Instant now);
 
   /**
    * Atomically increments the attempts counter for the supplied OTP id and returns the new count.

pk-auth-core/src/test/java/com/codeheadsystems/pkauth/lifecycle/UserDeletionServiceTest.java

@@ -172,7 +172,8 @@ private static final class StubOtpRepository implements OtpRepository {
     public void save(StoredOtp otp) {}
 
     @Override
-    public Optional<StoredOtp> findLatestActive(UserHandle userHandle, String phoneE164) {
+    public Optional<StoredOtp> findLatestActive(
+        UserHandle userHandle, String phoneE164, Instant now) {
       return Optional.empty();
     }
 

pk-auth-core/src/test/java/com/codeheadsystems/pkauth/spi/ChallengeRecordTest.java

@@ -5,6 +5,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import com.codeheadsystems.pkauth.api.UserHandle;
+import com.codeheadsystems.pkauth.api.UserVerificationRequirement;
 import java.time.Instant;
 import org.junit.jupiter.api.Test;
 
@@ -24,6 +25,29 @@ void buildsAndCopiesChallenge() {
     assertThat(rec.expiresAt()).isEqualTo(exp);
   }
 
+  @Test
+  void carriesUserVerificationAndDefaultsToNullViaCompatConstructor() {
+    Instant exp = Instant.parse("2024-01-01T00:05:00Z");
+    UserHandle uh = UserHandle.random();
+
+    ChallengeRecord withUv =
+        new ChallengeRecord(
+            new byte[] {1},
+            ChallengeRecord.Purpose.AUTHENTICATION,
+            uh,
+            UserVerificationRequirement.REQUIRED,
+            exp);
+    assertThat(withUv.userVerification()).isEqualTo(UserVerificationRequirement.REQUIRED);
+
+    // The back-compat 4-arg constructor records no resolved requirement.
+    ChallengeRecord legacy =
+        new ChallengeRecord(new byte[] {1}, ChallengeRecord.Purpose.AUTHENTICATION, uh, exp);
+    assertThat(legacy.userVerification()).isNull();
+
+    // userVerification participates in equality.
+    assertThat(withUv).isNotEqualTo(legacy);
+  }
+
   @Test
   void equalsAndHashCode() {
     Instant exp = Instant.parse("2024-01-01T00:05:00Z");

pk-auth-dropwizard/src/main/java/com/codeheadsystems/pkauth/dropwizard/dagger/AltFlowsModule.java

@@ -8,8 +8,8 @@
 import com.codeheadsystems.pkauth.backupcodes.BackupCodeService;
 import com.codeheadsystems.pkauth.dropwizard.admin.PkAuthAdminResource;
 import com.codeheadsystems.pkauth.dropwizard.config.PkAuthConfig;
-import com.codeheadsystems.pkauth.jwt.PkAuthJwtIssuer;
-import com.codeheadsystems.pkauth.jwt.PkAuthJwtValidator;
+import com.codeheadsystems.pkauth.jwt.JwtConfig;
+import com.codeheadsystems.pkauth.jwt.JwtKeyset;
 import com.codeheadsystems.pkauth.lifecycle.BackupCodeRepositoryDeletionListener;
 import com.codeheadsystems.pkauth.lifecycle.OtpRepositoryDeletionListener;
 import com.codeheadsystems.pkauth.lifecycle.UserDeletionListener;
@@ -156,8 +156,8 @@ BackupCodeService provideBackupCodeService(BackupCodeRepository repo, ClockProvi
   @Provides
   @Singleton
   MagicLinkService provideMagicLinkService(
-      PkAuthJwtIssuer issuer,
-      PkAuthJwtValidator validator,
+      JwtConfig jwtConfig,
+      JwtKeyset keyset,
       EmailSender emailSender,
       UserLookup userLookup,
       ClockProvider clock,
@@ -168,8 +168,11 @@ MagicLinkService provideMagicLinkService(
           "pkAuth.magicLink configuration block is required when alt-flow auto-wiring is enabled"
               + " (set magicLink.baseUrl).");
     }
+    // Dedicated magic-link issuer/validator scoped to MagicLinkService.DEFAULT_AUDIENCE so the
+    // resource-server validator (application audience) rejects magic-link tokens as bearer tokens.
     return MagicLinkService.create(
-        MagicLinkService.Dependencies.of(issuer, validator, emailSender, userLookup, clock),
+        MagicLinkService.Dependencies.ofDedicatedAudience(
+            keyset, jwtConfig.issuer(), emailSender, userLookup, clock),
         ml.baseUrl());
   }
 

pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/JwtKeyset.java

@@ -64,10 +64,13 @@ public static JwtKeyset hs256(byte[] secret) {
   public static JwtKeyset es256(ECKey current, ECKey... retired) {
     Objects.requireNonNull(current, "current");
     List<JWK> all = new ArrayList<>();
-    all.add(current);
+    // Store ONLY public halves in the verification set. verificationSource() is public and a host
+    // may publish it as a JWKS endpoint; including the private `d` parameter here would leak the
+    // signing key. The full `current` (with the private key) is retained separately for signer().
+    all.add(current.toPublicJWK());
     for (ECKey r : retired) {
       Objects.requireNonNull(r, "retired key");
-      all.add(r);
+      all.add(r.toPublicJWK());
     }
     return new JwtKeyset(current, JWSAlgorithm.ES256, all);
   }

pk-auth-jwt/src/main/java/com/codeheadsystems/pkauth/jwt/PkAuthJwtIssuer.java

@@ -7,13 +7,15 @@
 import com.nimbusds.jose.JWSHeader;
 import com.nimbusds.jwt.JWTClaimsSet;
 import com.nimbusds.jwt.SignedJWT;
+import java.time.Duration;
 import java.time.Instant;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.UUID;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Issues pk-auth JWTs. Maps {@link JwtClaims} onto the standard {@code iss/sub/aud/iat/nbf/exp/jti}
@@ -78,11 +80,30 @@ public PkAuthJwtIssuer(
    * to the caller — the host must retry or surface the failure upstream.
    */
   public String issue(JwtClaims claims) {
+    return issue(claims, null);
+  }
+
+  /**
+   * Issues a signed JWT whose lifetime is {@code ttlOverride} instead of the per-audience
+   * access-token TTL resolved from {@link JwtConfig#ttlPolicy()}. Use this for short-lived,
+   * single-purpose tokens (e.g. magic links) that must NOT inherit the full access-token validity
+   * window — a magic link minted at the 1-hour access TTL stays redeemable as a bearer token (and
+   * replayable past its single-use JTI retention) far longer than intended. A {@code null} {@code
+   * ttlOverride} falls back to the audience's access TTL, making this exactly equivalent to {@link
+   * #issue(JwtClaims)}.
+   *
+   * @param claims the claims to sign
+   * @param ttlOverride the token lifetime, or {@code null} to use the audience access TTL
+   * @return the serialized, signed JWT
+   * @since 2.1.0
+   */
+  public String issue(JwtClaims claims, @Nullable Duration ttlOverride) {
     Objects.requireNonNull(claims, "claims");
     Instant now = clockProvider.now();
     Instant nbf = now.minus(config.notBeforeSkew());
     String audience = claims.audience() != null ? claims.audience() : config.defaultAudience();
-    Instant exp = now.plus(config.ttlPolicy().accessTtl(audience));
+    Instant exp =
+        now.plus(ttlOverride != null ? ttlOverride : config.ttlPolicy().accessTtl(audience));
     String jti = UUID.randomUUID().toString();
 
     JWTClaimsSet.Builder body =