From 314d448c996694062027a60fadc4f66e62523856 Mon Sep 17 00:00:00 2001 From: Miroslav Blasko Date: Thu, 19 Sep 2024 17:38:13 +0200 Subject: [PATCH 01/17] [DOES NOT WORK] Migrate to role groups --- .../java/cz/cvut/kbss/study/model/Role.java | 118 ++++++++++++++++++ .../cz/cvut/kbss/study/model/RoleGroup.java | 57 +++++++++ .../java/cz/cvut/kbss/study/model/User.java | 19 ++- .../study/persistence/dao/RoleGroupDao.java | 44 +++++++ .../kbss/study/persistence/dao/UserDao.java | 8 +- .../study/security/SecurityConstants.java | 4 + .../cvut/kbss/study/security/model/Role.java | 4 +- .../kbss/study/service/RoleGroupService.java | 12 ++ .../kbss/study/service/SystemInitializer.java | 35 +++++- .../repository/RepositoryRoleGroup.java | 28 +++++ .../repository/RepositoryUserService.java | 2 +- .../study/service/security/SecurityUtils.java | 9 +- src/main/resources/model.ttl | 93 +++++++++++++- .../environment/generator/Generator.java | 23 +++- .../cz/cvut/kbss/study/model/RoleTest.java | 71 +++++++++++ .../cz/cvut/kbss/study/model/UserTest.java | 4 +- .../study/persistence/dao/UserDaoTest.java | 7 +- .../study/rest/StatisticsControllerTest.java | 1 + .../security/CustomSwitchUserFilterTest.java | 7 +- .../study/service/BaseServiceTestRunner.java | 5 +- .../RepositoryPatientRecordServiceTest.java | 5 +- .../service/security/SecurityUtilsTest.java | 9 +- 22 files changed, 525 insertions(+), 40 deletions(-) create mode 100644 src/main/java/cz/cvut/kbss/study/model/Role.java create mode 100644 src/main/java/cz/cvut/kbss/study/model/RoleGroup.java create mode 100644 src/main/java/cz/cvut/kbss/study/persistence/dao/RoleGroupDao.java create mode 100644 src/main/java/cz/cvut/kbss/study/service/RoleGroupService.java create mode 100644 src/main/java/cz/cvut/kbss/study/service/repository/RepositoryRoleGroup.java create mode 100644 src/test/java/cz/cvut/kbss/study/model/RoleTest.java diff --git a/src/main/java/cz/cvut/kbss/study/model/Role.java b/src/main/java/cz/cvut/kbss/study/model/Role.java new file mode 100644 index 00000000..83f8d19b --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/model/Role.java @@ -0,0 +1,118 @@ +package cz.cvut.kbss.study.model; + +import cz.cvut.kbss.jopa.model.annotations.Individual; +import java.util.Optional; +import org.apache.poi.ss.formula.atp.Switch; + +public enum Role { + + // TODO deprecated -- should be removed. + @Individual(iri=Vocabulary.s_i_administrator) + administrator(Vocabulary.s_i_administrator), + // TODO deprecated -- should be removed. + @Individual(iri = Vocabulary.s_i_user) + user(Vocabulary.s_i_user), + + @Individual(iri = Vocabulary.s_i_impersonate_role) + impersonate(Vocabulary.s_i_impersonate_role), + + @Individual(iri = Vocabulary.s_i_delete_all_records_role) + deleteAllRecords(Vocabulary.s_i_delete_all_records_role), + + @Individual(iri = Vocabulary.s_i_view_all_records_role) + viewAllRecords(Vocabulary.s_i_view_all_records_role), + + @Individual(iri = Vocabulary.s_i_edit_all_records_role) + editAllRecords(Vocabulary.s_i_edit_all_records_role), + + @Individual(iri = Vocabulary.s_i_delete_organization_records_role) + deleteOrganizationRecords(Vocabulary.s_i_delete_organization_records_role), + + @Individual(iri = Vocabulary.s_i_view_organization_records_role) + viewOrganizationRecords(Vocabulary.s_i_view_organization_records_role), + + @Individual(iri = Vocabulary.s_i_edit_organization_records_role) + editOrganizationRecords(Vocabulary.s_i_edit_organization_records_role), + + @Individual(iri = Vocabulary.s_i_edit_users_role) + editUsers(Vocabulary.s_i_edit_users_role), + + @Individual(iri = Vocabulary.s_i_complete_records_role) + completeRecords(Vocabulary.s_i_complete_records_role), + + @Individual(iri = Vocabulary.s_i_reject_records_role) + rejectRecords(Vocabulary.s_i_reject_records_role), + + @Individual(iri = Vocabulary.s_i_publish_records_role) + publishRecords(Vocabulary.s_i_publish_records_role), + + @Individual(iri = Vocabulary.s_i_import_codelists_role) + importCodelists(Vocabulary.s_i_import_codelists_role); + + private final String iri; + + Role(String iri) { + this.iri = iri; + } + + public static Role getFromSecurityRole(cz.cvut.kbss.study.security.model.Role r) { + return switch (r) { + case USER -> user; + case ADMIN -> administrator; + }; + } + + public String getIri() { + return iri; + } + + /** + * Returns {@link Role} with the specified IRI. + * + * @param iri role identifier + * @return matching {@code Role} + * @throws IllegalArgumentException When no matching role is found + */ + public static Role fromIri(String iri) { + for (Role r : values()) { + if (r.getIri().equals(iri)) { + return r; + } + } + throw new IllegalArgumentException("Unknown role identifier '" + iri + "'."); + } + + /** + * Returns {@link Role} with the specified constant name. + * + * @param name role name + * @return matching {@code Role} + * @throws IllegalArgumentException When no matching role is found + */ + public static Role fromName(String name) { + for (Role r : values()) { + if (r.name().equalsIgnoreCase(name)) { + return r; + } + } + throw new IllegalArgumentException("Unknown role '" + name + "'."); + } + + /** + * Returns a {@link Role} with the specified IRI or constant name. + *

+ * This function first tries to find the enum constant by IRI. If it is not found, constant name matching is + * attempted. + * + * @param identification Constant IRI or name to find match by + * @return matching {@code Role} + * @throws IllegalArgumentException When no matching role is found + */ + public static Role fromIriOrName(String identification) { + try { + return fromIri(identification); + } catch (IllegalArgumentException e) { + return fromName(identification); + } + } +} diff --git a/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java b/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java new file mode 100644 index 00000000..8a7684b3 --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java @@ -0,0 +1,57 @@ +package cz.cvut.kbss.study.model; +import cz.cvut.kbss.jopa.model.annotations.*; +import cz.cvut.kbss.study.model.util.HasOwlKey; +import cz.cvut.kbss.study.model.util.HasUri; +import cz.cvut.kbss.study.util.Constants; + +import java.io.Serializable; +import java.net.URI; +import java.util.HashSet; +import java.util.Set; + +@OWLClass(iri = Vocabulary.s_c_role_group) +public class RoleGroup implements Serializable, HasUri { + + @Id + private URI uri; + + @OWLAnnotationProperty(iri = Vocabulary.s_p_label) + private String name; + + @Enumerated(EnumType.OBJECT_ONE_OF) + @OWLObjectProperty(iri = Vocabulary.s_p_has_role, cascade = {CascadeType.MERGE,CascadeType.PERSIST}) + private Set roles = new HashSet<>(); + + public void addRole(Role role){ + roles.add(role); + } + + + public URI getUri() { + return uri; + } + + public void setUri(URI uri) { + this.uri = uri; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Set getRoles() { + return roles; + } + + public void setRoles(Set roles) { + this.roles = roles; + } + + public void generateUri() { + this.uri = URI.create(Constants.BASE_URI + name); + } +} diff --git a/src/main/java/cz/cvut/kbss/study/model/User.java b/src/main/java/cz/cvut/kbss/study/model/User.java index 0685c80c..3084efde 100644 --- a/src/main/java/cz/cvut/kbss/study/model/User.java +++ b/src/main/java/cz/cvut/kbss/study/model/User.java @@ -1,6 +1,7 @@ package cz.cvut.kbss.study.model; import com.fasterxml.jackson.annotation.JsonProperty; +import cz.cvut.kbss.jopa.model.annotations.CascadeType; import cz.cvut.kbss.jopa.model.annotations.FetchType; import cz.cvut.kbss.jopa.model.annotations.Id; import cz.cvut.kbss.jopa.model.annotations.OWLClass; @@ -61,12 +62,15 @@ public class User implements HasDerivableUri, Serializable { @OWLObjectProperty(iri = Vocabulary.s_p_is_member_of, fetch = FetchType.EAGER) private Institution institution; + @OWLObjectProperty(iri = Vocabulary.s_p_has_role_group, cascade = {CascadeType.MERGE, CascadeType.PERSIST}) + private RoleGroup roleGroup; + @Types private Set types; public User() { this.types = new HashSet<>(); - types.add(Vocabulary.s_c_doctor); + types.add(Vocabulary.s_c_doctor_role_group); } @Override @@ -137,6 +141,14 @@ public void setInstitution(Institution institution) { this.institution = institution; } + public RoleGroup getRoleGroup() { + return roleGroup; + } + + public void setRoleGroup(RoleGroup roleGroup) { + this.roleGroup = roleGroup; + } + public Set getTypes() { return types; } @@ -158,8 +170,7 @@ public void addType(String type) { * @return {@code true} if this is admin, {@code false} otherwise */ public boolean isAdmin() { - assert types != null; - return getTypes().contains(Vocabulary.s_c_administrator); + return roleGroup.getRoles().contains(Role.administrator); } public String getToken() { @@ -216,7 +227,7 @@ public User copy() { copy.setInstitution(institution); copy.setIsInvited(isInvited); copy.setToken(token); - types.forEach(copy::addType); + copy.setRoleGroup(roleGroup); return copy; } diff --git a/src/main/java/cz/cvut/kbss/study/persistence/dao/RoleGroupDao.java b/src/main/java/cz/cvut/kbss/study/persistence/dao/RoleGroupDao.java new file mode 100644 index 00000000..895cb139 --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/persistence/dao/RoleGroupDao.java @@ -0,0 +1,44 @@ +package cz.cvut.kbss.study.persistence.dao; + +import cz.cvut.kbss.jopa.exceptions.NoResultException; +import cz.cvut.kbss.jopa.model.EntityManager; +import cz.cvut.kbss.study.exception.PersistenceException; +import cz.cvut.kbss.study.model.RoleGroup; +import cz.cvut.kbss.study.util.Constants; +import java.util.Objects; +import org.springframework.stereotype.Repository; +import java.net.URI; +import cz.cvut.kbss.study.model.Vocabulary; + +@Repository +public class RoleGroupDao extends BaseDao { + + protected RoleGroupDao(EntityManager em) { + super(RoleGroup.class, em); + } + + public RoleGroup findByName(String name) { + if (name == null) { + return null; + } + try { + return em.createNativeQuery("SELECT ?x WHERE { ?x ?hasName ?name . }", RoleGroup.class) + .setParameter("hasName", URI.create(Vocabulary.s_p_label)) + .setParameter("name", name, Constants.PU_LANGUAGE).getSingleResult(); + } catch (NoResultException e) { + return null; + } + } + + @Override + public void persist(RoleGroup entity) { + Objects.requireNonNull(entity); + try { + em.persist(entity); + } catch (RuntimeException e) { + LOG.error("Error when persisting entity.", e); + throw new PersistenceException(e); + } + } + +} diff --git a/src/main/java/cz/cvut/kbss/study/persistence/dao/UserDao.java b/src/main/java/cz/cvut/kbss/study/persistence/dao/UserDao.java index 9e461d9f..ea2a51ce 100644 --- a/src/main/java/cz/cvut/kbss/study/persistence/dao/UserDao.java +++ b/src/main/java/cz/cvut/kbss/study/persistence/dao/UserDao.java @@ -83,9 +83,11 @@ public List findByInstitution(Institution institution) { public int getNumberOfInvestigators() { return ((BigInteger) em.createNativeQuery( - "SELECT (count(?p) as ?investigatorCount) WHERE { ?p a ?typeDoctor . MINUS {?p a ?typeAdmin}}") - .setParameter("typeDoctor", URI.create(Vocabulary.s_c_doctor)) - .setParameter("typeAdmin", URI.create(Vocabulary.s_c_administrator)).getSingleResult() + "SELECT (count(?p) as ?investigatorCount) WHERE { ?p a ?typeUser . MINUS {?p ?hasRoleGroup ?roleGroup . ?roleGroup ?hasRole ?typeAdmin}}") + .setParameter("typeUser", URI.create(Vocabulary.s_c_Person)) + .setParameter("hasRoleGroup", URI.create(Vocabulary.s_p_has_role_group)) + .setParameter("hasRole", URI.create(Vocabulary.s_p_has_role)) + .setParameter("typeAdmin", URI.create(Vocabulary.s_i_administrator)).getSingleResult() ).intValue(); } } diff --git a/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java b/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java index a60c8ba3..516c7798 100644 --- a/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java +++ b/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java @@ -30,4 +30,8 @@ private SecurityConstants() { public static final String ROLE_USER = "ROLE_USER"; public static final String ROLE_ADMIN = "ROLE_ADMIN"; + + public static final String ROLE_GROUP_USER = "ROLE_GROUP_USER";; + + public static final Object ROLE_GROUP_ADMIN = "ROLE_GROUP_ADMIN"; } diff --git a/src/main/java/cz/cvut/kbss/study/security/model/Role.java b/src/main/java/cz/cvut/kbss/study/security/model/Role.java index 4b794953..64665215 100644 --- a/src/main/java/cz/cvut/kbss/study/security/model/Role.java +++ b/src/main/java/cz/cvut/kbss/study/security/model/Role.java @@ -7,8 +7,8 @@ import java.util.stream.Stream; public enum Role { - USER(SecurityConstants.ROLE_USER, Vocabulary.s_c_doctor), - ADMIN(SecurityConstants.ROLE_ADMIN, Vocabulary.s_c_administrator); + USER(SecurityConstants.ROLE_USER, Vocabulary.s_i_user), + ADMIN(SecurityConstants.ROLE_ADMIN, Vocabulary.s_i_administrator); private final String name; private final String type; diff --git a/src/main/java/cz/cvut/kbss/study/service/RoleGroupService.java b/src/main/java/cz/cvut/kbss/study/service/RoleGroupService.java new file mode 100644 index 00000000..b18f421f --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/service/RoleGroupService.java @@ -0,0 +1,12 @@ +package cz.cvut.kbss.study.service; + +import cz.cvut.kbss.study.model.RoleGroup; +import cz.cvut.kbss.study.service.repository.BaseRepositoryService; +import org.springframework.stereotype.Service; + +public interface RoleGroupService extends BaseService { + + RoleGroup findByName(String name); + + +} diff --git a/src/main/java/cz/cvut/kbss/study/service/SystemInitializer.java b/src/main/java/cz/cvut/kbss/study/service/SystemInitializer.java index dc6795ff..1bb6e4ae 100644 --- a/src/main/java/cz/cvut/kbss/study/service/SystemInitializer.java +++ b/src/main/java/cz/cvut/kbss/study/service/SystemInitializer.java @@ -1,8 +1,11 @@ package cz.cvut.kbss.study.service; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.Role; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.model.Vocabulary; +import cz.cvut.kbss.study.util.Constants; import jakarta.annotation.PostConstruct; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -17,21 +20,35 @@ public class SystemInitializer { private static final String ADMIN_USERNAME = "admin"; private static final String INSTITUTION_NAME = "admin_institution"; + private static final String ADMIN_ROLE_GROUP_NAME = "admin-role-group"; private final UserService userService; private final InstitutionService institutionService; + private final RoleGroupService roleGroupService; + public SystemInitializer(UserService userService, - InstitutionService institutionService) { + InstitutionService institutionService, + RoleGroupService roleGroupService) { this.userService = userService; this.institutionService = institutionService; + this.roleGroupService = roleGroupService; } @PostConstruct private void initializeSystem() { - addDefaultInstitution(); - addDefaultAdministrator(); + if (noAdminExists()) { + addAdminRoleGroup(); + addDefaultInstitution(); + addDefaultAdministrator(); + } + } + + private boolean noAdminExists() { + return userService.findAll().stream().filter( + user -> user.getRoleGroup().getRoles().contains(Role.administrator) + ).findAny().isEmpty(); } private void addDefaultInstitution() { @@ -52,9 +69,19 @@ private void addDefaultAdministrator() { admin.setPassword("5y5t3mAdm1n."); admin.setInstitution(institutionService.findByName(INSTITUTION_NAME)); admin.setIsInvited(true); - admin.getTypes().add(Vocabulary.s_c_administrator); + admin.setRoleGroup(roleGroupService.findByName(ADMIN_ROLE_GROUP_NAME)); LOG.debug("Persisting default administrator {}", admin); userService.persist(admin); } } + + private void addAdminRoleGroup() { + if (roleGroupService.findByName(ADMIN_ROLE_GROUP_NAME) == null) { + final RoleGroup roleGroup = new RoleGroup(); + roleGroup.setName(ADMIN_ROLE_GROUP_NAME); + roleGroup.addRole(Role.administrator); + roleGroup.generateUri(); + roleGroupService.persist(roleGroup); + } + } } diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryRoleGroup.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryRoleGroup.java new file mode 100644 index 00000000..0e8875cd --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryRoleGroup.java @@ -0,0 +1,28 @@ +package cz.cvut.kbss.study.service.repository; + +import cz.cvut.kbss.study.model.ActionHistory; +import cz.cvut.kbss.study.model.RoleGroup; +import cz.cvut.kbss.study.persistence.dao.GenericDao; +import cz.cvut.kbss.study.persistence.dao.RoleGroupDao; +import cz.cvut.kbss.study.service.RoleGroupService; +import org.springframework.stereotype.Service; + +@Service +public class RepositoryRoleGroup extends BaseRepositoryService implements RoleGroupService { + + private final RoleGroupDao roleGroupDao; + + public RepositoryRoleGroup(RoleGroupDao roleGroupDao) { + this.roleGroupDao = roleGroupDao; + } + + @Override + public RoleGroup findByName(String name) { + return roleGroupDao.findByName(name); + } + + @Override + protected GenericDao getPrimaryDao() { + return roleGroupDao; + } +} diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java index 1ed305a0..d402df8b 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java @@ -202,7 +202,7 @@ protected void prePersist(User instance) { @Override protected void preUpdate(User instance) { final User currentUser = securityUtils.getCurrentUser(); - if (!currentUser.getTypes().contains(Vocabulary.s_c_administrator) + if (!currentUser.isAdmin() && (!instance.getTypes().equals(currentUser.getTypes()) || (instance.getInstitution() != null && !instance.getInstitution().getKey().equals(currentUser.getInstitution().getKey())))) { throw new UnauthorizedException("Cannot update user."); diff --git a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java index 343b7762..6233af86 100644 --- a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java +++ b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java @@ -2,6 +2,7 @@ import cz.cvut.kbss.study.exception.NotFoundException; import cz.cvut.kbss.study.model.PatientRecord; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.model.Vocabulary; import cz.cvut.kbss.study.persistence.dao.PatientRecordDao; @@ -88,7 +89,7 @@ public User getCurrentUser() { final User user = userDao.findByUsername(username).copy(); if (context.getAuthentication().getAuthorities().stream().anyMatch(a -> a.getAuthority().equals( SwitchUserWebFilter.ROLE_PREVIOUS_ADMINISTRATOR))) { - user.addType(Vocabulary.s_c_impersonator); + user.getRoleGroup().addRole(cz.cvut.kbss.study.model.Role.impersonate); } return user; } @@ -102,7 +103,11 @@ private User resolveAccountFromOAuthPrincipal(Jwt principal) { throw new NotFoundException( "User with username '" + userInfo.getPreferredUsername() + "' not found in repository."); } - roles.stream().map(Role::forName).filter(Optional::isPresent).forEach(r -> user.addType(r.get().getType())); + + RoleGroup roleGroup = new RoleGroup(); + user.setRoleGroup(roleGroup); + roles.stream().map(Role::forName).filter(Optional::isPresent).map(Optional::get) + .forEach(r -> roleGroup.addRole(cz.cvut.kbss.study.model.Role.getFromSecurityRole(r))); return user; } diff --git a/src/main/resources/model.ttl b/src/main/resources/model.ttl index 379c2e74..1915480e 100644 --- a/src/main/resources/model.ttl +++ b/src/main/resources/model.ttl @@ -66,6 +66,10 @@ rm:has-question rdf:type owl:ObjectProperty ; rm:is-member-of rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rm:relates-to . +### http://onto.fel.cvut.cz/ontologies/record-manager/role-group +rm:role-group rdf:type owl:ObjectProperty ; + rdfs:subPropertyOf rm:relates-to . + ### http://onto.fel.cvut.cz/ontologies/record-manager/relates-to rm:relates-to rdf:type owl:ObjectProperty . @@ -80,6 +84,15 @@ rm:has-phase rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rdf:type ; rdfs:label "has phase"@en . +### http://onto.fel.cvut.cz/ontologies/record-manager/has-role-group +rm:has-role-group rdf:type owl:ObjectProperty ; + rdfs:subPropertyOf rm:relates-to; + rdfs:label "has role group"@en. + +### http://onto.fel.cvut.cz/ontologies/record-manager/has-role +rm:has-role rdf:type owl:ObjectProperty ; + rdfs:subPropertyOf rm:relates-to; + rdfs:label "has role"@en. ################################################################# # Data properties @@ -144,13 +157,13 @@ rm:action-history rdf:type owl:Class ; rdfs:label "ActionHistory"@en . -### http://onto.fel.cvut.cz/ontologies/record-manager/administrator -rm:administrator rdf:type owl:Class ; +### http://onto.fel.cvut.cz/ontologies/record-manager/administrator-role-group +rm:administrator-role-group rdf:type owl:Class ; rdfs:label "Administrator"@en . -### http://onto.fel.cvut.cz/ontologies/record-manager/doctor -rm:doctor rdf:type owl:Class ; +### http://onto.fel.cvut.cz/ontologies/record-manager/doctor-role-group +rm:doctor-role-group rdf:type owl:Class ; rdfs:label "Doctor"@en . @@ -202,4 +215,74 @@ rm:rejected-record-phase rdf:type owl:Class ; rdfs:subClassOf rm:record-phase ; rdfs:label "rejected record phase"@en . -### Generated by the OWL API (version 4.2.8.20170104-2310) https://github.com/owlcs/owlapi +### http://onto.fel.cvut.cz/ontologies/record-manager/role +rm:role rdf:type owl:Class; + rdfs:label "user role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/role-group +rm:role-group rdf:type owl:Class; + rdfs:label "user role group" . + +################################################################# +# Roles +################################################################# + +### http://onto.fel.cvut.cz/ontologies/record-manager/administrator +### TODO deprecated +rm:administrator rdf:type owl:NamedIndividual, rm:role ; + rdfs:label "administrator"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/user +### TODO deprecated +rm:user rdf:type owl:NamedIndividual, rm:role ; + rdfs:label "user"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/complete-records-role +rm:complete-records-role rdf:type owl:NamedIndividual, rm:role ; + rdfs:label "complete records role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/delete-all-records-role +rm:delete-all-records-role rdf:type owl:NamedIndividual, rm:role ; + rdfs:label "delete all records role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/edit-all-records-role +rm:edit-all-records-role rdf:type owl:NamedIndividual, rm:role ; + rdfs:label "edit all records role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/view-all-records-role +rm:view-all-records-role rdf:type owl:NamedIndividual, rm:role ; + rdfs:label "view all records role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/delete-organization-records-role +rm:delete-organization-records-role rdf:type owl:NamedIndividual, rm:role ; + rdfs:label "delete organization records role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/edit-organization-records-role +rm:edit-organization-records-role rdf:type owl:NamedIndividual, rm:role; + rdfs:label "edit organization records role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/view-organization-records-role +rm:view-organization-records-role rdf:type owl:NamedIndividual, rm:role; + rdfs:label "view organization records role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/edit-users-role +rm:edit-users-role rdf:type owl:NamedIndividual, rm:role; + rdfs:label "edit users role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/import-codelists-role +rm:import-codelists-role rdf:type owl:NamedIndividual, rm:role; + rdfs:label "import codelists role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/publish-records-role +rm:publish-records-role rdf:type owl:NamedIndividual, rm:role; + rdfs:label "publish records role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/reject-records-role +rm:reject-records-role rdf:type owl:NamedIndividual, rm:role; + rdfs:label "reject records role"@en . + +### http://onto.fel.cvut.cz/ontologies/record-manager/impersonate-role +rm:impersonate-role rdf:type owl:NamedIndividual, rm:role; + rdfs:label "impersonate role"@en . + +### Generated by the OWL API (version 4.2.8.20170104-2310) https://github.com/owlcs/owlapi \ No newline at end of file diff --git a/src/test/java/cz/cvut/kbss/study/environment/generator/Generator.java b/src/test/java/cz/cvut/kbss/study/environment/generator/Generator.java index 93f8a21c..771bf65e 100644 --- a/src/test/java/cz/cvut/kbss/study/environment/generator/Generator.java +++ b/src/test/java/cz/cvut/kbss/study/environment/generator/Generator.java @@ -119,7 +119,7 @@ public static boolean randomBoolean() { * @param institution * @return user based on params */ - public static User getUser(String username, String password, String firstName, String lastName, String email, Institution institution) { + public static User getUser(String username, String password, String firstName, String lastName, String email, Institution institution, RoleGroup roleGroup) { final User person = new User(); person.setUsername(username); person.setPassword(password); @@ -127,15 +127,16 @@ public static User getUser(String username, String password, String firstName, S person.setLastName(lastName); person.setEmailAddress(email); person.setInstitution(institution); + person.setRoleGroup(roleGroup); return person; } /** - * Generators a (pseudo) random institution. + * Generators a (pseudo) random user. * * @return Random user */ - public static User generateUser(Institution institution){ + public static User generateUser(Institution institution) { final User person = new User(); person.setUsername("RandomUsername" + randomInt()); person.setPassword("RandomPassword" + randomInt()); @@ -143,9 +144,25 @@ public static User generateUser(Institution institution){ person.setLastName("RandomLastName" + randomInt()); person.setEmailAddress("RandomEmail" + randomInt() + "@random.rand"); person.setInstitution(institution); + person.setRoleGroup(generateRoleGroupWithOneRole(Role.administrator)); return person; } + public static RoleGroup generateRoleGroup() { + final RoleGroup roleGroup = new RoleGroup(); + roleGroup.setName("RandomRoleGroup" + randomInt()); + roleGroup.generateUri(); + return roleGroup; + } + + + public static RoleGroup generateRoleGroupWithOneRole(Role role) { + final RoleGroup roleGroup = new RoleGroup(); + roleGroup.setName("RandomRoleGroup" + randomInt()); + roleGroup.addRole(role); + return roleGroup; + } + /** * Generators a (pseudo) random institution. * diff --git a/src/test/java/cz/cvut/kbss/study/model/RoleTest.java b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java new file mode 100644 index 00000000..8f7209dc --- /dev/null +++ b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java @@ -0,0 +1,71 @@ +package cz.cvut.kbss.study.model; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + +class RoleTest { + + @Test + void fromIriReturnsCorrectRole() { + assertEquals(Role.administrator, Role.fromIri(Vocabulary.s_i_administrator)); + assertEquals(Role.viewAllRecords, Role.fromIri(Vocabulary.s_i_view_all_records_role)); + } + + @Test + void fromIriThrowsExceptionForUnknownIri() { + String unknownIri = "unknown_iri"; + Exception exception = assertThrows(IllegalArgumentException.class, () -> { + Role.fromIri(unknownIri); + }); + assertEquals("Unknown role identifier '" + unknownIri + "'.", exception.getMessage()); + } + + + @Test + void fromNameReturnsCorrectRole() { + assertEquals(Role.administrator, Role.fromName("administrator")); + assertEquals(Role.viewAllRecords, Role.fromName("viewAllRecords")); + } + + @Test + void fromNameIsCaseInsensitive() { + assertEquals(Role.administrator, Role.fromName("ADMINISTRATOR")); + assertEquals(Role.viewAllRecords, Role.fromName("VIEWALLRECORDS")); + } + + @Test + void fromNameThrowsExceptionForUnknownName() { + String unknownName = "unknown_role"; + Exception exception = assertThrows(IllegalArgumentException.class, () -> { + Role.fromName(unknownName); + }); + assertEquals("Unknown role '" + unknownName + "'.", exception.getMessage()); + } + + + @Test + void fromIriOrNameReturnsRoleByIri() { + assertEquals(Role.administrator, Role.fromIriOrName(Vocabulary.s_i_administrator)); + assertEquals(Role.viewAllRecords, Role.fromIriOrName(Vocabulary.s_i_view_all_records_role)); + } + + @Test + void fromIriOrNameReturnsRoleByName() { + assertEquals(Role.administrator, Role.fromIriOrName("administrator")); + assertEquals(Role.viewAllRecords, Role.fromIriOrName("viewAllRecords")); + } + + @Test + void fromIriOrNameIsCaseInsensitiveForName() { + assertEquals(Role.administrator, Role.fromIriOrName("ADMINISTRATOR")); + } + + @Test + void fromIriOrNameThrowsExceptionForUnknownIdentifier() { + String unknownIdentifier = "unknown_identifier"; + Exception exception = assertThrows(IllegalArgumentException.class, () -> { + Role.fromIriOrName(unknownIdentifier); + }); + assertEquals("Unknown role '" + unknownIdentifier + "'.", exception.getMessage()); + } +} diff --git a/src/test/java/cz/cvut/kbss/study/model/UserTest.java b/src/test/java/cz/cvut/kbss/study/model/UserTest.java index 3b384e27..627a49b9 100644 --- a/src/test/java/cz/cvut/kbss/study/model/UserTest.java +++ b/src/test/java/cz/cvut/kbss/study/model/UserTest.java @@ -23,7 +23,7 @@ public void setUp() { @Test public void newInstanceHasAgentInTypes() { - assertTrue(user.getTypes().contains(Vocabulary.s_c_doctor)); + assertTrue(user.getRoleGroup().getRoles().contains(Role.user)); } @Test @@ -105,6 +105,6 @@ public void generateUriEncodesUsersWithComplexName() { @Test public void newUserHasRoleDoctor() { User user = new User(); - assertTrue(user.getTypes().toString().contains(Vocabulary.s_c_doctor)); + assertTrue(user.getRoleGroup().getRoles().contains(Role.user)); } } \ No newline at end of file diff --git a/src/test/java/cz/cvut/kbss/study/persistence/dao/UserDaoTest.java b/src/test/java/cz/cvut/kbss/study/persistence/dao/UserDaoTest.java index 5395b647..866af0b2 100644 --- a/src/test/java/cz/cvut/kbss/study/persistence/dao/UserDaoTest.java +++ b/src/test/java/cz/cvut/kbss/study/persistence/dao/UserDaoTest.java @@ -2,6 +2,7 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.Role; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.model.Vocabulary; import cz.cvut.kbss.study.persistence.BaseDaoTestRunner; @@ -121,11 +122,7 @@ public void getNumberOfInvestigators() { User user2 = Generator.generateUser(institution); User user3 = Generator.generateUser(institution); - Set types = new HashSet<>(); - types.add(Vocabulary.s_c_administrator); - types.add(Vocabulary.s_c_doctor); - - user3.setTypes(types); + user3.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); transactional(() -> { institutionDao.persist(institution); diff --git a/src/test/java/cz/cvut/kbss/study/rest/StatisticsControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/StatisticsControllerTest.java index 554d5e26..b12645d0 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/StatisticsControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/StatisticsControllerTest.java @@ -3,6 +3,7 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.service.StatisticsService; import org.junit.jupiter.api.BeforeEach; diff --git a/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java b/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java index f7e2b4b4..dbba35bb 100644 --- a/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java +++ b/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java @@ -2,6 +2,7 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; +import cz.cvut.kbss.study.model.Role; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.model.Vocabulary; import cz.cvut.kbss.study.rest.exception.BadRequestException; @@ -36,7 +37,7 @@ void setUp() { @Test void attemptSwitchUserSwitchesCurrentUserToTarget() { final User source = Generator.generateUser(null); - source.addType(Vocabulary.s_c_administrator); + source.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); Environment.setCurrentUser(source); final User target = Generator.generateUser(null); when(userDetailsService.loadUserByUsername(target.getUsername())).thenReturn(new UserDetails(target)); @@ -49,10 +50,10 @@ void attemptSwitchUserSwitchesCurrentUserToTarget() { @Test void attemptSwitchUserThrowsBadRequestExceptionWhenTargetUserIsAdmin() { final User source = Generator.generateUser(null); - source.addType(Vocabulary.s_c_administrator); + source.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); Environment.setCurrentUser(source); final User target = Generator.generateUser(null); - target.addType(Vocabulary.s_c_administrator); + target.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); when(userDetailsService.loadUserByUsername(target.getUsername())).thenReturn(new UserDetails(target)); final MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("username", target.getUsername()); diff --git a/src/test/java/cz/cvut/kbss/study/service/BaseServiceTestRunner.java b/src/test/java/cz/cvut/kbss/study/service/BaseServiceTestRunner.java index 9b655a7b..d28c6130 100644 --- a/src/test/java/cz/cvut/kbss/study/service/BaseServiceTestRunner.java +++ b/src/test/java/cz/cvut/kbss/study/service/BaseServiceTestRunner.java @@ -5,6 +5,8 @@ import cz.cvut.kbss.study.environment.config.TestServiceConfig; import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.Role; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.ConfigDataApplicationContextInitializer; import cz.cvut.kbss.study.persistence.dao.InstitutionDao; @@ -43,7 +45,8 @@ public abstract class BaseServiceTestRunner extends TransactionalTestRunner { @BeforeEach public void setUp() throws Exception { Institution institution = Generator.generateInstitution(); - user = Generator.getUser(USERNAME, PASSWORD, "John", "Grant", EMAIL, institution); + RoleGroup roleGroup = Generator.generateRoleGroupWithOneRole(Role.administrator); //TODO + user = Generator.getUser(USERNAME, PASSWORD, "John", "Grant", EMAIL, institution, roleGroup); transactional(() -> { institutionDao.persist(institution); if (userDao.findByUsername(user.getUsername()) == null) { diff --git a/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java b/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java index a337ff52..7c29ce7a 100644 --- a/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java +++ b/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java @@ -6,6 +6,7 @@ import cz.cvut.kbss.study.exception.RecordAuthorNotFoundException; import cz.cvut.kbss.study.model.PatientRecord; import cz.cvut.kbss.study.model.RecordPhase; +import cz.cvut.kbss.study.model.Role; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.model.Vocabulary; import cz.cvut.kbss.study.persistence.dao.PatientRecordDao; @@ -97,7 +98,7 @@ private List generateRecordsToImport(User originalAuthor) { void importRecordsRetainsRecordProvenanceDataWhenCurrentUserIsAdmin() { final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); final List toImport = generateRecordsToImport(originalAuthor); - user.addType(Vocabulary.s_c_administrator); + user.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.user)); Environment.setCurrentUser(user); when(userService.exists(originalAuthor.getUri())).thenReturn(true); when(recordDao.exists(any())).thenReturn(false); @@ -120,7 +121,7 @@ void importRecordsRetainsRecordProvenanceDataWhenCurrentUserIsAdmin() { void importRecordsThrowsRecordAuthorNotFoundExceptionWhenAdminImportsRecordsAndRecordAuthorIsNotFound() { final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); final List toImport = generateRecordsToImport(originalAuthor); - user.addType(Vocabulary.s_c_administrator); + user.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); Environment.setCurrentUser(user); assertThrows(RecordAuthorNotFoundException.class, () -> sut.importRecords(toImport)); diff --git a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java index 8ee0bfc4..6cb2e39d 100644 --- a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java +++ b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java @@ -4,6 +4,8 @@ import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.model.Institution; import cz.cvut.kbss.study.model.PatientRecord; +import cz.cvut.kbss.study.model.Role; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.model.Vocabulary; import cz.cvut.kbss.study.persistence.dao.PatientRecordDao; @@ -65,7 +67,8 @@ public class SecurityUtilsTest { public void setUp() { Institution institution = Generator.generateInstitution(); institution.setKey(IdentificationUtils.generateKey()); - this.user = Generator.getUser(USERNAME, PASSWORD, "John", "Johnie", "Johnie@gmail.com", institution); + RoleGroup roleGroup = new RoleGroup(); + this.user = Generator.getUser(USERNAME, PASSWORD, "John", "Johnie", "Johnie@gmail.com", institution, roleGroup); user.generateUri(); } @@ -186,7 +189,7 @@ void getCurrentUserEnhancesRetrievedUserWithTypesCorrespondingToRolesSpecifiedIn when(userDao.findByUsername(user.getUsername())).thenReturn(user); final User result = sut.getCurrentUser(); - assertThat(result.getTypes(), hasItem(Vocabulary.s_c_administrator)); + assertThat(result.getRoleGroup().getRoles(), hasItem(Role.administrator)); } @Test @@ -197,7 +200,7 @@ void getCurrentUserEnhancesRetrievedUserWithImpersonatorTypeWhenItHasSwitchAutho when(userDao.findByUsername(user.getUsername())).thenReturn(user); final User result = sut.getCurrentUser(); assertEquals(user, result); - assertThat(result.getTypes(), hasItem(Vocabulary.s_c_impersonator)); + assertThat(result.getRoleGroup().getRoles(), hasItem(Role.impersonate)); } @Test From 703e73409f7df90c55bdda9c461bccd8408c961f Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Fri, 20 Sep 2024 12:52:52 +0200 Subject: [PATCH 02/17] [kbss-cvut/record-manager-ui#202] Fix tests --- .../cz/cvut/kbss/study/model/RoleGroup.java | 4 +- .../java/cz/cvut/kbss/study/model/User.java | 2 +- .../RepositoryPatientRecordService.java | 2 +- .../environment/generator/Generator.java | 34 +++++++++++----- .../cz/cvut/kbss/study/model/UserTest.java | 4 +- .../persistence/dao/ActionHistoryDaoTest.java | 39 ++++++++++++------- .../persistence/dao/DerivableUriDaoTest.java | 9 ++++- .../persistence/dao/PatientRecordDaoTest.java | 36 ++++++++++------- .../study/persistence/dao/UserDaoTest.java | 37 ++++++++++++------ .../rest/ActionHistoryControllerTest.java | 9 +++-- .../study/rest/InstitutionControllerTest.java | 6 ++- .../rest/PatientRecordControllerTest.java | 19 ++++----- .../study/rest/StatisticsControllerTest.java | 4 +- .../kbss/study/rest/UserControllerTest.java | 25 +++++++----- .../handler/HateoasPagingListenerTest.java | 2 +- .../security/CustomSwitchUserFilterTest.java | 17 ++++---- .../study/service/BaseServiceTestRunner.java | 11 +++++- .../study/service/UserDetailsServiceTest.java | 9 ++++- .../RepositoryPatientRecordServiceTest.java | 29 +++++++------- .../service/security/SecurityUtilsTest.java | 12 +++--- 20 files changed, 199 insertions(+), 111 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java b/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java index 8a7684b3..eb7e5511 100644 --- a/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java +++ b/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java @@ -19,7 +19,7 @@ public class RoleGroup implements Serializable, HasUri { private String name; @Enumerated(EnumType.OBJECT_ONE_OF) - @OWLObjectProperty(iri = Vocabulary.s_p_has_role, cascade = {CascadeType.MERGE,CascadeType.PERSIST}) + @OWLObjectProperty(iri = Vocabulary.s_p_has_role) private Set roles = new HashSet<>(); public void addRole(Role role){ @@ -52,6 +52,6 @@ public void setRoles(Set roles) { } public void generateUri() { - this.uri = URI.create(Constants.BASE_URI + name); + this.uri = URI.create(Constants.BASE_URI + "sdfsf"); } } diff --git a/src/main/java/cz/cvut/kbss/study/model/User.java b/src/main/java/cz/cvut/kbss/study/model/User.java index 3084efde..99b0ad67 100644 --- a/src/main/java/cz/cvut/kbss/study/model/User.java +++ b/src/main/java/cz/cvut/kbss/study/model/User.java @@ -62,7 +62,7 @@ public class User implements HasDerivableUri, Serializable { @OWLObjectProperty(iri = Vocabulary.s_p_is_member_of, fetch = FetchType.EAGER) private Institution institution; - @OWLObjectProperty(iri = Vocabulary.s_p_has_role_group, cascade = {CascadeType.MERGE, CascadeType.PERSIST}) + @OWLObjectProperty(iri = Vocabulary.s_p_has_role_group) private RoleGroup roleGroup; @Types diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java index 69fa886a..d42056e6 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java @@ -117,7 +117,7 @@ private void setImportedRecordProvenance(User currentUser, Date now, Optional roleGroupDao.persist(this.roleGroupAdmin)); + } + @Test public void findByKeyReturnsActionWithPayload() { Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + + User user = Generator.generateUser(institution, this.roleGroupAdmin); ActionHistory action = Generator.generateActionHistory(user); transactional(() -> { @@ -53,8 +64,8 @@ public void findByKeyReturnsActionWithPayload() { @Test public void findAllWithParamsWithoutParamsReturnsAllActions() { Institution institution = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institution); + User user1 = Generator.generateUser(institution, this.roleGroupAdmin); + User user2 = Generator.generateUser(institution, this.roleGroupAdmin); ActionHistory action1 = Generator.generateActionHistory(user1); ActionHistory action2 = Generator.generateActionHistory(user1); ActionHistory action3 = Generator.generateActionHistory(user2); @@ -73,9 +84,9 @@ public void findAllWithParamsWithoutParamsReturnsAllActions() { @Test public void findAllWithParamsWithAuthorReturnsAuthorsActions() { Institution institution = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institution); - User user3 = Generator.generateUser(institution); + User user1 = Generator.generateUser(institution, this.roleGroupAdmin); + User user2 = Generator.generateUser(institution, this.roleGroupAdmin); + User user3 = Generator.generateUser(institution, this.roleGroupAdmin); ActionHistory action1 = Generator.generateActionHistory(user1); ActionHistory action2 = Generator.generateActionHistory(user1); ActionHistory action3 = Generator.generateActionHistory(user2); @@ -98,7 +109,7 @@ public void findAllWithParamsWithAuthorReturnsAuthorsActions() { @Test public void findAllWithParamsWithTypeReturnsActionsWithExactType() { Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + User user = Generator.generateUser(institution, this.roleGroupAdmin); ActionHistory action1 = Generator.generateActionHistory(user); action1.setType(LOAD_SUCCESS); ActionHistory action2 = Generator.generateActionHistory(user); @@ -124,7 +135,7 @@ public void findAllWithParamsWithTypeReturnsActionsWithExactType() { @Test public void findAllWithParamsWithTypeReturnsActionsWithTypeContained() { Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + User user = Generator.generateUser(institution, this.roleGroupAdmin); ActionHistory action1 = Generator.generateActionHistory(user); action1.setType(LOAD_SUCCESS); ActionHistory action2 = Generator.generateActionHistory(user); @@ -145,8 +156,8 @@ public void findAllWithParamsWithTypeReturnsActionsWithTypeContained() { @Test public void findAllWithParamsReturnsMatchingActions() { Institution institution = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institution); + User user1 = Generator.generateUser(institution, this.roleGroupAdmin); + User user2 = Generator.generateUser(institution, this.roleGroupAdmin); ActionHistory action1 = Generator.generateActionHistory(user1); action1.setType(LOAD_SUCCESS); ActionHistory action2 = Generator.generateActionHistory(user1); @@ -174,7 +185,7 @@ public void findAllWithParamsReturnsMatchingActions() { @Test void findAllReturnsActionsOnMatchingPage() { Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + User user = Generator.generateUser(institution, this.roleGroupAdmin); final List allActions = IntStream.range(0, 10).mapToObj(i -> Generator.generateActionHistory(user)).toList(); transactional(() -> { institutionDao.persist(institution); diff --git a/src/test/java/cz/cvut/kbss/study/persistence/dao/DerivableUriDaoTest.java b/src/test/java/cz/cvut/kbss/study/persistence/dao/DerivableUriDaoTest.java index e191e2de..767eef67 100644 --- a/src/test/java/cz/cvut/kbss/study/persistence/dao/DerivableUriDaoTest.java +++ b/src/test/java/cz/cvut/kbss/study/persistence/dao/DerivableUriDaoTest.java @@ -2,6 +2,8 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.Role; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.BaseDaoTestRunner; import org.junit.jupiter.api.Test; @@ -17,12 +19,17 @@ public class DerivableUriDaoTest extends BaseDaoTestRunner { @Autowired private InstitutionDao institutionDao; + @Autowired + RoleGroupDao roleGroupDao; + @Test public void persistedInstanceHasGeneratedUri(){ final Institution institution = Generator.generateInstitution(); - final User user = Generator.generateUser(institution); + final RoleGroup roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + final User user = Generator.generateUser(institution, roleGroupAdmin); transactional(() -> { + roleGroupDao.persist(roleGroupAdmin); institutionDao.persist(institution); userDao.persist(user); }); diff --git a/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java b/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java index c6ec6d77..450b9f72 100644 --- a/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java +++ b/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java @@ -7,16 +7,14 @@ import cz.cvut.kbss.study.dto.PatientRecordDto; import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; -import cz.cvut.kbss.study.model.Institution; -import cz.cvut.kbss.study.model.PatientRecord; -import cz.cvut.kbss.study.model.RecordPhase; -import cz.cvut.kbss.study.model.User; +import cz.cvut.kbss.study.model.*; import cz.cvut.kbss.study.model.qam.Answer; import cz.cvut.kbss.study.persistence.BaseDaoTestRunner; import cz.cvut.kbss.study.persistence.dao.util.QuestionSaver; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; import cz.cvut.kbss.study.persistence.dao.util.RecordSort; import cz.cvut.kbss.study.util.IdentificationUtils; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; @@ -57,12 +55,24 @@ public class PatientRecordDaoTest extends BaseDaoTestRunner { @Autowired private InstitutionDao institutionDao; + @Autowired + private RoleGroupDao roleGroupDao; + + RoleGroup roleGroupAdmin; + + @BeforeEach + public void setUp() { + this.roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + transactional(() -> roleGroupDao.persist(roleGroupAdmin)); + int a =4; + } + @Test public void findByInstitutionReturnsMatchingRecords() { Institution institution = Generator.generateInstitution(); Institution institutionOther = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institutionOther); + User user1 = Generator.generateUser(institution, this.roleGroupAdmin); + User user2 = Generator.generateUser(institutionOther, this.roleGroupAdmin); PatientRecord record1 = Generator.generatePatientRecord(user1); PatientRecord record2 = Generator.generatePatientRecord(user1); PatientRecord recordOther = Generator.generatePatientRecord(user2); @@ -88,8 +98,8 @@ public void findByInstitutionReturnsMatchingRecords() { public void findAllRecordsReturnAllRecords() { Institution institution1 = Generator.generateInstitution(); Institution institution2 = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution1); - User user2 = Generator.generateUser(institution2); + User user1 = Generator.generateUser(institution1, this.roleGroupAdmin); + User user2 = Generator.generateUser(institution2, this.roleGroupAdmin); PatientRecord record1 = Generator.generatePatientRecord(user1); PatientRecord record2 = Generator.generatePatientRecord(user1); PatientRecord record3 = Generator.generatePatientRecord(user2); @@ -112,7 +122,7 @@ public void findAllRecordsReturnAllRecords() { @Test public void getNumberOfProcessedRecords() { Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + User user = Generator.generateUser(institution, this.roleGroupAdmin); PatientRecord record1 = Generator.generatePatientRecord(user); PatientRecord record2 = Generator.generatePatientRecord(user); transactional(() -> { @@ -130,8 +140,8 @@ public void getNumberOfProcessedRecords() { @Test public void findByAuthorReturnsMatchingRecords() { Institution institution = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institution); + User user1 = Generator.generateUser(institution, this.roleGroupAdmin); + User user2 = Generator.generateUser(institution, this.roleGroupAdmin); PatientRecord record1 = Generator.generatePatientRecord(user1); PatientRecord record2 = Generator.generatePatientRecord(user1); PatientRecord record3 = Generator.generatePatientRecord(user2); @@ -156,7 +166,7 @@ public void findByAuthorReturnsMatchingRecords() { void persistGeneratesIdentifierBeforeSavingRecord() { final Institution institution = Generator.generateInstitution(); institution.setKey(IdentificationUtils.generateKey()); - final User author = Generator.generateUser(institution); + final User author = Generator.generateUser(institution, this.roleGroupAdmin); author.generateUri(); transactional(() -> { em.persist(author); @@ -175,7 +185,7 @@ void persistGeneratesIdentifierBeforeSavingRecord() { private User generateAuthorWithInstitution() { final Institution institution = Generator.generateInstitution(); institution.setKey(IdentificationUtils.generateKey()); - final User author = Generator.generateUser(institution); + final User author = Generator.generateUser(institution, this.roleGroupAdmin); author.generateUri(); transactional(() -> { em.persist(author); diff --git a/src/test/java/cz/cvut/kbss/study/persistence/dao/UserDaoTest.java b/src/test/java/cz/cvut/kbss/study/persistence/dao/UserDaoTest.java index 866af0b2..24486790 100644 --- a/src/test/java/cz/cvut/kbss/study/persistence/dao/UserDaoTest.java +++ b/src/test/java/cz/cvut/kbss/study/persistence/dao/UserDaoTest.java @@ -1,11 +1,9 @@ package cz.cvut.kbss.study.persistence.dao; import cz.cvut.kbss.study.environment.generator.Generator; -import cz.cvut.kbss.study.model.Institution; -import cz.cvut.kbss.study.model.Role; -import cz.cvut.kbss.study.model.User; -import cz.cvut.kbss.study.model.Vocabulary; +import cz.cvut.kbss.study.model.*; import cz.cvut.kbss.study.persistence.BaseDaoTestRunner; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -25,6 +23,22 @@ public class UserDaoTest extends BaseDaoTestRunner { @Autowired private InstitutionDao institutionDao; + @Autowired + private RoleGroupDao roleGroupDao; + + private RoleGroup roleGroupAdmin; + private RoleGroup roleGroupUser; + + @BeforeEach + public void setUp() { + this.roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + this.roleGroupUser = Generator.generateRoleGroupWithRoles(Role.user); + transactional(() -> { + roleGroupDao.persist(this.roleGroupAdmin); + roleGroupDao.persist(this.roleGroupUser); + }); + } + @Test public void getUserByUsername() { User user1 = getPersistedUser(); @@ -74,7 +88,7 @@ public void getUserByEmailWithNonExistingEmailReturnsNull() { @Test public void getUsersByToken() { Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + User user = Generator.generateUser(institution, this.roleGroupAdmin); user.setToken("Token"); transactional(() -> { @@ -93,8 +107,8 @@ public void getUsersByToken() { public void getUsersByInstitution() { Institution institution1 = Generator.generateInstitution(); Institution institution2 = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution1); - User user2 = Generator.generateUser(institution1); + User user1 = Generator.generateUser(institution1, this.roleGroupAdmin); + User user2 = Generator.generateUser(institution1, this.roleGroupAdmin); transactional(() -> { institutionDao.persist(institution1); @@ -118,11 +132,10 @@ public void getUsersByInstitution() { @Test public void getNumberOfInvestigators() { Institution institution = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institution); - User user3 = Generator.generateUser(institution); + User user1 = Generator.generateUser(institution, this.roleGroupAdmin); + User user2 = Generator.generateUser(institution, this.roleGroupUser); + User user3 = Generator.generateUser(institution, this.roleGroupUser); - user3.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); transactional(() -> { institutionDao.persist(institution); @@ -137,7 +150,7 @@ public void getNumberOfInvestigators() { private User getPersistedUser() { Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + User user = Generator.generateUser(institution, this.roleGroupAdmin); transactional(() -> { institutionDao.persist(institution); userDao.persist(user); diff --git a/src/test/java/cz/cvut/kbss/study/rest/ActionHistoryControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/ActionHistoryControllerTest.java index 52e3d31a..f335f016 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/ActionHistoryControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/ActionHistoryControllerTest.java @@ -4,9 +4,7 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.exception.NotFoundException; -import cz.cvut.kbss.study.model.ActionHistory; -import cz.cvut.kbss.study.model.Institution; -import cz.cvut.kbss.study.model.User; +import cz.cvut.kbss.study.model.*; import cz.cvut.kbss.study.rest.event.PaginatedResultRetrievedEvent; import cz.cvut.kbss.study.service.ActionHistoryService; import cz.cvut.kbss.study.service.UserService; @@ -57,11 +55,14 @@ public class ActionHistoryControllerTest extends BaseControllerTestRunner { private User user; + private RoleGroup roleGroupAdmin; + @BeforeEach public void setUp() { super.setUp(controller); + this.roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); Institution institution = Generator.generateInstitution(); - this.user = Generator.generateUser(institution); + this.user = Generator.generateUser(institution, roleGroupAdmin); Environment.setCurrentUser(user); } diff --git a/src/test/java/cz/cvut/kbss/study/rest/InstitutionControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/InstitutionControllerTest.java index 03c5b2be..f9db7fbc 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/InstitutionControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/InstitutionControllerTest.java @@ -5,6 +5,8 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.Role; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; import cz.cvut.kbss.study.service.InstitutionService; @@ -46,11 +48,13 @@ public class InstitutionControllerTest extends BaseControllerTestRunner { @InjectMocks private InstitutionController controller; + @BeforeEach public void setUp() { super.setUp(controller); Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + RoleGroup roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + User user = Generator.generateUser(institution, roleGroupAdmin); Environment.setCurrentUser(user); } diff --git a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java index 0917640d..2a0f5622 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/PatientRecordControllerTest.java @@ -7,10 +7,7 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.exception.RecordAuthorNotFoundException; -import cz.cvut.kbss.study.model.Institution; -import cz.cvut.kbss.study.model.PatientRecord; -import cz.cvut.kbss.study.model.RecordPhase; -import cz.cvut.kbss.study.model.User; +import cz.cvut.kbss.study.model.*; import cz.cvut.kbss.study.persistence.dao.util.RecordFilterParams; import cz.cvut.kbss.study.persistence.dao.util.RecordSort; import cz.cvut.kbss.study.rest.event.PaginatedResultRetrievedEvent; @@ -81,13 +78,17 @@ public class PatientRecordControllerTest extends BaseControllerTestRunner { private User user; + private RoleGroup roleGroupAdmin; + @BeforeEach public void setUp() { super.setUp(controller); Institution institution = Generator.generateInstitution(); institution.setKey(IdentificationUtils.generateKey()); - this.user = Generator.generateUser(institution); + this.roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + this.user = Generator.generateUser(institution, roleGroupAdmin); Environment.setCurrentUser(user); + } @Test @@ -146,8 +147,8 @@ public void getRecordsReturnsEmptyListWhenNoReportsAreFound() throws Exception { public void getRecordsReturnsAllRecords() throws Exception { Institution institution = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institution); + User user1 = Generator.generateUser(institution, roleGroupAdmin); + User user2 = Generator.generateUser(institution, roleGroupAdmin); List records = List.of(Generator.generatePatientRecordDto(user1), Generator.generatePatientRecordDto(user1), @@ -174,8 +175,8 @@ public void findByInstitutionReturnsRecords() throws Exception { Institution institution = Generator.generateInstitution(); institution.setKey(key); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institution); + User user1 = Generator.generateUser(institution, roleGroupAdmin); + User user2 = Generator.generateUser(institution, roleGroupAdmin); PatientRecordDto record1 = Generator.generatePatientRecordDto(user1); PatientRecordDto record2 = Generator.generatePatientRecordDto(user2); diff --git a/src/test/java/cz/cvut/kbss/study/rest/StatisticsControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/StatisticsControllerTest.java index b12645d0..7bbda82f 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/StatisticsControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/StatisticsControllerTest.java @@ -3,6 +3,7 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.Role; import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.service.StatisticsService; @@ -32,7 +33,8 @@ public class StatisticsControllerTest extends BaseControllerTestRunner { public void setUp() { super.setUp(controller); Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + RoleGroup roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + User user = Generator.generateUser(institution, roleGroupAdmin); Environment.setCurrentUser(user); } diff --git a/src/test/java/cz/cvut/kbss/study/rest/UserControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/UserControllerTest.java index ba0159fe..8646cf98 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/UserControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/UserControllerTest.java @@ -5,6 +5,8 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.Role; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.service.InstitutionService; import cz.cvut.kbss.study.service.UserService; @@ -45,11 +47,14 @@ public class UserControllerTest extends BaseControllerTestRunner { @InjectMocks private UserController controller; + private RoleGroup roleGroupAdmin; + @BeforeEach public void setUp() { super.setUp(controller); Institution institution = Generator.generateInstitution(); - User user = Generator.generateUser(institution); + this.roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + User user = Generator.generateUser(institution, this.roleGroupAdmin); user.setUsername("tom"); Environment.setCurrentUser(user); } @@ -100,9 +105,9 @@ public void getUsersReturnsEmptyListWhenNoUsersAreFound() throws Exception { public void getUsersReturnsAllUsers() throws Exception { Institution institution = Generator.generateInstitution(); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institution); - User user3 = Generator.generateUser(institution); + User user1 = Generator.generateUser(institution, this.roleGroupAdmin); + User user2 = Generator.generateUser(institution, this.roleGroupAdmin); + User user3 = Generator.generateUser(institution, this.roleGroupAdmin); List users = new ArrayList<>(); users.add(user1); @@ -127,9 +132,9 @@ public void findAllUsersByInstitutionReturnsInstitutionUsers() throws Exception Institution institution = Generator.generateInstitution(); institution.setKey(key); - User user1 = Generator.generateUser(institution); - User user2 = Generator.generateUser(institution); - User user3 = Generator.generateUser(institution); + User user1 = Generator.generateUser(institution, this.roleGroupAdmin); + User user2 = Generator.generateUser(institution, this.roleGroupAdmin); + User user3 = Generator.generateUser(institution, this.roleGroupAdmin); List users = new ArrayList<>(); users.add(user1); @@ -292,7 +297,7 @@ public void changePasswordByTokenReturnsResponseStatusNoContent() throws Excepti public void sendInvitationReturnsResponseStatusBadRequest() throws Exception { final String username = "tom"; final Institution institution = Generator.generateInstitution(); - final User user = Generator.generateUser(institution); + final User user = Generator.generateUser(institution, this.roleGroupAdmin); user.setIsInvited(true); when(userServiceMock.findByUsername(username)).thenReturn(user); @@ -307,7 +312,7 @@ public void sendInvitationReturnsResponseStatusBadRequest() throws Exception { public void sendInvitationReturnsResponseStatusNoContent() throws Exception { final String username = "tom"; final Institution institution = Generator.generateInstitution(); - final User user = Generator.generateUser(institution); + final User user = Generator.generateUser(institution, this.roleGroupAdmin); user.setIsInvited(false); when(userServiceMock.findByUsername(username)).thenReturn(user); @@ -322,7 +327,7 @@ public void sendInvitationReturnsResponseStatusNoContent() throws Exception { public void sendInvitationDeleteReturnsResponseStatusNoContent() throws Exception { final String username = "tom"; final Institution institution = Generator.generateInstitution(); - final User user = Generator.generateUser(institution); + final User user = Generator.generateUser(institution, this.roleGroupAdmin); user.setIsInvited(false); when(userServiceMock.findByUsername(username)).thenReturn(user); diff --git a/src/test/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListenerTest.java b/src/test/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListenerTest.java index d326c106..5a5f8d38 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListenerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/handler/HateoasPagingListenerTest.java @@ -42,7 +42,7 @@ public void setUp() { this.listener = new HateoasPagingListener(); this.uriBuilder = UriComponentsBuilder.newInstance().scheme("http").host("localhost").path("rest/records"); this.responseMock = new MockHttpServletResponse(); - final User author = Generator.generateUser(null); + final User author = Generator.generateUser(null, null); this.records = IntStream.range(0, 10).mapToObj(i -> Generator.generatePatientRecordDto(author)) .collect(Collectors.toList()); } diff --git a/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java b/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java index dbba35bb..57aae8ce 100644 --- a/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java +++ b/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java @@ -3,6 +3,7 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.model.Role; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.model.Vocabulary; import cz.cvut.kbss.study.rest.exception.BadRequestException; @@ -36,10 +37,10 @@ void setUp() { @Test void attemptSwitchUserSwitchesCurrentUserToTarget() { - final User source = Generator.generateUser(null); - source.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); + final User source = Generator.generateUser(null, null); + source.setRoleGroup(Generator.generateRoleGroupWithRoles(Role.administrator)); Environment.setCurrentUser(source); - final User target = Generator.generateUser(null); + final User target = Generator.generateUser(null, null); when(userDetailsService.loadUserByUsername(target.getUsername())).thenReturn(new UserDetails(target)); final MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("username", target.getUsername()); @@ -49,11 +50,13 @@ void attemptSwitchUserSwitchesCurrentUserToTarget() { @Test void attemptSwitchUserThrowsBadRequestExceptionWhenTargetUserIsAdmin() { - final User source = Generator.generateUser(null); - source.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); + RoleGroup roleGroup = Generator.generateRoleGroupWithRoles(Role.administrator); + final User source = Generator.generateUser(null, roleGroup); + source.setRoleGroup(Generator.generateRoleGroupWithRoles(Role.administrator)); Environment.setCurrentUser(source); - final User target = Generator.generateUser(null); - target.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); + final User target = Generator.generateUser(null, roleGroup); + target.addType(Vocabulary.s_i_administrator); + target.setRoleGroup(Generator.generateRoleGroupWithRoles(Role.administrator)); when(userDetailsService.loadUserByUsername(target.getUsername())).thenReturn(new UserDetails(target)); final MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("username", target.getUsername()); diff --git a/src/test/java/cz/cvut/kbss/study/service/BaseServiceTestRunner.java b/src/test/java/cz/cvut/kbss/study/service/BaseServiceTestRunner.java index d28c6130..84de406f 100644 --- a/src/test/java/cz/cvut/kbss/study/service/BaseServiceTestRunner.java +++ b/src/test/java/cz/cvut/kbss/study/service/BaseServiceTestRunner.java @@ -10,6 +10,7 @@ import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.persistence.ConfigDataApplicationContextInitializer; import cz.cvut.kbss.study.persistence.dao.InstitutionDao; +import cz.cvut.kbss.study.persistence.dao.RoleGroupDao; import cz.cvut.kbss.study.persistence.dao.UserDao; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.ExtendWith; @@ -38,6 +39,11 @@ public abstract class BaseServiceTestRunner extends TransactionalTestRunner { protected User user; + @Autowired + private RoleGroupDao roleGroupDao; + + protected RoleGroup roleGroupAdmin; + public static final String USERNAME = "halsey"; public static final String PASSWORD = "john117"; public static final String EMAIL = "john117@gmail.com"; @@ -45,9 +51,10 @@ public abstract class BaseServiceTestRunner extends TransactionalTestRunner { @BeforeEach public void setUp() throws Exception { Institution institution = Generator.generateInstitution(); - RoleGroup roleGroup = Generator.generateRoleGroupWithOneRole(Role.administrator); //TODO - user = Generator.getUser(USERNAME, PASSWORD, "John", "Grant", EMAIL, institution, roleGroup); + this.roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + user = Generator.getUser(USERNAME, PASSWORD, "John", "Grant", EMAIL, institution, this.roleGroupAdmin); transactional(() -> { + roleGroupDao.persist(this.roleGroupAdmin); institutionDao.persist(institution); if (userDao.findByUsername(user.getUsername()) == null) { user.encodePassword(passwordEncoder); diff --git a/src/test/java/cz/cvut/kbss/study/service/UserDetailsServiceTest.java b/src/test/java/cz/cvut/kbss/study/service/UserDetailsServiceTest.java index 0e58a1d6..79dda4ce 100644 --- a/src/test/java/cz/cvut/kbss/study/service/UserDetailsServiceTest.java +++ b/src/test/java/cz/cvut/kbss/study/service/UserDetailsServiceTest.java @@ -2,7 +2,10 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.Role; +import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; +import cz.cvut.kbss.study.persistence.dao.RoleGroupDao; import cz.cvut.kbss.study.service.security.UserDetailsService; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -23,12 +26,14 @@ public class UserDetailsServiceTest extends BaseServiceTestRunner { @Autowired private InstitutionService institutionService; + @Autowired + private RoleGroupService roleGroupService; + @Test public void loadUserByUsername() { Institution institution = Generator.generateInstitution(); institutionService.persist(institution); - - User user = Generator.generateUser(institution); + User user = Generator.generateUser(institution, this.roleGroupAdmin); userService.persist(user); UserDetails userDetails = userDetailsService.loadUserByUsername(user.getUsername()); diff --git a/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java b/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java index 7c29ce7a..a7e3058a 100644 --- a/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java +++ b/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java @@ -4,11 +4,7 @@ import cz.cvut.kbss.study.environment.generator.Generator; import cz.cvut.kbss.study.environment.util.Environment; import cz.cvut.kbss.study.exception.RecordAuthorNotFoundException; -import cz.cvut.kbss.study.model.PatientRecord; -import cz.cvut.kbss.study.model.RecordPhase; -import cz.cvut.kbss.study.model.Role; -import cz.cvut.kbss.study.model.User; -import cz.cvut.kbss.study.model.Vocabulary; +import cz.cvut.kbss.study.model.*; import cz.cvut.kbss.study.persistence.dao.PatientRecordDao; import cz.cvut.kbss.study.service.UserService; import cz.cvut.kbss.study.service.security.SecurityUtils; @@ -54,16 +50,21 @@ class RepositoryPatientRecordServiceTest { private User user; + private RoleGroup roleGroupAdmin; + private RoleGroup roleGroupUser; + @BeforeEach void setUp() { - this.user = Generator.generateUser(Generator.generateInstitution()); + this.roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + this.roleGroupUser = Generator.generateRoleGroupWithRoles(Role.user); + this.user = Generator.generateUser(Generator.generateInstitution(), roleGroupUser); Environment.setCurrentUser(user); when(securityUtils.getCurrentUser()).thenReturn(user); } @Test void importRecordsSetsCurrentUserAsAuthorWhenTheyAreRegularUserAndImportsSpecifiedRecords() { - final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final User originalAuthor = Generator.generateUser(Generator.generateInstitution(), this.roleGroupUser); final List toImport = generateRecordsToImport(originalAuthor); when(recordDao.exists(any())).thenReturn(false); @@ -96,10 +97,10 @@ private List generateRecordsToImport(User originalAuthor) { @Test void importRecordsRetainsRecordProvenanceDataWhenCurrentUserIsAdmin() { - final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final User originalAuthor = Generator.generateUser(Generator.generateInstitution(), this.roleGroupAdmin); final List toImport = generateRecordsToImport(originalAuthor); - user.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.user)); Environment.setCurrentUser(user); + user.getRoleGroup().addRole(Role.administrator); when(userService.exists(originalAuthor.getUri())).thenReturn(true); when(recordDao.exists(any())).thenReturn(false); @@ -119,9 +120,9 @@ void importRecordsRetainsRecordProvenanceDataWhenCurrentUserIsAdmin() { @Test void importRecordsThrowsRecordAuthorNotFoundExceptionWhenAdminImportsRecordsAndRecordAuthorIsNotFound() { - final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final User originalAuthor = Generator.generateUser(Generator.generateInstitution(), this.roleGroupAdmin); final List toImport = generateRecordsToImport(originalAuthor); - user.setRoleGroup(Generator.generateRoleGroupWithOneRole(Role.administrator)); + user.setRoleGroup(Generator.generateRoleGroupWithRoles(Role.administrator)); Environment.setCurrentUser(user); assertThrows(RecordAuthorNotFoundException.class, () -> sut.importRecords(toImport)); @@ -129,7 +130,7 @@ void importRecordsThrowsRecordAuthorNotFoundExceptionWhenAdminImportsRecordsAndR @Test void importRecordsSkipsImportingRecordsThatAlreadyExist() { - final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final User originalAuthor = Generator.generateUser(Generator.generateInstitution(), this.roleGroupUser); final List toImport = generateRecordsToImport(originalAuthor); final PatientRecord existing = toImport.get(Generator.randomIndex(toImport)); when(recordDao.exists(any(URI.class))).thenReturn(false); @@ -144,7 +145,7 @@ void importRecordsSkipsImportingRecordsThatAlreadyExist() { @Test void importRecordsWithPhaseSetsSpecifiedPhaseToAllRecords() { - final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final User originalAuthor = Generator.generateUser(Generator.generateInstitution(), this.roleGroupUser); final List toImport = generateRecordsToImport(originalAuthor); final RecordPhase targetPhase = RecordPhase.values()[Generator.randomInt(0, RecordPhase.values().length)]; when(recordDao.exists(any())).thenReturn(false); @@ -157,7 +158,7 @@ void importRecordsWithPhaseSetsSpecifiedPhaseToAllRecords() { @Test void importRecordsSetsRecordPhaseToOpenOnAllImportedRecordsWhenCurrentUserIsRegularUser() { - final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final User originalAuthor = Generator.generateUser(Generator.generateInstitution(), this.roleGroupUser); final List toImport = generateRecordsToImport(originalAuthor); when(recordDao.exists(any())).thenReturn(false); diff --git a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java index 6cb2e39d..d35787c5 100644 --- a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java +++ b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java @@ -60,6 +60,8 @@ public class SecurityUtilsTest { private User user; + private RoleGroup roleGroupAdmin; + public static final String USERNAME = "username"; public static final String PASSWORD = "pass" + Generator.randomInt(0, 1000); @@ -67,8 +69,8 @@ public class SecurityUtilsTest { public void setUp() { Institution institution = Generator.generateInstitution(); institution.setKey(IdentificationUtils.generateKey()); - RoleGroup roleGroup = new RoleGroup(); - this.user = Generator.getUser(USERNAME, PASSWORD, "John", "Johnie", "Johnie@gmail.com", institution, roleGroup); + this.roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); + this.user = Generator.getUser(USERNAME, PASSWORD, "John", "Johnie", "Johnie@gmail.com", institution, this.roleGroupAdmin); user.generateUri(); } @@ -125,7 +127,7 @@ public void areFromSameInstitutionReturnsTrueForUserFromSameInstitution() { Environment.setCurrentUser(user); when(userDao.findByUsername(user.getUsername())).thenReturn(user); - User userFromSameInstitution = Generator.generateUser(user.getInstitution()); + User userFromSameInstitution = Generator.generateUser(user.getInstitution(), this.roleGroupAdmin); when(userDao.findByInstitution(user.getInstitution())).thenReturn(List.of(user, userFromSameInstitution)); assertTrue(sut.areFromSameInstitution(userFromSameInstitution.getUsername())); @@ -138,7 +140,7 @@ public void areFromSameInstitutionReturnsFalseForUserFromDifferentInstitution() Institution institutionAnother = Generator.generateInstitution(); - User userFromAnotherInstitution = Generator.generateUser(institutionAnother); + User userFromAnotherInstitution = Generator.generateUser(institutionAnother, this.roleGroupAdmin); when(userDao.findByInstitution(user.getInstitution())).thenReturn(List.of(user)); assertFalse(sut.areFromSameInstitution(userFromAnotherInstitution.getUsername())); @@ -162,7 +164,7 @@ public void isRecordInUsersInstitutionReturnsFalseWhenRecordBelongsToInstitution when(userDao.findByUsername(user.getUsername())).thenReturn(user); Institution institutionAnother = Generator.generateInstitution(); institutionAnother.setKey(IdentificationUtils.generateKey()); - User userFromAnotherInstitution = Generator.generateUser(institutionAnother); + User userFromAnotherInstitution = Generator.generateUser(institutionAnother, this.roleGroupAdmin); PatientRecord record = Generator.generatePatientRecord(userFromAnotherInstitution); record.setKey(IdentificationUtils.generateKey()); From fc1242718e8fea945d626827588134fd240dd914 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Mon, 23 Sep 2024 09:39:58 +0200 Subject: [PATCH 03/17] [kbss-cvut/record-manager-ui#202] Replace types with roles --- .../cvut/kbss/study/security/CustomSwitchUserFilter.java | 3 ++- .../cz/cvut/kbss/study/security/model/UserDetails.java | 9 ++++----- .../kbss/study/security/CustomSwitchUserFilterTest.java | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/security/CustomSwitchUserFilter.java b/src/main/java/cz/cvut/kbss/study/security/CustomSwitchUserFilter.java index 80723dc4..fbc4654a 100644 --- a/src/main/java/cz/cvut/kbss/study/security/CustomSwitchUserFilter.java +++ b/src/main/java/cz/cvut/kbss/study/security/CustomSwitchUserFilter.java @@ -1,5 +1,6 @@ package cz.cvut.kbss.study.security; +import cz.cvut.kbss.study.model.Role; import cz.cvut.kbss.study.rest.exception.BadRequestException; import jakarta.servlet.http.HttpServletRequest; import org.springframework.security.core.Authentication; @@ -14,7 +15,7 @@ public class CustomSwitchUserFilter extends SwitchUserFilter { @Override protected Authentication attemptSwitchUser(HttpServletRequest request) throws AuthenticationException { final Authentication switchTo = super.attemptSwitchUser(request); - if (switchTo.getAuthorities().stream().anyMatch(a -> SecurityConstants.ROLE_ADMIN.equals(a.getAuthority()))) { + if (switchTo.getAuthorities().stream().anyMatch(a -> Role.administrator.name().equals(a.getAuthority()))) { throw new BadRequestException("Cannot impersonate admin."); } return switchTo; diff --git a/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java b/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java index 90c882f5..2c7b3897 100644 --- a/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java +++ b/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java @@ -1,5 +1,6 @@ package cz.cvut.kbss.study.security.model; +import cz.cvut.kbss.study.model.Role; import cz.cvut.kbss.study.model.User; import cz.cvut.kbss.study.security.SecurityConstants; import org.springframework.security.core.GrantedAuthority; @@ -36,12 +37,10 @@ public UserDetails(User user, Collection authorities) { private void resolveRoles() { authorities.addAll( - user.getTypes().stream() - .map(Role::forType) - .filter(Optional::isPresent) - .map(r -> new SimpleGrantedAuthority(r.get().getName())) + user.getRoleGroup().getRoles().stream() + .map(r -> new SimpleGrantedAuthority(r.name())) .toList()); - authorities.add(new SimpleGrantedAuthority(SecurityConstants.ROLE_USER)); + authorities.add(new SimpleGrantedAuthority(Role.user.name())); } public void eraseCredentials() { diff --git a/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java b/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java index 57aae8ce..80aa2c58 100644 --- a/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java +++ b/src/test/java/cz/cvut/kbss/study/security/CustomSwitchUserFilterTest.java @@ -41,6 +41,7 @@ void attemptSwitchUserSwitchesCurrentUserToTarget() { source.setRoleGroup(Generator.generateRoleGroupWithRoles(Role.administrator)); Environment.setCurrentUser(source); final User target = Generator.generateUser(null, null); + target.setRoleGroup(Generator.generateRoleGroupWithRoles(Role.user)); when(userDetailsService.loadUserByUsername(target.getUsername())).thenReturn(new UserDetails(target)); final MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("username", target.getUsername()); @@ -55,7 +56,6 @@ void attemptSwitchUserThrowsBadRequestExceptionWhenTargetUserIsAdmin() { source.setRoleGroup(Generator.generateRoleGroupWithRoles(Role.administrator)); Environment.setCurrentUser(source); final User target = Generator.generateUser(null, roleGroup); - target.addType(Vocabulary.s_i_administrator); target.setRoleGroup(Generator.generateRoleGroupWithRoles(Role.administrator)); when(userDetailsService.loadUserByUsername(target.getUsername())).thenReturn(new UserDetails(target)); final MockHttpServletRequest request = new MockHttpServletRequest(); From 152c841661a435a37d340926fff8879da0065c87 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Wed, 25 Sep 2024 14:50:09 +0200 Subject: [PATCH 04/17] [kbss-cvut/record-manager-ui#202] Replace hasRole with hasAuthorities --- .../study/config/OAuth2SecurityConfig.java | 2 +- .../kbss/study/config/SecurityConfig.java | 3 +- .../study/rest/ActionHistoryController.java | 4 +-- .../kbss/study/rest/FormGenController.java | 2 +- .../study/rest/InstitutionController.java | 16 +++++----- .../kbss/study/rest/OidcUserController.java | 12 ++++---- .../study/rest/PatientRecordController.java | 14 ++++----- .../kbss/study/rest/StatisticsController.java | 4 +-- .../cvut/kbss/study/rest/UserController.java | 24 +++++++-------- .../study/security/SecurityConstants.java | 29 ++++++++++++++++--- .../service/security/SecurityUtilsTest.java | 4 +-- .../OidcGrantedAuthoritiesExtractorTest.java | 6 ++-- 12 files changed, 71 insertions(+), 49 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java b/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java index c791dfcb..34be7fdf 100644 --- a/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java +++ b/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java @@ -79,7 +79,7 @@ private Converter grantedAuthoritiesExtractor( assert extractedRoles != null; final Set authorities = new HashSet<>(extractedRoles); // Add default role if it is not present - authorities.add(new SimpleGrantedAuthority(SecurityConstants.ROLE_USER)); + authorities.add(new SimpleGrantedAuthority(SecurityConstants.user)); return new JwtAuthenticationToken(source, authorities); }; } diff --git a/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java b/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java index dc57f81d..a9036481 100644 --- a/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java +++ b/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java @@ -1,6 +1,7 @@ package cz.cvut.kbss.study.config; import cz.cvut.kbss.study.exception.RecordManagerException; +import cz.cvut.kbss.study.model.Role; import cz.cvut.kbss.study.security.CsrfHeaderFilter; import cz.cvut.kbss.study.security.CustomSwitchUserFilter; import cz.cvut.kbss.study.security.SecurityConstants; @@ -79,7 +80,7 @@ public SecurityFilterChain filterChain(HttpSecurity http, ConfigReader config, LOG.debug("Using internal security mechanisms."); final AuthenticationManager authManager = buildAuthenticationManager(http); http.authorizeHttpRequests( - (auth) -> auth.requestMatchers("/rest/users/impersonate").hasAuthority(SecurityConstants.ROLE_ADMIN) + (auth) -> auth.requestMatchers("/rest/users/impersonate").hasAuthority(Role.administrator.name()) .anyRequest().permitAll()) .cors((auth) -> auth.configurationSource(corsConfigurationSource(config))) .csrf(AbstractHttpConfigurer::disable) diff --git a/src/main/java/cz/cvut/kbss/study/rest/ActionHistoryController.java b/src/main/java/cz/cvut/kbss/study/rest/ActionHistoryController.java index 026d7f92..e460f51d 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/ActionHistoryController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/ActionHistoryController.java @@ -54,7 +54,7 @@ public void create(@RequestBody ActionHistory actionHistory) { } } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getActions(@RequestParam(value = "author", required = false) String authorUsername, @RequestParam(value = "type", required = false) String type, @@ -73,7 +73,7 @@ public List getActions(@RequestParam(value = "author", required = return result.getContent(); } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @GetMapping(value = "/{key}", produces = MediaType.APPLICATION_JSON_VALUE) public ActionHistory getByKey(@PathVariable("key") String key) { final ActionHistory action = actionHistoryService.findByKey(key); diff --git a/src/main/java/cz/cvut/kbss/study/rest/FormGenController.java b/src/main/java/cz/cvut/kbss/study/rest/FormGenController.java index c9969946..91b5c3bf 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/FormGenController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/FormGenController.java @@ -9,7 +9,7 @@ import org.springframework.web.bind.annotation.*; @RestController -@PreAuthorize("hasRole('" + SecurityConstants.ROLE_USER + "')") +@PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") @RequestMapping("/formGen") public class FormGenController extends BaseController { diff --git a/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java b/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java index d96e4bae..1a3bbc9a 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java @@ -30,7 +30,7 @@ import static cz.cvut.kbss.study.rest.util.RecordFilterMapper.constructRecordFilter; @RestController -@PreAuthorize("hasRole('" + SecurityConstants.ROLE_USER + "')") +@PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") @RequestMapping("/institutions") public class InstitutionController extends BaseController { @@ -44,7 +44,7 @@ public InstitutionController(InstitutionService institutionService, this.recordService = recordService; } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getAllInstitutions() { final List institutions = institutionService.findAll(); @@ -52,8 +52,8 @@ public List getAllInstitutions() { return institutions; } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') " + - "or hasRole('" + SecurityConstants.ROLE_USER + "') and @securityUtils.isMemberOfInstitution(#key)") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') " + + "or hasAuthority('" + SecurityConstants.user + "') and @securityUtils.isMemberOfInstitution(#key)") @GetMapping(value = "/{key}", produces = MediaType.APPLICATION_JSON_VALUE) public Institution findByKey(@PathVariable("key") String key) { return findInternal(key); @@ -67,7 +67,7 @@ private Institution findInternal(String key) { return result; } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isRecordInUsersInstitution(#key)") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or @securityUtils.isRecordInUsersInstitution(#key)") @GetMapping(value = "/{key}/patients", produces = MediaType.APPLICATION_JSON_VALUE) public List getTreatedPatientRecords(@PathVariable("key") String key) { final Institution inst = findInternal(key); @@ -75,7 +75,7 @@ public List getTreatedPatientRecords(@PathVariable("key") Stri return recordService.findAll(constructRecordFilter("institution", key), Pageable.unpaged()).getContent(); } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.CREATED) public ResponseEntity createInstitution(@RequestBody Institution institution) { @@ -88,7 +88,7 @@ public ResponseEntity createInstitution(@RequestBody Institution instituti return new ResponseEntity<>(headers, HttpStatus.CREATED); } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @PutMapping(value = "/{key}", consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void updateInstitution(@PathVariable("key") String key, @RequestBody Institution institution) { @@ -104,7 +104,7 @@ public void updateInstitution(@PathVariable("key") String key, @RequestBody Inst } } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @DeleteMapping(value = "/{key}") @ResponseStatus(HttpStatus.NO_CONTENT) public void deleteInstitution(@PathVariable("key") String key) { diff --git a/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java b/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java index 46321dc5..f757a2d3 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java @@ -38,14 +38,14 @@ public OidcUserController(UserService userService, InstitutionService institutio this.institutionService = institutionService; } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_USER + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") @GetMapping(value = "/current", produces = MediaType.APPLICATION_JSON_VALUE) public User getCurrent() { return userService.getCurrentUser(); } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name or " + - "hasRole('" + SecurityConstants.ROLE_USER + "') and @securityUtils.areFromSameInstitution(#username)") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name or " + + "hasAuthority('" + SecurityConstants.user + "') and @securityUtils.areFromSameInstitution(#username)") @GetMapping(value = "/{username}", produces = MediaType.APPLICATION_JSON_VALUE) public User getByUsername(@PathVariable("username") String username) { final User user = userService.findByUsername(username); @@ -56,14 +56,14 @@ public User getByUsername(@PathVariable("username") String username) { } @PreAuthorize( - "hasRole('" + SecurityConstants.ROLE_ADMIN + "') " + - "or hasRole('" + SecurityConstants.ROLE_USER + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") + "hasAuthority('" + SecurityConstants.administrator + "') " + + "or hasAuthority('" + SecurityConstants.administrator + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getUsers(@RequestParam(value = "institution", required = false) String institutionKey) { return institutionKey != null ? getByInstitution(institutionKey) : userService.findAll(); } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name") @PutMapping(value = "/{username}", consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void updateUser(@PathVariable("username") String username, @RequestBody User user, diff --git a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java index aac31673..632d2e03 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java @@ -42,7 +42,7 @@ import java.util.stream.Stream; @RestController -@PreAuthorize("hasRole('" + SecurityConstants.ROLE_USER + "')") +@PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") @RequestMapping("/records") public class PatientRecordController extends BaseController { @@ -70,7 +70,7 @@ public PatientRecordController(PatientRecordService recordService, ApplicationEv this.publishRecordsService = publishRecordsService; } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or #institutionKey==null or @securityUtils.isMemberOfInstitution(#institutionKey)") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #institutionKey==null or @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getRecords( @RequestParam(value = "institution", required = false) String institutionKey, @@ -79,7 +79,7 @@ public List getRecords( Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); boolean hasAdminRole = authentication.getAuthorities().stream() - .anyMatch(authority -> authority.getAuthority().equals(SecurityConstants.ROLE_ADMIN)); + .anyMatch(authority -> authority.getAuthority().equals(SecurityConstants.administrator)); if (!hasAdminRole && institutionKey == null) { throw new ValidationException("record.save-error.user-not-assigned-to-institution", @@ -91,7 +91,7 @@ public List getRecords( return result.getContent(); } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(value="used-record-phases", produces = MediaType.APPLICATION_JSON_VALUE) public Set getUsedRecordPhases(@RequestParam(value = "institution", required = false) String institutionKey){ return recordService.findUsedRecordPhases(); @@ -99,7 +99,7 @@ public Set getUsedRecordPhases(@RequestParam(value = "institution", @PreAuthorize( - "hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") + "hasAuthority('" + SecurityConstants.administrator + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(value = "/export", produces = {MediaType.APPLICATION_JSON_VALUE, Constants.MEDIA_TYPE_EXCEL}) public ResponseEntity exportRecords( @RequestParam(name = "institution", required = false) String institutionKey, @@ -158,7 +158,7 @@ public ResponseEntity exportRecordsExcel(MultiValueMap createRecord(@RequestBody PatientRecord record) { } @PreAuthorize( - "hasRole('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") + "hasAuthority('" + SecurityConstants.administrator + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @PostMapping(value = "/publish", produces = {MediaType.APPLICATION_JSON_VALUE}) public RecordImportResult publishRecords( @RequestParam(name = "institution", required = false) String institutionKey, diff --git a/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java b/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java index ca1b70d5..742d0527 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java @@ -11,7 +11,7 @@ import java.util.HashMap; import java.util.Map; -@PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") +@PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @RestController @RequestMapping("/statistics") public class StatisticsController extends BaseController { @@ -22,7 +22,7 @@ public StatisticsController(StatisticsService statisticsService) { this.statisticsService = statisticsService; } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public Map getStatistics() { Map data = new HashMap<>(); diff --git a/src/main/java/cz/cvut/kbss/study/rest/UserController.java b/src/main/java/cz/cvut/kbss/study/rest/UserController.java index b0976217..c94274b1 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/UserController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/UserController.java @@ -47,8 +47,8 @@ public UserController(UserService userService, InstitutionService institutionSer this.institutionService = institutionService; } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name or " + - "hasRole('" + SecurityConstants.ROLE_USER + "') and @securityUtils.areFromSameInstitution(#username)") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name or " + + "hasAuthority('" + SecurityConstants.user + "') and @securityUtils.areFromSameInstitution(#username)") @GetMapping(value = "/{username}", produces = MediaType.APPLICATION_JSON_VALUE) public User getByUsername(@PathVariable("username") String username) { final User user = userService.findByUsername(username); @@ -58,13 +58,13 @@ public User getByUsername(@PathVariable("username") String username) { return user; } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_USER + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") @GetMapping(value = "/current", produces = MediaType.APPLICATION_JSON_VALUE) public User getCurrent() { return userService.getCurrentUser(); } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE) public ResponseEntity create(@RequestBody User user) { userService.persist(user); @@ -77,8 +77,8 @@ public ResponseEntity create(@RequestBody User user) { } @PreAuthorize( - "hasRole('" + SecurityConstants.ROLE_ADMIN + "') " + - "or hasRole('" + SecurityConstants.ROLE_USER + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") + "hasAuthority('" + SecurityConstants.administrator + "') " + + "or hasAuthority('" + SecurityConstants.user + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getUsers(@RequestParam(value = "institution", required = false) String institutionKey) { return institutionKey != null ? getByInstitution(institutionKey) : userService.findAll(); @@ -90,7 +90,7 @@ private List getByInstitution(String institutionKey) { return userService.findByInstitution(institution); } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @DeleteMapping(value = "/{username}") @ResponseStatus(HttpStatus.NO_CONTENT) public void removeUser(@PathVariable("username") String username) { @@ -101,7 +101,7 @@ public void removeUser(@PathVariable("username") String username) { } } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name") @PutMapping(value = "/{username}", consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void updateUser(@PathVariable("username") String username, @RequestBody User user, @@ -117,7 +117,7 @@ public void updateUser(@PathVariable("username") String username, @RequestBody U } } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name") @PutMapping(value = "/{username}/password-change", consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void updatePassword(@PathVariable("username") String username, @RequestBody Map password, @@ -147,7 +147,7 @@ public void resetPassword(@RequestBody String emailAddress) { } } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @GetMapping(value = "/generate-username/{usernamePrefix}", produces = MediaType.TEXT_PLAIN_VALUE) public String generateUsername(@PathVariable(value = "usernamePrefix") String usernamePrefix) { return userService.generateUsername(usernamePrefix); @@ -173,7 +173,7 @@ public void changePasswordByToken(@RequestBody Map data) { } } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @PutMapping(value = "/send-invitation/{username}") @ResponseStatus(HttpStatus.NO_CONTENT) public void sendInvitation(@PathVariable(value = "username") String username) { @@ -188,7 +188,7 @@ public void sendInvitation(@PathVariable(value = "username") String username) { } } - @PreAuthorize("hasRole('" + SecurityConstants.ROLE_ADMIN + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") @PostMapping(value = "/send-invitation/delete", consumes = MediaType.TEXT_PLAIN_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void deleteInvitationOption(@RequestBody String username) { diff --git a/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java b/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java index 516c7798..0443e8e7 100644 --- a/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java +++ b/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java @@ -27,11 +27,32 @@ private SecurityConstants() { */ public static final int SESSION_TIMEOUT = 12 * 60 * 60; - public static final String ROLE_USER = "ROLE_USER"; + public static final String user = "user"; - public static final String ROLE_ADMIN = "ROLE_ADMIN"; + public static final String administrator = "administrator"; - public static final String ROLE_GROUP_USER = "ROLE_GROUP_USER";; + public static final String impersonate = "impersonate"; + + public static final String deleteAllRecords = "deleteAllRecords"; + + public static final String viewAllRecords = "viewAllRecords"; + + public static final String editAllRecords = "editAllRecords"; + + public static final String deleteOrganizationRecords = "deleteOrganizationRecords"; + + public static final String viewOrganizationRecords = "viewOrganizationRecords"; + + public static final String editOrganizationRecords = "editOrganizationRecords"; + + public static final String editUsers = "editUsers"; + + public static final String completeRecords = "completeRecords"; + + public static final String rejectRecords = "rejectRecords"; + + public static final String publishRecords = "publishRecords"; + + public static final String importCodelists = "importCodelists"; - public static final Object ROLE_GROUP_ADMIN = "ROLE_GROUP_ADMIN"; } diff --git a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java index d35787c5..00c37805 100644 --- a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java +++ b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java @@ -93,7 +93,7 @@ void getCurrentUserRetrievesCurrentUserForOauthJwtAccessToken() { final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") - .claim("roles", List.of(SecurityConstants.ROLE_USER)) + .claim("roles", List.of(SecurityConstants.user)) .issuer("http://localhost:8080/termit") .subject(USERNAME) .claim("preferred_username", USERNAME) @@ -179,7 +179,7 @@ void getCurrentUserEnhancesRetrievedUserWithTypesCorrespondingToRolesSpecifiedIn final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") - .claim("roles", List.of(SecurityConstants.ROLE_ADMIN)) + .claim("roles", List.of(SecurityConstants.administrator)) .issuer("http://localhost:8080/termit") .subject(USERNAME) .claim("preferred_username", USERNAME) diff --git a/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java b/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java index 881a3320..da906cee 100644 --- a/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java +++ b/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java @@ -30,7 +30,7 @@ class OidcGrantedAuthoritiesExtractorTest { @Test void convertMapsTopLevelClaimWithRolesToGrantedAuthorities() { when(config.getConfig(ConfigParam.OIDC_ROLE_CLAIM)).thenReturn("roles"); - final List roles = List.of(SecurityConstants.ROLE_ADMIN, SecurityConstants.ROLE_USER); + final List roles = List.of(SecurityConstants.administrator, SecurityConstants.user); final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") @@ -51,7 +51,7 @@ void convertMapsTopLevelClaimWithRolesToGrantedAuthorities() { @Test void convertSupportsNestedRolesClaim() { when(config.getConfig(ConfigParam.OIDC_ROLE_CLAIM)).thenReturn("realm_access.roles"); - final List roles = List.of(SecurityConstants.ROLE_ADMIN, SecurityConstants.ROLE_USER); + final List roles = List.of(SecurityConstants.administrator, SecurityConstants.user); final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") @@ -91,7 +91,7 @@ void convertThrowsIllegalArgumentExceptionWhenNestedRolesClaimIsNotList() { final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") - .claim("realm_access", Map.of("roles", Map.of("notlist", SecurityConstants.ROLE_USER))) + .claim("realm_access", Map.of("roles", Map.of("notlist", SecurityConstants.user))) .issuer("http://localhost:8080/termit") .subject("termit") .expiresAt(Instant.now().truncatedTo(ChronoUnit.SECONDS).plusSeconds(300)) From 2fc603b0d81072c1e7db6dc7ed3311b54498af9a Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Wed, 25 Sep 2024 14:51:31 +0200 Subject: [PATCH 05/17] [kbss-cvut/record-manager-ui#202] Remove Security Role --- .../java/cz/cvut/kbss/study/model/Role.java | 7 +-- .../cvut/kbss/study/security/model/Role.java | 36 --------------- .../study/service/security/SecurityUtils.java | 9 +--- .../kbss/study/security/model/RoleTest.java | 44 ------------------- 4 files changed, 3 insertions(+), 93 deletions(-) delete mode 100644 src/main/java/cz/cvut/kbss/study/security/model/Role.java delete mode 100644 src/test/java/cz/cvut/kbss/study/security/model/RoleTest.java diff --git a/src/main/java/cz/cvut/kbss/study/model/Role.java b/src/main/java/cz/cvut/kbss/study/model/Role.java index 83f8d19b..cb7807f8 100644 --- a/src/main/java/cz/cvut/kbss/study/model/Role.java +++ b/src/main/java/cz/cvut/kbss/study/model/Role.java @@ -55,12 +55,7 @@ public enum Role { this.iri = iri; } - public static Role getFromSecurityRole(cz.cvut.kbss.study.security.model.Role r) { - return switch (r) { - case USER -> user; - case ADMIN -> administrator; - }; - } + public String getIri() { return iri; diff --git a/src/main/java/cz/cvut/kbss/study/security/model/Role.java b/src/main/java/cz/cvut/kbss/study/security/model/Role.java deleted file mode 100644 index 64665215..00000000 --- a/src/main/java/cz/cvut/kbss/study/security/model/Role.java +++ /dev/null @@ -1,36 +0,0 @@ -package cz.cvut.kbss.study.security.model; - -import cz.cvut.kbss.study.model.Vocabulary; -import cz.cvut.kbss.study.security.SecurityConstants; - -import java.util.Optional; -import java.util.stream.Stream; - -public enum Role { - USER(SecurityConstants.ROLE_USER, Vocabulary.s_i_user), - ADMIN(SecurityConstants.ROLE_ADMIN, Vocabulary.s_i_administrator); - - private final String name; - private final String type; - - Role(String name, String type) { - this.name = name; - this.type = type; - } - - public static Optional forType(String type) { - return Stream.of(Role.values()).filter(r -> r.type.equals(type)).findAny(); - } - - public static Optional forName(String name) { - return Stream.of(Role.values()).filter(r -> r.name.equals(name)).findAny(); - } - - public String getName() { - return name; - } - - public String getType() { - return type; - } -} diff --git a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java index 6233af86..587e144b 100644 --- a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java +++ b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java @@ -1,13 +1,9 @@ package cz.cvut.kbss.study.service.security; import cz.cvut.kbss.study.exception.NotFoundException; -import cz.cvut.kbss.study.model.PatientRecord; -import cz.cvut.kbss.study.model.RoleGroup; -import cz.cvut.kbss.study.model.User; -import cz.cvut.kbss.study.model.Vocabulary; +import cz.cvut.kbss.study.model.*; import cz.cvut.kbss.study.persistence.dao.PatientRecordDao; import cz.cvut.kbss.study.persistence.dao.UserDao; -import cz.cvut.kbss.study.security.model.Role; import cz.cvut.kbss.study.security.model.UserDetails; import cz.cvut.kbss.study.service.ConfigReader; import cz.cvut.kbss.study.util.ConfigParam; @@ -106,8 +102,7 @@ private User resolveAccountFromOAuthPrincipal(Jwt principal) { RoleGroup roleGroup = new RoleGroup(); user.setRoleGroup(roleGroup); - roles.stream().map(Role::forName).filter(Optional::isPresent).map(Optional::get) - .forEach(r -> roleGroup.addRole(cz.cvut.kbss.study.model.Role.getFromSecurityRole(r))); + roles.forEach(r -> roleGroup.addRole(Role.fromIriOrName(r))); return user; } diff --git a/src/test/java/cz/cvut/kbss/study/security/model/RoleTest.java b/src/test/java/cz/cvut/kbss/study/security/model/RoleTest.java deleted file mode 100644 index b006c35c..00000000 --- a/src/test/java/cz/cvut/kbss/study/security/model/RoleTest.java +++ /dev/null @@ -1,44 +0,0 @@ -package cz.cvut.kbss.study.security.model; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; - -import java.util.Optional; -import java.util.stream.Stream; - -import static org.junit.jupiter.api.Assertions.*; - -class RoleTest { - - static Stream generator() { - return Stream.of(Role.values()).map(Arguments::of); - } - - @ParameterizedTest - @MethodSource("generator") - void forTypeReturnsRoleMatchingSpecifiedType(Role r) { - final Optional result = Role.forType(r.getType()); - assertTrue(result.isPresent()); - assertEquals(r, result.get()); - } - - @Test - void forTypeReturnsEmptyOptionalForUnknownRoleType() { - assertTrue(Role.forType("unknownType").isEmpty()); - } - - @ParameterizedTest - @MethodSource("generator") - void forNameReturnsRoleMatchingSpecifiedRoleName(Role r) { - final Optional result = Role.forName(r.getName()); - assertTrue(result.isPresent()); - assertEquals(r, result.get()); - } - - @Test - void forNameReturnsEmptyOptionalForUnknownRoleName() { - assertTrue(Role.forName("unknownName").isEmpty()); - } -} \ No newline at end of file From 0e3bc349fcfa91d80511b7848c6046e20194efe3 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Wed, 25 Sep 2024 14:51:44 +0200 Subject: [PATCH 06/17] [kbss-cvut/record-manager-ui#202] Remove types from the user --- .../java/cz/cvut/kbss/study/model/User.java | 24 +------------------ .../repository/RepositoryUserService.java | 2 +- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/model/User.java b/src/main/java/cz/cvut/kbss/study/model/User.java index 99b0ad67..498c8c28 100644 --- a/src/main/java/cz/cvut/kbss/study/model/User.java +++ b/src/main/java/cz/cvut/kbss/study/model/User.java @@ -1,27 +1,22 @@ package cz.cvut.kbss.study.model; import com.fasterxml.jackson.annotation.JsonProperty; -import cz.cvut.kbss.jopa.model.annotations.CascadeType; import cz.cvut.kbss.jopa.model.annotations.FetchType; import cz.cvut.kbss.jopa.model.annotations.Id; import cz.cvut.kbss.jopa.model.annotations.OWLClass; import cz.cvut.kbss.jopa.model.annotations.OWLDataProperty; import cz.cvut.kbss.jopa.model.annotations.OWLObjectProperty; import cz.cvut.kbss.jopa.model.annotations.ParticipationConstraints; -import cz.cvut.kbss.jopa.model.annotations.Types; import cz.cvut.kbss.study.model.util.HasDerivableUri; import cz.cvut.kbss.study.util.Constants; import cz.cvut.kbss.study.util.IdentificationUtils; import org.springframework.security.crypto.password.PasswordEncoder; - import java.io.Serializable; import java.io.UnsupportedEncodingException; import java.net.URI; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.Date; -import java.util.HashSet; -import java.util.Set; @OWLClass(iri = Vocabulary.s_c_Person) public class User implements HasDerivableUri, Serializable { @@ -65,12 +60,8 @@ public class User implements HasDerivableUri, Serializable { @OWLObjectProperty(iri = Vocabulary.s_p_has_role_group) private RoleGroup roleGroup; - @Types - private Set types; - public User() { - this.types = new HashSet<>(); - types.add(Vocabulary.s_c_doctor_role_group); + } @Override @@ -149,19 +140,6 @@ public void setRoleGroup(RoleGroup roleGroup) { this.roleGroup = roleGroup; } - public Set getTypes() { - return types; - } - - public void setTypes(Set types) { - this.types = types; - } - - public void addType(String type) { - assert types != null; - getTypes().add(type); - } - /** * Returns true if this user is an admin. *

diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java index d402df8b..be04c2b2 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java @@ -203,7 +203,7 @@ protected void prePersist(User instance) { protected void preUpdate(User instance) { final User currentUser = securityUtils.getCurrentUser(); if (!currentUser.isAdmin() - && (!instance.getTypes().equals(currentUser.getTypes()) || (instance.getInstitution() != null + && (!instance.getRoleGroup().getRoles().equals(currentUser.getRoleGroup().getRoles()) || (instance.getInstitution() != null && !instance.getInstitution().getKey().equals(currentUser.getInstitution().getKey())))) { throw new UnauthorizedException("Cannot update user."); } From 27528336ae62227636dc4a016ea871b899bc3a25 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Wed, 25 Sep 2024 16:00:23 +0200 Subject: [PATCH 07/17] [kbss-cvut/record-manager-ui#202] Implement RoleGroup controller --- .../cz/cvut/kbss/study/model/RoleGroup.java | 14 +++ .../kbss/study/rest/RoleGroupController.java | 48 ++++++++++ .../kbss/study/service/RoleGroupService.java | 1 - .../study/rest/RoleGroupControllerTest.java | 89 +++++++++++++++++++ 4 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java create mode 100644 src/test/java/cz/cvut/kbss/study/rest/RoleGroupControllerTest.java diff --git a/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java b/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java index eb7e5511..f058af50 100644 --- a/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java +++ b/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java @@ -7,6 +7,7 @@ import java.io.Serializable; import java.net.URI; import java.util.HashSet; +import java.util.Objects; import java.util.Set; @OWLClass(iri = Vocabulary.s_c_role_group) @@ -54,4 +55,17 @@ public void setRoles(Set roles) { public void generateUri() { this.uri = URI.create(Constants.BASE_URI + "sdfsf"); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + RoleGroup roleGroup = (RoleGroup) o; + return Objects.equals(name, roleGroup.name); + } + + @Override + public int hashCode() { + return Objects.hash(name); + } } diff --git a/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java b/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java new file mode 100644 index 00000000..494e348f --- /dev/null +++ b/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java @@ -0,0 +1,48 @@ +package cz.cvut.kbss.study.rest; + + +import cz.cvut.kbss.study.exception.NotFoundException; +import cz.cvut.kbss.study.model.RoleGroup; +import cz.cvut.kbss.study.security.SecurityConstants; +import cz.cvut.kbss.study.service.RoleGroupService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.http.MediaType; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import java.util.List; + + +@ConditionalOnProperty(prefix = "security", name = "provider", havingValue = "internal", matchIfMissing = true) +@RestController +@RequestMapping("/roleGroup") +public class RoleGroupController extends BaseController{ + + private final RoleGroupService roleGroupService; + + @Autowired + public RoleGroupController(RoleGroupService roleGroupService) { + this.roleGroupService = roleGroupService; + } + + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) + public List getRoleGroups() { + return roleGroupService.findAll(); + } + + @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @GetMapping(value = "/{name}",produces = MediaType.APPLICATION_JSON_VALUE) + public RoleGroup findByName(@PathVariable("name") String name) { + RoleGroup result = roleGroupService.findByName(name); + if(result == null){ + throw NotFoundException.create("RoleGroup", name); + } + return result; + } + +} diff --git a/src/main/java/cz/cvut/kbss/study/service/RoleGroupService.java b/src/main/java/cz/cvut/kbss/study/service/RoleGroupService.java index b18f421f..1f373f74 100644 --- a/src/main/java/cz/cvut/kbss/study/service/RoleGroupService.java +++ b/src/main/java/cz/cvut/kbss/study/service/RoleGroupService.java @@ -8,5 +8,4 @@ public interface RoleGroupService extends BaseService { RoleGroup findByName(String name); - } diff --git a/src/test/java/cz/cvut/kbss/study/rest/RoleGroupControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/RoleGroupControllerTest.java new file mode 100644 index 00000000..d27c1ca7 --- /dev/null +++ b/src/test/java/cz/cvut/kbss/study/rest/RoleGroupControllerTest.java @@ -0,0 +1,89 @@ +package cz.cvut.kbss.study.rest; + +import com.fasterxml.jackson.core.type.TypeReference; +import cz.cvut.kbss.study.model.Institution; +import cz.cvut.kbss.study.model.RoleGroup; +import cz.cvut.kbss.study.service.RoleGroupService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.test.web.servlet.MvcResult; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; + +@ExtendWith(MockitoExtension.class) +public class RoleGroupControllerTest extends BaseControllerTestRunner { + + @Mock + private RoleGroupService roleGroupServiceMock; + + + @InjectMocks + private RoleGroupController controller; + + @BeforeEach + public void setUp() { + super.setUp(controller); + } + + @Test + public void testGetRoleGroups() throws Exception { + RoleGroup roleGroup = new RoleGroup(); + roleGroup.setName("admin-role-group"); + + when(roleGroupServiceMock.findAll()).thenReturn(List.of(roleGroup)); + + final MvcResult result = mockMvc.perform(get("/roleGroup/")).andReturn(); + + final List body = objectMapper.readValue(result.getResponse().getContentAsString(), + new TypeReference<>() { + }); + + assertEquals(HttpStatus.OK, HttpStatus.valueOf(result.getResponse().getStatus())); + assertEquals(body, List.of(roleGroup)); + } + + @Test + public void testFindByName() throws Exception { + String roleName = "admin-role-group"; + RoleGroup roleGroup = new RoleGroup(); + roleGroup.setName(roleName); + + when(roleGroupServiceMock.findByName(roleName)).thenReturn(roleGroup); + + final MvcResult result = mockMvc.perform(get("/roleGroup/" + roleName)).andReturn(); + + final RoleGroup body = objectMapper.readValue(result.getResponse().getContentAsString(), + new TypeReference<>() { + }); + + assertEquals(HttpStatus.OK, HttpStatus.valueOf(result.getResponse().getStatus())); + assertEquals(body, roleGroup); + } + + @Test + public void testFindByName_NotFound() throws Exception { + String roleName = "NonExistentRole"; + + when(roleGroupServiceMock.findByName(roleName)).thenReturn(null); + + mockMvc.perform(get("/roleGroup/{name}", roleName) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isNotFound()); + } + +} From fea4ef170c01d2dd3dd0ce1c5d2c382c1f65734a Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Thu, 26 Sep 2024 16:00:29 +0200 Subject: [PATCH 08/17] [kbss-cvut/record-manager-ui#202] Refactor roles names --- .../kbss/study/config/SecurityConfig.java | 2 +- .../java/cz/cvut/kbss/study/model/Role.java | 48 +++++++++++-------- .../cz/cvut/kbss/study/model/RoleGroup.java | 2 +- .../java/cz/cvut/kbss/study/model/User.java | 2 +- .../kbss/study/persistence/dao/UserDao.java | 2 +- .../security/CustomSwitchUserFilter.java | 2 +- .../study/security/SecurityConstants.java | 26 +++++----- .../study/security/model/UserDetails.java | 2 +- src/main/resources/model.ttl | 22 ++------- .../cz/cvut/kbss/study/model/RoleTest.java | 19 ++++---- .../persistence/dao/PatientRecordDaoTest.java | 1 - 11 files changed, 61 insertions(+), 67 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java b/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java index a9036481..51483041 100644 --- a/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java +++ b/src/main/java/cz/cvut/kbss/study/config/SecurityConfig.java @@ -80,7 +80,7 @@ public SecurityFilterChain filterChain(HttpSecurity http, ConfigReader config, LOG.debug("Using internal security mechanisms."); final AuthenticationManager authManager = buildAuthenticationManager(http); http.authorizeHttpRequests( - (auth) -> auth.requestMatchers("/rest/users/impersonate").hasAuthority(Role.administrator.name()) + (auth) -> auth.requestMatchers("/rest/users/impersonate").hasAuthority(Role.administrator.getRoleName()) .anyRequest().permitAll()) .cors((auth) -> auth.configurationSource(corsConfigurationSource(config))) .csrf(AbstractHttpConfigurer::disable) diff --git a/src/main/java/cz/cvut/kbss/study/model/Role.java b/src/main/java/cz/cvut/kbss/study/model/Role.java index cb7807f8..37bf8dab 100644 --- a/src/main/java/cz/cvut/kbss/study/model/Role.java +++ b/src/main/java/cz/cvut/kbss/study/model/Role.java @@ -1,61 +1,67 @@ package cz.cvut.kbss.study.model; +import com.fasterxml.jackson.annotation.JsonValue; import cz.cvut.kbss.jopa.model.annotations.Individual; -import java.util.Optional; -import org.apache.poi.ss.formula.atp.Switch; +import cz.cvut.kbss.study.security.SecurityConstants; public enum Role { // TODO deprecated -- should be removed. - @Individual(iri=Vocabulary.s_i_administrator) - administrator(Vocabulary.s_i_administrator), + @Individual(iri=Vocabulary.s_i_RM_ADMIN) + administrator(SecurityConstants.administrator, Vocabulary.s_i_RM_ADMIN), // TODO deprecated -- should be removed. - @Individual(iri = Vocabulary.s_i_user) - user(Vocabulary.s_i_user), + @Individual(iri = Vocabulary.s_i_RM_USER) + user(SecurityConstants.user, Vocabulary.s_i_RM_USER), @Individual(iri = Vocabulary.s_i_impersonate_role) - impersonate(Vocabulary.s_i_impersonate_role), + impersonate(SecurityConstants.impersonate, Vocabulary.s_i_impersonate_role), @Individual(iri = Vocabulary.s_i_delete_all_records_role) - deleteAllRecords(Vocabulary.s_i_delete_all_records_role), + deleteAllRecords(SecurityConstants.deleteAllRecords, Vocabulary.s_i_delete_all_records_role), @Individual(iri = Vocabulary.s_i_view_all_records_role) - viewAllRecords(Vocabulary.s_i_view_all_records_role), + viewAllRecords(SecurityConstants.viewAllRecords, Vocabulary.s_i_view_all_records_role), @Individual(iri = Vocabulary.s_i_edit_all_records_role) - editAllRecords(Vocabulary.s_i_edit_all_records_role), + editAllRecords(SecurityConstants.editAllRecords, Vocabulary.s_i_edit_all_records_role), @Individual(iri = Vocabulary.s_i_delete_organization_records_role) - deleteOrganizationRecords(Vocabulary.s_i_delete_organization_records_role), + deleteOrganizationRecords(SecurityConstants.deleteOrganizationRecords, Vocabulary.s_i_delete_organization_records_role), @Individual(iri = Vocabulary.s_i_view_organization_records_role) - viewOrganizationRecords(Vocabulary.s_i_view_organization_records_role), + viewOrganizationRecords(SecurityConstants.viewOrganizationRecords, Vocabulary.s_i_view_organization_records_role), @Individual(iri = Vocabulary.s_i_edit_organization_records_role) - editOrganizationRecords(Vocabulary.s_i_edit_organization_records_role), + editOrganizationRecords(SecurityConstants.editOrganizationRecords, Vocabulary.s_i_edit_organization_records_role), @Individual(iri = Vocabulary.s_i_edit_users_role) - editUsers(Vocabulary.s_i_edit_users_role), + editUsers(SecurityConstants.editUsers, Vocabulary.s_i_edit_users_role), @Individual(iri = Vocabulary.s_i_complete_records_role) - completeRecords(Vocabulary.s_i_complete_records_role), + completeRecords(SecurityConstants.completeRecords, Vocabulary.s_i_complete_records_role), @Individual(iri = Vocabulary.s_i_reject_records_role) - rejectRecords(Vocabulary.s_i_reject_records_role), + rejectRecords(SecurityConstants.rejectRecords, Vocabulary.s_i_reject_records_role), @Individual(iri = Vocabulary.s_i_publish_records_role) - publishRecords(Vocabulary.s_i_publish_records_role), + publishRecords(SecurityConstants.publishRecords ,Vocabulary.s_i_publish_records_role), @Individual(iri = Vocabulary.s_i_import_codelists_role) - importCodelists(Vocabulary.s_i_import_codelists_role); + importCodelists(SecurityConstants.importCodelists, Vocabulary.s_i_import_codelists_role); private final String iri; - Role(String iri) { + public final String roleName; + + Role(String roleName, String iri) { this.iri = iri; + this.roleName = roleName; } - + @JsonValue + public String getRoleName(){ + return roleName; + } public String getIri() { return iri; @@ -86,7 +92,7 @@ public static Role fromIri(String iri) { */ public static Role fromName(String name) { for (Role r : values()) { - if (r.name().equalsIgnoreCase(name)) { + if (r.roleName.equalsIgnoreCase(name)) { return r; } } diff --git a/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java b/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java index f058af50..411f5f32 100644 --- a/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java +++ b/src/main/java/cz/cvut/kbss/study/model/RoleGroup.java @@ -53,7 +53,7 @@ public void setRoles(Set roles) { } public void generateUri() { - this.uri = URI.create(Constants.BASE_URI + "sdfsf"); + this.uri = URI.create(Constants.BASE_URI + name); } @Override diff --git a/src/main/java/cz/cvut/kbss/study/model/User.java b/src/main/java/cz/cvut/kbss/study/model/User.java index 498c8c28..dd44586d 100644 --- a/src/main/java/cz/cvut/kbss/study/model/User.java +++ b/src/main/java/cz/cvut/kbss/study/model/User.java @@ -57,7 +57,7 @@ public class User implements HasDerivableUri, Serializable { @OWLObjectProperty(iri = Vocabulary.s_p_is_member_of, fetch = FetchType.EAGER) private Institution institution; - @OWLObjectProperty(iri = Vocabulary.s_p_has_role_group) + @OWLObjectProperty(iri = Vocabulary.s_p_has_role_group, fetch = FetchType.EAGER) private RoleGroup roleGroup; public User() { diff --git a/src/main/java/cz/cvut/kbss/study/persistence/dao/UserDao.java b/src/main/java/cz/cvut/kbss/study/persistence/dao/UserDao.java index ea2a51ce..ed06a0f0 100644 --- a/src/main/java/cz/cvut/kbss/study/persistence/dao/UserDao.java +++ b/src/main/java/cz/cvut/kbss/study/persistence/dao/UserDao.java @@ -87,7 +87,7 @@ public int getNumberOfInvestigators() { .setParameter("typeUser", URI.create(Vocabulary.s_c_Person)) .setParameter("hasRoleGroup", URI.create(Vocabulary.s_p_has_role_group)) .setParameter("hasRole", URI.create(Vocabulary.s_p_has_role)) - .setParameter("typeAdmin", URI.create(Vocabulary.s_i_administrator)).getSingleResult() + .setParameter("typeAdmin", URI.create(Vocabulary.s_i_RM_ADMIN)).getSingleResult() ).intValue(); } } diff --git a/src/main/java/cz/cvut/kbss/study/security/CustomSwitchUserFilter.java b/src/main/java/cz/cvut/kbss/study/security/CustomSwitchUserFilter.java index fbc4654a..a34b7c52 100644 --- a/src/main/java/cz/cvut/kbss/study/security/CustomSwitchUserFilter.java +++ b/src/main/java/cz/cvut/kbss/study/security/CustomSwitchUserFilter.java @@ -15,7 +15,7 @@ public class CustomSwitchUserFilter extends SwitchUserFilter { @Override protected Authentication attemptSwitchUser(HttpServletRequest request) throws AuthenticationException { final Authentication switchTo = super.attemptSwitchUser(request); - if (switchTo.getAuthorities().stream().anyMatch(a -> Role.administrator.name().equals(a.getAuthority()))) { + if (switchTo.getAuthorities().stream().anyMatch(a -> Role.administrator.getRoleName().equals(a.getAuthority()))) { throw new BadRequestException("Cannot impersonate admin."); } return switchTo; diff --git a/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java b/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java index 0443e8e7..ea255f64 100644 --- a/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java +++ b/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java @@ -27,32 +27,32 @@ private SecurityConstants() { */ public static final int SESSION_TIMEOUT = 12 * 60 * 60; - public static final String user = "user"; + public static final String user = "ROLE_USER"; - public static final String administrator = "administrator"; + public static final String administrator = "ROLE_ADMIN"; public static final String impersonate = "impersonate"; - public static final String deleteAllRecords = "deleteAllRecords"; + public static final String deleteAllRecords = "delete-all-records"; - public static final String viewAllRecords = "viewAllRecords"; + public static final String viewAllRecords = "view-all-records"; - public static final String editAllRecords = "editAllRecords"; + public static final String editAllRecords = "edit-all-records"; - public static final String deleteOrganizationRecords = "deleteOrganizationRecords"; + public static final String deleteOrganizationRecords = "delete-organization-records"; - public static final String viewOrganizationRecords = "viewOrganizationRecords"; + public static final String viewOrganizationRecords = "view-organization-records"; - public static final String editOrganizationRecords = "editOrganizationRecords"; + public static final String editOrganizationRecords = "edit-organization-records"; - public static final String editUsers = "editUsers"; + public static final String editUsers = "edit-users"; - public static final String completeRecords = "completeRecords"; + public static final String completeRecords = "complete-records"; - public static final String rejectRecords = "rejectRecords"; + public static final String rejectRecords = "reject-records"; - public static final String publishRecords = "publishRecords"; + public static final String publishRecords = "publish-records"; - public static final String importCodelists = "importCodelists"; + public static final String importCodelists = "import-codelists"; } diff --git a/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java b/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java index 2c7b3897..b02a9f21 100644 --- a/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java +++ b/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java @@ -38,7 +38,7 @@ public UserDetails(User user, Collection authorities) { private void resolveRoles() { authorities.addAll( user.getRoleGroup().getRoles().stream() - .map(r -> new SimpleGrantedAuthority(r.name())) + .map(r -> new SimpleGrantedAuthority(r.getRoleName())) .toList()); authorities.add(new SimpleGrantedAuthority(Role.user.name())); } diff --git a/src/main/resources/model.ttl b/src/main/resources/model.ttl index 1915480e..0f889f4f 100644 --- a/src/main/resources/model.ttl +++ b/src/main/resources/model.ttl @@ -66,10 +66,6 @@ rm:has-question rdf:type owl:ObjectProperty ; rm:is-member-of rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rm:relates-to . -### http://onto.fel.cvut.cz/ontologies/record-manager/role-group -rm:role-group rdf:type owl:ObjectProperty ; - rdfs:subPropertyOf rm:relates-to . - ### http://onto.fel.cvut.cz/ontologies/record-manager/relates-to rm:relates-to rdf:type owl:ObjectProperty . @@ -79,16 +75,19 @@ rm:relates-to rdf:type owl:ObjectProperty . rm:was-treated-at rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rm:relates-to . + ### http://onto.fel.cvut.cz/ontologies/record-manager/has-phase rm:has-phase rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rdf:type ; rdfs:label "has phase"@en . + ### http://onto.fel.cvut.cz/ontologies/record-manager/has-role-group rm:has-role-group rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rm:relates-to; rdfs:label "has role group"@en. + ### http://onto.fel.cvut.cz/ontologies/record-manager/has-role rm:has-role rdf:type owl:ObjectProperty ; rdfs:subPropertyOf rm:relates-to; @@ -156,17 +155,6 @@ rm:reject-reason rdf:type owl:DatatypeProperty . rm:action-history rdf:type owl:Class ; rdfs:label "ActionHistory"@en . - -### http://onto.fel.cvut.cz/ontologies/record-manager/administrator-role-group -rm:administrator-role-group rdf:type owl:Class ; - rdfs:label "Administrator"@en . - - -### http://onto.fel.cvut.cz/ontologies/record-manager/doctor-role-group -rm:doctor-role-group rdf:type owl:Class ; - rdfs:label "Doctor"@en . - - ### http://onto.fel.cvut.cz/ontologies/record-manager/institution rm:institution rdf:type owl:Class ; rdfs:label "Institution"@en . @@ -229,12 +217,12 @@ rm:role-group rdf:type owl:Class; ### http://onto.fel.cvut.cz/ontologies/record-manager/administrator ### TODO deprecated -rm:administrator rdf:type owl:NamedIndividual, rm:role ; +rm:RM_ADMIN rdf:type owl:NamedIndividual, rm:role ; rdfs:label "administrator"@en . ### http://onto.fel.cvut.cz/ontologies/record-manager/user ### TODO deprecated -rm:user rdf:type owl:NamedIndividual, rm:role ; +rm:RM_USER rdf:type owl:NamedIndividual, rm:role ; rdfs:label "user"@en . ### http://onto.fel.cvut.cz/ontologies/record-manager/complete-records-role diff --git a/src/test/java/cz/cvut/kbss/study/model/RoleTest.java b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java index 8f7209dc..e48a9985 100644 --- a/src/test/java/cz/cvut/kbss/study/model/RoleTest.java +++ b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java @@ -1,5 +1,6 @@ package cz.cvut.kbss.study.model; +import cz.cvut.kbss.study.security.SecurityConstants; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; @@ -7,7 +8,7 @@ class RoleTest { @Test void fromIriReturnsCorrectRole() { - assertEquals(Role.administrator, Role.fromIri(Vocabulary.s_i_administrator)); + assertEquals(Role.administrator, Role.fromIri(Vocabulary.s_i_RM_ADMIN)); assertEquals(Role.viewAllRecords, Role.fromIri(Vocabulary.s_i_view_all_records_role)); } @@ -23,14 +24,14 @@ void fromIriThrowsExceptionForUnknownIri() { @Test void fromNameReturnsCorrectRole() { - assertEquals(Role.administrator, Role.fromName("administrator")); - assertEquals(Role.viewAllRecords, Role.fromName("viewAllRecords")); + assertEquals(Role.administrator, Role.fromName(SecurityConstants.administrator)); + assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords)); } @Test void fromNameIsCaseInsensitive() { - assertEquals(Role.administrator, Role.fromName("ADMINISTRATOR")); - assertEquals(Role.viewAllRecords, Role.fromName("VIEWALLRECORDS")); + assertEquals(Role.administrator, Role.fromName(SecurityConstants.administrator.toLowerCase())); + assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords.toUpperCase())); } @Test @@ -45,19 +46,19 @@ void fromNameThrowsExceptionForUnknownName() { @Test void fromIriOrNameReturnsRoleByIri() { - assertEquals(Role.administrator, Role.fromIriOrName(Vocabulary.s_i_administrator)); + assertEquals(Role.administrator, Role.fromIriOrName(Vocabulary.s_i_RM_ADMIN)); assertEquals(Role.viewAllRecords, Role.fromIriOrName(Vocabulary.s_i_view_all_records_role)); } @Test void fromIriOrNameReturnsRoleByName() { - assertEquals(Role.administrator, Role.fromIriOrName("administrator")); - assertEquals(Role.viewAllRecords, Role.fromIriOrName("viewAllRecords")); + assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.administrator)); + assertEquals(Role.viewAllRecords, Role.fromIriOrName(SecurityConstants.viewAllRecords)); } @Test void fromIriOrNameIsCaseInsensitiveForName() { - assertEquals(Role.administrator, Role.fromIriOrName("ADMINISTRATOR")); + assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.administrator.toLowerCase())); } @Test diff --git a/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java b/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java index 450b9f72..2c466d60 100644 --- a/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java +++ b/src/test/java/cz/cvut/kbss/study/persistence/dao/PatientRecordDaoTest.java @@ -64,7 +64,6 @@ public class PatientRecordDaoTest extends BaseDaoTestRunner { public void setUp() { this.roleGroupAdmin = Generator.generateRoleGroupWithRoles(Role.administrator); transactional(() -> roleGroupDao.persist(roleGroupAdmin)); - int a =4; } @Test From 4d53a143e72a15d303fd0ced97c3d3757f314ccb Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Mon, 30 Sep 2024 12:47:50 +0200 Subject: [PATCH 09/17] [kbss-cvut/record-manager-ui#202] Rename security constants --- .../study/config/OAuth2SecurityConfig.java | 2 +- .../java/cz/cvut/kbss/study/model/Role.java | 4 +-- .../study/rest/ActionHistoryController.java | 4 +-- .../kbss/study/rest/FormGenController.java | 2 +- .../study/rest/InstitutionController.java | 16 +++++------ .../kbss/study/rest/OidcUserController.java | 12 ++++---- .../study/rest/PatientRecordController.java | 14 +++++----- .../kbss/study/rest/RoleGroupController.java | 4 +-- .../kbss/study/rest/StatisticsController.java | 4 +-- .../cvut/kbss/study/rest/UserController.java | 24 ++++++++-------- .../study/security/SecurityConstants.java | 28 +++++++++---------- .../cz/cvut/kbss/study/model/RoleTest.java | 8 +++--- .../service/security/SecurityUtilsTest.java | 5 ++-- .../OidcGrantedAuthoritiesExtractorTest.java | 6 ++-- 14 files changed, 66 insertions(+), 67 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java b/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java index 34be7fdf..c791dfcb 100644 --- a/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java +++ b/src/main/java/cz/cvut/kbss/study/config/OAuth2SecurityConfig.java @@ -79,7 +79,7 @@ private Converter grantedAuthoritiesExtractor( assert extractedRoles != null; final Set authorities = new HashSet<>(extractedRoles); // Add default role if it is not present - authorities.add(new SimpleGrantedAuthority(SecurityConstants.user)); + authorities.add(new SimpleGrantedAuthority(SecurityConstants.ROLE_USER)); return new JwtAuthenticationToken(source, authorities); }; } diff --git a/src/main/java/cz/cvut/kbss/study/model/Role.java b/src/main/java/cz/cvut/kbss/study/model/Role.java index 37bf8dab..988cbe26 100644 --- a/src/main/java/cz/cvut/kbss/study/model/Role.java +++ b/src/main/java/cz/cvut/kbss/study/model/Role.java @@ -8,10 +8,10 @@ public enum Role { // TODO deprecated -- should be removed. @Individual(iri=Vocabulary.s_i_RM_ADMIN) - administrator(SecurityConstants.administrator, Vocabulary.s_i_RM_ADMIN), + administrator(SecurityConstants.ROLE_ADMIN, Vocabulary.s_i_RM_ADMIN), // TODO deprecated -- should be removed. @Individual(iri = Vocabulary.s_i_RM_USER) - user(SecurityConstants.user, Vocabulary.s_i_RM_USER), + user(SecurityConstants.ROLE_USER, Vocabulary.s_i_RM_USER), @Individual(iri = Vocabulary.s_i_impersonate_role) impersonate(SecurityConstants.impersonate, Vocabulary.s_i_impersonate_role), diff --git a/src/main/java/cz/cvut/kbss/study/rest/ActionHistoryController.java b/src/main/java/cz/cvut/kbss/study/rest/ActionHistoryController.java index e460f51d..f6a32a4e 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/ActionHistoryController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/ActionHistoryController.java @@ -54,7 +54,7 @@ public void create(@RequestBody ActionHistory actionHistory) { } } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getActions(@RequestParam(value = "author", required = false) String authorUsername, @RequestParam(value = "type", required = false) String type, @@ -73,7 +73,7 @@ public List getActions(@RequestParam(value = "author", required = return result.getContent(); } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @GetMapping(value = "/{key}", produces = MediaType.APPLICATION_JSON_VALUE) public ActionHistory getByKey(@PathVariable("key") String key) { final ActionHistory action = actionHistoryService.findByKey(key); diff --git a/src/main/java/cz/cvut/kbss/study/rest/FormGenController.java b/src/main/java/cz/cvut/kbss/study/rest/FormGenController.java index 91b5c3bf..dc7628f0 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/FormGenController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/FormGenController.java @@ -9,7 +9,7 @@ import org.springframework.web.bind.annotation.*; @RestController -@PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") +@PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_USER + "')") @RequestMapping("/formGen") public class FormGenController extends BaseController { diff --git a/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java b/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java index 1a3bbc9a..e64abc18 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/InstitutionController.java @@ -30,7 +30,7 @@ import static cz.cvut.kbss.study.rest.util.RecordFilterMapper.constructRecordFilter; @RestController -@PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") +@PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_USER + "')") @RequestMapping("/institutions") public class InstitutionController extends BaseController { @@ -44,7 +44,7 @@ public InstitutionController(InstitutionService institutionService, this.recordService = recordService; } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getAllInstitutions() { final List institutions = institutionService.findAll(); @@ -52,8 +52,8 @@ public List getAllInstitutions() { return institutions; } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') " + - "or hasAuthority('" + SecurityConstants.user + "') and @securityUtils.isMemberOfInstitution(#key)") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') " + + "or hasAuthority('" + SecurityConstants.ROLE_USER + "') and @securityUtils.isMemberOfInstitution(#key)") @GetMapping(value = "/{key}", produces = MediaType.APPLICATION_JSON_VALUE) public Institution findByKey(@PathVariable("key") String key) { return findInternal(key); @@ -67,7 +67,7 @@ private Institution findInternal(String key) { return result; } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or @securityUtils.isRecordInUsersInstitution(#key)") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isRecordInUsersInstitution(#key)") @GetMapping(value = "/{key}/patients", produces = MediaType.APPLICATION_JSON_VALUE) public List getTreatedPatientRecords(@PathVariable("key") String key) { final Institution inst = findInternal(key); @@ -75,7 +75,7 @@ public List getTreatedPatientRecords(@PathVariable("key") Stri return recordService.findAll(constructRecordFilter("institution", key), Pageable.unpaged()).getContent(); } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.CREATED) public ResponseEntity createInstitution(@RequestBody Institution institution) { @@ -88,7 +88,7 @@ public ResponseEntity createInstitution(@RequestBody Institution instituti return new ResponseEntity<>(headers, HttpStatus.CREATED); } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @PutMapping(value = "/{key}", consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void updateInstitution(@PathVariable("key") String key, @RequestBody Institution institution) { @@ -104,7 +104,7 @@ public void updateInstitution(@PathVariable("key") String key, @RequestBody Inst } } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @DeleteMapping(value = "/{key}") @ResponseStatus(HttpStatus.NO_CONTENT) public void deleteInstitution(@PathVariable("key") String key) { diff --git a/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java b/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java index f757a2d3..85d05b22 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java @@ -38,14 +38,14 @@ public OidcUserController(UserService userService, InstitutionService institutio this.institutionService = institutionService; } - @PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_USER + "')") @GetMapping(value = "/current", produces = MediaType.APPLICATION_JSON_VALUE) public User getCurrent() { return userService.getCurrentUser(); } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name or " + - "hasAuthority('" + SecurityConstants.user + "') and @securityUtils.areFromSameInstitution(#username)") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name or " + + "hasAuthority('" + SecurityConstants.ROLE_USER + "') and @securityUtils.areFromSameInstitution(#username)") @GetMapping(value = "/{username}", produces = MediaType.APPLICATION_JSON_VALUE) public User getByUsername(@PathVariable("username") String username) { final User user = userService.findByUsername(username); @@ -56,14 +56,14 @@ public User getByUsername(@PathVariable("username") String username) { } @PreAuthorize( - "hasAuthority('" + SecurityConstants.administrator + "') " + - "or hasAuthority('" + SecurityConstants.administrator + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") + "hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') " + + "or hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getUsers(@RequestParam(value = "institution", required = false) String institutionKey) { return institutionKey != null ? getByInstitution(institutionKey) : userService.findAll(); } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name") @PutMapping(value = "/{username}", consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void updateUser(@PathVariable("username") String username, @RequestBody User user, diff --git a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java index 632d2e03..ad216c4e 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/PatientRecordController.java @@ -42,7 +42,7 @@ import java.util.stream.Stream; @RestController -@PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") +@PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_USER + "')") @RequestMapping("/records") public class PatientRecordController extends BaseController { @@ -70,7 +70,7 @@ public PatientRecordController(PatientRecordService recordService, ApplicationEv this.publishRecordsService = publishRecordsService; } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #institutionKey==null or @securityUtils.isMemberOfInstitution(#institutionKey)") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or #institutionKey==null or @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getRecords( @RequestParam(value = "institution", required = false) String institutionKey, @@ -79,7 +79,7 @@ public List getRecords( Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); boolean hasAdminRole = authentication.getAuthorities().stream() - .anyMatch(authority -> authority.getAuthority().equals(SecurityConstants.administrator)); + .anyMatch(authority -> authority.getAuthority().equals(SecurityConstants.ROLE_ADMIN)); if (!hasAdminRole && institutionKey == null) { throw new ValidationException("record.save-error.user-not-assigned-to-institution", @@ -91,7 +91,7 @@ public List getRecords( return result.getContent(); } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(value="used-record-phases", produces = MediaType.APPLICATION_JSON_VALUE) public Set getUsedRecordPhases(@RequestParam(value = "institution", required = false) String institutionKey){ return recordService.findUsedRecordPhases(); @@ -99,7 +99,7 @@ public Set getUsedRecordPhases(@RequestParam(value = "institution", @PreAuthorize( - "hasAuthority('" + SecurityConstants.administrator + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") + "hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(value = "/export", produces = {MediaType.APPLICATION_JSON_VALUE, Constants.MEDIA_TYPE_EXCEL}) public ResponseEntity exportRecords( @RequestParam(name = "institution", required = false) String institutionKey, @@ -158,7 +158,7 @@ public ResponseEntity exportRecordsExcel(MultiValueMap createRecord(@RequestBody PatientRecord record) { } @PreAuthorize( - "hasAuthority('" + SecurityConstants.administrator + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") + "hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or @securityUtils.isMemberOfInstitution(#institutionKey)") @PostMapping(value = "/publish", produces = {MediaType.APPLICATION_JSON_VALUE}) public RecordImportResult publishRecords( @RequestParam(name = "institution", required = false) String institutionKey, diff --git a/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java b/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java index 494e348f..4af3e733 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java @@ -29,13 +29,13 @@ public RoleGroupController(RoleGroupService roleGroupService) { this.roleGroupService = roleGroupService; } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getRoleGroups() { return roleGroupService.findAll(); } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @GetMapping(value = "/{name}",produces = MediaType.APPLICATION_JSON_VALUE) public RoleGroup findByName(@PathVariable("name") String name) { RoleGroup result = roleGroupService.findByName(name); diff --git a/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java b/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java index 742d0527..4a99fd35 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/StatisticsController.java @@ -11,7 +11,7 @@ import java.util.HashMap; import java.util.Map; -@PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") +@PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @RestController @RequestMapping("/statistics") public class StatisticsController extends BaseController { @@ -22,7 +22,7 @@ public StatisticsController(StatisticsService statisticsService) { this.statisticsService = statisticsService; } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public Map getStatistics() { Map data = new HashMap<>(); diff --git a/src/main/java/cz/cvut/kbss/study/rest/UserController.java b/src/main/java/cz/cvut/kbss/study/rest/UserController.java index c94274b1..36ea699f 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/UserController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/UserController.java @@ -47,8 +47,8 @@ public UserController(UserService userService, InstitutionService institutionSer this.institutionService = institutionService; } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name or " + - "hasAuthority('" + SecurityConstants.user + "') and @securityUtils.areFromSameInstitution(#username)") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name or " + + "hasAuthority('" + SecurityConstants.ROLE_USER + "') and @securityUtils.areFromSameInstitution(#username)") @GetMapping(value = "/{username}", produces = MediaType.APPLICATION_JSON_VALUE) public User getByUsername(@PathVariable("username") String username) { final User user = userService.findByUsername(username); @@ -58,13 +58,13 @@ public User getByUsername(@PathVariable("username") String username) { return user; } - @PreAuthorize("hasAuthority('" + SecurityConstants.user + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_USER + "')") @GetMapping(value = "/current", produces = MediaType.APPLICATION_JSON_VALUE) public User getCurrent() { return userService.getCurrentUser(); } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE) public ResponseEntity create(@RequestBody User user) { userService.persist(user); @@ -77,8 +77,8 @@ public ResponseEntity create(@RequestBody User user) { } @PreAuthorize( - "hasAuthority('" + SecurityConstants.administrator + "') " + - "or hasAuthority('" + SecurityConstants.user + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") + "hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') " + + "or hasAuthority('" + SecurityConstants.ROLE_USER + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getUsers(@RequestParam(value = "institution", required = false) String institutionKey) { return institutionKey != null ? getByInstitution(institutionKey) : userService.findAll(); @@ -90,7 +90,7 @@ private List getByInstitution(String institutionKey) { return userService.findByInstitution(institution); } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @DeleteMapping(value = "/{username}") @ResponseStatus(HttpStatus.NO_CONTENT) public void removeUser(@PathVariable("username") String username) { @@ -101,7 +101,7 @@ public void removeUser(@PathVariable("username") String username) { } } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name") @PutMapping(value = "/{username}", consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void updateUser(@PathVariable("username") String username, @RequestBody User user, @@ -117,7 +117,7 @@ public void updateUser(@PathVariable("username") String username, @RequestBody U } } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "') or #username == authentication.name") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') or #username == authentication.name") @PutMapping(value = "/{username}/password-change", consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void updatePassword(@PathVariable("username") String username, @RequestBody Map password, @@ -147,7 +147,7 @@ public void resetPassword(@RequestBody String emailAddress) { } } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @GetMapping(value = "/generate-username/{usernamePrefix}", produces = MediaType.TEXT_PLAIN_VALUE) public String generateUsername(@PathVariable(value = "usernamePrefix") String usernamePrefix) { return userService.generateUsername(usernamePrefix); @@ -173,7 +173,7 @@ public void changePasswordByToken(@RequestBody Map data) { } } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @PutMapping(value = "/send-invitation/{username}") @ResponseStatus(HttpStatus.NO_CONTENT) public void sendInvitation(@PathVariable(value = "username") String username) { @@ -188,7 +188,7 @@ public void sendInvitation(@PathVariable(value = "username") String username) { } } - @PreAuthorize("hasAuthority('" + SecurityConstants.administrator + "')") + @PreAuthorize("hasAuthority('" + SecurityConstants.ROLE_ADMIN + "')") @PostMapping(value = "/send-invitation/delete", consumes = MediaType.TEXT_PLAIN_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void deleteInvitationOption(@RequestBody String username) { diff --git a/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java b/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java index ea255f64..2beef27f 100644 --- a/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java +++ b/src/main/java/cz/cvut/kbss/study/security/SecurityConstants.java @@ -27,32 +27,32 @@ private SecurityConstants() { */ public static final int SESSION_TIMEOUT = 12 * 60 * 60; - public static final String user = "ROLE_USER"; + public static final String ROLE_USER = "ROLE_USER"; - public static final String administrator = "ROLE_ADMIN"; + public static final String ROLE_ADMIN = "ROLE_ADMIN"; - public static final String impersonate = "impersonate"; + public static final String impersonate = "rm-impersonate"; - public static final String deleteAllRecords = "delete-all-records"; + public static final String deleteAllRecords = "rm-delete-all-records"; - public static final String viewAllRecords = "view-all-records"; + public static final String viewAllRecords = "rm-view-all-records"; - public static final String editAllRecords = "edit-all-records"; + public static final String editAllRecords = "rm-edit-all-records"; - public static final String deleteOrganizationRecords = "delete-organization-records"; + public static final String deleteOrganizationRecords = "rm-delete-organization-records"; - public static final String viewOrganizationRecords = "view-organization-records"; + public static final String viewOrganizationRecords = "rm-view-organization-records"; - public static final String editOrganizationRecords = "edit-organization-records"; + public static final String editOrganizationRecords = "rm-edit-organization-records"; - public static final String editUsers = "edit-users"; + public static final String editUsers = "rm-edit-users"; - public static final String completeRecords = "complete-records"; + public static final String completeRecords = "rm-complete-records"; - public static final String rejectRecords = "reject-records"; + public static final String rejectRecords = "rm-reject-records"; - public static final String publishRecords = "publish-records"; + public static final String publishRecords = "rm-publish-records"; - public static final String importCodelists = "import-codelists"; + public static final String importCodelists = "rm-import-codelists"; } diff --git a/src/test/java/cz/cvut/kbss/study/model/RoleTest.java b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java index e48a9985..2346223a 100644 --- a/src/test/java/cz/cvut/kbss/study/model/RoleTest.java +++ b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java @@ -24,13 +24,13 @@ void fromIriThrowsExceptionForUnknownIri() { @Test void fromNameReturnsCorrectRole() { - assertEquals(Role.administrator, Role.fromName(SecurityConstants.administrator)); + assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN)); assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords)); } @Test void fromNameIsCaseInsensitive() { - assertEquals(Role.administrator, Role.fromName(SecurityConstants.administrator.toLowerCase())); + assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN.toLowerCase())); assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords.toUpperCase())); } @@ -52,13 +52,13 @@ void fromIriOrNameReturnsRoleByIri() { @Test void fromIriOrNameReturnsRoleByName() { - assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.administrator)); + assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN)); assertEquals(Role.viewAllRecords, Role.fromIriOrName(SecurityConstants.viewAllRecords)); } @Test void fromIriOrNameIsCaseInsensitiveForName() { - assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.administrator.toLowerCase())); + assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN.toLowerCase())); } @Test diff --git a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java index 00c37805..8709e87a 100644 --- a/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java +++ b/src/test/java/cz/cvut/kbss/study/service/security/SecurityUtilsTest.java @@ -7,7 +7,6 @@ import cz.cvut.kbss.study.model.Role; import cz.cvut.kbss.study.model.RoleGroup; import cz.cvut.kbss.study.model.User; -import cz.cvut.kbss.study.model.Vocabulary; import cz.cvut.kbss.study.persistence.dao.PatientRecordDao; import cz.cvut.kbss.study.persistence.dao.UserDao; import cz.cvut.kbss.study.security.SecurityConstants; @@ -93,7 +92,7 @@ void getCurrentUserRetrievesCurrentUserForOauthJwtAccessToken() { final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") - .claim("roles", List.of(SecurityConstants.user)) + .claim("roles", List.of(SecurityConstants.ROLE_USER)) .issuer("http://localhost:8080/termit") .subject(USERNAME) .claim("preferred_username", USERNAME) @@ -179,7 +178,7 @@ void getCurrentUserEnhancesRetrievedUserWithTypesCorrespondingToRolesSpecifiedIn final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") - .claim("roles", List.of(SecurityConstants.administrator)) + .claim("roles", List.of(SecurityConstants.ROLE_ADMIN)) .issuer("http://localhost:8080/termit") .subject(USERNAME) .claim("preferred_username", USERNAME) diff --git a/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java b/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java index da906cee..881a3320 100644 --- a/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java +++ b/src/test/java/cz/cvut/kbss/study/util/oidc/OidcGrantedAuthoritiesExtractorTest.java @@ -30,7 +30,7 @@ class OidcGrantedAuthoritiesExtractorTest { @Test void convertMapsTopLevelClaimWithRolesToGrantedAuthorities() { when(config.getConfig(ConfigParam.OIDC_ROLE_CLAIM)).thenReturn("roles"); - final List roles = List.of(SecurityConstants.administrator, SecurityConstants.user); + final List roles = List.of(SecurityConstants.ROLE_ADMIN, SecurityConstants.ROLE_USER); final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") @@ -51,7 +51,7 @@ void convertMapsTopLevelClaimWithRolesToGrantedAuthorities() { @Test void convertSupportsNestedRolesClaim() { when(config.getConfig(ConfigParam.OIDC_ROLE_CLAIM)).thenReturn("realm_access.roles"); - final List roles = List.of(SecurityConstants.administrator, SecurityConstants.user); + final List roles = List.of(SecurityConstants.ROLE_ADMIN, SecurityConstants.ROLE_USER); final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") @@ -91,7 +91,7 @@ void convertThrowsIllegalArgumentExceptionWhenNestedRolesClaimIsNotList() { final Jwt token = Jwt.withTokenValue("abcdef12345") .header("alg", "RS256") .header("typ", "JWT") - .claim("realm_access", Map.of("roles", Map.of("notlist", SecurityConstants.user))) + .claim("realm_access", Map.of("roles", Map.of("notlist", SecurityConstants.ROLE_USER))) .issuer("http://localhost:8080/termit") .subject("termit") .expiresAt(Instant.now().truncatedTo(ChronoUnit.SECONDS).plusSeconds(300)) From d5b29caccd3290668eb7425ff41d9067a0eda5e9 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Mon, 30 Sep 2024 16:32:38 +0200 Subject: [PATCH 10/17] [kbss-cvut/record-manager-ui#202] Fix RoleGroupController path --- .../java/cz/cvut/kbss/study/rest/RoleGroupController.java | 2 +- .../cz/cvut/kbss/study/rest/RoleGroupControllerTest.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java b/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java index 4af3e733..04afc860 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/RoleGroupController.java @@ -19,7 +19,7 @@ @ConditionalOnProperty(prefix = "security", name = "provider", havingValue = "internal", matchIfMissing = true) @RestController -@RequestMapping("/roleGroup") +@RequestMapping("/roleGroups") public class RoleGroupController extends BaseController{ private final RoleGroupService roleGroupService; diff --git a/src/test/java/cz/cvut/kbss/study/rest/RoleGroupControllerTest.java b/src/test/java/cz/cvut/kbss/study/rest/RoleGroupControllerTest.java index d27c1ca7..0ad2c425 100644 --- a/src/test/java/cz/cvut/kbss/study/rest/RoleGroupControllerTest.java +++ b/src/test/java/cz/cvut/kbss/study/rest/RoleGroupControllerTest.java @@ -47,7 +47,7 @@ public void testGetRoleGroups() throws Exception { when(roleGroupServiceMock.findAll()).thenReturn(List.of(roleGroup)); - final MvcResult result = mockMvc.perform(get("/roleGroup/")).andReturn(); + final MvcResult result = mockMvc.perform(get("/roleGroups/")).andReturn(); final List body = objectMapper.readValue(result.getResponse().getContentAsString(), new TypeReference<>() { @@ -65,7 +65,7 @@ public void testFindByName() throws Exception { when(roleGroupServiceMock.findByName(roleName)).thenReturn(roleGroup); - final MvcResult result = mockMvc.perform(get("/roleGroup/" + roleName)).andReturn(); + final MvcResult result = mockMvc.perform(get("/roleGroups/" + roleName)).andReturn(); final RoleGroup body = objectMapper.readValue(result.getResponse().getContentAsString(), new TypeReference<>() { @@ -81,7 +81,7 @@ public void testFindByName_NotFound() throws Exception { when(roleGroupServiceMock.findByName(roleName)).thenReturn(null); - mockMvc.perform(get("/roleGroup/{name}", roleName) + mockMvc.perform(get("/roleGroups/{name}", roleName) .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isNotFound()); } From d52b1deea05bbe94e871085a712a92b3cf9448c2 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Tue, 1 Oct 2024 16:00:07 +0200 Subject: [PATCH 11/17] [kbss-cvut/record-manager-ui#202] FromIri and FromName dont' throw an exception if the provided role does not exist --- .../java/cz/cvut/kbss/study/model/Role.java | 69 +++++++++++-------- .../study/service/security/SecurityUtils.java | 7 +- .../cz/cvut/kbss/study/model/RoleTest.java | 50 +++----------- 3 files changed, 55 insertions(+), 71 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/model/Role.java b/src/main/java/cz/cvut/kbss/study/model/Role.java index 988cbe26..700b7734 100644 --- a/src/main/java/cz/cvut/kbss/study/model/Role.java +++ b/src/main/java/cz/cvut/kbss/study/model/Role.java @@ -3,11 +3,12 @@ import com.fasterxml.jackson.annotation.JsonValue; import cz.cvut.kbss.jopa.model.annotations.Individual; import cz.cvut.kbss.study.security.SecurityConstants; +import java.util.Optional; public enum Role { // TODO deprecated -- should be removed. - @Individual(iri=Vocabulary.s_i_RM_ADMIN) + @Individual(iri = Vocabulary.s_i_RM_ADMIN) administrator(SecurityConstants.ROLE_ADMIN, Vocabulary.s_i_RM_ADMIN), // TODO deprecated -- should be removed. @Individual(iri = Vocabulary.s_i_RM_USER) @@ -44,7 +45,7 @@ public enum Role { rejectRecords(SecurityConstants.rejectRecords, Vocabulary.s_i_reject_records_role), @Individual(iri = Vocabulary.s_i_publish_records_role) - publishRecords(SecurityConstants.publishRecords ,Vocabulary.s_i_publish_records_role), + publishRecords(SecurityConstants.publishRecords, Vocabulary.s_i_publish_records_role), @Individual(iri = Vocabulary.s_i_import_codelists_role) importCodelists(SecurityConstants.importCodelists, Vocabulary.s_i_import_codelists_role); @@ -59,7 +60,7 @@ public enum Role { } @JsonValue - public String getRoleName(){ + public String getRoleName() { return roleName; } @@ -68,52 +69,60 @@ public String getIri() { } /** - * Returns {@link Role} with the specified IRI. + * Retrieves a role based on its IRI. * - * @param iri role identifier - * @return matching {@code Role} - * @throws IllegalArgumentException When no matching role is found + *

This method iterates over all available roles and checks if any role's IRI + * matches the provided IRI string. If a match is found, the corresponding role + * is returned as an Optional. If no match is found, an empty Optional is returned.

+ * + * @param iri the IRI of the role to retrieve + * @return an Optional containing the corresponding Role if found, + * or an empty Optional if no matching role exists */ - public static Role fromIri(String iri) { + public static Optional fromIri(String iri) { for (Role r : values()) { if (r.getIri().equals(iri)) { - return r; + return Optional.of(r); } } - throw new IllegalArgumentException("Unknown role identifier '" + iri + "'."); + return Optional.empty(); } /** - * Returns {@link Role} with the specified constant name. + * Retrieves a role based on its role name. + * + *

This method iterates over all available roles and checks if any role's + * name matches the provided name string (case-insensitive). If a match is found, + * the corresponding role is returned as an Optional. If no match is found, + * an empty Optional is returned.

* - * @param name role name - * @return matching {@code Role} - * @throws IllegalArgumentException When no matching role is found + * @param name the name of the role to retrieve + * @return an Optional containing the corresponding Role if found, + * or an empty Optional if no matching role exists */ - public static Role fromName(String name) { + public static Optional fromName(String name) { for (Role r : values()) { if (r.roleName.equalsIgnoreCase(name)) { - return r; + return Optional.of(r); } } - throw new IllegalArgumentException("Unknown role '" + name + "'."); + return Optional.empty(); } /** - * Returns a {@link Role} with the specified IRI or constant name. - *

- * This function first tries to find the enum constant by IRI. If it is not found, constant name matching is - * attempted. + * Retrieves a role based on either its IRI or role name. * - * @param identification Constant IRI or name to find match by - * @return matching {@code Role} - * @throws IllegalArgumentException When no matching role is found + *

This method first attempts to find a role using the provided identification string + * as an IRI. If no role is found, it then attempts to find a role using the + * identification string as a role name. The first successful match will be returned + * as an Optional. If no match is found, an empty Optional is returned.

+ * + * @param identification the IRI or role name of the role to retrieve + * @return an Optional containing the corresponding Role if found, + * or an empty Optional if no matching role exists */ - public static Role fromIriOrName(String identification) { - try { - return fromIri(identification); - } catch (IllegalArgumentException e) { - return fromName(identification); - } + public static Optional fromIriOrName(String identification) { + Optional role = fromIri(identification); + return role.isPresent() ? role : fromName(identification); } } diff --git a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java index 587e144b..05288653 100644 --- a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java +++ b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java @@ -99,10 +99,13 @@ private User resolveAccountFromOAuthPrincipal(Jwt principal) { throw new NotFoundException( "User with username '" + userInfo.getPreferredUsername() + "' not found in repository."); } - RoleGroup roleGroup = new RoleGroup(); user.setRoleGroup(roleGroup); - roles.forEach(r -> roleGroup.addRole(Role.fromIriOrName(r))); + roles.stream() + .map(Role::fromIriOrName) + .filter(Optional::isPresent) + .map(Optional::get) + .forEach(roleGroup::addRole); return user; } diff --git a/src/test/java/cz/cvut/kbss/study/model/RoleTest.java b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java index 2346223a..46e4c901 100644 --- a/src/test/java/cz/cvut/kbss/study/model/RoleTest.java +++ b/src/test/java/cz/cvut/kbss/study/model/RoleTest.java @@ -8,65 +8,37 @@ class RoleTest { @Test void fromIriReturnsCorrectRole() { - assertEquals(Role.administrator, Role.fromIri(Vocabulary.s_i_RM_ADMIN)); - assertEquals(Role.viewAllRecords, Role.fromIri(Vocabulary.s_i_view_all_records_role)); + assertEquals(Role.administrator, Role.fromIri(Vocabulary.s_i_RM_ADMIN).get()); + assertEquals(Role.viewAllRecords, Role.fromIri(Vocabulary.s_i_view_all_records_role).get()); } - @Test - void fromIriThrowsExceptionForUnknownIri() { - String unknownIri = "unknown_iri"; - Exception exception = assertThrows(IllegalArgumentException.class, () -> { - Role.fromIri(unknownIri); - }); - assertEquals("Unknown role identifier '" + unknownIri + "'.", exception.getMessage()); - } - - @Test void fromNameReturnsCorrectRole() { - assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN)); - assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords)); + assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN).get()); + assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords).get()); } @Test void fromNameIsCaseInsensitive() { - assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN.toLowerCase())); - assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords.toUpperCase())); - } - - @Test - void fromNameThrowsExceptionForUnknownName() { - String unknownName = "unknown_role"; - Exception exception = assertThrows(IllegalArgumentException.class, () -> { - Role.fromName(unknownName); - }); - assertEquals("Unknown role '" + unknownName + "'.", exception.getMessage()); + assertEquals(Role.administrator, Role.fromName(SecurityConstants.ROLE_ADMIN.toLowerCase()).get()); + assertEquals(Role.viewAllRecords, Role.fromName(SecurityConstants.viewAllRecords.toUpperCase()).get()); } - @Test void fromIriOrNameReturnsRoleByIri() { - assertEquals(Role.administrator, Role.fromIriOrName(Vocabulary.s_i_RM_ADMIN)); - assertEquals(Role.viewAllRecords, Role.fromIriOrName(Vocabulary.s_i_view_all_records_role)); + assertEquals(Role.administrator, Role.fromIriOrName(Vocabulary.s_i_RM_ADMIN).get()); + assertEquals(Role.viewAllRecords, Role.fromIriOrName(Vocabulary.s_i_view_all_records_role).get()); } @Test void fromIriOrNameReturnsRoleByName() { - assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN)); - assertEquals(Role.viewAllRecords, Role.fromIriOrName(SecurityConstants.viewAllRecords)); + assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN).get()); + assertEquals(Role.viewAllRecords, Role.fromIriOrName(SecurityConstants.viewAllRecords).get()); } @Test void fromIriOrNameIsCaseInsensitiveForName() { - assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN.toLowerCase())); + assertEquals(Role.administrator, Role.fromIriOrName(SecurityConstants.ROLE_ADMIN.toLowerCase()).get()); } - @Test - void fromIriOrNameThrowsExceptionForUnknownIdentifier() { - String unknownIdentifier = "unknown_identifier"; - Exception exception = assertThrows(IllegalArgumentException.class, () -> { - Role.fromIriOrName(unknownIdentifier); - }); - assertEquals("Unknown role '" + unknownIdentifier + "'.", exception.getMessage()); - } } From fb316d7408001dc1e2657edba582746e8ef58496 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Tue, 1 Oct 2024 16:40:19 +0200 Subject: [PATCH 12/17] [kbss-cvut/record-manager-ui#202] Fix the bug while loading all users http-nio-8080-exec-7] WARN o.s.w.s.m.s.DefaultHandlerExceptionResolver - Resolved [org.springframework.http.converter.HttpMessageNotWritableException: Could not write JSON: Cannot invoke "cz.cvut.kbss.study.model.RoleGroup.getRoles()" because "this.roleGroup" is null] --- src/main/java/cz/cvut/kbss/study/model/User.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/cz/cvut/kbss/study/model/User.java b/src/main/java/cz/cvut/kbss/study/model/User.java index dd44586d..c8c55538 100644 --- a/src/main/java/cz/cvut/kbss/study/model/User.java +++ b/src/main/java/cz/cvut/kbss/study/model/User.java @@ -148,7 +148,7 @@ public void setRoleGroup(RoleGroup roleGroup) { * @return {@code true} if this is admin, {@code false} otherwise */ public boolean isAdmin() { - return roleGroup.getRoles().contains(Role.administrator); + return roleGroup != null && roleGroup.getRoles().contains(Role.administrator); } public String getToken() { From 4a87a5b554c7a86a05a174493ae49b46816b8700 Mon Sep 17 00:00:00 2001 From: Miroslav Blasko Date: Mon, 25 Nov 2024 14:42:58 +0100 Subject: [PATCH 13/17] [kbss-cvut/record-manager-ui#202] Add method to get current user without security context --- .../cz/cvut/kbss/study/service/UserService.java | 13 +++++++++++++ .../service/repository/RepositoryUserService.java | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/src/main/java/cz/cvut/kbss/study/service/UserService.java b/src/main/java/cz/cvut/kbss/study/service/UserService.java index af744aad..270bdcbe 100644 --- a/src/main/java/cz/cvut/kbss/study/service/UserService.java +++ b/src/main/java/cz/cvut/kbss/study/service/UserService.java @@ -9,8 +9,21 @@ public interface UserService extends BaseService { User findByUsername(String username); + /** + * Retrieve currently authenticated user. The returned User entity is populated + * with information from the security context. + * @return User currently authenticated user or null. + */ User getCurrentUser(); + /** + * Retrieve persisted user from the current authenticated user. The returned User + * entity is not populated with information from the security context. See + * {@link #getCurrentUser()} to retrieve security context as well. + * @return User currently authenticated user or null. + */ + User findCurrentUser(); + User findByEmail(String email); User findByToken(String token); diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java index be04c2b2..f003bcd0 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java @@ -76,6 +76,12 @@ public User getCurrentUser() { return securityUtils.getCurrentUser(); } + @Transactional(readOnly = true) + @Override + public User findCurrentUser() { + return userDao.findByUsername(securityUtils.getCurrentUser().getUsername()); + } + @Transactional(readOnly = true) @Override public List findByInstitution(Institution institution) { From c4ee42afdb0c5909ec1904fb3886c58583c998d7 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Fri, 6 Dec 2024 12:36:50 +0100 Subject: [PATCH 14/17] [kbss-cvut/record-manager-ui#202] Fix user.getRoleGroup() null --- .../java/cz/cvut/kbss/study/service/SystemInitializer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/service/SystemInitializer.java b/src/main/java/cz/cvut/kbss/study/service/SystemInitializer.java index 1bb6e4ae..0da6d97b 100644 --- a/src/main/java/cz/cvut/kbss/study/service/SystemInitializer.java +++ b/src/main/java/cz/cvut/kbss/study/service/SystemInitializer.java @@ -46,11 +46,11 @@ private void initializeSystem() { } private boolean noAdminExists() { - return userService.findAll().stream().filter( - user -> user.getRoleGroup().getRoles().contains(Role.administrator) - ).findAny().isEmpty(); + return userService.findAll().stream() + .noneMatch(user -> user.getRoleGroup() != null && user.getRoleGroup().getRoles().contains(Role.administrator)); } + private void addDefaultInstitution() { if (institutionService.findByName(INSTITUTION_NAME) == null) { final Institution institution = new Institution(); From 8a3c607191eeecebe56fb43c1358a319850ce344 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Fri, 6 Dec 2024 16:55:30 +0100 Subject: [PATCH 15/17] [kbss-cvut/record-manager-ui#202] Fix RoleGroup persistence context --- .../study/service/formgen/FormGenService.java | 10 +++++---- .../repository/RepositoryUserService.java | 4 ++-- .../study/service/security/SecurityUtils.java | 21 +++++++++++++++++++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/service/formgen/FormGenService.java b/src/main/java/cz/cvut/kbss/study/service/formgen/FormGenService.java index 0184ca4d..4822b5de 100644 --- a/src/main/java/cz/cvut/kbss/study/service/formgen/FormGenService.java +++ b/src/main/java/cz/cvut/kbss/study/service/formgen/FormGenService.java @@ -6,6 +6,7 @@ import cz.cvut.kbss.study.rest.dto.RawJson; import cz.cvut.kbss.study.rest.util.RestUtils; import cz.cvut.kbss.study.persistence.data.DataLoader; +import cz.cvut.kbss.study.service.UserService; import cz.cvut.kbss.study.service.security.SecurityUtils; import cz.cvut.kbss.study.util.ConfigParam; import cz.cvut.kbss.study.util.Utils; @@ -44,16 +45,17 @@ public class FormGenService { private final Environment environment; - private final SecurityUtils securityUtils; + private final UserService userService; public FormGenService(FormGenDao formGenDao, DataLoader dataLoader, Environment environment, - SecurityUtils securityUtils) { + SecurityUtils securityUtils, + UserService userService) { this.formGenDao = formGenDao; this.dataLoader = dataLoader; this.environment = environment; - this.securityUtils = securityUtils; + this.userService = userService; } @PostConstruct @@ -72,7 +74,7 @@ private void loadConfiguration() { */ public RawJson generateForm(PatientRecord record) { Objects.requireNonNull(record); - final User author = securityUtils.getCurrentUser(); + final User author = userService.findCurrentUser(); if (author.getInstitution() != null) { author.getInstitution().setMembers(null); } diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java index f003bcd0..2cfcda2f 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryUserService.java @@ -5,7 +5,6 @@ import cz.cvut.kbss.study.exception.ValidationException; import cz.cvut.kbss.study.model.Institution; import cz.cvut.kbss.study.model.User; -import cz.cvut.kbss.study.model.Vocabulary; import cz.cvut.kbss.study.persistence.dao.GenericDao; import cz.cvut.kbss.study.persistence.dao.PatientRecordDao; import cz.cvut.kbss.study.persistence.dao.UserDao; @@ -79,7 +78,8 @@ public User getCurrentUser() { @Transactional(readOnly = true) @Override public User findCurrentUser() { - return userDao.findByUsername(securityUtils.getCurrentUser().getUsername()); + String currentUserName = securityUtils.getCurrentUserUsername(); + return userDao.findByUsername(currentUserName); } @Transactional(readOnly = true) diff --git a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java index 05288653..35fcd15f 100644 --- a/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java +++ b/src/main/java/cz/cvut/kbss/study/service/security/SecurityUtils.java @@ -109,6 +109,27 @@ private User resolveAccountFromOAuthPrincipal(Jwt principal) { return user; } + /** + * Gets the currently authenticated user username. + * + * @return Current user username + */ + public String getCurrentUserUsername() { + final SecurityContext context = SecurityContextHolder.getContext(); + assert context != null; + final Object principal = context.getAuthentication().getPrincipal(); + if (principal instanceof Jwt) { + return resolveAccountUsernameFromOAuthPrincipal((Jwt) principal); + } else { + return context.getAuthentication().getName(); + } + } + + private String resolveAccountUsernameFromOAuthPrincipal(Jwt principal) { + final OidcUserInfo userInfo = new OidcUserInfo(principal.getClaims()); + return userDao.findByUsername(userInfo.getPreferredUsername()).getUsername(); + } + /** * Checks whether the current user is a member of a institution with the specified key. * From fad3222d7e52527c664bd3d54ac93574daffe723 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Wed, 11 Dec 2024 10:07:34 +0100 Subject: [PATCH 16/17] [kbss-cvut/record-manager-ui#202] Fix role name in resoleRoles --- .../java/cz/cvut/kbss/study/security/model/UserDetails.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java b/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java index b02a9f21..f170ac30 100644 --- a/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java +++ b/src/main/java/cz/cvut/kbss/study/security/model/UserDetails.java @@ -40,7 +40,7 @@ private void resolveRoles() { user.getRoleGroup().getRoles().stream() .map(r -> new SimpleGrantedAuthority(r.getRoleName())) .toList()); - authorities.add(new SimpleGrantedAuthority(Role.user.name())); + authorities.add(new SimpleGrantedAuthority(Role.user.getRoleName())); } public void eraseCredentials() { From 9c56d4988347a8e1acc7c274e05fbe312cc2fc44 Mon Sep 17 00:00:00 2001 From: Daniil Palagin Date: Wed, 11 Dec 2024 10:17:33 +0100 Subject: [PATCH 17/17] [kbss-cvut/record-manager-ui#202] Fix the access control to allow ROLE_USER access to institution retrieval --- src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java b/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java index 85d05b22..f863fa87 100644 --- a/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java +++ b/src/main/java/cz/cvut/kbss/study/rest/OidcUserController.java @@ -57,7 +57,7 @@ public User getByUsername(@PathVariable("username") String username) { @PreAuthorize( "hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') " + - "or hasAuthority('" + SecurityConstants.ROLE_ADMIN + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") + "or hasAuthority('" + SecurityConstants.ROLE_USER + "') and @securityUtils.isMemberOfInstitution(#institutionKey)") @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE) public List getUsers(@RequestParam(value = "institution", required = false) String institutionKey) { return institutionKey != null ? getByInstitution(institutionKey) : userService.findAll();