Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase coverage target #4

Merged
merged 2 commits into from
Aug 22, 2023
Merged

Increase coverage target #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8bb213c
Increase coverage target
peternied Aug 22, 2023
b1ac98b
Merge remote-tracking branch 'steven/boostedCoverage' into boostedCov…
peternied Aug 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ public Collection<Object> createComponents(
principalExtractor = ReflectionHelper.instantiatePrincipalExtractor(principalExtractorClass);
}

restLayerEvaluator = new RestLayerPrivilegesEvaluator(clusterService, threadPool, auditLog, cih, namedXContentRegistry);
restLayerEvaluator = new RestLayerPrivilegesEvaluator(clusterService, threadPool);

securityRestHandler = new SecurityRestFilter(
backendRegistry,
Expand Down
47 changes: 10 additions & 37 deletions src/main/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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> namedXContentRegistry;

public RestLayerPrivilegesEvaluator(
final ClusterService clusterService,
final ThreadPool threadPool,
AuditLog auditLog,
final ClusterInfoHolder clusterInfoHolder,
AtomicReference<NamedXContentRegistry> 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<String> roles) {
SecurityRoles getSecurityRoles(final Set<String> 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<String> actions) {

public PrivilegesEvaluatorResponse evaluate(final User user, final Set<String> actions) {
if (!isInitialized()) {
throw new OpenSearchSecurityException("OpenSearch Security is not initialized.");
}
Expand All @@ -86,7 +60,7 @@ public PrivilegesEvaluatorResponse evaluate(final User user, Set<String> actions

final TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS);

Set<String> mappedRoles = mapRoles(user, caller);
final Set<String> mappedRoles = mapRoles(user, caller);

presponse.resolvedSecurityRoles.addAll(mappedRoles);
final SecurityRoles securityRoles = getSecurityRoles(mappedRoles);
Expand All @@ -98,7 +72,7 @@ public PrivilegesEvaluatorResponse evaluate(final User user, Set<String> 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;
Expand All @@ -123,8 +97,7 @@ public PrivilegesEvaluatorResponse evaluate(final User user, Set<String> actions
return presponse;
}

public Set<String> mapRoles(final User user, final TransportAddress caller) {
Set<String> mapRoles(final User user, final TransportAddress caller) {
return this.configModel.mapSecurityRoles(user, caller);
}

}
131 changes: 72 additions & 59 deletions src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> 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
Expand All @@ -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<String> mappedRoles = Collections.singleton("role1");
when(configModel.mapSecurityRoles(any(), any())).thenReturn(mappedRoles);
final User user = mock(User.class);
final Set<String> mappedRoles = Collections.singleton("role1");
when(configModel.mapSecurityRoles(user, any(TransportAddress.class))).thenReturn(mappedRoles);

Set<String> result = privilegesEvaluator.mapRoles(any(), any());
final Set<String> 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
Expand All @@ -128,9 +152,9 @@ 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());

assertThat(response.allowed, equalTo(true));
verify(securityRoles).impliesClusterPermissionPermission(action);
}

@Test
Expand All @@ -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
Expand All @@ -157,19 +182,7 @@ public void testEvaluate_Unsuccessful() {

PrivilegesEvaluatorResponse response = privilegesEvaluator.evaluate(TEST_USER, Set.of(action));

assertFalse(response.allowed);
verify(securityRoles).impliesClusterPermissionPermission(any());
}

@Test
public void testIsInitialized_False() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rearranged to testEvaluate_NotInitialized_NullModel_ExceptionThrown method

privilegesEvaluator = new RestLayerPrivilegesEvaluator(
clusterService,
threadPool,
mock(AuditLog.class),
mock(ClusterInfoHolder.class),
namedXContentRegistry
);
assertFalse(privilegesEvaluator.isInitialized());
assertThat(response.allowed, equalTo(false));
verify(securityRoles).impliesClusterPermissionPermission(action);
}
}