You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@juneau.apache.org by ja...@apache.org on 2026/08/16 17:40:49 UTC
(juneau) branch master updated (4ebe17dbe8 -> bd08d89b25)
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
from 4ebe17dbe8 TODO-372: Decouple REST debug single-signal side effects (372a + 372b)
new b0c2630caa READY-378: Fail closed when SAML assertions lack a NotOnOrAfter bound
new eed5513459 READY-380: Require exact OAuth scope match in MCP resource server
new ac39eb9338 READY-386: Add aggregate decompression budget to the Parquet parser
new bd08d89b25 TODO-397: Add injectable LoginStateStore SPI for multi-node OIDC login state
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../juneau/marshall/parquet/ParquetConfig.java | 13 +
.../marshall/parquet/ParquetConfigAnnotation.java | 1 +
.../juneau/marshall/parquet/ParquetParser.java | 37 +-
.../marshall/parquet/ParquetParserSession.java | 66 +++-
.../parquet/ParquetParser_MaxLength_Test.java | 70 +++-
.../auth/oidc/rp/InMemoryLoginStateStore_Test.java | 74 ++--
.../auth/oidc/rp/InMemorySessionStore_Test.java | 2 +-
.../oidc/rp/SignedCookieSessionStore_Test.java | 2 +-
...eralStore.java => InMemoryLoginStateStore.java} | 78 ++---
.../rest/server/auth/oidc/rp/LoginStateStore.java | 172 ++++++++++
.../rest/server/auth/oidc/rp/OidcRelyingParty.java | 100 +++++-
.../auth/oidc/rp/InMemoryLoginStateStore_Test.java | 75 +++--
.../auth/oidc/rp/InMemorySessionStore_Test.java | 2 +-
.../server/auth/oidc/rp/LoginStateStore_Test.java | 373 +++++++++++++++++++++
.../auth/oidc/rp/OidcRelyingPartyBuilder_Test.java | 54 +++
.../rest/server/auth/oidc/rp/OidcTestSupport.java | 35 ++
.../oidc/rp/SignedCookieSessionStore_Test.java | 2 +-
.../server/auth/saml/SamlAssertionValidator.java | 51 ++-
.../saml/SamlAssertionValidator_Branches_Test.java | 12 +-
.../SamlAssertionValidator_ReplayExpiry_Test.java | 202 +++++++++++
.../mcp/v20260728/McpResourceServerSupport.java | 25 +-
.../v20260728/McpResourceServerBinding_Test.java | 28 +-
.../v20260728/McpResourceServerSupport_Test.java | 38 ++-
23 files changed, 1320 insertions(+), 192 deletions(-)
rename juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore_Test.java => juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore_Test.java (52%)
rename juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/{EphemeralStore.java => InMemoryLoginStateStore.java} (58%)
create mode 100644 juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/LoginStateStore.java
rename juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore_Test.java => juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore_Test.java (51%)
create mode 100644 juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/LoginStateStore_Test.java
create mode 100644 juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_ReplayExpiry_Test.java
(juneau) 04/04: TODO-397: Add injectable LoginStateStore SPI for multi-node OIDC login state
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit bd08d89b252c0f7635e2651eb0a3e35083309f01
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 13:40:36 2026 -0400
TODO-397: Add injectable LoginStateStore SPI for multi-node OIDC login state
Extract a LoginStateStore SPI (rename EphemeralStore -> InMemoryLoginStateStore) so
clustered deployments can share ephemeral login state. Framework owns the TTL ceiling
on the custom-store path, re-validates the redirect target and expiry on consume
(fail-closed), and redacts secrets in PendingLogin.toString().
---
.../auth/oidc/rp/InMemoryLoginStateStore_Test.java | 74 ++--
.../auth/oidc/rp/InMemorySessionStore_Test.java | 2 +-
.../oidc/rp/SignedCookieSessionStore_Test.java | 2 +-
...eralStore.java => InMemoryLoginStateStore.java} | 78 ++---
.../rest/server/auth/oidc/rp/LoginStateStore.java | 172 ++++++++++
.../rest/server/auth/oidc/rp/OidcRelyingParty.java | 100 +++++-
.../auth/oidc/rp/InMemoryLoginStateStore_Test.java | 75 +++--
.../auth/oidc/rp/InMemorySessionStore_Test.java | 2 +-
.../server/auth/oidc/rp/LoginStateStore_Test.java | 373 +++++++++++++++++++++
.../auth/oidc/rp/OidcRelyingPartyBuilder_Test.java | 54 +++
.../rest/server/auth/oidc/rp/OidcTestSupport.java | 35 ++
.../oidc/rp/SignedCookieSessionStore_Test.java | 2 +-
12 files changed, 849 insertions(+), 120 deletions(-)
diff --git a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore_Test.java
similarity index 52%
rename from juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore_Test.java
rename to juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore_Test.java
index bfd2230ee0..e53ec7f20b 100644
--- a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore_Test.java
+++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore_Test.java
@@ -21,29 +21,39 @@ import static org.junit.jupiter.api.Assertions.*;
import java.time.*;
import org.apache.juneau.*;
+import org.apache.juneau.rest.server.auth.oidc.rp.LoginStateStore.PendingLogin;
+import org.apache.juneau.rest.server.auth.oidc.rp.OidcTestSupport.MutableClock;
import org.junit.jupiter.api.*;
/**
- * Tests for {@link EphemeralStore} — single-use, TTL-bounded, size-bounded state/nonce storage.
+ * Tests for {@link InMemoryLoginStateStore} — the shipped single-node, single-use, TTL-bounded,
+ * size-bounded default {@link LoginStateStore}.
*
* @since 10.0.0
*/
@SuppressWarnings({
"java:S5778" // assertThrows lambdas with chained calls; intermediate invocations do not throw in practice
})
-class EphemeralStore_Test extends TestBase {
+class InMemoryLoginStateStore_Test extends TestBase {
// Fixed clock seam: these cases don't depend on wall-clock time, so a deterministic clock
// replaces the system clock (java:S8692) without changing behavior.
private static final Clock CLOCK = Clock.fixed(Instant.parse("2026-01-01T00:00:00Z"), ZoneOffset.UTC);
+ private static final Duration TTL = Duration.ofMinutes(5);
- private static EphemeralStore store(Clock clock) {
- return new EphemeralStore(Duration.ofMinutes(5), 100, clock);
+ private static InMemoryLoginStateStore store(Clock clock) {
+ return new InMemoryLoginStateStore(TTL, 100, clock);
+ }
+
+ /** Builds a framework-shaped record stamped from the given clock (createdAt=now, expiresAt=now+TTL). */
+ private static PendingLogin pending(Clock clock, String nonce, String verifier, String redirect) {
+ var now = clock.instant();
+ return new PendingLogin(nonce, verifier, redirect, now, now.plus(TTL));
}
@Test void a01_storeThenConsume_roundTrips() {
var s = store(CLOCK);
- s.store("state-1", "nonce-1", "verifier-1", "/dashboard");
+ s.store("state-1", pending(CLOCK, "nonce-1", "verifier-1", "/dashboard"));
var p = s.consume("state-1");
assertTrue(p.isPresent());
assertEquals("nonce-1", p.get().nonce());
@@ -53,7 +63,7 @@ class EphemeralStore_Test extends TestBase {
@Test void a02_consume_isSingleUse() {
var s = store(CLOCK);
- s.store("state-1", "nonce-1", "verifier-1", null);
+ s.store("state-1", pending(CLOCK, "nonce-1", "verifier-1", null));
assertTrue(s.consume("state-1").isPresent());
assertTrue(s.consume("state-1").isEmpty(), "second consume of same state must miss (replay defense)");
}
@@ -71,61 +81,65 @@ class EphemeralStore_Test extends TestBase {
@Test void b01_expiredEntry_isMissed() {
var base = Instant.parse("2026-01-01T00:00:00Z");
var clock = new MutableClock(base);
- var s = new EphemeralStore(Duration.ofMinutes(5), 100, clock);
- s.store("state-1", "nonce-1", "verifier-1", null);
+ var s = new InMemoryLoginStateStore(TTL, 100, clock);
+ s.store("state-1", pending(clock, "nonce-1", "verifier-1", null));
clock.advance(Duration.ofMinutes(6));
- assertTrue(s.consume("state-1").isEmpty(), "entry past TTL must be treated as a miss");
+ assertTrue(s.consume("state-1").isEmpty(), "entry past expiresAt must be treated as a miss");
}
@Test void b02_notYetExpired_survives() {
var base = Instant.parse("2026-01-01T00:00:00Z");
var clock = new MutableClock(base);
- var s = new EphemeralStore(Duration.ofMinutes(5), 100, clock);
- s.store("state-1", "nonce-1", "verifier-1", null);
+ var s = new InMemoryLoginStateStore(TTL, 100, clock);
+ s.store("state-1", pending(clock, "nonce-1", "verifier-1", null));
clock.advance(Duration.ofMinutes(4));
assertTrue(s.consume("state-1").isPresent());
}
+ /** A stored record with a {@code null} expiresAt fails closed as expired (defensive against a foreign blob). */
+ @Test void b03_nullExpiresAt_isTreatedAsExpired() {
+ var s = store(CLOCK);
+ var now = CLOCK.instant();
+ s.store("state-1", new PendingLogin("nonce-1", "verifier-1", null, now, null));
+ assertTrue(s.consume("state-1").isEmpty(), "null expiresAt must fail closed as expired");
+ }
+
@Test void c01_sizeCap_evictsEldest() {
- var s = new EphemeralStore(Duration.ofMinutes(5), 3, CLOCK);
- s.store("s1", "n", "v", null);
- s.store("s2", "n", "v", null);
- s.store("s3", "n", "v", null);
- s.store("s4", "n", "v", null); // evicts s1
+ var s = new InMemoryLoginStateStore(TTL, 3, CLOCK);
+ s.store("s1", pending(CLOCK, "n", "v", null));
+ s.store("s2", pending(CLOCK, "n", "v", null));
+ s.store("s3", pending(CLOCK, "n", "v", null));
+ s.store("s4", pending(CLOCK, "n", "v", null)); // evicts s1
assertEquals(3, s.size());
assertTrue(s.consume("s1").isEmpty());
assertTrue(s.consume("s4").isPresent());
}
@Test void d01_rejectsNonPositiveTtl() {
- assertThrows(IllegalArgumentException.class, () -> new EphemeralStore(Duration.ZERO, 100, CLOCK));
+ assertThrows(IllegalArgumentException.class, () -> new InMemoryLoginStateStore(Duration.ZERO, 100, CLOCK));
}
- /** Negative TTL rejected — line 82 second bytecode branch of {@code !isZero && !isNegative}. */
+ /** Negative TTL rejected — second bytecode branch of {@code !isZero && !isNegative}. */
@Test void d05_rejectsNegativeTtl() {
- assertThrows(IllegalArgumentException.class, () -> new EphemeralStore(Duration.ofSeconds(-1), 100, CLOCK));
+ assertThrows(IllegalArgumentException.class, () -> new InMemoryLoginStateStore(Duration.ofSeconds(-1), 100, CLOCK));
}
@Test void d02_rejectsTtlAbove30Minutes() {
- assertThrows(IllegalArgumentException.class, () -> new EphemeralStore(Duration.ofMinutes(31), 100, CLOCK));
+ assertThrows(IllegalArgumentException.class, () -> new InMemoryLoginStateStore(Duration.ofMinutes(31), 100, CLOCK));
}
@Test void d03_rejectsNonPositiveMaxEntries() {
- assertThrows(IllegalArgumentException.class, () -> new EphemeralStore(Duration.ofMinutes(5), 0, CLOCK));
+ assertThrows(IllegalArgumentException.class, () -> new InMemoryLoginStateStore(Duration.ofMinutes(5), 0, CLOCK));
}
@Test void d04_rejectsBlankStateOnStore() {
var s = store(CLOCK);
- assertThrows(IllegalArgumentException.class, () -> s.store("", "n", "v", null));
+ var p = pending(CLOCK, "n", "v", null);
+ assertThrows(IllegalArgumentException.class, () -> s.store("", p));
}
- /** A test clock the test can advance manually. */
- static final class MutableClock extends Clock {
- private Instant now;
- MutableClock(Instant start) { now = start; }
- void advance(Duration d) { now = now.plus(d); }
- @Override public ZoneId getZone() { return ZoneOffset.UTC; }
- @Override public Clock withZone(ZoneId zone) { return this; }
- @Override public Instant instant() { return now; }
+ @Test void d06_rejectsNullPendingOnStore() {
+ var s = store(CLOCK);
+ assertThrows(IllegalArgumentException.class, () -> s.store("state-1", null));
}
}
diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemorySessionStore_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemorySessionStore_Test.java
index 9089550ba3..bcc035e4b2 100644
--- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemorySessionStore_Test.java
+++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemorySessionStore_Test.java
@@ -103,7 +103,7 @@ class InMemorySessionStore_Test extends TestBase {
@Test void c01_expiredSession_missedAndRemoved() {
var base = Instant.parse("2026-01-01T00:00:00Z");
- var clock = new EphemeralStore_Test.MutableClock(base);
+ var clock = new OidcTestSupport.MutableClock(base);
var store = new InMemorySessionStore(100, clock);
store.createSessionCookieValue(session("id-1", "alice", "sess-1", base));
clock.advance(Duration.ofHours(9));
diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/SignedCookieSessionStore_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/SignedCookieSessionStore_Test.java
index 9a13b36a92..da0af03e2f 100644
--- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/SignedCookieSessionStore_Test.java
+++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/SignedCookieSessionStore_Test.java
@@ -86,7 +86,7 @@ class SignedCookieSessionStore_Test extends TestBase {
@Test void c01_expiredCookie_isRejected() {
var base = Instant.parse("2026-01-01T00:00:00Z");
- var clock = new EphemeralStore_Test.MutableClock(base);
+ var clock = new OidcTestSupport.MutableClock(base);
var store = SignedCookieSessionStore.create().signingKey(KEY).clock(clock).build();
var cookie = store.createSessionCookieValue(session(base, null));
clock.advance(Duration.ofHours(9));
diff --git a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore.java b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore.java
similarity index 58%
rename from juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore.java
rename to juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore.java
index e1206b3309..27d0f8f910 100644
--- a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore.java
+++ b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore.java
@@ -23,26 +23,35 @@ import java.time.*;
import java.util.*;
/**
- * Single-use, TTL-bounded store for the per-login {@code state} → {@code (nonce, codeVerifier,
- * redirectTarget)} association created during {@link OidcRelyingParty#startLogin} and consumed during
- * {@link OidcRelyingParty#completeLogin}.
+ * The shipped, single-node default {@link LoginStateStore}: a single-use, TTL-bounded, size-bounded
+ * in-memory store for the per-login {@code state} → {@link LoginStateStore.PendingLogin} association
+ * created during {@link OidcRelyingParty#startLogin} and consumed during {@link OidcRelyingParty#completeLogin}.
*
* <p>
* Security properties (see OpenID Connect Core §3.1.2.1 and the RP charter):
* <ul>
- * <li><b>Single-use</b> — {@link #consume(String)} atomically removes the entry, so a replayed
- * callback with the same {@code state} fails.
- * <li><b>TTL-bounded</b> — entries older than the configured TTL are treated as a miss and swept.
+ * <li><b>Single-use</b> — {@link #consume(String)} atomically removes the entry under a monitor, so a
+ * replayed callback with the same {@code state} fails. Cross-node single-use requires a shared store
+ * with an atomic consume primitive (see {@link LoginStateStore#consume(String)}); this impl only
+ * guarantees single-use within one JVM.
+ * <li><b>TTL-bounded</b> — entries at or past their framework-minted {@code expiresAt} are treated as a
+ * miss and swept.
* <li><b>Size-bounded</b> — an LRU cap (default 10 000) prevents unbounded growth from abandoned
* login attempts; eviction mirrors the {@code BoundedLruTokenCache} shape.
* </ul>
*
* <p>
+ * The framework mints {@code createdAt}/{@code expiresAt} on the {@link LoginStateStore.PendingLogin} record;
+ * this impl persists them verbatim and does not stamp timestamps of its own. The {@code ttl} argument here
+ * only bounds sweep/GC of abandoned entries and asserts the framework's {@code (0, MAX_TTL]} ceiling for the
+ * default path; the relying party re-checks {@code expiresAt} independently on consume.
+ *
+ * <p>
* Thread-safe.
*
* @since 10.0.0
*/
-public class EphemeralStore {
+public class InMemoryLoginStateStore implements LoginStateStore {
/** Default maximum number of in-flight login attempts retained. */
public static final int DEFAULT_MAX_ENTRIES = 10_000;
@@ -50,22 +59,7 @@ public class EphemeralStore {
/** Default time-to-live for a pending login (the user has at most a few minutes at the IdP). */
public static final Duration DEFAULT_TTL = Duration.ofMinutes(5);
- /** Hard cap on the configurable TTL. */
- static final Duration MAX_TTL = Duration.ofMinutes(30);
-
- /**
- * A pending-login association awaiting the IdP callback.
- *
- * @param nonce The OIDC {@code nonce} value bound into the authorization request.
- * @param codeVerifier The PKCE {@code code_verifier} string.
- * @param redirectTarget The application URL to redirect to after a successful login. May be
- * {@code null}.
- * @param createdAt The instant the entry was stored.
- */
- public record PendingLogin(String nonce, String codeVerifier, String redirectTarget, Instant createdAt) {}
-
private final int maxEntries;
- private final Duration ttl;
private final Clock clock;
private final Map<String,PendingLogin> entries;
private final Object lock = new Object();
@@ -73,53 +67,40 @@ public class EphemeralStore {
/**
* Constructor.
*
- * @param ttl The single-use TTL. Must be positive and not exceed 30 minutes.
+ * @param ttl The single-use TTL. Must be positive and not exceed {@link LoginStateStore#MAX_TTL}. Bounds
+ * sweep of abandoned entries and asserts the framework ceiling for the default path; per-entry expiry is
+ * driven by the framework-minted {@link LoginStateStore.PendingLogin#expiresAt()}.
* @param maxEntries The LRU size cap. Must be positive.
- * @param clock The clock for TTL comparisons. Must not be <jk>null</jk>.
+ * @param clock The clock for TTL sweeping. Must not be <jk>null</jk>.
*/
- public EphemeralStore(Duration ttl, int maxEntries, Clock clock) {
+ public InMemoryLoginStateStore(Duration ttl, int maxEntries, Clock clock) {
assertArgNotNull("ttl", ttl);
assertArg(!ttl.isZero() && !ttl.isNegative(), "ttl must be positive");
assertArg(ttl.compareTo(MAX_TTL) <= 0, "ttl must not exceed 30 minutes (was %s)", ttl);
assertArg(maxEntries > 0, "maxEntries must be positive (was %s)", maxEntries);
- this.ttl = ttl;
this.maxEntries = maxEntries;
this.clock = assertArgNotNull("clock", clock);
this.entries = new LinkedHashMap<>(16, 0.75f, true) {
private static final long serialVersionUID = 1L;
@Override
protected boolean removeEldestEntry(Map.Entry<String,PendingLogin> eldest) {
- return size() > EphemeralStore.this.maxEntries;
+ return size() > InMemoryLoginStateStore.this.maxEntries;
}
};
}
- /**
- * Stores a pending-login association keyed by {@code state}.
- *
- * @param state The opaque {@code state} value. Must not be <jk>null</jk> or blank.
- * @param nonce The OIDC {@code nonce}. Must not be <jk>null</jk> or blank.
- * @param codeVerifier The PKCE {@code code_verifier}. Must not be <jk>null</jk> or blank.
- * @param redirectTarget The post-login redirect target. May be <jk>null</jk>.
- */
- public void store(String state, String nonce, String codeVerifier, String redirectTarget) {
+ @Override /* LoginStateStore */
+ public void store(String state, PendingLogin pending) {
assertArgNotNullOrBlank("state", state);
- assertArgNotNullOrBlank("nonce", nonce);
- assertArgNotNullOrBlank("codeVerifier", codeVerifier);
+ assertArgNotNull("pending", pending);
var now = clock.instant();
synchronized (lock) {
sweepExpired(now);
- entries.put(state, new PendingLogin(nonce, codeVerifier, redirectTarget, now));
+ entries.put(state, pending);
}
}
- /**
- * Atomically removes and returns the pending-login association for {@code state}, if present and
- * not expired.
- *
- * @param state The {@code state} value from the callback. Must not be <jk>null</jk>.
- * @return The association, or {@link Optional#empty()} if absent or expired.
- */
+ @Override /* LoginStateStore */
public Optional<PendingLogin> consume(String state) {
if (state == null)
return oe();
@@ -146,7 +127,10 @@ public class EphemeralStore {
}
private boolean isExpired(PendingLogin p, Instant now) {
- return !now.isBefore(p.createdAt().plus(ttl));
+ var exp = p.expiresAt();
+ if (exp == null)
+ return true;
+ return !now.isBefore(exp);
}
private void sweepExpired(Instant now) {
diff --git a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/LoginStateStore.java b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/LoginStateStore.java
new file mode 100644
index 0000000000..28d7efb476
--- /dev/null
+++ b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/LoginStateStore.java
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juneau.rest.server.auth.oidc.rp;
+
+import java.time.*;
+import java.util.*;
+
+/**
+ * SPI for the single-use, TTL-bounded {@code state} → ({@code nonce}, PKCE {@code code_verifier},
+ * redirect target) association created during {@link OidcRelyingParty#startLogin} and consumed during
+ * {@link OidcRelyingParty#completeLogin}.
+ *
+ * <p>
+ * The shipped default is {@link InMemoryLoginStateStore} (per-JVM, bounded, TTL-swept). Supply a shared
+ * (Redis / JDBC) implementation via {@link OidcRelyingParty.Builder#loginStateStore(LoginStateStore)} to
+ * support a clustered deployment where {@code /login} and {@code /callback} may land on different nodes
+ * without sticky sessions — the same motivation as a distributed {@link SessionStore}. When the
+ * hook is unused, behavior is identical to the single-node default.
+ *
+ * <h5 class='section'>Security contract (read before implementing)</h5>
+ *
+ * <p>
+ * <b>Secret-bearing.</b> A stored entry carries the PKCE {@code code_verifier} (and {@code nonce}). With a
+ * stolen authorization {@code code} (Referer, proxy log, browser history) the {@code code_verifier} IS the
+ * token-exchange credential. The in-memory default keeps it on the JVM heap; a shared (Redis / JDBC)
+ * implementation writes it to an external backend. Implementations therefore MUST protect confidentiality
+ * and integrity: TLS in transit and a locked-down ACL are the floor; application-level encryption-at-rest
+ * is strongly recommended. Implementations and callers MUST NOT log a {@link PendingLogin} (its
+ * {@code toString()} is redacted for exactly this reason), and MUST NOT put {@link PendingLogin#codeVerifier()}
+ * or {@link PendingLogin#nonce()} into MDC, metric tags, or field-wise serializers — the accessors
+ * exist only for the token exchange and the ID-token {@code nonce} check.
+ *
+ * <p>
+ * <b>The framework does not trust the store as an oracle.</b> A writable shared store (mis-ACL,
+ * SSRF-to-Redis, replica, backup restore, compromised sibling service) can mint or alter records. The
+ * relying party therefore re-validates on consume: it re-sanitizes {@code redirectTarget} to a safe
+ * relative path before redirecting (a tampered absolute / external URL is discarded in favor of the
+ * configured post-login default), and it re-checks {@code expiresAt} against its own {@link java.time.Clock}.
+ * The store MAY evict early; it is NOT the authoritative expiry decision-maker. Note this defeats an
+ * <i>honest</i> store that merely forgot to expire a record (past {@code expiresAt}); a malicious rewrite
+ * of {@code expiresAt} to a far-future instant is the same trust boundary as writing a fresh forged blob
+ * (an integrity concern an HMAC would address), which the framework does not attempt to defeat here.
+ *
+ * <p>
+ * <b>Thread-safe.</b> Implementations MUST be safe for concurrent {@code store} / {@code consume} from
+ * multiple request threads (as {@link SessionStore} requires).
+ *
+ * <p>
+ * <b>Per-process by default.</b> The shipped default enforces single-use only within one JVM. Cross-node
+ * single-use requires a shared backend with an atomic consume primitive (see {@link #consume(String)}).
+ *
+ * <h5 class='section'>See Also:</h5><ul>
+ * <li class='jc'>{@link InMemoryLoginStateStore}
+ * <li class='jc'>{@link OidcRelyingParty.Builder#loginStateStore(LoginStateStore)}
+ * <li class='jc'>{@link SessionStore}
+ * </ul>
+ *
+ * @since 10.0.0
+ */
+public interface LoginStateStore {
+
+ /** Hard cap on the framework-enforced login-state TTL — the single public home for the ceiling. */
+ Duration MAX_TTL = Duration.ofMinutes(30);
+
+ /**
+ * A pending-login association awaiting the IdP callback.
+ *
+ * <p>
+ * <b>Secret-bearing record.</b> {@code codeVerifier} and {@code nonce} are credentials (see the SPI
+ * security contract). {@code toString()} redacts both, matching {@code OAuthToken.toString()} and the
+ * MCP {@code PendingAuthorization} precedent, so a stray log line, exception message, or debugger dump
+ * does not disclose them.
+ *
+ * <p>
+ * <b>Framework-minted timestamps.</b> The relying party stamps {@code createdAt} (from its configured
+ * {@link java.time.Clock}) and {@code expiresAt} ({@code createdAt} plus the RP's capped
+ * {@code stateNonceTtl}). A store MUST persist these as opaque values and MUST NOT recompute them at
+ * consume time — the RP re-checks {@code expiresAt} independently, so an honest store that forgot
+ * to expire a record cannot extend usability. Either timestamp may be {@code null} only for a foreign /
+ * deserialized blob; the RP treats a {@code null} {@code createdAt} / {@code expiresAt} on consume as
+ * expired (fail-closed), so no compact-constructor {@code requireNonNull} is imposed here.
+ *
+ * @param nonce The OIDC {@code nonce}. Redacted in {@code toString()}.
+ * @param codeVerifier The PKCE {@code code_verifier}. Redacted in {@code toString()}.
+ * @param redirectTarget The post-login redirect target. May be {@code null}. Re-validated by the RP.
+ * @param createdAt Framework-minted store instant.
+ * @param expiresAt Framework-minted expiry ceiling ({@code createdAt + stateNonceTtl}).
+ */
+ record PendingLogin(String nonce, String codeVerifier, String redirectTarget, Instant createdAt, Instant expiresAt) {
+ @Override
+ public String toString() {
+ return "PendingLogin(nonce=<redacted>,codeVerifier=<redacted>,redirectTarget=" + redirectTarget
+ + ",createdAt=" + createdAt + ",expiresAt=" + expiresAt + ")";
+ }
+ }
+
+ /**
+ * Stores a framework-built pending-login association keyed by {@code state}.
+ *
+ * <p>
+ * Implementations MUST persist the association durably enough that a subsequent {@link #consume(String)}
+ * on any node handling the callback can retrieve it. On an <b>infrastructure failure</b> (backend
+ * unreachable, write rejected) implementations MUST throw rather than silently drop the record:
+ * {@code store} is invoked on the {@code startLogin} leg before any authorization URL is issued, so a
+ * thrown exception fails the flow closed with no half-initiated login. Implementations MUST NOT log the
+ * {@link PendingLogin} payload; wrap / sanitize backend exceptions so the {@code codeVerifier} /
+ * {@code state} do not leak into a 500 body or log.
+ *
+ * <p>
+ * Last-writer-wins: a repeated {@code store} for the same {@code state} overwrites (a Nimbus
+ * {@code State} collision is astronomical). A backend MAY use {@code SET NX} but is not required to. A
+ * backend MAY use the remaining time to {@code expiresAt} as a GC / {@code EXPIRE} hint, but GC failure
+ * MUST NOT extend usability past the framework's re-checked {@code expiresAt}.
+ *
+ * @param state The opaque {@code state} value. Must not be {@code null} or blank.
+ * @param pending The framework-built association (already stamped with {@code createdAt} / {@code expiresAt}).
+ * Must not be {@code null}.
+ */
+ void store(String state, PendingLogin pending);
+
+ /**
+ * Atomically removes and returns the pending-login association for {@code state}, if present.
+ *
+ * <p>
+ * <b>Single-use / atomicity is mandatory (TOCTOU).</b> The removal and the read MUST be a single atomic
+ * operation so a given {@code state} is consumed at most once. Of any two concurrent calls with the same
+ * {@code state}, <b>at most one</b> returns a present {@link Optional}; the other MUST miss —
+ * including across nodes sharing the store. Use an atomic primitive against the primary (Redis
+ * {@code GETDEL} or a Lua {@code GET}+{@code DEL}), <b>not</b> a read-then-delete and <b>not</b> a
+ * possibly-stale replica read. A consume that removes the entry but fails to return it (lost response)
+ * MUST be treated as a miss on retry — never a best-effort put-back (put-back reopens replay); the
+ * user simply restarts login.
+ *
+ * <p>
+ * <b>Bounded I/O.</b> Implementations MUST apply a bounded deadline to the backend round-trip so a hung
+ * backend cannot stall {@code completeLogin} on a request thread indefinitely.
+ *
+ * <p>
+ * <b>{@code empty()} vs. throw — distinct meanings.</b> Return {@link Optional#empty()}
+ * <b>only</b> for absent / already-consumed / (optionally) store-expired states — the caller
+ * treats {@code empty()} as a CSRF / replay / expiry rejection. On an <b>infrastructure failure</b>
+ * (backend unreachable) the implementation MUST throw (not return {@code empty()}) so the caller can
+ * distinguish an attack from an outage; the RP catches it, logs without the payload, and rejects with an
+ * "unavailable" authentication error. Either way the login fails closed; implementations MUST NEVER
+ * fabricate a {@link PendingLogin}.
+ *
+ * <p>
+ * <b>Expiry is not the store's sole responsibility.</b> An implementation MAY treat past-{@code expiresAt}
+ * entries as a miss (early eviction), but the RP re-checks {@code expiresAt} on the returned record
+ * regardless, so it is not a security defect if a store returns an over-age record — the framework
+ * rejects it.
+ *
+ * @param state The {@code state} value from the callback. May be {@code null} (returns {@code empty()},
+ * matching the defensive callback path — do not throw).
+ * @return The association, or {@link Optional#empty()} if absent, expired, or already consumed.
+ */
+ Optional<PendingLogin> consume(String state);
+}
diff --git a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcRelyingParty.java b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcRelyingParty.java
index f83e6056da..db467f4c25 100644
--- a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcRelyingParty.java
+++ b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/main/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcRelyingParty.java
@@ -144,9 +144,10 @@ public class OidcRelyingParty {
String postLoginRedirect = "/";
Set<String> scopes = st();
SessionStore sessionStore;
+ LoginStateStore loginStateStore;
String rolesClaim = DEFAULT_ROLES_CLAIM;
- Duration stateNonceTtl = EphemeralStore.DEFAULT_TTL;
- int ephemeralMaxEntries = EphemeralStore.DEFAULT_MAX_ENTRIES;
+ Duration stateNonceTtl = InMemoryLoginStateStore.DEFAULT_TTL;
+ int ephemeralMaxEntries = InMemoryLoginStateStore.DEFAULT_MAX_ENTRIES;
Duration sessionTtl = DEFAULT_SESSION_TTL;
String cookieName = DEFAULT_COOKIE_NAME;
boolean cookieSecure = true;
@@ -300,13 +301,54 @@ public class OidcRelyingParty {
}
/**
- * Sets the single-use TTL for the {@code state} / {@code nonce} store. Defaults to 5 minutes.
+ * Sets the single-use TTL the framework enforces for the login-state ({@code state} / {@code nonce} /
+ * PKCE {@code code_verifier}) association. Defaults to 5 minutes.
*
- * @param value The TTL. Must be positive and not exceed 30 minutes.
+ * <p>
+ * This is the RP-enforced policy applied to <b>every</b> consumed record — both the default
+ * {@link InMemoryLoginStateStore} and any custom {@link #loginStateStore(LoginStateStore) injected store}.
+ * The framework mints {@code expiresAt = createdAt + stateNonceTtl} on the record and re-checks it on
+ * consume, so the ceiling holds even for a shared store that fails to expire an entry. The
+ * {@code (0, }{@link LoginStateStore#MAX_TTL}{@code ]} cap is enforced here (not only in the default
+ * impl's constructor, which is skipped when a custom store is injected).
+ *
+ * @param value The TTL. Must be positive and not exceed {@link LoginStateStore#MAX_TTL} (30 minutes).
* @return This object.
*/
public Builder stateNonceTtl(Duration value) {
- stateNonceTtl = assertArgNotNull("value", value);
+ assertArgNotNull("value", value);
+ assertArg(!value.isZero() && !value.isNegative(), "stateNonceTtl must be positive");
+ assertArg(value.compareTo(LoginStateStore.MAX_TTL) <= 0, "stateNonceTtl must not exceed 30 minutes (was %s)", value);
+ stateNonceTtl = value;
+ return this;
+ }
+
+ /**
+ * Sets an optional custom {@link LoginStateStore} for the pending-login ({@code state} →
+ * {@code nonce} / PKCE {@code code_verifier} / redirect) association.
+ *
+ * <p>
+ * Optional (unlike {@link #sessionStore(SessionStore)}): when omitted the RP uses the shipped per-JVM
+ * {@link InMemoryLoginStateStore}, and behavior is identical to a single-node deployment. Supply a
+ * shared (Redis / JDBC) implementation to support a clustered deployment where {@code /login} and
+ * {@code /callback} may land on different nodes without sticky sessions — the same motivation as a
+ * distributed {@link SessionStore}.
+ *
+ * <p>
+ * <b>The store is secret-bearing</b> ({@code code_verifier} + {@code nonce}); read the
+ * {@link LoginStateStore} security contract before implementing one (TLS + locked-down ACL floor,
+ * encryption-at-rest recommended, never log the payload, atomic {@code GETDEL}/Lua single-use consume).
+ * The framework retains authority over the security-critical decisions regardless of the store: it
+ * re-validates {@code redirectTarget} via {@code safeRelativePath} and re-checks {@code expiresAt}
+ * against its own {@link #clock(Clock) clock} on consume, and enforces the
+ * {@link #stateNonceTtl(Duration)} ceiling — the store persists the record and enforces single-use,
+ * but is treated as a blob store, not a trusted oracle.
+ *
+ * @param value The login-state store. Must not be <jk>null</jk>.
+ * @return This object.
+ */
+ public Builder loginStateStore(LoginStateStore value) {
+ loginStateStore = assertArgNotNull("value", value);
return this;
}
@@ -458,7 +500,14 @@ public class OidcRelyingParty {
}
/**
- * Overrides the clock used for session expiry + the ephemeral store. Useful in tests.
+ * Overrides the clock used for session expiry and the login-state store. Useful in tests.
+ *
+ * <p>
+ * The login-state {@code expiresAt} is both minted (on {@code startLogin}) and re-checked (on
+ * {@code completeLogin}) against this clock. In a clustered deployment with a shared
+ * {@link #loginStateStore(LoginStateStore) LoginStateStore}, the ceiling is therefore only as
+ * trustworthy as clock synchronization across nodes; a backend {@code EXPIRE}/TTL is a GC hint, not the
+ * authoritative expiry decision.
*
* @param value The clock. Must not be <jk>null</jk>.
* @return This object.
@@ -508,7 +557,8 @@ public class OidcRelyingParty {
private final JWKSource<SecurityContext> injectedJwkSource;
private final Set<String> userInfoClaims;
private final Clock clock;
- private final EphemeralStore ephemeralStore;
+ private final Duration stateNonceTtl;
+ private final LoginStateStore loginStateStore;
@SuppressWarnings({
"java:S3077" // Publish-once cache: assigned once under double-checked locking in metadata(); the OidcMetadata payload is fully built before assignment, so volatile safe-publication is sufficient.
@@ -564,7 +614,10 @@ public class OidcRelyingParty {
this.injectedJwkSource = b.jwkSource;
this.userInfoClaims = u(cp(b.userInfoClaims));
this.clock = b.clock;
- this.ephemeralStore = new EphemeralStore(b.stateNonceTtl, b.ephemeralMaxEntries, b.clock);
+ this.stateNonceTtl = b.stateNonceTtl;
+ this.loginStateStore = b.loginStateStore != null
+ ? b.loginStateStore
+ : new InMemoryLoginStateStore(b.stateNonceTtl, b.ephemeralMaxEntries, b.clock);
}
//-----------------------------------------------------------------------------------------------------------------
@@ -588,7 +641,16 @@ public class OidcRelyingParty {
var verifier = new CodeVerifier();
var challenge = CodeChallenge.compute(CodeChallengeMethod.S256, verifier);
var redirectTarget = safeRelativePath(req.getParameter("redirect"));
- ephemeralStore.store(state, nonce, verifier.getValue(), redirectTarget);
+ var createdAt = clock.instant();
+ var pending = new LoginStateStore.PendingLogin(nonce, verifier.getValue(), redirectTarget, createdAt, createdAt.plus(stateNonceTtl));
+ try {
+ loginStateStore.store(state, pending);
+ } catch (RuntimeException e) {
+ // Fail closed before any authorization URL is issued. Do NOT chain the raw backend exception as the
+ // cause: a shared-store backend may echo the state/code_verifier into its own message, and
+ // AuthenticationException publishes the cause via getCause(), so a raw chain would re-expose the secret.
+ throw new AuthenticationException("OIDC login-state store unavailable");
+ }
var authUrl = codeFlow().buildAuthenticationUrl(state, challenge, nonce, authenticationRequestCustomizer);
noStore(res);
res.sendRedirect(authUrl.toString());
@@ -626,9 +688,23 @@ public class OidcRelyingParty {
var code = success.getAuthorizationCode().getValue();
var state = success.getState() == null ? null : success.getState().getValue(); // HTT: null state branch: AuthCode response always carries state per PKCE flow; null branch is defensive dead code
- var pending = ephemeralStore.consume(state).orElseThrow(
+ Optional<LoginStateStore.PendingLogin> consumed;
+ try {
+ consumed = loginStateStore.consume(state);
+ } catch (RuntimeException e) {
+ // Infra failure (backend unreachable) is distinct from a CSRF/replay miss; reject as unavailable
+ // without leaking the payload (do not chain the raw backend exception as cause).
+ throw new AuthenticationException("OIDC login-state store unavailable");
+ }
+ var pending = consumed.orElseThrow(
() -> new AuthenticationException("OIDC callback state is missing, expired, or already used"));
+ // Fail-closed expiry re-check BEFORE the token exchange: the framework owns the TTL ceiling independently
+ // of the store, so an honest store that failed to expire an over-age record cannot extend usability. Null
+ // createdAt/expiresAt (a foreign/deserialized blob) is treated as expired, not an NPE->500.
+ if (pending.createdAt() == null || pending.expiresAt() == null || ! clock.instant().isBefore(pending.expiresAt()))
+ throw new AuthenticationException("OIDC callback state is missing, expired, or already used");
+
OAuthToken token;
try {
token = codeFlow().exchange(code, new CodeVerifier(pending.codeVerifier()));
@@ -658,7 +734,9 @@ public class OidcRelyingParty {
var cookieValue = sessionStore.createSessionCookieValue(session);
noStore(res);
res.addHeader("Set-Cookie", buildSetCookie(cookieValue, sessionTtl.toSeconds()));
- res.sendRedirect(or(pending.redirectTarget(), postLoginRedirect));
+ // Re-validate the redirect target on consume: a writable/compromised shared store cannot inject an open
+ // redirect. A failing re-validation falls back to the operator-configured default (not itself re-sanitized).
+ res.sendRedirect(or(safeRelativePath(pending.redirectTarget()), postLoginRedirect));
}
/**
diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore_Test.java b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore_Test.java
similarity index 51%
rename from juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore_Test.java
rename to juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore_Test.java
index 15ad79cfe4..e53ec7f20b 100644
--- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/EphemeralStore_Test.java
+++ b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemoryLoginStateStore_Test.java
@@ -21,29 +21,39 @@ import static org.junit.jupiter.api.Assertions.*;
import java.time.*;
import org.apache.juneau.*;
+import org.apache.juneau.rest.server.auth.oidc.rp.LoginStateStore.PendingLogin;
+import org.apache.juneau.rest.server.auth.oidc.rp.OidcTestSupport.MutableClock;
import org.junit.jupiter.api.*;
/**
- * Tests for {@link EphemeralStore} — single-use, TTL-bounded, size-bounded state/nonce storage.
+ * Tests for {@link InMemoryLoginStateStore} — the shipped single-node, single-use, TTL-bounded,
+ * size-bounded default {@link LoginStateStore}.
*
* @since 10.0.0
*/
@SuppressWarnings({
"java:S5778" // assertThrows lambdas with chained calls; intermediate invocations do not throw in practice
})
-class EphemeralStore_Test extends TestBase {
+class InMemoryLoginStateStore_Test extends TestBase {
// Fixed clock seam: these cases don't depend on wall-clock time, so a deterministic clock
// replaces the system clock (java:S8692) without changing behavior.
private static final Clock CLOCK = Clock.fixed(Instant.parse("2026-01-01T00:00:00Z"), ZoneOffset.UTC);
+ private static final Duration TTL = Duration.ofMinutes(5);
- private static EphemeralStore store(Clock clock) {
- return new EphemeralStore(Duration.ofMinutes(5), 100, clock);
+ private static InMemoryLoginStateStore store(Clock clock) {
+ return new InMemoryLoginStateStore(TTL, 100, clock);
+ }
+
+ /** Builds a framework-shaped record stamped from the given clock (createdAt=now, expiresAt=now+TTL). */
+ private static PendingLogin pending(Clock clock, String nonce, String verifier, String redirect) {
+ var now = clock.instant();
+ return new PendingLogin(nonce, verifier, redirect, now, now.plus(TTL));
}
@Test void a01_storeThenConsume_roundTrips() {
var s = store(CLOCK);
- s.store("state-1", "nonce-1", "verifier-1", "/dashboard");
+ s.store("state-1", pending(CLOCK, "nonce-1", "verifier-1", "/dashboard"));
var p = s.consume("state-1");
assertTrue(p.isPresent());
assertEquals("nonce-1", p.get().nonce());
@@ -53,7 +63,7 @@ class EphemeralStore_Test extends TestBase {
@Test void a02_consume_isSingleUse() {
var s = store(CLOCK);
- s.store("state-1", "nonce-1", "verifier-1", null);
+ s.store("state-1", pending(CLOCK, "nonce-1", "verifier-1", null));
assertTrue(s.consume("state-1").isPresent());
assertTrue(s.consume("state-1").isEmpty(), "second consume of same state must miss (replay defense)");
}
@@ -71,56 +81,65 @@ class EphemeralStore_Test extends TestBase {
@Test void b01_expiredEntry_isMissed() {
var base = Instant.parse("2026-01-01T00:00:00Z");
var clock = new MutableClock(base);
- var s = new EphemeralStore(Duration.ofMinutes(5), 100, clock);
- s.store("state-1", "nonce-1", "verifier-1", null);
+ var s = new InMemoryLoginStateStore(TTL, 100, clock);
+ s.store("state-1", pending(clock, "nonce-1", "verifier-1", null));
clock.advance(Duration.ofMinutes(6));
- assertTrue(s.consume("state-1").isEmpty(), "entry past TTL must be treated as a miss");
+ assertTrue(s.consume("state-1").isEmpty(), "entry past expiresAt must be treated as a miss");
}
@Test void b02_notYetExpired_survives() {
var base = Instant.parse("2026-01-01T00:00:00Z");
var clock = new MutableClock(base);
- var s = new EphemeralStore(Duration.ofMinutes(5), 100, clock);
- s.store("state-1", "nonce-1", "verifier-1", null);
+ var s = new InMemoryLoginStateStore(TTL, 100, clock);
+ s.store("state-1", pending(clock, "nonce-1", "verifier-1", null));
clock.advance(Duration.ofMinutes(4));
assertTrue(s.consume("state-1").isPresent());
}
+ /** A stored record with a {@code null} expiresAt fails closed as expired (defensive against a foreign blob). */
+ @Test void b03_nullExpiresAt_isTreatedAsExpired() {
+ var s = store(CLOCK);
+ var now = CLOCK.instant();
+ s.store("state-1", new PendingLogin("nonce-1", "verifier-1", null, now, null));
+ assertTrue(s.consume("state-1").isEmpty(), "null expiresAt must fail closed as expired");
+ }
+
@Test void c01_sizeCap_evictsEldest() {
- var s = new EphemeralStore(Duration.ofMinutes(5), 3, CLOCK);
- s.store("s1", "n", "v", null);
- s.store("s2", "n", "v", null);
- s.store("s3", "n", "v", null);
- s.store("s4", "n", "v", null); // evicts s1
+ var s = new InMemoryLoginStateStore(TTL, 3, CLOCK);
+ s.store("s1", pending(CLOCK, "n", "v", null));
+ s.store("s2", pending(CLOCK, "n", "v", null));
+ s.store("s3", pending(CLOCK, "n", "v", null));
+ s.store("s4", pending(CLOCK, "n", "v", null)); // evicts s1
assertEquals(3, s.size());
assertTrue(s.consume("s1").isEmpty());
assertTrue(s.consume("s4").isPresent());
}
@Test void d01_rejectsNonPositiveTtl() {
- assertThrows(IllegalArgumentException.class, () -> new EphemeralStore(Duration.ZERO, 100, CLOCK));
+ assertThrows(IllegalArgumentException.class, () -> new InMemoryLoginStateStore(Duration.ZERO, 100, CLOCK));
+ }
+
+ /** Negative TTL rejected — second bytecode branch of {@code !isZero && !isNegative}. */
+ @Test void d05_rejectsNegativeTtl() {
+ assertThrows(IllegalArgumentException.class, () -> new InMemoryLoginStateStore(Duration.ofSeconds(-1), 100, CLOCK));
}
@Test void d02_rejectsTtlAbove30Minutes() {
- assertThrows(IllegalArgumentException.class, () -> new EphemeralStore(Duration.ofMinutes(31), 100, CLOCK));
+ assertThrows(IllegalArgumentException.class, () -> new InMemoryLoginStateStore(Duration.ofMinutes(31), 100, CLOCK));
}
@Test void d03_rejectsNonPositiveMaxEntries() {
- assertThrows(IllegalArgumentException.class, () -> new EphemeralStore(Duration.ofMinutes(5), 0, CLOCK));
+ assertThrows(IllegalArgumentException.class, () -> new InMemoryLoginStateStore(Duration.ofMinutes(5), 0, CLOCK));
}
@Test void d04_rejectsBlankStateOnStore() {
var s = store(CLOCK);
- assertThrows(IllegalArgumentException.class, () -> s.store("", "n", "v", null));
+ var p = pending(CLOCK, "n", "v", null);
+ assertThrows(IllegalArgumentException.class, () -> s.store("", p));
}
- /** A test clock the test can advance manually. */
- static final class MutableClock extends Clock {
- private Instant now;
- MutableClock(Instant start) { now = start; }
- void advance(Duration d) { now = now.plus(d); }
- @Override public ZoneId getZone() { return ZoneOffset.UTC; }
- @Override public Clock withZone(ZoneId zone) { return this; }
- @Override public Instant instant() { return now; }
+ @Test void d06_rejectsNullPendingOnStore() {
+ var s = store(CLOCK);
+ assertThrows(IllegalArgumentException.class, () -> s.store("state-1", null));
}
}
diff --git a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemorySessionStore_Test.java b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemorySessionStore_Test.java
index 9731941e6b..52a823c7c8 100644
--- a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemorySessionStore_Test.java
+++ b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/InMemorySessionStore_Test.java
@@ -103,7 +103,7 @@ class InMemorySessionStore_Test extends TestBase {
@Test void c01_expiredSession_missedAndRemoved() {
var base = Instant.parse("2026-01-01T00:00:00Z");
- var clock = new EphemeralStore_Test.MutableClock(base);
+ var clock = new OidcTestSupport.MutableClock(base);
var store = new InMemorySessionStore(100, clock);
store.createSessionCookieValue(session("id-1", "alice", "sess-1", base));
clock.advance(Duration.ofHours(9));
diff --git a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/LoginStateStore_Test.java b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/LoginStateStore_Test.java
new file mode 100644
index 0000000000..ad6d492730
--- /dev/null
+++ b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/LoginStateStore_Test.java
@@ -0,0 +1,373 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juneau.rest.server.auth.oidc.rp;
+
+import static org.apache.juneau.rest.server.auth.oidc.rp.OidcTestSupport.*;
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.net.*;
+import java.time.*;
+import java.util.*;
+import java.util.concurrent.*;
+import java.util.concurrent.atomic.*;
+
+import org.apache.juneau.*;
+import org.apache.juneau.commons.lang.*;
+import org.apache.juneau.rest.mock.*;
+import org.apache.juneau.rest.server.auth.*;
+import org.apache.juneau.rest.server.auth.oidc.rp.LoginStateStore.PendingLogin;
+import org.junit.jupiter.api.*;
+
+import com.nimbusds.jose.jwk.*;
+
+/**
+ * Security gates for the injectable {@link LoginStateStore} SPI: injected-store wiring, fail-closed store
+ * outages, consume-time redirect re-validation, the framework expiry ceiling (enforced before the token
+ * exchange), {@link PendingLogin} redaction, and the default impl's concurrent single-use race.
+ *
+ * <p>
+ * These are the substance ("red-on-broken") gates for TODO-397: each is constructed so a no-op setter,
+ * a mis-ordered check, or a class-only assertion would fail it. The recording fake plus distinctive
+ * hardcoded values ensure the injected instance (not the hidden default store) is actually exercised.
+ *
+ * @since 10.0.0
+ */
+@SuppressWarnings({
+ "java:S8692", // Nimbus oauth2-oidc-sdk exposes no clock hook on the ID-token path; the login-state expiry re-check IS clock-injectable and is tested deterministically.
+ "java:S1130", // Test methods declare throws Exception for checked exceptions MockServletResponse may propagate; declarations are intentionally broad.
+ "java:S5778", // assertThrows lambdas with chained calls; intermediate invocations do not throw in practice.
+ "java:S5976", // Similar-shaped gates assert distinct security outcomes; parameterizing would obscure intent.
+ "resource" // Closeable StubIdp fixture; lifecycle managed by @BeforeEach/@AfterEach, not a real leak.
+})
+class LoginStateStore_Test extends TestBase {
+
+ private static final String CID = "web-app";
+ private static final URI REDIRECT_URI = URI.create("https://app.example.com/auth/callback");
+ private static final URI AUTHZ = URI.create("https://stub-idp.example.com/authorize");
+ private static final URI END_SESSION = URI.create("https://stub-idp.example.com/logout");
+
+ // A fixed clock shared by the RP AND the fixture timestamps (mandatory — mixing clocks makes a correct
+ // expiry check look like a failure). The ID-token exp uses wall-clock time internally in Nimbus, so ID
+ // tokens are still signed with Instant.now().
+ private static final Instant NOW = Instant.parse("2026-06-01T12:00:00Z");
+ private static final Clock CLOCK = Clock.fixed(NOW, ZoneOffset.UTC);
+
+ // Distinctive hardcoded values the hidden default store could NOT have produced (S8 / gate 1).
+ private static final String FAKE_NONCE = "fake-nonce-distinctive-8f3a";
+ private static final String FAKE_VERIFIER = "fakeVERIFIERfakeVERIFIERfakeVERIFIERfakeVER1"; // 44 chars, PKCE-valid [A-Za-z0-9]
+ private static final String FAKE_REDIRECT = "/from-fake-store";
+
+ // A distinctive secret marker embedded in a backend exception message (gate 4 / gate 5), so "the error
+ // surface does not echo the secret" is an observable assertion, not vacuously true.
+ private static final String LEAK_SENTINEL = "code_verifier=SUPERSECRET-LEAK-SENTINEL-9x7q";
+
+ private StubIdp idp;
+ private RSAKey key;
+
+ @BeforeEach void setup() throws Exception {
+ key = generateRsa("k1");
+ idp = new StubIdp();
+ }
+
+ @AfterEach void teardown() {
+ if (idp != null)
+ idp.close();
+ }
+
+ private OidcRelyingParty rp(LoginStateStore loginStateStore) {
+ return OidcRelyingParty.create()
+ .metadata(idp.metadata(AUTHZ, END_SESSION))
+ .clientId(CID)
+ .clientSecret("client-secret")
+ .redirectUri(REDIRECT_URI)
+ .scope("openid", "profile")
+ .sessionStore(InMemorySessionStore.create())
+ .jwkSet(publicJwks(key))
+ .loginStateStore(loginStateStore)
+ .clock(CLOCK)
+ .build();
+ }
+
+ private String startLoginGetState(OidcRelyingParty rp) throws Exception {
+ var req = MockServletRequest.create("GET", "/auth/login");
+ var res = MockServletResponse.create();
+ rp.startLogin(req, res);
+ var loc = res.getHeader("Location");
+ assertNotNull(loc, "startLogin must redirect to the IdP");
+ return queryParam(loc, "state");
+ }
+
+ private MockServletResponse completeLogin(OidcRelyingParty rp, String state) throws Exception {
+ var req = MockServletRequest.create("GET", REDIRECT_URI + "?code=abc123&state=" + state);
+ var res = MockServletResponse.create();
+ rp.completeLogin(req, res);
+ return res;
+ }
+
+ private void signIdTokenWith(String nonce) throws Exception {
+ idp.idToken = signIdToken(key, idp.issuer, CID, "alice", "sess-1", nonce, Instant.now(), Duration.ofMinutes(5),
+ Map.of("scope", "openid profile"));
+ }
+
+ private static PendingLogin pending(String redirect, Instant createdAt, Instant expiresAt) {
+ return new PendingLogin(FAKE_NONCE, FAKE_VERIFIER, redirect, createdAt, expiresAt);
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // A recording fake LoginStateStore configurable per gate.
+ //-----------------------------------------------------------------------------------------------------------------
+
+ static final class FakeStore implements LoginStateStore {
+ final IntegerHolder storeCount = IntegerHolder.create();
+ final IntegerHolder consumeCount = IntegerHolder.create();
+ volatile String lastStoreState;
+ volatile String lastConsumeState;
+ volatile Optional<PendingLogin> lastConsumeReturned = Optional.empty();
+
+ RuntimeException storeThrows;
+ RuntimeException consumeThrows;
+ Optional<PendingLogin> consumeResult = Optional.empty();
+
+ @Override public void store(String state, PendingLogin p) {
+ storeCount.increment();
+ lastStoreState = state;
+ if (storeThrows != null)
+ throw storeThrows;
+ }
+
+ @Override public Optional<PendingLogin> consume(String state) {
+ consumeCount.increment();
+ lastConsumeState = state;
+ if (consumeThrows != null)
+ throw consumeThrows;
+ lastConsumeReturned = consumeResult;
+ return consumeResult;
+ }
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // Gate 1 — injected store honored on both legs, with observable distinctive outputs + mandatory call-counts.
+ //-----------------------------------------------------------------------------------------------------------------
+
+ @Test void g01_injectedStore_honoredOnBothLegs() throws Exception {
+ var fake = new FakeStore();
+ fake.consumeResult = Optional.of(pending(FAKE_REDIRECT, NOW, NOW.plus(Duration.ofMinutes(5))));
+ var rp = rp(fake);
+
+ var state = startLoginGetState(rp);
+ assertTrue(fake.storeCount.is(1), "store must be invoked once on the injected instance");
+ assertEquals(state, fake.lastStoreState, "store must receive the generated state");
+
+ signIdTokenWith(FAKE_NONCE); // sign with the fake's distinctive nonce; a mismatch fails validation
+ var res = completeLogin(rp, state);
+
+ assertTrue(fake.consumeCount.is(1), "consume must be invoked once on the injected instance");
+ assertEquals(state, fake.lastConsumeState, "consume must receive the callback state");
+ assertNotNull(cookieValue(res.getHeader("Set-Cookie")), "login must succeed (session cookie set)");
+ assertEquals(FAKE_REDIRECT, res.getHeader("Location"), "Location must be the fake's distinctive redirectTarget");
+ // The verifier exchanged with the IdP is the fake's distinctive one (record-only capture on the stub).
+ assertEquals(FAKE_VERIFIER, idp.lastCodeVerifier, "the fake's distinctive code_verifier must be exchanged");
+ // Login succeeding with an ID token signed by FAKE_NONCE proves the fake's nonce was the one validated.
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // Gate 3 — consume miss on the INJECTED store is the CSRF/replay decision (fails against a no-op setter).
+ //-----------------------------------------------------------------------------------------------------------------
+
+ @Test void g03_consumeMiss_onInjectedStore_rejected() throws Exception {
+ var fake = new FakeStore();
+ fake.consumeResult = Optional.empty(); // injected consume misses
+ var rp = rp(fake);
+
+ // startLogin FIRST: a hidden default store would hold a live entry and login would succeed if the fake
+ // were ignored. So a no-op setter is caught two ways below (consume count stays 0; no miss).
+ var state = startLoginGetState(rp);
+ assertTrue(fake.storeCount.is(1));
+ assertEquals(state, fake.lastStoreState);
+
+ signIdTokenWith(FAKE_NONCE);
+ var ex = assertThrows(AuthenticationException.class, () -> completeLogin(rp, state));
+
+ assertTrue(fake.consumeCount.is(1), "the injected consume must be the CSRF/replay decision point");
+ assertEquals(state, fake.lastConsumeState);
+ assertTrue(ex.getMessage().contains("missing, expired, or already used"),
+ "must reject with the consume-miss message, got: " + ex.getMessage());
+ assertEquals(0, idp.tokenHits.get(), "a consume miss must not reach the token exchange");
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // Gate 4 — store outage on startLogin → fail closed (no redirect) + no secret leak in the error surface.
+ //-----------------------------------------------------------------------------------------------------------------
+
+ @Test void g04_storeOutage_onStartLogin_failsClosed_noLeak() {
+ var fake = new FakeStore();
+ fake.storeThrows = new RuntimeException("backend echoed " + LEAK_SENTINEL);
+ var rp = rp(fake);
+
+ var req = MockServletRequest.create("GET", "/auth/login");
+ var res = MockServletResponse.create();
+ var ex = assertThrows(AuthenticationException.class, () -> rp.startLogin(req, res));
+
+ assertNull(res.getHeader("Location"), "no authorization redirect may be issued on a store outage (fail closed)");
+ assertFalse(String.valueOf(ex.getMessage()).contains(LEAK_SENTINEL),
+ "the propagated error must not echo the backend secret");
+ assertNull(ex.getCause(), "the raw backend exception (carrying the secret) must not be chained as the cause");
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // Gate 5 — consume outage → rejected with the DISTINCT "unavailable" message (vs the CSRF-miss message).
+ //-----------------------------------------------------------------------------------------------------------------
+
+ @Test void g05_consumeOutage_rejectedAsUnavailable() throws Exception {
+ var fake = new FakeStore();
+ fake.consumeThrows = new RuntimeException("backend down " + LEAK_SENTINEL);
+ var rp = rp(fake);
+
+ var state = startLoginGetState(rp);
+ signIdTokenWith(FAKE_NONCE);
+ var ex = assertThrows(AuthenticationException.class, () -> completeLogin(rp, state));
+
+ assertTrue(ex.getMessage().contains("unavailable"),
+ "consume outage must be rejected as unavailable, got: " + ex.getMessage());
+ assertFalse(ex.getMessage().contains("missing, expired, or already used"),
+ "the infra-outage signal must be distinct from the CSRF/replay miss");
+ assertFalse(String.valueOf(ex.getMessage()).contains(LEAK_SENTINEL), "must not echo the backend secret");
+ assertEquals(0, idp.tokenHits.get(), "a consume outage must not reach the token exchange");
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // Gate 6 — consume-time redirect re-validation: an evil redirectTarget from the store is discarded → "/".
+ //-----------------------------------------------------------------------------------------------------------------
+
+ @Test void g06_evilRedirectTarget_fromStore_isDiscarded() throws Exception {
+ var fake = new FakeStore();
+ fake.consumeResult = Optional.of(pending("https://evil.example.com", NOW, NOW.plus(Duration.ofMinutes(5))));
+ var rp = rp(fake);
+
+ var state = startLoginGetState(rp);
+ signIdTokenWith(FAKE_NONCE);
+ var res = completeLogin(rp, state);
+
+ assertEquals("/", res.getHeader("Location"),
+ "a writable-store absolute redirect must be re-validated away in favor of the post-login default");
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // Gate 7 — framework expiry ceiling beats an over-age store record, BEFORE the token exchange.
+ //-----------------------------------------------------------------------------------------------------------------
+
+ @Test void g07_overTtlRecord_rejected_beforeExchange() throws Exception {
+ // Past its minted expiresAt (now-1min) but well within MAX_TTL of createdAt (only 6 min old): a
+ // MAX_TTL-only implementation would accept it, so this pins the minted-expiresAt re-check.
+ var overTtl = pending(FAKE_REDIRECT, NOW.minus(Duration.ofMinutes(6)), NOW.minus(Duration.ofMinutes(1)));
+ var fake = new FakeStore();
+ fake.consumeResult = Optional.of(overTtl);
+ var rp = rp(fake);
+
+ var state = startLoginGetState(rp);
+ signIdTokenWith(FAKE_NONCE); // would-succeed setup: the ONLY thing between success and rejection is the expiry re-check
+ var ex = assertThrows(AuthenticationException.class, () -> completeLogin(rp, state));
+
+ // (2) message-specific, not class-only — distinct from the post-exchange throws.
+ assertTrue(ex.getMessage().contains("missing, expired, or already used"),
+ "over-age record must be rejected with the expiry/miss message, got: " + ex.getMessage());
+ // (3) token endpoint never hit — proves the re-check runs BEFORE codeFlow().exchange.
+ assertEquals(0, idp.tokenHits.get(), "the expiry re-check must run before the token exchange (code not burned)");
+ // consume-miss disambiguation — the present over-TTL record WAS consumed once, then rejected for age.
+ assertTrue(fake.consumeCount.is(1), "the injected consume must have been invoked once");
+ assertEquals(state, fake.lastConsumeState);
+ assertTrue(fake.lastConsumeReturned.isPresent(), "the record was present (not a consume miss)");
+ assertEquals(NOW.minus(Duration.ofMinutes(1)), fake.lastConsumeReturned.get().expiresAt(),
+ "the rejected record is the over-TTL one the fake returned");
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // Gate 7b — null timestamps fail closed (rejected as expired, NOT an NPE→500).
+ //-----------------------------------------------------------------------------------------------------------------
+
+ @Test void g07b_nullExpiresAt_failsClosed() throws Exception {
+ var fake = new FakeStore();
+ fake.consumeResult = Optional.of(pending(FAKE_REDIRECT, NOW, null));
+ var rp = rp(fake);
+
+ var state = startLoginGetState(rp);
+ signIdTokenWith(FAKE_NONCE);
+ var ex = assertThrows(AuthenticationException.class, () -> completeLogin(rp, state));
+
+ assertTrue(ex.getMessage().contains("missing, expired, or already used"),
+ "null expiresAt must be treated as expired, got: " + ex.getMessage());
+ assertEquals(0, idp.tokenHits.get(), "must reject before the token exchange");
+ }
+
+ @Test void g07c_nullCreatedAt_failsClosed() throws Exception {
+ var fake = new FakeStore();
+ fake.consumeResult = Optional.of(pending(FAKE_REDIRECT, null, NOW.plus(Duration.ofMinutes(5))));
+ var rp = rp(fake);
+
+ var state = startLoginGetState(rp);
+ signIdTokenWith(FAKE_NONCE);
+ var ex = assertThrows(AuthenticationException.class, () -> completeLogin(rp, state));
+
+ assertTrue(ex.getMessage().contains("missing, expired, or already used"),
+ "null createdAt must be treated as expired, got: " + ex.getMessage());
+ assertEquals(0, idp.tokenHits.get(), "must reject before the token exchange");
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // Gate 8 — PendingLogin.toString() redacts both secrets.
+ //-----------------------------------------------------------------------------------------------------------------
+
+ @Test void g08_pendingLogin_toString_redactsSecrets() {
+ var p = new PendingLogin("nonce123", "verifierXYZ", "/home", NOW, NOW.plus(Duration.ofMinutes(5)));
+ var s = p.toString();
+ assertFalse(s.contains("nonce123"), "nonce must be redacted");
+ assertFalse(s.contains("verifierXYZ"), "code_verifier must be redacted");
+ assertTrue(s.contains("<redacted>"), "toString must mark redacted secrets");
+ assertTrue(s.contains("/home"), "non-secret redirectTarget may be shown for diagnostics");
+ }
+
+ //-----------------------------------------------------------------------------------------------------------------
+ // Gate 9 — default impl atomic single-use survives a concurrent-consume race (in-process only).
+ //-----------------------------------------------------------------------------------------------------------------
+
+ @Test void g09_defaultStore_concurrentConsume_isSingleUse() throws Exception {
+ var store = new InMemoryLoginStateStore(Duration.ofMinutes(5), 1000, CLOCK);
+ var pool = Executors.newFixedThreadPool(2);
+ try {
+ for (var i = 0; i < 300; i++) {
+ var stateKey = "state-" + i;
+ store.store(stateKey, pending(null, NOW, NOW.plus(Duration.ofMinutes(5))));
+ var present = new AtomicInteger();
+ var start = new CountDownLatch(1);
+ Callable<Void> task = () -> {
+ start.await();
+ if (store.consume(stateKey).isPresent())
+ present.incrementAndGet();
+ return null;
+ };
+ var f1 = pool.submit(task);
+ var f2 = pool.submit(task);
+ start.countDown();
+ f1.get();
+ f2.get();
+ assertEquals(1, present.get(), "exactly one concurrent consume may win the single-use race");
+ }
+ } finally {
+ pool.shutdownNow();
+ }
+ }
+}
diff --git a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcRelyingPartyBuilder_Test.java b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcRelyingPartyBuilder_Test.java
index 31119c3f9e..48d14adc1a 100644
--- a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcRelyingPartyBuilder_Test.java
+++ b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcRelyingPartyBuilder_Test.java
@@ -20,8 +20,10 @@ import static org.junit.jupiter.api.Assertions.*;
import java.net.*;
import java.time.*;
+import java.util.*;
import org.apache.juneau.*;
+import org.apache.juneau.rest.server.auth.oidc.rp.LoginStateStore.PendingLogin;
import org.junit.jupiter.api.*;
/**
@@ -134,4 +136,56 @@ class OidcRelyingPartyBuilder_Test extends TestBase {
.build();
assertNotNull(rp);
}
+
+ // -----------------------------------------------------------------------------------------------------------------
+ // F: stateNonceTtl (0, MAX_TTL] cap enforced on a site that runs for CUSTOM stores + loginStateStore hook.
+ // -----------------------------------------------------------------------------------------------------------------
+
+ // A TTL-oblivious no-op store, constructed independently of any TTL value (S1): it does NOT implement the
+ // (0, MAX_TTL] cap, so a rejection with this store injected proves the cap comes from the builder side, not
+ // from the default InMemoryLoginStateStore constructor (which is skipped when a custom store is supplied).
+ private static final LoginStateStore NOOP_STORE = new LoginStateStore() {
+ @Override public void store(String state, PendingLogin pending) { /* no-op */ }
+ @Override public Optional<PendingLogin> consume(String state) { return Optional.empty(); }
+ };
+
+ /**
+ * Over-30-min TTL rejected even when a TTL-oblivious custom store is injected — proves the cap runs on the
+ * custom-store path (Phase 2 gate). On main the cap lives only in the default impl ctor, which a custom
+ * store skips, so this over-TTL config would build successfully → gate red.
+ */
+ @Test void f01_stateNonceTtl_over30Min_rejected_customStorePath() {
+ assertThrows(IllegalArgumentException.class,
+ () -> base().loginStateStore(NOOP_STORE).stateNonceTtl(Duration.ofMinutes(31)).build());
+ }
+
+ /** Zero TTL rejected on the custom-store path (fail-closed = immediately-expired), sibling of the 31-min case. */
+ @Test void f02_stateNonceTtl_zero_rejected_customStorePath() {
+ assertThrows(IllegalArgumentException.class,
+ () -> base().loginStateStore(NOOP_STORE).stateNonceTtl(Duration.ZERO).build());
+ }
+
+ /** Negative TTL rejected — second bytecode branch of {@code !isZero && !isNegative}. */
+ @Test void f03_stateNonceTtl_negative_rejected() {
+ assertThrows(IllegalArgumentException.class, () -> base().stateNonceTtl(Duration.ofSeconds(-1)));
+ }
+
+ /** A valid TTL with a custom store is accepted. */
+ @Test void f04_stateNonceTtl_valid_customStore_accepted() {
+ assertDoesNotThrow(() -> base().loginStateStore(NOOP_STORE).stateNonceTtl(Duration.ofMinutes(10)).build());
+ }
+
+ /** The optional loginStateStore hook rejects null. */
+ @Test void f05_loginStateStore_null_rejected() {
+ assertThrows(IllegalArgumentException.class, () -> base().loginStateStore(null));
+ }
+
+ /**
+ * Over-30-min TTL rejected on the DEFAULT-store path too (S5 throw-site test). Same
+ * {@link IllegalArgumentException} type as on main; only the throw <i>site</i> moves (impl ctor at
+ * {@code build()} → the {@code stateNonceTtl(...)} setter), pinning the Decision-3 delta.
+ */
+ @Test void f06_stateNonceTtl_over30Min_rejected_defaultStorePath() {
+ assertThrows(IllegalArgumentException.class, () -> base().stateNonceTtl(Duration.ofMinutes(31)).build());
+ }
}
diff --git a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcTestSupport.java b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcTestSupport.java
index bc2c02c3e2..4385c0ac3f 100644
--- a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcTestSupport.java
+++ b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/OidcTestSupport.java
@@ -121,6 +121,19 @@ final class OidcTestSupport {
return null;
}
+ /** Extracts a single {@code application/x-www-form-urlencoded} field value from a request body (URL-decoded). */
+ static String formField(String body, String name) {
+ if (body == null)
+ return null;
+ for (var pair : body.split("&")) {
+ var i = pair.indexOf('=');
+ var k = i < 0 ? pair : pair.substring(0, i);
+ if (k.equals(name))
+ return i < 0 ? "" : URLDecoder.decode(pair.substring(i + 1), StandardCharsets.UTF_8);
+ }
+ return null;
+ }
+
/** Parses the cookie value out of a {@code Set-Cookie} header (the bit between {@code =} and the first {@code ;}). */
static String cookieValue(String setCookieHeader) {
if (setCookieHeader == null)
@@ -154,6 +167,16 @@ final class OidcTestSupport {
volatile boolean userInfoFail;
/** Extra claims served by {@code /userinfo}. */
volatile Map<String,Object> userInfo = new LinkedHashMap<>();
+ /**
+ * Number of times {@code /token} was invoked. An order proof for gate 7: a rejected {@code completeLogin}
+ * that never reaches {@code codeFlow().exchange} leaves this at {@code 0}.
+ */
+ final java.util.concurrent.atomic.AtomicInteger tokenHits = new java.util.concurrent.atomic.AtomicInteger();
+ /**
+ * The PKCE {@code code_verifier} observed on the most recent {@code /token} call. Record-only — the
+ * stub still succeeds with any verifier; this only lets a test assert the injected verifier was exchanged.
+ */
+ volatile String lastCodeVerifier;
StubIdp() throws IOException {
server = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0);
@@ -163,7 +186,9 @@ final class OidcTestSupport {
}
private void handleToken(HttpExchange ex) throws IOException {
+ tokenHits.incrementAndGet();
var body = new String(ex.getRequestBody().readAllBytes(), StandardCharsets.UTF_8);
+ lastCodeVerifier = formField(body, "code_verifier");
if (failToken) {
writeJson(ex, 400, "{\"error\":\"invalid_grant\"}");
return;
@@ -237,4 +262,14 @@ final class OidcTestSupport {
server.stop(0);
}
}
+
+ /** A test clock the test can advance manually. */
+ static final class MutableClock extends Clock {
+ private Instant now;
+ MutableClock(Instant start) { now = start; }
+ void advance(Duration d) { now = now.plus(d); }
+ @Override public ZoneId getZone() { return ZoneOffset.UTC; }
+ @Override public Clock withZone(ZoneId zone) { return this; }
+ @Override public Instant instant() { return now; }
+ }
}
diff --git a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/SignedCookieSessionStore_Test.java b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/SignedCookieSessionStore_Test.java
index 0bd532927d..033ff700d7 100644
--- a/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/SignedCookieSessionStore_Test.java
+++ b/juneau-rest/juneau-rest-server-auth-oidc-rp/src/test/java/org/apache/juneau/rest/server/auth/oidc/rp/SignedCookieSessionStore_Test.java
@@ -90,7 +90,7 @@ class SignedCookieSessionStore_Test extends TestBase {
@Test void c01_expiredCookie_isRejected() {
var base = Instant.parse("2026-01-01T00:00:00Z");
- var clock = new EphemeralStore_Test.MutableClock(base);
+ var clock = new OidcTestSupport.MutableClock(base);
var store = SignedCookieSessionStore.create().signingKey(KEY).clock(clock).build();
var cookie = store.createSessionCookieValue(session(base, null));
clock.advance(Duration.ofHours(9));
(juneau) 01/04: READY-378: Fail closed when SAML assertions lack a NotOnOrAfter bound
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit b0c2630caaecfd205130d8c4f3bc13e706be4944
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 13:40:18 2026 -0400
READY-378: Fail closed when SAML assertions lack a NotOnOrAfter bound
An assertion with no NotOnOrAfter had no replay-expiry bound, so its replay-cache
record could be evicted early and the assertion replayed. Reject such unbounded
assertions and retain replay records until the resolved NotOnOrAfter + clock skew.
---
.../server/auth/saml/SamlAssertionValidator.java | 51 ++++--
.../saml/SamlAssertionValidator_Branches_Test.java | 12 +-
.../SamlAssertionValidator_ReplayExpiry_Test.java | 202 +++++++++++++++++++++
3 files changed, 247 insertions(+), 18 deletions(-)
diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java b/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java
index 70cef30352..62044f1d9c 100644
--- a/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java
+++ b/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java
@@ -79,7 +79,10 @@ import net.shibboleth.shared.resolver.*;
* {@code InResponseTo}/{@code Address} expectations.
* <li><b>One-time use</b> — each assertion ID is recorded in a {@link ReplayCache} and a second
* presentation of the same ID is rejected. The check is fail-closed: if the cache cannot answer, the
- * assertion is rejected.
+ * assertion is rejected. An assertion whose validity window is unbounded — carrying neither a
+ * {@code Conditions/NotOnOrAfter} nor a bearer {@code SubjectConfirmationData/NotOnOrAfter} — is
+ * rejected outright, and the cache record is retained until the resolved {@code NotOnOrAfter} (plus clock
+ * skew) rather than a short default window that would later expire and re-open replay.
* <li><b>Encrypted assertions</b> — if the response contains
* {@code <EncryptedAssertion>}, a {@code decryptionCredential} must be configured.
* </ul>
@@ -134,12 +137,6 @@ public class SamlAssertionValidator {
/** XML-DSig namespace URI, used when scanning the signature DOM for reference digest methods. */
private static final String XMLSIG_NS = "http://www.w3.org/2000/09/xmldsig#";
- /**
- * Retention window applied to a one-time-use cache entry when the assertion carries no
- * {@code Conditions/NotOnOrAfter} to bound it (5 minutes).
- */
- private static final Duration DEFAULT_REPLAY_RETENTION = Duration.ofMinutes(5);
-
/** Default allowlist of XML signature algorithms. */
private static final Set<String> DEFAULT_ALGORITHMS = Set.of(
SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256,
@@ -814,7 +811,10 @@ public class SamlAssertionValidator {
var id = assertion.getID();
if (id == null || id.isEmpty())
throw rejectAssertion("assertion has no ID for single-use enforcement");
- var expiresAtMs = singleUseExpiry(assertion);
+ var notOnOrAfter = resolveNotOnOrAfter(assertion);
+ if (notOnOrAfter == null)
+ throw rejectAssertion("assertion has no NotOnOrAfter to bound one-time use");
+ var expiresAtMs = notOnOrAfter.plus(clockSkew).toEpochMilli();
var firstSeen = replayCache.checkAndRecord(id, expiresAtMs, ReplayCache.FailMode.FAIL_CLOSED,
e -> LOG.log(Level.WARNING, e, () -> "SAML assertion single-use check failed; rejecting (fail-closed)."));
if (! firstSeen)
@@ -822,11 +822,36 @@ public class SamlAssertionValidator {
.wwwAuthenticate("SAML error=\"assertion_replayed\"");
}
- private long singleUseExpiry(Assertion assertion) {
- var conditions = assertion.getConditions();
- if (conditions != null && conditions.getNotOnOrAfter() != null)
- return conditions.getNotOnOrAfter().plus(clockSkew).toEpochMilli();
- return clock.instant().plus(DEFAULT_REPLAY_RETENTION).toEpochMilli();
+ /**
+ * Resolves the effective {@code NotOnOrAfter} that bounds one-time use of the assertion.
+ *
+ * <p>
+ * The resolved value is the latest of the assertion's {@code Conditions/NotOnOrAfter} and any bearer
+ * {@code SubjectConfirmationData/NotOnOrAfter}. Returns <jk>null</jk> when the assertion carries neither
+ * — in that case its validity window is unbounded, so single-use cannot be enforced past a cache
+ * eviction and the caller rejects the assertion rather than inventing a short fallback that later re-opens
+ * replay. {@code <Conditions>} itself is guaranteed non-<jk>null</jk> here (a null {@code <Conditions>} is
+ * rejected earlier by {@link #validateConditions(Assertion)}).
+ *
+ * @param assertion The assertion.
+ * @return The latest bounding instant, or <jk>null</jk> if the assertion carries no {@code NotOnOrAfter}.
+ */
+ private static Instant resolveNotOnOrAfter(Assertion assertion) {
+ var result = assertion.getConditions().getNotOnOrAfter();
+ var subject = assertion.getSubject();
+ if (subject != null) {
+ for (var sc : subject.getSubjectConfirmations()) {
+ if (! SubjectConfirmation.METHOD_BEARER.equals(sc.getMethod()))
+ continue;
+ var data = sc.getSubjectConfirmationData();
+ if (data == null)
+ continue;
+ var noa = data.getNotOnOrAfter();
+ if (noa != null && (result == null || noa.isAfter(result)))
+ result = noa;
+ }
+ }
+ return result;
}
private static String extractStringValue(org.opensaml.core.xml.XMLObject av) {
diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_Branches_Test.java b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_Branches_Test.java
index fc45f2ce69..2a04593d3b 100644
--- a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_Branches_Test.java
+++ b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_Branches_Test.java
@@ -73,17 +73,19 @@ class SamlAssertionValidator_Branches_Test extends TestBase {
}
// -----------------------------------------------------------------------------------------------------------------
- // B: nbf == null and noa == null → timestamps not checked (lines 561–564 both false branches)
+ // B: nbf == null and noa == null → the NotBefore/NotOnOrAfter checks are skipped in validateConditions, but
+ // an assertion with no NotOnOrAfter anywhere has an unbounded validity window and is rejected at the
+ // one-time-use stage (its lifetime cannot be bounded, so single-use cannot be enforced past eviction).
// -----------------------------------------------------------------------------------------------------------------
- @Test void b01_noNbf_noNoa_accepted() throws Exception {
+ @Test void b01_noNbf_noNoa_rejected() throws Exception {
var pair = SamlTestSupport.generateRsaKeyPair();
var cred = SamlTestSupport.credential(pair);
- // conditions() with null notBefore and null notOnOrAfter → neither timestamp check fires
+ // conditions() with null notBefore and null notOnOrAfter → neither timestamp check fires, but the missing
+ // NotOnOrAfter leaves the assertion unbounded → rejected (fail-closed) rather than replayable.
var assertion = SamlTestSupport.buildMinimalAssertion(ISSUER, AUDIENCE, "alice", null, null);
var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
- var principal = validator(cred).validate(xml);
- assertEquals("alice", principal.getName());
+ assertThrows(AuthenticationException.class, () -> validator(cred).validate(xml));
}
// -----------------------------------------------------------------------------------------------------------------
diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_ReplayExpiry_Test.java b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_ReplayExpiry_Test.java
new file mode 100644
index 0000000000..67f12bfc6b
--- /dev/null
+++ b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_ReplayExpiry_Test.java
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juneau.rest.server.auth.saml;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.time.*;
+import java.util.concurrent.atomic.*;
+
+import org.apache.juneau.*;
+import org.apache.juneau.commons.concurrent.*;
+import org.apache.juneau.rest.server.auth.*;
+import org.junit.jupiter.api.*;
+import org.opensaml.saml.common.*;
+import org.opensaml.saml.saml2.core.*;
+import org.opensaml.security.credential.*;
+
+/**
+ * Tests that one-time-use enforcement in {@link SamlAssertionValidator} holds for the assertion's full validity
+ * window, including when {@code Conditions/NotOnOrAfter} is absent.
+ *
+ * <p>
+ * An assertion with no {@code NotOnOrAfter} anywhere (neither {@code Conditions/NotOnOrAfter} nor a bearer
+ * {@code SubjectConfirmationData/NotOnOrAfter}) has an unbounded validity window and must be rejected rather than
+ * pinned to a short replay-cache fallback that later expires and re-opens replay. When a bounding
+ * {@code NotOnOrAfter} <i>is</i> present, the replay-cache record must be retained until that resolved instant
+ * (plus clock skew), not a fixed default.
+ *
+ * @since 10.0.0
+ */
+class SamlAssertionValidator_ReplayExpiry_Test extends TestBase {
+
+ private static final String ISSUER = "https://idp.example.com";
+ private static final String AUDIENCE = "https://sp.example.com";
+ private static final String ACS = "https://sp.example.com/saml/acs";
+ private static final Instant NOW = Instant.parse("2026-01-01T00:00:00Z");
+ private static final Clock CLOCK = Clock.fixed(NOW, ZoneOffset.UTC);
+ private static final Duration SKEW = Duration.ofSeconds(60);
+ private static final Instant NBF = NOW.minusSeconds(60);
+
+ private static SamlAssertionValidator.Builder base(Credential cred) {
+ return SamlAssertionValidator.create()
+ .spEntityId(AUDIENCE)
+ .expectedIssuer(ISSUER)
+ .signingCredential(cred)
+ .clock(CLOCK);
+ }
+
+ /** A replay cache that always reports first-seen and records the expiry it was handed. */
+ private static final class CapturingCache implements ReplayCache {
+ private final AtomicLong lastExpiry = new AtomicLong(Long.MIN_VALUE);
+
+ @Override /* ReplayCache */
+ public boolean checkAndRecord(String id, long expiresAtMs) {
+ lastExpiry.set(expiresAtMs);
+ return true;
+ }
+ }
+
+ // -----------------------------------------------------------------------------------------------------------------
+ // A: Missing NotOnOrAfter everywhere → rejected (unbounded validity window).
+ // -----------------------------------------------------------------------------------------------------------------
+
+ @Test void a01_noNotOnOrAfterAnywhere_rejected() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ // Conditions carries an audience but no NotOnOrAfter; no bearer SubjectConfirmation at all.
+ var assertion = SamlTestSupport.buildMinimalAssertion(ISSUER, AUDIENCE, "alice", NBF, null);
+ var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
+ var validator = base(cred).build();
+ assertThrows(AuthenticationException.class, () -> validator.validate(xml));
+ }
+
+ @Test void a02_conditionsNotOnOrAfterPresent_accepted() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var assertion = SamlTestSupport.buildMinimalAssertion(ISSUER, AUDIENCE, "alice", NBF, NOW.plusSeconds(300));
+ var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
+ assertEquals("alice", base(cred).build().validate(xml).getName());
+ }
+
+ // -----------------------------------------------------------------------------------------------------------------
+ // B: Replay-cache retention uses the resolved NotOnOrAfter (plus skew), not a fixed default.
+ // -----------------------------------------------------------------------------------------------------------------
+
+ @Test void b01_missingConditionsNoa_fallsBackToBearerConfirmationNoa() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var confNoa = NOW.plusSeconds(600);
+ // Conditions has no NotOnOrAfter, but a bearer confirmation supplies one; recipient(...) is NOT configured,
+ // so the confirmation is not enforced during validation, yet must still bound the replay-cache retention.
+ var sc = SamlTestSupport.bearerConfirmation(ACS, null, confNoa, null, null);
+ var sub = SamlTestSupport.subjectWithConfirmations("alice", sc);
+ var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, null);
+ var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
+ var cache = new CapturingCache();
+ assertEquals("alice", base(cred).replayCache(cache).build().validate(xml).getName());
+ assertEquals(confNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get());
+ }
+
+ @Test void b02_conditionsNoa_usedForRetention() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var condNoa = NOW.plusSeconds(300);
+ var assertion = SamlTestSupport.buildMinimalAssertion(ISSUER, AUDIENCE, "alice", NBF, condNoa);
+ var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
+ var cache = new CapturingCache();
+ base(cred).replayCache(cache).build().validate(xml);
+ assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get());
+ }
+
+ @Test void b03_laterBearerConfirmationNoa_wins() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var condNoa = NOW.plusSeconds(300);
+ var confNoa = NOW.plusSeconds(900); // later than the Conditions window
+ var sc = SamlTestSupport.bearerConfirmation(ACS, null, confNoa, null, null);
+ var sub = SamlTestSupport.subjectWithConfirmations("alice", sc);
+ var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa);
+ var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
+ var cache = new CapturingCache();
+ base(cred).replayCache(cache).build().validate(xml);
+ assertEquals(confNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get());
+ }
+
+ @Test void b04_laterConditionsNoa_wins() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var condNoa = NOW.plusSeconds(900); // later than the confirmation window
+ var confNoa = NOW.plusSeconds(300);
+ var sc = SamlTestSupport.bearerConfirmation(ACS, null, confNoa, null, null);
+ var sub = SamlTestSupport.subjectWithConfirmations("alice", sc);
+ var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa);
+ var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
+ var cache = new CapturingCache();
+ base(cred).replayCache(cache).build().validate(xml);
+ assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get());
+ }
+
+ // -----------------------------------------------------------------------------------------------------------------
+ // C: Confirmations that do not contribute a NotOnOrAfter fall through to the Conditions value.
+ // -----------------------------------------------------------------------------------------------------------------
+
+ @Test void c01_nonBearerConfirmation_ignoredForRetention() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var condNoa = NOW.plusSeconds(300);
+ var sub = SamlTestSupport.subjectWithConfirmations("alice", holderOfKeyConfirmation());
+ var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa);
+ var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
+ var cache = new CapturingCache();
+ base(cred).replayCache(cache).build().validate(xml);
+ assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get());
+ }
+
+ @Test void c02_bearerConfirmationWithNoData_ignoredForRetention() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var condNoa = NOW.plusSeconds(300);
+ var sub = SamlTestSupport.subjectWithConfirmations("alice", bearerConfirmationNoData());
+ var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa);
+ var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
+ var cache = new CapturingCache();
+ base(cred).replayCache(cache).build().validate(xml);
+ assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get());
+ }
+
+ @Test void c03_bearerConfirmationWithNullNoa_ignoredForRetention() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var condNoa = NOW.plusSeconds(300);
+ // Bearer confirmation carries a SubjectConfirmationData but no NotOnOrAfter → contributes nothing.
+ var sc = SamlTestSupport.bearerConfirmation(null, null, null, null, null);
+ var sub = SamlTestSupport.subjectWithConfirmations("alice", sc);
+ var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa);
+ var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion);
+ var cache = new CapturingCache();
+ base(cred).replayCache(cache).build().validate(xml);
+ assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get());
+ }
+
+ @SuppressWarnings("unchecked")
+ private static SubjectConfirmation holderOfKeyConfirmation() {
+ var scb = (SAMLObjectBuilder<SubjectConfirmation>) SamlTestSupport.bf().getBuilder(SubjectConfirmation.DEFAULT_ELEMENT_NAME);
+ var sc = scb.buildObject();
+ sc.setMethod(SubjectConfirmation.METHOD_HOLDER_OF_KEY);
+ return sc;
+ }
+
+ @SuppressWarnings("unchecked")
+ private static SubjectConfirmation bearerConfirmationNoData() {
+ var scb = (SAMLObjectBuilder<SubjectConfirmation>) SamlTestSupport.bf().getBuilder(SubjectConfirmation.DEFAULT_ELEMENT_NAME);
+ var sc = scb.buildObject();
+ sc.setMethod(SubjectConfirmation.METHOD_BEARER);
+ return sc; // no SubjectConfirmationData
+ }
+}
(juneau) 03/04: READY-386: Add aggregate decompression budget to the Parquet parser
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit ac39eb933888f3145d748b01311ed3d22b46fef4
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 13:40:28 2026 -0400
READY-386: Add aggregate decompression budget to the Parquet parser
The page loop capped per-page size and page count but never summed decompressed
bytes, so many valid pages could still trigger unbounded aggregate decompression.
Charge each page against a per-parse budget (new maxDecompressedBytes, default 64 MiB).
---
.../juneau/marshall/parquet/ParquetConfig.java | 13 ++++
.../marshall/parquet/ParquetConfigAnnotation.java | 1 +
.../juneau/marshall/parquet/ParquetParser.java | 37 +++++++++++-
.../marshall/parquet/ParquetParserSession.java | 66 +++++++++++++++++---
.../parquet/ParquetParser_MaxLength_Test.java | 70 +++++++++++++++++++++-
5 files changed, 175 insertions(+), 12 deletions(-)
diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetConfig.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetConfig.java
index 29ef57c46a..ce600a1028 100644
--- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetConfig.java
+++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetConfig.java
@@ -84,6 +84,19 @@ public @interface ParquetConfig {
*/
String maxInputLength() default "";
+ /**
+ * Maximum allowed aggregate decompressed-byte total across every Parquet page read during a single
+ * parse.
+ *
+ * <p>
+ * Guards against a decompression bomb assembled from many pages that each individually pass the
+ * per-page {@link #maxLength()} cap. Default is <js>"67108864"</js> (64 MiB). A value of
+ * <js>"0"</js> or less disables the cap.
+ *
+ * @return The annotation value.
+ */
+ String maxDecompressedBytes() default "";
+
/** Rank for application order. */
int rank() default 0;
}
diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetConfigAnnotation.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetConfigAnnotation.java
index bfe72d6d32..fa89863616 100644
--- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetConfigAnnotation.java
+++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetConfigAnnotation.java
@@ -85,6 +85,7 @@ public class ParquetConfigAnnotation {
integer(a.maxLength(), "maxLength").ifPresent(b::maxLength);
integer(a.maxCount(), "maxCount").ifPresent(b::maxCount);
integer(a.maxInputLength(), "maxInputLength").ifPresent(b::maxInputLength);
+ integer(a.maxDecompressedBytes(), "maxDecompressedBytes").ifPresent(b::maxDecompressedBytes);
}
}
diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetParser.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetParser.java
index 10a90e1db9..e9f1133392 100644
--- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetParser.java
+++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetParser.java
@@ -51,6 +51,7 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
private static final int DEFAULT_MAX_LENGTH = ParquetParserSession.DEFAULT_MAX_LENGTH;
private static final int DEFAULT_MAX_COUNT = ParquetParserSession.DEFAULT_MAX_COUNT;
private static final int DEFAULT_MAX_INPUT_LENGTH = ParquetParserSession.DEFAULT_MAX_INPUT_LENGTH;
+ private static final int DEFAULT_MAX_DECOMPRESSED_BYTES = ParquetParserSession.DEFAULT_MAX_DECOMPRESSED_BYTES;
private final Map<BeanPropertyMeta,ParquetBeanPropertyMeta> parquetBeanPropertyMetas = new ConcurrentHashMap<>();
private final Map<ClassMeta<?>,ParquetClassMeta> parquetClassMetas = new ConcurrentHashMap<>();
@@ -69,6 +70,7 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
private int maxLength = DEFAULT_MAX_LENGTH;
private int maxCount = DEFAULT_MAX_COUNT;
private int maxInputLength = DEFAULT_MAX_INPUT_LENGTH;
+ private int maxDecompressedBytes = DEFAULT_MAX_DECOMPRESSED_BYTES;
/**
* Constructor, default settings.
@@ -79,6 +81,7 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
maxLength = env("ParquetParser.maxLength", DEFAULT_MAX_LENGTH);
maxCount = env("ParquetParser.maxCount", DEFAULT_MAX_COUNT);
maxInputLength = env("ParquetParser.maxInputLength", DEFAULT_MAX_INPUT_LENGTH);
+ maxDecompressedBytes = env("ParquetParser.maxDecompressedBytes", DEFAULT_MAX_DECOMPRESSED_BYTES);
}
protected Builder(Builder copyFrom) {
@@ -87,6 +90,7 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
maxLength = copyFrom.maxLength;
maxCount = copyFrom.maxCount;
maxInputLength = copyFrom.maxInputLength;
+ maxDecompressedBytes = copyFrom.maxDecompressedBytes;
}
protected Builder(ParquetParser copyFrom) {
@@ -95,6 +99,7 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
maxLength = copyFrom.maxLength;
maxCount = copyFrom.maxCount;
maxInputLength = copyFrom.maxInputLength;
+ maxDecompressedBytes = copyFrom.maxDecompressedBytes;
}
/**
@@ -161,6 +166,24 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
return this;
}
+ /**
+ * The maximum allowed aggregate decompressed-byte total across every Parquet page read during a
+ * single parse.
+ *
+ * <p>
+ * Guards against a decompression bomb assembled from many pages that each individually pass the
+ * per-page {@link #maxLength(int)} cap: this ceiling charges every page's declared uncompressed
+ * size against a running per-parse total, so a chunk (or a whole file) whose pages sum past the
+ * ceiling is rejected with a clean parse error instead of buffering the excess.
+ *
+ * @param value The maximum aggregate decompressed-byte total. Default is 64 MiB. Values ≤ 0 disable the cap.
+ * @return This object.
+ */
+ public Builder maxDecompressedBytes(int value) {
+ maxDecompressedBytes = value;
+ return this;
+ }
+
@Override /* InputStreamParser.Builder<?> */
public Builder copy() {
return new Builder(this);
@@ -173,7 +196,7 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
@Override
public HashKey hashKey() {
- return HashKey.of(super.hashKey(), nullKeyString, maxLength, maxCount, maxInputLength);
+ return HashKey.of(super.hashKey(), nullKeyString, maxLength, maxCount, maxInputLength, maxDecompressedBytes);
}
}
@@ -190,6 +213,7 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
private final int maxLength;
private final int maxCount;
private final int maxInputLength;
+ private final int maxDecompressedBytes;
/**
* Constructor.
@@ -202,6 +226,7 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
maxLength = builder.maxLength;
maxCount = builder.maxCount;
maxInputLength = builder.maxInputLength;
+ maxDecompressedBytes = builder.maxDecompressedBytes;
}
/**
@@ -231,6 +256,16 @@ public class ParquetParser extends InputStreamParser implements ParquetMetaProvi
return maxInputLength;
}
+ /**
+ * Returns the maximum allowed aggregate decompressed-byte total across every page read during a
+ * single parse.
+ *
+ * @return The maximum aggregate decompressed-byte total.
+ */
+ public int getMaxDecompressedBytes() {
+ return maxDecompressedBytes;
+ }
+
@Override /* Overridden from Context */
public Builder copy() {
return new Builder(this);
diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetParserSession.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetParserSession.java
index bf032bf61d..b3f349500f 100644
--- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetParserSession.java
+++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/parquet/ParquetParserSession.java
@@ -150,6 +150,17 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
return v <= 0 ? Long.MAX_VALUE : v;
}
+ /**
+ * Resolves the configured {@link ParquetParser#getMaxDecompressedBytes()} to an effective bound,
+ * translating the {@code <= 0} "disabled" sentinel to {@link Long#MAX_VALUE}.
+ *
+ * @return The effective aggregate decompressed-byte budget.
+ */
+ private long effectiveMaxDecompressedBytes() {
+ var v = ctx.getMaxDecompressedBytes();
+ return v <= 0 ? Long.MAX_VALUE : v;
+ }
+
/**
* Opens a whole-value pull-parser cursor over a Parquet document, bound to this live session.
* {@link RecordReader#read(Class) read(...)} delegates to the polymorphic
@@ -372,6 +383,13 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
*/
static final int DEFAULT_MAX_INPUT_LENGTH = 256 * 1024 * 1024;
+ /**
+ * Default cap (64 MiB) on the aggregate decompressed-byte total across every page read during a
+ * single parse, used when {@link ParquetParser#getMaxDecompressedBytes()} has not been explicitly
+ * configured.
+ */
+ static final int DEFAULT_MAX_DECOMPRESSED_BYTES = 64 * 1024 * 1024;
+
/**
* Clamps an untrusted row-count to a sane initial-capacity ceiling before it is handed to an
* {@link ArrayList} constructor. Defense in depth: the count is already bounded upstream (see
@@ -414,6 +432,32 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
/** Sentinel for a null intermediate OPTIONAL group at the given def level (GAP-14 multi-level nesting). */
private record GroupNull(int defLevel) {}
+ /**
+ * Mutable per-parse aggregate decompressed-byte budget, threaded through every {@code codec.decompress}
+ * call site (plain, list, and map column-chunk page reads) so many pages that each individually pass
+ * the per-page {@code maxLength} cap cannot together accumulate into an unbounded decompression bomb.
+ */
+ private static final class DecompressionBudget {
+
+ private long remaining;
+
+ DecompressionBudget(long remaining) {
+ this.remaining = remaining;
+ }
+
+ /**
+ * Charges {@code uncompressedSize} bytes against the running budget before the page is actually
+ * decompressed, so an over-budget page fails fast without performing the decompression work.
+ *
+ * @throws ParseException If the charge would drive the remaining budget negative.
+ */
+ void charge(long uncompressedSize, String columnPath) throws ParseException {
+ remaining -= uncompressedSize;
+ if (remaining < 0)
+ throw new ParseException("Column '%s' exceeds the aggregate decompressed-byte budget", columnPath);
+ }
+ }
+
private record FileMeta(long numRows, List<RowGroupMeta> rowGroups, Map<String,Integer> schemaRepetition, Set<String> rawByteArrayPaths, Set<String> uuidPaths, Map<String,ColumnLogical> columnLogical) {}
private record RowGroupMeta(long numRows, List<ColumnChunkMeta> columns) {}
private record ColumnChunkMeta(int type, List<String> pathInSchema, int codec, long numValues, long dataPageOffset, long totalCompressedSize) {}
@@ -776,6 +820,9 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
// per-group count wasn't recorded (older Juneau-written files).
var maxCount = effectiveMaxCount();
var allRows = new ArrayList<Object>((int)Math.min(meta.numRows(), maxCount));
+ // Shared across every row group/column in this parse (see DecompressionBudget): per-page maxLength
+ // alone can't stop many individually-valid pages from summing to a decompression bomb.
+ var budget = new DecompressionBudget(effectiveMaxDecompressedBytes());
// Each per-row-group num_rows is untrusted and drives ArrayList pre-allocation and reassembly loops
// downstream. Bound every group count (and the running total) against the same configured ceiling
// used for the file-level total above, before it is used to size anything, so a tiny footer
@@ -788,7 +835,7 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
if (totalGroupRows > maxCount)
throw new ParseException("Total row group numRows %s exceeds maximum allowed %s", totalGroupRows, maxCount);
int groupRows = (int)group.numRows();
- allRows.addAll(readRowGroupRows(fileBytes, group, groupRows, elementType, schemaRepetition, rawByteArrayPaths, uuidPaths, columnLogical));
+ allRows.addAll(readRowGroupRows(fileBytes, group, groupRows, elementType, schemaRepetition, rawByteArrayPaths, uuidPaths, columnLogical, budget));
}
return allRows;
}
@@ -796,7 +843,7 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
@SuppressWarnings({
"java:S107" // Parser-internal method threads decode state (column paths, schema repetition, logical types); parameter count is intentional.
})
- private List<?> readRowGroupRows(byte[] fileBytes, RowGroupMeta group, int numRows, ClassMeta<?> elementType, Map<String,Integer> schemaRepetition, Set<String> rawByteArrayPaths, Set<String> uuidPaths, Map<String,ColumnLogical> columnLogical) throws ParseException {
+ private List<?> readRowGroupRows(byte[] fileBytes, RowGroupMeta group, int numRows, ClassMeta<?> elementType, Map<String,Integer> schemaRepetition, Set<String> rawByteArrayPaths, Set<String> uuidPaths, Map<String,ColumnLogical> columnLogical, DecompressionBudget budget) throws ParseException {
var columnData = new LinkedHashMap<String,List<Object>>();
var maxLength = effectiveMaxLength();
var maxCount = effectiveMaxCount();
@@ -805,11 +852,11 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
List<Object> values;
var trim = isTrimStrings();
if (isListColumnPath(path))
- values = readListColumnChunk(fileBytes, cc, numRows, trim, maxLength, maxCount);
+ values = readListColumnChunk(fileBytes, cc, numRows, trim, maxLength, maxCount, budget);
else if (isMapKeyValueColumnPath(path))
- values = readMapKeyValueColumnChunk(fileBytes, cc, numRows, trim, maxLength, maxCount);
+ values = readMapKeyValueColumnChunk(fileBytes, cc, numRows, trim, maxLength, maxCount, budget);
else
- values = readColumnChunk(fileBytes, cc, numRows, schemaRepetition, rawByteArrayPaths, uuidPaths, columnLogical, trim, maxLength, maxCount);
+ values = readColumnChunk(fileBytes, cc, numRows, schemaRepetition, rawByteArrayPaths, uuidPaths, columnLogical, trim, maxLength, maxCount, budget);
columnData.put(path, values);
}
if (parquetDebug()) {
@@ -852,7 +899,7 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
return idx < 0 ? listColumnPath : listColumnPath.substring(0, idx);
}
- private static List<Object> readListColumnChunk(byte[] fileBytes, ColumnChunkMeta cc, int numRows, boolean trimStrings, int maxLength, long maxCount) throws ParseException {
+ private static List<Object> readListColumnChunk(byte[] fileBytes, ColumnChunkMeta cc, int numRows, boolean trimStrings, int maxLength, long maxCount, DecompressionBudget budget) throws ParseException {
try {
var path = String.join(".", cc.pathInSchema());
var rowRelPath = rowRelativePath(path);
@@ -872,6 +919,7 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
var chunkPath = String.join(".", cc.pathInSchema());
int dataPageOff = skipToDataPage(fileBytes, (int)cc.dataPageOffset(), chunkPath, maxLength, maxCount);
var ph = readPageHeader(fileBytes, dataPageOff, chunkPath, maxLength, maxCount);
+ budget.charge(ph.uncompressedSize(), chunkPath);
var compressedData = Arrays.copyOfRange(fileBytes, ph.bodyStart(), ph.bodyStart() + ph.compressedSize());
var decompressed = codec.decompress(compressedData, ph.uncompressedSize());
@@ -1021,7 +1069,7 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
private static final int MAP_MAX_DEF = 2;
private static final int MAP_MAX_REP = 1;
- private static List<Object> readMapKeyValueColumnChunk(byte[] fileBytes, ColumnChunkMeta cc, int numRows, boolean trimStrings, int maxLength, long maxCount) throws ParseException {
+ private static List<Object> readMapKeyValueColumnChunk(byte[] fileBytes, ColumnChunkMeta cc, int numRows, boolean trimStrings, int maxLength, long maxCount, DecompressionBudget budget) throws ParseException {
try {
int maxDef = MAP_MAX_DEF;
int maxRep = MAP_MAX_REP;
@@ -1038,6 +1086,7 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
var chunkPath = String.join(".", cc.pathInSchema());
int dataPageOff = skipToDataPage(fileBytes, (int)cc.dataPageOffset(), chunkPath, maxLength, maxCount);
var ph = readPageHeader(fileBytes, dataPageOff, chunkPath, maxLength, maxCount);
+ budget.charge(ph.uncompressedSize(), chunkPath);
var compressedData = Arrays.copyOfRange(fileBytes, ph.bodyStart(), ph.bodyStart() + ph.compressedSize());
var decompressed = codec.decompress(compressedData, ph.uncompressedSize());
@@ -1111,7 +1160,7 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
@SuppressWarnings({
"java:S107" // Parser-internal method threads decode state (column paths, schema repetition, logical types); parameter count is intentional.
})
- private static List<Object> readColumnChunk(byte[] fileBytes, ColumnChunkMeta cc, int numRows, Map<String,Integer> schemaRepetition, Set<String> rawByteArrayPaths, Set<String> uuidPaths, Map<String,ColumnLogical> columnLogical, boolean trimStrings, int maxLength, long maxCount) throws ParseException {
+ private static List<Object> readColumnChunk(byte[] fileBytes, ColumnChunkMeta cc, int numRows, Map<String,Integer> schemaRepetition, Set<String> rawByteArrayPaths, Set<String> uuidPaths, Map<String,ColumnLogical> columnLogical, boolean trimStrings, int maxLength, long maxCount, DecompressionBudget budget) throws ParseException {
try {
if (cc.numValues() < 0 || cc.numValues() > maxCount)
throw new ParseException("Invalid numValues for column '%s': %s", String.join(".", cc.pathInSchema()), cc.numValues());
@@ -1147,6 +1196,7 @@ public class ParquetParserSession extends InputStreamParserSession implements Re
if (++pageGuard > MAX_PAGES_PER_CHUNK)
throw new ParseException("Column '%s' exceeds the maximum of %s pages per chunk", path, MAX_PAGES_PER_CHUNK);
var ph = readPageHeader(fileBytes, pageOff, path, maxLength, maxCount);
+ budget.charge(ph.uncompressedSize(), path);
var compressedData = Arrays.copyOfRange(fileBytes, ph.bodyStart(), ph.bodyStart() + ph.compressedSize());
var decompressed = codec.decompress(compressedData, ph.uncompressedSize());
if (ph.pageType() == PAGE_DICTIONARY) {
diff --git a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/parquet/ParquetParser_MaxLength_Test.java b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/parquet/ParquetParser_MaxLength_Test.java
index e6c146c184..ba58c53a38 100644
--- a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/parquet/ParquetParser_MaxLength_Test.java
+++ b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/parquet/ParquetParser_MaxLength_Test.java
@@ -42,9 +42,12 @@ import org.junit.jupiter.api.*;
* A full Parquet file embeds these values deep inside a Thrift-compact footer/page, so hand-crafting a
* complete malformed file to exercise the end-to-end path is impractical for the low-level helpers; those
* are covered directly (oversized rejected, in-range accepted, and the boundary). The configurable
- * {@code maxLength}/{@code maxCount} knobs on {@link ParquetParser} themselves are covered end-to-end via
- * real serialize/parse round trips (category <b>c</b>/<b>d</b> below), mirroring
- * {@code MsgPackParser_MaxLength_Test}.
+ * {@code maxLength}/{@code maxCount}/{@code maxInputLength}/{@code maxDecompressedBytes} knobs on
+ * {@link ParquetParser} themselves are covered end-to-end via real serialize/parse round trips (category
+ * <b>c</b>/<b>d</b>/<b>e</b>/<b>f</b> below), mirroring {@code MsgPackParser_MaxLength_Test}. Category
+ * <b>f</b> additionally proves the <i>aggregate</i> decompressed-byte budget (as opposed to the per-page
+ * {@code maxLength} bound above): many pages that each individually pass {@code maxLength} must still be
+ * rejected once their decompressed sizes sum past the configured ceiling.
*/
@SuppressWarnings("unchecked")
class ParquetParser_MaxLength_Test extends TestBase {
@@ -254,4 +257,65 @@ class ParquetParser_MaxLength_Test extends TestBase {
var parsed = (List<SimpleBean>) p.read(bytes, List.class, SimpleBean.class);
assertEquals(3, parsed.size());
}
+
+ // ================================================================
+ // f. Configurable maxDecompressedBytes (aggregate decompressed-byte budget)
+ // ================================================================
+
+ /**
+ * Serializes {@code n} rows with a small {@code pageSize}, forcing the writer to split each column
+ * chunk into many small data pages (32 rows/page at the 1024-byte pageSize floor) instead of the
+ * single page a default-sized write produces. Every individual page stays far under the (huge
+ * default) {@code maxLength} bound; only their aggregate decompressed size is at risk.
+ */
+ private static byte[] manyPagesBytes(int n) throws Exception {
+ return ParquetSerializer.create().pageSize(1024).build().write(beans(n));
+ }
+
+ @Test
+ void f01_configurableAggregateBudgetEnforcedEndToEnd() throws Exception {
+ var bytes = manyPagesBytes(2_000);
+
+ // A generous cap parses fine even though the file has dozens of pages per column.
+ var lenient = ParquetParser.create().maxDecompressedBytes(1_000_000).build();
+ var parsed = (List<SimpleBean>) lenient.read(bytes, List.class, SimpleBean.class);
+ assertEquals(2_000, parsed.size());
+
+ // Every individual page is tiny (well under the default 256 MiB maxLength), but a cap that only
+ // a handful of pages can satisfy on their own is still exceeded once their sizes accumulate.
+ var strict = ParquetParser.create().maxDecompressedBytes(2_000).build();
+ assertThrowsWithMessage(ParseException.class, "aggregate decompressed-byte budget",
+ () -> strict.read(bytes, List.class, SimpleBean.class));
+ }
+
+ @Test
+ void f02_singlePageWithinBothCapsStillParses() throws Exception {
+ // A single-page file (default pageSize) comfortably under both the per-page and aggregate caps.
+ var bytes = ParquetSerializer.DEFAULT.write(beans(3));
+ var p = ParquetParser.create().maxDecompressedBytes(1_024).build();
+ var parsed = (List<SimpleBean>) p.read(bytes, List.class, SimpleBean.class);
+ assertEquals(3, parsed.size());
+ }
+
+ @Test
+ void f03_maxDecompressedBytesAffectsCacheKey() {
+ // Different maxDecompressedBytes values must NOT collide in the parser cache (hashKey wiring).
+ var p1 = ParquetParser.create().maxDecompressedBytes(100).build();
+ var p2 = ParquetParser.create().maxDecompressedBytes(200).build();
+ var p3 = ParquetParser.create().maxDecompressedBytes(100).build();
+ assertNotSame(p1, p2);
+ assertSame(p1, p3);
+ assertEquals(100, p1.getMaxDecompressedBytes());
+ assertEquals(200, p2.getMaxDecompressedBytes());
+ }
+
+ @Test
+ void f04_maxDecompressedBytesNonPositiveDisablesCap() throws Exception {
+ // A non-positive cap disables the aggregate-budget check, even for the many-small-pages file that
+ // a small positive cap (f01) would reject.
+ var bytes = manyPagesBytes(2_000);
+ var p = ParquetParser.create().maxDecompressedBytes(0).build();
+ var parsed = (List<SimpleBean>) p.read(bytes, List.class, SimpleBean.class);
+ assertEquals(2_000, parsed.size());
+ }
}
(juneau) 02/04: READY-380: Require exact OAuth scope match in MCP resource server
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit eed551345938ffd98940e559b5f8735588ea63e3
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 13:40:23 2026 -0400
READY-380: Require exact OAuth scope match in MCP resource server
McpResourceServerSupport.satisfies() treated a granted scope as implying any
required scope under a "scope:"/"scope." prefix, letting a broad low-privilege
grant authorize a more-privileged operation. Switch to exact-match set semantics.
---
.../mcp/v20260728/McpResourceServerSupport.java | 25 ++++----------
.../v20260728/McpResourceServerBinding_Test.java | 28 +++++++++-------
.../v20260728/McpResourceServerSupport_Test.java | 38 ++++++++++++++--------
3 files changed, 49 insertions(+), 42 deletions(-)
diff --git a/juneau-rest/juneau-rest-server-mcp-v20260728/src/main/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport.java b/juneau-rest/juneau-rest-server-mcp-v20260728/src/main/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport.java
index c1b946d06f..b83e1df9cb 100644
--- a/juneau-rest/juneau-rest-server-mcp-v20260728/src/main/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport.java
+++ b/juneau-rest/juneau-rest-server-mcp-v20260728/src/main/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport.java
@@ -513,32 +513,21 @@ public final class McpResourceServerSupport {
}
/**
- * Returns whether the granted scopes satisfy every required scope, honoring OAuth scope hierarchies where a broader
- * granted scope implies its narrower children.
+ * Returns whether the granted scopes satisfy every required scope, using exact string equality.
*
* <p>
- * A required scope {@code r} is satisfied when some granted scope {@code g} either equals {@code r} exactly, or is a
- * hierarchical <em>ancestor</em> of it — i.e. {@code r} begins with {@code g} followed by a {@code :} or
- * {@code .} delimiter (so granted {@code repo} implies required {@code repo:read} / {@code repo.write}, but granted
- * {@code repo:read} does <b>not</b> imply required {@code repo}).
+ * A required scope {@code r} is satisfied only when some granted scope {@code g} is exactly equal to it. OAuth
+ * scopes have no universal hierarchy, so a granted {@code repo} does <b>not</b> imply a required {@code repo:write}
+ * or {@code repo.admin} — a broad-but-low-privilege scope must never authorize a differently named privileged
+ * operation. Applications that want scope implication must expand their granted scopes themselves before calling
+ * this method; this method does not provide a hierarchical-implication SPI.
*
* @param grantedScopes The caller's granted scopes. Never <jk>null</jk>.
* @param requiredScopes The scopes required to invoke the operation. Never <jk>null</jk>.
* @return <jk>true</jk> if every required scope is satisfied.
*/
public static boolean satisfies(Collection<String> grantedScopes, Collection<String> requiredScopes) {
- for (var r : requiredScopes) {
- var ok = false;
- for (var g : grantedScopes) {
- if (g.equals(r) || r.startsWith(g + ":") || r.startsWith(g + ".")) {
- ok = true;
- break;
- }
- }
- if (!ok)
- return false;
- }
- return true;
+ return grantedScopes.containsAll(requiredScopes);
}
/**
diff --git a/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerBinding_Test.java b/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerBinding_Test.java
index 660d5a352f..415cd660a6 100644
--- a/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerBinding_Test.java
+++ b/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerBinding_Test.java
@@ -72,11 +72,12 @@ class McpResourceServerBinding_Test extends TestBase {
case "good4" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "mcp.read mcp.write", "iss", "https://idp-b", "sub", "alice"));
case "noscope" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "other"));
case "wrongaud" -> new ClaimsPrincipal("alice", Map.of("aud", "http://evil.example.com", "scope", "mcp.read"));
- // SEP-2350 step-up fixtures: baseline mcp.read + the per-operation tools.exec (exact and hierarchical grants).
+ // SEP-2350 step-up fixtures: baseline mcp.read + the per-operation tools.exec (exact grant vs. a
+ // broader-but-not-exact grant that must NOT satisfy it).
case "opok" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "mcp.read tools.exec"));
case "broad" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "mcp.read tools"));
- // H1 baseline-hierarchy fixture: granted ONLY the broad ancestor "mcp", which satisfies the baseline
- // "mcp.read" hierarchically (never exact-string).
+ // Baseline fixture: granted ONLY the broader "mcp" scope, which does NOT satisfy the exact-match baseline
+ // requirement "mcp.read" (OAuth scopes have no universal hierarchy).
case "basehier" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "mcp"));
default -> throw new AuthenticationException("bad token");
};
@@ -201,13 +202,15 @@ class McpResourceServerBinding_Test extends TestBase {
assertContains("alice", s);
}
- // H1: a token granted ONLY the broad ancestor "mcp" satisfies the baseline "mcp.read" hierarchically and
- // dispatches. The old exact-string containsAll baseline gate would have 403'd this.
- @Test void a10_baselineHierarchicalScope_dispatches() throws Exception {
+ // A token granted only the broader "mcp" scope does NOT satisfy the exact-match baseline requirement
+ // "mcp.read": a broad-but-differently-named scope must never authorize a differently named privileged operation.
+ @Test void a10_baselineBroaderScopeDoesNotSatisfy_403() throws Exception {
clientA().post("/").contentString(body(1, "server/discover", null))
.header("Mcp-Method", "server/discover").header("Mcp-Name", "")
.header("Authorization", "Bearer basehier")
- .run().assertStatus(200);
+ .run()
+ .assertStatus(403)
+ .assertHeader("WWW-Authenticate").is("Bearer realm=\"mcp\", error=\"insufficient_scope\", scope=\"mcp.read\", resource_metadata=\"" + PRM_URL + "\"");
}
// M5: the wildcard well-known route must not serve this endpoint's PRM for an arbitrary suffix.
@@ -218,7 +221,7 @@ class McpResourceServerBinding_Test extends TestBase {
// ---------------------------------------------------------------------------------------------
// SEP-2350 (F3) per-operation step-up: an end-to-end HTTP round-trip exercising the POST-parse enforcement
// point in McpRevision.dispatch (a token satisfying the endpoint-wide baseline but not the operation's scope
- // yields a scoped 403 insufficient_scope challenge; the correct scope, exact or hierarchical, dispatches).
+ // yields a scoped 403 insufficient_scope challenge; only the exact required scope dispatches).
// ---------------------------------------------------------------------------------------------
private static McpOptions rsEnabledWithOpScope() {
@@ -266,12 +269,15 @@ class McpResourceServerBinding_Test extends TestBase {
.run().assertStatus(200);
}
- // A broader ancestor scope (tools) satisfies the narrower required tools.exec (server MUST: hierarchy-aware).
- @Test void d03_stepUp_hierarchicalScope_dispatches() throws Exception {
+ // A broader scope (tools) does NOT satisfy the exact required scope tools.exec: OAuth scopes have no universal
+ // hierarchy, so this must be a scoped 403 step-up challenge, not a dispatch.
+ @Test void d03_stepUp_broaderScopeDoesNotSatisfy_403() throws Exception {
clientD().post("/").contentString(body(1, "tools/call", callEcho()))
.header("Mcp-Method", "tools/call").header("Mcp-Name", "echo")
.header("Authorization", "Bearer broad")
- .run().assertStatus(200);
+ .run()
+ .assertStatus(403)
+ .assertHeader("WWW-Authenticate").is("Bearer realm=\"mcp\", error=\"insufficient_scope\", scope=\"tools.exec\", resource_metadata=\"" + PRM_URL + "\"");
}
// An operation with no per-operation scope configured is unaffected: baseline mcp.read alone dispatches it.
diff --git a/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport_Test.java b/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport_Test.java
index 90338624bb..20f9446b53 100644
--- a/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport_Test.java
+++ b/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport_Test.java
@@ -191,8 +191,9 @@ class McpResourceServerSupport_Test {
}
// ---------------------------------------------------------------------------------------------
- // SEP-2350 per-operation step-up: scope satisfaction (hierarchy-aware), scoped challenge shape, and the
- // POST-parse enforcement decision (throws 403 insufficient_scope + WWW-Authenticate, or is a no-op).
+ // SEP-2350 per-operation step-up: scope satisfaction (exact-match only, no hierarchy/prefix inheritance),
+ // scoped challenge shape, and the POST-parse enforcement decision (throws 403 insufficient_scope +
+ // WWW-Authenticate, or is a no-op).
// ---------------------------------------------------------------------------------------------
private static McpOperationContext op(String method, String name) {
@@ -203,10 +204,12 @@ class McpResourceServerSupport_Test {
assertTrue(McpResourceServerSupport.satisfies(Set.of("repo.delete"), Set.of("repo.delete")));
}
- @Test void f02_satisfies_broaderAncestorImpliesNarrower() {
- // Granted "repo" implies required "repo.delete" (dot) and "repo:delete" (colon).
- assertTrue(McpResourceServerSupport.satisfies(Set.of("repo"), Set.of("repo.delete")));
- assertTrue(McpResourceServerSupport.satisfies(Set.of("repo"), Set.of("repo:delete")));
+ @Test void f02_satisfies_broaderScopeDoesNotImplyNarrower() {
+ // OAuth scopes have no universal hierarchy: granted "repo" must NOT satisfy required "repo.delete" (dot) or
+ // "repo:delete" (colon) — a broad-but-low-privilege scope must never authorize a differently named,
+ // more-privileged operation.
+ assertFalse(McpResourceServerSupport.satisfies(Set.of("repo"), Set.of("repo.delete")));
+ assertFalse(McpResourceServerSupport.satisfies(Set.of("repo"), Set.of("repo:delete")));
}
@Test void f03_satisfies_narrowerDoesNotImplyBroader() {
@@ -214,8 +217,8 @@ class McpResourceServerSupport_Test {
assertFalse(McpResourceServerSupport.satisfies(Set.of("repo.read"), Set.of("repo")));
}
- @Test void f04_satisfies_partialTokenIsNotAncestor() {
- // "rep" is a string prefix of "repo.read" but NOT a hierarchical ancestor (no delimiter), so it must not satisfy.
+ @Test void f04_satisfies_partialTokenIsNotAMatch() {
+ // "rep" is a string prefix of "repo.read" but not an exact match, so it must not satisfy.
assertFalse(McpResourceServerSupport.satisfies(Set.of("rep"), Set.of("repo.read")));
}
@@ -224,11 +227,20 @@ class McpResourceServerSupport_Test {
assertTrue(McpResourceServerSupport.satisfies(Set.of("a", "b"), Set.of("a", "b")));
}
- @Test void f05b_satisfies_baselineParity_grantedAncestorSatisfiesBaseline() {
- // H1 parity: the baseline required-scope gate uses satisfies(...) exactly like the per-operation gate, so a
- // token granted "mcp" satisfies a baseline of "mcp.read" hierarchically (no exact-string containsAll needed).
- assertTrue(McpResourceServerSupport.satisfies(Set.of("mcp"), Set.of("mcp.read")));
- assertFalse(Set.of("mcp").containsAll(Set.of("mcp.read"))); // the OLD containsAll gate would have 403'd this.
+ @Test void f05b_satisfies_baselineParity_grantedBroaderScopeDoesNotSatisfyBaseline() {
+ // Parity: the baseline required-scope gate uses satisfies(...) exactly like the per-operation gate, so a
+ // token granted only "mcp" does NOT satisfy a baseline of "mcp.read" — exact-match only, no
+ // prefix/hierarchy inheritance.
+ assertFalse(McpResourceServerSupport.satisfies(Set.of("mcp"), Set.of("mcp.read")));
+ }
+
+ @Test void f05c_satisfies_emptyRequired_alwaysTrue() {
+ assertTrue(McpResourceServerSupport.satisfies(Set.of(), Set.of()));
+ assertTrue(McpResourceServerSupport.satisfies(Set.of("a"), Set.of()));
+ }
+
+ @Test void f05d_satisfies_emptyGrantedNonEmptyRequired_false() {
+ assertFalse(McpResourceServerSupport.satisfies(Set.of(), Set.of("a")));
}
@Test void f06_stepUpChallengeShape() {