You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by co...@apache.org on 2026/08/07 11:35:55 UTC
(cxf) branch coheigea/OidcClientCodeRequestFilter updated (92d624d2085 -> dd195944d60)
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a change to branch coheigea/OidcClientCodeRequestFilter
in repository https://gitbox.apache.org/repos/asf/cxf.git
discard 92d624d2085 Fix max_age check in OidcClientCodeRequestFilter
new dd195944d60 Fix max_age check in OidcClientCodeRequestFilter
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (92d624d2085)
\
N -- N -- N refs/heads/coheigea/OidcClientCodeRequestFilter (dd195944d60)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 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:
.../rp/OidcClientCodeRequestFilterMaxAgeTest.java | 332 +++++++++++++++++++++
1 file changed, 332 insertions(+)
create mode 100644 rt/rs/security/sso/oidc/src/test/java/org/apache/cxf/rs/security/oidc/rp/OidcClientCodeRequestFilterMaxAgeTest.java
(cxf) 01/01: Fix max_age check in OidcClientCodeRequestFilter
Posted by co...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch coheigea/OidcClientCodeRequestFilter
in repository https://gitbox.apache.org/repos/asf/cxf.git
commit dd195944d604d62b05bdbf149a46043cf73ab154
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Fri Aug 7 12:35:09 2026 +0100
Fix max_age check in OidcClientCodeRequestFilter
---
.../oidc/rp/OidcClientCodeRequestFilter.java | 14 +-
.../rp/OidcClientCodeRequestFilterMaxAgeTest.java | 332 +++++++++++++++++++++
2 files changed, 341 insertions(+), 5 deletions(-)
diff --git a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/rp/OidcClientCodeRequestFilter.java b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/rp/OidcClientCodeRequestFilter.java
index 1d9fa450ef5..d745dc2c4e9 100644
--- a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/rp/OidcClientCodeRequestFilter.java
+++ b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/rp/OidcClientCodeRequestFilter.java
@@ -135,7 +135,9 @@ public class OidcClientCodeRequestFilter extends ClientCodeRequestFilter {
protected MultivaluedMap<String, String> toCodeRequestState(ContainerRequestContext rc, UriInfo ui) {
MultivaluedMap<String, String> state = super.toCodeRequestState(rc, ui);
if (maxAgeOffset != null) {
- state.putSingle(MAX_AGE_PARAMETER, Long.toString(System.currentTimeMillis() + maxAgeOffset));
+ // Store the earliest acceptable auth_time (seconds) so validateIdToken can compare
+ // directly against the id_token auth_time claim, which is also in seconds.
+ state.putSingle(MAX_AGE_PARAMETER, Long.toString(System.currentTimeMillis() / 1000 - maxAgeOffset));
}
// Per OIDC Core §3.2.2.1 and §3.3.2.1, a nonce is REQUIRED for Implicit and Hybrid flows
// (any response_type containing "id_token"). Auto-generate one if the caller has not
@@ -165,9 +167,10 @@ public class OidcClientCodeRequestFilter extends ClientCodeRequestFilter {
throw new OAuthServiceException(OAuthConstants.INVALID_REQUEST);
}
if (maxAgeOffset != null) {
- long authTime = Long.parseLong(state.getFirst(MAX_AGE_PARAMETER));
+ long minAuthTime = Long.parseLong(state.getFirst(MAX_AGE_PARAMETER));
Long tokenAuthTime = idToken.getAuthenticationTime();
- if (tokenAuthTime > authTime) {
+ // auth_time is required when max_age was requested (OIDC Core §3.1.3.7).
+ if (tokenAuthTime == null || tokenAuthTime < minAuthTime) {
throw new OAuthServiceException(OAuthConstants.INVALID_REQUEST);
}
}
@@ -209,8 +212,9 @@ public class OidcClientCodeRequestFilter extends ClientCodeRequestFilter {
if (nonce != null) {
ub.queryParam(IdToken.NONCE_CLAIM, nonce);
}
- if (redirectState != null && redirectState.getFirst(MAX_AGE_PARAMETER) != null) {
- ub.queryParam(MAX_AGE_PARAMETER, redirectState.getFirst(MAX_AGE_PARAMETER));
+ if (maxAgeOffset != null) {
+ // max_age is a duration in seconds per OIDC Core §3.1.2.1, not a timestamp.
+ ub.queryParam(MAX_AGE_PARAMETER, Long.toString(maxAgeOffset));
}
if (codeRequestState != null && codeRequestState.getFirst(LOGIN_HINT_PARAMETER) != null) {
ub.queryParam(LOGIN_HINT_PARAMETER, codeRequestState.getFirst(LOGIN_HINT_PARAMETER));
diff --git a/rt/rs/security/sso/oidc/src/test/java/org/apache/cxf/rs/security/oidc/rp/OidcClientCodeRequestFilterMaxAgeTest.java b/rt/rs/security/sso/oidc/src/test/java/org/apache/cxf/rs/security/oidc/rp/OidcClientCodeRequestFilterMaxAgeTest.java
new file mode 100644
index 00000000000..59d480dc990
--- /dev/null
+++ b/rt/rs/security/sso/oidc/src/test/java/org/apache/cxf/rs/security/oidc/rp/OidcClientCodeRequestFilterMaxAgeTest.java
@@ -0,0 +1,332 @@
+/**
+ * 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.cxf.rs.security.oidc.rp;
+
+import java.lang.reflect.Method;
+import java.net.URI;
+
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.core.MultivaluedHashMap;
+import jakarta.ws.rs.core.MultivaluedMap;
+import jakarta.ws.rs.core.UriBuilder;
+import jakarta.ws.rs.core.UriInfo;
+import org.apache.cxf.jaxrs.impl.MetadataMap;
+import org.apache.cxf.rs.security.oauth2.provider.OAuthServiceException;
+import org.apache.cxf.rs.security.oauth2.utils.OAuthConstants;
+import org.apache.cxf.rs.security.oidc.common.IdToken;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for max_age enforcement in {@link OidcClientCodeRequestFilter}.
+ *
+ * OIDC Core §3.1.3.7 requires the RP to validate auth_time when max_age is used.
+ * The max_age parameter sent to the OP must be a duration in seconds (§3.1.2.1).
+ */
+public class OidcClientCodeRequestFilterMaxAgeTest {
+
+ private static final URI ABSOLUTE_PATH = URI.create("https://app.example.com/rp/callback");
+ private static final String MAX_AGE_PARAMETER = "max_age";
+
+ // -----------------------------------------------------------------------
+ // toCodeRequestState – state encoding
+ // -----------------------------------------------------------------------
+
+ /**
+ * The state entry must be a Unix timestamp in seconds, comparable to the
+ * id_token auth_time claim. Storing milliseconds here (old bug) would produce a
+ * value ~1000x larger than any real auth_time, making the validation unreachable.
+ */
+ @Test
+ public void testStateStoresMinAuthTimeInSeconds() {
+ OidcClientCodeRequestFilter filter = new OidcClientCodeRequestFilter();
+ filter.setMaxAgeOffset(60L);
+
+ long beforeSeconds = System.currentTimeMillis() / 1000;
+ MultivaluedMap<String, String> state = invokeToCodeRequestState(filter, new MultivaluedHashMap<>());
+ long afterSeconds = System.currentTimeMillis() / 1000;
+
+ String stored = state.getFirst(MAX_AGE_PARAMETER);
+ assertNotNull("State must contain max_age entry", stored);
+
+ long minAuthTime = Long.parseLong(stored);
+ // The stored value must be in the range [before - 60, after - 60]:
+ // a milliseconds value (~1.7e12) would fail this range entirely.
+ assertTrue("Stored min-auth-time must be in seconds, not milliseconds",
+ minAuthTime >= beforeSeconds - 60 && minAuthTime <= afterSeconds - 60);
+ }
+
+ /**
+ * When maxAgeOffset is not set, no max_age entry should appear in the state.
+ */
+ @Test
+ public void testStateHasNoMaxAgeWhenNotConfigured() {
+ OidcClientCodeRequestFilter filter = new OidcClientCodeRequestFilter();
+
+ MultivaluedMap<String, String> state = invokeToCodeRequestState(filter, new MultivaluedHashMap<>());
+
+ assertTrue("State must not contain max_age when maxAgeOffset is not set",
+ state.getFirst(MAX_AGE_PARAMETER) == null);
+ }
+
+ // -----------------------------------------------------------------------
+ // validateIdToken – max_age enforcement
+ // -----------------------------------------------------------------------
+
+ /**
+ * A token whose auth_time is within the max_age window must be accepted.
+ */
+ @Test
+ public void testAcceptsFreshAuthTime() {
+ OidcClientCodeRequestFilter filter = new OidcClientCodeRequestFilter();
+ filter.setMaxAgeOffset(300L); // 5 minutes
+
+ long nowSeconds = System.currentTimeMillis() / 1000;
+ long recentAuthTime = nowSeconds - 60; // authenticated 1 minute ago
+
+ IdToken token = new IdToken();
+ token.setAuthenticationTime(recentAuthTime);
+
+ // State contains the minimum acceptable auth_time.
+ MultivaluedMap<String, String> state = new MetadataMap<>();
+ state.putSingle(MAX_AGE_PARAMETER, Long.toString(nowSeconds - 300));
+
+ invokeValidateIdToken(filter, token, state); // must not throw
+ }
+
+ /**
+ * A token whose auth_time is older than the max_age window must be rejected.
+ * This was the primary regression: the old ms-vs-s bug made rejection unreachable.
+ */
+ @Test
+ public void testRejectsStaleAuthTime() {
+ OidcClientCodeRequestFilter filter = new OidcClientCodeRequestFilter();
+ filter.setMaxAgeOffset(60L); // 1 minute
+
+ long nowSeconds = System.currentTimeMillis() / 1000;
+ long staleAuthTime = nowSeconds - 86400; // authenticated 24 hours ago
+
+ IdToken token = new IdToken();
+ token.setAuthenticationTime(staleAuthTime);
+
+ MultivaluedMap<String, String> state = new MetadataMap<>();
+ state.putSingle(MAX_AGE_PARAMETER, Long.toString(nowSeconds - 60));
+
+ try {
+ invokeValidateIdToken(filter, token, state);
+ fail("Expected OAuthServiceException: auth_time is older than max_age allows");
+ } catch (OAuthServiceException ex) {
+ assertEquals(OAuthConstants.INVALID_REQUEST, ex.getMessage());
+ }
+ }
+
+ /**
+ * An auth_time exactly equal to minAuthTime (boundary) must be accepted.
+ */
+ @Test
+ public void testAcceptsAuthTimeAtBoundary() {
+ OidcClientCodeRequestFilter filter = new OidcClientCodeRequestFilter();
+ filter.setMaxAgeOffset(60L);
+
+ long nowSeconds = System.currentTimeMillis() / 1000;
+ long minAuthTime = nowSeconds - 60;
+
+ IdToken token = new IdToken();
+ token.setAuthenticationTime(minAuthTime); // exactly at the boundary
+
+ MultivaluedMap<String, String> state = new MetadataMap<>();
+ state.putSingle(MAX_AGE_PARAMETER, Long.toString(minAuthTime));
+
+ invokeValidateIdToken(filter, token, state); // must not throw
+ }
+
+ /**
+ * A missing auth_time claim must be rejected when max_age was requested.
+ * OIDC Core §3.1.3.7 requires auth_time to be present in this case.
+ * Previously this caused a NullPointerException (HTTP 500).
+ */
+ @Test
+ public void testRejectsNullAuthTime() {
+ OidcClientCodeRequestFilter filter = new OidcClientCodeRequestFilter();
+ filter.setMaxAgeOffset(60L);
+
+ IdToken token = new IdToken();
+ // auth_time deliberately absent
+
+ MultivaluedMap<String, String> state = new MetadataMap<>();
+ long nowSeconds = System.currentTimeMillis() / 1000;
+ state.putSingle(MAX_AGE_PARAMETER, Long.toString(nowSeconds - 60));
+
+ try {
+ invokeValidateIdToken(filter, token, state);
+ fail("Expected OAuthServiceException: auth_time absent when max_age was requested");
+ } catch (OAuthServiceException ex) {
+ assertEquals(OAuthConstants.INVALID_REQUEST, ex.getMessage());
+ }
+ }
+
+ /**
+ * When maxAgeOffset is not configured, a token without auth_time must be accepted
+ * — the null-guard must not fire in the non-max_age path.
+ */
+ @Test
+ public void testNoMaxAgeOffsetSkipsAuthTimeCheck() {
+ OidcClientCodeRequestFilter filter = new OidcClientCodeRequestFilter();
+ // maxAgeOffset not set
+
+ IdToken token = new IdToken();
+ // auth_time absent
+
+ invokeValidateIdToken(filter, token, new MetadataMap<>()); // must not throw
+ }
+
+ // -----------------------------------------------------------------------
+ // setAdditionalCodeRequestParams – max_age request parameter to OP
+ // -----------------------------------------------------------------------
+
+ /**
+ * The max_age query parameter forwarded to the Authorization Endpoint must be the
+ * duration in seconds, not a millisecond timestamp. OIDC Core §3.1.2.1 defines
+ * max_age as "Maximum Authentication Age" (elapsed time in seconds).
+ */
+ @Test
+ public void testMaxAgeRequestParamIsDuration() {
+ OidcClientCodeRequestFilter filter = new OidcClientCodeRequestFilter();
+ filter.setMaxAgeOffset(120L);
+
+ UriBuilder ub = UriBuilder.fromUri("https://idp.example.com/authorize");
+ invokeSetAdditionalCodeRequestParams(filter, ub, null, null);
+
+ URI built = ub.build();
+ String query = built.getQuery();
+ assertNotNull("URI must have a query string", query);
+ assertTrue("max_age must appear in query", query.contains("max_age="));
+
+ // Extract the max_age value and verify it is the duration, not a large timestamp.
+ String maxAgeValue = extractQueryParam(query, MAX_AGE_PARAMETER);
+ assertNotNull("max_age must be present", maxAgeValue);
+
+ long sent = Long.parseLong(maxAgeValue);
+ // A milliseconds timestamp would be ~1.7e12; the duration is 120.
+ assertEquals("max_age must be the configured duration in seconds, not a timestamp", 120L, sent);
+ }
+
+ /**
+ * When maxAgeOffset is not set, no max_age parameter must be added to the request.
+ */
+ @Test
+ public void testNoMaxAgeParamWhenNotConfigured() {
+ OidcClientCodeRequestFilter filter = new OidcClientCodeRequestFilter();
+
+ UriBuilder ub = UriBuilder.fromUri("https://idp.example.com/authorize");
+ invokeSetAdditionalCodeRequestParams(filter, ub, null, null);
+
+ String query = ub.build().getQuery();
+ assertTrue("max_age must not be added when maxAgeOffset is not configured",
+ query == null || !query.contains("max_age="));
+ }
+
+ // -----------------------------------------------------------------------
+ // Helpers
+ // -----------------------------------------------------------------------
+
+ private static void invokeValidateIdToken(OidcClientCodeRequestFilter filter,
+ IdToken idToken,
+ MultivaluedMap<String, String> state) {
+ try {
+ Method method = OidcClientCodeRequestFilter.class.getDeclaredMethod(
+ "validateIdToken", IdToken.class, MultivaluedMap.class, boolean.class);
+ method.setAccessible(true);
+ method.invoke(filter, idToken, state, false);
+ } catch (java.lang.reflect.InvocationTargetException ex) {
+ Throwable cause = ex.getCause();
+ if (cause instanceof OAuthServiceException) {
+ throw (OAuthServiceException) cause;
+ }
+ throw new IllegalStateException(cause);
+ } catch (ReflectiveOperationException ex) {
+ throw new IllegalStateException(ex);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private static MultivaluedMap<String, String> invokeToCodeRequestState(
+ OidcClientCodeRequestFilter filter, MultivaluedMap<String, String> queryParams) {
+ UriInfo uriInfo = mock(UriInfo.class);
+ when(uriInfo.getQueryParameters(anyBoolean())).thenReturn(queryParams);
+ when(uriInfo.getAbsolutePath()).thenReturn(ABSOLUTE_PATH);
+
+ ContainerRequestContext rc = mock(ContainerRequestContext.class);
+ when(rc.getUriInfo()).thenReturn(uriInfo);
+ when(rc.getMediaType()).thenReturn(null);
+
+ try {
+ Method method = OidcClientCodeRequestFilter.class.getDeclaredMethod(
+ "toCodeRequestState", ContainerRequestContext.class, UriInfo.class);
+ method.setAccessible(true);
+ return (MultivaluedMap<String, String>) method.invoke(filter, rc, uriInfo);
+ } catch (java.lang.reflect.InvocationTargetException ex) {
+ Throwable cause = ex.getCause();
+ if (cause instanceof RuntimeException) {
+ throw (RuntimeException) cause;
+ }
+ throw new IllegalStateException(cause);
+ } catch (ReflectiveOperationException ex) {
+ throw new IllegalStateException(ex);
+ }
+ }
+
+ private static void invokeSetAdditionalCodeRequestParams(OidcClientCodeRequestFilter filter,
+ UriBuilder ub,
+ MultivaluedMap<String, String> redirectState,
+ MultivaluedMap<String, String> codeRequestState) {
+ try {
+ Method method = OidcClientCodeRequestFilter.class.getDeclaredMethod(
+ "setAdditionalCodeRequestParams",
+ UriBuilder.class, MultivaluedMap.class, MultivaluedMap.class);
+ method.setAccessible(true);
+ method.invoke(filter, ub, redirectState, codeRequestState);
+ } catch (java.lang.reflect.InvocationTargetException ex) {
+ Throwable cause = ex.getCause();
+ if (cause instanceof RuntimeException) {
+ throw (RuntimeException) cause;
+ }
+ throw new IllegalStateException(cause);
+ } catch (ReflectiveOperationException ex) {
+ throw new IllegalStateException(ex);
+ }
+ }
+
+ private static String extractQueryParam(String query, String name) {
+ for (String param : query.split("&")) {
+ if (param.startsWith(name + "=")) {
+ return param.substring(name.length() + 1);
+ }
+ }
+ return null;
+ }
+}