From 7eded8d4f45041a93751d7d47a6b1a6363521203 Mon Sep 17 00:00:00 2001 From: Prarthona Paul Date: Thu, 21 Mar 2024 11:35:54 -0400 Subject: [PATCH] [squash] addressed comments --- .../http/oidc/OidcRequestAuthenticator.java | 12 ++++----- .../http/oidc/KeycloakConfiguration.java | 27 ++++++++++++++----- .../security/http/oidc/OidcBaseTest.java | 1 + .../wildfly/security/http/oidc/OidcTest.java | 16 +++++------ 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java b/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java index fe7bbf413d6..dbb3f056874 100644 --- a/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java +++ b/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java @@ -169,7 +169,7 @@ protected String getRedirectUri(String state) { List forwardableQueryParams = Arrays.asList(LOGIN_HINT, DOMAIN_HINT, KC_IDP_HINT, PROMPT, MAX_AGE, UI_LOCALES, SCOPE); List forwardedQueryParams = new ArrayList<>(forwardableQueryParams.size()); Set allScopes = new HashSet<>(); - addScope(deployment.getScope(), allScopes); + addScopes(deployment.getScope(), allScopes); for (String paramName : forwardableQueryParams) { String paramValue = getQueryParamValue(facade, paramName); @@ -425,21 +425,19 @@ private static boolean hasScope(String scopeParam, String targetScope) { private String combineAndReorderScopes(Set allScopes, String paramValue) { StringBuilder combinedScopes = new StringBuilder(); - addScope(paramValue, allScopes); + addScopes(paramValue, allScopes); - if (allScopes.contains(OIDC_SCOPE)) { //some OpenID providers require openid scope to be added in the beginning - allScopes.remove(OIDC_SCOPE); - } + //some OpenID providers require openid scope to be added in the beginning combinedScopes.append(OIDC_SCOPE); for (String scope : allScopes) { - if (scope != null && !scope.isEmpty()) { + if (!scope.equals(OIDC_SCOPE)) { combinedScopes.append(" ").append(scope); } } return combinedScopes.toString(); } - private void addScope(String scopes, Set allScopes) { + private void addScopes(String scopes, Set allScopes) { if (scopes != null && !scopes.isEmpty()) { allScopes.addAll(Arrays.asList(scopes.split("\\s+"))); } diff --git a/http/oidc/src/test/java/org/wildfly/security/http/oidc/KeycloakConfiguration.java b/http/oidc/src/test/java/org/wildfly/security/http/oidc/KeycloakConfiguration.java index 6f27fe3a6a5..bbe6e091e5e 100644 --- a/http/oidc/src/test/java/org/wildfly/security/http/oidc/KeycloakConfiguration.java +++ b/http/oidc/src/test/java/org/wildfly/security/http/oidc/KeycloakConfiguration.java @@ -33,6 +33,8 @@ import io.restassured.RestAssured; +import static org.wildfly.security.http.oidc.Oidc.OIDC_SCOPE; + /** * Keycloak configuration for testing. * @@ -61,8 +63,8 @@ public class KeycloakConfiguration { * */ public static RealmRepresentation getRealmRepresentation(final String realmName, String clientId, String clientSecret, - String clientHostName, int clientPort, String clientApp) { - return createRealm(realmName, clientId, clientSecret, clientHostName, clientPort, clientApp); + String clientHostName, int clientPort, String clientApp, boolean configureClientScopes) { + return createRealm(realmName, clientId, clientSecret, clientHostName, clientPort, clientApp, configureClientScopes); } public static RealmRepresentation getRealmRepresentation(final String realmName, String clientId, String clientSecret, @@ -102,15 +104,22 @@ public static String getAccessToken(String authServerUrl, String realmName, Stri .as(AccessTokenResponse.class).getToken(); } + private static RealmRepresentation createRealm(final String realmName, String clientId, String clientSecret, + String clientHostName, int clientPort, String clientApp, + boolean directAccessGrantEnabled, String bearerOnlyClientId, + String corsClientId) { + return createRealm(realmName, clientId, clientSecret, clientHostName, clientPort, clientApp, directAccessGrantEnabled, bearerOnlyClientId, corsClientId, false); + } + private static RealmRepresentation createRealm(String name, String clientId, String clientSecret, - String clientHostName, int clientPort, String clientApp) { - return createRealm(name, clientId, clientSecret, clientHostName, clientPort, clientApp, false, null, null); + String clientHostName, int clientPort, String clientApp, boolean configureClientScopes) { + return createRealm(name, clientId, clientSecret, clientHostName, clientPort, clientApp, false, null, null, configureClientScopes); } private static RealmRepresentation createRealm(String name, String clientId, String clientSecret, String clientHostName, int clientPort, String clientApp, boolean directAccessGrantEnabled, String bearerOnlyClientId, - String corsClientId) { + String corsClientId, boolean configureClientScopes) { RealmRepresentation realm = new RealmRepresentation(); realm.setRealm(name); @@ -128,8 +137,12 @@ private static RealmRepresentation createRealm(String name, String clientId, Str realm.getRoles().getRealm().add(new RoleRepresentation("user", null, false)); realm.getRoles().getRealm().add(new RoleRepresentation("admin", null, false)); - - realm.getClients().add(createWebAppClient(clientId, clientSecret, clientHostName, clientPort, clientApp, directAccessGrantEnabled)); + ClientRepresentation webAppClient = createWebAppClient(clientId, clientSecret, clientHostName, clientPort, clientApp, directAccessGrantEnabled); + if (configureClientScopes) { + webAppClient.setDefaultClientScopes(Collections.singletonList(OIDC_SCOPE)); + webAppClient.setOptionalClientScopes(Arrays.asList("phone", "email", "profile")); + } + realm.getClients().add(webAppClient); if (bearerOnlyClientId != null) { realm.getClients().add(createBearerOnlyClient(bearerOnlyClientId)); 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 7a3a2b7b786..f7a90bc22d7 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 @@ -82,6 +82,7 @@ public class OidcBaseTest extends AbstractBaseHttpTest { public static final String CLIENT_PAGE_TEXT = "Welcome page!"; public static final String CLIENT_HOST_NAME = "localhost"; public static MockWebServer client; // to simulate the application being secured + public static final Boolean CONFIGURE_CLIENT_SCOPES = true; // to simulate the application being secured protected HttpServerAuthenticationMechanismFactory oidcFactory; 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 4839d85720e..65a67f8cb2d 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 @@ -57,7 +57,7 @@ public static void startTestContainers() throws Exception { assumeTrue("Docker isn't available, OIDC tests will be skipped", isDockerAvailable()); KEYCLOAK_CONTAINER = new KeycloakContainer(); KEYCLOAK_CONTAINER.start(); - sendRealmCreationRequest(KeycloakConfiguration.getRealmRepresentation(TEST_REALM, CLIENT_ID, CLIENT_SECRET, CLIENT_HOST_NAME, CLIENT_PORT, CLIENT_APP)); + sendRealmCreationRequest(KeycloakConfiguration.getRealmRepresentation(TEST_REALM, CLIENT_ID, CLIENT_SECRET, CLIENT_HOST_NAME, CLIENT_PORT, CLIENT_APP, CONFIGURE_CLIENT_SCOPES)); client = new MockWebServer(); client.start(CLIENT_PORT); } @@ -166,41 +166,41 @@ public void testTokenSignatureAlgorithm() throws Exception { @Test public void testInvalidScope() throws Exception { String expectedScope = OIDC_SCOPE + "+INVALID_SCOPE"; - performAuthentication(getOidcConfigurationInputStreamWithScope(CLIENT_SECRET, "INVALID_SCOPE"), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, + performAuthentication(getOidcConfigurationInputStreamWithScope("INVALID_SCOPE"), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), "error=invalid_scope", expectedScope, true); } @Test public void testEmptyScope() throws Exception { - performAuthentication(getOidcConfigurationInputStreamWithScope(CLIENT_SECRET, ""), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, + performAuthentication(getOidcConfigurationInputStreamWithScope(""), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, OIDC_SCOPE, false); } @Test public void testSingleScopeValue() throws Exception { String expectedScope = OIDC_SCOPE + "+profile"; - performAuthentication(getOidcConfigurationInputStreamWithScope(CLIENT_SECRET, "profile"), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, + performAuthentication(getOidcConfigurationInputStreamWithScope("profile"), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, expectedScope, false); } @Test public void testMultipleScopeValue() throws Exception { String expectedScope = OIDC_SCOPE + "+phone+profile+email"; - performAuthentication(getOidcConfigurationInputStreamWithScope(CLIENT_SECRET, "email phone profile"), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, + performAuthentication(getOidcConfigurationInputStreamWithScope("email phone profile"), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, expectedScope, false); } @Test public void testOpenIDScopeValue() throws Exception { String expectedScope = OIDC_SCOPE; - performAuthentication(getOidcConfigurationInputStreamWithScope(CLIENT_SECRET, OIDC_SCOPE), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, + performAuthentication(getOidcConfigurationInputStreamWithScope(OIDC_SCOPE), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, expectedScope, false); } @Test public void testOpenIDWithMultipleScopeValue() throws Exception { String expectedScope = OIDC_SCOPE + "+phone+profile+email";//order gets changed when combining with query parameters - performAuthentication(getOidcConfigurationInputStreamWithScope(CLIENT_SECRET, "email phone profile " + OIDC_SCOPE), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, + performAuthentication(getOidcConfigurationInputStreamWithScope("email phone profile " + OIDC_SCOPE), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD, true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT, expectedScope, false); } @@ -354,7 +354,7 @@ private InputStream getOidcConfigurationInputStreamWithTokenSignatureAlgorithm() return new ByteArrayInputStream(oidcConfig.getBytes(StandardCharsets.UTF_8)); } - private InputStream getOidcConfigurationInputStreamWithScope(String clientSecret, String scopeValue){ + private InputStream getOidcConfigurationInputStreamWithScope(String scopeValue){ String oidcConfig = "{\n" + " \"client-id\" : \"" + CLIENT_ID + "\",\n" + " \"provider-url\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "/" + "\",\n" +