Mercurial > hg > thermostat-ng > web-gateway
changeset 226:bc1a040d8234
Trim role string and use exception for invalid role
Reviewed-by: jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-August/024470.html
author | Jie Kang <jkang@redhat.com> |
---|---|
date | Tue, 08 Aug 2017 13:04:31 -0400 |
parents | 631f6e045fe5 |
children | c3e64deb1f12 |
files | common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/InvalidRoleException.java 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/RoleFactory.java common/core/src/test/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactoryTest.java |
diffstat | 4 files changed, 89 insertions(+), 25 deletions(-) [+] |
line wrap: on
line diff
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/InvalidRoleException.java Tue Aug 08 13:04:31 2017 -0400 @@ -0,0 +1,43 @@ +/* + * Copyright 2012-2017 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.gateway.common.core.auth.keycloak; + +public class InvalidRoleException extends Exception { + public InvalidRoleException(String s) { + super(s); + } +}
--- a/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RealmAuthorizer.java Fri Aug 04 11:15:28 2017 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RealmAuthorizer.java Tue Aug 08 13:04:31 2017 -0400 @@ -133,8 +133,10 @@ .getAttribute(KeycloakSecurityContext.class.getName()); for (String role : keycloakSecurityContext.getToken().getRealmAccess().getRoles()) { - if (roleFactory.isValidRole(role)) { + try { keycloakRoles.add(roleFactory.buildRole(role)); + } catch (InvalidRoleException e) { + //Do nothing } }
--- a/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactory.java Fri Aug 04 11:15:28 2017 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactory.java Tue Aug 08 13:04:31 2017 -0400 @@ -37,13 +37,12 @@ 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) { + private boolean isValidRole(String role) { if (!role.contains(Role.ROLE_DELIMITER)) { return false; } @@ -59,7 +58,13 @@ return index > 0 && index < role.length() - 1; } - public Role buildRole(String role) { + public Role buildRole(String role) throws InvalidRoleException { + role = role.trim(); + + if (!isValidRole(role)) { + throw new InvalidRoleException("Invalid role: " + role); + } + int index = role.indexOf(Role.ROLE_DELIMITER); String actions = role.substring(0, index);
--- a/common/core/src/test/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactoryTest.java Fri Aug 04 11:15:28 2017 +0200 +++ b/common/core/src/test/java/com/redhat/thermostat/gateway/common/core/auth/keycloak/RoleFactoryTest.java Tue Aug 08 13:04:31 2017 -0400 @@ -56,9 +56,9 @@ } @Test - public void testValidRole() { + public void testValidRole() throws InvalidRoleException { String role = "a-valid,role"; - assertTrue(roleFactory.isValidRole(role)); + roleFactory.buildRole(role); Set<String> actions = new HashSet<>(); actions.add("a"); @@ -67,11 +67,9 @@ } @Test - public void testValidRoleWithActions() { + public void testValidRoleWithActions() throws InvalidRoleException { String role = "r,w,d-role"; - assertTrue(roleFactory.isValidRole(role)); - Role r = roleFactory.buildRole(role); Set<String> actions = new HashSet<>(); actions.add("r"); @@ -80,22 +78,21 @@ verifyRole(r, actions, "role"); } - @Test - public void testNoActionRole() { + @Test(expected = InvalidRoleException.class) + public void testNoActionRole() throws InvalidRoleException { String role = "-role"; - assertFalse(roleFactory.isValidRole(role)); + roleFactory.buildRole(role); + } + + @Test(expected = InvalidRoleException.class) + public void testNoRealmRole() throws InvalidRoleException { + String role = "a-"; + roleFactory.buildRole(role); } @Test - public void testNoRealmRole() { - String role = "a-"; - assertFalse(roleFactory.isValidRole(role)); - } - - @Test - public void testHyphenRealm() { + public void testHyphenRealm() throws InvalidRoleException { String role = "a-realm-with-hyphens"; - assertTrue(roleFactory.isValidRole(role)); Role r = roleFactory.buildRole(role); Set<String> actions = new HashSet<>(); @@ -103,13 +100,30 @@ verifyRole(r, actions, "realm-with-hyphens"); } + @Test(expected = InvalidRoleException.class) + public void testRealmWithWhitespaceIsInvalid() throws InvalidRoleException { + String role = "a-invalid \trealm"; + roleFactory.buildRole(role); + } + @Test - public void testRealmWithWhitespaceIsInvalid() { - String role = "a-invalid realm"; - assertFalse(roleFactory.isValidRole(role)); + public void testRealmWithLeadingWhitespace() throws InvalidRoleException { + String role = " a-role"; + + Role r = roleFactory.buildRole(role); + Set<String> actions = new HashSet<>(); + actions.add("a"); + verifyRole(r, actions, "role"); + } - role = "a-nother\tinvalidrealm "; - assertFalse(roleFactory.isValidRole(role)); + @Test + public void testRealmWithTrailingWhitespace() throws InvalidRoleException { + String role = "a-role\t"; + + Role r = roleFactory.buildRole(role); + Set<String> actions = new HashSet<>(); + actions.add("a"); + verifyRole(r, actions, "role"); } private void verifyRole(Role role, Set<String> expectedActions, String expectedRole) {