Skip to content

Commit

Permalink
[squash] addressed comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@PrarthonaPaul
PrarthonaPaul committed Mar 21, 2024
1 parent f42da1e commit 7eded8d
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 22 deletions.
12 changes: 5 additions & 7 deletions http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java
View file
Expand Up @@ -169,7 +169,7 @@ protected String getRedirectUri(String state) {
List<String> forwardableQueryParams = Arrays.asList(LOGIN_HINT, DOMAIN_HINT, KC_IDP_HINT, PROMPT, MAX_AGE, UI_LOCALES, SCOPE);
List<NameValuePair> forwardedQueryParams = new ArrayList<>(forwardableQueryParams.size());
Set<String> allScopes = new HashSet<>();
addScope(deployment.getScope(), allScopes);
addScopes(deployment.getScope(), allScopes);

for (String paramName : forwardableQueryParams) {
String paramValue = getQueryParamValue(facade, paramName);
Expand Down Expand Up @@ -425,21 +425,19 @@ private static boolean hasScope(String scopeParam, String targetScope) {

private String combineAndReorderScopes(Set<String> 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<String> allScopes) {
private void addScopes(String scopes, Set<String> allScopes) {
if (scopes != null && !scopes.isEmpty()) {
allScopes.addAll(Arrays.asList(scopes.split("\\s+")));
}
Expand Down
27 changes: 20 additions & 7 deletions http/oidc/src/test/java/org/wildfly/security/http/oidc/KeycloakConfiguration.java
View file
Expand Up @@ -33,6 +33,8 @@

import io.restassured.RestAssured;

import static org.wildfly.security.http.oidc.Oidc.OIDC_SCOPE;

/**
* Keycloak configuration for testing.
*
Expand Down Expand Up @@ -61,8 +63,8 @@ public class KeycloakConfiguration {
* </ul>
*/
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,
Expand Down Expand Up @@ -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);
Expand All @@ -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));
Expand Down
1 change: 1 addition & 0 deletions http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java
View file
Expand Up @@ -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;

Expand Down
16 changes: 8 additions & 8 deletions http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcTest.java
View file
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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" +
Expand Down

0 comments on commit 7eded8d

Please sign in to comment.