This is an automated email from the ASF dual-hosted git repository.
nscendoni pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-oauth-client.git
The following commit(s) were added to refs/heads/master by this push:
new 091488d SLING-13048 Enhance OIDC authentication flow by adding
configurable request key cookie max age (#43)
091488d is described below
commit 091488d7a57047934039784fadf8417f32735910
Author: Nicola Scendoni <[email protected]>
AuthorDate: Tue Feb 10 16:56:17 2026 +0100
SLING-13048 Enhance OIDC authentication flow by adding configurable request
key cookie max age (#43)
* SLING-13048 Enhance OIDC authentication flow by adding configurable
request key cookie max age
- Introduced a new configuration option for the request key cookie max age
in OidcAuthenticationHandler.
- Updated RedirectHelper to utilize the configurable max age when setting
cookies.
- Modified OAuthEntryPointServlet to pass the new max age parameter.
- Added tests to verify the correct usage of the configured cookie max age.
* Refactor RedirectHelperTest to use a constant for cookie expiration time
- Introduced a constant COOKIE_EXPIRE_SECONDS to replace hardcoded values
in test cases.
- Updated all relevant test methods to utilize the new constant for
improved readability and maintainability.
* Reformatted method calls in RedirectHelperTest
* Enhance OAuthEntryPointServlet with configurable request key cookie max
age
- Added a new configuration option for the request key cookie max age in
OAuthEntryPointServlet.
- Updated the constructor to accept the configuration and utilize the
specified max age for the request key cookie.
- Enhanced unit tests to verify the correct application of the configured
cookie max age.
---
.../oauth_client/impl/OAuthEntryPointServlet.java | 31 +++++++++++-
.../impl/OidcAuthenticationHandler.java | 14 ++++-
.../auth/oauth_client/impl/RedirectHelper.java | 17 +++++--
.../impl/OAuthEntryPointServletTest.java | 29 ++++++++++-
.../impl/OidcAuthenticationHandlerTest.java | 1 +
.../auth/oauth_client/impl/RedirectHelperTest.java | 59 ++++++++++++++++++++--
6 files changed, 137 insertions(+), 14 deletions(-)
diff --git
a/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServlet.java
b/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServlet.java
index fcda20b..aef6e1b 100644
---
a/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServlet.java
+++
b/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServlet.java
@@ -40,6 +40,9 @@ import org.jetbrains.annotations.NotNull;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.metatype.annotations.AttributeDefinition;
+import org.osgi.service.metatype.annotations.Designate;
+import org.osgi.service.metatype.annotations.ObjectClassDefinition;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -48,6 +51,7 @@ import static
org.osgi.service.component.annotations.ReferencePolicyOption.GREED
@Component(
service = {Servlet.class},
property = {AuthConstants.AUTH_REQUIREMENTS + "=" +
OAuthEntryPointServlet.PATH})
+@Designate(ocd = OAuthEntryPointServlet.Config.class)
@SlingServletPaths(OAuthEntryPointServlet.PATH)
public class OAuthEntryPointServlet extends SlingAllMethodsServlet {
@@ -61,12 +65,29 @@ public class OAuthEntryPointServlet extends
SlingAllMethodsServlet {
private final CryptoService cryptoService;
+ private final int requestKeyCookieMaxAgeSeconds;
+
+ @ObjectClassDefinition(
+ name = "Apache Sling OAuth Entry Point Servlet",
+ description = "OAuth entry point servlet for initiating the OAuth
authentication flow.")
+ @interface Config {
+ @AttributeDefinition(
+ name = "Request key cookie max age (seconds)",
+ description = "Max age in seconds for the
sling.oauth-request-key cookie used during the OAuth "
+ + "authentication flow. The cookie holds state between
the redirect to the IdP and the "
+ + "callback. Default is 300 (5 minutes). Use a higher
value if users often take longer "
+ + "(e.g. consent, 2FA).")
+ int requestKeyCookieMaxAgeSeconds() default
RedirectHelper.DEFAULT_REQUEST_KEY_COOKIE_MAX_AGE_SECONDS;
+ }
+
@Activate
public OAuthEntryPointServlet(
@Reference(policyOption = GREEDY) List<ClientConnection>
connections,
- @Reference CryptoService cryptoService) {
+ @Reference CryptoService cryptoService,
+ Config config) {
this.connections =
connections.stream().collect(Collectors.toMap(ClientConnection::name,
Function.identity()));
this.cryptoService = cryptoService;
+ this.requestKeyCookieMaxAgeSeconds =
config.requestKeyCookieMaxAgeSeconds();
}
@Override
@@ -116,6 +137,12 @@ public class OAuthEntryPointServlet extends
SlingAllMethodsServlet {
OAuthCookieValue oAuthCookieValue = new
OAuthCookieValue(perRequestKey, connection.name(), redirect);
return RedirectHelper.buildRedirectTarget(
- new String[] {PATH}, callbackUri, conn, oAuthCookieValue,
cryptoService, null);
+ new String[] {PATH},
+ callbackUri,
+ conn,
+ oAuthCookieValue,
+ cryptoService,
+ null,
+ requestKeyCookieMaxAgeSeconds);
}
}
diff --git
a/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java
b/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java
index 9eb31c7..b5b9174 100644
---
a/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java
+++
b/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java
@@ -111,6 +111,8 @@ public class OidcAuthenticationHandler extends
DefaultAuthenticationFeedbackHand
private final String[] resource;
+ private final int requestKeyCookieMaxAgeSeconds;
+
private final CryptoService cryptoService;
@ObjectClassDefinition(
@@ -148,6 +150,14 @@ public class OidcAuthenticationHandler extends
DefaultAuthenticationFeedbackHand
+ "Multiple values can be specified.",
cardinality = Integer.MAX_VALUE)
String[] resource() default {};
+
+ @AttributeDefinition(
+ name = "Request key cookie max age (seconds)",
+ description = "Max age in seconds for the
sling.oauth-request-key cookie used during the OIDC "
+ + "authentication flow. The cookie holds state between
the redirect to the IdP and the "
+ + "callback. Default is 300 (5 minutes). Use a higher
value if users often take longer "
+ + "(e.g. consent, 2FA).")
+ int requestKeyCookieMaxAgeSeconds() default
RedirectHelper.DEFAULT_REQUEST_KEY_COOKIE_MAX_AGE_SECONDS;
}
@Activate
@@ -170,6 +180,7 @@ public class OidcAuthenticationHandler extends
DefaultAuthenticationFeedbackHand
this.pkceEnabled = config.pkceEnabled();
this.path = config.path();
this.resource = config.resource();
+ this.requestKeyCookieMaxAgeSeconds =
config.requestKeyCookieMaxAgeSeconds();
this.cryptoService = cryptoService;
logger.debug("activate: registering ExternalIdentityProvider");
@@ -548,7 +559,8 @@ public class OidcAuthenticationHandler extends
DefaultAuthenticationFeedbackHand
OAuthCookieValue oAuthCookieValue =
new OAuthCookieValue(perRequestKey, connection.name(),
redirect, nonce, codeVerifier);
- return RedirectHelper.buildRedirectTarget(path, callbackUri, conn,
oAuthCookieValue, cryptoService, resource);
+ return RedirectHelper.buildRedirectTarget(
+ path, callbackUri, conn, oAuthCookieValue, cryptoService,
resource, requestKeyCookieMaxAgeSeconds);
}
@Override
diff --git
a/src/main/java/org/apache/sling/auth/oauth_client/impl/RedirectHelper.java
b/src/main/java/org/apache/sling/auth/oauth_client/impl/RedirectHelper.java
index 7a7f2af..e62bb8d 100644
--- a/src/main/java/org/apache/sling/auth/oauth_client/impl/RedirectHelper.java
+++ b/src/main/java/org/apache/sling/auth/oauth_client/impl/RedirectHelper.java
@@ -44,7 +44,9 @@ class RedirectHelper {
// We don't want leave the cookie lying around for a long time because it
is not needed.
// At the same time, some OAuth user authentication flows take a long time
due to
// consent, account selection, 2FA, etc. so we cannot make this too short.
- private static final int COOKIE_MAX_AGE_SECONDS = 300;
+ /** Default max age in seconds for the sling.oauth-request-key cookie.
Used when not configured by the handler. */
+ public static final int DEFAULT_REQUEST_KEY_COOKIE_MAX_AGE_SECONDS = 300;
+
private static final Logger logger =
LoggerFactory.getLogger(RedirectHelper.class);
private RedirectHelper() {
@@ -57,13 +59,17 @@ class RedirectHelper {
@NotNull ResolvedConnection conn,
@NotNull OAuthCookieValue oAuthCookieValue,
@NotNull CryptoService cryptoService,
- @Nullable String[] resource) {
+ @Nullable String[] resource,
+ int requestKeyCookieMaxAgeSeconds) {
String path = findLongestPathMatching(paths, callbackUri.getPath());
// Set the cookie with state, connection name, redirect uri, nonce and
codeverifier
Cookie requestKeyCookie = buildCookie(
- path, OAuthCookieValue.COOKIE_NAME_REQUEST_KEY,
cryptoService.encrypt(oAuthCookieValue.getValue()));
+ path,
+ OAuthCookieValue.COOKIE_NAME_REQUEST_KEY,
+ cryptoService.encrypt(oAuthCookieValue.getValue()),
+ requestKeyCookieMaxAgeSeconds);
// We build th redirect url to be sent to the browser
URI authorizationEndpointUri =
URI.create(conn.authorizationEndpoint());
@@ -111,11 +117,12 @@ class RedirectHelper {
return new RedirectTarget(uri, requestKeyCookie);
}
- private static @NotNull Cookie buildCookie(@Nullable String path, @NotNull
String name, @NotNull String value) {
+ private static @NotNull Cookie buildCookie(
+ @Nullable String path, @NotNull String name, @NotNull String
value, int maxAgeSeconds) {
Cookie cookie = new Cookie(name, value);
cookie.setHttpOnly(true);
cookie.setSecure(true);
- cookie.setMaxAge(COOKIE_MAX_AGE_SECONDS);
+ cookie.setMaxAge(maxAgeSeconds);
if (path != null) cookie.setPath(path);
return cookie;
}
diff --git
a/src/test/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServletTest.java
b/src/test/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServletTest.java
index dc37de6..b5a1cac 100644
---
a/src/test/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServletTest.java
+++
b/src/test/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServletTest.java
@@ -19,6 +19,7 @@
package org.apache.sling.auth.oauth_client.impl;
import javax.servlet.ServletException;
+import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
@@ -36,7 +37,10 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
@ExtendWith(SlingContextExtension.class)
class OAuthEntryPointServletTest {
@@ -57,7 +61,10 @@ class OAuthEntryPointServletTest {
"client-secret",
"http://example.com";,
new String[] {"access_type=offline"}));
- servlet = new OAuthEntryPointServlet(connections, new
StubCryptoService());
+ OAuthEntryPointServlet.Config config =
mock(OAuthEntryPointServlet.Config.class);
+ when(config.requestKeyCookieMaxAgeSeconds())
+
.thenReturn(RedirectHelper.DEFAULT_REQUEST_KEY_COOKIE_MAX_AGE_SECONDS);
+ servlet = new OAuthEntryPointServlet(connections, new
StubCryptoService(), config);
}
@Test
@@ -190,4 +197,24 @@ class OAuthEntryPointServletTest {
assertThat(context.response().getStatus()).as("response
code").isEqualTo(HttpServletResponse.SC_BAD_REQUEST);
}
+
+ @Test
+ void usesConfiguredCookieMaxAge() throws ServletException, IOException {
+ int customMaxAge = 600;
+ OAuthEntryPointServlet.Config config =
mock(OAuthEntryPointServlet.Config.class);
+ when(config.requestKeyCookieMaxAgeSeconds()).thenReturn(customMaxAge);
+ OAuthEntryPointServlet servletWithCustomConfig = new
OAuthEntryPointServlet(
+ Arrays.asList(MockOidcConnection.DEFAULT_CONNECTION), new
StubCryptoService(), config);
+
+ context.request().setQueryString("c=" +
MockOidcConnection.DEFAULT_CONNECTION.name());
+ MockSlingHttpServletResponse response = context.response();
+
+ servletWithCustomConfig.service(context.request(), response);
+
+ Cookie requestKeyCookie = Arrays.stream(response.getCookies())
+ .filter(c ->
OAuthCookieValue.COOKIE_NAME_REQUEST_KEY.equals(c.getName()))
+ .findFirst()
+ .orElseThrow();
+ assertEquals(customMaxAge, requestKeyCookie.getMaxAge());
+ }
}
diff --git
a/src/test/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandlerTest.java
b/src/test/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandlerTest.java
index f257fc8..b07254f 100644
---
a/src/test/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandlerTest.java
+++
b/src/test/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandlerTest.java
@@ -102,6 +102,7 @@ class OidcAuthenticationHandlerTest {
config = mock(OidcAuthenticationHandler.Config.class);
when(config.idp()).thenReturn("myIdP");
when(config.path()).thenReturn(new String[] {"/"});
+ when(config.requestKeyCookieMaxAgeSeconds()).thenReturn(300);
loginCookieManager = mock(LoginCookieManager.class);
SlingUserInfoProcessorImpl.Config userInfoConfig =
Converters.standardConverter()
diff --git
a/src/test/java/org/apache/sling/auth/oauth_client/impl/RedirectHelperTest.java
b/src/test/java/org/apache/sling/auth/oauth_client/impl/RedirectHelperTest.java
index ef4266b..0bc6420 100644
---
a/src/test/java/org/apache/sling/auth/oauth_client/impl/RedirectHelperTest.java
+++
b/src/test/java/org/apache/sling/auth/oauth_client/impl/RedirectHelperTest.java
@@ -33,6 +33,8 @@ import static org.mockito.Mockito.when;
class RedirectHelperTest {
+ final int COOKIE_EXPIRE_SECONDS = 300;
+
@Test
void testFindLongestPathMatchingWithValidPaths() {
String[] paths = {"/a/b/c", "/a/b", "/a"};
@@ -151,7 +153,13 @@ class RedirectHelperTest {
String[] audience = new String[] {"https://api.example.com"};
RedirectTarget result = RedirectHelper.buildRedirectTarget(
- new String[] {"/"}, URI.create("/callback"), conn,
oAuthCookieValue, cryptoService, audience);
+ new String[] {"/"},
+ URI.create("/callback"),
+ conn,
+ oAuthCookieValue,
+ cryptoService,
+ audience,
+ COOKIE_EXPIRE_SECONDS);
assertNotNull(result);
assertNotNull(result.uri());
@@ -170,7 +178,13 @@ class RedirectHelperTest {
String[] audience = new String[] {"https://api1.example.com";,
"https://api2.example.com"};
RedirectTarget result = RedirectHelper.buildRedirectTarget(
- new String[] {"/"}, URI.create("/callback"), conn,
oAuthCookieValue, cryptoService, audience);
+ new String[] {"/"},
+ URI.create("/callback"),
+ conn,
+ oAuthCookieValue,
+ cryptoService,
+ audience,
+ COOKIE_EXPIRE_SECONDS);
assertNotNull(result);
assertNotNull(result.uri());
@@ -193,7 +207,13 @@ class RedirectHelperTest {
String[] audience = new String[] {};
RedirectTarget result = RedirectHelper.buildRedirectTarget(
- new String[] {"/"}, URI.create("/callback"), conn,
oAuthCookieValue, cryptoService, audience);
+ new String[] {"/"},
+ URI.create("/callback"),
+ conn,
+ oAuthCookieValue,
+ cryptoService,
+ audience,
+ COOKIE_EXPIRE_SECONDS);
assertRedirectTargetHasNoResourceParameter(result);
}
@@ -206,7 +226,13 @@ class RedirectHelperTest {
new OAuthCookieValue("perRequestKey", "connectionName",
"/redirect", new Nonce("nonce"), null);
RedirectTarget result = RedirectHelper.buildRedirectTarget(
- new String[] {"/"}, URI.create("/callback"), conn,
oAuthCookieValue, cryptoService, null);
+ new String[] {"/"},
+ URI.create("/callback"),
+ conn,
+ oAuthCookieValue,
+ cryptoService,
+ null,
+ COOKIE_EXPIRE_SECONDS);
assertRedirectTargetHasNoResourceParameter(result);
}
@@ -221,7 +247,13 @@ class RedirectHelperTest {
String[] audience = new String[] {"", " ", "https://api.example.com";,
null};
RedirectTarget result = RedirectHelper.buildRedirectTarget(
- new String[] {"/"}, URI.create("/callback"), conn,
oAuthCookieValue, cryptoService, audience);
+ new String[] {"/"},
+ URI.create("/callback"),
+ conn,
+ oAuthCookieValue,
+ cryptoService,
+ audience,
+ COOKIE_EXPIRE_SECONDS);
assertNotNull(result);
assertNotNull(result.uri());
@@ -234,6 +266,23 @@ class RedirectHelperTest {
assertEquals(1, count, "Expected exactly one resource parameter but
found " + count);
}
+ @Test
+ void testBuildRedirectTargetUsesConfiguredCookieMaxAge() {
+ ResolvedConnection conn = createMockResolvedConnection();
+ CryptoService cryptoService = new StubCryptoService();
+ OAuthCookieValue oAuthCookieValue =
+ new OAuthCookieValue("perRequestKey", "connectionName",
"/redirect", new Nonce("nonce"), null);
+
+ int customMaxAge = 600;
+ RedirectTarget result = RedirectHelper.buildRedirectTarget(
+ new String[] {"/"}, URI.create("/callback"), conn,
oAuthCookieValue, cryptoService, null, customMaxAge);
+
+ assertNotNull(result);
+ assertNotNull(result.cookie());
+ assertEquals(OAuthCookieValue.COOKIE_NAME_REQUEST_KEY,
result.cookie().getName());
+ assertEquals(customMaxAge, result.cookie().getMaxAge());
+ }
+
private ResolvedConnection createMockResolvedConnection() {
ResolvedConnection conn = mock(ResolvedConnection.class);
when(conn.authorizationEndpoint()).thenReturn("http://localhost:8080/authorize";);
