KEYCLOAK-1959 Role offline_access was effective only when explicitly …

…added to user
keycloak · Oct 15, 2015 · b4520ba · b4520ba
1 parent 802a39b
commit b4520ba
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 1 deletion.
diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java
@@ -44,6 +44,8 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -325,6 +327,21 @@ public static Set<RoleModel> getAccess(String scopeParam, boolean applyScopePara
                     }
                 }
             }
+
+            // Add all roles specified in scope parameter directly into requestedRoles, even if they are available just through composite role
+            List<RoleModel> scopeRoles = new LinkedList<>();
+            for (String scopeParamPart : scopeParamRoles) {
+                RoleModel scopeParamRole = getRoleFromScopeParam(client.getRealm(), scopeParamPart);
+                if (scopeParamRole != null) {
+                    for (RoleModel role : roles) {
+                        if (role.hasRole(scopeParamRole)) {
+                            scopeRoles.add(scopeParamRole);
+                        }
+                    }
+                }
+            }
+
+            roles.addAll(scopeRoles);
             requestedRoles = roles;
         }
 
@@ -341,6 +358,17 @@ private static String getRoleNameForScopeParam(RoleModel role) {
         }
     }
 
+    // For now, just use "roleName" for realm roles and "clientId/roleName" for client roles
+    private static RoleModel getRoleFromScopeParam(RealmModel realm, String scopeParamRole) {
+        String[] parts = scopeParamRole.split("/");
+        if (parts.length == 1) {
+            return realm.getRole(parts[0]);
+        } else {
+            ClientModel roleClient = realm.getClientByClientId(parts[0]);
+            return roleClient!=null ? roleClient.getRole(parts[1]) : null;
+        }
+    }
+
     public void verifyAccess(AccessToken token, AccessToken newToken) throws OAuthErrorException {
         if (token.getRealmAccess() != null) {
             if (newToken.getRealmAccess() == null) throw new OAuthErrorException(OAuthErrorException.INVALID_SCOPE, "User no long has permission for realm roles");

diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OfflineTokenTest.java
@@ -30,6 +30,7 @@
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RoleModel;
 import org.keycloak.models.UserModel;
+import org.keycloak.protocol.oidc.TokenManager;
 import org.keycloak.representations.AccessToken;
 import org.keycloak.representations.RefreshToken;
 import org.keycloak.services.managers.ClientManager;
@@ -285,7 +286,6 @@ public void config(RealmManager manager, RealmModel adminstrationRealm, RealmMod
 
         Assert.assertEquals(userId, refreshedToken.getSubject());
 
-        Assert.assertEquals(2, refreshedToken.getRealmAccess().getRoles().size());
         Assert.assertTrue(refreshedToken.getRealmAccess().isUserInRole("user"));
         Assert.assertTrue(refreshedToken.getRealmAccess().isUserInRole(Constants.OFFLINE_ACCESS_ROLE));
 
@@ -384,6 +384,54 @@ public void offlineTokenServiceAccountFlow() throws Exception {
         testRefreshWithOfflineToken(token2, offlineToken2, offlineTokenString2, token2.getSessionState(), serviceAccountUserId);
     }
 
+    @Test
+    public void offlineTokenAllowedWithCompositeRole() throws Exception {
+        keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                ClientModel offlineClient = appRealm.getClientByClientId("offline-client");
+                UserModel testUser = session.users().getUserByUsername("test-user@localhost", appRealm);
+                RoleModel offlineAccess = appRealm.getRole(Constants.OFFLINE_ACCESS_ROLE);
+
+                // Test access
+                Assert.assertFalse(TokenManager.getAccess(null, true, offlineClient, testUser).contains(offlineAccess));
+                Assert.assertTrue(TokenManager.getAccess(OAuth2Constants.OFFLINE_ACCESS, true, offlineClient, testUser).contains(offlineAccess));
+
+                // Grant offline_access role indirectly through composite role
+                RoleModel composite = appRealm.addRole("composite");
+                composite.addCompositeRole(offlineAccess);
+
+                testUser.deleteRoleMapping(offlineAccess);
+                testUser.grantRole(composite);
+
+                // Test access
+                Assert.assertFalse(TokenManager.getAccess(null, true, offlineClient, testUser).contains(offlineAccess));
+                Assert.assertTrue(TokenManager.getAccess(OAuth2Constants.OFFLINE_ACCESS, true, offlineClient, testUser).contains(offlineAccess));
+            }
+
+        });
+
+        // Integration test
+        offlineTokenDirectGrantFlow();
+
+        // Revert changes
+        keycloakRule.update(new KeycloakRule.KeycloakSetup() {
+
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                RoleModel composite = appRealm.getRole("composite");
+                RoleModel offlineAccess = appRealm.getRole(Constants.OFFLINE_ACCESS_ROLE);
+                UserModel testUser = session.users().getUserByUsername("test-user@localhost", appRealm);
+
+                testUser.deleteRoleMapping(composite);
+                appRealm.removeRole(composite);
+                testUser.grantRole(offlineAccess);
+            }
+
+        });
+    }
+
     @Test
     public void testServlet() {
         OfflineTokenServlet.tokenInfo = null;