modified again according to rick's comment

yahoo · Nov 17, 2016 · bf007a7 · bf007a7
1 parent 7adfc89
commit bf007a7
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 5 deletions.
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/filters/RoleBasedAuthFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/filters/RoleBasedAuthFilter.java
@@ -94,7 +94,7 @@ protected boolean isBypassMethod(ContainerRequestContext containerRequestContext
      */
     protected boolean isUserInRole (List<String> allowedUserRoles, ContainerRequestContext containerRequestContext) {
         SecurityContext securityContext = containerRequestContext.getSecurityContext();
-        return allowedUserRoles.isEmpty() ? true : allowedUserRoles.stream().anyMatch(securityContext::isUserInRole);
+        return allowedUserRoles.isEmpty() || allowedUserRoles.stream().anyMatch(securityContext::isUserInRole);
     }
 
     /**

diff --git a/...core/src/test/groovy/com/yahoo/bard/webservice/web/filters/RoleBasedAuthFilterSpec.groovy b/...core/src/test/groovy/com/yahoo/bard/webservice/web/filters/RoleBasedAuthFilterSpec.groovy
@@ -3,6 +3,7 @@
 package com.yahoo.bard.webservice.web.filters;
 
 import com.yahoo.bard.webservice.config.SystemConfig
+import com.yahoo.bard.webservice.config.SystemConfigException
 import com.yahoo.bard.webservice.config.SystemConfigProvider
 
 import com.fasterxml.jackson.databind.ObjectMapper
@@ -17,7 +18,11 @@ import javax.ws.rs.core.SecurityContext
 @Timeout(30)
 class RoleBasedAuthFilterSpec extends Specification {
 
-    private final SystemConfig SYSTEM_CONFIG = SystemConfigProvider.getInstance()
+    private static final SystemConfig SYSTEM_CONFIG = SystemConfigProvider.getInstance()
+    private static final String userRolesSetting = SYSTEM_CONFIG.getPackageVariableName("user_roles")
+
+    // Memorize pre-test setting value
+    Optional<String> oldUserRoles
 
     ObjectMapper objectMapper
     ContainerRequestContext containerRequestContext
@@ -29,13 +34,19 @@ class RoleBasedAuthFilterSpec extends Specification {
         containerRequestContext = Mock(ContainerRequestContext)
         securityContext = Mock(SecurityContext)
         containerRequestContext.getSecurityContext() >> securityContext
-
         roleBasedAuthFilter = new RoleBasedAuthFilter(objectMapper)
-        SYSTEM_CONFIG.setProperty(SYSTEM_CONFIG.getPackageVariableName("user_roles"), "foo,bar");
+
+        try {
+            oldUserRoles = SYSTEM_CONFIG.getStringProperty(userRolesSetting)
+        } catch(SystemConfigException e) {
+            oldUserRoles = null
+        }
+        SYSTEM_CONFIG.setProperty(userRolesSetting, "foo,bar");
     }
 
     def cleanup() {
-        SYSTEM_CONFIG.clearProperty(SYSTEM_CONFIG.getPackageVariableName("user_roles"))
+        // Restore pre-test setting value if exists, clear otherwise
+        SYSTEM_CONFIG.resetProperty(userRolesSetting, oldUserRoles)
     }
 
     def "GET request is not authorized if the user is not in an allowed role"() {