From bf007a7ad1b628f557e57e8f947320abcf7aec5d Mon Sep 17 00:00:00 2001 From: Gary Luo Date: Thu, 17 Nov 2016 12:16:44 -0600 Subject: [PATCH] modified again according to rick's comment --- .../web/filters/RoleBasedAuthFilter.java | 2 +- .../filters/RoleBasedAuthFilterSpec.groovy | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 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 index aa5f2787c2..1aa867721a 100644 --- 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 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/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/filters/RoleBasedAuthFilterSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/filters/RoleBasedAuthFilterSpec.groovy index 02a462da78..52a107c618 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/filters/RoleBasedAuthFilterSpec.groovy +++ b/fili-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 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"() {