Commit 7471cc47 authored by Rostislav Vatolin [SLB]'s avatar Rostislav Vatolin [SLB]
Browse files

add protection for bootstrapped groups

update test BootstrapGroupsConfigurationServiceTest

update ServiceAccountsConfigurationService

fix unit tests
parent 52ccca36
......@@ -31,4 +31,9 @@ public abstract class AppProperties {
* @return a path of configuration file
*/
public abstract String getGroupsOfServicePrincipal();
/**
* Returns members which are protected from removal from their groups
*/
public abstract List<String> getProtectedMembers();
}
......@@ -4,7 +4,8 @@ import lombok.RequiredArgsConstructor;
import org.opengroup.osdu.core.common.logging.JaxRsDpsLog;
import org.opengroup.osdu.core.common.model.http.AppException;
import org.opengroup.osdu.core.common.model.http.RequestInfo;
import org.opengroup.osdu.entitlements.v2.di.KeySvcAccBeanConfiguration;
import org.opengroup.osdu.entitlements.v2.validation.BootstrapGroupsConfigurationService;
import org.opengroup.osdu.entitlements.v2.validation.ServiceAccountsConfigurationService;
import org.opengroup.osdu.entitlements.v2.model.EntityNode;
import org.opengroup.osdu.entitlements.v2.model.removemember.RemoveMemberServiceDto;
import org.opengroup.osdu.entitlements.v2.spi.removemember.RemoveMemberRepo;
......@@ -20,7 +21,8 @@ public class RemoveMemberService {
private final RemoveMemberRepo removeMemberRepo;
private final RetrieveGroupRepo retrieveGroupRepo;
private final JaxRsDpsLog log;
private final KeySvcAccBeanConfiguration keySvcAccBeanConfiguration;
private final ServiceAccountsConfigurationService serviceAccountsConfigurationService;
private final BootstrapGroupsConfigurationService bootstrapGroupsConfigurationService;
private final RequestInfo requestInfo;
private final PermissionService permissionService;
......@@ -44,13 +46,20 @@ public class RemoveMemberService {
() -> new AppException(HttpStatus.NOT_FOUND.value(), HttpStatus.NOT_FOUND.getReasonPhrase(), String.format("Group %s does not have %s as a child/member", groupEmail, memberEmail))
));
// check whether it deletes key service account from bootstrap groups
if (keySvcAccBeanConfiguration.isKeySvcAccountInBootstrapGroup(groupEmail, memberEmail)) {
throw new AppException(HttpStatus.BAD_REQUEST.value(), HttpStatus.BAD_REQUEST.getReasonPhrase(), String.format("Key service accounts hierarchy is enforced, %s cannot be removed from group %s", memberEmail, groupEmail));
if (serviceAccountsConfigurationService.isMemberProtectedServiceAccount(memberNode, existingGroupEntityNode)) {
throw new AppException(HttpStatus.BAD_REQUEST.value(), HttpStatus.BAD_REQUEST.getReasonPhrase(),
String.format("Key service accounts hierarchy is enforced, %s cannot be removed from group %s", memberEmail, groupEmail));
}
if (memberNode.isUsersDataRootGroup() && (existingGroupEntityNode.isDataGroup() || existingGroupEntityNode.isUserGroup())) {
throw new AppException(HttpStatus.BAD_REQUEST.value(), HttpStatus.BAD_REQUEST.getReasonPhrase(), "users data root group hierarchy is enforced, cannot be removed");
throw new AppException(HttpStatus.BAD_REQUEST.value(), HttpStatus.BAD_REQUEST.getReasonPhrase(),
"Users data root group hierarchy is enforced, member users.data.root cannot be removed");
}
if (bootstrapGroupsConfigurationService.isMemberProtectedFromRemoval(memberNode, existingGroupEntityNode)) {
throw new AppException(HttpStatus.BAD_REQUEST.value(), HttpStatus.BAD_REQUEST.getReasonPhrase(),
String.format("Bootstrap group hierarchy is enforced, member %s cannot be removed from group %s",
memberNode.getName(), existingGroupEntityNode.getName()));
}
return removeMemberRepo.removeMember(existingGroupEntityNode, memberNode, removeMemberServiceDto);
......
package org.opengroup.osdu.entitlements.v2.validation;
import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonParser;
import lombok.RequiredArgsConstructor;
import org.opengroup.osdu.entitlements.v2.AppProperties;
import org.opengroup.osdu.entitlements.v2.model.EntityNode;
import org.opengroup.osdu.entitlements.v2.util.FileReaderService;
import org.springframework.stereotype.Service;
import javax.annotation.PostConstruct;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
@Service
@RequiredArgsConstructor
public class BootstrapGroupsConfigurationService {
private final FileReaderService fileReaderService;
private final AppProperties appProperties;
private Map<String, Set<String>> membersPerGroup;
@PostConstruct
private void init() {
membersPerGroup = new HashMap<>();
for (String filePath: appProperties.getProtectedMembers()) {
loadConfiguration(filePath);
}
}
public boolean isMemberProtectedFromRemoval(EntityNode memberNode, EntityNode groupNode) {
if (!memberNode.getDataPartitionId().equals(groupNode.getDataPartitionId())) {
return false;
}
return membersPerGroup.getOrDefault(groupNode.getName(), new HashSet<>()).contains(memberNode.getName());
}
private void loadConfiguration(final String fileName) {
String fileContent = fileReaderService.readFile(fileName);
JsonArray groups = getGroupList(fileContent);
for (JsonElement group: groups) {
String groupName = group.getAsJsonObject().get("name").getAsString();
Set<String> setOfMembers = new HashSet<>();
JsonArray arrayOfMembers = group.getAsJsonObject().get("members").getAsJsonArray();
for (JsonElement member: arrayOfMembers) {
setOfMembers.add(member.getAsJsonObject().get("name").getAsString());
}
membersPerGroup.put(groupName, setOfMembers);
}
}
private JsonArray getGroupList(final String fileContent) {
return JsonParser.parseString(fileContent)
.getAsJsonObject()
.get("groups")
.getAsJsonArray();
}
}
package org.opengroup.osdu.entitlements.v2.di;
package org.opengroup.osdu.entitlements.v2.validation;
import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
......@@ -6,6 +6,7 @@ import com.google.gson.JsonParser;
import lombok.RequiredArgsConstructor;
import org.opengroup.osdu.core.common.model.http.RequestInfo;
import org.opengroup.osdu.entitlements.v2.AppProperties;
import org.opengroup.osdu.entitlements.v2.model.EntityNode;
import org.opengroup.osdu.entitlements.v2.util.FileReaderService;
import org.springframework.stereotype.Component;
......@@ -17,7 +18,7 @@ import java.util.Set;
@Component
@RequiredArgsConstructor
public class KeySvcAccBeanConfiguration {
public class ServiceAccountsConfigurationService {
private final FileReaderService fileReaderService;
private final RequestInfo requestInfo;
......@@ -31,10 +32,20 @@ public class KeySvcAccBeanConfiguration {
loadConfiguration(appProperties.getGroupsOfServicePrincipal());
}
public boolean isKeySvcAccountInBootstrapGroup(final String groupEmail, final String memberEmail) {
return isKeyServiceAccount(memberEmail) && getServiceAccountGroups(memberEmail).contains(groupEmail);
/**
* Checks if a member is a service account and if it belongs to its default group
*/
public boolean isMemberProtectedServiceAccount(final EntityNode memberNode, final EntityNode groupNode) {
if (!memberNode.getDataPartitionId().equals(groupNode.getDataPartitionId())) {
return false;
}
return isKeyServiceAccount(memberNode.getNodeId())
&& getServiceAccountGroups(memberNode.getNodeId()).contains(groupNode.getName());
}
/**
* Returns a set of default groups of service account
*/
public Set<String> getServiceAccountGroups(final String email) {
if (isServicePrincipalAccount(email)) {
return this.svcAccGroupConfig.computeIfAbsent("SERVICE_PRINCIPAL", k -> new HashSet<>());
......@@ -42,7 +53,7 @@ public class KeySvcAccBeanConfiguration {
return new HashSet<>();
}
public boolean isKeyServiceAccount(final String email) {
private boolean isKeyServiceAccount(final String email) {
return isServicePrincipalAccount(email);
}
......
......@@ -21,6 +21,11 @@ public class AppPropertiesTestConfiguration {
public String getGroupsOfServicePrincipal() {
return null;
}
@Override
public List<String> getProtectedMembers() {
return null;
}
};
}
}
package org.opengroup.osdu.entitlements.v2.di;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.opengroup.osdu.core.common.logging.JaxRsDpsLog;
import org.opengroup.osdu.core.common.model.http.RequestInfo;
import org.opengroup.osdu.core.common.model.tenant.TenantInfo;
import org.opengroup.osdu.entitlements.v2.AppProperties;
import org.opengroup.osdu.entitlements.v2.util.FileReaderService;
import org.powermock.modules.junit4.PowerMockRunner;
import org.powermock.reflect.Whitebox;
import java.util.Set;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.powermock.api.mockito.PowerMockito.when;
@RunWith(PowerMockRunner.class)
public class KeySvcAccBeanConfigurationTests {
@Mock
private AppProperties appProperties;
@Mock
private FileReaderService fileReaderService;
@Mock
private RequestInfo requestInfo;
@Mock
private JaxRsDpsLog log;
@InjectMocks
private KeySvcAccBeanConfiguration sut;
private TenantInfo tenantInfo;
private final String SERVICE_PRINCIPAL = "{\n" +
" \"users\": [\n" +
" {\n" +
" \"email\": \"SERVICE_PRINCIPAL\",\n" +
" \"role\": \"OWNER\"\n" +
" }\n" +
" ],\n" +
" \"ownersOf\": [\n" +
" {\n" +
" \"groupName\": \"users.data.root\"\n" +
" },\n" +
" {\n" +
" \"groupName\": \"users\"\n" +
" }\n" +
" ]\n" +
"}";
@Before
public void setup() throws Exception {
when(appProperties.getGroupsOfServicePrincipal()).thenReturn("groups_of_service_principal.json");
prepareFileReaderForUsersTesting();
tenantInfo = Mockito.mock(TenantInfo.class);
when(requestInfo.getTenantInfo()).thenReturn(tenantInfo);
Whitebox.invokeMethod(sut, "init");
}
@Test
public void shouldReturnTrue_IfServicePrincipalAccount() {
when(tenantInfo.getServiceAccount()).thenReturn("datafier@xxx.iam.gserviceaccount.com");
boolean res = sut.isKeyServiceAccount("datafier@xxx.iam.gserviceaccount.com");
assertTrue(res);
}
@Test
public void shouldReturnFalse_IfDNonKeySvcAccount() {
when(tenantInfo.getServiceAccount()).thenReturn("datafier@xxx.iam.gserviceaccount.com");
boolean res = sut.isKeyServiceAccount("member@xxx.com");
assertFalse(res);
}
@Test
public void shouldReturnGroups_ifGivenDatafierAcc() {
when(tenantInfo.getServiceAccount()).thenReturn("datafier@xxx.iam.gserviceaccount.com");
Set<String> res = sut.getServiceAccountGroups("datafier@xxx.iam.gserviceaccount.com");
assertEquals(2, res.size());
assertTrue(res.contains("users"));
assertTrue(res.contains("users.data.root"));
}
@Test
public void shouldReturnEmptyGroupSet_ifGivenEmailIsNotKeySvcAcc() {
when(tenantInfo.getServiceAccount()).thenReturn("datafier@xxx.iam.gserviceaccount.com");
Set<String> res = sut.getServiceAccountGroups("member@xxx.com");
assertEquals(0, res.size());
}
@Test
public void shouldReturnTrue_ifKeySvcAccInBootstrapGroup() {
when(tenantInfo.getServiceAccount()).thenReturn("datafier@xxx.iam.gserviceaccount.com");
boolean res = sut.isKeySvcAccountInBootstrapGroup("users.data.root", "datafier@xxx.iam.gserviceaccount.com");
assertTrue(res);
}
@Test
public void shouldReturnFalse_ifGivenNonKeySvcAccInBootstrapGroup() {
when(tenantInfo.getServiceAccount()).thenReturn("datafier@xxx.iam.gserviceaccount.com");
boolean res = sut.isKeySvcAccountInBootstrapGroup("users.data.root", "member@xxx.iam.gserviceaccount.com");
assertFalse(res);
}
@Test
public void shouldReturnFalse_ifGivenKeySvcAccInNonBootstrapGroup() {
when(tenantInfo.getServiceAccount()).thenReturn("datafier@xxx.iam.gserviceaccount.com");
boolean res = sut.isKeySvcAccountInBootstrapGroup("users.test", "service-project-id@appspot.gserviceaccount.com");
assertFalse(res);
}
private void prepareFileReaderForUsersTesting() {
when(fileReaderService.readFile("groups_of_service_principal.json")).thenReturn(SERVICE_PRINCIPAL);
}
}
\ No newline at end of file
......@@ -9,7 +9,8 @@ import org.opengroup.osdu.core.common.model.http.RequestInfo;
import org.opengroup.osdu.core.common.model.tenant.TenantInfo;
import org.opengroup.osdu.entitlements.v2.AppProperties;
import org.opengroup.osdu.entitlements.v2.configuration.AppPropertiesTestConfiguration;
import org.opengroup.osdu.entitlements.v2.di.KeySvcAccBeanConfiguration;
import org.opengroup.osdu.entitlements.v2.validation.BootstrapGroupsConfigurationService;
import org.opengroup.osdu.entitlements.v2.validation.ServiceAccountsConfigurationService;
import org.opengroup.osdu.entitlements.v2.model.ChildrenReference;
import org.opengroup.osdu.entitlements.v2.model.EntityNode;
import org.opengroup.osdu.entitlements.v2.model.NodeType;
......@@ -47,7 +48,9 @@ public class RemoveMemberServiceTests {
@MockBean
private JaxRsDpsLog logger;
@MockBean
private KeySvcAccBeanConfiguration keySvcAccBeanConfiguration;
private ServiceAccountsConfigurationService serviceAccountsConfigurationService;
@MockBean
private BootstrapGroupsConfigurationService bootstrapGroupsConfigurationService;
@MockBean
private RequestInfoUtilService requestInfoUtilService;
@MockBean
......@@ -253,6 +256,63 @@ public class RemoveMemberServiceTests {
}
}
@Test
public void shouldThrow400OnRemovalOfBootstrapGroup() {
EntityNode memberNode = EntityNode.builder()
.type(NodeType.GROUP)
.nodeId("users@common.contoso.com")
.name("users")
.dataPartitionId("common")
.build();
when(retrieveGroupRepo.getEntityNode("users@common.contoso.com", "common")).thenReturn(Optional.of(memberNode));
EntityNode groupNode = EntityNode.builder()
.type(NodeType.GROUP)
.nodeId("data.default.owners@common.contoso.com")
.name("data.default.owners")
.dataPartitionId("common")
.build();
EntityNode requesterNode = EntityNode.builder()
.type(NodeType.USER)
.nodeId("requesterid")
.name("requesterid")
.dataPartitionId("common")
.build();
when(retrieveGroupRepo.groupExistenceValidation("data.default.owners@common.contoso.com", "common")).thenReturn(groupNode);
EntityNode rootDataGroupNode = EntityNode.builder()
.type(NodeType.GROUP)
.nodeId("users.data.root@common.contoso.com")
.name("users.data.root")
.dataPartitionId("common")
.build();
when(retrieveGroupRepo.groupExistenceValidation(
"users.data.root@common.contoso.com", "common")).thenReturn(rootDataGroupNode);
when(retrieveGroupRepo.hasDirectChild(
rootDataGroupNode,
ChildrenReference.createChildrenReference(requesterNode, Role.MEMBER))).thenReturn(Boolean.TRUE);
when(retrieveGroupRepo.hasDirectChild(
groupNode,
ChildrenReference.createChildrenReference(memberNode, Role.MEMBER))).thenReturn(Boolean.TRUE);
when(requestInfoUtilService.getDomain("common")).thenReturn("common.contoso.com");
RemoveMemberServiceDto removeMemberServiceDto = RemoveMemberServiceDto.builder()
.groupEmail("data.default.owners@common.contoso.com")
.memberEmail("users@common.contoso.com")
.requesterId("requesterid")
.partitionId("common")
.build();
when(bootstrapGroupsConfigurationService.isMemberProtectedFromRemoval(memberNode, groupNode)).thenReturn(true);
try {
removeMemberService.removeMember(removeMemberServiceDto);
fail("should throw exception");
} catch (AppException ex) {
verify(removeMemberRepo, never()).removeMember(any(), any(), any());
assertThat(ex.getError().getCode()).isEqualTo(400);
assertEquals("Bootstrap group hierarchy is enforced, member users cannot be removed from group data.default.owners", ex.getError().getMessage());
} catch (Exception ex) {
fail(String.format("should now throw exception: %s", ex.getMessage()));
}
}
@Test
public void shouldThrow401IfCallerDoesNotOwnTheGroup() {
EntityNode memberNode = EntityNode.builder()
......@@ -350,8 +410,7 @@ public class RemoveMemberServiceTests {
.partitionId("common")
.build();
when(keySvcAccBeanConfiguration.isKeySvcAccountInBootstrapGroup(
removeMemberServiceDto.getGroupEmail(), removeMemberServiceDto.getMemberEmail())).thenReturn(true);
when(serviceAccountsConfigurationService.isMemberProtectedServiceAccount(memberNode, groupNode)).thenReturn(true);
try {
removeMemberService.removeMember(removeMemberServiceDto);
fail("should throw exception");
......@@ -411,15 +470,14 @@ public class RemoveMemberServiceTests {
.partitionId("common")
.build();
when(keySvcAccBeanConfiguration.isKeySvcAccountInBootstrapGroup(
removeMemberServiceDto.getGroupEmail(), removeMemberServiceDto.getMemberEmail())).thenReturn(false);
when(serviceAccountsConfigurationService.isMemberProtectedServiceAccount(memberNode, groupNode)).thenReturn(false);
try {
removeMemberService.removeMember(removeMemberServiceDto);
fail("should throw exception");
} catch (AppException ex) {
verify(removeMemberRepo, never()).removeMember(any(), any(), any());
assertThat(ex.getError().getCode()).isEqualTo(400);
assertEquals("users data root group hierarchy is enforced, cannot be removed", ex.getError().getMessage());
assertEquals("Users data root group hierarchy is enforced, member users.data.root cannot be removed", ex.getError().getMessage());
} catch (Exception ex) {
fail(String.format("should now throw exception: %s", ex.getMessage()));
}
......
package org.opengroup.osdu.entitlements.v2.validation;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import org.opengroup.osdu.entitlements.v2.AppProperties;
import org.opengroup.osdu.entitlements.v2.model.EntityNode;
import org.opengroup.osdu.entitlements.v2.model.NodeType;
import org.opengroup.osdu.entitlements.v2.util.FileReaderService;
import org.powermock.reflect.Whitebox;
import java.util.Collections;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@RunWith(MockitoJUnitRunner.class)
public class BootstrapGroupsConfigurationServiceTest {
@Mock
private AppProperties appProperties;
@Spy
private FileReaderService fileReaderService = new FileReaderService();
@InjectMocks
private BootstrapGroupsConfigurationService bootstrapGroupsConfigurationService;
@Before
public void setup() throws Exception {
when(appProperties.getProtectedMembers()).thenReturn(Collections.singletonList("/test_groups.json"));
Whitebox.invokeMethod(bootstrapGroupsConfigurationService, "init");
verify(fileReaderService).readFile("/test_groups.json");
}
@Test
public void shouldReturnTrueIfMemberIsProtectedFromRemoval() {
EntityNode memberNode = EntityNode.builder().nodeId("memberId").name("users").type(NodeType.GROUP).dataPartitionId("common").build();
EntityNode groupNode = EntityNode.builder().nodeId("groupId").name("data.default.viewers").type(NodeType.GROUP).dataPartitionId("common").build();
Assert.assertTrue(bootstrapGroupsConfigurationService.isMemberProtectedFromRemoval(memberNode, groupNode));
}
@Test
public void shouldReturnFalseIfMemberIsFromAnotherPartition() {
EntityNode memberNode = EntityNode.builder().nodeId("memberId").name("users").type(NodeType.GROUP).dataPartitionId("common").build();
EntityNode groupNode = EntityNode.builder().nodeId("groupId").name("data.default.viewers").type(NodeType.GROUP).dataPartitionId("tenant1").build();
Assert.assertFalse(bootstrapGroupsConfigurationService.isMemberProtectedFromRemoval(memberNode, groupNode));
}
@Test
public void shouldReturnFalseIfMemberIsUnprotected() {
EntityNode memberNode = EntityNode.builder().nodeId("memberId").name("memberId").type(NodeType.GROUP).dataPartitionId("common").build();
EntityNode groupNode = EntityNode.builder().nodeId("groupId").name("data.default.viewers").type(NodeType.GROUP).dataPartitionId("tenant1").build();
Assert.assertFalse(bootstrapGroupsConfigurationService.isMemberProtectedFromRemoval(memberNode, groupNode));
}
}
package org.opengroup.osdu.entitlements.v2.validation;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import org.opengroup.osdu.core.common.model.http.RequestInfo;
import org.opengroup.osdu.core.common.model.tenant.TenantInfo;
import org.opengroup.osdu.entitlements.v2.AppProperties;
import org.opengroup.osdu.entitlements.v2.model.EntityNode;
import org.opengroup.osdu.entitlements.v2.util.FileReaderService;
import org.powermock.reflect.Whitebox;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.mock;
@RunWith(MockitoJUnitRunner.class)
public class ServiceAccountsConfigurationServiceTest {
private static final String SERVICE_P_ID = "service_principal@xxx.com";
@Mock
private AppProperties appProperties;
@Spy
private FileReaderService fileReaderService = new FileReaderService();
@Mock
private RequestInfo requestInfo;
@InjectMocks
private ServiceAccountsConfigurationService serviceAccountsConfigurationService;
private TenantInfo tenantInfo;
@Before
public void setup() throws Exception {
when(appProperties.getGroupsOfServicePrincipal()).thenReturn("/test_groups_of_service_principal.json");
tenantInfo = mock(TenantInfo.class);
when(tenantInfo.getServiceAccount()).thenReturn(SERVICE_P_ID);
when(requestInfo.getTenantInfo()).thenReturn(tenantInfo);
Whitebox.invokeMethod(serviceAccountsConfigurationService, "init");
verify(fileReaderService).readFile("/test_groups_of_service_principal.json");
}
@Test
public void shouldReturnGroupsOfServicePrincipal() {
Set<String> res = serviceAccountsConfigurationService.getServiceAccountGroups(SERVICE_P_ID);
assertEquals(new HashSet<>(Arrays.asList("users", "users.data.root")), res);
}