[ELY-2752] Ensure it's possible to make use of a custom principal-att…

…ribute value for OIDC
wildfly-security · May 13, 2024 · 937f7a8 · 937f7a8
1 parent 73a1023
commit 937f7a8
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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,14 +385,21 @@ 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<String, Object> props = new HashMap<>();
             OidcClientConfiguration oidcClientConfiguration = OidcClientConfigurationBuilder.build(oidcConfig);
             assertEquals(OidcClientConfiguration.RelativeUrlsUsed.NEVER, oidcClientConfiguration.getRelativeUrls());
 
             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" +