8000 RAR scope parsing should explicitly accept client reference by mabartos · Pull Request #48981 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -623,8 +623,7 @@ public static ClientSessionContext attachAuthenticationSession(KeycloakSession s
Set<ClientScopeModel> clientScopes;

if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
session.getContext().setClient(client);
clientScopes = AuthorizationContextUtil.getClientScopesStreamFromAuthorizationRequestContextWithClient(session, scopeParam)
clientScopes = AuthorizationContextUtil.getClientScopesStreamFromAuthorizationRequestContextWithClient(session, client, scopeParam)
.collect(Collectors.toSet());
} else {
clientScopes = getRequestedClientScopes(session, scopeParam, client, userSession.getUser())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public static AuthorizationEndpointRequest parseRequest(EventBuilder event, Keyc
}

if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
request.authorizationRequestContext = AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, request.getScope());
request.authorizationRequestContext = AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, client, request.getScope());
}

return request;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ protected String getRequestedScopes() {

boolean validScopes;
if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
AuthorizationRequestContext authorizationRequestContext = AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, scope);
AuthorizationRequestContext authorizationRequestContext = AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, client, scope);
validScopes = TokenManager.isValidScope(session, scope, authorizationRequestContext, client, null);
} else {
validScopes = TokenManager.isValidScope(session, scope, client, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public static AuthorizationEndpointRequest parseRequest(EventBuilder event, Keyc
}

if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
request.setAuthorizationRequestContext(AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, request.getScope()));
request.setAuthorizationRequestContext(AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, client, request.getScope()));
}

return request;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.keycloak.protocol.oidc.rar;

import org.keycloak.models.ClientModel;
import org.keycloak.provider.Provider;
import org.keycloak.rar.AuthorizationRequestContext;

Expand All @@ -24,6 +25,6 @@
*/
public interface AuthorizationRequestParserProvider extends Provider {

AuthorizationRequestContext parseScopes(String scopeParam);
AuthorizationRequestContext parseScopes(ClientModel client, String scopeParam);

}
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,6 @@ public class ClientScopeAuthorizationRequestParser implements AuthorizationReque

protected static final Logger logger = Logger.getLogger(ClientScopeAuthorizationRequestParser.class);

/**
* This parser will be created on a per-request basis. When the adapter is created, the request's client is passed
* as a parameter
*/
private final ClientModel client;

public ClientScopeAuthorizationRequestParser(ClientModel client) {
this.client = client;
}

/**
* Creates a {@link AuthorizationRequestContext} with a list of {@link AuthorizationDetails} that will be parsed from
* the provided OAuth scopes that have been requested in a given Auth request, together with default client scopes.
Expand All @@ -68,9 +58,9 @@ public ClientScopeAuthorizationRequestParser(ClientModel client) {
* @return see description
*/
@Override
public AuthorizationRequestContext parseScopes(String scopeParam) {
public AuthorizationRequestContext parseScopes(ClientModel client, String scopeParam) {
// Process all the default ClientScopeModels for the current client, and maps them to the DynamicScopeRepresentation to make use of a HashSet
Set<IntermediaryScopeRepresentation> clientScopeModelSet = this.client.getClientScopes(true).values().stream()
Set<IntermediaryScopeRepresentation> clientScopeModelSet = client.getClientScopes(true).values().stream()
.filter(clientScopeModel -> !clientScopeModel.isDynamicScope()) // not strictly needed as Dynamic Scopes are going to be Optional scopes for now
.map(IntermediaryScopeRepresentation::new)
.collect(Collectors.toSet());
Expand All @@ -79,7 +69,7 @@ public AuthorizationRequestContext parseScopes(String scopeParam) {
if (scopeParam != null) {
// Go through the parsed requested scopes and attempt to match them against the optional scopes list
intermediaryScopeRepresentations = TokenManager.parseScopeParameter(scopeParam).collect(Collectors.toSet()).stream()
.map((String requestScope) -> getMatchingClientScope(requestScope, this.client.getClientScopes(false).values()))
.map((String requestScope) -> getMatchingClientScope(requestScope, client.getClientScopes(false).values()))
.filter(Optional::isPresent)
.map(Optional::get)
.collect(Collectors.toSet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class ClientScopeAuthorizationRequestParserProviderFactory implements Aut

@Override
public AuthorizationRequestParserProvider create(KeycloakSession session) {
return new ClientScopeAuthorizationRequestParser(session.getContext().getClient());
return new ClientScopeAuthorizationRequestParser();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ protected String getRequestedScope(AccessToken token, List<ClientModel> targetAu

boolean validScopes;
if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
AuthorizationRequestContext authorizationRequestContext = AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, scope);
AuthorizationRequestContext authorizationRequestContext = AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, client, scope);
validScopes = TokenManager.isValidScope(session, scope, authorizationRequestContext, client, null);
} else {
validScopes = TokenManager.isValidScope(session, scope, client, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1146,8 +1146,7 @@ public static String nextRequiredAction(final KeycloakSession session, final Aut
UserConsentModel grantedConsent = getEffectiveGrantedConsent(session, authSession);

// See if any clientScopes need to be approved on consent screen
List<AuthorizationDetails> clientScopesToApprove =
getClientScopesToApproveOnConsentScreen(grantedConsent, session, authSession);
List<AuthorizationDetails> clientScopesToApprove = getClientScopesToApproveOnConsentScreen(client, grantedConsent, session, authSession);
if (!clientScopesToApprove.isEmpty()) {
return Action.OAUTH_GRANT.name();
}
Expand Down Expand Up @@ -1207,8 +1206,7 @@ private static Response actionRequired(final KeycloakSession session, final Auth

UserConsentModel grantedConsent = getEffectiveGrantedConsent(session, authSession);

List<AuthorizationDetails> clientScopesToApprove =
getClientScopesToApproveOnConsentScreen(grantedConsent, session, authSession);
List<AuthorizationDetails> clientScopesToApprove = getClientScopesToApproveOnConsentScreen(client, grantedConsent, session, authSession);

// Skip grant screen if everything was already approved by this user
if (!clientScopesToApprove.isEmpty()) {
Expand Down Expand Up @@ -1237,12 +1235,12 @@ private static Response actionRequired(final KeycloakSession session, final Auth

}

private static List<AuthorizationDetails> getClientScopesToApproveOnConsentScreen(UserConsentModel grantedConsent, KeycloakSession session, AuthenticationSessionModel authSession) {
private static List<AuthorizationDetails> getClientScopesToApproveOnConsentScreen(ClientModel client, UserConsentModel grantedConsent, KeycloakSession session, AuthenticationSessionModel authSession) {
// Client Scopes to be displayed on consent screen
List<AuthorizationDetails> clientScopesToDisplay = new LinkedList<>();

// AuthorizationDetails are going to be returned regardless of the Dynamic Scope feature state
for (AuthorizationDetails authDetails : getClientScopeModelStream(session).toList()) {
for (AuthorizationDetails authDetails : getClientScopeModelStream(session, client).toList()) {
ClientScopeModel clientScope = authDetails.getClientScope();
if (clientScope == null || !clientScope.isDisplayOnConsentScreen()) {
continue;
Expand Down Expand Up @@ -1273,12 +1271,12 @@ private static boolean isDynamicScopeWithParam(AuthorizationDetails authorizatio
}


private static Stream<AuthorizationDetails> getClientScopeModelStream(KeycloakSession session) {
private static Stream<AuthorizationDetails> getClientScopeModelStream(KeycloakSession session, ClientModel client) {
AuthenticationSessionModel authSession = session.getContext().getAuthenticationSession();
//if Dynamic Scopes are enabled, get the scopes from the AuthorizationRequestContext, passing the session and scopes as parameters
// then concat a Stream with the ClientModel, as it's discarded in the getAuthorizationRequestContext method
if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
return AuthorizationContextUtil.getAuthorizationRequestsStreamFromScopesWithClient(session, authSession.getClientNote(OAuth2Constants.SCOPE));
return AuthorizationContextUtil.getAuthorizationRequestsStreamFromScopesWithClient(session, client, authSession.getClientNote(OAuth2Constants.SCOPE));
}
// if dynamic scopes are not enabled, we retain the old behaviour, but the ClientScopes will be wrapped in
// AuthorizationRequest objects to standardize the code handling these.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.stream.Stream;

import org.keycloak.common.Profile;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.protocol.oidc.rar.AuthorizationRequestParserProvider;
Expand All @@ -39,10 +40,11 @@ public class AuthorizationContextUtil {
/**
* Base function to obtain a bare AuthorizationRequestContext with just OAuth2 Scopes
* @param session
* @param client
* @param scope
* @return an {@link AuthorizationRequestContext} with scope entries
*/
public static AuthorizationRequestContext getAuthorizationRequestContextFromScopes(KeycloakSession session, String scope) {
public static AuthorizationRequestContext getAuthorizationRequestContextFromScopes(KeycloakSession session, ClientModel client, String scope) {
if (!Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
throw new RuntimeException("The Dynamic Scopes feature is not enabled and the AuthorizationRequestContext hasn't been generated");
}
Expand All @@ -54,18 +56,19 @@ public static AuthorizationRequestContext getAuthorizationRequestContextFromScop
ClientScopeAuthorizationRequestParserProviderFactory.CLIENT_SCOPE_PARSER_ID));
}

return clientScopeParser.parseScopes(scope);
return clientScopeParser.parseScopes(client, scope);
}

/**
* An extension of {@link AuthorizationContextUtil#getAuthorizationRequestContextFromScopes} that appends the current context's client
* An extension of {@link AuthorizationContextUtil#getAuthorizationRequestContextFromScopes} that appends the client reference
* @param session
* @param client
* @param scope
* @return an {@link AuthorizationRequestContext} with scope entries and a ClientModel
*/
public static AuthorizationRequestContext getAuthorizationRequestContextFromScopesWithClient(KeycloakSession session, String scope) {
AuthorizationRequestContext authorizationRequestContext = getAuthorizationRequestContextFromScopes(session, scope);
authorizationRequestContext.getAuthorizationDetailEntries().add(new AuthorizationDetails(session.getContext().getClient()));
public static AuthorizationRequestContext getAuthorizationRequestContextFromScopesWithClient(KeycloakSession session, ClientModel client, String scope) {
AuthorizationRequestContext authorizationRequestContext = getAuthorizationRequestContextFromScopes(session, client, scope);
authorizationRequestContext.getAuthorizationDetailEntries().add(new AuthorizationDetails(client));
return authorizationRequestContext;
}

Expand All @@ -75,8 +78,8 @@ public static AuthorizationRequestContext getAuthorizationRequestContextFromScop
* @param scope
* @return a Stream of {@link AuthorizationDetails} containing a ClientModel
*/
public static Stream<AuthorizationDetails> getAuthorizationRequestsStreamFromScopesWithClient(KeycloakSession session, String scope) {
AuthorizationRequestContext authorizationRequestContext = getAuthorizationRequestContextFromScopesWithClient(session, scope);
public static Stream<AuthorizationDetails> getAuthorizationRequestsStreamFromScopesWithClient(KeycloakSession session, ClientModel client, String scope) {
AuthorizationRequestContext authorizationRequestContext = getAuthorizationRequestContextFromScopesWithClient(session, client, scope);
return authorizationRequestContext.getAuthorizationDetailEntries().stream();
}

Expand All @@ -86,8 +89,8 @@ public static Stream<AuthorizationDetails> getAuthorizationRequestsStreamFromSco
* @param scope
* @return see description
*/
public static Stream<ClientScopeModel> getClientScopesStreamFromAuthorizationRequestContextWithClient(KeycloakSession session, String scope) {
return getAuthorizationRequestContextFromScopesWithClient(session, scope).getAuthorizationDetailEntries().stream()
public static Stream<ClientScopeModel> getClientScopesStreamFromAuthorizationRequestContextWithClient(KeycloakSession session, ClientModel client, String scope) {
return getAuthorizationRequestContextFromScopesWithClient(session, client, scope).getAuthorizationDetailEntries().stream()
.filter(authorizationDetails -> authorizationDetails.getSource() == AuthorizationRequestSource.SCOPE)
.map(AuthorizationDetails::getClientScope);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ public static DefaultClientSessionContext fromClientSessionScopeParameter(Authen
public static DefaultClientSessionContext fromClientSessionAndScopeParameter(AuthenticatedClientSessionModel clientSession, String scopeParam, KeycloakSession session) {
Stream<ClientScopeModel> requestedScopes;
if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
session.getContext().setClient(clientSession.getClient());
requestedScopes = AuthorizationContextUtil.getClientScopesStreamFromAuthorizationRequestContextWithClient(session, scopeParam);
requestedScopes = AuthorizationContextUtil.getClientScopesStreamFromAuthorizationRequestContextWithClient(session, clientSession.getClient(), scopeParam);
} else {
requestedScopes = TokenManager.getRequestedClientScopes(session, scopeParam, clientSession.getClient(), clientSession.getUserSession().getUser());
}
Expand Down Expand Up @@ -223,7 +222,7 @@ public String getScopeString(boolean ignoreIncludeInTokenScope) {
* @return see description
*/
private String buildScopesStringFromAuthorizationRequest(boolean ignoreIncludeInTokenScope) {
return AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, requestedScopeString).getAuthorizationDetailEntries().stream()
return AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, clientSession.getClient(), requestedScopeString).getAuthorizationDetailEntries().stream()
.filter(authorizationDetails -> authorizationDetails.getSource().equals(AuthorizationRequestSource.SCOPE))
.filter(authorizationDetails -> authorizationDetails.getClientScope().isIncludeInTokenScope() || ignoreIncludeInTokenScope)
.filter(authorizationDetails -> isClientScopePermittedForUser(authorizationDetails.getClientScope()))
Expand All @@ -246,7 +245,7 @@ public <T> T getAttribute(String name, Class<T> clazz) {

@Override
public AuthorizationRequestContext getAuthorizationRequestContext() {
return AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, requestedScopeString);
return AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, clientSession.getClient(), requestedScopeString);
}

// Loading data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ protected AuthorizationRequestContextHolder fetchAuthorizationRequestContextHold
String clientUUID = client.getId();
AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessionByClient(clientUUID);
session.getContext().setClient(client);
List<AuthorizationRequestContextHolder.AuthorizationRequestHolder> authorizationRequestHolders = AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, clientSession.getNote(OAuth2Constants.SCOPE))
List<AuthorizationRequestContextHolder.AuthorizationRequestHolder> authorizationRequestHolders = AuthorizationContextUtil.getAuthorizationRequestContextFromScopes(session, client, clientSession.getNote(OAuth2Constants.SCOPE))
.getAuthorizationDetailEntries().stream()
.map(AuthorizationRequestContextHolder.AuthorizationRequestHolder::new)
.collect(Collectors.toList());
Expand Down
Loading
0