[KEYCLOAK-8071] - Properly validating requested scopes
This commit is contained in:
parent
38195ca789
commit
cbab159aa8
8 changed files with 153 additions and 35 deletions
|
@ -77,7 +77,9 @@ import java.util.Arrays;
|
|||
import java.util.Collection;
|
||||
import java.util.HashSet;
|
||||
import java.util.Map;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
/**
|
||||
* Stateless object that creates tokens and manages oauth access codes
|
||||
|
@ -510,8 +512,7 @@ public class TokenManager {
|
|||
}
|
||||
|
||||
// Add optional client scopes requested by scope parameter
|
||||
String[] scopes = scopeParam.split(" ");
|
||||
Collection<String> scopeParamParts = Arrays.asList(scopes);
|
||||
Collection<String> scopeParamParts = parseScopeParameter(scopeParam);
|
||||
Map<String, ClientScopeModel> allOptionalScopes = client.getClientScopes(false, true);
|
||||
for (String scopeParamPart : scopeParamParts) {
|
||||
ClientScopeModel scope = allOptionalScopes.get(scopeParamPart);
|
||||
|
@ -522,6 +523,39 @@ public class TokenManager {
|
|||
|
||||
return clientScopes;
|
||||
}
|
||||
|
||||
public static boolean isValidScope(String scopes, ClientModel client) {
|
||||
if (scopes == null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
Set<String> clientScopes = getRequestedClientScopes(scopes, client).stream()
|
||||
.filter(obj -> !ClientModel.class.isInstance(obj))
|
||||
.map(ClientScopeModel::getName)
|
||||
.collect(Collectors.toSet());
|
||||
Collection<String> requestedScopes = TokenManager.parseScopeParameter(scopes);
|
||||
|
||||
if (TokenUtil.isOIDCRequest(scopes)) {
|
||||
requestedScopes.remove(OAuth2Constants.SCOPE_OPENID);
|
||||
}
|
||||
|
||||
if (!requestedScopes.isEmpty() && clientScopes.isEmpty()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
for (String requestedScope : requestedScopes) {
|
||||
// we also check dynamic scopes in case the client is from a provider that dynamically provides scopes to their clients
|
||||
if (!clientScopes.contains(requestedScope) && client.getDynamicClientScope(requestedScope) == null) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
public static Collection<String> parseScopeParameter(String scopeParam) {
|
||||
return new HashSet<>(Arrays.asList(scopeParam.split(" ")));
|
||||
}
|
||||
|
||||
// Check if user still has granted consents to all requested client scopes
|
||||
public static boolean verifyConsentStillAvailable(KeycloakSession session, UserModel user, ClientModel client, Set<ClientScopeModel> requestedClientScopes) {
|
||||
|
|
|
@ -34,6 +34,7 @@ import org.keycloak.models.RealmModel;
|
|||
import org.keycloak.protocol.AuthorizationEndpointBase;
|
||||
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
|
||||
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
|
||||
import org.keycloak.protocol.oidc.TokenManager;
|
||||
import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest;
|
||||
import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequestParserProcessor;
|
||||
import org.keycloak.protocol.oidc.utils.OIDCRedirectUriBuilder;
|
||||
|
@ -135,6 +136,12 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
|
|||
ServicesLogger.LOGGER.oidcScopeMissing();
|
||||
}
|
||||
|
||||
if (!TokenManager.isValidScope(request.getScope(), client)) {
|
||||
ServicesLogger.LOGGER.invalidParameter(OIDCLoginProtocol.SCOPE_PARAM);
|
||||
event.error(Errors.INVALID_REQUEST);
|
||||
return redirectErrorToClient(parsedResponseMode, OAuthErrorException.INVALID_REQUEST, "Invalid scopes: " + request.getScope());
|
||||
}
|
||||
|
||||
errorResponse = checkOIDCParams();
|
||||
if (errorResponse != null) {
|
||||
return errorResponse;
|
||||
|
|
|
@ -567,7 +567,7 @@ public class TokenEndpoint {
|
|||
event.error(Errors.CONSENT_DENIED);
|
||||
throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_CLIENT, "Client requires user consent", Response.Status.BAD_REQUEST);
|
||||
}
|
||||
String scope = formParams.getFirst(OAuth2Constants.SCOPE);
|
||||
String scope = getRequestedScopes();
|
||||
|
||||
RootAuthenticationSessionModel rootAuthSession = new AuthenticationSessionManager(session).createAuthenticationSession(realm, false);
|
||||
AuthenticationSessionModel authSession = rootAuthSession.createAuthenticationSession(client);
|
||||
|
@ -657,7 +657,7 @@ public class TokenEndpoint {
|
|||
throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_REQUEST, "User '" + clientUsername + "' disabled", Response.Status.UNAUTHORIZED);
|
||||
}
|
||||
|
||||
String scope = formParams.getFirst(OAuth2Constants.SCOPE);
|
||||
String scope = getRequestedScopes();
|
||||
|
||||
RootAuthenticationSessionModel rootAuthSession = new AuthenticationSessionManager(session).createAuthenticationSession(realm, false);
|
||||
AuthenticationSessionModel authSession = rootAuthSession.createAuthenticationSession(client);
|
||||
|
@ -698,6 +698,18 @@ public class TokenEndpoint {
|
|||
return cors.builder(Response.ok(res, MediaType.APPLICATION_JSON_TYPE)).build();
|
||||
}
|
||||
|
||||
private String getRequestedScopes() {
|
||||
String scope = formParams.getFirst(OAuth2Constants.SCOPE);
|
||||
|
||||
if (!TokenManager.isValidScope(scope, client)) {
|
||||
event.error(Errors.INVALID_REQUEST);
|
||||
throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_SCOPE, "Invalid scopes: " + scope,
|
||||
Status.BAD_REQUEST);
|
||||
}
|
||||
|
||||
return scope;
|
||||
}
|
||||
|
||||
public Response tokenExchange() {
|
||||
ProfileHelper.requireFeature(Profile.Feature.TOKEN_EXCHANGE);
|
||||
|
||||
|
|
|
@ -89,7 +89,7 @@ public class AccountBrokerTest extends AbstractBaseBrokerTest {
|
|||
log.debug("adding client " + client.getName() + " to realm " + bc.providerRealmName());
|
||||
|
||||
// Remove default client scopes for this test
|
||||
client.setDefaultClientScopes(Collections.emptyList());
|
||||
// client.setDefaultClientScopes(Collections.emptyList());
|
||||
|
||||
fixAuthServerHostAndPortForClientRepresentation(client);
|
||||
|
||||
|
@ -150,7 +150,7 @@ public class AccountBrokerTest extends AbstractBaseBrokerTest {
|
|||
Assert.assertEquals(1, identities.size());
|
||||
|
||||
Assert.assertEquals("kc-oidc-idp", identities.get(0).getProvider());
|
||||
Assert.assertEquals("user@localhost.com", identities.get(0).getSubject());
|
||||
Assert.assertEquals("testuser", identities.get(0).getSubject());
|
||||
Assert.assertEquals("remove-link-kc-oidc-idp", identities.get(0).getAction().getAttribute("id"));
|
||||
|
||||
identities.get(0).getAction().click();
|
||||
|
|
|
@ -210,7 +210,6 @@ public class OAuthGrantTest extends AbstractKeycloakTest {
|
|||
// Grant permissions on grant screen
|
||||
oauth.clientId(THIRD_PARTY_APP);
|
||||
oauth.doLoginGrant("test-user@localhost", "password");
|
||||
oauth.scope(OAuth2Constants.GRANT_TYPE);
|
||||
|
||||
// Create new clientScope and add to client
|
||||
RealmResource appRealm = adminClient.realm(REALM_NAME);
|
||||
|
|
|
@ -1,6 +1,8 @@
|
|||
package org.keycloak.testsuite.oauth;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson;
|
||||
|
||||
import java.util.Arrays;
|
||||
|
@ -12,7 +14,11 @@ import javax.ws.rs.core.Response;
|
|||
import org.junit.Assert;
|
||||
import org.junit.Test;
|
||||
import org.keycloak.OAuth2Constants;
|
||||
import org.keycloak.OAuthErrorException;
|
||||
import org.keycloak.admin.client.resource.ClientResource;
|
||||
import org.keycloak.admin.client.resource.ClientsResource;
|
||||
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
|
||||
import org.keycloak.representations.idm.ClientRepresentation;
|
||||
import org.keycloak.representations.idm.ClientScopeRepresentation;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.testsuite.AbstractKeycloakTest;
|
||||
|
@ -49,12 +55,12 @@ public class OAuthScopeInTokenResponseTest extends AbstractKeycloakTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
public void specifySingleNotExistingScopeTest() throws Exception {
|
||||
public void specifyEmptyScopeTest() throws Exception {
|
||||
String loginUser = "john-doh@localhost";
|
||||
String loginPassword = "password";
|
||||
String clientSecret = "password";
|
||||
|
||||
String requestedScope = "user";
|
||||
String requestedScope = "";
|
||||
String expectedScope = "openid profile email";
|
||||
|
||||
oauth.scope(requestedScope);
|
||||
|
@ -64,6 +70,88 @@ public class OAuthScopeInTokenResponseTest extends AbstractKeycloakTest {
|
|||
|
||||
expectSuccessfulResponseFromTokenEndpoint(code, expectedScope, clientSecret);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void failCodeNotExistingScope() throws Exception {
|
||||
String loginUser = "john-doh@localhost";
|
||||
String loginPassword = "password";
|
||||
String clientSecret = "password";
|
||||
|
||||
ClientsResource clients = realmsResouce().realm("test").clients();
|
||||
ClientRepresentation clientRep = clients.findByClientId(oauth.getClientId()).get(0);
|
||||
ClientResource client = clients.get(clientRep.getId());
|
||||
List<ClientScopeRepresentation> scopes = client.getDefaultClientScopes();
|
||||
|
||||
for (ClientScopeRepresentation scope : scopes) {
|
||||
client.removeDefaultClientScope(scope.getId());
|
||||
}
|
||||
|
||||
oauth.openid(false);
|
||||
|
||||
oauth.scope("user openid phone");
|
||||
oauth.openLoginForm();
|
||||
assertTrue(driver.getCurrentUrl().contains("error_description=Invalid+scopes"));
|
||||
|
||||
oauth.scope("user");
|
||||
oauth.openLoginForm();
|
||||
assertTrue(driver.getCurrentUrl().contains("error_description=Invalid+scopes"));
|
||||
|
||||
oauth.scope("phone");
|
||||
oauth.doLogin(loginUser, loginPassword);
|
||||
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
|
||||
expectSuccessfulResponseFromTokenEndpoint(code, "phone", clientSecret);
|
||||
|
||||
oauth.openLogout();
|
||||
oauth.scope(null);
|
||||
oauth.doLogin(loginUser, loginPassword);
|
||||
code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
|
||||
expectSuccessfulResponseFromTokenEndpoint(code, "", clientSecret);
|
||||
|
||||
for (ClientScopeRepresentation scope : scopes) {
|
||||
client.addDefaultClientScope(scope.getId());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void failTokenNotExistingScope() throws Exception {
|
||||
String loginUser = "john-doh@localhost";
|
||||
String loginPassword = "password";
|
||||
String clientSecret = "password";
|
||||
|
||||
ClientsResource clients = realmsResouce().realm("test").clients();
|
||||
ClientRepresentation clientRep = clients.findByClientId(oauth.getClientId()).get(0);
|
||||
clientRep.setDirectAccessGrantsEnabled(true);
|
||||
ClientResource client = clients.get(clientRep.getId());
|
||||
client.update(clientRep);
|
||||
|
||||
List<ClientScopeRepresentation> scopes = client.getDefaultClientScopes();
|
||||
|
||||
for (ClientScopeRepresentation scope : scopes) {
|
||||
client.removeDefaultClientScope(scope.getId());
|
||||
}
|
||||
|
||||
oauth.openid(false);
|
||||
oauth.scope("user phone");
|
||||
OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest(clientSecret, loginUser, loginPassword);
|
||||
|
||||
assertNotNull(response.getError());
|
||||
assertEquals(OAuthErrorException.INVALID_SCOPE, response.getError());
|
||||
|
||||
oauth.scope("user");
|
||||
response = oauth.doGrantAccessTokenRequest(clientSecret, loginUser, loginPassword);
|
||||
|
||||
assertNotNull(response.getError());
|
||||
assertEquals(OAuthErrorException.INVALID_SCOPE, response.getError());
|
||||
|
||||
oauth.scope(null);
|
||||
response = oauth.doGrantAccessTokenRequest(clientSecret, loginUser, loginPassword);
|
||||
|
||||
assertNotNull(response.getAccessToken());
|
||||
|
||||
for (ClientScopeRepresentation scope : scopes) {
|
||||
client.addDefaultClientScope(scope.getId());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void specifyMultipleScopeTest() throws Exception {
|
||||
|
@ -71,7 +159,7 @@ public class OAuthScopeInTokenResponseTest extends AbstractKeycloakTest {
|
|||
String loginPassword = "password";
|
||||
String clientSecret = "password";
|
||||
|
||||
String requestedScope = "user address";
|
||||
String requestedScope = "address";
|
||||
String expectedScope = "openid profile email address";
|
||||
|
||||
oauth.scope(requestedScope);
|
||||
|
|
|
@ -163,30 +163,8 @@ public class OfflineTokenTest extends AbstractKeycloakTest {
|
|||
oauth.scope(OAuth2Constants.OFFLINE_ACCESS);
|
||||
oauth.clientId("offline-client");
|
||||
oauth.redirectUri(offlineClientAppUri);
|
||||
oauth.doLogin("test-user@localhost", "password");
|
||||
|
||||
EventRepresentation loginEvent = events.expectLogin()
|
||||
.client("offline-client")
|
||||
.detail(Details.REDIRECT_URI, offlineClientAppUri)
|
||||
.assertEvent();
|
||||
|
||||
String sessionId = loginEvent.getSessionId();
|
||||
String codeId = loginEvent.getDetails().get(Details.CODE_ID);
|
||||
|
||||
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
|
||||
|
||||
OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code, "secret1");
|
||||
String offlineTokenString = tokenResponse.getRefreshToken();
|
||||
RefreshToken refreshToken = oauth.parseRefreshToken(offlineTokenString);
|
||||
|
||||
// Token is refreshed, but it's not offline token
|
||||
events.expectCodeToToken(codeId, sessionId)
|
||||
.client("offline-client")
|
||||
.detail(Details.REFRESH_TOKEN_TYPE, TokenUtil.TOKEN_TYPE_REFRESH)
|
||||
.assertEvent();
|
||||
|
||||
assertEquals(TokenUtil.TOKEN_TYPE_REFRESH, refreshToken.getType());
|
||||
assertFalse(tokenResponse.getScope().contains(OAuth2Constants.OFFLINE_ACCESS));
|
||||
oauth.openLoginForm();
|
||||
assertTrue(driver.getCurrentUrl().contains("error_description=Invalid+scopes"));
|
||||
|
||||
// Revert changes
|
||||
ClientManager.realm(adminClient.realm("test")).clientId("offline-client")
|
||||
|
|
|
@ -306,7 +306,7 @@ public class OIDCScopeTest extends AbstractOIDCScopeTest {
|
|||
.removeDetail(Details.REDIRECT_URI).assertEvent();
|
||||
|
||||
// Login with scope parameter. Just 'profile' is there
|
||||
oauth.scope("openid profile email");
|
||||
oauth.scope("openid profile");
|
||||
oauth.doLogin("john", "password");
|
||||
loginEvent = events.expectLogin()
|
||||
.user(userId)
|
||||
|
|
Loading…
Reference in a new issue