KEYCLOAK-7470 Ability to order client scopes

This commit is contained in:
mposolda 2018-08-28 14:29:40 +02:00 committed by Hynek Mlnařík
parent 21b71e83dd
commit b70468341e
11 changed files with 214 additions and 112 deletions

View file

@ -23,7 +23,7 @@ import java.util.Map;
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public interface ClientScopeModel extends ProtocolMapperContainerModel, ScopeContainerModel {
public interface ClientScopeModel extends ProtocolMapperContainerModel, ScopeContainerModel, OrderedModel {
String getId();
String getName();
@ -48,6 +48,7 @@ public interface ClientScopeModel extends ProtocolMapperContainerModel, ScopeCon
String DISPLAY_ON_CONSENT_SCREEN = "display.on.consent.screen";
String CONSENT_SCREEN_TEXT = "consent.screen.text";
String GUI_ORDER = "gui.order";
default boolean isDisplayOnConsentScreen() {
String displayVal = getAttribute(DISPLAY_ON_CONSENT_SCREEN);
@ -71,5 +72,14 @@ public interface ClientScopeModel extends ProtocolMapperContainerModel, ScopeCon
setAttribute(CONSENT_SCREEN_TEXT, consentScreenText);
}
@Override
default String getGuiOrder() {
return getAttribute(GUI_ORDER);
}
default void setGuiOrder(String guiOrder) {
setAttribute(GUI_ORDER, guiOrder);
}
}

View file

@ -0,0 +1,51 @@
/*
* Copyright 2017 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.models;
import java.util.Comparator;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public interface OrderedModel {
String getGuiOrder();
class OrderedModelComparator<OM extends OrderedModel> implements Comparator<OM> {
@Override
public int compare(OM o1, OM o2) {
int o1order = parseOrder(o1);
int o2order = parseOrder(o2);
return o1order - o2order;
}
private int parseOrder(OM model) {
if (model != null && model.getGuiOrder() != null) {
try {
return Integer.parseInt(model.getGuiOrder());
} catch (NumberFormatException e) {
// ignore it and use default
}
}
return 10000;
}
}
}

View file

@ -20,19 +20,16 @@ package org.keycloak.forms.account.freemarker.model;
import org.keycloak.models.FederatedIdentityModel;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.OrderedModel;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.services.resources.account.AccountFormService;
import org.keycloak.services.Urls;
import javax.ws.rs.core.UriBuilder;
import java.net.URI;
import java.util.Comparator;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@ -40,7 +37,9 @@ import java.util.TreeSet;
*/
public class AccountFederatedIdentityBean {
private final List<FederatedIdentityEntry> identities;
private static OrderedModel.OrderedModelComparator<FederatedIdentityEntry> IDP_COMPARATOR_INSTANCE = new OrderedModel.OrderedModelComparator<>();
private final List<FederatedIdentityEntry> identities = new ArrayList<>();
private final boolean removeLinkPossible;
private final KeycloakSession session;
@ -50,7 +49,6 @@ public class AccountFederatedIdentityBean {
List<IdentityProviderModel> identityProviders = realm.getIdentityProviders();
Set<FederatedIdentityModel> identities = session.users().getFederatedIdentities(user, realm);
Set<FederatedIdentityEntry> orderedSet = new TreeSet<>(IdentityProviderComparator.INSTANCE);
int availableIdentities = 0;
if (identityProviders != null && !identityProviders.isEmpty()) {
for (IdentityProviderModel provider : identityProviders) {
@ -68,11 +66,11 @@ public class AccountFederatedIdentityBean {
String displayName = KeycloakModelUtils.getIdentityProviderDisplayName(session, provider);
FederatedIdentityEntry entry = new FederatedIdentityEntry(identity, displayName, provider.getAlias(), provider.getAlias(),
provider.getConfig() != null ? provider.getConfig().get("guiOrder") : null);
orderedSet.add(entry);
this.identities.add(entry);
}
}
this.identities = new LinkedList<FederatedIdentityEntry>(orderedSet);
this.identities.sort(IDP_COMPARATOR_INSTANCE);
// Removing last social provider is not possible if you don't have other possibility to authenticate
this.removeLinkPossible = availableIdentities > 1 || user.getFederationLink() != null || AccountFormService.isPasswordSet(session, realm, user);
@ -95,7 +93,7 @@ public class AccountFederatedIdentityBean {
return removeLinkPossible;
}
public class FederatedIdentityEntry {
public class FederatedIdentityEntry implements OrderedModel {
private FederatedIdentityModel federatedIdentityModel;
private final String providerId;
@ -132,6 +130,7 @@ public class AccountFederatedIdentityBean {
return federatedIdentityModel != null;
}
@Override
public String getGuiOrder() {
return guiOrder;
}
@ -141,38 +140,5 @@ public class AccountFederatedIdentityBean {
}
}
public static class IdentityProviderComparator implements Comparator<FederatedIdentityEntry> {
public static IdentityProviderComparator INSTANCE = new IdentityProviderComparator();
private IdentityProviderComparator() {
}
@Override
public int compare(FederatedIdentityEntry o1, FederatedIdentityEntry o2) {
int o1order = parseOrder(o1);
int o2order = parseOrder(o2);
if (o1order > o2order)
return 1;
else if (o1order < o2order)
return -1;
return 1;
}
private int parseOrder(FederatedIdentityEntry ip) {
if (ip != null && ip.getGuiOrder() != null) {
try {
return Integer.parseInt(ip.getGuiOrder());
} catch (NumberFormatException e) {
// ignore it and use defaulr
}
}
return 10000;
}
}
}

View file

@ -18,10 +18,12 @@
package org.keycloak.forms.account.freemarker.model;
import org.keycloak.common.util.MultivaluedHashMap;
import org.keycloak.forms.login.freemarker.model.OAuthGrantBean;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.Constants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.OrderedModel;
import org.keycloak.models.ProtocolMapperModel;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
@ -36,6 +38,8 @@ import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Collectors;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@ -72,18 +76,18 @@ public class ApplicationsBean {
MultivaluedHashMap<String, ClientRoleEntry> resourceRolesAvailable = new MultivaluedHashMap<String, ClientRoleEntry>();
processRoles(availableRoles, realmRolesAvailable, resourceRolesAvailable);
List<String> clientScopesGranted = new LinkedList<String>();
List<ClientScopeModel> orderedScopes = new ArrayList<>();
if (client.isConsentRequired()) {
UserConsentModel consent = session.users().getConsentByClient(realm, user.getId(), client.getId());
if (consent != null) {
for (ClientScopeModel clientScope : consent.getGrantedClientScopes()) {
String consentText = clientScope.getConsentScreenText();
clientScopesGranted.add(consentText);
}
orderedScopes.addAll(consent.getGrantedClientScopes());
orderedScopes.sort(new OrderedModel.OrderedModelComparator<>());
}
}
List<String> clientScopesGranted = orderedScopes.stream()
.map(ClientScopeModel::getConsentScreenText)
.collect(Collectors.toList());
List<String> additionalGrants = new ArrayList<>();
if (offlineClients.contains(client)) {

View file

@ -18,18 +18,15 @@ package org.keycloak.forms.login.freemarker.model;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.OrderedModel;
import org.keycloak.models.RealmModel;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.services.Urls;
import javax.ws.rs.core.UriInfo;
import java.net.URI;
import java.util.Comparator;
import java.util.LinkedList;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
/**
* @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@ -37,6 +34,8 @@ import java.util.TreeSet;
*/
public class IdentityProviderBean {
public static OrderedModel.OrderedModelComparator<IdentityProvider> IDP_COMPARATOR_INSTANCE = new OrderedModel.OrderedModelComparator<>();
private boolean displaySocial;
private List<IdentityProvider> providers;
private RealmModel realm;
@ -47,21 +46,22 @@ public class IdentityProviderBean {
this.session = session;
if (!identityProviders.isEmpty()) {
Set<IdentityProvider> orderedSet = new TreeSet<>(IdentityProviderComparator.INSTANCE);
List<IdentityProvider> orderedList = new ArrayList<>();
for (IdentityProviderModel identityProvider : identityProviders) {
if (identityProvider.isEnabled() && !identityProvider.isLinkOnly()) {
addIdentityProvider(orderedSet, realm, baseURI, identityProvider);
addIdentityProvider(orderedList, realm, baseURI, identityProvider);
}
}
if (!orderedSet.isEmpty()) {
providers = new LinkedList<>(orderedSet);
if (!orderedList.isEmpty()) {
orderedList.sort(IDP_COMPARATOR_INSTANCE);
providers = orderedList;
displaySocial = true;
}
}
}
private void addIdentityProvider(Set<IdentityProvider> orderedSet, RealmModel realm, URI baseURI, IdentityProviderModel identityProvider) {
private void addIdentityProvider(List<IdentityProvider> orderedSet, RealmModel realm, URI baseURI, IdentityProviderModel identityProvider) {
String loginUrl = Urls.identityProviderAuthnRequest(baseURI, identityProvider.getAlias(), realm.getName()).toString();
String displayName = KeycloakModelUtils.getIdentityProviderDisplayName(session, identityProvider);
Map<String, String> config = identityProvider.getConfig();
@ -81,7 +81,7 @@ public class IdentityProviderBean {
return realm.isRegistrationAllowed() || displaySocial;
}
public static class IdentityProvider {
public static class IdentityProvider implements OrderedModel {
private final String alias;
private final String providerId; // This refer to providerType (facebook, google, etc.)
@ -109,6 +109,7 @@ public class IdentityProviderBean {
return providerId;
}
@Override
public String getGuiOrder() {
return guiOrder;
}
@ -118,37 +119,4 @@ public class IdentityProviderBean {
}
}
public static class IdentityProviderComparator implements Comparator<IdentityProvider> {
public static IdentityProviderComparator INSTANCE = new IdentityProviderComparator();
private IdentityProviderComparator() {
}
@Override
public int compare(IdentityProvider o1, IdentityProvider o2) {
int o1order = parseOrder(o1);
int o2order = parseOrder(o2);
if (o1order > o2order)
return 1;
else if (o1order < o2order)
return -1;
return 1;
}
private int parseOrder(IdentityProvider ip) {
if (ip != null && ip.getGuiOrder() != null) {
try {
return Integer.parseInt(ip.getGuiOrder());
} catch (NumberFormatException e) {
// ignore it and use defaulr
}
}
return 10000;
}
}
}

View file

@ -18,16 +18,22 @@ package org.keycloak.forms.login.freemarker.model;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.OrderedModel;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
/**
* @author <a href="mailto:vrockai@redhat.com">Viliam Rockai</a>
*/
public class OAuthGrantBean {
private List<ClientScopeEntry> clientScopesRequested = new LinkedList<>();
private static OrderedModel.OrderedModelComparator<ClientScopeEntry> COMPARATOR_INSTANCE = new OrderedModel.OrderedModelComparator<>();
private List<ClientScopeEntry> clientScopesRequested = new ArrayList<>();
private String code;
private ClientModel client;
@ -36,8 +42,9 @@ public class OAuthGrantBean {
this.client = client;
for (ClientScopeModel clientScope : clientScopesRequested) {
this.clientScopesRequested.add(new ClientScopeEntry(clientScope.getConsentScreenText()));
this.clientScopesRequested.add(new ClientScopeEntry(clientScope.getConsentScreenText(), clientScope.getGuiOrder()));
}
this.clientScopesRequested.sort(COMPARATOR_INSTANCE);
}
public String getCode() {
@ -56,16 +63,23 @@ public class OAuthGrantBean {
// Converting ClientScopeModel due the freemarker limitations. It's not able to read "getConsentScreenText" default method defined on interface
public static class ClientScopeEntry {
public static class ClientScopeEntry implements OrderedModel {
private final String consentScreenText;
private final String guiOrder;
private ClientScopeEntry(String consentScreenText) {
private ClientScopeEntry(String consentScreenText, String guiOrder) {
this.consentScreenText = consentScreenText;
this.guiOrder = guiOrder;
}
public String getConsentScreenText() {
return consentScreenText;
}
@Override
public String getGuiOrder() {
return guiOrder;
}
}
}

View file

@ -16,10 +16,14 @@
*/
package org.keycloak.test.login.freemarker.model;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import org.junit.Assert;
import org.junit.Test;
import org.keycloak.forms.login.freemarker.model.IdentityProviderBean;
import org.keycloak.forms.login.freemarker.model.IdentityProviderBean.IdentityProvider;
import org.keycloak.forms.login.freemarker.model.IdentityProviderBean.IdentityProviderComparator;
/**
* Unit test for {@link org.keycloak.forms.login.freemarker.model.IdentityProviderBean}
@ -35,32 +39,49 @@ public class IdentityProviderBeanTest {
IdentityProvider o1 = new IdentityProvider("alias1", "displayName1", "id1", "ur1", null);
IdentityProvider o2 = new IdentityProvider("alias2", "displayName2", "id2", "ur2", null);
// guiOrder not defined at any object - first is always lower
Assert.assertEquals(1, IdentityProviderComparator.INSTANCE.compare(o1, o2));
Assert.assertEquals(1, IdentityProviderComparator.INSTANCE.compare(o2, o1));
// guiOrder not defined at any object
Assert.assertEquals(0, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o1, o2));
Assert.assertEquals(0, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o2, o1));
// guiOrder is not a number so it is same as not defined - first is always lower
// guiOrder is not a number so it is same as not defined
o1 = new IdentityProvider("alias1", "displayName1", "id1", "ur1", "not a number");
Assert.assertEquals(1, IdentityProviderComparator.INSTANCE.compare(o1, o2));
Assert.assertEquals(1, IdentityProviderComparator.INSTANCE.compare(o2, o1));
Assert.assertEquals(0, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o1, o2));
Assert.assertEquals(0, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o2, o1));
// guiOrder is defined for one only to it is always first
o1 = new IdentityProvider("alias1", "displayName1", "id1", "ur1", "0");
Assert.assertEquals(-1, IdentityProviderComparator.INSTANCE.compare(o1, o2));
Assert.assertEquals(1, IdentityProviderComparator.INSTANCE.compare(o2, o1));
Assert.assertEquals(-10000, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o1, o2));
Assert.assertEquals(10000, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o2, o1));
// guiOrder is defined for both but is same - first is always lower
// guiOrder is defined for both but is same
o1 = new IdentityProvider("alias1", "displayName1", "id1", "ur1", "0");
o2 = new IdentityProvider("alias2", "displayName2", "id2", "ur2", "0");
Assert.assertEquals(1, IdentityProviderComparator.INSTANCE.compare(o1, o2));
Assert.assertEquals(1, IdentityProviderComparator.INSTANCE.compare(o2, o1));
Assert.assertEquals(0, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o1, o2));
Assert.assertEquals(0, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o2, o1));
// guiOrder is reflected
o1 = new IdentityProvider("alias1", "displayName1", "id1", "ur1", "0");
o2 = new IdentityProvider("alias2", "displayName2", "id2", "ur2", "1");
Assert.assertEquals(-1, IdentityProviderComparator.INSTANCE.compare(o1, o2));
Assert.assertEquals(1, IdentityProviderComparator.INSTANCE.compare(o2, o1));
Assert.assertEquals(-1, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o1, o2));
Assert.assertEquals(1, IdentityProviderBean.IDP_COMPARATOR_INSTANCE.compare(o2, o1));
}
@Test
public void testIdentityProviderComparatorForEqualObjects() {
IdentityProvider o1 = new IdentityProvider("alias1", "displayName1", "id1", "ur1", null);
IdentityProvider o2 = new IdentityProvider("alias2", "displayName2", "id2", "ur2", null);
// Gui order is not specified on the objects, but those are 2 different objects. Assert we have 2 items in the list and first is lower
List<IdentityProvider> idp2 = new ArrayList<>();
idp2.add(o1);
idp2.add(o2);
idp2.sort(IdentityProviderBean.IDP_COMPARATOR_INSTANCE);
Assert.assertEquals(2, idp2.size());
Iterator<IdentityProvider> itr2 = idp2.iterator();
Assert.assertEquals("alias1", itr2.next().getAlias());
Assert.assertEquals("alias2", itr2.next().getAlias());
}
}

View file

@ -23,11 +23,13 @@ import org.junit.Rule;
import org.junit.Test;
import org.keycloak.OAuth2Constants;
import org.keycloak.admin.client.resource.ClientResource;
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.constants.KerberosConstants;
import org.keycloak.events.Details;
import org.keycloak.events.EventType;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.representations.AccessToken;
import org.keycloak.representations.idm.ClientRepresentation;
@ -402,4 +404,59 @@ public class OAuthGrantTest extends AbstractKeycloakTest {
Assert.assertEquals(backToAppLink, thirdParty.getBaseUrl());
}
// KEYCLOAK-7470
@Test
public void oauthGrantOrderedClientScopes() throws Exception {
// Add GUI Order to client scopes --- email=1, profile=2
RealmResource appRealm = adminClient.realm(REALM_NAME);
ClientScopeResource emailScope = ApiUtil.findClientScopeByName(appRealm, "email");
ClientScopeRepresentation emailRep = emailScope.toRepresentation();
emailRep.getAttributes().put(ClientScopeModel.GUI_ORDER, "1");
emailScope.update(emailRep);
ClientScopeResource profileScope = ApiUtil.findClientScopeByName(appRealm, "profile");
ClientScopeRepresentation profileRep = profileScope.toRepresentation();
profileRep.getAttributes().put(ClientScopeModel.GUI_ORDER, "2");
profileScope.update(profileRep);
// Display consent screen --- assert email, then profile
oauth.clientId(THIRD_PARTY_APP);
oauth.doLoginGrant("test-user@localhost", "password");
grantPage.assertCurrent();
List<String> displayedScopes = grantPage.getDisplayedGrants();
Assert.assertEquals("Email address", displayedScopes.get(0));
Assert.assertEquals("User profile", displayedScopes.get(1));
grantPage.accept();
// Display account mgmt --- assert email, then profile
accountAppsPage.open();
displayedScopes = accountAppsPage.getApplications().get(THIRD_PARTY_APP).getClientScopesGranted();
Assert.assertEquals("Email address", displayedScopes.get(0));
Assert.assertEquals("User profile", displayedScopes.get(1));
// Update GUI Order --- email=3
emailRep = emailScope.toRepresentation();
emailRep.getAttributes().put(ClientScopeModel.GUI_ORDER, "3");
emailScope.update(emailRep);
// Display account mgmt --- assert profile, then email
accountAppsPage.open();
displayedScopes = accountAppsPage.getApplications().get(THIRD_PARTY_APP).getClientScopesGranted();
Assert.assertEquals("User profile", displayedScopes.get(0));
Assert.assertEquals("Email address", displayedScopes.get(1));
// Revoke grant and display consent screen --- assert profile, then email
accountAppsPage.revokeGrant(THIRD_PARTY_APP);
oauth.openLoginForm();
grantPage.assertCurrent();
displayedScopes = grantPage.getDisplayedGrants();
Assert.assertEquals("User profile", displayedScopes.get(0));
Assert.assertEquals("Email address", displayedScopes.get(1));
}
}

View file

@ -829,6 +829,8 @@ client-scope.display-on-consent-screen=Display On Consent Screen
client-scope.display-on-consent-screen.tooltip=If on, and this client scope is added to some client with consent required, then the text specified by 'Consent Screen Text' will be displayed on consent screen. If off, then this client scope won't be displayed on consent screen
client-scope.consent-screen-text=Consent Screen Text
client-scope.consent-screen-text.tooltip=Text, which will be shown on consent screen when this client scope is added to some client with consent required. Defaults to name of client scope if it's not filled
client-scope.gui-order=GUI order
client-scope.gui-order.tooltip=Specify order of the provider in GUI (e.g. in Consent page) as integer
add-user-federation-provider=Add user federation provider
add-user-storage-provider=Add user storage provider

View file

@ -52,6 +52,13 @@
</div>
<kc-tooltip>{{:: 'client-scope.consent-screen-text.tooltip' | translate}}</kc-tooltip>
</div>
<div class="form-group">
<label class="col-md-2 control-label" for="guiOrder">{{:: 'client-scope.gui-order' | translate}} </label>
<div class="col-sm-6">
<input class="form-control" type="text" id="guiOrder" name="guiOrder" data-ng-model="clientScope.attributes['gui.order']">
</div>
<kc-tooltip>{{:: 'client-scope.gui-order.tooltip' | translate}}</kc-tooltip>
</div>
</fieldset>
<div class="form-group">

View file

@ -37,6 +37,7 @@
<tr data-ng-hide="clients.length == 0">
<th>{{:: 'name' | translate}}</th>
<th>{{:: 'protocol' | translate}}</th>
<th width="15%">{{:: 'gui-order' | translate}}</th>
<th colspan="2" class="w-25">{{:: 'actions' | translate}}</th>
</tr>
</thead>
@ -44,6 +45,7 @@
<tr ng-repeat="clientScope in clientScopes | filter:search | orderBy:'name'">
<td><a href="#/realms/{{realm.realm}}/client-scopes/{{clientScope.id}}">{{clientScope.name}}</a></td>
<td>{{clientScope.protocol}}</td>
<td>{{clientScope.attributes['gui.order']}}</td>
<td class="kc-action-cell" kc-open="/realms/{{realm.realm}}/client-scopes/{{clientScope.id}}">{{:: 'edit' | translate}}</td>
<td class="kc-action-cell" data-ng-click="removeClientScope(clientScope)">{{:: 'delete' | translate}}</td>
</tr>