This is an automated email from the ASF dual-hosted git repository.
angela 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 bd3f85b SLING-12851 SlingUserInfoProcessor should be a ServiceFactory
(#26)
bd3f85b is described below
commit bd3f85b32c8e8e8329a2cfa3291810b248b898bd
Author: Nicola Scendoni <[email protected]>
AuthorDate: Tue Aug 5 08:25:38 2025 +0200
SLING-12851 SlingUserInfoProcessor should be a ServiceFactory (#26)
---
.../impl/OidcAuthenticationHandler.java | 15 ++++--
.../impl/SlingUserInfoProcessorImpl.java | 15 +++++-
.../auth/oauth_client/spi/UserInfoProcessor.java | 17 +++---
.../auth/oauth_client/AuthorizationCodeFlowIT.java | 3 +-
.../impl/OidcAuthenticationHandlerTest.java | 63 +++++++++++++++++-----
.../impl/SlingUserInfoProcessorImplTest.java | 27 ++++++++--
6 files changed, 110 insertions(+), 30 deletions(-)
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 f97bf6a..e52f59e 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
@@ -101,7 +101,7 @@ public class OidcAuthenticationHandler extends
DefaultAuthenticationFeedbackHand
private final String defaultConnectionName;
- private final UserInfoProcessor userInfoProcessor;
+ private final Map<String, UserInfoProcessor> userInfoProcessors;
private final boolean userInfoEnabled;
@@ -146,7 +146,7 @@ public class OidcAuthenticationHandler extends
DefaultAuthenticationFeedbackHand
@Reference List<ClientConnection> connections,
Config config,
@Reference(policyOption = ReferencePolicyOption.GREEDY)
LoginCookieManager loginCookieManager,
- @Reference(policyOption = ReferencePolicyOption.GREEDY)
UserInfoProcessor userInfoProcessor,
+ @Reference(policyOption = ReferencePolicyOption.GREEDY)
List<UserInfoProcessor> userInfoProcessors,
@Reference CryptoService cryptoService) {
this.connections =
connections.stream().collect(Collectors.toMap(ClientConnection::name,
Function.identity()));
@@ -154,7 +154,8 @@ public class OidcAuthenticationHandler extends
DefaultAuthenticationFeedbackHand
this.callbackUri = config.callbackUri();
this.loginCookieManager = loginCookieManager;
this.defaultConnectionName = config.defaultConnectionName();
- this.userInfoProcessor = userInfoProcessor;
+ this.userInfoProcessors = userInfoProcessors.stream()
+ .collect(Collectors.toMap(UserInfoProcessor::connection,
Function.identity()));
this.userInfoEnabled = config.userInfoEnabled();
this.pkceEnabled = config.pkceEnabled();
this.path = config.path();
@@ -278,6 +279,13 @@ public class OidcAuthenticationHandler extends
DefaultAuthenticationFeedbackHand
UserInfo userInfo =
userInfoResponse.toSuccessResponse().getUserInfo();
// process credentials
+ UserInfoProcessor userInfoProcessor =
userInfoProcessors.get(connection.name());
+ if (userInfoProcessor == null) {
+ throw new IllegalStateException(
+ "No matching UserInfoProcessor found for
connection " + connection.name());
+ }
+
+ // Process the user info and token response and return the
processed credentials
return userInfoProcessor.process(
userInfo.toJSONObject().toJSONString(),
tokenResponse.toSuccessResponse().toJSONObject().toJSONString(),
@@ -289,6 +297,7 @@ public class OidcAuthenticationHandler extends
DefaultAuthenticationFeedbackHand
throw new RuntimeException(e);
}
} else {
+ UserInfoProcessor userInfoProcessor =
userInfoProcessors.get(connection.name());
return userInfoProcessor.process(
null,
tokenResponse
diff --git
a/src/main/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImpl.java
b/src/main/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImpl.java
index f680cbd..a41d023 100644
---
a/src/main/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImpl.java
+++
b/src/main/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImpl.java
@@ -45,7 +45,7 @@ import org.slf4j.LoggerFactory;
@Component(
service = UserInfoProcessor.class,
property = {"service.ranking:Integer=10"})
-@Designate(ocd = SlingUserInfoProcessorImpl.Config.class)
+@Designate(ocd = SlingUserInfoProcessorImpl.Config.class, factory = true)
public class SlingUserInfoProcessorImpl implements UserInfoProcessor {
@ObjectClassDefinition(
@@ -67,6 +67,9 @@ public class SlingUserInfoProcessorImpl implements
UserInfoProcessor {
description = "Name of the claim in the ID Token or UserInfo
that contains the groups. "
+ "If not set, the default 'groups' is used")
String groupsClaimName() default "groups";
+
+ @AttributeDefinition(name = "connection", description = "OIDC
Connection Name")
+ String connection();
}
private static final Logger logger =
LoggerFactory.getLogger(SlingUserInfoProcessorImpl.class);
@@ -76,6 +79,7 @@ public class SlingUserInfoProcessorImpl implements
UserInfoProcessor {
private final boolean storeRefreshToken;
private final boolean groupsInIdToken;
private final String groupsClaimName;
+ private final String connection;
@Activate
public SlingUserInfoProcessorImpl(
@@ -85,6 +89,10 @@ public class SlingUserInfoProcessorImpl implements
UserInfoProcessor {
this.storeRefreshToken = config.storeRefreshToken();
this.groupsInIdToken = config.groupsInIdToken();
this.groupsClaimName = config.groupsClaimName();
+ if (config.connection() == null || config.connection().isEmpty()) {
+ throw new IllegalArgumentException("Connection name must not be
null or empty");
+ }
+ this.connection = config.connection();
}
@Override
@@ -187,4 +195,9 @@ public class SlingUserInfoProcessorImpl implements
UserInfoProcessor {
throw new RuntimeException("Failed to parse TokenResponse in
UserInfoProcessor", e);
}
}
+
+ @Override
+ public @NotNull String connection() {
+ return connection;
+ }
}
diff --git
a/src/main/java/org/apache/sling/auth/oauth_client/spi/UserInfoProcessor.java
b/src/main/java/org/apache/sling/auth/oauth_client/spi/UserInfoProcessor.java
index d117a9f..dcb9dea 100644
---
a/src/main/java/org/apache/sling/auth/oauth_client/spi/UserInfoProcessor.java
+++
b/src/main/java/org/apache/sling/auth/oauth_client/spi/UserInfoProcessor.java
@@ -29,25 +29,24 @@ import org.jetbrains.annotations.Nullable;
public interface UserInfoProcessor {
/**
+ * <p>This method is called by the OIDC authentication handler after the
user info and token response have been received from the identity provider.</p>
*
- * <p>This method is called by the OIDC authentication handler after the
user info and token response have been received from the identity provider.
- * If a failure occurs during processing, a {@link RuntimeException}
should be thrown to indicate the failure.
- * </p>
- *
- * @param userInfo the user info received from the identity provider, may
be null if not available. See:
https://openid.net/specs/openid-connect-core-1_0.html#UserInfo
+ * @param userInfo the user info received from the identity provider,
may be null if not available. See:
https://openid.net/specs/openid-connect-core-1_0.html#UserInfo
* @param tokenResponse the token response received from the identity
provider, must not be null. See:
https://openid.net/specs/openid-connect-core-1_0.html#HybridTokenResponse
- * @param oidcSubject the OIDC subject identifier as defined in ID Token,
must not be null
- * @param idp the identity provider identifier as defined in
OidcAuthenticationHandler configuration, must not be null
- * @return the credentials to be returned by the authentication handler,
must not be null
- *
+ * @param oidcSubject the OIDC subject identifier as defined in ID
Token, must not be null
+ * @param idp the identity provider identifier as defined in
OidcAuthenticationHandler configuration, must not be null
* @param userInfo
* @param tokenResponse
* @param oidcSubject
* @param idp
+ * @return the credentials to be returned by the authentication handler,
must not be null
* @return
*/
@NotNull
OidcAuthCredentials process(
@Nullable String userInfo, @NotNull String tokenResponse, @NotNull
String oidcSubject, @NotNull String idp)
throws RuntimeException;
+
+ @NotNull
+ String connection();
}
diff --git
a/src/test/java/org/apache/sling/auth/oauth_client/AuthorizationCodeFlowIT.java
b/src/test/java/org/apache/sling/auth/oauth_client/AuthorizationCodeFlowIT.java
index 1836212..4509bc5 100644
---
a/src/test/java/org/apache/sling/auth/oauth_client/AuthorizationCodeFlowIT.java
+++
b/src/test/java/org/apache/sling/auth/oauth_client/AuthorizationCodeFlowIT.java
@@ -437,7 +437,8 @@ class AuthorizationCodeFlowIT {
null,
Map.of(
"storeAccessToken", "true",
- "storeRefreshToken", "true")));
+ "storeRefreshToken", "true",
+ "connection", oidcConnectionName)));
HashMap<String, Object> authenticationHandlerConfig = new HashMap<>();
authenticationHandlerConfig.put("path", TEST_PATH);
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 282ddaf..781c782 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
@@ -66,6 +66,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.osgi.framework.BundleContext;
+import org.osgi.util.converter.Converters;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.anyString;
@@ -85,7 +86,7 @@ class OidcAuthenticationHandlerTest {
private OidcAuthenticationHandler.Config config;
private LoginCookieManager loginCookieManager;
- private UserInfoProcessor userInfoProcessor;
+ private List<UserInfoProcessor> userInfoProcessors;
private HttpServletRequest request;
private HttpServletResponse response;
private HttpServer tokenEndpointServer;
@@ -103,10 +104,18 @@ class OidcAuthenticationHandlerTest {
when(config.path()).thenReturn(new String[] {"/"});
loginCookieManager = mock(LoginCookieManager.class);
- SlingUserInfoProcessorImpl.Config userInfoConfig =
mock(SlingUserInfoProcessorImpl.Config.class);
- when(userInfoConfig.storeAccessToken()).thenReturn(false);
- when(userInfoConfig.storeRefreshToken()).thenReturn(false);
- userInfoProcessor = new
SlingUserInfoProcessorImpl(mock(CryptoService.class), userInfoConfig);
+ SlingUserInfoProcessorImpl.Config userInfoConfig =
Converters.standardConverter()
+ .convert(Map.of(
+ "storeAccessToken", false,
+ "storeRefreshToken", false,
+ "connection", "mock-oidc-param",
+ "groupsInIdToken", false,
+ "groupsClaimName", "groups"))
+ .to(SlingUserInfoProcessorImpl.Config.class);
+
+ UserInfoProcessor userInfoProcessor = new
SlingUserInfoProcessorImpl(mock(CryptoService.class), userInfoConfig);
+ userInfoProcessors = new ArrayList<>();
+ userInfoProcessors.add(userInfoProcessor);
when(config.userInfoEnabled()).thenReturn(true);
when(config.pkceEnabled()).thenReturn(false);
@@ -382,7 +391,8 @@ class OidcAuthenticationHandlerTest {
rsaJWK,
"http://localhost:4567";,
createMockCookies(),
- false));
+ false,
+ true));
assertEquals(
"Signed JWT rejected: Another algorithm expected, or no
matching key(s) found", exception.getMessage());
}
@@ -399,7 +409,8 @@ class OidcAuthenticationHandlerTest {
rsaJWK,
"http://localhost:4567";,
createMockCookies(),
- false));
+ false,
+ true));
assertEquals("Unexpected JWT audience: [wrong-client-id]",
exception.getMessage());
}
@@ -415,7 +426,8 @@ class OidcAuthenticationHandlerTest {
rsaJWK,
"http://localhost:4567";,
createMockCookies(),
- false));
+ false,
+ true));
assertEquals("Unexpected JWT issuer: wrong-issuer",
exception.getMessage());
}
@@ -429,12 +441,34 @@ class OidcAuthenticationHandlerTest {
rsaJWK,
"http://localhost:4567";,
createMockCookies(),
- false);
+ false,
+ true);
assertEquals("1234567890", authInfo.get("user.name"));
assertEquals(
"testUser", ((OidcAuthCredentials)
authInfo.get("user.jcr.credentials")).getAttribute("profile/name"));
}
+ @Test
+ void
extractCredentials_WithMatchingState_WithValidConnection_WithValidIdToken_WithMissingUserInfo()
+ throws JOSEException {
+ RSAKey rsaJWK = new RSAKeyGenerator(2048).keyID("123").generate();
+ when(config.userInfoEnabled()).thenReturn(true);
+ userInfoProcessors = new ArrayList<>();
+ createOidcAuthenticationHandler();
+
+ // Test with an id token signed by another key, and expired
+ RuntimeException exception = assertThrows(
+ RuntimeException.class,
+ () ->
extractCredentials_WithMatchingState_WithValidConnection_WithIdToken(
+ createIdToken(rsaJWK, "client-id", ISSUER),
+ rsaJWK,
+ "http://localhost:4567";,
+ createMockCookies(),
+ false,
+ true));
+ assertEquals("No matching UserInfoProcessor found for connection
mock-oidc-param", exception.getMessage());
+ }
+
@Test
void
extractCredentials_WithMatchingState_WithValidConnection_WithValidIdToken_WithUserInfo_WithInvalidNonce()
throws JOSEException {
@@ -453,7 +487,8 @@ class OidcAuthenticationHandlerTest {
rsaJWK,
"http://localhost:4567";,
new Cookie[] {stateCookie},
- false));
+ false,
+ true));
assertEquals("Unexpected JWT nonce (nonce) claim: nonce",
exception.getMessage());
}
@@ -478,6 +513,7 @@ class OidcAuthenticationHandlerTest {
rsaJWK,
"http://localhost:4567";,
new Cookie[] {stateCookie},
+ true,
true);
// Remark: presence of state and code verifier parameter are checked
inside
// extractCredentials_WithMatchingState_WithValidConnection_WithIdToken
@@ -495,7 +531,8 @@ class OidcAuthenticationHandlerTest {
rsaJWK,
"http://localhost:4567";,
createMockCookies(),
- false);
+ false,
+ true);
assertEquals("1234567890", authInfo.get("user.name"));
}
@@ -700,7 +737,7 @@ class OidcAuthenticationHandlerTest {
}
private AuthenticationInfo
extractCredentials_WithMatchingState_WithValidConnection_WithIdToken(
- String idToken, RSAKey rsaJWK, String baseUrl, Cookie[] cookies,
boolean withPkce) {
+ String idToken, RSAKey rsaJWK, String baseUrl, Cookie[] cookies,
boolean withPkce, boolean withUserInfo) {
idpServer.createContext("/token", exchange -> {
if (withPkce) {
assertTrue(new String(exchange.getRequestBody().readAllBytes())
@@ -796,7 +833,7 @@ class OidcAuthenticationHandlerTest {
private void createOidcAuthenticationHandler() {
oidcAuthenticationHandler = new OidcAuthenticationHandler(
- bundleContext, connections, config, loginCookieManager,
userInfoProcessor, cryptoService);
+ bundleContext, connections, config, loginCookieManager,
userInfoProcessors, cryptoService);
}
private HttpServer createHttpServer() throws IOException {
diff --git
a/src/test/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImplTest.java
b/src/test/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImplTest.java
index e748053..b296cb8 100644
---
a/src/test/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImplTest.java
+++
b/src/test/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImplTest.java
@@ -20,6 +20,7 @@ package org.apache.sling.auth.oauth_client.impl;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -61,7 +62,8 @@ class SlingUserInfoProcessorImplTest {
"groupsInIdToken", false,
"storeAccessToken", false,
"storeRefreshToken", false,
- "groupsClaimName", "groups"))
+ "groupsClaimName", "groups",
+ "connection", "test"))
.to(SlingUserInfoProcessorImpl.Config.class);
processor = new SlingUserInfoProcessorImpl(cryptoService, cfg);
@@ -137,7 +139,8 @@ class SlingUserInfoProcessorImplTest {
"groupsInIdToken", true,
"storeAccessToken", false,
"storeRefreshToken", false,
- "groupsClaimName", "groups"))
+ "groupsClaimName", "groups",
+ "connection", "test"))
.to(SlingUserInfoProcessorImpl.Config.class);
processor = new SlingUserInfoProcessorImpl(cryptoService, cfg);
@@ -158,7 +161,8 @@ class SlingUserInfoProcessorImplTest {
"groupsInIdToken", false,
"storeAccessToken", true,
"storeRefreshToken", false,
- "groupsClaimName", "groups"))
+ "groupsClaimName", "groups",
+ "connection", "test"))
.to(SlingUserInfoProcessorImpl.Config.class);
processor = new SlingUserInfoProcessorImpl(cryptoService, cfg);
@@ -206,6 +210,23 @@ class SlingUserInfoProcessorImplTest {
});
}
+ @Test
+ void testNullConnection() {
+ Map<String, String> configMap = new HashMap<>();
+ configMap.put("connection", null);
+
+ SlingUserInfoProcessorImpl.Config cfg =
+
Converters.standardConverter().convert(configMap).to(SlingUserInfoProcessorImpl.Config.class);
+
+ try {
+ new SlingUserInfoProcessorImpl(cryptoService, cfg);
+ fail("Expected IllegalArgumentException for null connection name");
+ } catch (IllegalArgumentException e) {
+ // success
+ assertEquals("Connection name must not be null or empty",
e.getMessage());
+ }
+ }
+
private String createTokenResponse(String accessToken, String
refreshToken) throws Exception {
// Create a properly formatted OAuth 2.0 token response
JSONObject tokenResponse = new JSONObject();
