From 2111543027324689ab94e026f63c0990b1d73e61 Mon Sep 17 00:00:00 2001 From: florian Date: Mon, 2 Sep 2019 23:47:35 +0200 Subject: [PATCH] Add role checking method. --- .../controllers/ApplicationController.java | 14 +- .../entities/persistence/Application.java | 6 +- .../cerberus/services/ApplicationService.java | 6 + .../cerberus/services/SecurityService.java | 37 +++++ .../org/cerberus/services/UserService.java | 9 +- .../services/SecurityServiceTest.java | 138 ++++++++++++++++++ 6 files changed, 202 insertions(+), 8 deletions(-) create mode 100644 src/test/java/org/cerberus/services/SecurityServiceTest.java diff --git a/src/main/java/org/cerberus/controllers/ApplicationController.java b/src/main/java/org/cerberus/controllers/ApplicationController.java index b1205d7..0e439b4 100644 --- a/src/main/java/org/cerberus/controllers/ApplicationController.java +++ b/src/main/java/org/cerberus/controllers/ApplicationController.java @@ -4,13 +4,13 @@ import org.cerberus.entities.persistence.Application; import org.cerberus.entities.persistence.User; import org.cerberus.services.ApplicationService; import org.cerberus.services.SecurityService; -import org.springframework.web.bind.annotation.PostMapping; -import org.springframework.web.bind.annotation.RequestBody; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.bind.annotation.*; import java.security.Principal; +import static org.cerberus.core.constant.RoleSecurity.ADMIN; +import static org.cerberus.core.constant.RoleSecurity.MAINTAINER; + @RestController @RequestMapping("/api/applications") public class ApplicationController { @@ -28,4 +28,10 @@ public class ApplicationController { User user = securityService.getAdminUser(connectedUser); return applicationService.create(application, user); } + + @PutMapping + public Application update(@RequestBody Application application, Principal connectedUser) { + securityService.checkHasAnyRole(connectedUser, application, ADMIN, MAINTAINER); + return applicationService.update(application); + } } diff --git a/src/main/java/org/cerberus/entities/persistence/Application.java b/src/main/java/org/cerberus/entities/persistence/Application.java index d6d7f58..f275675 100644 --- a/src/main/java/org/cerberus/entities/persistence/Application.java +++ b/src/main/java/org/cerberus/entities/persistence/Application.java @@ -7,6 +7,8 @@ import java.util.LinkedList; import java.util.List; import java.util.UUID; +import static javax.persistence.CascadeType.REMOVE; + @Entity @Table(name="application") @Proxy(lazy = false) @@ -20,10 +22,10 @@ public class Application { @Column(nullable = false) private String serviceName; - @OneToMany(mappedBy = "application", cascade = CascadeType.ALL) + @OneToMany(mappedBy = "application", cascade = { REMOVE }) private List configurationFileList; - @OneToMany(mappedBy = "application", cascade = CascadeType.ALL) + @OneToMany(mappedBy = "application", cascade = { REMOVE }) private List administratorList; @PrePersist diff --git a/src/main/java/org/cerberus/services/ApplicationService.java b/src/main/java/org/cerberus/services/ApplicationService.java index ffc5301..026e951 100644 --- a/src/main/java/org/cerberus/services/ApplicationService.java +++ b/src/main/java/org/cerberus/services/ApplicationService.java @@ -40,4 +40,10 @@ public class ApplicationService { return application; } + + public Application update(Application application) { + applicationValidator.checkAllAttributsConstraints(application); + applicationRepository.save(application); + return application; + } } diff --git a/src/main/java/org/cerberus/services/SecurityService.java b/src/main/java/org/cerberus/services/SecurityService.java index 3175579..ea26476 100644 --- a/src/main/java/org/cerberus/services/SecurityService.java +++ b/src/main/java/org/cerberus/services/SecurityService.java @@ -1,12 +1,18 @@ package org.cerberus.services; import org.cerberus.core.exceptions.ForbiddenException; +import org.cerberus.entities.persistence.Application; import org.cerberus.entities.persistence.User; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; import java.security.Principal; +import java.util.Arrays; +import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; + +import static org.cerberus.core.constant.RoleSecurity.ADMIN; @Service public class SecurityService { @@ -29,6 +35,37 @@ public class SecurityService { return user.get(); } + /** + * Checks if the connectedUser has at least one of {@code roles} about the {@code application}. + * @param connectedUser The connected user. + * @param application The application about the user should have role. + * @param roles Allowed role to check. + */ + public void checkHasAnyRole(Principal connectedUser, Application application, String... roles) { + Optional user = getUserByPrincipal(connectedUser); + + List roleList = Arrays.stream(roles).collect(Collectors.toList()); + boolean userHasRole = false; + + if(user.isPresent()) { + // Admin is required ? + userHasRole = roleList.contains(ADMIN) && userService.isAdmin(user.get()); + + if(!userHasRole) { + // Application role required ? + userHasRole = userService.getApplicationRolesByEmail(user.get().getEmail()).stream() + .anyMatch(appRole -> + appRole.getApplication().getId().equals(application.getId()) + && roleList.contains(appRole.getRole().name()) + ); + } + } + + if(!userHasRole) { + throw new ForbiddenException("Illegal access attempt."); + } + } + public Optional getUserByPrincipal(final Principal pPrincipal) { Optional result = Optional.empty(); diff --git a/src/main/java/org/cerberus/services/UserService.java b/src/main/java/org/cerberus/services/UserService.java index 4936748..98ecd06 100644 --- a/src/main/java/org/cerberus/services/UserService.java +++ b/src/main/java/org/cerberus/services/UserService.java @@ -1,7 +1,7 @@ package org.cerberus.services; -import org.cerberus.core.constant.Role; import org.cerberus.core.config.security.CustomAuthenticationProvider; +import org.cerberus.core.constant.Role; import org.cerberus.core.constant.RoleSecurity; import org.cerberus.core.exceptions.BadRequestException; import org.cerberus.entities.dto.SignUpDTO; @@ -16,6 +16,7 @@ import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.stereotype.Service; import java.util.Collection; +import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -57,7 +58,7 @@ public class UserService { } Collection fetchGrantedAuthorities(User user) { - Collection grantedAuthorityCollection = userRepository.getApplicationRolesByEmail(user.getEmail()) + Collection grantedAuthorityCollection = getApplicationRolesByEmail(user.getEmail()) .stream() .map(ApplicationRole::getRole) .map(Role::name) @@ -71,6 +72,10 @@ public class UserService { return grantedAuthorityCollection; } + public List getApplicationRolesByEmail(String email) { + return userRepository.getApplicationRolesByEmail(email); + } + public void signUp(SignUpDTO inputData) { signUpValidator.checkAllAttributsConstraints(inputData); diff --git a/src/test/java/org/cerberus/services/SecurityServiceTest.java b/src/test/java/org/cerberus/services/SecurityServiceTest.java new file mode 100644 index 0000000..14de889 --- /dev/null +++ b/src/test/java/org/cerberus/services/SecurityServiceTest.java @@ -0,0 +1,138 @@ +package org.cerberus.services; + +import org.cerberus.core.constant.Role; +import org.cerberus.entities.persistence.Application; +import org.cerberus.entities.persistence.ApplicationRole; +import org.cerberus.entities.persistence.User; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import java.security.Principal; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.ThrowableAssert.catchThrowable; +import static org.cerberus.core.constant.Role.MAINTAINER; +import static org.cerberus.core.constant.Role.VIEWER; +import static org.cerberus.core.constant.RoleSecurity.ADMIN; +import static org.mockito.BDDMockito.given; + +@RunWith(MockitoJUnitRunner.class) +public class SecurityServiceTest { + + private SecurityService service; + @Mock + private UserService userService; + + @Before + public void before() { + service = new SecurityService(userService); + } + + @Test + public void checkHasAnyRole_should_throw_an_exception_if_no_user_is_connected() { + // when + Throwable throwable = catchThrowable(() -> service.checkHasAnyRole(null, new Application(), ADMIN)); + + // then + assertThat(throwable).isNotNull() + .extracting("message") + .containsExactly("Illegal access attempt."); + } + + @Test + public void checkHasAnyRole_should_throw_an_exception_if_user_has_no_role() { + // given + Principal principal = Mockito.mock(Principal.class); + given(principal.getName()).willReturn("name"); + + Application application = mockAllAndGetApp(false); + + // when + Throwable throwable = catchThrowable(() -> service.checkHasAnyRole(principal, application, ADMIN)); + + // then + assertThat(throwable).isNotNull() + .extracting("message") + .containsExactly("Illegal access attempt."); + } + + @Test + public void checkHasAnyRole_should_not_throw_any_exception_about_admin_role() { + // given + Principal principal = Mockito.mock(Principal.class); + given(principal.getName()).willReturn("name"); + + Application application = mockAllAndGetApp(true); + + // when + service.checkHasAnyRole(principal, application, ADMIN); + } + + @Test + public void checkHasAnyRole_should_not_throw_any_exception_about_some_roles() { + // given + Principal principal = Mockito.mock(Principal.class); + given(principal.getName()).willReturn("name"); + + Application application = mockAllAndGetApp(false, VIEWER); + + // when + service.checkHasAnyRole(principal, application, "VIEWER"); + } + + @Test + public void checkHasAnyRole_should_not_throw_any_exception_about_some_roles2() { + // given + Principal principal = Mockito.mock(Principal.class); + given(principal.getName()).willReturn("name"); + + Application application = mockAllAndGetApp(false, MAINTAINER); + + // when + service.checkHasAnyRole(principal, application, "ADMIN", "MAINTAINER"); + } + + + private Application mockAllAndGetApp(boolean isAdmin, Role... roles) { + List roleList = Arrays.stream(roles).map(Role::name).collect(Collectors.toList()); + + User user = new User(); + user.setEmail("email"); + given(userService.findByEmail("name")).willReturn(Optional.of(user)); + + Application application = new Application(); + application.setId(UUID.randomUUID()); + + if(isAdmin) { + if(!roleList.contains("ADMIN")) { + roleList.add("ADMIN"); + } + given(userService.isAdmin(user)).willReturn(true); + } + + List appRoleList = roleList.stream() + .map(roleStr -> { + try { + ApplicationRole appRole = new ApplicationRole(); + appRole.setRole(Role.valueOf(roleStr)); + appRole.setApplication(application); + return appRole; + } catch(IllegalArgumentException e) { + return null; + } + }) + .collect(Collectors.toList()); + given(userService.getApplicationRolesByEmail("email")).willReturn(appRoleList); + + return application; + } +}