[fixes #9224] - Get consented scopes from AuthorizationContext

Always show the consent screen when a dynamic scope is requested and show the requested parameter

Improve the code that handles dynamic scopes consent and add some log traces

Add a test to check how we show dynamic scope in the consent screen and added missing template file change

Fix merge problem in comment and improve other comments

Fix the Dynamic Scope test by assigning it to the client as optional instead of default

Change how dynamic scopes are represented in the consent screen and adapt test
This commit is contained in:
Daniel Gozalo 2022-01-19 14:43:25 +01:00 committed by Marek Posolda
parent 0301630480
commit 3528e7ba54
11 changed files with 192 additions and 37 deletions

View file

@ -137,6 +137,13 @@ public class AuthorizationDetailsJSONRepresentation implements Serializable {
return null;
}
public String getDynamicScopeParamFromCustomData() {
if(this.getType().equalsIgnoreCase(DYNAMIC_SCOPE_RAR_TYPE)) {
return (String) this.customData.get("scope_parameter");
}
return null;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;

View file

@ -18,10 +18,10 @@
package org.keycloak.forms.login;
import org.keycloak.authentication.AuthenticationFlowContext;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.FormMessage;
import org.keycloak.provider.Provider;
import org.keycloak.rar.AuthorizationDetails;
import org.keycloak.sessions.AuthenticationSessionModel;
import javax.ws.rs.core.MultivaluedMap;
@ -104,7 +104,7 @@ public interface LoginFormsProvider extends Provider {
LoginFormsProvider setClientSessionCode(String accessCode);
LoginFormsProvider setAccessRequest(List<ClientScopeModel> clientScopesRequested);
LoginFormsProvider setAccessRequest(List<AuthorizationDetails> clientScopesRequested);
/**
* Set one global error message.

View file

@ -46,6 +46,11 @@ public class AuthorizationDetails implements Serializable {
this.authorizationDetails = authorizationDetails;
}
public AuthorizationDetails(ClientScopeModel clientScope) {
this.clientScope = clientScope;
this.source = AuthorizationRequestSource.SCOPE;
}
public ClientScopeModel getClientScope() {
return clientScope;
}
@ -70,6 +75,25 @@ public class AuthorizationDetails implements Serializable {
this.authorizationDetails = authorizationDetails;
}
/**
* Returns whether the current {@link AuthorizationDetails} object is a dynamic scope
* @return see description
*/
public boolean isDynamicScope() {
return this.source.equals(AuthorizationRequestSource.SCOPE) && this.getClientScope().isDynamicScope();
}
/**
* Returns the Dynamic Scope parameter from the underlying {@link AuthorizationDetailsJSONRepresentation} representation
* @return see description
*/
public String getDynamicScopeParam() {
if(isDynamicScope()) {
return this.authorizationDetails.getDynamicScopeParamFromCustomData();
}
return null;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;

View file

@ -46,7 +46,6 @@ import org.keycloak.forms.login.freemarker.model.UrlBean;
import org.keycloak.forms.login.freemarker.model.VerifyProfileBean;
import org.keycloak.forms.login.freemarker.model.X509ConfirmBean;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.Constants;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
@ -54,6 +53,7 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.FormMessage;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.rar.AuthorizationDetails;
import org.keycloak.services.Urls;
import org.keycloak.services.messages.Messages;
import org.keycloak.services.resources.LoginActionsService;
@ -100,7 +100,7 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider {
protected String accessCode;
protected Response.Status status;
protected javax.ws.rs.core.MediaType contentType;
protected List<ClientScopeModel> clientScopesRequested;
protected List<AuthorizationDetails> clientScopesRequested;
protected Map<String, String> httpResponseHeaders = new HashMap<>();
protected URI actionUri;
protected String execution;
@ -767,7 +767,7 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider {
}
@Override
public LoginFormsProvider setAccessRequest(List<ClientScopeModel> clientScopesRequested) {
public LoginFormsProvider setAccessRequest(List<AuthorizationDetails> clientScopesRequested) {
this.clientScopesRequested = clientScopesRequested;
return this;
}

View file

@ -19,6 +19,7 @@ package org.keycloak.forms.login.freemarker.model;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.OrderedModel;
import org.keycloak.rar.AuthorizationDetails;
import java.util.ArrayList;
import java.util.List;
@ -34,12 +35,13 @@ public class OAuthGrantBean {
private String code;
private ClientModel client;
public OAuthGrantBean(String code, ClientModel client, List<ClientScopeModel> clientScopesRequested) {
public OAuthGrantBean(String code, ClientModel client, List<AuthorizationDetails> clientScopesRequested) {
this.code = code;
this.client = client;
for (ClientScopeModel clientScope : clientScopesRequested) {
this.clientScopesRequested.add(new ClientScopeEntry(clientScope.getConsentScreenText(), clientScope.getGuiOrder()));
for (AuthorizationDetails authDetails : clientScopesRequested) {
ClientScopeModel clientScope = authDetails.getClientScope();
this.clientScopesRequested.add(new ClientScopeEntry(clientScope.getConsentScreenText(), clientScope.getGuiOrder(), authDetails));
}
this.clientScopesRequested.sort(COMPARATOR_INSTANCE);
}
@ -64,10 +66,12 @@ public class OAuthGrantBean {
private final String consentScreenText;
private final String guiOrder;
private final String dynamicScopeParameter;
private ClientScopeEntry(String consentScreenText, String guiOrder) {
public ClientScopeEntry(String consentScreenText, String guiOrder, AuthorizationDetails authorizationDetails) {
this.consentScreenText = consentScreenText;
this.guiOrder = guiOrder;
this.dynamicScopeParameter = authorizationDetails.getDynamicScopeParam();
}
public String getConsentScreenText() {
@ -78,5 +82,9 @@ public class OAuthGrantBean {
public String getGuiOrder() {
return guiOrder;
}
public String getDynamicScopeParameter() {
return dynamicScopeParameter;
}
}
}

View file

@ -659,6 +659,11 @@ public class TokenManager {
requestedScopes.remove(OAuth2Constants.SCOPE_OPENID);
}
if (logger.isTraceEnabled()) {
logger.tracef("Rar scopes to validate requested scopes against: %1s", String.join(" ", rarScopes));
logger.tracef("Requested scopes: %1s", String.join(" ", requestedScopes));
}
for (String requestedScope : requestedScopes) {
// We keep the check to the getDynamicClientScope for the OpenshiftSAClientAdapter
if (!rarScopes.contains(requestedScope) && client.getDynamicClientScope(requestedScope) == null) {
@ -667,7 +672,7 @@ public class TokenManager {
}
return true;
}
public static boolean isValidScope(String scopes, ClientModel client) {
if (scopes == null) {
return true;

View file

@ -16,6 +16,7 @@
*/
package org.keycloak.protocol.oidc.rar.parsers;
import org.jboss.logging.Logger;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.protocol.oidc.TokenManager;
@ -46,6 +47,8 @@ import static org.keycloak.representations.AuthorizationDetailsJSONRepresentatio
*/
public class ClientScopeAuthorizationRequestParser implements AuthorizationRequestParserProvider {
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

View file

@ -36,6 +36,7 @@ import org.keycloak.authentication.actiontoken.DefaultActionTokenKey;
import org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator;
import org.keycloak.broker.provider.IdentityProvider;
import org.keycloak.common.ClientConnection;
import org.keycloak.common.Profile;
import org.keycloak.common.VerificationException;
import org.keycloak.common.util.Base64Url;
import org.keycloak.common.util.SecretGenerator;
@ -69,6 +70,7 @@ import org.keycloak.protocol.oidc.BackchannelLogoutResponse;
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.oidc.TokenManager;
import org.keycloak.rar.AuthorizationDetails;
import org.keycloak.representations.AccessToken;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.ServicesLogger;
@ -80,6 +82,7 @@ import org.keycloak.services.resources.IdentityBrokerService;
import org.keycloak.services.resources.LoginActionsService;
import org.keycloak.services.resources.RealmsResource;
import org.keycloak.services.util.CookieHelper;
import org.keycloak.services.util.DefaultClientSessionContext;
import org.keycloak.services.util.P3PHelper;
import org.keycloak.sessions.AuthenticationSessionModel;
import org.keycloak.sessions.CommonClientSessionModel;
@ -361,7 +364,7 @@ public class AuthenticationManager {
BackchannelLogoutResponse.DownStreamBackchannelLogoutResponse downStreamBackchannelLogoutResponse =
new BackchannelLogoutResponse.DownStreamBackchannelLogoutResponse();
downStreamBackchannelLogoutResponse.setWithBackchannelLogoutUrl(backchannelLogoutUrl != null);
if (clientSessionLogoutResponse != null) {
downStreamBackchannelLogoutResponse.setResponseCode(clientSessionLogoutResponse.getStatus());
} else {
@ -425,7 +428,7 @@ public class AuthenticationManager {
/**
* Logs out the given client session and records the result into {@code logoutAuthSession} if set.
*
*
* @param session
* @param realm
* @param clientSession
@ -687,7 +690,7 @@ public class AuthenticationManager {
return response;
}
public static void finishUnconfirmedUserSession(KeycloakSession session, RealmModel realm, UserSessionModel userSessionModel) {
if (userSessionModel.getAuthenticatedClientSessions().values().stream().anyMatch(cs -> !CommonClientSessionModel.Action.LOGGED_OUT.name().equals(cs.getAction()))) {
logger.warnf("UserSession with id %s is removed while there are still some user sessions that are not logged out properly.", userSessionModel.getId());
@ -1059,7 +1062,7 @@ public class AuthenticationManager {
UserConsentModel grantedConsent = getEffectiveGrantedConsent(session, authSession);
// See if any clientScopes need to be approved on consent screen
List<ClientScopeModel> clientScopesToApprove = getClientScopesToApproveOnConsentScreen(realm, grantedConsent, authSession);
List<AuthorizationDetails> clientScopesToApprove = getClientScopesToApproveOnConsentScreen(grantedConsent, session);
if (!clientScopesToApprove.isEmpty()) {
return CommonClientSessionModel.Action.OAUTH_GRANT.name();
}
@ -1123,7 +1126,7 @@ public class AuthenticationManager {
UserConsentModel grantedConsent = getEffectiveGrantedConsent(session, authSession);
List<ClientScopeModel> clientScopesToApprove = getClientScopesToApproveOnConsentScreen(realm, grantedConsent, authSession);
List<AuthorizationDetails> clientScopesToApprove = getClientScopesToApproveOnConsentScreen(grantedConsent, session);
// Skip grant screen if everything was already approved by this user
if (clientScopesToApprove.size() > 0) {
@ -1150,27 +1153,54 @@ public class AuthenticationManager {
}
private static List<ClientScopeModel> getClientScopesToApproveOnConsentScreen(RealmModel realm, UserConsentModel grantedConsent,
AuthenticationSessionModel authSession) {
private static List<AuthorizationDetails> getClientScopesToApproveOnConsentScreen(UserConsentModel grantedConsent, KeycloakSession session) {
// Client Scopes to be displayed on consent screen
List<ClientScopeModel> clientScopesToDisplay = new LinkedList<>();
for (String clientScopeId : authSession.getClientScopes()) {
ClientScopeModel clientScope = KeycloakModelUtils.findClientScopeById(realm, authSession.getClient(), clientScopeId);
List<AuthorizationDetails> clientScopesToDisplay = new LinkedList<>();
// AuthorizationDetails are going to be returned regardless of the Dynamic Scope feature state
for (AuthorizationDetails authDetails : getClientScopeModelStream(session).collect(Collectors.toList())) {
ClientScopeModel clientScope = authDetails.getClientScope();
if (clientScope == null || !clientScope.isDisplayOnConsentScreen()) {
continue;
}
// Check if consent already granted by user
if (grantedConsent == null || !grantedConsent.isClientScopeGranted(clientScope)) {
clientScopesToDisplay.add(clientScope);
// we need to add dynamic scopes with params to the scopes to consent every time for now
if (grantedConsent == null || !grantedConsent.isClientScopeGranted(clientScope) || isDynamicScopeWithParam(authDetails)) {
clientScopesToDisplay.add(authDetails);
}
}
return clientScopesToDisplay;
}
private static boolean isDynamicScopeWithParam(AuthorizationDetails authorizationDetails) {
boolean dynamicScopeWithParam = authorizationDetails.getClientScope().isDynamicScope()
&& authorizationDetails.getAuthorizationDetails() != null;
if (dynamicScopeWithParam) {
logger.debugf("Scope %1s is a dynamic scope with param: %2s",
authorizationDetails.getAuthorizationDetails().getScopeNameFromCustomData(),
authorizationDetails.getDynamicScopeParam());
}
return dynamicScopeWithParam;
}
private static Stream<AuthorizationDetails> getClientScopeModelStream(KeycloakSession session) {
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 Stream.concat(DefaultClientSessionContext.getAuthorizationRequestContext(session, authSession.getClientNote(OAuth2Constants.SCOPE))
.getAuthorizationDetailEntries().stream(),
Collections.singletonList(new AuthorizationDetails(session.getContext().getClient())).stream());
}
// 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.
return authSession.getClientScopes().stream()
.map(scopeId -> KeycloakModelUtils.findClientScopeById(authSession.getRealm(), authSession.getClient(), scopeId))
.map(AuthorizationDetails::new);
}
public static void setClientScopesInSession(AuthenticationSessionModel authSession) {
ClientModel client = authSession.getClient();
@ -1301,7 +1331,7 @@ public class AuthenticationManager {
.filter(RequiredActionProviderModel::isEnabled)
.sorted(RequiredActionProviderModel.RequiredActionComparator.SINGLETON);
}
public static void evaluateRequiredActionTriggers(final KeycloakSession session, final AuthenticationSessionModel authSession,
final HttpRequest request, final EventBuilder event,
final RealmModel realm, final UserModel user) {

View file

@ -159,8 +159,9 @@ public class DefaultClientSessionContext implements ClientSessionContext {
@Override
public String getScopeString() {
if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
if(Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
String scopeParam = buildScopesStringFromAuthorizationRequest();
logger.tracef("Generated scope param with Dynamic Scopes enabled: %1s", scopeParam);
String scopeSent = clientSession.getNote(OAuth2Constants.SCOPE);
if (TokenUtil.isOIDCRequest(scopeSent)) {
scopeParam = TokenUtil.attachOIDCScope(scopeParam);
@ -214,18 +215,22 @@ public class DefaultClientSessionContext implements ClientSessionContext {
@Override
public AuthorizationRequestContext getAuthorizationRequestContext() {
if (!Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
return DefaultClientSessionContext.getAuthorizationRequestContext(this.session, clientSession.getNote(OAuth2Constants.SCOPE));
}
public static AuthorizationRequestContext getAuthorizationRequestContext(KeycloakSession keycloakSession, String scopes) {
if(!Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {
throw new RuntimeException("The Dynamic Scopes feature is not enabled and the AuthorizationRequestContext hasn't been generated");
}
AuthorizationRequestParserProvider clientScopeParser = session.getProvider(AuthorizationRequestParserProvider.class,
AuthorizationRequestParserProvider clientScopeParser = keycloakSession.getProvider(AuthorizationRequestParserProvider.class,
ClientScopeAuthorizationRequestParserProviderFactory.CLIENT_SCOPE_PARSER_ID);
if (clientScopeParser == null) {
if(clientScopeParser == null) {
throw new RuntimeException(String.format("No provider found for authorization requests parser %1s",
ClientScopeAuthorizationRequestParserProviderFactory.CLIENT_SCOPE_PARSER_ID));
}
return clientScopeParser.parseScopes(clientSession.getNote(OAuth2Constants.SCOPE));
return clientScopeParser.parseScopes(scopes);
}
// Loading data

View file

@ -27,7 +27,6 @@ import org.keycloak.admin.client.resource.ClientScopeResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.common.Profile;
import org.keycloak.common.constants.KerberosConstants;
import org.keycloak.events.Details;
import org.keycloak.events.EventType;
import org.keycloak.models.ClientScopeModel;
@ -43,18 +42,16 @@ import org.keycloak.testsuite.AbstractKeycloakTest;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
import org.keycloak.testsuite.pages.AccountApplicationsPage;
import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.ErrorPage;
import org.keycloak.testsuite.pages.OAuthGrantPage;
import org.keycloak.testsuite.util.ClientManager;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.ProtocolMapperUtil;
import org.keycloak.testsuite.util.RoleBuilder;
import org.openqa.selenium.By;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -63,7 +60,6 @@ import javax.ws.rs.core.Response;
import static org.junit.Assert.assertEquals;
import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson;
import static org.keycloak.testsuite.admin.ApiUtil.findClientByClientId;
import static org.keycloak.testsuite.admin.ApiUtil.findUserByUsernameId;
/**
* @author <a href="mailto:vrockai@redhat.com">Viliam Rockai</a>
@ -321,6 +317,78 @@ public class OAuthGrantTest extends AbstractKeycloakTest {
thirdParty.removeOptionalClientScope(fooScopeId);
}
@Test
@EnableFeature(value = Profile.Feature.DYNAMIC_SCOPES, skipRestart = true)
public void oauthGrantDynamicScopeParamRequired() {
RealmResource appRealm = adminClient.realm(REALM_NAME);
ClientResource thirdParty = findClientByClientId(appRealm, THIRD_PARTY_APP);
// Create clientScope
ClientScopeRepresentation scope = new ClientScopeRepresentation();
scope.setName("foo-dynamic-scope");
scope.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL);
scope.setAttributes(new HashMap<String, String>() {{
put(ClientScopeModel.IS_DYNAMIC_SCOPE, "true");
put(ClientScopeModel.DYNAMIC_SCOPE_REGEXP, "foo-dynamic-scope:*");
}});
Response response = appRealm.clientScopes().create(scope);
String dynamicFooScopeId = ApiUtil.getCreatedId(response);
response.close();
getCleanup().addClientScopeId(dynamicFooScopeId);
// Add clientScope as optional to client
thirdParty.addOptionalClientScope(dynamicFooScopeId);
// Assert clientScope not on grant screen when not requested
oauth.clientId(THIRD_PARTY_APP);
oauth.scope("foo-dynamic-scope:withparam");
oauth.doLogin("test-user@localhost", "password");
grantPage.assertCurrent();
List<String> grants = grantPage.getDisplayedGrants();
Assert.assertTrue(grants.contains("foo-dynamic-scope: withparam"));
grantPage.accept();
EventRepresentation loginEvent = events.expectLogin()
.client(THIRD_PARTY_APP)
.detail(Details.CONSENT, Details.CONSENT_VALUE_CONSENT_GRANTED)
.assertEvent();
String code = new OAuthClient.AuthorizationEndpointResponse(oauth).getCode();
OAuthClient.AccessTokenResponse res = oauth.doAccessTokenRequest(code, "password");
events.expectCodeToToken(loginEvent.getDetails().get(Details.CODE_ID), loginEvent.getSessionId())
.client(THIRD_PARTY_APP)
.assertEvent();
oauth.openLogout();
events.expectLogout(loginEvent.getSessionId()).assertEvent();
// login again to check whether the Dynamic scope and only the dynamic scope is requested again
oauth.scope("foo-dynamic-scope:withparam");
oauth.doLogin("test-user@localhost", "password");
grantPage.assertCurrent();
grants = grantPage.getDisplayedGrants();
Assert.assertEquals(1, grants.size());
Assert.assertTrue(grants.contains("foo-dynamic-scope: withparam"));
grantPage.accept();
events.expectLogin()
.client(THIRD_PARTY_APP)
.detail(Details.CONSENT, Details.CONSENT_VALUE_CONSENT_GRANTED)
.assertEvent();
// Revoke
accountAppsPage.open();
accountAppsPage.revokeGrant(THIRD_PARTY_APP);
events.expect(EventType.REVOKE_GRANT)
.client("account").detail(Details.REVOKED_CLIENT, THIRD_PARTY_APP).assertEvent();
// cleanup
oauth.scope(null);
thirdParty.removeOptionalClientScope(dynamicFooScopeId);
}
// KEYCLOAK-4326
@Test

View file

@ -18,7 +18,12 @@
<#if oauth.clientScopesRequested??>
<#list oauth.clientScopesRequested as clientScope>
<li>
<span>${advancedMsg(clientScope.consentScreenText)}</span>
<span><#if !clientScope.dynamicScopeParameter??>
${advancedMsg(clientScope.consentScreenText)}
<#else>
${advancedMsg(clientScope.consentScreenText)}: <b>${clientScope.dynamicScopeParameter}</b>
</#if>
</span>
</li>
</#list>
</#if>