Skip to content

Commit f12fa0b

Browse files
vramikpedroigor
vramik
authored and
pedroigor
committed
[FGAP] remove transitiveness from auth scopes
Closes #38557 Signed-off-by: vramik <[email protected]>
1 parent f3b7628 commit f12fa0b

File tree

10 files changed

+64
-91
lines changed

10 files changed

+64
-91
lines changed

‎services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionEvaluator.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public interface ClientPermissionEvaluator {
159159
/**
160160
* Returns {@code true} if the caller has at least one of the {@link org.keycloak.models.AdminRoles#VIEW_CLIENTS} or {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} roles.
161161
* <p/>
162-
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#VIEW} or {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE}.
162+
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#VIEW}.
163163
*/
164164
boolean canView(ClientScopeModel clientScope);
165165

@@ -196,6 +196,5 @@ public interface ClientPermissionEvaluator {
196196
*
197197
* @return Stream of IDs of clients with view permission.
198198
*/
199-
200199
Set<String> getClientIdsWithViewPermission(String scope);
201200
}

‎services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionsV2.java‎

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import static org.keycloak.authorization.AdminPermissionsSchema.CLIENTS_RESOURCE_TYPE;
2020

21-
import org.jboss.logging.Logger;
2221
import org.keycloak.authorization.AdminPermissionsSchema;
2322
import org.keycloak.authorization.AuthorizationProvider;
2423
import org.keycloak.authorization.model.Policy;
@@ -34,7 +33,6 @@
3433
import org.keycloak.representations.idm.authorization.Permission;
3534

3635
import java.util.ArrayList;
37-
import java.util.Arrays;
3836
import java.util.Collection;
3937
import java.util.Collections;
4038
import java.util.HashSet;
@@ -44,10 +42,9 @@
4442
import java.util.function.Function;
4543
import java.util.stream.Stream;
4644

45+
class ClientPermissionsV2 extends ClientPermissions {
4746

48-
public class ClientPermissionsV2 extends ClientPermissions {
49-
50-
public ClientPermissionsV2(KeycloakSession session, RealmModel realm, AuthorizationProvider authz, MgmtPermissionsV2 root) {
47+
ClientPermissionsV2(KeycloakSession session, RealmModel realm, AuthorizationProvider authz, MgmtPermissionsV2 root) {
5148
super(session, realm, authz, root);
5249
}
5350

@@ -67,21 +64,23 @@ public boolean canManage(ClientModel client) {
6764

6865
@Override
6966
public boolean canManage() {
70-
return super.canManage() || hasPermission(AdminPermissionsSchema.MANAGE);
67+
if (root.hasOneAdminRole(AdminRoles.MANAGE_CLIENTS)) return true;
68+
69+
return hasPermission(AdminPermissionsSchema.MANAGE);
7170
}
7271

7372
@Override
7473
public boolean canView(ClientModel client) {
75-
if (canView() || canConfigure(client)) {
76-
return true;
77-
}
74+
if (root.hasOneAdminRole(AdminRoles.MANAGE_CLIENTS, AdminRoles.VIEW_CLIENTS)) return true;
7875

7976
return hasPermission(client, AdminPermissionsSchema.VIEW);
8077
}
8178

8279
@Override
8380
public boolean canView() {
84-
return canViewClientDefault() || hasPermission(AdminPermissionsSchema.VIEW);
81+
if (root.hasOneAdminRole(AdminRoles.MANAGE_CLIENTS, AdminRoles.VIEW_CLIENTS)) return true;
82+
83+
return hasPermission(AdminPermissionsSchema.VIEW);
8584
}
8685

8786
@Override
@@ -115,7 +114,7 @@ public boolean canManage(ClientScopeModel clientScope) {
115114
public boolean canView(ClientScopeModel clientScope) {
116115
if (root.hasOneAdminRole(AdminRoles.VIEW_CLIENTS, AdminRoles.MANAGE_CLIENTS)) return true;
117116

118-
return hasPermission(AdminPermissionsSchema.VIEW) || hasPermission(AdminPermissionsSchema.MANAGE);
117+
return hasPermission(AdminPermissionsSchema.VIEW);
119118
}
120119

121120
@Override
@@ -226,14 +225,11 @@ private boolean hasPermission(ClientModel client, String scope) {
226225
}
227226

228227
Collection<Permission> permissions = root.evaluatePermission(new ResourcePermission(resourceType, resource, resource.getScopes(), server), server);
229-
List<String> expectedScopes = Arrays.asList(scope);
230228

231229
for (Permission permission : permissions) {
232230
if (permission.getResourceId().equals(resource.getId())) {
233-
for (String s : permission.getScopes()) {
234-
if (expectedScopes.contains(s)) {
235-
return true;
236-
}
231+
if (permission.getScopes().contains(scope)) {
232+
return true;
237233
}
238234
}
239235
}
@@ -268,12 +264,10 @@ private boolean hasPermission(String scope) {
268264
}
269265

270266
Collection<Permission> permissions = root.evaluatePermission(expected, server);
271-
List<String> expectedScopes = Arrays.asList(scope);
267+
272268
for (Permission permission : permissions) {
273-
for (String s : permission.getScopes()) {
274-
if (expectedScopes.contains(s)) {
275-
return true;
276-
}
269+
if (permission.getScopes().contains(scope)) {
270+
return true;
277271
}
278272
}
279273

‎services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,7 @@ public interface GroupPermissionEvaluator {
5959
* Returns {@code true} if the caller has one of {@link AdminRoles#MANAGE_USERS} or
6060
* {@link AdminRoles#VIEW_USERS} roles.
6161
* <p/>
62-
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} or
63-
* {@link AdminPermissionsSchema#MANAGE} the group.
62+
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} the group.
6463
*/
6564
boolean canView(GroupModel group);
6665

@@ -72,8 +71,7 @@ public interface GroupPermissionEvaluator {
7271
/**
7372
* Returns {@code true} if the caller has {@link AdminRoles#MANAGE_USERS} role.
7473
* <p/>
75-
* For V2 only: Also if it has permission to {@link AdminPermissionsSchema#VIEW} or
76-
* {@link AdminPermissionsSchema#MANAGE} groups.
74+
* For V2 only: Also if it has permission to {@link AdminPermissionsSchema#MANAGE} groups.
7775
*/
7876
boolean canManage();
7977

@@ -86,8 +84,7 @@ public interface GroupPermissionEvaluator {
8684
* Returns {@code true} if the caller has one of {@link AdminRoles#MANAGE_USERS} or
8785
* {@link AdminRoles#VIEW_USERS} roles.
8886
* <p/>
89-
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} or
90-
* {@link AdminPermissionsSchema#MANAGE} groups.
87+
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} groups.
9188
*/
9289
boolean canView();
9390

@@ -102,25 +99,24 @@ public interface GroupPermissionEvaluator {
10299
void requireViewMembers(GroupModel group);
103100

104101
/**
105-
* Returns {@code true} if {@link UserPermissionEvaluator#canManage()} evaluates to {@code true}.
102+
* Returns {@code true} if the caller has {@link AdminRoles#MANAGE_USERS} role.
106103
* <p/>
107104
* Or if it has a permission to {@link AdminPermissionsSchema#MANAGE_MEMBERS} of the group.
108105
*/
109106
boolean canManageMembers(GroupModel group);
110107

111108
/**
112-
* Returns {@code true} if the caller has one of {@link AdminRoles#MANAGE_USERS} role.
109+
* Returns {@code true} if the caller has {@link AdminRoles#MANAGE_USERS} role.
113110
* <p/>
114-
* Or if it has a permission to {@link AdminPermissionsSchema#MANAGE} the group or
115-
* {@link AdminPermissionsSchema#MANAGE_MEMBERSHIP} of the group.
111+
* Or if it has a permission to {@link AdminPermissionsSchema#MANAGE_MEMBERSHIP} of the group.
116112
*/
117113
boolean canManageMembership(GroupModel group);
118114

119115
/**
120-
* Returns {@code true} if {@link UserPermissionEvaluator#canView()} evaluates to {@code true}.
116+
* Returns {@code true} if the caller has one of {@link AdminRoles#MANAGE_USERS} or
117+
* {@link AdminRoles#VIEW_USERS} roles.
121118
* <p/>
122-
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW_MEMBERS} or
123-
* {@link AdminPermissionsSchema#MANAGE_MEMBERS} of the group.
119+
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW_MEMBERS} of the group.
124120
*/
125121
boolean canViewMembers(GroupModel group);
126122

‎services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public boolean canView() {
5555
return true;
5656
}
5757

58-
return hasPermission(null, AdminPermissionsSchema.VIEW, AdminPermissionsSchema.MANAGE);
58+
return hasPermission(null, AdminPermissionsSchema.VIEW);
5959
}
6060

6161
@Override
@@ -64,7 +64,7 @@ public boolean canView(GroupModel group) {
6464
return true;
6565
}
6666

67-
return hasPermission(group.getId(), AdminPermissionsSchema.VIEW, AdminPermissionsSchema.MANAGE);
67+
return hasPermission(group.getId(), AdminPermissionsSchema.VIEW);
6868
}
6969

7070
@Override
@@ -73,7 +73,7 @@ public boolean canManage() {
7373
return true;
7474
}
7575

76-
return hasPermission(null, AdminPermissionsSchema.VIEW, AdminPermissionsSchema.MANAGE);
76+
return hasPermission(null, AdminPermissionsSchema.MANAGE);
7777
}
7878

7979
@Override
@@ -91,7 +91,7 @@ public boolean canViewMembers(GroupModel group) {
9191
return true;
9292
}
9393

94-
return hasPermission(group.getId(), AdminPermissionsSchema.VIEW_MEMBERS, AdminPermissionsSchema.MANAGE_MEMBERS);
94+
return hasPermission(group.getId(), AdminPermissionsSchema.VIEW_MEMBERS);
9595
}
9696

9797
@Override
@@ -109,7 +109,7 @@ public boolean canManageMembership(GroupModel group) {
109109
return true;
110110
}
111111

112-
return hasPermission(group.getId(), AdminPermissionsSchema.MANAGE, AdminPermissionsSchema.MANAGE_MEMBERSHIP);
112+
return hasPermission(group.getId(), AdminPermissionsSchema.MANAGE_MEMBERSHIP);
113113
}
114114

115115
@Override

‎services/src/main/java/org/keycloak/services/resources/admin/permissions/RolePermissionsV2.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,16 @@
4545

4646
public class RolePermissionsV2 extends RolePermissions {
4747

48-
public RolePermissionsV2(KeycloakSession session, RealmModel realm, AuthorizationProvider authz, MgmtPermissions root) {
48+
RolePermissionsV2(KeycloakSession session, RealmModel realm, AuthorizationProvider authz, MgmtPermissions root) {
4949
super(session, realm, authz, root);
5050
}
5151

5252
@Override
5353
public boolean canMapClientScope(RoleModel role) {
5454
if (root.clients().canManageClientsDefault()) return true;
5555

56-
if (role.getContainer() instanceof ClientModel) {
57-
if (root.clients().canMapClientScopeRoles((ClientModel) role.getContainer())) return true;
56+
if (role.getContainer() instanceof ClientModel clientModel) {
57+
if (root.clients().canMapClientScopeRoles(clientModel)) return true;
5858
}
5959

6060
return hasPermission(role, MAP_ROLE_CLIENT_SCOPE_SCOPE);
@@ -64,8 +64,8 @@ public boolean canMapClientScope(RoleModel role) {
6464
public boolean canMapComposite(RoleModel role) {
6565
if (canManageDefault(role)) return checkAdminRoles(role);
6666

67-
if (role.getContainer() instanceof ClientModel) {
68-
if (root.clients().canMapCompositeRoles((ClientModel) role.getContainer())) return true;
67+
if (role.getContainer() instanceof ClientModel clientModel) {
68+
if (root.clients().canMapCompositeRoles(clientModel)) return true;
6969
}
7070

7171
return hasPermission(role, MAP_ROLE_COMPOSITE_SCOPE) && checkAdminRoles(role);
@@ -75,8 +75,8 @@ public boolean canMapComposite(RoleModel role) {
7575
public boolean canMapRole(RoleModel role) {
7676
if (root.hasOneAdminRole(AdminRoles.MANAGE_USERS)) return checkAdminRoles(role);
7777

78-
if (role.getContainer() instanceof ClientModel) {
79-
if (root.clients().canMapRoles((ClientModel) role.getContainer())) return true;
78+
if (role.getContainer() instanceof ClientModel clientModel) {
79+
if (root.clients().canMapRoles(clientModel)) return true;
8080
}
8181

8282
return hasPermission(role, MAP_ROLE_SCOPE) && checkAdminRoles(role);
@@ -158,7 +158,7 @@ private boolean hasGrantedPermission(ResourceServer server, Resource resource, S
158158
return true;
159159
}
160160
}
161-
};
161+
}
162162
}
163163

164164
return false;

‎services/src/main/java/org/keycloak/services/resources/admin/permissions/UserPermissionEvaluator.java‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ public interface UserPermissionEvaluator {
6666
* Returns {@code true} if the caller has at least one of {@link AdminRoles#QUERY_USERS},
6767
* {@link AdminRoles#MANAGE_USERS} or {@link AdminRoles#VIEW_USERS} roles.
6868
* <p/>
69-
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} or
70-
* {@link AdminPermissionsSchema#MANAGE} users.
69+
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} users.
7170
*/
7271
boolean canQuery();
7372

@@ -85,17 +84,15 @@ public interface UserPermissionEvaluator {
8584
* Returns {@code true} if the caller has one of {@link AdminRoles#MANAGE_USERS} or
8685
* {@link AdminRoles#VIEW_USERS} roles.
8786
* <p/>
88-
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} or
89-
* {@link AdminPermissionsSchema#MANAGE} users.
87+
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} users.
9088
*/
9189
boolean canView();
9290

9391
/**
9492
* Returns {@code true} if the caller has at least one of {@link AdminRoles#MANAGE_USERS} or
9593
* {@link AdminRoles#VIEW_USERS} roles.
9694
* <p/>
97-
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} or
98-
* {@link AdminPermissionsSchema#MANAGE} the user.
95+
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW} the user.
9996
* <p/>
10097
* Or if it has a permission to {@link AdminPermissionsSchema#VIEW_MEMBERS}
10198
* of the group chain the user is associated with.
@@ -136,11 +133,7 @@ public interface UserPermissionEvaluator {
136133
/**
137134
* Returns {@code true} if the caller has {@link AdminRoles#MANAGE_USERS} role.
138135
* <p/>
139-
* Or if it has a permission to {@link AdminPermissionsSchema#MANAGE} the user or
140-
* {@link AdminPermissionsSchema#MAP_ROLES} of the user.
141-
* <p/>
142-
* Or if it has a permission to {@link AdminPermissionsSchema#MANAGE_MEMBERS}
143-
* of the group chain the user is associated with.
136+
* Or if it has a permission to {@link AdminPermissionsSchema#MAP_ROLES} of the user.
144137
*/
145138
boolean canMapRoles(UserModel user);
146139

@@ -152,15 +145,12 @@ public interface UserPermissionEvaluator {
152145
/**
153146
* Returns {@code true} if the caller has {@link AdminRoles#MANAGE_USERS} role.
154147
* <p/>
155-
* Or if it has a permission to {@link AdminPermissionsSchema#MANAGE} the user or
156-
* {@link AdminPermissionsSchema#MANAGE_GROUP_MEMBERSHIP} of the user.
157-
* <p/>
158-
* Or if it has a permission to {@link AdminPermissionsSchema#MANAGE_MEMBERS}
159-
* of the group chain the user is associated with.
148+
* Or if it has a permission to {@link AdminPermissionsSchema#MANAGE_GROUP_MEMBERSHIP} of the user.
160149
*/
161150
boolean canManageGroupMembership(UserModel user);
162151

163152
@Deprecated
164153
boolean isImpersonatable(UserModel user, ClientModel requester);
154+
@Deprecated
165155
void grantIfNoPermission(boolean grantIfNoPermission);
166156
}

‎services/src/main/java/org/keycloak/services/resources/admin/permissions/UserPermissionsV2.java‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public boolean canView(UserModel user) {
5151
return true;
5252
}
5353

54-
return hasPermission(user, null, AdminPermissionsSchema.VIEW, AdminPermissionsSchema.MANAGE);
54+
return hasPermission(user, null, AdminPermissionsSchema.VIEW);
5555
}
5656

5757
@Override
@@ -60,7 +60,7 @@ public boolean canView() {
6060
return true;
6161
}
6262

63-
return hasPermission((UserModel) null, null, AdminPermissionsSchema.VIEW, AdminPermissionsSchema.MANAGE);
63+
return hasPermission((UserModel) null, null, AdminPermissionsSchema.VIEW);
6464
}
6565

6666
@Override
@@ -110,7 +110,7 @@ public boolean canMapRoles(UserModel user) {
110110
return true;
111111
}
112112

113-
return hasPermission(user, null, AdminPermissionsSchema.MANAGE, AdminPermissionsSchema.MAP_ROLES);
113+
return hasPermission(user, null, AdminPermissionsSchema.MAP_ROLES);
114114
}
115115

116116
@Override
@@ -119,10 +119,10 @@ public boolean canManageGroupMembership(UserModel user) {
119119
return true;
120120
}
121121

122-
return hasPermission(user, null, AdminPermissionsSchema.MANAGE, AdminPermissionsSchema.MANAGE_GROUP_MEMBERSHIP);
122+
return hasPermission(user, null, AdminPermissionsSchema.MANAGE_GROUP_MEMBERSHIP);
123123
}
124124

125-
private boolean hasPermission(UserModel user, EvaluationContext context, String... scopes) {
125+
private boolean hasPermission(UserModel user, EvaluationContext context, String scope) {
126126
if (!root.isAdminSameRealm()) {
127127
return false;
128128
}
@@ -145,14 +145,10 @@ private boolean hasPermission(UserModel user, EvaluationContext context, String.
145145
root.evaluatePermission(new ResourcePermission(resourceType, resource, resource.getScopes(), server), server) :
146146
root.evaluatePermission(new ResourcePermission(resourceType, resource, resource.getScopes(), server), server, context);
147147

148-
List<String> expectedScopes = List.of(scopes);
149-
150148
for (Permission permission : permissions) {
151149
if (permission.getResourceId().equals(resource.getId())) {
152-
for (String scope : permission.getScopes()) {
153-
if (expectedScopes.contains(scope)) {
154-
return true;
155-
}
150+
if (permission.getScopes().contains(scope)) {
151+
return true;
156152
}
157153
}
158154
}

0 commit comments

Comments
 (0)