From 13da686515b2ca83a24204c2c74a51750c600a8d 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 | 25 +++++++++++++-- .../wildfly/security/http/oidc/OidcTest.java | 32 +++++++++++++++++++ 4 files changed, 68 insertions(+), 3 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 da8efee9981..fb9d8345431 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 @@ -134,11 +134,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) { @@ -304,7 +311,21 @@ protected void performAuthentication(InputStream oidcConfig, String username, St } protected void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak, - int expectedDispatcherStatusCode, String clientUrl, String expectedLocation, String clientPageText) throws Exception { + int expectedDispatcherStatusCode, String expectedLocation, String clientPageText, + CallbackHandler callbackHandler) throws Exception { + performAuthentication(oidcConfig, username, password, loginToKeycloak, expectedDispatcherStatusCode, getClientUrl(), expectedLocation, clientPageText, + callbackHandler); + } + + protected void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak, + int expectedDispatcherStatusCode, String clientUrl, String expectedLocation, String clientPageText) throws Exception { + performAuthentication(oidcConfig, username, password, loginToKeycloak, expectedDispatcherStatusCode, clientUrl, expectedLocation, clientPageText, + getCallbackHandler()); + } + + protected void performAuthentication(InputStream oidcConfig, String username, String password, boolean loginToKeycloak, + int expectedDispatcherStatusCode, String clientUrl, String expectedLocation, String clientPageText, + CallbackHandler callbackHandler) throws Exception { try { Map props = new HashMap<>(); OidcClientConfiguration oidcClientConfiguration = OidcClientConfigurationBuilder.build(oidcConfig); @@ -312,7 +333,7 @@ protected void performAuthentication(InputStream oidcConfig, String username, St 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(clientUrl); TestingHttpServerRequest request = new TestingHttpServerRequest(null, requestUri); 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 4716f78d6c5..a9b13687551 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 @@ -188,6 +188,24 @@ public void testTokenSignatureAlgorithm() throws Exception { true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT); } + @Test + public void testPrincipalAttribute() throws Exception { + // custom principal-attribute + performAuthentication(getOidcConfigurationInputStreamWithPrincipalAttribute("aud"), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, + getCallbackHandler("test-webapp")); + + // standard principal-attribute + performAuthentication(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 + performAuthentication(getOidcConfigurationInputStreamWithPrincipalAttribute("invalid_claim"), KeycloakConfiguration.ALICE, + KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, + getCallbackHandler()); + } + /***************************************************************************************************************************************** * Tests for multi-tenancy. * @@ -503,6 +521,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" +