diff --git a/server/src/main/java/access/api/RoleController.java b/server/src/main/java/access/api/RoleController.java index 7492d0f4..888c499e 100644 --- a/server/src/main/java/access/api/RoleController.java +++ b/server/src/main/java/access/api/RoleController.java @@ -13,6 +13,7 @@ import access.repository.ApplicationRepository; import access.repository.ApplicationUsageRepository; import access.repository.RoleRepository; +import access.security.RemoteUser; import access.security.UserPermissions; import access.validation.URLFormatValidator; import io.swagger.v3.oas.annotations.Parameter; @@ -22,6 +23,8 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; @@ -136,7 +139,10 @@ public ResponseEntity> search(@RequestParam(value = "query") String q } @PostMapping("") - public ResponseEntity newRole(@Validated @RequestBody Role role, @Parameter(hidden = true) User user) { + public ResponseEntity newRole(@Validated @RequestBody Role role, + @Parameter(hidden = true) User user, + @Parameter(hidden = true) @AuthenticationPrincipal RemoteUser remoteUser) { + //TODO differentiate between RemoteUser and User UserPermissions.assertAuthority(user, Authority.INSTITUTION_ADMIN); role.setShortName(GroupURN.sanitizeRoleShortName(role.getShortName())); role.setIdentifier(UUID.randomUUID().toString()); diff --git a/server/src/main/java/access/api/UserController.java b/server/src/main/java/access/api/UserController.java index 07815ff4..31304b0b 100644 --- a/server/src/main/java/access/api/UserController.java +++ b/server/src/main/java/access/api/UserController.java @@ -172,7 +172,9 @@ public ResponseEntity> logout(HttpServletRequest request) { @GetMapping("ms-accept-return/{manageId}/{userId}") public View msAcceptReturn(@PathVariable("manageId") String manageId, @PathVariable("userId") Long userId) { User user = userRepository.findById(userId).orElseThrow(() -> new NotFoundException("User not found")); - LOG.debug(String.format("Return from MS accept. User %s",user.getEduPersonPrincipalName())); + + LOG.info(String.format("Return from MS accept. User %s",user.getEduPersonPrincipalName())); + Map provisioningMap = manage.providerById(EntityType.PROVISIONING, manageId); Provisioning provisioning = new Provisioning(provisioningMap); AtomicReference redirectReference = new AtomicReference<>(this.config.getWelcomeUrl()); diff --git a/server/src/main/java/access/cron/RoleExpirationNotifier.java b/server/src/main/java/access/cron/RoleExpirationNotifier.java index 935de669..21434b7f 100644 --- a/server/src/main/java/access/cron/RoleExpirationNotifier.java +++ b/server/src/main/java/access/cron/RoleExpirationNotifier.java @@ -58,8 +58,8 @@ public List doSweep() { List groupedProviders = manage.getGroupedProviders(List.of(userRole.getRole())); GroupedProviders groupedProvider = groupedProviders.isEmpty() ? null : groupedProviders.get(0); String mail = mailBox.sendUserRoleExpirationNotificationMail(userRole, groupedProvider, roleExpirationNotificationDays); - userRole.setExpiryNotifications(1); - userRoleRepository.save(userRole); + //https://stackoverflow.com/a/75121707 + userRoleRepository.updateExpiryNotifications(1, userRole.getId()); LOG.info("Send expiration notification mail to " + userRole.getUser().getEmail()); diff --git a/server/src/main/java/access/manage/Manage.java b/server/src/main/java/access/manage/Manage.java index ccc730b5..4fe731a3 100644 --- a/server/src/main/java/access/manage/Manage.java +++ b/server/src/main/java/access/manage/Manage.java @@ -125,8 +125,9 @@ default List getGroupedProviders(List requestedRoles) { .map(application -> new ManageIdentifier(application.getManageId(), application.getManageType())) .collect(Collectors.toSet()) .stream() - .map(manageIdentifier -> { - Map provider = providerById(manageIdentifier.manageType(), manageIdentifier.manageId()); + .map(manageIdentifier -> providerById(manageIdentifier.manageType(), manageIdentifier.manageId())) + .filter(provider -> !CollectionUtils.isEmpty(provider)) + .map(provider -> { String id = (String) provider.get("id"); return new GroupedProviders( provider, diff --git a/server/src/main/java/access/manage/RemoteManage.java b/server/src/main/java/access/manage/RemoteManage.java index 59542bba..da8c052a 100644 --- a/server/src/main/java/access/manage/RemoteManage.java +++ b/server/src/main/java/access/manage/RemoteManage.java @@ -5,6 +5,7 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.http.client.support.BasicAuthenticationInterceptor; import org.springframework.util.CollectionUtils; +import org.springframework.web.client.ResponseErrorHandler; import org.springframework.web.client.RestTemplate; import java.io.IOException; @@ -28,6 +29,8 @@ public RemoteManage(String url, String user, String password, ObjectMapper objec this.queries = objectMapper.readValue(new ClassPathResource("/manage/query_templates.json").getInputStream(), new TypeReference<>() { }); restTemplate.getInterceptors().add(new BasicAuthenticationInterceptor(user, password)); + ResponseErrorHandler resilientErrorHandler = new ResilientErrorHandler(); + restTemplate.setErrorHandler(resilientErrorHandler); } @Override diff --git a/server/src/main/java/access/manage/ResilientErrorHandler.java b/server/src/main/java/access/manage/ResilientErrorHandler.java new file mode 100644 index 00000000..3ba2d212 --- /dev/null +++ b/server/src/main/java/access/manage/ResilientErrorHandler.java @@ -0,0 +1,37 @@ +package access.manage; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.web.client.ResponseErrorHandler; + +import java.io.IOException; +import java.net.URI; + +public class ResilientErrorHandler implements ResponseErrorHandler { + + private static final Log LOG = LogFactory.getLog(ResilientErrorHandler.class); + + @Override + public boolean hasError(ClientHttpResponse response) throws IOException { + HttpStatusCode statusCode = response.getStatusCode(); + return statusCode.isError(); + } + + @Override + public void handleError(ClientHttpResponse response) throws IOException { + this.handleError(null, null, response); + } + + @Override + public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException { + //ignore + LOG.warn(String.format("Error from Manage: %s, %s, %s, %s", + url, + method, + response.getStatusText(), + new String(response.getBody().readAllBytes()))); + } +} diff --git a/server/src/main/java/access/repository/UserRoleRepository.java b/server/src/main/java/access/repository/UserRoleRepository.java index 4d3890ba..3227a843 100644 --- a/server/src/main/java/access/repository/UserRoleRepository.java +++ b/server/src/main/java/access/repository/UserRoleRepository.java @@ -1,9 +1,7 @@ package access.repository; -import access.model.Authority; import access.model.Role; import access.model.UserRole; -import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; @@ -12,9 +10,7 @@ import org.springframework.transaction.annotation.Transactional; import java.time.Instant; -import java.util.Collection; import java.util.List; -import java.util.Optional; @Repository public interface UserRoleRepository extends JpaRepository { @@ -28,7 +24,12 @@ public interface UserRoleRepository extends JpaRepository { List findByRoleName(String roleName); @Modifying - @Query("DELETE FROM user_roles WHERE id = ?1") + @Query(value = "DELETE FROM user_roles WHERE id = ?1", nativeQuery = true) @Transactional(isolation = Isolation.SERIALIZABLE) void deleteUserRoleById(Long id); + + @Modifying + @Query(value = "UPDATE user_roles SET expiry_notifications= ?1 WHERE id = ?2", nativeQuery = true) + @Transactional(isolation = Isolation.SERIALIZABLE) + void updateExpiryNotifications(Integer expiryNotifications, Long id); } diff --git a/server/src/main/java/access/security/ExtendedInMemoryUserDetailsManager.java b/server/src/main/java/access/security/ExtendedInMemoryUserDetailsManager.java index 71c3df7e..2a8f9d78 100644 --- a/server/src/main/java/access/security/ExtendedInMemoryUserDetailsManager.java +++ b/server/src/main/java/access/security/ExtendedInMemoryUserDetailsManager.java @@ -32,6 +32,10 @@ private void fixPassword(RemoteUser remoteUser) { @Override public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException { RemoteUser remoteUser = users.get(username); - return remoteUser != null ? new RemoteUser(remoteUser) : null; + //Need to make copy, otherwise the password is erased. See RemoteUser#CredentialsContainer + if (remoteUser == null) { + throw new UsernameNotFoundException("User not found: " + username); + } + return new RemoteUser(remoteUser); } } diff --git a/server/src/main/java/access/security/RemoteUser.java b/server/src/main/java/access/security/RemoteUser.java index 3df5e93c..c8396b77 100644 --- a/server/src/main/java/access/security/RemoteUser.java +++ b/server/src/main/java/access/security/RemoteUser.java @@ -29,6 +29,7 @@ public RemoteUser(RemoteUser remoteUser) { @Override public Collection getAuthorities() { + //Convention dictates upperCase Role names to be used in @PreAuthorize annotations return scopes.stream() .map(scope -> new SimpleGrantedAuthority("ROLE_" + scope.toUpperCase())) .collect(Collectors.toList()); diff --git a/server/src/main/java/access/security/SecurityConfig.java b/server/src/main/java/access/security/SecurityConfig.java index 647fe459..00706a24 100644 --- a/server/src/main/java/access/security/SecurityConfig.java +++ b/server/src/main/java/access/security/SecurityConfig.java @@ -58,6 +58,8 @@ public class SecurityConfig { private final ProvisioningService provisioningService; private final ExternalApiConfiguration externalApiConfiguration; + private final RequestHeaderRequestMatcher apiTokenRequestMatcher = new RequestHeaderRequestMatcher(API_TOKEN_HEADER); + @Autowired public SecurityConfig(ClientRegistrationRepository clientRegistrationRepository, InvitationRepository invitationRepository, @@ -188,11 +190,15 @@ SecurityFilterChain basicAuthenticationSecurityFilterChain(HttpSecurity http) th "/api/aa/**", "/api/external/v1/aa/**", "/api/deprovision/**", - "/api/external/v1/deprovision/**") + "/api/external/v1/deprovision/**", + "/api/external/v1/roles") .sessionManagement(c -> c .sessionCreationPolicy(SessionCreationPolicy.STATELESS) ) .authorizeHttpRequests(c -> c + //The API token is secured in the UserHandlerMethodArgumentResolver + .requestMatchers(apiTokenRequestMatcher) + .permitAll() .anyRequest() .authenticated() ) @@ -203,7 +209,7 @@ SecurityFilterChain basicAuthenticationSecurityFilterChain(HttpSecurity http) th @Bean @Order(3) SecurityFilterChain jwtSecurityFilterChain(HttpSecurity http) throws Exception { - final RequestHeaderRequestMatcher apiTokenRequestMatcher = new RequestHeaderRequestMatcher(API_TOKEN_HEADER); + http.csrf(c -> c.disable()) .securityMatcher("/api/external/v1/**") .authorizeHttpRequests(c -> c diff --git a/server/src/main/java/access/teams/TeamsController.java b/server/src/main/java/access/teams/TeamsController.java index c0973c91..edb6cf1b 100644 --- a/server/src/main/java/access/teams/TeamsController.java +++ b/server/src/main/java/access/teams/TeamsController.java @@ -112,8 +112,8 @@ public ResponseEntity> migrateTeam(@RequestBody Team team) private boolean applicationExists(Application application) { try { - manage.providerById(application.getManageType(), application.getManageId()); - return true; + Map provider = manage.providerById(application.getManageType(), application.getManageId()); + return provider != null; } catch (RuntimeException e) { return false; } diff --git a/server/src/main/resources/application.yml b/server/src/main/resources/application.yml index 215a95ca..dc7dec51 100644 --- a/server/src/main/resources/application.yml +++ b/server/src/main/resources/application.yml @@ -158,7 +158,7 @@ email: # ignored then and the different manage entities are loaded from json files in `server/src/main/resources/manage`. # Each *.json file in this directory corresponds with the contents of that specific entity_type. # To test the provisioning (e.g. SCIM, EVA, Graphp) with real endpoints you can set the manage.local property below to -# False and then the provisioning.local.json file is used which is not in git as it is n .gitignore. You can safely +# True and then the provisioning.local.json file is used which is not in git as it is n .gitignore. You can safely # configure real users / passwords and test against those. See server/src/main/java/access/manage/ManageConf.java # and server/src/main/java/access/manage/LocalManage.java to see how it works. manage: diff --git a/server/src/test/java/access/api/RoleControllerTest.java b/server/src/test/java/access/api/RoleControllerTest.java index dc6a6897..6559d2a7 100644 --- a/server/src/test/java/access/api/RoleControllerTest.java +++ b/server/src/test/java/access/api/RoleControllerTest.java @@ -12,6 +12,7 @@ import org.junit.jupiter.api.Test; import org.springframework.http.HttpStatus; +import java.lang.reflect.Type; import java.util.List; import java.util.Map; import java.util.Set; @@ -434,6 +435,27 @@ void deleteRoleWithAPI() throws Exception { assertEquals(0, roleRepository.search("wiki", 1).size()); } + @Test + void createWithAPIUser() throws Exception { + Role role = new Role("New", "New desc", application("1", EntityType.SAML20_SP), 365, false, false); + + super.stubForManagerProvidersByIdIn(EntityType.SAML20_SP, List.of("1")); + super.stubForManageProvisioning(List.of("1")); + super.stubForCreateScimRole(); + + given() + .when() + .auth().preemptive().basic("voot", "secret") + .accept(ContentType.JSON) + .contentType(ContentType.JSON) + .body(role) + .post("/api/external/v1/roles") + .then() + .statusCode(400); + //TODO +// assertNotNull(result.get("id")); + } + @Test void rolesByApplicationSuperUserWithAPIToken() { super.stubForManagerProvidersByIdIn(EntityType.SAML20_SP, List.of("1", "2", "3", "4")); diff --git a/server/src/test/java/access/cron/RoleExpirationNotifierTest.java b/server/src/test/java/access/cron/RoleExpirationNotifierTest.java index b2628f36..a2703de5 100644 --- a/server/src/test/java/access/cron/RoleExpirationNotifierTest.java +++ b/server/src/test/java/access/cron/RoleExpirationNotifierTest.java @@ -37,6 +37,10 @@ void sweep() { userRole = userRoleRepository.findByRoleName("Mail").get(0); assertEquals(1, userRole.getExpiryNotifications()); + + Instant instant = Instant.now().plus(100, ChronoUnit.DAYS); + List userRoles = userRoleRepository.findByEndDateBeforeAndExpiryNotifications(instant, 0); + assertEquals(0, userRoles.size()); } @Test diff --git a/server/src/test/java/access/manage/RemoteManageTest.java b/server/src/test/java/access/manage/RemoteManageTest.java index f7c840c1..2a3553e3 100644 --- a/server/src/test/java/access/manage/RemoteManageTest.java +++ b/server/src/test/java/access/manage/RemoteManageTest.java @@ -11,6 +11,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.*; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; class RemoteManageTest extends AbstractTest { @@ -64,4 +65,13 @@ void providersByIdIn() throws JsonProcessingException { assertEquals(providers, remoteProviders); } + @Test + void providerByIdExceptionHandling() { + stubFor(get(urlPathMatching("/manage/api/internal/metadata/saml20_sp/1")).willReturn(aResponse() + .withHeader("Content-Type", "application/json") + .withStatus(404))); + Map remoteProvider = manage.providerById(EntityType.SAML20_SP, "1"); + assertNull(remoteProvider); + } + } \ No newline at end of file