From 937f7a8f299df27ad69c952ccb759d8caedfcbca Mon Sep 17 00:00:00 2001 From: Farah Juma Date: Wed, 24 Apr 2024 17:17:29 -0400 Subject: [PATCH] [ELY-2752] Ensure it's possible to make use of a custom principal-attribute value for OIDC --- .../security/http/oidc/ElytronMessages.java | 5 +++ .../security/http/oidc/JsonWebToken.java | 9 +++- .../security/http/oidc/OidcBaseTest.java | 7 +++ .../wildfly/security/http/oidc/OidcTest.java | 43 ++++++++++++++++++- 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/http/oidc/src/main/java/org/wildfly/security/http/oidc/ElytronMessages.java b/http/oidc/src/main/java/org/wildfly/security/http/oidc/ElytronMessages.java index c4ba08c8fb2..773b59d8bb3 100644 --- a/http/oidc/src/main/java/org/wildfly/security/http/oidc/ElytronMessages.java +++ b/http/oidc/src/main/java/org/wildfly/security/http/oidc/ElytronMessages.java @@ -18,6 +18,7 @@ package org.wildfly.security.http.oidc; +import static org.jboss.logging.Logger.Level.DEBUG; import static org.jboss.logging.Logger.Level.ERROR; import static org.jboss.logging.Logger.Level.WARN; import static org.jboss.logging.annotations.Message.NONE; @@ -233,5 +234,9 @@ interface ElytronMessages extends BasicLogger { @Message(id = 23056, value = "No message entity") IOException noMessageEntity(); + @LogMessage(level = DEBUG) + @Message(id = 23057, value = "principal-attribute '%s' claim does not exist, falling back to 'sub'") + void principalAttributeClaimDoesNotExist(String principalAttributeClaim); + } diff --git a/http/oidc/src/main/java/org/wildfly/security/http/oidc/JsonWebToken.java b/http/oidc/src/main/java/org/wildfly/security/http/oidc/JsonWebToken.java index 1b27f19a031..b806a0e7122 100644 --- a/http/oidc/src/main/java/org/wildfly/security/http/oidc/JsonWebToken.java +++ b/http/oidc/src/main/java/org/wildfly/security/http/oidc/JsonWebToken.java @@ -297,7 +297,14 @@ public String getPrincipalName(OidcClientConfiguration deployment) { case NICKNAME: return getNickName(); default: - return getSubject(); + String claimValue = getClaimValueAsString(attr); + if (claimValue != null) { + return claimValue; + } else { + // fall back to sub claim + log.principalAttributeClaimDoesNotExist(attr); + return getSubject(); + } } } diff --git a/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java b/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java index cee8c1b11eb..b0a2c45e79b 100644 --- a/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java +++ b/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java @@ -127,11 +127,18 @@ protected static boolean isDockerAvailable() { } protected CallbackHandler getCallbackHandler() { + return getCallbackHandler(null); + } + + protected CallbackHandler getCallbackHandler(String expectedPrincipal) { return callbacks -> { for(Callback callback : callbacks) { if (callback instanceof EvidenceVerifyCallback) { Evidence evidence = ((EvidenceVerifyCallback) callback).getEvidence(); ((EvidenceVerifyCallback) callback).setVerified(evidence.getDecodedPrincipal() != null); + if (expectedPrincipal != null) { + assertEquals(expectedPrincipal, evidence.getDecodedPrincipal().getName()); + } } else if (callback instanceof AuthenticationCompleteCallback) { // NO-OP } else if (callback instanceof IdentityCredentialCallback) { diff --git a/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcTest.java b/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcTest.java index 7b211c9e38e..b1c82528553 100644 --- a/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcTest.java +++ b/http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcTest.java @@ -39,6 +39,8 @@ import java.util.HashMap; import java.util.Map; +import javax.security.auth.callback.CallbackHandler; + import org.apache.http.HttpStatus; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -188,6 +190,24 @@ public void testTokenSignatureAlgorithm() throws Exception { true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT); } + @Test + public void testPrincipalAttribute() throws Exception { + // custom principal-attribute + loginToAppMultiTenancy(getOidcConfigurationInputStreamWithPrincipalAttribute("aud"), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, + getCallbackHandler("test-webapp")); + + // standard principal-attribute + loginToAppMultiTenancy(getOidcConfigurationInputStreamWithPrincipalAttribute("given_name"), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, + getCallbackHandler("Alice")); + + // invalid principal-attribute, logging in should still succeed + loginToAppMultiTenancy(getOidcConfigurationInputStreamWithPrincipalAttribute("invalid_claim"), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, + getCallbackHandler()); + } + /***************************************************************************************************************************************** * Tests for multi-tenancy. * @@ -365,6 +385,13 @@ private void testNonExistingUser(String username, String password, String tenant private void loginToAppMultiTenancy(InputStream oidcConfig, String username, String password, boolean loginToKeycloak, int expectedDispatcherStatusCode, String expectedLocation, String clientPageText) throws Exception { + loginToAppMultiTenancy(oidcConfig, username, password, loginToKeycloak, expectedDispatcherStatusCode, expectedLocation, clientPageText, + getCallbackHandler()); + } + + private void loginToAppMultiTenancy(InputStream oidcConfig, String username, String password, boolean loginToKeycloak, + int expectedDispatcherStatusCode, String expectedLocation, String clientPageText, + CallbackHandler callbackHandler) throws Exception { try { Map props = new HashMap<>(); OidcClientConfiguration oidcClientConfiguration = OidcClientConfigurationBuilder.build(oidcConfig); @@ -372,7 +399,7 @@ private void loginToAppMultiTenancy(InputStream oidcConfig, String username, Str OidcClientContext oidcClientContext = new OidcClientContext(oidcClientConfiguration); oidcFactory = new OidcMechanismFactory(oidcClientContext); - HttpServerAuthenticationMechanism mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, getCallbackHandler()); + HttpServerAuthenticationMechanism mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, callbackHandler); URI requestUri = new URI(getClientUrl()); TestingHttpServerRequest request = new TestingHttpServerRequest(null, requestUri); @@ -545,6 +572,20 @@ private InputStream getOidcConfigurationInputStreamWithTokenSignatureAlgorithm() return new ByteArrayInputStream(oidcConfig.getBytes(StandardCharsets.UTF_8)); } + private InputStream getOidcConfigurationInputStreamWithPrincipalAttribute(String principalAttributeValue) { + String oidcConfig = "{\n" + + " \"principal-attribute\" : \"" + principalAttributeValue + "\",\n" + + " \"resource\" : \"" + CLIENT_ID + "\",\n" + + " \"public-client\" : \"false\",\n" + + " \"provider-url\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "\",\n" + + " \"ssl-required\" : \"EXTERNAL\",\n" + + " \"credentials\" : {\n" + + " \"secret\" : \"" + CLIENT_SECRET + "\"\n" + + " }\n" + + "}"; + return new ByteArrayInputStream(oidcConfig.getBytes(StandardCharsets.UTF_8)); + } + static InputStream getTenantConfigWithAuthServerUrl(String tenant) { String oidcConfig = "{\n" + " \"realm\" : \"" + tenant + "\",\n" +