Skip to content

Commit

Permalink
moved security to the service layer of collections
Browse files Browse the repository at this point in the history
  • Loading branch information
@marcos-lg
marcos-lg committed Apr 29, 2021
1 parent 5f0d94a commit 0d8b7ad
Show file tree
Hide file tree
Showing 14 changed files with 81 additions and 87 deletions.
23 changes: 6 additions & 17 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
<spring-cloud-openfeign.version>2.2.6.RELEASE</spring-cloud-openfeign.version>
<spring-cloud-zookeeper.version>2.2.4.RELEASE</spring-cloud-zookeeper.version>
<mybatis-spring-boot-starter.version>2.0.1</mybatis-spring-boot-starter.version>
<spring-cloud-sleuth.version>2.2.8.RELEASE</spring-cloud-sleuth.version>

<!-- GBIF -->
<gbif-api.version>0.144-SNAPSHOT</gbif-api.version>
Expand Down Expand Up @@ -160,23 +161,6 @@

<dependencyManagement>
<dependencies>

<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-starter-zipkin</artifactId>
<version>2.2.8.RELEASE</version>
</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-starter-sleuth</artifactId>
<version>2.2.8.RELEASE</version>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-aop</artifactId>
<version>${spring-boot.version}</version>
</dependency>

<!-- Spring dependencies -->
<dependency>
<groupId>org.springframework.boot</groupId>
Expand Down Expand Up @@ -232,6 +216,11 @@
<artifactId>spring-boot-admin-starter-client</artifactId>
<version>${spring-boot-admin.version}</version>
</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-starter-sleuth</artifactId>
<version>${spring-cloud-sleuth.version}</version>
</dependency>

<!-- GBIF dependencies -->
<dependency>
Expand Down
32 changes: 16 additions & 16 deletions ...ation-tests/src/test/java/org/gbif/registry/ws/it/collections/BaseCollectionEntityIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public abstract class BaseCollectionEntityIT<
T extends CollectionEntity & Identifiable & Taggable & MachineTaggable & Commentable>
extends BaseItTest {

protected final CrudService<T> resource;
protected final CrudService<T> entityService;
protected final CrudService<T> client;

public static final Pageable DEFAULT_PAGE = new PagingRequest(0L, 5);
Expand All @@ -73,15 +73,15 @@ public abstract class BaseCollectionEntityIT<
protected TestCaseDatabaseInitializer databaseRule = new TestCaseDatabaseInitializer();

public BaseCollectionEntityIT(
CrudService<T> resource,
CrudService<T> entityService,
Class<? extends CrudService<T>> cls,
SimplePrincipalProvider principalProvider,
EsManageServer esServer,
IdentityService identityService,
int localServerPort,
KeyStore keyStore) {
super(principalProvider, esServer);
this.resource = resource;
this.entityService = entityService;
this.client = prepareClient(localServerPort, keyStore, cls);
collectionsDatabaseInitializer = new CollectionsDatabaseInitializer(identityService);
}
Expand All @@ -99,7 +99,7 @@ public BaseCollectionEntityIT(
@ParameterizedTest
@EnumSource(ServiceType.class)
public void crudTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);

// create
T entity = newEntity();
Expand Down Expand Up @@ -131,21 +131,21 @@ public void crudTest(ServiceType serviceType) {
@ParameterizedTest
@EnumSource(ServiceType.class)
public void createInvalidEntityTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);
assertThrows(ValidationException.class, () -> service.create(newInvalidEntity()));
}

@ParameterizedTest
@EnumSource(ServiceType.class)
public void deleteMissingEntityTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);
assertThrows(IllegalArgumentException.class, () -> service.delete(UUID.randomUUID()));
}

@ParameterizedTest
@EnumSource(ServiceType.class)
public void updateDeletedEntityTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);

T entity = newEntity();
UUID key = service.create(entity);
Expand All @@ -160,7 +160,7 @@ public void updateDeletedEntityTest(ServiceType serviceType) {
@ParameterizedTest
@EnumSource(ServiceType.class)
public void restoreDeletedEntityTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);

T entity = newEntity();
UUID key = service.create(entity);
Expand All @@ -179,7 +179,7 @@ public void restoreDeletedEntityTest(ServiceType serviceType) {
@ParameterizedTest
@EnumSource(ServiceType.class)
public void updateInvalidEntityTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);

T entity = newEntity();
UUID key = service.create(entity);
Expand All @@ -205,7 +205,7 @@ public void updateEntityKeyMismatchTest() {
@ParameterizedTest
@EnumSource(ServiceType.class)
public void getMissingEntity(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);

try {
T entity = service.get(UUID.randomUUID());
Expand All @@ -218,7 +218,7 @@ public void getMissingEntity(ServiceType serviceType) {
@ParameterizedTest
@EnumSource(ServiceType.class)
public void createFullEntityTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);

T entity = newEntity();

Expand Down Expand Up @@ -249,7 +249,7 @@ public void createFullEntityTest(ServiceType serviceType) {
@ParameterizedTest
@EnumSource(ServiceType.class)
public void tagsTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);
TagService tagService = (TagService) service;

UUID key = service.create(newEntity());
Expand All @@ -270,7 +270,7 @@ public void tagsTest(ServiceType serviceType) {
@ParameterizedTest
@EnumSource(ServiceType.class)
public void machineTagsTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);
MachineTagService machineTagService = (MachineTagService) service;

T entity = newEntity();
Expand All @@ -291,7 +291,7 @@ public void machineTagsTest(ServiceType serviceType) {
@ParameterizedTest
@EnumSource(ServiceType.class)
public void identifiersTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);
IdentifierService identifierService = (IdentifierService) service;

T entity = newEntity();
Expand All @@ -316,7 +316,7 @@ public void identifiersTest(ServiceType serviceType) {
@ParameterizedTest
@EnumSource(ServiceType.class)
public void commentsTest(ServiceType serviceType) {
CrudService<T> service = getService(serviceType, resource, client);
CrudService<T> service = getService(serviceType, entityService, client);
CommentService commentService = (CommentService) service;

T entity = newEntity();
Expand All @@ -341,7 +341,7 @@ protected CrudService<T> getService(ServiceType param) {
case CLIENT:
return client;
case RESOURCE:
return resource;
return entityService;
default:
throw new IllegalStateException("Must be resource or client");
}
Expand Down
20 changes: 20 additions & 0 deletions ...vice/src/main/java/org/gbif/registry/service/collections/BaseCollectionEntityService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.security.access.annotation.Secured;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.transaction.annotation.Transactional;
Expand All @@ -42,6 +43,8 @@
import org.springframework.web.bind.annotation.PathVariable;

import static com.google.common.base.Preconditions.checkArgument;
import static org.gbif.registry.security.UserRoles.GRSCICOLL_ADMIN_ROLE;
import static org.gbif.registry.security.UserRoles.GRSCICOLL_EDITOR_ROLE;

// TODO: Move @Secured ??
@Validated
Expand Down Expand Up @@ -84,6 +87,7 @@ public T get(UUID key) {
return baseMapper.get(key);
}

@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Transactional
@Override
public void delete(UUID key) {
Expand All @@ -100,6 +104,7 @@ public void delete(UUID key) {
eventManager.post(DeleteCollectionEntityEvent.newInstance(get(key), objectClass));
}

@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Transactional
@Validated({PrePersist.class, Default.class})
@Override
Expand All @@ -114,6 +119,7 @@ public int addIdentifier(UUID entityKey, Identifier identifier) {
return identifierKey;
}

@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Transactional
@Override
public void deleteIdentifier(UUID entityKey, int identifierKey) {
Expand All @@ -128,6 +134,7 @@ public List<Identifier> listIdentifiers(UUID key) {
return baseMapper.listIdentifiers(key);
}

@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Validated({PrePersist.class, Default.class})
@Override
public int addTag(UUID entityKey, Tag tag) {
Expand All @@ -139,13 +146,15 @@ public int addTag(UUID entityKey, Tag tag) {
return tagKey;
}

@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Override
public int addTag(UUID key, String value) {
Tag tag = new Tag();
tag.setValue(value);
return addTag(key, tag);
}

@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Override
public void deleteTag(UUID entityKey, int tagKey) {
baseMapper.deleteTag(entityKey, tagKey);
Expand All @@ -169,6 +178,7 @@ public List<Tag> listTags(UUID key, String owner) {
* @param machineTag MachineTag to add
* @return key of MachineTag created
*/
@Secured(GRSCICOLL_ADMIN_ROLE)
@Transactional
@Override
public int addMachineTag(UUID targetEntityKey, MachineTag machineTag) {
Expand All @@ -182,6 +192,7 @@ public int addMachineTag(UUID targetEntityKey, MachineTag machineTag) {
return key;
}

@Secured(GRSCICOLL_ADMIN_ROLE)
@Transactional
@Validated({PrePersist.class, Default.class})
@Override
Expand All @@ -193,6 +204,7 @@ public int addMachineTag(UUID targetEntityKey, String namespace, String name, St
return addMachineTag(targetEntityKey, machineTag);
}

@Secured(GRSCICOLL_ADMIN_ROLE)
@Transactional
@Override
public int addMachineTag(UUID targetEntityKey, TagName tagName, String value) {
Expand All @@ -206,16 +218,19 @@ public int addMachineTag(UUID targetEntityKey, TagName tagName, String value) {
* @param targetEntityKey key of target entity to delete MachineTag from
* @param machineTagKey key of MachineTag to delete
*/
@Secured(GRSCICOLL_ADMIN_ROLE)
@Override
public void deleteMachineTag(UUID targetEntityKey, int machineTagKey) {
baseMapper.deleteMachineTag(targetEntityKey, machineTagKey);
}

@Secured(GRSCICOLL_ADMIN_ROLE)
@Override
public void deleteMachineTags(UUID targetEntityKey, TagNamespace tagNamespace) {
deleteMachineTags(targetEntityKey, tagNamespace.getNamespace());
}

@Secured(GRSCICOLL_ADMIN_ROLE)
@Override
public void deleteMachineTags(UUID targetEntityKey, String namespace) {
baseMapper.deleteMachineTags(targetEntityKey, namespace, null);
Expand All @@ -224,6 +239,7 @@ public void deleteMachineTags(UUID targetEntityKey, String namespace) {
targetEntityKey, objectClass, MachineTag.class));
}

@Secured(GRSCICOLL_ADMIN_ROLE)
@Override
public void deleteMachineTags(UUID targetEntityKey, TagName tagName) {
deleteMachineTags(targetEntityKey, tagName.getNamespace().getNamespace(), tagName.getName());
Expand All @@ -236,6 +252,7 @@ public void deleteMachineTags(UUID targetEntityKey, TagName tagName) {
* The webservice method to delete all machine tag of a particular name in a namespace. Ensures
* that the caller is authorized to perform the action by looking at the namespace.
*/
@Secured(GRSCICOLL_ADMIN_ROLE)
@Override
public void deleteMachineTags(UUID targetEntityKey, String namespace, String name) {
baseMapper.deleteMachineTags(targetEntityKey, namespace, name);
Expand All @@ -257,6 +274,7 @@ public List<MachineTag> listMachineTags(UUID targetEntityKey) {
* @param comment Comment to add
* @return key of Comment created
*/
@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Transactional
@Validated({PrePersist.class, Default.class})
@Override
Expand All @@ -278,6 +296,7 @@ public int addComment(UUID targetEntityKey, Comment comment) {
* @param targetEntityKey key of target entity to delete comment from
* @param commentKey key of Comment to delete
*/
@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Override
public void deleteComment(UUID targetEntityKey, int commentKey) {
baseMapper.deleteComment(targetEntityKey, commentKey);
Expand All @@ -304,6 +323,7 @@ protected void preUpdate(T entity) {
entity.setModifiedBy(authentication.getName());
}

@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Transactional
@Override
public abstract void update(T entity);
Expand Down
5 changes: 5 additions & 0 deletions ...try-service/src/main/java/org/gbif/registry/service/collections/DefaultPersonService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import javax.validation.groups.Default;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.access.annotation.Secured;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.validation.annotation.Validated;
Expand All @@ -36,6 +37,8 @@
import com.google.common.base.Strings;

import static com.google.common.base.Preconditions.checkArgument;
import static org.gbif.registry.security.UserRoles.GRSCICOLL_ADMIN_ROLE;
import static org.gbif.registry.security.UserRoles.GRSCICOLL_EDITOR_ROLE;

@Validated
@Service
Expand Down Expand Up @@ -68,6 +71,7 @@ protected DefaultPersonService(
this.personMapper = personMapper;
}

@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Transactional
@Validated({PrePersist.class, Default.class})
@Override
Expand Down Expand Up @@ -110,6 +114,7 @@ public UUID create(Person person) {
return person.getKey();
}

@Secured({GRSCICOLL_ADMIN_ROLE, GRSCICOLL_EDITOR_ROLE})
@Transactional
@Override
public void update(Person person) {
Expand Down
Loading

0 comments on commit 0d8b7ad

Please sign in to comment.