Mercurial > hg > thermostat-ng > web-gateway

changeset 223:e233372c70ed

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Add support for multiple action roles Reviewed-by: jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-July/024212.html Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-August/024419.html
author Jie Kang <jkang@redhat.com>
date Fri, 04 Aug 2017 10:27:42 -0400
parents 52e4c97adcd7
children 99eebeb47f89
files common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RealmAuthorizer.java common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/Role.java common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactory.java common/core/src/test/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactoryTest.java common/core/src/test/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleTest.java
diffstat 5 files changed, 98 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- a/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RealmAuthorizer.java	Mon Jul 31 19:14:29 2017 +0200
+++ b/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RealmAuthorizer.java	Fri Aug 04 10:27:42 2017 -0400
@@ -77,7 +77,7 @@
 
     public boolean checkActionExists(String action) {
         for (Role role : clientRoles) {
-            if (role.getActions().contains(action)) {
+            if (role.containsAction(action)) {
                 return  true;
             }
         }
@@ -101,7 +101,7 @@
     public Set<String> getRealmsWithAction(String action) {
         Set<String> realms = new HashSet<>();
         for (Role role : clientRoles) {
-            if (role.getActions().contains(action)) {
+            if (role.containsAction(action)) {
                 realms.add(role.getRealm());
             }
         }
--- a/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/Role.java	Mon Jul 31 19:14:29 2017 +0200
+++ b/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/Role.java	Fri Aug 04 10:27:42 2017 -0400
@@ -36,23 +36,30 @@
 
 package com.redhat.thermostat.gateway.common.core.auth.keycloak;
 
+import java.util.Collections;
 import java.util.Objects;
+import java.util.Set;
 
 public class Role {
+    public static final String ACTION_DELIMITER = ",";
     public static final String ROLE_DELIMITER = "-";
-    public static final String[] RESTRICTED_CHARACTERS = new String[]{" "};
+    /**
+     * Array of regex that valid roles cannot match
+     * Role cannot contain any whitespaces
+     */
+    public static final String[] RESTRICTED_CHARACTERS_REGEX = new String[]{".*\\s+.*"};
 
-    private final String actions;
+    private final Set<String> actions;
     private final String realm;
 
-    public Role(String actions, String realm) {
+    public Role(Set<String> actions, String realm) {
         Objects.requireNonNull(actions);
         Objects.requireNonNull(realm);
-        this.actions = actions;
+        this.actions = Collections.unmodifiableSet(actions);
         this.realm = realm;
     }
 
-    public String getActions() {
+    public Set<String> getActions() {
         return this.actions;
     }
 
@@ -78,6 +85,10 @@
         return realm.equals(role.realm);
     }
 
+    public boolean containsAction(String action) {
+        return this.actions.contains(action);
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(actions, realm);
--- a/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactory.java	Mon Jul 31 19:14:29 2017 +0200
+++ b/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactory.java	Fri Aug 04 10:27:42 2017 -0400
@@ -36,14 +36,19 @@
 
 package com.redhat.thermostat.gateway.common.core.auth.keycloak;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
 public class RoleFactory {
 
     public boolean isValidRole(String role) {
         if (!role.contains(Role.ROLE_DELIMITER)) {
             return false;
         }
-        for (String restrictedCharacter : Role.RESTRICTED_CHARACTERS) {
-            if (role.contains(restrictedCharacter)) {
+        for (String restrictedCharacter : Role.RESTRICTED_CHARACTERS_REGEX) {
+            if (role.matches(restrictedCharacter)) {
                 return false;
             }
         }
@@ -55,11 +60,12 @@
     }
 
     public Role buildRole(String role) {
-
         int index = role.indexOf(Role.ROLE_DELIMITER);
         String actions = role.substring(0, index);
+
         String realm = role.substring(index + 1);
 
-        return new Role(actions, realm);
+        Set<String> actionSet = new HashSet<>(Arrays.asList(actions.split(Role.ACTION_DELIMITER)));
+        return new Role(actionSet, realm);
     }
 }
--- a/common/core/src/test/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactoryTest.java	Mon Jul 31 19:14:29 2017 +0200
+++ b/common/core/src/test/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactoryTest.java	Fri Aug 04 10:27:42 2017 -0400
@@ -40,6 +40,9 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import org.junit.Before;
 import org.junit.Test;
 
@@ -57,18 +60,24 @@
         String role = "a-valid,role";
         assertTrue(roleFactory.isValidRole(role));
 
+        Set<String> actions = new HashSet<>();
+        actions.add("a");
         Role r = roleFactory.buildRole(role);
-        verifyRole(r, "a", "valid,role");
+        verifyRole(r, actions, "valid,role");
     }
 
     @Test
     public void testValidRoleWithActions() {
-        String role = "rwd-role";
+        String role = "r,w,d-role";
 
         assertTrue(roleFactory.isValidRole(role));
 
         Role r = roleFactory.buildRole(role);
-        verifyRole(r, "rwd", "role");
+        Set<String> actions = new HashSet<>();
+        actions.add("r");
+        actions.add("w");
+        actions.add("d");
+        verifyRole(r, actions, "role");
     }
 
     @Test
@@ -89,17 +98,24 @@
         assertTrue(roleFactory.isValidRole(role));
 
         Role r = roleFactory.buildRole(role);
-        verifyRole(r, "a", "realm-with-hyphens");
+        Set<String> actions = new HashSet<>();
+        actions.add("a");
+        verifyRole(r, actions, "realm-with-hyphens");
     }
 
     @Test
-    public void testRealmWithSpaceIsInvalid() {
+    public void testRealmWithWhitespaceIsInvalid() {
         String role = "a-invalid realm";
         assertFalse(roleFactory.isValidRole(role));
+
+        role = "a-nother\tinvalidrealm ";
+        assertFalse(roleFactory.isValidRole(role));
     }
 
-    private void verifyRole(Role role, String expectedActions, String expectedRole) {
-        assertEquals(expectedActions, role.getActions());
+    private void verifyRole(Role role, Set<String> expectedActions, String expectedRole) {
+        for (String item : expectedActions) {
+            assertTrue(role.containsAction(item));
+        }
         assertEquals(expectedRole, role.getRealm());
     }
 }
--- a/common/core/src/test/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleTest.java	Mon Jul 31 19:14:29 2017 +0200
+++ b/common/core/src/test/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleTest.java	Fri Aug 04 10:27:42 2017 -0400
@@ -40,22 +40,29 @@
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import org.junit.Test;
 
 public class RoleTest {
 
     @Test
     public void testSimpleRole() {
-        Role r = new Role("a", "realm");
+        Set<String> actions = new HashSet<>();
+        actions.add("a");
+        Role r = new Role(actions, "realm");
 
-        verifyRole(r, "a", "realm");
+        verifyRole(r, actions, "realm");
     }
 
     @Test
     public void testMultipleActionsRole() {
-        Role r = new Role("rw", "realm-1.2-3");
+        Set<String> actions = new HashSet<>();
+        actions.add("a");
+        Role r = new Role(actions, "realm-1.2-3");
 
-        verifyRole(r, "rw", "realm-1.2-3");
+        verifyRole(r, actions, "realm-1.2-3");
     }
 
     /*
@@ -64,24 +71,53 @@
      */
     @Test
     public void testEquals() {
-        Role one = new Role("a", "b");
-        Role two = new Role("a", "b");
+        Set<String> actionsOne = new HashSet<>();
+        actionsOne.add("a");
+
+        Role one = new Role(actionsOne, "b");
+        Role two = new Role(actionsOne, "b");
 
         assertEquals(one, two);
+
+        Set<String> actionsTwo = new HashSet<>();
+        actionsTwo.add("a");
+
+        Role three = new Role(actionsTwo, "b");
+
+        assertEquals(one, three);
     }
 
     @Test
     public void testNotEquals() {
-        Role one = new Role("a", "b");
-        Role two = new Role("a", "c");
-        Role three = new Role("c", "b");
+        Set<String> actionsOne = new HashSet<>();
+        actionsOne.add("a");
+
+        Set<String> actionsTwo = new HashSet<>();
+        actionsTwo.add("b");
+
+        Role one = new Role(actionsOne, "b");
+        Role two = new Role(actionsOne, "c");
+        Role three = new Role(actionsTwo, "b");
 
         assertNotEquals(one, two);
         assertNotEquals(one, three);
     }
 
-    private void verifyRole(Role role, String expectedActions, String expectedRole) {
-        assertEquals(expectedActions, role.getActions());
+    @Test(expected = UnsupportedOperationException.class)
+    public void testActionsCannotBeModified() {
+        Set<String> actions = new HashSet<>();
+        actions.add("a");
+        Role r = new Role(actions, "realm-1.2-3");
+
+        verifyRole(r, actions, "realm-1.2-3");
+
+        r.getActions().add("not-allowed");
+    }
+
+    private void verifyRole(Role role, Set<String> expectedActions, String expectedRole) {
+        for (String item : expectedActions) {
+            assertTrue(role.containsAction(item));
+        }
         assertEquals(expectedRole, role.getRealm());
     }
 }