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 6353159
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 31 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
17 changes: 9 additions & 8 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 Expand Up @@ -124,10 +125,10 @@ protected static boolean isDockerAvailable() {
}
}
protected CallbackHandler getCallbackHandler() {
return getCallbackHandler(false);
return getCallbackHandler(false, null);
}

protected CallbackHandler getCallbackHandler(boolean checkScope) {
protected CallbackHandler getCallbackHandler(boolean checkScope, String expectedScopes) {
return callbacks -> {
for(Callback callback : callbacks) {
if (callback instanceof EvidenceVerifyCallback) {
Expand All @@ -138,7 +139,7 @@ protected CallbackHandler getCallbackHandler(boolean checkScope) {
} else if (callback instanceof IdentityCredentialCallback) {
if (checkScope) {
try {
checkForScopeClaims(callback);
checkForScopeClaims(callback, expectedScopes);
} catch (InvalidJwtException e) {
throw new RuntimeException(e);
}
Expand Down Expand Up @@ -231,16 +232,16 @@ protected String getCookieString(HttpServerCookie cookie) {
return header.toString();
}

protected void checkForScopeClaims(Callback callback) throws InvalidJwtException {
protected void checkForScopeClaims(Callback callback, String expectedScopes) throws InvalidJwtException {
Credential credential = ((IdentityCredentialCallback)callback).getCredential();
String token = ((BearerTokenCredential) credential).getToken();
JwtClaims jwtClaims = new JwtConsumerBuilder().setSkipSignatureVerification().setSkipAllValidators().build().processToClaims(token);
String scopes = jwtClaims.getClaimValueAsString("scope");
if (scopes != null) {
if (scopes.contains("email")) {

if (expectedScopes != null) {
if (expectedScopes.contains("email")) {
assertTrue(jwtClaims.getClaimValueAsString("email_verified").contains(String.valueOf(KeycloakConfiguration.EMAIL_VERIFIED)));
}
if (scopes.contains("profile")) {
if (expectedScopes.contains("profile")) {
assertTrue(jwtClaims.getClaimValueAsString("preferred_username").contains(KeycloakConfiguration.ALICE));
}
}
Expand Down
18 changes: 9 additions & 9 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 All @@ -223,7 +223,7 @@ private void performAuthentication(InputStream oidcConfig, String username, Stri
if (expectedScope == null) {
mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, getCallbackHandler());
} else {
mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, getCallbackHandler(true));
mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, getCallbackHandler(true, expectedScope));
}

URI requestUri = new URI(getClientUrl());
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 6353159

Please sign in to comment.