From 64bdf8718e56941a460b5a73317513e6c6912017 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Mon, 21 Aug 2023 10:02:23 -0400 Subject: [PATCH 1/7] PathMatches Signed-off-by: Stephen Crawford --- .../security/filter/RestPathMatchesTest.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/test/java/org/opensearch/security/filter/RestPathMatchesTest.java b/src/test/java/org/opensearch/security/filter/RestPathMatchesTest.java index 9a5335bdb7..920d640776 100644 --- a/src/test/java/org/opensearch/security/filter/RestPathMatchesTest.java +++ b/src/test/java/org/opensearch/security/filter/RestPathMatchesTest.java @@ -63,4 +63,25 @@ public void testDifferentPathSegments() throws InvocationTargetException, Illega String handlerPath = "_plugins/security/api/x/y"; assertFalse((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath)); } + + @Test + public void testRequestPathWithNamedParam() throws InvocationTargetException, IllegalAccessException { + String requestPath = "_plugins/security/api/123/y"; + String handlerPath = "_plugins/security/api/{id}/z"; + assertFalse((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath)); + } + + @Test + public void testRequestPathMismatch() throws InvocationTargetException, IllegalAccessException { + String requestPath = "_plugins/security/api/x/y"; + String handlerPath = "_plugins/security/api/z/y"; + assertFalse((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath)); + } + + @Test + public void testRequestPathWithExtraSegments() throws InvocationTargetException, IllegalAccessException { + String requestPath = "_plugins/security/api/x/y/z"; + String handlerPath = "_plugins/security/api/x/y"; + assertFalse((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath)); + } } From 8efb311d001bdddb66c30304a590a284d4615c9d Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Mon, 21 Aug 2023 10:02:54 -0400 Subject: [PATCH 2/7] coverage Signed-off-by: Stephen Crawford --- .../RestLayerPrivilegesEvaluatorTest.java | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java index 3c8145c393..1e4f2a564d 100644 --- a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java @@ -73,11 +73,11 @@ public void setUp() throws InstantiationException, IllegalAccessException { when(threadPool.getThreadContext()).thenReturn(context); privilegesEvaluator = new RestLayerPrivilegesEvaluator( - clusterService, - threadPool, - mock(AuditLog.class), - mock(ClusterInfoHolder.class), - namedXContentRegistry + clusterService, + threadPool, + mock(AuditLog.class), + mock(ClusterInfoHolder.class), + namedXContentRegistry ); privilegesEvaluator.onConfigModelChanged(configModel); privilegesEvaluator.onDynamicConfigModelChanged(dcm); @@ -128,7 +128,7 @@ public void testEvaluate_Successful_NewPermission() { when(securityRoles.impliesClusterPermissionPermission(action)).thenReturn(true); PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); - + assertTrue(privilegesEvaluator.isInitialized()); assertTrue(response.allowed); verify(securityRoles).impliesClusterPermissionPermission(any()); } @@ -160,4 +160,16 @@ public void testEvaluate_Unsuccessful() { assertFalse(response.allowed); verify(securityRoles).impliesClusterPermissionPermission(any()); } + + @Test + public void testIsInitialized_False() { + privilegesEvaluator = new RestLayerPrivilegesEvaluator( + clusterService, + threadPool, + mock(AuditLog.class), + mock(ClusterInfoHolder.class), + namedXContentRegistry + ); + assertFalse(privilegesEvaluator.isInitialized()); + } } From d4d15ec7df3d8d25a7f8579e2e8bf4a283fb0315 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Mon, 21 Aug 2023 10:03:16 -0400 Subject: [PATCH 3/7] Spotless Signed-off-by: Stephen Crawford --- .../RestLayerPrivilegesEvaluatorTest.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java index 1e4f2a564d..9bf7b6d3a8 100644 --- a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java @@ -73,11 +73,11 @@ public void setUp() throws InstantiationException, IllegalAccessException { when(threadPool.getThreadContext()).thenReturn(context); privilegesEvaluator = new RestLayerPrivilegesEvaluator( - clusterService, - threadPool, - mock(AuditLog.class), - mock(ClusterInfoHolder.class), - namedXContentRegistry + clusterService, + threadPool, + mock(AuditLog.class), + mock(ClusterInfoHolder.class), + namedXContentRegistry ); privilegesEvaluator.onConfigModelChanged(configModel); privilegesEvaluator.onDynamicConfigModelChanged(dcm); @@ -164,11 +164,11 @@ public void testEvaluate_Unsuccessful() { @Test public void testIsInitialized_False() { privilegesEvaluator = new RestLayerPrivilegesEvaluator( - clusterService, - threadPool, - mock(AuditLog.class), - mock(ClusterInfoHolder.class), - namedXContentRegistry + clusterService, + threadPool, + mock(AuditLog.class), + mock(ClusterInfoHolder.class), + namedXContentRegistry ); assertFalse(privilegesEvaluator.isInitialized()); } From acb23d2a3125546687ac843142f0d40a0c2492fa Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Tue, 22 Aug 2023 09:41:42 -0400 Subject: [PATCH 4/7] Fix naming Signed-off-by: Stephen Crawford --- .../{RestPathMatchesTest.java => RestPathMatchesTests.java} | 2 +- .../{SecurityFilterTest.java => SecurityFilterTests.java} | 4 ++-- ...curityRestFilterTest.java => SecurityRestFilterTests.java} | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename src/test/java/org/opensearch/security/filter/{RestPathMatchesTest.java => RestPathMatchesTests.java} (99%) rename src/test/java/org/opensearch/security/filter/{SecurityFilterTest.java => SecurityFilterTests.java} (97%) rename src/test/java/org/opensearch/security/filter/{SecurityRestFilterTest.java => SecurityRestFilterTests.java} (99%) diff --git a/src/test/java/org/opensearch/security/filter/RestPathMatchesTest.java b/src/test/java/org/opensearch/security/filter/RestPathMatchesTests.java similarity index 99% rename from src/test/java/org/opensearch/security/filter/RestPathMatchesTest.java rename to src/test/java/org/opensearch/security/filter/RestPathMatchesTests.java index 920d640776..eed095c9b6 100644 --- a/src/test/java/org/opensearch/security/filter/RestPathMatchesTest.java +++ b/src/test/java/org/opensearch/security/filter/RestPathMatchesTests.java @@ -18,7 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; -public class RestPathMatchesTest { +public class RestPathMatchesTests { Method restPathMatches; SecurityRestFilter securityRestFilter; diff --git a/src/test/java/org/opensearch/security/filter/SecurityFilterTest.java b/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java similarity index 97% rename from src/test/java/org/opensearch/security/filter/SecurityFilterTest.java rename to src/test/java/org/opensearch/security/filter/SecurityFilterTests.java index ea2978302e..58a12a84a8 100644 --- a/src/test/java/org/opensearch/security/filter/SecurityFilterTest.java +++ b/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java @@ -47,12 +47,12 @@ import static org.mockito.Mockito.when; @RunWith(Parameterized.class) -public class SecurityFilterTest { +public class SecurityFilterTests { private final Settings settings; private final WildcardMatcher expected; - public SecurityFilterTest(Settings settings, WildcardMatcher expected) { + public SecurityFilterTests(Settings settings, WildcardMatcher expected) { this.settings = settings; this.expected = expected; } diff --git a/src/test/java/org/opensearch/security/filter/SecurityRestFilterTest.java b/src/test/java/org/opensearch/security/filter/SecurityRestFilterTests.java similarity index 99% rename from src/test/java/org/opensearch/security/filter/SecurityRestFilterTest.java rename to src/test/java/org/opensearch/security/filter/SecurityRestFilterTests.java index 692a950fb1..5adcadb1f2 100644 --- a/src/test/java/org/opensearch/security/filter/SecurityRestFilterTest.java +++ b/src/test/java/org/opensearch/security/filter/SecurityRestFilterTests.java @@ -27,7 +27,7 @@ * Currently tests that the whitelisting functionality works correctly. * Uses the test/resources/restapi folder for setup. */ -public class SecurityRestFilterTest extends AbstractRestApiUnitTest { +public class SecurityRestFilterTests extends AbstractRestApiUnitTest { private RestHelper.HttpResponse response; From 8bb213c99acc4fdbf88efcf497ab3acd585954ef Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Tue, 22 Aug 2023 11:28:21 -0500 Subject: [PATCH 5/7] Increase coverage target In RestLayerPrivilegesEvaluator there was a number of unused parameters removed those to trim down the test requirements. For the test cases switched to assertThat for easy of reading, enabled debug logging to exercise debug logging code, adjusted usages of mocks for cleaner test flow in a couple of spaces. Verified locally with the following command and then checked the jacoco report under build/reports/jacoco/test/html/... ./gradlew jacocoTestReport test --tests \ org.opensearch.security.privileges.RestLayerPrivilegesEvaluatorTest Signed-off-by: Peter Nied --- .../security/OpenSearchSecurityPlugin.java | 2 +- .../RestLayerPrivilegesEvaluator.java | 47 ++----- .../RestLayerPrivilegesEvaluatorTest.java | 117 +++++++++++------- 3 files changed, 82 insertions(+), 84 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index a43afcb187..c7c666bdaf 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1023,7 +1023,7 @@ public Collection createComponents( principalExtractor = ReflectionHelper.instantiatePrincipalExtractor(principalExtractorClass); } - restLayerEvaluator = new RestLayerPrivilegesEvaluator(clusterService, threadPool, auditLog, cih, namedXContentRegistry); + restLayerEvaluator = new RestLayerPrivilegesEvaluator(clusterService, threadPool); securityRestHandler = new SecurityRestFilter( backendRegistry, diff --git a/src/main/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluator.java index 8602fb91e6..dc95e98d11 100644 --- a/src/main/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluator.java @@ -12,7 +12,6 @@ package org.opensearch.security.privileges; import java.util.Set; -import java.util.concurrent.atomic.AtomicReference; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -22,11 +21,7 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.core.xcontent.NamedXContentRegistry; -import org.opensearch.security.auditlog.AuditLog; -import org.opensearch.security.configuration.ClusterInfoHolder; import org.opensearch.security.securityconf.ConfigModel; -import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.securityconf.SecurityRoles; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.User; @@ -35,49 +30,28 @@ public class RestLayerPrivilegesEvaluator { protected final Logger log = LogManager.getLogger(this.getClass()); private final ClusterService clusterService; - private final AuditLog auditLog; private ThreadContext threadContext; - private final ClusterInfoHolder clusterInfoHolder; private ConfigModel configModel; - private DynamicConfigModel dcm; - private final AtomicReference namedXContentRegistry; - - public RestLayerPrivilegesEvaluator( - final ClusterService clusterService, - final ThreadPool threadPool, - AuditLog auditLog, - final ClusterInfoHolder clusterInfoHolder, - AtomicReference namedXContentRegistry - ) { - this.clusterService = clusterService; - this.auditLog = auditLog; + public RestLayerPrivilegesEvaluator(final ClusterService clusterService, final ThreadPool threadPool) { + this.clusterService = clusterService; this.threadContext = threadPool.getThreadContext(); - - this.clusterInfoHolder = clusterInfoHolder; - this.namedXContentRegistry = namedXContentRegistry; } @Subscribe - public void onConfigModelChanged(ConfigModel configModel) { + public void onConfigModelChanged(final ConfigModel configModel) { this.configModel = configModel; } - @Subscribe - public void onDynamicConfigModelChanged(DynamicConfigModel dcm) { - this.dcm = dcm; - } - - private SecurityRoles getSecurityRoles(Set roles) { + SecurityRoles getSecurityRoles(final Set roles) { return configModel.getSecurityRoles().filter(roles); } - public boolean isInitialized() { - return configModel != null && configModel.getSecurityRoles() != null && dcm != null; + boolean isInitialized() { + return configModel != null && configModel.getSecurityRoles() != null; } - public PrivilegesEvaluatorResponse evaluate(final User user, Set actions) { - + public PrivilegesEvaluatorResponse evaluate(final User user, final Set actions) { if (!isInitialized()) { throw new OpenSearchSecurityException("OpenSearch Security is not initialized."); } @@ -86,7 +60,7 @@ public PrivilegesEvaluatorResponse evaluate(final User user, Set actions final TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS); - Set mappedRoles = mapRoles(user, caller); + final Set mappedRoles = mapRoles(user, caller); presponse.resolvedSecurityRoles.addAll(mappedRoles); final SecurityRoles securityRoles = getSecurityRoles(mappedRoles); @@ -98,7 +72,7 @@ public PrivilegesEvaluatorResponse evaluate(final User user, Set actions log.debug("Mapped roles: {}", mappedRoles.toString()); } - for (String action : actions) { + for (final String action : actions) { if (!securityRoles.impliesClusterPermissionPermission(action)) { presponse.missingPrivileges.add(action); presponse.allowed = false; @@ -123,8 +97,7 @@ public PrivilegesEvaluatorResponse evaluate(final User user, Set actions return presponse; } - public Set mapRoles(final User user, final TransportAddress caller) { + Set mapRoles(final User user, final TransportAddress caller) { return this.configModel.mapSecurityRoles(user, caller); } - } diff --git a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java index 3c8145c393..145bf56d9d 100644 --- a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java @@ -13,76 +13,83 @@ import java.util.Collections; import java.util.Set; -import java.util.concurrent.atomic.AtomicReference; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.core.config.Configurator; +import org.hamcrest.Matchers; +import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; - +import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.quality.Strictness; import org.opensearch.OpenSearchSecurityException; +import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.core.xcontent.NamedXContentRegistry; -import org.opensearch.security.auditlog.AuditLog; -import org.opensearch.security.configuration.ClusterInfoHolder; +import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.security.securityconf.ConfigModel; import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.securityconf.SecurityRoles; import org.opensearch.security.user.User; import org.opensearch.threadpool.ThreadPool; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.any; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; +@RunWith(MockitoJUnitRunner.class) public class RestLayerPrivilegesEvaluatorTest { - @Mock + @Mock(strictness = Mock.Strictness.LENIENT) private ClusterService clusterService; @Mock private ThreadPool threadPool; @Mock - private AtomicReference namedXContentRegistry; - @Mock private ConfigModel configModel; @Mock private DynamicConfigModel dcm; @Mock private PrivilegesEvaluatorResponse presponse; - @Mock - private Logger log; private RestLayerPrivilegesEvaluator privilegesEvaluator; private static final User TEST_USER = new User("test_user"); + private void setLoggingLevel(final Level level) { + final Logger restLayerPrivilegesEvaluatorLogger = LogManager.getLogger(RestLayerPrivilegesEvaluator.class); + Configurator.setLevel(restLayerPrivilegesEvaluatorLogger, level); + } + @Before public void setUp() throws InstantiationException, IllegalAccessException { - MockitoAnnotations.openMocks(this); - - ThreadContext context = new ThreadContext(Settings.EMPTY); - when(threadPool.getThreadContext()).thenReturn(context); + when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); + when(clusterService.localNode()).thenReturn(mock(DiscoveryNode.class, withSettings().strictness(Strictness.LENIENT))); privilegesEvaluator = new RestLayerPrivilegesEvaluator( clusterService, - threadPool, - mock(AuditLog.class), - mock(ClusterInfoHolder.class), - namedXContentRegistry + threadPool ); - privilegesEvaluator.onConfigModelChanged(configModel); - privilegesEvaluator.onDynamicConfigModelChanged(dcm); + privilegesEvaluator.onConfigModelChanged(configModel); // Defaults to the mocked config model + verify(threadPool).getThreadContext(); // Called during construction of RestLayerPrivilegesEvaluator + setLoggingLevel(Level.DEBUG); // Enable debug logging scenarios for verification + } - when(log.isDebugEnabled()).thenReturn(false); + @After + public void after() { + setLoggingLevel(Level.INFO); } @Test @@ -95,28 +102,45 @@ public void testEvaluate_Initialized_Success() { PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); - assertNotNull(response); - assertFalse(response.isAllowed()); - assertFalse(response.getMissingPrivileges().isEmpty()); - assertTrue(response.getResolvedSecurityRoles().isEmpty()); + assertThat(response.isAllowed(), equalTo(false)); + assertThat(response.getMissingPrivileges(), equalTo(Set.of(action))); + assertThat(response.getResolvedSecurityRoles(), Matchers.empty()); verify(configModel, times(3)).getSecurityRoles(); } - @Test(expected = OpenSearchSecurityException.class) - public void testEvaluate_NotInitialized_ExceptionThrown() throws Exception { - String action = "action"; - privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); + @Test + public void testEvaluate_NotInitialized_NullModel_ExceptionThrown() throws Exception { + // Null out the config model + privilegesEvaluator.onConfigModelChanged(null); + final OpenSearchSecurityException exception = assertThrows( + OpenSearchSecurityException.class, + () -> privilegesEvaluator.evaluate(TEST_USER, null) + ); + assertThat(exception.getMessage(), equalTo("OpenSearch Security is not initialized.")); + verify(configModel, never()).getSecurityRoles(); + } + + @Test + public void testEvaluate_NotInitialized_NoSecurityRoles_ExceptionThrown() throws Exception { + final OpenSearchSecurityException exception = assertThrows( + OpenSearchSecurityException.class, + () -> privilegesEvaluator.evaluate(TEST_USER, null) + ); + assertThat(exception.getMessage(), equalTo("OpenSearch Security is not initialized.")); + verify(configModel).getSecurityRoles(); } @Test public void testMapRoles_ReturnsMappedRoles() { - Set mappedRoles = Collections.singleton("role1"); - when(configModel.mapSecurityRoles(any(), any())).thenReturn(mappedRoles); + final User user = mock(User.class); + final Set mappedRoles = Collections.singleton("role1"); + when(configModel.mapSecurityRoles(user, any(TransportAddress.class))).thenReturn(mappedRoles); - Set result = privilegesEvaluator.mapRoles(any(), any()); + final Set result = privilegesEvaluator.mapRoles(user, null); - assertEquals(mappedRoles, result); - verify(configModel).mapSecurityRoles(any(), any()); + assertThat(result, equalTo(mappedRoles)); + verifyNoInteractions(user); + verify(configModel).mapSecurityRoles(user, null); } @Test @@ -129,8 +153,8 @@ public void testEvaluate_Successful_NewPermission() { PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); - assertTrue(response.allowed); - verify(securityRoles).impliesClusterPermissionPermission(any()); + assertThat(response.allowed, equalTo(true)); + verify(securityRoles).impliesClusterPermissionPermission(action); } @Test @@ -143,8 +167,9 @@ public void testEvaluate_Successful_LegacyPermission() { PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); - assertTrue(response.allowed); - verify(securityRoles).impliesClusterPermissionPermission(any()); + assertThat(response.allowed, equalTo(true)); + verify(securityRoles).impliesClusterPermissionPermission(action); + verify(configModel, times(3)).getSecurityRoles(); } @Test @@ -157,7 +182,7 @@ public void testEvaluate_Unsuccessful() { PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action)); - assertFalse(response.allowed); - verify(securityRoles).impliesClusterPermissionPermission(any()); + assertThat(response.allowed, equalTo(false)); + verify(securityRoles).impliesClusterPermissionPermission(action); } } From e408e40f70caedd6eb504d44bd69126062333a52 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Tue, 22 Aug 2023 14:52:16 -0400 Subject: [PATCH 6/7] Fix mock Signed-off-by: Stephen Crawford --- .../security/privileges/RestLayerPrivilegesEvaluatorTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java index 145bf56d9d..7ed149dac3 100644 --- a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java @@ -31,7 +31,6 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.security.securityconf.ConfigModel; import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.securityconf.SecurityRoles; @@ -134,7 +133,7 @@ public void testEvaluate_NotInitialized_NoSecurityRoles_ExceptionThrown() throws public void testMapRoles_ReturnsMappedRoles() { final User user = mock(User.class); final Set mappedRoles = Collections.singleton("role1"); - when(configModel.mapSecurityRoles(user, any(TransportAddress.class))).thenReturn(mappedRoles); + when(configModel.mapSecurityRoles(any(), any())).thenReturn(mappedRoles); final Set result = privilegesEvaluator.mapRoles(user, null); From c76249008e930e2859f081421c6b70724df214c5 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Wed, 23 Aug 2023 11:53:31 -0400 Subject: [PATCH 7/7] Cleans up RestLayerPrivilegesEvaluatorTest Signed-off-by: Darshit Chanpura --- .../privileges/RestLayerPrivilegesEvaluatorTest.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java index 7ed149dac3..2f6189bab2 100644 --- a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java @@ -32,7 +32,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.security.securityconf.ConfigModel; -import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.securityconf.SecurityRoles; import org.opensearch.security.user.User; import org.opensearch.threadpool.ThreadPool; @@ -58,10 +57,6 @@ public class RestLayerPrivilegesEvaluatorTest { private ThreadPool threadPool; @Mock private ConfigModel configModel; - @Mock - private DynamicConfigModel dcm; - @Mock - private PrivilegesEvaluatorResponse presponse; private RestLayerPrivilegesEvaluator privilegesEvaluator; @@ -73,7 +68,7 @@ private void setLoggingLevel(final Level level) { } @Before - public void setUp() throws InstantiationException, IllegalAccessException { + public void setUp() { when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); when(clusterService.localNode()).thenReturn(mock(DiscoveryNode.class, withSettings().strictness(Strictness.LENIENT))); @@ -108,7 +103,7 @@ public void testEvaluate_Initialized_Success() { } @Test - public void testEvaluate_NotInitialized_NullModel_ExceptionThrown() throws Exception { + public void testEvaluate_NotInitialized_NullModel_ExceptionThrown() { // Null out the config model privilegesEvaluator.onConfigModelChanged(null); final OpenSearchSecurityException exception = assertThrows( @@ -120,7 +115,7 @@ public void testEvaluate_NotInitialized_NullModel_ExceptionThrown() throws Excep } @Test - public void testEvaluate_NotInitialized_NoSecurityRoles_ExceptionThrown() throws Exception { + public void testEvaluate_NotInitialized_NoSecurityRoles_ExceptionThrown() { final OpenSearchSecurityException exception = assertThrows( OpenSearchSecurityException.class, () -> privilegesEvaluator.evaluate(TEST_USER, null)