From f54ba385a61f18b52756f3d318d963077ee1c261 Mon Sep 17 00:00:00 2001 From: dominicwest <101722961+dominicwest@users.noreply.github.com> Date: Wed, 6 Sep 2023 07:08:41 +0100 Subject: [PATCH 1/4] Release/3.2 (#28) * Adding constraint on gap_admin table * Adding unique constraint on grant_applicant_organisation_profile table * Removing unique constraints on tables with foreign key constraints --- .github/workflows/feature.yml | 2 ++ .../db/migration/V1_64__add_unique_constraints_to_users.sql | 2 ++ 2 files changed, 4 insertions(+) create mode 100644 src/main/resources/db/migration/V1_64__add_unique_constraints_to_users.sql diff --git a/.github/workflows/feature.yml b/.github/workflows/feature.yml index d8146704..07cac6ff 100644 --- a/.github/workflows/feature.yml +++ b/.github/workflows/feature.yml @@ -7,12 +7,14 @@ on: - AFG-** - bug/** - GAP-** + - fix/** pull_request: branches: - feature/** - AFG-** - bug/** - GAP-** + - fix/** jobs: build: diff --git a/src/main/resources/db/migration/V1_64__add_unique_constraints_to_users.sql b/src/main/resources/db/migration/V1_64__add_unique_constraints_to_users.sql new file mode 100644 index 00000000..8fb8d785 --- /dev/null +++ b/src/main/resources/db/migration/V1_64__add_unique_constraints_to_users.sql @@ -0,0 +1,2 @@ +ALTER TABLE gap_user ADD CONSTRAINT unique_user_sub UNIQUE (user_sub); +ALTER TABLE grant_applicant ADD CONSTRAINT unique_user_id UNIQUE (user_id); \ No newline at end of file From c74f3749fd496957996e55e628f8010d36abd92a Mon Sep 17 00:00:00 2001 From: dylanwrightCO <135221918+dylanwrightCO@users.noreply.github.com> Date: Thu, 28 Sep 2023 11:56:00 +0100 Subject: [PATCH 2/4] Release/3.3 (#39) Release 3.3 --- .gitignore | 4 +- .pre-commit-config.yaml | 5 + .../config/JwtTokenFilterConfig.java | 15 ++ .../controllers/LoginController.java | 14 +- .../controllers/UserController.java | 44 ++++ .../ValidateSessionsRolesRequestBodyDTO.java | 4 + .../gap/adminbackend/models/AdminSession.java | 3 + .../repositories/GapUserRepository.java | 2 + .../GrantApplicantRepository.java | 3 +- .../schedulers/GrantAdvertsScheduler.java | 2 +- .../adminbackend/security/AuthManager.java | 2 +- .../adminbackend/security/JwtTokenFilter.java | 70 +++++ .../security/WebSecurityConfig.java | 44 ++-- .../services/GrantAdvertService.java | 4 +- .../adminbackend/services/UserService.java | 35 +++ src/main/resources/application.properties | 2 + ...ission_and_grant_applicant_org_profile.sql | 3 + .../V1_66__cascade_delete_applicant.sql | 63 +++++ ...V1_67__grant_beneficiary_fk_constraint.sql | 8 + .../V1_68__alter_scheduler_database_view.sql | 25 ++ .../V1_69__fix_scheduler_database_view.sql | 21 ++ .../controllers/LoginControllerTest.java | 18 +- .../controllers/UserControllerTest.java | 247 +++++++++++++++--- .../security/JwtTokenFilterTest.java | 92 +++++++ ...ithAdminSessionSecurityContextFactory.java | 3 +- .../services/UserServiceTest.java | 160 +++++++++--- .../utils/ApplicationFormUtilsTest.java | 4 +- 27 files changed, 780 insertions(+), 117 deletions(-) create mode 100644 .pre-commit-config.yaml create mode 100644 src/main/java/gov/cabinetoffice/gap/adminbackend/config/JwtTokenFilterConfig.java create mode 100644 src/main/java/gov/cabinetoffice/gap/adminbackend/dtos/ValidateSessionsRolesRequestBodyDTO.java create mode 100644 src/main/java/gov/cabinetoffice/gap/adminbackend/security/JwtTokenFilter.java create mode 100644 src/main/resources/db/migration/V1_65__add_indexes_to_grant_submission_and_grant_applicant_org_profile.sql create mode 100644 src/main/resources/db/migration/V1_66__cascade_delete_applicant.sql create mode 100644 src/main/resources/db/migration/V1_67__grant_beneficiary_fk_constraint.sql create mode 100644 src/main/resources/db/migration/V1_68__alter_scheduler_database_view.sql create mode 100644 src/main/resources/db/migration/V1_69__fix_scheduler_database_view.sql create mode 100644 src/test/java/gov/cabinetoffice/gap/adminbackend/security/JwtTokenFilterTest.java diff --git a/.gitignore b/.gitignore index bd853a23..d520e8c3 100644 --- a/.gitignore +++ b/.gitignore @@ -38,4 +38,6 @@ build/ /bin -application-local.properties \ No newline at end of file +### Local properties ### +application-local.properties +.env.local \ No newline at end of file diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..abadc6ca --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,5 @@ +repos: +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.4.0 + hooks: + - id: detect-aws-credentials diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/config/JwtTokenFilterConfig.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/config/JwtTokenFilterConfig.java new file mode 100644 index 00000000..b890bc08 --- /dev/null +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/config/JwtTokenFilterConfig.java @@ -0,0 +1,15 @@ +package gov.cabinetoffice.gap.adminbackend.config; + +import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class JwtTokenFilterConfig { + + @Value("${feature.onelogin.enabled}") + public boolean oneLoginEnabled; + + @Value("${feature.validate-user-roles-in-middleware}") + public boolean validateUserRolesInMiddleware; + +} diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/LoginController.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/LoginController.java index 71cbf62c..d0aba545 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/LoginController.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/LoginController.java @@ -50,23 +50,11 @@ public class LoginController { @PostMapping("/login") public ResponseEntity login(HttpServletRequest httpRequest) { - // so local dev work won't be quite as miserable - if (Objects.equals(this.profile, "LOCAL") && this.ignoreJwt) { - if (this.grantAdminId == null || this.funderId == null) { - return ResponseEntity.internalServerError().build(); - } - SecurityContextHolder.getContext() - .setAuthentication(new UsernamePasswordAuthenticationToken( - new AdminSession(this.grantAdminId, this.funderId, "Test", "User", this.funderName, - emailAddress), - null, Collections.singletonList(new SimpleGrantedAuthority("ROLE_ADMIN")))); - return ResponseEntity.ok().build(); - } - UsernamePasswordAuthenticationToken authenticationToken = new UsernamePasswordAuthenticationToken("", httpRequest.getHeader(HttpHeaders.AUTHORIZATION)); Authentication authenticate = this.authManager.authenticate(authenticationToken); SecurityContextHolder.getContext().setAuthentication(authenticate); + return ResponseEntity.ok().build(); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/UserController.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/UserController.java index 7c2e34d3..7375422d 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/UserController.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/UserController.java @@ -5,15 +5,21 @@ import gov.cabinetoffice.gap.adminbackend.dtos.UserDTO; import gov.cabinetoffice.gap.adminbackend.mappers.UserMapper; import gov.cabinetoffice.gap.adminbackend.models.AdminSession; +import gov.cabinetoffice.gap.adminbackend.models.JwtPayload; import gov.cabinetoffice.gap.adminbackend.services.JwtService; import gov.cabinetoffice.gap.adminbackend.services.UserService; import gov.cabinetoffice.gap.adminbackend.utils.HelperUtils; import lombok.RequiredArgsConstructor; import lombok.extern.log4j.Log4j2; +import org.springframework.beans.factory.annotation.Value; import org.springframework.http.ResponseEntity; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.web.bind.annotation.*; import java.util.Objects; +import java.util.Optional; +import java.util.UUID; import static org.springframework.util.ObjectUtils.isEmpty; @@ -29,12 +35,33 @@ public class UserController { private final UserService userService; + @Value("${feature.onelogin.enabled}") + private boolean oneLoginEnabled; + @GetMapping("/loggedInUser") public ResponseEntity getLoggedInUserDetails() { AdminSession session = HelperUtils.getAdminSessionForAuthenticatedUser(); return ResponseEntity.ok(userMapper.adminSessionToUserDTO(session)); } + @GetMapping("/validateAdminSession") + public ResponseEntity validateAdminSession() { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + + if (authentication == null || !authentication.isAuthenticated()) { + return ResponseEntity.ok(Boolean.FALSE); + } + + AdminSession adminSession = ((AdminSession) authentication.getPrincipal()); + if (!oneLoginEnabled) { + return ResponseEntity.ok(Boolean.TRUE); + } + String emailAddress = adminSession.getEmailAddress(); + String roles = adminSession.getRoles(); + + return ResponseEntity.ok(userService.verifyAdminRoles(emailAddress, roles)); + } + @PatchMapping("/migrate") public ResponseEntity migrateUser(@RequestBody MigrateUserDto migrateUserDto, @RequestHeader("Authorization") String token) { @@ -51,4 +78,21 @@ public ResponseEntity migrateUser(@RequestBody MigrateUserDto migrateUse return ResponseEntity.ok("User migrated successfully"); } + @DeleteMapping("/delete/{oneLoginSub}") + public ResponseEntity deleteUser(@PathVariable Optional oneLoginSub, + @RequestParam(required = false) Optional colaSub, @RequestHeader("Authorization") String token) { + // Called from our user service only. Does not have an admin session so authing + // via the jwt + if (isEmpty(token) || !token.startsWith("Bearer ")) + return ResponseEntity.status(401).body("Delete user: Expected Authorization header not provided"); + final DecodedJWT decodedJWT = jwtService.verifyToken(token.split(" ")[1]); + final JwtPayload jwtPayload = this.jwtService.getPayloadFromJwtV2(decodedJWT); + if (!jwtPayload.getRoles().contains("SUPER_ADMIN")) { + return ResponseEntity.status(403).body("User not authorized to delete user: " + oneLoginSub); + } + + userService.deleteUser(oneLoginSub, colaSub); + return ResponseEntity.ok("User deleted successfully"); + } + } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/dtos/ValidateSessionsRolesRequestBodyDTO.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/dtos/ValidateSessionsRolesRequestBodyDTO.java new file mode 100644 index 00000000..bf6b114b --- /dev/null +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/dtos/ValidateSessionsRolesRequestBodyDTO.java @@ -0,0 +1,4 @@ +package gov.cabinetoffice.gap.adminbackend.dtos; + +public record ValidateSessionsRolesRequestBodyDTO(String emailAddress, String roles) { +} diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/models/AdminSession.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/models/AdminSession.java index 020e9c8b..b6230738 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/models/AdminSession.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/models/AdminSession.java @@ -23,6 +23,8 @@ public class AdminSession implements Serializable { private String emailAddress; + private String roles; + public AdminSession(Integer grantAdminId, Integer funderId, JwtPayload jwtPayload) { this.grantAdminId = grantAdminId; this.funderId = funderId; @@ -30,6 +32,7 @@ public AdminSession(Integer grantAdminId, Integer funderId, JwtPayload jwtPayloa this.lastName = jwtPayload.getFamilyName(); this.organisationName = jwtPayload.getDepartmentName(); this.emailAddress = jwtPayload.getEmailAddress(); + this.roles = jwtPayload.getRoles(); } } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GapUserRepository.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GapUserRepository.java index 444e927b..d72bd355 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GapUserRepository.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GapUserRepository.java @@ -10,4 +10,6 @@ public interface GapUserRepository extends JpaRepository { Optional findByUserSub(String userSub); + void deleteByUserSub(String userSub); + } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GrantApplicantRepository.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GrantApplicantRepository.java index 40c24616..d71e0e70 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GrantApplicantRepository.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GrantApplicantRepository.java @@ -1,13 +1,14 @@ package gov.cabinetoffice.gap.adminbackend.repositories; import gov.cabinetoffice.gap.adminbackend.dtos.submission.GrantApplicant; -import gov.cabinetoffice.gap.adminbackend.entities.GapUser; import org.springframework.data.jpa.repository.JpaRepository; import java.util.Optional; public interface GrantApplicantRepository extends JpaRepository { + void deleteByUserId(String userId); + Optional findByUserId(String userId); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/schedulers/GrantAdvertsScheduler.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/schedulers/GrantAdvertsScheduler.java index 532d2e16..35abd3e2 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/schedulers/GrantAdvertsScheduler.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/schedulers/GrantAdvertsScheduler.java @@ -29,7 +29,7 @@ public class GrantAdvertsScheduler { private final GrantAdvertsSchedulerConfigProperties advertsSchedulerConfigProperties; - @Scheduled(cron = "${grant-adverts-scheduler.cronExpression:0 0 0 * * ?}", zone = "Europe/London") + @Scheduled(cron = "${grant-adverts-scheduler.cronExpression:0 0 0 * * ?}", zone = "UTC") @SchedulerLock(name = "grantAdverts_publishUnpublishScheduler", lockAtMostFor = "${grant-adverts-scheduler.lock.atMostFor:30m}", lockAtLeastFor = "${grant-adverts-scheduler.lock.atLeastFor:5m}") diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/security/AuthManager.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/security/AuthManager.java index 86cfd95a..7946755f 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/security/AuthManager.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/security/AuthManager.java @@ -42,7 +42,6 @@ public class AuthManager implements AuthenticationManager { @Override public Authentication authenticate(Authentication authentication) throws AuthenticationException { - String authHeader = authentication.getCredentials().toString(); if (isEmpty(authHeader) || !authHeader.startsWith("Bearer ")) { throw new UnauthorizedException("Expected Authorization header not provided"); @@ -56,6 +55,7 @@ public Authentication authenticate(Authentication authentication) throws Authent JwtPayload JWTPayload; if (oneLoginEnabled) { JWTPayload = this.jwtService.getPayloadFromJwtV2(decodedJWT); + if (!JWTPayload.getRoles().contains("ADMIN")) { throw new AccessDeniedException("User is not an admin"); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/security/JwtTokenFilter.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/security/JwtTokenFilter.java new file mode 100644 index 00000000..9b326438 --- /dev/null +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/security/JwtTokenFilter.java @@ -0,0 +1,70 @@ +package gov.cabinetoffice.gap.adminbackend.security; + +import gov.cabinetoffice.gap.adminbackend.config.JwtTokenFilterConfig; +import gov.cabinetoffice.gap.adminbackend.exceptions.ForbiddenException; +import gov.cabinetoffice.gap.adminbackend.exceptions.UnauthorizedException; +import gov.cabinetoffice.gap.adminbackend.models.AdminSession; +import gov.cabinetoffice.gap.adminbackend.services.JwtService; +import gov.cabinetoffice.gap.adminbackend.services.UserService; +import lombok.RequiredArgsConstructor; +import org.jetbrains.annotations.NotNull; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.authentication.logout.SecurityContextLogoutHandler; +import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.RestClientException; +import org.springframework.web.filter.OncePerRequestFilter; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import static org.springframework.util.ObjectUtils.isEmpty; + +/** + * This class cannot be a Spring bean, otherwise Spring will automatically apply it to all + * requests, regardless of whether they've been specifically ignored + */ +@RequiredArgsConstructor +public class JwtTokenFilter extends OncePerRequestFilter { + + private final UserService userService; + + private final JwtTokenFilterConfig jwtTokenFilterConfig; + + @Override + protected void doFilterInternal(final @NotNull HttpServletRequest request, + final @NotNull HttpServletResponse response, final @NotNull FilterChain chain) + throws ServletException, IOException { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + + boolean skipSessionValidation = authentication == null || !authentication.isAuthenticated() + || !jwtTokenFilterConfig.oneLoginEnabled || !jwtTokenFilterConfig.validateUserRolesInMiddleware; + + if (skipSessionValidation) { + chain.doFilter(request, response); + return; + } + + AdminSession adminSession = ((AdminSession) authentication.getPrincipal()); + + String emailAddress = adminSession.getEmailAddress(); + String roles = adminSession.getRoles(); + try { + userService.verifyAdminRoles(emailAddress, roles); + chain.doFilter(request, response); + } + catch (RestClientException | UnauthorizedException error) { + SecurityContextLogoutHandler securityContextLogoutHandler = new SecurityContextLogoutHandler(); + securityContextLogoutHandler.logout(request, null, authentication); + throw new UnauthorizedException("Payload is out of date"); + } + } + +} diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java index 4bb3a8fd..cc890933 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java @@ -1,5 +1,8 @@ package gov.cabinetoffice.gap.adminbackend.security; +import gov.cabinetoffice.gap.adminbackend.config.JwtTokenFilterConfig; +import gov.cabinetoffice.gap.adminbackend.services.JwtService; +import gov.cabinetoffice.gap.adminbackend.services.UserService; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpStatus; @@ -9,37 +12,46 @@ import org.springframework.security.config.http.SessionCreationPolicy; import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.HttpStatusEntryPoint; +import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; @Configuration @EnableWebSecurity @EnableGlobalMethodSecurity(prePostEnabled = true) public class WebSecurityConfig { + private final JwtTokenFilter jwtTokenFilter; + private static final String UUID_REGEX_STRING = "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}"; + public WebSecurityConfig(final UserService userService, final JwtTokenFilterConfig jwtTokenFilterConfig) { + this.jwtTokenFilter = new JwtTokenFilter(userService, jwtTokenFilterConfig); + } + @Bean public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { http.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED).and() - .authorizeHttpRequests( - auth -> auth - .mvcMatchers("/login", "/health", "/emails/sendLambdaConfirmationEmail", - "/submissions/{submissionId:" + UUID_REGEX_STRING - + "}/export-batch/{batchExportId:" + UUID_REGEX_STRING + "}/submission", - "/submissions/*/export-batch/*/status", - "/submissions/{submissionId:" + UUID_REGEX_STRING - + "}/export-batch/{batchExportId:" + UUID_REGEX_STRING + "}/signedUrl", - "/export-batch/{exportId:" + UUID_REGEX_STRING + "}/outstandingCount", - "/grant-advert/lambda/{grantAdvertId:" + UUID_REGEX_STRING + "}/publish", - "/grant-advert/lambda/{grantAdvertId:" + UUID_REGEX_STRING + "}/unpublish", - "/users/migrate") - .permitAll() - .antMatchers("/v3/api-docs/**", "/swagger-ui/**", "/swagger-resources/**", - "/swagger-ui.html", "/webjars/**") - .permitAll().anyRequest().authenticated()) + .authorizeHttpRequests(auth -> auth + .mvcMatchers("/login", "/health", "/emails/sendLambdaConfirmationEmail", + "/users/validateAdminSession", "/submissions/{submissionId:" + UUID_REGEX_STRING + + "}/export-batch/{batchExportId:" + UUID_REGEX_STRING + "}/submission", + "/submissions/*/export-batch/*/status", + "/submissions/{submissionId:" + UUID_REGEX_STRING + "}/export-batch/{batchExportId:" + + UUID_REGEX_STRING + "}/signedUrl", + "/export-batch/{exportId:" + UUID_REGEX_STRING + "}/outstandingCount", + "/grant-advert/lambda/{grantAdvertId:" + UUID_REGEX_STRING + "}/publish", + "/grant-advert/lambda/{grantAdvertId:" + UUID_REGEX_STRING + "}/unpublish", + "/users/migrate", "/users/delete/**") + .permitAll() + .antMatchers("/v3/api-docs/**", "/swagger-ui/**", "/swagger-resources/**", "/swagger-ui.html", + "/webjars/**") + .permitAll().anyRequest().authenticated()) .formLogin().disable().httpBasic().disable().logout().disable().csrf().disable().exceptionHandling() .authenticationEntryPoint(new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED)); + + http.addFilterAfter(jwtTokenFilter, UsernamePasswordAuthenticationFilter.class); + return http.build(); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertService.java index 5a9aec2f..e90f9954 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertService.java @@ -514,11 +514,11 @@ private void updateGrantAdvertApplicationDates(final GrantAdvert grantAdvert) { // build LocalDateTimes, convert to Instant Instant openingDateInstant = LocalDateTime .of(openingResponse[2], openingResponse[1], openingResponse[0], openingResponse[3], openingResponse[4]) - .atZone(ZoneId.of("Europe/London")).toInstant(); + .atZone(ZoneId.of("Z")).toInstant(); Instant closingDateInstant = LocalDateTime .of(closingResponse[2], closingResponse[1], closingResponse[0], closingResponse[3], closingResponse[4]) - .atZone(ZoneId.of("Europe/London")).toInstant(); + .atZone(ZoneId.of("Z")).toInstant(); // set dates on advert grantAdvert.setOpeningDate(openingDateInstant); diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/UserService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/UserService.java index 1b96e2fd..40df95d3 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/UserService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/UserService.java @@ -1,11 +1,18 @@ package gov.cabinetoffice.gap.adminbackend.services; +import gov.cabinetoffice.gap.adminbackend.config.UserServiceConfig; +import gov.cabinetoffice.gap.adminbackend.dtos.ValidateSessionsRolesRequestBodyDTO; +import gov.cabinetoffice.gap.adminbackend.exceptions.UnauthorizedException; import gov.cabinetoffice.gap.adminbackend.repositories.GapUserRepository; import gov.cabinetoffice.gap.adminbackend.repositories.GrantApplicantRepository; import lombok.RequiredArgsConstructor; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpMethod; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.client.RestTemplate; +import java.util.Optional; import java.util.UUID; @Service @@ -16,6 +23,10 @@ public class UserService { private final GrantApplicantRepository grantApplicantRepository; + private final UserServiceConfig userServiceConfig; + + private final RestTemplate restTemplate; + @Transactional public void migrateUser(final String oneLoginSub, final UUID colaSub) { gapUserRepository.findByUserSub(colaSub.toString()).ifPresent(gapUser -> { @@ -29,4 +40,28 @@ public void migrateUser(final String oneLoginSub, final UUID colaSub) { }); } + @Transactional + public void deleteUser(final Optional oneLoginSubOptional, final Optional colaSubOptional) { + // Deleting COLA and OneLogin subs as either could be stored against the user + oneLoginSubOptional.ifPresent(grantApplicantRepository::deleteByUserId); + colaSubOptional.ifPresent(sub -> grantApplicantRepository.deleteByUserId(sub.toString())); + } + + public Boolean verifyAdminRoles(final String emailAddress, final String roles) { + // TODO: after admin-session token handling is aligned with applicant we should + // use '/is-user-logged-in' + final String url = userServiceConfig.getDomain() + "/v2/validateSessionsRoles"; + ValidateSessionsRolesRequestBodyDTO requestBody = new ValidateSessionsRolesRequestBodyDTO(emailAddress, roles); + + final HttpEntity requestEntity = new HttpEntity( + requestBody); + + Boolean isAdminSessionValid = restTemplate.exchange(url, HttpMethod.POST, requestEntity, Boolean.class) + .getBody(); + if (isAdminSessionValid == null) { + throw new UnauthorizedException("Invalid roles"); + } + return isAdminSessionValid; + } + } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 503045bb..801648dd 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -99,3 +99,5 @@ contentful.deliveryAPIAccessToken=Axq2ANwlrcHAOh0pjvm6gRaQEDblzddoZ8Bt2FJHCmA contentful.environmentId=dev feature.onelogin.enabled=false + +feature.validate-user-roles-in-middleware=true \ No newline at end of file diff --git a/src/main/resources/db/migration/V1_65__add_indexes_to_grant_submission_and_grant_applicant_org_profile.sql b/src/main/resources/db/migration/V1_65__add_indexes_to_grant_submission_and_grant_applicant_org_profile.sql new file mode 100644 index 00000000..0c7fa716 --- /dev/null +++ b/src/main/resources/db/migration/V1_65__add_indexes_to_grant_submission_and_grant_applicant_org_profile.sql @@ -0,0 +1,3 @@ +CREATE UNIQUE INDEX grant_applicant_organisation_profile_applicant_id_idx ON grant_applicant_organisation_profile (applicant_id); +CREATE INDEX grant_submission_applicant_id_idx ON grant_submission (applicant_id); +CREATE INDEX grant_submission_application_id_idx ON grant_submission (application_id); \ No newline at end of file diff --git a/src/main/resources/db/migration/V1_66__cascade_delete_applicant.sql b/src/main/resources/db/migration/V1_66__cascade_delete_applicant.sql new file mode 100644 index 00000000..ba71e750 --- /dev/null +++ b/src/main/resources/db/migration/V1_66__cascade_delete_applicant.sql @@ -0,0 +1,63 @@ +-- grant_export table + +alter table public.grant_export +drop constraint grant_export_submission_id_fkey, +add constraint grant_export_submission_id_fkey + foreign key (submission_id) + references public.grant_submission (id) match simple + on delete cascade + on update no action; + +-- grant_applicant_organisation_profile table + +alter table public.grant_applicant_organisation_profile +drop constraint grant_applicant_organisation_profile_applicant_id_fkey, +add constraint grant_applicant_organisation_profile_applicant_id_fkey + foreign key (applicant_id) + references public.grant_applicant (id) match simple + on delete cascade + on update no action; + +-- grant_attachment table + +alter table public.grant_attachment +drop constraint fkauex62pbrqwu8rh0g28l8phq9, +add constraint fkauex62pbrqwu8rh0g28l8phq9 + foreign key (created_by) + references public.grant_applicant (id) match simple + on delete cascade + on update no action; + +alter table public.grant_attachment +drop constraint fkq3von08d8ihi8rxngekli01l0, +add constraint fkq3von08d8ihi8rxngekli01l0 + foreign key (submission_id) + references public.grant_submission (id) match simple + on delete cascade + on update no action; + +-- grant_submission table + +alter table public.grant_submission +drop constraint submission_applicant_id_fkey, +add constraint submission_applicant_id_fkey + foreign key (applicant_id) + references public.grant_applicant (id) match simple + on delete cascade + on update no action; + +alter table public.grant_submission +drop constraint submission_last_updated_by_fkey, +add constraint submission_last_updated_by_fkey + foreign key (last_updated_by) + references public.grant_applicant (id) match simple + on delete cascade + on update no action; + +alter table public.grant_submission +drop constraint submission_created_by_fkey, +add constraint submission_created_by_fkey + foreign key (created_by) + references public.grant_applicant (id) match simple + on delete cascade + on update no action; \ No newline at end of file diff --git a/src/main/resources/db/migration/V1_67__grant_beneficiary_fk_constraint.sql b/src/main/resources/db/migration/V1_67__grant_beneficiary_fk_constraint.sql new file mode 100644 index 00000000..169fc0ee --- /dev/null +++ b/src/main/resources/db/migration/V1_67__grant_beneficiary_fk_constraint.sql @@ -0,0 +1,8 @@ +alter table public.grant_beneficiary +alter column created_by drop not null, +drop constraint if exists created_by_fkey, +add constraint created_by_fkey + foreign key (created_by) + references public.grant_applicant (id) match simple + on delete set null + on update no action; \ No newline at end of file diff --git a/src/main/resources/db/migration/V1_68__alter_scheduler_database_view.sql b/src/main/resources/db/migration/V1_68__alter_scheduler_database_view.sql new file mode 100644 index 00000000..82536c09 --- /dev/null +++ b/src/main/resources/db/migration/V1_68__alter_scheduler_database_view.sql @@ -0,0 +1,25 @@ +-- Replace view for scheduler + +DROP VIEW IF EXISTS ADVERT_SCHEDULER_VIEW; + +ALTER TABLE grant_advert ALTER COLUMN opening_date TYPE timestamp with time zone; +ALTER TABLE grant_advert ALTER COLUMN closing_date TYPE timestamp with time zone; + +CREATE VIEW ADVERT_SCHEDULER_VIEW AS +SELECT GRANT_ADVERT_ID AS ID, + CASE + WHEN + STATUS = 'SCHEDULED' + AND (OPENING_DATE AT TIME ZONE 'Europe/London')::date <= (NOW() AT TIME ZONE 'Europe/London')::date + THEN 'PUBLISH' + WHEN + STATUS = 'PUBLISHED' + AND (CLOSING_DATE AT TIME ZONE 'Europe/London')::date <= ((NOW() AT TIME ZONE 'Europe/London')::date - interval '1' DAY) + THEN 'UNPUBLISH' + END AS ACTION +FROM GRANT_ADVERT +WHERE ( + STATUS = 'SCHEDULED' AND (OPENING_DATE AT TIME ZONE 'Europe/London')::date <= (NOW() AT TIME ZONE 'Europe/London')::date +) OR ( + STATUS = 'PUBLISHED' AND (CLOSING_DATE AT TIME ZONE 'Europe/London')::date <= ((NOW() AT TIME ZONE 'Europe/London')::date - interval '1' DAY) +); \ No newline at end of file diff --git a/src/main/resources/db/migration/V1_69__fix_scheduler_database_view.sql b/src/main/resources/db/migration/V1_69__fix_scheduler_database_view.sql new file mode 100644 index 00000000..fcba2f3f --- /dev/null +++ b/src/main/resources/db/migration/V1_69__fix_scheduler_database_view.sql @@ -0,0 +1,21 @@ +-- Replace view for scheduler + +DROP VIEW IF EXISTS ADVERT_SCHEDULER_VIEW; +CREATE VIEW ADVERT_SCHEDULER_VIEW AS +SELECT GRANT_ADVERT_ID AS ID, + CASE + WHEN + STATUS = 'SCHEDULED' + AND (OPENING_DATE AT TIME ZONE 'Z')::date <= (NOW() AT TIME ZONE 'Z')::date + THEN 'PUBLISH' + WHEN + STATUS = 'PUBLISHED' + AND (CLOSING_DATE AT TIME ZONE 'Z')::date <= ((NOW() AT TIME ZONE 'Z')::date - interval '1' DAY) + THEN 'UNPUBLISH' + END AS ACTION +FROM GRANT_ADVERT +WHERE ( + STATUS = 'SCHEDULED' AND (OPENING_DATE AT TIME ZONE 'Z')::date <= (NOW() AT TIME ZONE 'Z')::date +) OR ( + STATUS = 'PUBLISHED' AND (CLOSING_DATE AT TIME ZONE 'Z')::date <= ((NOW() AT TIME ZONE 'Z')::date - interval '1' DAY) +); \ No newline at end of file diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/LoginControllerTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/LoginControllerTest.java index 80cfadd0..c010edc2 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/LoginControllerTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/LoginControllerTest.java @@ -1,21 +1,28 @@ package gov.cabinetoffice.gap.adminbackend.controllers; import gov.cabinetoffice.gap.adminbackend.annotations.WithAdminSession; +import gov.cabinetoffice.gap.adminbackend.config.JwtTokenFilterConfig; import gov.cabinetoffice.gap.adminbackend.dtos.errors.GenericErrorDTO; import gov.cabinetoffice.gap.adminbackend.exceptions.UnauthorizedException; import gov.cabinetoffice.gap.adminbackend.mappers.ValidationErrorMapperImpl; import gov.cabinetoffice.gap.adminbackend.models.AdminSession; import gov.cabinetoffice.gap.adminbackend.security.AuthManager; +import gov.cabinetoffice.gap.adminbackend.security.JwtTokenFilter; import gov.cabinetoffice.gap.adminbackend.security.WebSecurityConfig; +import gov.cabinetoffice.gap.adminbackend.services.JwtService; +import gov.cabinetoffice.gap.adminbackend.services.UserService; import gov.cabinetoffice.gap.adminbackend.utils.HelperUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.web.SecurityFilterChain; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; @@ -41,6 +48,15 @@ class LoginControllerTest { @SpyBean private ValidationErrorMapperImpl validationErrorMapper; + @MockBean + private JwtTokenFilterConfig jwtTokenFilterConfig; + + @MockBean + private JwtService jwtService; + + @MockBean + private UserService userService; + @Resource private WebApplicationContext context; @@ -54,7 +70,7 @@ public void setUp() { @Test void SuccessfulLoginTest() throws Exception { Mockito.when(this.authenticationManager.authenticate(any())).thenReturn(new UsernamePasswordAuthenticationToken( - new AdminSession(1, 1, "Test", "User", "AND Digital", "test@domain.com"), null)); + new AdminSession(1, 1, "Test", "User", "AND Digital", "test@domain.com", null), null)); this.mockMvc.perform(post("/login")).andExpect(status().isOk()); } diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/UserControllerTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/UserControllerTest.java index 1e1694a0..3d772783 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/UserControllerTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/UserControllerTest.java @@ -5,10 +5,13 @@ import gov.cabinetoffice.gap.adminbackend.exceptions.UnauthorizedException; import gov.cabinetoffice.gap.adminbackend.mappers.UserMapper; import gov.cabinetoffice.gap.adminbackend.mappers.ValidationErrorMapperImpl; +import gov.cabinetoffice.gap.adminbackend.models.AdminSession; +import gov.cabinetoffice.gap.adminbackend.models.JwtPayload; import gov.cabinetoffice.gap.adminbackend.services.JwtService; import gov.cabinetoffice.gap.adminbackend.services.UserService; import gov.cabinetoffice.gap.adminbackend.utils.HelperUtils; import gov.cabinetoffice.gap.adminbackend.utils.TestDecodedJwt; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; @@ -17,20 +20,27 @@ import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestPropertySource; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.web.context.WebApplicationContext; import javax.annotation.Resource; +import java.util.Optional; import java.util.UUID; import static org.mockito.Mockito.*; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @WebMvcTest(UserController.class) @AutoConfigureMockMvc(addFilters = false) @ContextConfiguration(classes = { UserController.class, ControllerExceptionHandler.class }) +@TestPropertySource(properties = { "feature.onelogin.enabled=true" }) class UserControllerTest { @Resource @@ -51,55 +61,212 @@ class UserControllerTest { @Autowired private MockMvc mockMvc; - @Test - void migrateUser_HappyPath() throws Exception { - final MigrateUserDto migrateUserDto = MigrateUserDto.builder().colaSub(UUID.randomUUID()) - .oneLoginSub("oneLoginSub").build(); - final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("oneLoginSub").build(); - when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); - - mockMvc.perform(MockMvcRequestBuilders.patch("/users/migrate").contentType(MediaType.APPLICATION_JSON) - .content(HelperUtils.asJsonString(migrateUserDto)).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) - .andExpect(status().isOk()).andReturn(); - verify(userService).migrateUser("oneLoginSub", migrateUserDto.getColaSub()); + @Nested + class MigrateUser { + + @Test + void HappyPath() throws Exception { + final MigrateUserDto migrateUserDto = MigrateUserDto.builder().colaSub(UUID.randomUUID()) + .oneLoginSub("oneLoginSub").build(); + final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("oneLoginSub").build(); + when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); + + mockMvc.perform(MockMvcRequestBuilders.patch("/users/migrate").contentType(MediaType.APPLICATION_JSON) + .content(HelperUtils.asJsonString(migrateUserDto)).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) + .andExpect(status().isOk()).andReturn(); + verify(userService).migrateUser("oneLoginSub", migrateUserDto.getColaSub()); + } + + @Test + void NoJwt() throws Exception { + final MigrateUserDto migrateUserDto = MigrateUserDto.builder().colaSub(UUID.randomUUID()) + .oneLoginSub("oneLoginSub").build(); + final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("oneLoginSub").build(); + when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); + + mockMvc.perform(MockMvcRequestBuilders.patch("/users/migrate").contentType(MediaType.APPLICATION_JSON) + .content(HelperUtils.asJsonString(migrateUserDto)).header(HttpHeaders.AUTHORIZATION, "")) + .andExpect(status().isUnauthorized()).andReturn(); + verify(userService, times(0)).migrateUser("oneLoginSub", migrateUserDto.getColaSub()); + } + + @Test + void InvalidJwt() throws Exception { + final MigrateUserDto migrateUserDto = MigrateUserDto.builder().colaSub(UUID.randomUUID()) + .oneLoginSub("oneLoginSub").build(); + doThrow(new UnauthorizedException("Invalid JWT")).when(jwtService).verifyToken("jwt"); + + mockMvc.perform(MockMvcRequestBuilders.patch("/users/migrate").contentType(MediaType.APPLICATION_JSON) + .content(HelperUtils.asJsonString(migrateUserDto)).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) + .andExpect(status().isUnauthorized()).andReturn(); + verify(userService, times(0)).migrateUser("oneLoginSub", migrateUserDto.getColaSub()); + } + + @Test + void JwtDoesNotMatchMUserToMigrate() throws Exception { + final MigrateUserDto migrateUserDto = MigrateUserDto.builder().colaSub(UUID.randomUUID()) + .oneLoginSub("oneLoginSub").build(); + final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("anotherUsersOneLoginSub").build(); + when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); + + mockMvc.perform(MockMvcRequestBuilders.patch("/users/migrate").contentType(MediaType.APPLICATION_JSON) + .content(HelperUtils.asJsonString(migrateUserDto)).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) + .andExpect(status().isForbidden()).andReturn(); + verify(userService, times(0)).migrateUser("oneLoginSub", migrateUserDto.getColaSub()); + } + + } + + @Nested + class DeleteUser { + + @Test + void NoColaSubHappyPath() throws Exception { + final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("oneLoginSub").build(); + final JwtPayload jwtPayload = JwtPayload.builder().roles("SUPER_ADMIN").build(); + when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); + when(jwtService.getPayloadFromJwtV2(decodedJWT)).thenReturn(jwtPayload); + + mockMvc.perform(MockMvcRequestBuilders.delete("/users/delete/oneLoginSub") + .contentType(MediaType.APPLICATION_JSON).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) + .andExpect(status().isOk()).andReturn(); + + verify(userService, times(1)).deleteUser(Optional.of("oneLoginSub"), Optional.empty()); + } + + @Test + void EmptyColaSubHappyPath() throws Exception { + final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("oneLoginSub").build(); + final JwtPayload jwtPayload = JwtPayload.builder().roles("SUPER_ADMIN").build(); + when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); + when(jwtService.getPayloadFromJwtV2(decodedJWT)).thenReturn(jwtPayload); + + mockMvc.perform(MockMvcRequestBuilders.delete("/users/delete/oneLoginSub?colaSub=") + .contentType(MediaType.APPLICATION_JSON).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) + .andExpect(status().isOk()).andReturn(); + + verify(userService, times(1)).deleteUser(Optional.of("oneLoginSub"), Optional.empty()); + } + + @Test + void ColaSubHappyPath() throws Exception { + final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("oneLoginSub").build(); + final UUID colaSub = UUID.randomUUID(); + final JwtPayload jwtPayload = JwtPayload.builder().roles("SUPER_ADMIN").build(); + when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); + when(jwtService.getPayloadFromJwtV2(decodedJWT)).thenReturn(jwtPayload); + + mockMvc.perform(MockMvcRequestBuilders.delete("/users/delete/oneLoginSub?colaSub=" + colaSub) + .contentType(MediaType.APPLICATION_JSON).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) + .andExpect(status().isOk()).andReturn(); + + verify(userService, times(1)).deleteUser(Optional.of("oneLoginSub"), Optional.of(colaSub)); + } + + @Test + void InvalidColaSub() throws Exception { + final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("oneLoginSub").build(); + final String colaSub = "not-a-uuid"; + final JwtPayload jwtPayload = JwtPayload.builder().roles("SUPER_ADMIN").build(); + when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); + when(jwtService.getPayloadFromJwtV2(decodedJWT)).thenReturn(jwtPayload); + + mockMvc.perform(MockMvcRequestBuilders.delete("/users/delete/oneLoginSub?colaSub=" + colaSub) + .contentType(MediaType.APPLICATION_JSON).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) + .andExpect(status().isBadRequest()).andReturn(); + + verify(userService, times(0)).deleteUser(any(), any()); + } + + @Test + void NoJwt() throws Exception { + final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("oneLoginSub").build(); + when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); + + mockMvc.perform(MockMvcRequestBuilders.delete("/users/delete/oneLoginSub") + .contentType(MediaType.APPLICATION_JSON).header(HttpHeaders.AUTHORIZATION, "")) + .andExpect(status().isUnauthorized()).andReturn(); + verify(userService, times(0)).deleteUser(Optional.of("oneLoginSub"), Optional.empty()); + } + + @Test + void InvalidJwt() throws Exception { + doThrow(new UnauthorizedException("Invalid JWT")).when(jwtService).verifyToken("jwt"); + + mockMvc.perform(MockMvcRequestBuilders.delete("/users/delete/oneLoginSub") + .contentType(MediaType.APPLICATION_JSON).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) + .andExpect(status().isUnauthorized()).andReturn(); + verify(userService, times(0)).deleteUser(Optional.of("oneLoginSub"), Optional.empty()); + } + + @Test + void NotSuperAdmin() throws Exception { + final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("anotherUsersOneLoginSub").build(); + final JwtPayload jwtPayload = JwtPayload.builder().roles("ADMIN").build(); + when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); + when(jwtService.getPayloadFromJwtV2(decodedJWT)).thenReturn(jwtPayload); + + mockMvc.perform(MockMvcRequestBuilders.delete("/users/delete/oneLoginSub") + .contentType(MediaType.APPLICATION_JSON).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) + .andExpect(status().isForbidden()).andReturn(); + verify(userService, times(0)).deleteUser(Optional.of("oneLoginSub"), Optional.empty()); + } + } @Test - void migrateUser_NoJwt() throws Exception { - final MigrateUserDto migrateUserDto = MigrateUserDto.builder().colaSub(UUID.randomUUID()) - .oneLoginSub("oneLoginSub").build(); - final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("oneLoginSub").build(); - when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); - - mockMvc.perform(MockMvcRequestBuilders.patch("/users/migrate").contentType(MediaType.APPLICATION_JSON) - .content(HelperUtils.asJsonString(migrateUserDto)).header(HttpHeaders.AUTHORIZATION, "")) - .andExpect(status().isUnauthorized()).andReturn(); - verify(userService, times(0)).migrateUser("oneLoginSub", migrateUserDto.getColaSub()); + public void testValidateAdminSession() throws Exception { + AdminSession adminSession = new AdminSession(); + adminSession.setEmailAddress("admin@example.com"); + adminSession.setRoles("[FIND, APPLY, ADMIN]"); + + SecurityContext securityContext = mock(SecurityContext.class); + SecurityContextHolder.setContext(securityContext); + + Authentication authentication = mock(Authentication.class); + when(securityContext.getAuthentication()).thenReturn(authentication); + when(authentication.isAuthenticated()).thenReturn(true); + when(authentication.getPrincipal()).thenReturn(adminSession); + + when(userService.verifyAdminRoles("admin@example.com", "[FIND, APPLY, ADMIN]")).thenReturn(Boolean.TRUE); + + mockMvc.perform(MockMvcRequestBuilders.get("/users/validateAdminSession")).andExpect(status().isOk()) + .andExpect(content().string("true")); + + verify(userService, times(1)).verifyAdminRoles("admin@example.com", "[FIND, APPLY, ADMIN]"); } @Test - void migrateUser_InvalidJwt() throws Exception { - final MigrateUserDto migrateUserDto = MigrateUserDto.builder().colaSub(UUID.randomUUID()) - .oneLoginSub("oneLoginSub").build(); - doThrow(new UnauthorizedException("Invalid JWT")).when(jwtService).verifyToken("jwt"); - - mockMvc.perform(MockMvcRequestBuilders.patch("/users/migrate").contentType(MediaType.APPLICATION_JSON) - .content(HelperUtils.asJsonString(migrateUserDto)).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) - .andExpect(status().isUnauthorized()).andReturn(); - verify(userService, times(0)).migrateUser("oneLoginSub", migrateUserDto.getColaSub()); + public void testValidateAdminSessionAuthenticationNotAuthenticated() throws Exception { + SecurityContext securityContext = mock(SecurityContext.class); + SecurityContextHolder.setContext(securityContext); + + Authentication authentication = mock(Authentication.class); + when(securityContext.getAuthentication()).thenReturn(authentication); + when(authentication.isAuthenticated()).thenReturn(false); + + mockMvc.perform(MockMvcRequestBuilders.get("/users/validateAdminSession")).andExpect(status().isOk()) + .andExpect(content().string("false")); } @Test - void migrateUser_JwtDoesNotMatchMUserToMigrate() throws Exception { - final MigrateUserDto migrateUserDto = MigrateUserDto.builder().colaSub(UUID.randomUUID()) - .oneLoginSub("oneLoginSub").build(); - final DecodedJWT decodedJWT = TestDecodedJwt.builder().subject("anotherUsersOneLoginSub").build(); - when(jwtService.verifyToken("jwt")).thenReturn(decodedJWT); - - mockMvc.perform(MockMvcRequestBuilders.patch("/users/migrate").contentType(MediaType.APPLICATION_JSON) - .content(HelperUtils.asJsonString(migrateUserDto)).header(HttpHeaders.AUTHORIZATION, "Bearer jwt")) - .andExpect(status().isForbidden()).andReturn(); - verify(userService, times(0)).migrateUser("oneLoginSub", migrateUserDto.getColaSub()); + public void testValidateAdminSessionRolesDoNotMatch() throws Exception { + AdminSession adminSession = new AdminSession(); + adminSession.setEmailAddress("admin@example.com"); + adminSession.setRoles("[FIND, APPLY, ADMIN]"); + + SecurityContext securityContext = mock(SecurityContext.class); + SecurityContextHolder.setContext(securityContext); + + Authentication authentication = mock(Authentication.class); + when(securityContext.getAuthentication()).thenReturn(authentication); + when(authentication.isAuthenticated()).thenReturn(true); + when(authentication.getPrincipal()).thenReturn(adminSession); + + doThrow(new UnauthorizedException("Roles do not match")).when(userService).verifyAdminRoles("admin@example.com", + "[FIND, APPLY, ADMIN]"); + + mockMvc.perform(MockMvcRequestBuilders.get("/users/validateAdminSession")).andExpect(status().isUnauthorized()); } } diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/security/JwtTokenFilterTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/security/JwtTokenFilterTest.java new file mode 100644 index 00000000..5315ba3f --- /dev/null +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/security/JwtTokenFilterTest.java @@ -0,0 +1,92 @@ +package gov.cabinetoffice.gap.adminbackend.security; + +import gov.cabinetoffice.gap.adminbackend.config.JwtTokenFilterConfig; +import gov.cabinetoffice.gap.adminbackend.exceptions.UnauthorizedException; +import gov.cabinetoffice.gap.adminbackend.models.AdminSession; +import gov.cabinetoffice.gap.adminbackend.models.JwtPayload; +import gov.cabinetoffice.gap.adminbackend.services.UserService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.authentication.logout.SecurityContextLogoutHandler; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class JwtTokenFilterTest { + + private JwtTokenFilter jwtTokenFilter; + + private @Mock UserService userService; + + private @Mock JwtTokenFilterConfig jwtTokenFilterConfig; + + @BeforeEach + void setup() { + jwtTokenFilterConfig.oneLoginEnabled = true; + jwtTokenFilterConfig.validateUserRolesInMiddleware = true; + jwtTokenFilter = new JwtTokenFilter(userService, jwtTokenFilterConfig); + } + + @Test + void Authenticates_when_TokenIsValid() throws IOException, ServletException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + FilterChain chain = mock(FilterChain.class); + Authentication authentication = mock(Authentication.class); + when(authentication.isAuthenticated()).thenReturn(true); + final SecurityContext securityContext = mock(SecurityContext.class); + SecurityContextHolder.setContext(securityContext); + when(securityContext.getAuthentication()).thenReturn(authentication); + + JwtPayload payload = new JwtPayload(); + payload.setEmailAddress("test@example.com"); + payload.setRoles("ADMIN"); + + AdminSession adminSession = new AdminSession(1, 1, payload); + when(authentication.getPrincipal()).thenReturn(adminSession); + when(userService.verifyAdminRoles(eq("test@example.com"), eq("ADMIN"))).thenReturn(Boolean.TRUE); + + jwtTokenFilter.doFilterInternal(request, response, chain); + verify(chain, times(1)).doFilter(request, response); + verify(userService, times(1)).verifyAdminRoles("test@example.com", "ADMIN"); + } + + @Test + void verifyAdminRolesThrowsUnauthorizedException_when_PayloadIsInvalid() throws ServletException, IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + FilterChain chain = mock(FilterChain.class); + + Authentication authentication = mock(Authentication.class); + when(authentication.isAuthenticated()).thenReturn(true); + + SecurityContext securityContext = mock(SecurityContext.class); + SecurityContextHolder.setContext(securityContext); + when(securityContext.getAuthentication()).thenReturn(authentication); + + JwtPayload payload = new JwtPayload(); + payload.setEmailAddress("test@example.com"); + payload.setRoles("ADMIN"); + + AdminSession adminSession = new AdminSession(1, 1, payload); + when(authentication.getPrincipal()).thenReturn(adminSession); + doThrow(UnauthorizedException.class).when(userService).verifyAdminRoles(eq("test@example.com"), eq("ADMIN")); + + verify(chain, times(0)).doFilter(request, response); + assertThrows(UnauthorizedException.class, () -> jwtTokenFilter.doFilterInternal(request, response, chain)); + } + +} diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/security/WithAdminSessionSecurityContextFactory.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/security/WithAdminSessionSecurityContextFactory.java index f79376d0..29243b21 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/security/WithAdminSessionSecurityContextFactory.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/security/WithAdminSessionSecurityContextFactory.java @@ -18,7 +18,8 @@ public SecurityContext createSecurityContext(WithAdminSession adminSession) { SecurityContext context = SecurityContextHolder.createEmptyContext(); AdminSession principal = new AdminSession(adminSession.grantAdminId(), adminSession.funderId(), "Test", "User", - "AND Digital", "test@domain.com"); + "AND Digital", "test@domain.com", null); + Authentication auth = new UsernamePasswordAuthenticationToken(principal, null, Collections.singletonList(new SimpleGrantedAuthority("ROLE_ADMIN"))); context.setAuthentication(auth); diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/UserServiceTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/UserServiceTest.java index 72f71670..27c11269 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/UserServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/UserServiceTest.java @@ -1,18 +1,24 @@ package gov.cabinetoffice.gap.adminbackend.services; +import gov.cabinetoffice.gap.adminbackend.config.UserServiceConfig; import gov.cabinetoffice.gap.adminbackend.dtos.submission.GrantApplicant; import gov.cabinetoffice.gap.adminbackend.entities.GapUser; +import gov.cabinetoffice.gap.adminbackend.exceptions.UnauthorizedException; import gov.cabinetoffice.gap.adminbackend.repositories.GapUserRepository; import gov.cabinetoffice.gap.adminbackend.repositories.GrantApplicantRepository; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Spy; +import org.springframework.http.*; import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; +import org.springframework.web.client.RestTemplate; import java.util.Optional; import java.util.UUID; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -29,60 +35,138 @@ class UserServiceTest { @Mock private GrantApplicantRepository grantApplicantRepository; + @Mock + private UserServiceConfig userServiceConfig; + + @Mock + private RestTemplate restTemplate; + private final String oneLoginSub = "oneLoginSub"; private final UUID colaSub = UUID.randomUUID(); - @Test - void migrateUserNoMatches() { - when(gapUserRepository.findByUserSub(any())).thenReturn(Optional.empty()); - when(grantApplicantRepository.findByUserId(any())).thenReturn(Optional.empty()); + @Nested + class MigrateUser { - userService.migrateUser(oneLoginSub, colaSub); + @Test + void migrateUserNoMatches() { + when(gapUserRepository.findByUserSub(any())).thenReturn(Optional.empty()); + when(grantApplicantRepository.findByUserId(any())).thenReturn(Optional.empty()); - verify(gapUserRepository, times(0)).save(any()); - verify(grantApplicantRepository, times(0)).save(any()); - } + userService.migrateUser(oneLoginSub, colaSub); - @Test - void migrateUserMatchesGapUser() { - final GapUser gapUser = GapUser.builder().build(); - when(gapUserRepository.findByUserSub(any())).thenReturn(Optional.of(gapUser)); - when(grantApplicantRepository.findByUserId(any())).thenReturn(Optional.empty()); + verify(gapUserRepository, times(0)).save(any()); + verify(grantApplicantRepository, times(0)).save(any()); + } + + @Test + void migrateUserMatchesGapUser() { + final GapUser gapUser = GapUser.builder().build(); + when(gapUserRepository.findByUserSub(any())).thenReturn(Optional.of(gapUser)); + when(grantApplicantRepository.findByUserId(any())).thenReturn(Optional.empty()); + + userService.migrateUser(oneLoginSub, colaSub); + gapUser.setUserSub(oneLoginSub); + + verify(gapUserRepository, times(1)).save(gapUser); + verify(grantApplicantRepository, times(0)).save(any()); + } + + @Test + void migrateUserMatchesGrantApplicant() { + final GrantApplicant grantApplicant = GrantApplicant.builder().build(); + when(grantApplicantRepository.findByUserId(any())).thenReturn(Optional.of(grantApplicant)); + when(gapUserRepository.findByUserSub(any())).thenReturn(Optional.empty()); + + userService.migrateUser(oneLoginSub, colaSub); + grantApplicant.setUserId(oneLoginSub); + + verify(gapUserRepository, times(0)).save(any()); + verify(grantApplicantRepository, times(1)).save(grantApplicant); + } - userService.migrateUser(oneLoginSub, colaSub); - gapUser.setUserSub(oneLoginSub); + @Test + void migrateUserMatchesGrantApplicantAndGapUser() { + final GrantApplicant grantApplicant = GrantApplicant.builder().build(); + final GapUser gapUser = GapUser.builder().build(); + when(grantApplicantRepository.findByUserId(any())).thenReturn(Optional.of(grantApplicant)); + when(gapUserRepository.findByUserSub(any())).thenReturn(Optional.of(gapUser)); + + userService.migrateUser(oneLoginSub, colaSub); + grantApplicant.setUserId(oneLoginSub); + gapUser.setUserSub(oneLoginSub); + + verify(gapUserRepository, times(1)).save(gapUser); + verify(grantApplicantRepository, times(1)).save(grantApplicant); + } - verify(gapUserRepository, times(1)).save(gapUser); - verify(grantApplicantRepository, times(0)).save(any()); } - @Test - void migrateUserMatchesGrantApplicant() { - final GrantApplicant grantApplicant = GrantApplicant.builder().build(); - when(grantApplicantRepository.findByUserId(any())).thenReturn(Optional.of(grantApplicant)); - when(gapUserRepository.findByUserSub(any())).thenReturn(Optional.empty()); + @Nested + class DeleteUser { + + @Test + void deleteUserNoColaSub() { + userService.deleteUser(Optional.of(oneLoginSub), Optional.empty()); + + verify(grantApplicantRepository, times(1)).deleteByUserId(oneLoginSub); + verify(grantApplicantRepository, times(0)).deleteByUserId(colaSub.toString()); + } + + @Test + void deleteUserColaSubAndOLSub() { + userService.deleteUser(Optional.of(oneLoginSub), Optional.of(colaSub)); + + verify(grantApplicantRepository, times(1)).deleteByUserId(oneLoginSub); + verify(grantApplicantRepository, times(1)).deleteByUserId(colaSub.toString()); + } - userService.migrateUser(oneLoginSub, colaSub); - grantApplicant.setUserId(oneLoginSub); + @Test + void deleteUserNoOLSub() { + userService.deleteUser(Optional.empty(), Optional.of(colaSub)); - verify(gapUserRepository, times(0)).save(any()); - verify(grantApplicantRepository, times(1)).save(grantApplicant); + verify(grantApplicantRepository, times(0)).deleteByUserId(oneLoginSub); + verify(grantApplicantRepository, times(1)).deleteByUserId(colaSub.toString()); + } + + @Test + void deleteUserNoColaSubOrOLSub() { + userService.deleteUser(Optional.empty(), Optional.empty()); + + verify(grantApplicantRepository, times(0)).deleteByUserId(oneLoginSub); + verify(grantApplicantRepository, times(0)).deleteByUserId(colaSub.toString()); + } + + } + + @Test + public void testVerifyAdminRolesValid() { + String emailAddress = "admin@example.com"; + String roles = "[FIND, APPLY, ADMIN]"; + String url = "http://example.com/v2/validateSessionsRoles"; + ResponseEntity responseEntity = new ResponseEntity<>(true, HttpStatus.OK); + + when(restTemplate.exchange(eq(url), eq(HttpMethod.POST), any(HttpEntity.class), eq(Boolean.class))) + .thenReturn(responseEntity); + when(userServiceConfig.getDomain()).thenReturn("http://example.com"); + + userService.verifyAdminRoles(emailAddress, roles); + verify(restTemplate, times(1)).exchange(eq(url), eq(HttpMethod.POST), any(HttpEntity.class), eq(Boolean.class)); } @Test - void migrateUserMatchesGrantApplicantAndGapUser() { - final GrantApplicant grantApplicant = GrantApplicant.builder().build(); - final GapUser gapUser = GapUser.builder().build(); - when(grantApplicantRepository.findByUserId(any())).thenReturn(Optional.of(grantApplicant)); - when(gapUserRepository.findByUserSub(any())).thenReturn(Optional.of(gapUser)); - - userService.migrateUser(oneLoginSub, colaSub); - grantApplicant.setUserId(oneLoginSub); - gapUser.setUserSub(oneLoginSub); - - verify(gapUserRepository, times(1)).save(gapUser); - verify(grantApplicantRepository, times(1)).save(grantApplicant); + public void testVerifyAdminRolesWhenUnauthorizedResponse() { + String emailAddress = "admin@example.com"; + String roles = "[FIND, APPLY, ADMIN]"; + String url = "http://example.com/v2/validateSessionsRoles"; + HttpHeaders requestHeaders = new HttpHeaders(); + ResponseEntity responseEntity = new ResponseEntity<>(null, HttpStatus.UNAUTHORIZED); + + when(restTemplate.exchange(eq(url), eq(HttpMethod.POST), any(HttpEntity.class), eq(Boolean.class))) + .thenReturn(responseEntity); + when(userServiceConfig.getDomain()).thenReturn("http://example.com"); + + assertThrows(UnauthorizedException.class, () -> userService.verifyAdminRoles(emailAddress, roles)); } } \ No newline at end of file diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtilsTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtilsTest.java index a0a716f7..354d26df 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtilsTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtilsTest.java @@ -19,8 +19,8 @@ void updateAuditDetailsAfterFormChange_UpdatingExpectedAuditDetails() { ApplicationFormEntity applicationForm = RandomApplicationFormGenerators.randomApplicationFormEntity() .lastUpdateBy(007).lastUpdated(fiveSecondsAgo).version(version).build(); - AdminSession session = new AdminSession(1, 1, "Test", "User", "AND Digital", "test.user@and.digital"); - + AdminSession session = new AdminSession(1, 1, "Test", "User", "AND Digital", "test.user@and.digital", + "[FIND, APPLY, ADMIN]"); ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session); assertThat(applicationForm.getLastUpdated()).isAfter(fiveSecondsAgo); From b94090087c10d0b69ce5104605a7810e36442f8c Mon Sep 17 00:00:00 2001 From: dominicwest <101722961+dominicwest@users.noreply.github.com> Date: Tue, 17 Oct 2023 10:31:20 +0100 Subject: [PATCH 3/4] Release/3.6 (#47) * TMI2-253: Add pre-commit check and update .gitignore to include application-local.properties and .env.local * Fixing unique constraint on indexes (#33) * Fixing unique constraint on indexes * GAP-2070 delete applicant from apply DB (#15) Hard deletes an applicant after an admin deletes them * GAP-2070 grant_beneficiary fk constraint & set null delete cascade (#34) * GAP-2070 fixing not null constraint on created_by * GAP-2148: grant advert not publishing on selected date (#32) * GAP-2148 - fixing view to use UTC * GAP-2148 - fixing scheduled job & setting opening/closing dates to use UTC not BST (#40) - Removing code that formats the excel sheet as this was causing an unacceptable performance problem * GAP-2230: Removing transactional from inserting exports, as this results in the lambda being invoked before exports exist in the DB (#46) * Creates an endpoint which removes applications attached to an advert (#42) (#49) --------- Co-authored-by: paul-lawlor-tco Co-authored-by: Iain Cooper Co-authored-by: iaincooper-tco <99728291+iaincooper-tco@users.noreply.github.com> Co-authored-by: john-tco <135222889+john-tco@users.noreply.github.com> Co-authored-by: rachelswart <99667350+rachelswart@users.noreply.github.com> Co-authored-by: ConorFayleTCO <141320269+ConorFayleTCO@users.noreply.github.com> Co-authored-by: GavCookCO <99668051+GavCookCO@users.noreply.github.com> Co-authored-by: paul-lawlor-tco --- .../ApplicationFormController.java | 52 +++++++++- .../controllers/SubmissionsController.java | 7 ++ .../ApplicationFormRepository.java | 2 + .../security/WebSecurityConfig.java | 2 +- .../ApplicationFormSectionService.java | 6 +- .../services/ApplicationFormService.java | 27 ++++-- .../services/SubmissionsService.java | 2 - .../utils/ApplicationFormUtils.java | 6 +- .../gap/adminbackend/utils/XlsxGenerator.java | 6 +- .../ApplicationFormControllerTest.java | 97 +++++++++++++++++-- .../ApplicationFormSectionServiceTest.java | 7 +- .../services/ApplicationFormServiceTest.java | 45 +++++++-- .../testdata/ApplicationFormTestData.java | 2 + .../utils/ApplicationFormUtilsTest.java | 17 +++- 14 files changed, 236 insertions(+), 42 deletions(-) diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormController.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormController.java index 5cfda21c..29e811c2 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormController.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormController.java @@ -3,9 +3,14 @@ import gov.cabinetoffice.gap.adminbackend.dtos.GenericPostResponseDTO; import gov.cabinetoffice.gap.adminbackend.dtos.application.*; import gov.cabinetoffice.gap.adminbackend.dtos.errors.GenericErrorDTO; +import gov.cabinetoffice.gap.adminbackend.entities.ApplicationFormEntity; +import gov.cabinetoffice.gap.adminbackend.enums.ApplicationStatusEnum; import gov.cabinetoffice.gap.adminbackend.exceptions.ApplicationFormException; import gov.cabinetoffice.gap.adminbackend.exceptions.NotFoundException; +import gov.cabinetoffice.gap.adminbackend.exceptions.UnauthorizedException; import gov.cabinetoffice.gap.adminbackend.services.ApplicationFormService; +import gov.cabinetoffice.gap.adminbackend.services.GrantAdvertService; +import gov.cabinetoffice.gap.adminbackend.services.SecretAuthService; import io.swagger.v3.oas.annotations.media.ArraySchema; import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; @@ -13,6 +18,7 @@ import io.swagger.v3.oas.annotations.responses.ApiResponses; import io.swagger.v3.oas.annotations.tags.Tag; import lombok.RequiredArgsConstructor; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.access.AccessDeniedException; @@ -23,6 +29,7 @@ import javax.validation.constraints.NotNull; import java.util.List; +import java.util.UUID; @Tag(name = "Application Forms", description = "API for handling organisations.") @RestController @@ -32,6 +39,10 @@ public class ApplicationFormController { private final ApplicationFormService applicationFormService; + private final SecretAuthService secretAuthService; + + private final GrantAdvertService grantAdvertService; + @PostMapping @ApiResponses(value = { @ApiResponse(responseCode = "201", description = "Application form created successfully.", @@ -118,6 +129,45 @@ public ResponseEntity deleteApplicationForm(@PathVariable @NotNull Integer appli } + @DeleteMapping("/lambda/{grantAdvertId}/application") + @ApiResponses(value = { + @ApiResponse(responseCode = "204", description = "Application form updated successfully.", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "400", description = "Bad request body", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "403", + description = "Insufficient permissions to update this application form.", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "404", description = "Application not found with given id", + content = @Content(mediaType = "application/json")), }) + + public ResponseEntity removeApplicationAttachedToGrantAdvert( + @RequestHeader(HttpHeaders.AUTHORIZATION) String authHeader, @PathVariable @NotNull UUID grantAdvertId) { + try { + secretAuthService.authenticateSecret(authHeader); + Integer schemeId = grantAdvertService.getAdvertById(grantAdvertId, true).getScheme().getId(); + ApplicationFormEntity applicationForm = applicationFormService.getApplicationFromSchemeId(schemeId); + + ApplicationFormPatchDTO applicationFormPatchDTO = new ApplicationFormPatchDTO(); + applicationFormPatchDTO.setApplicationStatus(ApplicationStatusEnum.REMOVED); + this.applicationFormService.patchApplicationForm(applicationForm.getGrantApplicationId(), + applicationFormPatchDTO, true); + + return ResponseEntity.noContent().build(); + } + + catch (NotFoundException error) { + return ResponseEntity.status(HttpStatus.NOT_FOUND).build(); + } + catch (UnauthorizedException error) { + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); + } + catch (ApplicationFormException error) { + return ResponseEntity.internalServerError().build(); + } + + } + @PatchMapping("/{applicationId}") @ApiResponses(value = { @ApiResponse(responseCode = "204", description = "Application form updated successfully.", @@ -133,7 +183,7 @@ public ResponseEntity updateApplicationForm(@PathVariable @NotNull Integer appli @Valid @RequestBody ApplicationFormPatchDTO applicationFormPatchDTO) { try { - this.applicationFormService.patchApplicationForm(applicationId, applicationFormPatchDTO); + this.applicationFormService.patchApplicationForm(applicationId, applicationFormPatchDTO, false); return ResponseEntity.noContent().build(); } catch (NotFoundException nfe) { diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/SubmissionsController.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/SubmissionsController.java index 8fb1b168..6b52e977 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/SubmissionsController.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/SubmissionsController.java @@ -40,6 +40,9 @@ public class SubmissionsController { @GetMapping(value = "/spotlight-export/{applicationId}", produces = EXPORT_CONTENT_TYPE) public ResponseEntity exportSpotlightChecks(@PathVariable Integer applicationId) { + log.info("Started submissions export for application " + applicationId); + long start = System.currentTimeMillis(); + final ByteArrayOutputStream stream = submissionsService.exportSpotlightChecks(applicationId); final String exportFileName = submissionsService.generateExportFileName(applicationId); final InputStreamResource resource = createTemporaryFile(stream, exportFileName); @@ -54,6 +57,10 @@ public ResponseEntity exportSpotlightChecks(@PathVariable I final HttpHeaders headers = new HttpHeaders(); headers.setContentDisposition(ContentDisposition.parse("attachment; filename=" + exportFileName)); + long end = System.currentTimeMillis(); + log.info("Finished submissions export for application " + applicationId + ". Export time in millis: " + + (end - start)); + return ResponseEntity.ok().headers(headers).contentLength(length) .contentType(MediaType.parseMediaType(EXPORT_CONTENT_TYPE)).body(resource); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/ApplicationFormRepository.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/ApplicationFormRepository.java index a1adee9e..d689a346 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/ApplicationFormRepository.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/ApplicationFormRepository.java @@ -16,6 +16,8 @@ public interface ApplicationFormRepository extends JpaRepository findByGrantApplicationId(Integer applicationId); + Optional findByGrantSchemeId(Integer grantSchemeId); + @Query(nativeQuery = true, value = "SELECT q->>'responseType' from grant_application a, " + "json_array_elements(a.definition->'sections') sec, " + "json_array_elements(sec->'questions') q " diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java index cc890933..eba44ad0 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java @@ -41,7 +41,7 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { "/export-batch/{exportId:" + UUID_REGEX_STRING + "}/outstandingCount", "/grant-advert/lambda/{grantAdvertId:" + UUID_REGEX_STRING + "}/publish", "/grant-advert/lambda/{grantAdvertId:" + UUID_REGEX_STRING + "}/unpublish", - "/users/migrate", "/users/delete/**") + "/users/migrate", "/users/delete/**", "/application-forms/lambda/**") .permitAll() .antMatchers("/v3/api-docs/**", "/swagger-ui/**", "/swagger-resources/**", "/swagger-ui.html", "/webjars/**") diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormSectionService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormSectionService.java index 5408d8a1..aaed86d7 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormSectionService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormSectionService.java @@ -62,7 +62,7 @@ public String addSectionToApplicationForm(Integer applicationId, PostSectionDTO boolean isUniqueSectionName = sections.stream() .noneMatch(section -> Objects.equals(section.getSectionTitle(), newSection.getSectionTitle())); - ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session); + ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session, false); if (isUniqueSectionName) { sections.add(newSection); @@ -93,7 +93,7 @@ public void deleteSectionFromApplication(Integer applicationId, String sectionId throw new NotFoundException("Section with id " + sectionId + " does not exist"); } - ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session); + ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session, false); this.applicationFormRepository.save(applicationForm); @@ -113,7 +113,7 @@ public void updateSectionStatus(final Integer applicationId, final String sectio applicationForm.getDefinition().getSectionById(sectionId).setSectionStatus(newStatus); - ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session); + ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session, false); this.applicationFormRepository.save(applicationForm); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java index ed064cc5..e2a24b40 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java @@ -134,6 +134,10 @@ public void deleteApplicationForm(Integer applicationId) { this.applicationFormRepository.delete(applicationFormEntity); } + public ApplicationFormEntity getApplicationFromSchemeId(Integer schemeId) { + return applicationFormRepository.findByGrantSchemeId(schemeId).orElseThrow(); + } + public void patchQuestionValues(Integer applicationId, String sectionId, String questionId, ApplicationFormQuestionDTO questionDto) { AdminSession session = HelperUtils.getAdminSessionForAuthenticatedUser(); @@ -160,7 +164,7 @@ public void patchQuestionValues(Integer applicationId, String sectionId, String (QuestionOptionsPatchDTO) questionPatchDTO, questionById); } - ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session); + ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session, false); this.applicationFormRepository.save(applicationForm); }, () -> { @@ -225,7 +229,7 @@ public String addQuestionToApplicationForm(Integer applicationId, String section applicationForm.getDefinition().getSectionById(sectionId).getQuestions().add(applicationFormQuestionDTO); - ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, adminSession); + ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, adminSession, false); this.applicationFormRepository.save(applicationForm); this.sessionsService.deleteObjectFromSession(SessionObjectEnum.newQuestion, session); @@ -284,7 +288,7 @@ public void deleteQuestionFromSection(Integer applicationId, String sectionId, S throw new NotFoundException("Question with id " + questionId + " does not exist"); } - ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session); + ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session, false); this.applicationFormRepository.save(applicationForm); @@ -305,15 +309,18 @@ public ApplicationFormQuestionDTO retrieveQuestion(Integer applicationId, String } - public void patchApplicationForm(Integer applicationId, ApplicationFormPatchDTO patchDTO) { - AdminSession session = HelperUtils.getAdminSessionForAuthenticatedUser(); - + public void patchApplicationForm(Integer applicationId, ApplicationFormPatchDTO patchDTO, boolean isLambdaCall) { + AdminSession session = null; ApplicationFormEntity application = this.applicationFormRepository.findById(applicationId) .orElseThrow(() -> new NotFoundException("Application with id " + applicationId + " does not exist.")); - if (!application.getCreatedBy().equals(session.getGrantAdminId())) { - throw new AccessDeniedException("User " + session.getGrantAdminId() - + " is unable to access the application form with id " + applicationId); + if (!isLambdaCall) { + session = HelperUtils.getAdminSessionForAuthenticatedUser(); + + if (!application.getCreatedBy().equals(session.getGrantAdminId())) { + throw new AccessDeniedException("User " + session.getGrantAdminId() + + " is unable to access the application form with id " + applicationId); + } } if (patchDTO.getApplicationStatus() == ApplicationStatusEnum.PUBLISHED && application.getDefinition() @@ -327,7 +334,7 @@ public void patchApplicationForm(Integer applicationId, ApplicationFormPatchDTO application.setLastPublished(Instant.now()); } - ApplicationFormUtils.updateAuditDetailsAfterFormChange(application, session); + ApplicationFormUtils.updateAuditDetailsAfterFormChange(application, session, isLambdaCall); this.applicationFormRepository.save(application); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/SubmissionsService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/SubmissionsService.java index 61023234..a3adca84 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/SubmissionsService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/SubmissionsService.java @@ -34,7 +34,6 @@ import org.springframework.http.ResponseEntity; import org.springframework.security.access.AccessDeniedException; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; import org.springframework.web.client.RestTemplate; import java.io.ByteArrayOutputStream; @@ -196,7 +195,6 @@ public String generateExportFileName(Integer applicationId) { return dateString + "_" + ggisReference + "_" + applicationName + ".xlsx"; } - @Transactional public void triggerSubmissionsExport(Integer applicationId) { UUID exportBatchId = UUID.randomUUID(); AdminSession adminSession = HelperUtils.getAdminSessionForAuthenticatedUser(); diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtils.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtils.java index 79b305d3..b73f439a 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtils.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtils.java @@ -8,9 +8,11 @@ public class ApplicationFormUtils { public static void updateAuditDetailsAfterFormChange(ApplicationFormEntity applicationFormEntity, - AdminSession session) { + AdminSession session, boolean isLambdaCall) { applicationFormEntity.setLastUpdated(Instant.now()); - applicationFormEntity.setLastUpdateBy(session.getGrantAdminId()); + if (!isLambdaCall) { + applicationFormEntity.setLastUpdateBy(session.getGrantAdminId()); + } applicationFormEntity.setVersion(applicationFormEntity.getVersion() + 1); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/XlsxGenerator.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/XlsxGenerator.java index 1caf7077..c98ada74 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/XlsxGenerator.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/XlsxGenerator.java @@ -41,8 +41,8 @@ static Workbook createWorkbook(List headers, List> data) { Workbook workbook = new XSSFWorkbook(); Sheet sheet = workbook.createSheet(SHEET_NAME); - addHeaders(sheet, headers); addData(sheet, data); + addHeaders(sheet, headers); return workbook; } @@ -51,6 +51,9 @@ private static void addHeaders(Sheet worksheet, List headers) { for (int col = 0; col < headers.size(); col++) { Cell cell = row.createCell(col); cell.setCellValue(headers.get(col)); + + // leaving this method call in to apply basic formatting with minimal + // performance hit worksheet.autoSizeColumn(col); } } @@ -61,7 +64,6 @@ private static void addData(Sheet worksheet, List> data) { for (int col = 0; col < data.get(row).size(); col++) { Cell cell = dataRow.createCell(col); cell.setCellValue(data.get(row).get(col)); - worksheet.autoSizeColumn(col); } } } diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormControllerTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormControllerTest.java index 80d96886..64f97057 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormControllerTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormControllerTest.java @@ -1,14 +1,24 @@ package gov.cabinetoffice.gap.adminbackend.controllers; +import gov.cabinetoffice.gap.adminbackend.dtos.application.ApplicationFormNoSections; import gov.cabinetoffice.gap.adminbackend.dtos.application.ApplicationFormPatchDTO; import gov.cabinetoffice.gap.adminbackend.dtos.application.ApplicationFormsFoundDTO; import gov.cabinetoffice.gap.adminbackend.dtos.errors.GenericErrorDTO; +import gov.cabinetoffice.gap.adminbackend.entities.ApplicationFormEntity; +import gov.cabinetoffice.gap.adminbackend.entities.GrantAdvert; +import gov.cabinetoffice.gap.adminbackend.entities.SchemeEntity; +import gov.cabinetoffice.gap.adminbackend.enums.ApplicationStatusEnum; import gov.cabinetoffice.gap.adminbackend.exceptions.ApplicationFormException; import gov.cabinetoffice.gap.adminbackend.exceptions.NotFoundException; +import gov.cabinetoffice.gap.adminbackend.exceptions.UnauthorizedException; import gov.cabinetoffice.gap.adminbackend.mappers.ValidationErrorMapperImpl; +import gov.cabinetoffice.gap.adminbackend.repositories.ApplicationFormRepository; import gov.cabinetoffice.gap.adminbackend.services.ApplicationFormService; +import gov.cabinetoffice.gap.adminbackend.services.GrantAdvertService; +import gov.cabinetoffice.gap.adminbackend.services.SecretAuthService; import gov.cabinetoffice.gap.adminbackend.utils.HelperUtils; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; @@ -20,6 +30,7 @@ import org.springframework.test.web.servlet.MockMvc; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.web.client.HttpClientErrorException; import java.util.Collections; import java.util.List; @@ -50,6 +61,15 @@ class ApplicationFormControllerTest { @SpyBean private ValidationErrorMapperImpl validationErrorMapper; + @MockBean + private SecretAuthService secretAuthService; + + @MockBean + private GrantAdvertService grantAdvertService; + + @MockBean + private ApplicationFormRepository applicationFormRepository; + @Test void saveApplicationFormHappyPathTest() throws Exception { when(this.applicationFormService.saveApplicationForm(SAMPLE_APPLICATION_POST_FORM_DTO)) @@ -234,10 +254,71 @@ void deleteApplicationForm_AccessDeniedTest() throws Exception { .andExpect(content().string("")); } + @Test + void removesApplicationAttachedToGrantAdvert_Successfully() throws Exception { + SchemeEntity scheme = SchemeEntity.builder().id(1).name("scheme").build(); + GrantAdvert grantAdvert = GrantAdvert.builder().grantAdvertName("grant-advert").scheme(scheme).build(); + doNothing().when(this.secretAuthService).authenticateSecret("shh"); + when(grantAdvertService.getAdvertById(SAMPLE_ADVERT_ID, true)).thenReturn(grantAdvert); + when(applicationFormService.getApplicationFromSchemeId(scheme.getId())).thenReturn(ApplicationFormEntity + .builder().grantApplicationId(1).applicationName("application").grantSchemeId(scheme.getId()).build()); + doNothing().when(this.applicationFormService).patchApplicationForm(SAMPLE_APPLICATION_ID, + SAMPLE_PATCH_APPLICATION_DTO, true); + + this.mockMvc + .perform(delete("/application-forms/lambda/" + SAMPLE_ADVERT_ID + "/application/") + .contentType(MediaType.APPLICATION_JSON).header("Authorization", "shh")) + .andExpect(status().isNoContent()); + + Mockito.verify(applicationFormService, Mockito.times(1)).patchApplicationForm(1, + new ApplicationFormPatchDTO(ApplicationStatusEnum.REMOVED), true); + } + + @Test + void removesApplicationAttachedToGrantAdvert_throwsNotFoundWhenNoAdvertFound() throws Exception { + doNothing().when(this.secretAuthService).authenticateSecret("shh"); + doThrow(NotFoundException.class).when(grantAdvertService).getAdvertById(SAMPLE_ADVERT_ID, true); + + this.mockMvc + .perform(delete("/application-forms/lambda/" + SAMPLE_ADVERT_ID + "/application/") + .contentType(MediaType.APPLICATION_JSON).header("Authorization", "shh")) + .andExpect(status().isNotFound()); + } + + @Test + void removesApplicationAttachedToGrantAdvert_throwsUnAuthorizedWhenNoSecretProvided() throws Exception { + doThrow(UnauthorizedException.class).when(this.secretAuthService).authenticateSecret(any()); + + when(grantAdvertService.getAdvertById(SAMPLE_ADVERT_ID, true)).thenThrow(NotFoundException.class); + + this.mockMvc + .perform(delete("/application-forms/lambda/" + SAMPLE_ADVERT_ID + "/application/") + .contentType(MediaType.APPLICATION_JSON).header("Authorization", "not-correct")) + .andExpect(status().isUnauthorized()); + } + + @Test + void removesApplicationAttachedToGrantAdvert_throwsApplicationFormExceptionWhenUnableToPatch() throws Exception { + SchemeEntity scheme = SchemeEntity.builder().id(1).name("scheme").build(); + GrantAdvert grantAdvert = GrantAdvert.builder().grantAdvertName("grant-advert").scheme(scheme).build(); + doNothing().when(this.secretAuthService).authenticateSecret("shh"); + when(grantAdvertService.getAdvertById(SAMPLE_ADVERT_ID, true)).thenReturn(grantAdvert); + when(applicationFormService.getApplicationFromSchemeId(scheme.getId())).thenReturn(ApplicationFormEntity + .builder().grantApplicationId(1).applicationName("application").grantSchemeId(scheme.getId()).build()); + + doThrow(ApplicationFormException.class).when(this.applicationFormService).patchApplicationForm(anyInt(), any(), + eq(true)); + + this.mockMvc + .perform(delete("/application-forms/lambda/" + SAMPLE_ADVERT_ID + "/application/") + .contentType(MediaType.APPLICATION_JSON).header("Authorization", "shh")) + .andExpect(status().isInternalServerError()); + } + @Test void updateApplicationForm_SuccessfullyUpdatingApplication() throws Exception { doNothing().when(this.applicationFormService).patchApplicationForm(SAMPLE_APPLICATION_ID, - SAMPLE_PATCH_APPLICATION_DTO); + SAMPLE_PATCH_APPLICATION_DTO, false); this.mockMvc .perform(patch("/application-forms/" + SAMPLE_APPLICATION_ID).contentType(MediaType.APPLICATION_JSON) .content(HelperUtils.asJsonString(SAMPLE_PATCH_APPLICATION_DTO))) @@ -247,12 +328,13 @@ void updateApplicationForm_SuccessfullyUpdatingApplication() throws Exception { @Test void updateApplicationForm_BadRequest_NoApplicationPropertiesProvided() throws Exception { doNothing().when(this.applicationFormService).patchApplicationForm(SAMPLE_APPLICATION_ID, - SAMPLE_PATCH_APPLICATION_DTO); + SAMPLE_PATCH_APPLICATION_DTO, false); this.mockMvc.perform(patch("/application-forms/" + SAMPLE_APPLICATION_ID) .contentType(MediaType.APPLICATION_JSON).content("{ \"testProp\": \"doesnt exist\"}")) .andExpect(status().isBadRequest()); - verify(this.applicationFormService, never()).patchApplicationForm(anyInt(), any(ApplicationFormPatchDTO.class)); + verify(this.applicationFormService, never()).patchApplicationForm(anyInt(), any(ApplicationFormPatchDTO.class), + eq(false)); } @Test @@ -261,13 +343,14 @@ void updateApplicationForm_BadRequest_InvalidPropertieValue() throws Exception { .contentType(MediaType.APPLICATION_JSON).content("{ \"applicationStatus\": \"INCORRECT\"}")) .andExpect(status().isBadRequest()); - verify(this.applicationFormService, never()).patchApplicationForm(anyInt(), any(ApplicationFormPatchDTO.class)); + verify(this.applicationFormService, never()).patchApplicationForm(anyInt(), any(ApplicationFormPatchDTO.class), + eq(false)); } @Test void updateApplicationForm_ApplicationFormNotFound() throws Exception { doThrow(new NotFoundException("Not Found Message")).when(this.applicationFormService) - .patchApplicationForm(SAMPLE_APPLICATION_ID, SAMPLE_PATCH_APPLICATION_DTO); + .patchApplicationForm(SAMPLE_APPLICATION_ID, SAMPLE_PATCH_APPLICATION_DTO, false); this.mockMvc .perform(patch("/application-forms/" + SAMPLE_APPLICATION_ID).contentType(MediaType.APPLICATION_JSON) .content(HelperUtils.asJsonString(SAMPLE_PATCH_APPLICATION_DTO))) @@ -278,7 +361,7 @@ void updateApplicationForm_ApplicationFormNotFound() throws Exception { @Test void updateApplicationForm_AccessDenied() throws Exception { doThrow(new AccessDeniedException("Error")).when(this.applicationFormService) - .patchApplicationForm(SAMPLE_APPLICATION_ID, SAMPLE_PATCH_APPLICATION_DTO); + .patchApplicationForm(SAMPLE_APPLICATION_ID, SAMPLE_PATCH_APPLICATION_DTO, false); this.mockMvc .perform(patch("/application-forms/" + SAMPLE_APPLICATION_ID).contentType(MediaType.APPLICATION_JSON) .content(HelperUtils.asJsonString(SAMPLE_PATCH_APPLICATION_DTO))) @@ -288,7 +371,7 @@ void updateApplicationForm_AccessDenied() throws Exception { @Test void updateApplicationForm_GenericApplicationFormException() throws Exception { doThrow(new ApplicationFormException("Application Form Error Message")).when(this.applicationFormService) - .patchApplicationForm(SAMPLE_APPLICATION_ID, SAMPLE_PATCH_APPLICATION_DTO); + .patchApplicationForm(SAMPLE_APPLICATION_ID, SAMPLE_PATCH_APPLICATION_DTO, false); this.mockMvc .perform(patch("/application-forms/" + SAMPLE_APPLICATION_ID).contentType(MediaType.APPLICATION_JSON) .content(HelperUtils.asJsonString(SAMPLE_PATCH_APPLICATION_DTO))) diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormSectionServiceTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormSectionServiceTest.java index 50b2cdab..f9ea97ae 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormSectionServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormSectionServiceTest.java @@ -34,6 +34,7 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; import static org.assertj.core.api.AssertionsForClassTypes.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mockStatic; @SpringJUnitConfig @@ -164,7 +165,7 @@ void addNewSectionHappyPathTest() { fail("Returned id was was not a UUID"); } - utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any())); + utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any(), eq(false))); utilMock.close(); } @@ -238,7 +239,7 @@ void deleteSectionHappyPathTest() { assertThat(sectionExists).isFalse(); - utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any())); + utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any(), eq(false))); utilMock.close(); } @@ -313,7 +314,7 @@ void updateSectionStatusHappyPath() { .getSectionStatus(); assertThat(newSectionStatus).isEqualTo(SectionStatusEnum.COMPLETE); - utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any())); + utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any(), eq(false))); utilMock.close(); } diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java index e7028a8f..237ba6a8 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java @@ -36,9 +36,12 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; import static org.assertj.core.api.AssertionsForClassTypes.fail; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @SpringJUnitConfig @WithAdminSession @@ -306,7 +309,7 @@ void patchQuestionValuesGenericHappyPathTest() { assertThat(updatedQuestion.getFieldTitle()).isEqualTo(SAMPLE_UPDATED_FIELD_TITLE); - this.utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any())); + this.utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any(), eq(false))); } @@ -326,7 +329,7 @@ void patchQuestionValuesOptionsHappyPathTest() { assertThat(updatedQuestion.getOptions()).isEqualTo(SAMPLE_UPDATED_OPTIONS); - this.utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any())); + this.utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any(), eq(false))); } @Test @@ -413,7 +416,7 @@ void addNewQuestionValuesGenericHappyPathTest() { fail("Returned id was was not a UUID"); } - this.utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any())); + this.utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any(), eq(false))); } @@ -441,7 +444,7 @@ void addNewQuestionValuesOptionsHappyPathTest() { fail("Returned id was was not a UUID"); } - this.utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any())); + this.utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any(), eq(false))); } @Test @@ -547,7 +550,7 @@ void deleteQuestionHappyPathTest() { assertThat(sectionExists).isFalse(); - utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any())); + utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any(), eq(false))); utilMock.close(); } @@ -704,14 +707,14 @@ void successfullyPatchApplicationForm() { .thenReturn(patchedApplicationFormEntity); ApplicationFormServiceTest.this.applicationFormService.patchApplicationForm(applicationId, - SAMPLE_PATCH_APPLICATION_DTO); + SAMPLE_PATCH_APPLICATION_DTO, false); verify(ApplicationFormServiceTest.this.applicationFormRepository).findById(applicationId); verify(ApplicationFormServiceTest.this.applicationFormMapper) .updateApplicationEntityFromPatchDto(SAMPLE_PATCH_APPLICATION_DTO, testApplicationFormEntity); verify(ApplicationFormServiceTest.this.applicationFormRepository).save(patchedApplicationFormEntity); - utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any())); + utilMock.verify(() -> ApplicationFormUtils.updateAuditDetailsAfterFormChange(any(), any(), eq(false))); utilMock.close(); } @@ -722,7 +725,7 @@ void attemptingToPatchApplicationFormThatCantBeFound() { .thenReturn(Optional.empty()); assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService - .patchApplicationForm(SAMPLE_APPLICATION_ID, SAMPLE_PATCH_APPLICATION_DTO)) + .patchApplicationForm(SAMPLE_APPLICATION_ID, SAMPLE_PATCH_APPLICATION_DTO, eq(false))) .isInstanceOf(NotFoundException.class) .hasMessage("Application with id " + SAMPLE_APPLICATION_ID + " does not exist."); } @@ -740,7 +743,7 @@ void attemptingToPatchApplicationFormUnableToSave() { .thenThrow(new RuntimeException()); assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService - .patchApplicationForm(applicationId, SAMPLE_PATCH_APPLICATION_DTO)) + .patchApplicationForm(applicationId, SAMPLE_PATCH_APPLICATION_DTO, false)) .isInstanceOf(ApplicationFormException.class) .hasMessage("Error occured when patching appliction with id of " + applicationId); } @@ -754,11 +757,33 @@ void attemptingToPatchApplicationForm_AccessDenied() { .thenReturn(Optional.of(testApplicationEntity)); assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService - .patchApplicationForm(applicationId, SAMPLE_PATCH_APPLICATION_DTO)) + .patchApplicationForm(applicationId, SAMPLE_PATCH_APPLICATION_DTO, false)) .isInstanceOf(AccessDeniedException.class) .hasMessage("User 1 is unable to access the application form with id " + applicationId); } } + @Nested + class retrieveApplicationsFromScheme { + + @Test + void applicationsArePresent() { + ApplicationFormEntity applicationFormEntity = new ApplicationFormEntity(); + when(applicationFormRepository.findByGrantSchemeId(SAMPLE_SCHEME_ID)) + .thenReturn(Optional.of(applicationFormEntity)); + ApplicationFormEntity response = applicationFormService.getApplicationFromSchemeId(SAMPLE_SCHEME_ID); + + assertThat(response).isEqualTo(applicationFormEntity); + } + + @Test + void applicationsAreNotPresent() { + when(applicationFormRepository.findByGrantSchemeId(SAMPLE_SCHEME_ID)).thenReturn(Optional.empty()); + assertThrows(NoSuchElementException.class, + () -> applicationFormService.getApplicationFromSchemeId(SAMPLE_SCHEME_ID)); + } + + } + } diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/testdata/ApplicationFormTestData.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/testdata/ApplicationFormTestData.java index a1979d25..91b10576 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/testdata/ApplicationFormTestData.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/testdata/ApplicationFormTestData.java @@ -18,6 +18,8 @@ public class ApplicationFormTestData { public final static Integer SAMPLE_APPLICATION_ID = 111; + public final static UUID SAMPLE_ADVERT_ID = UUID.fromString("0-0-0-0-0"); + public final static Integer SAMPLE_SECOND_APPLICATION_ID = 222; public final static Integer SAMPLE_SCHEME_ID = 333; diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtilsTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtilsTest.java index 354d26df..a9c857fb 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtilsTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtilsTest.java @@ -4,11 +4,13 @@ import gov.cabinetoffice.gap.adminbackend.models.AdminSession; import gov.cabinetoffice.gap.adminbackend.testdata.generators.RandomApplicationFormGenerators; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import java.time.Instant; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; class ApplicationFormUtilsTest { @@ -21,11 +23,24 @@ void updateAuditDetailsAfterFormChange_UpdatingExpectedAuditDetails() { AdminSession session = new AdminSession(1, 1, "Test", "User", "AND Digital", "test.user@and.digital", "[FIND, APPLY, ADMIN]"); - ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session); + ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, session, false); assertThat(applicationForm.getLastUpdated()).isAfter(fiveSecondsAgo); assertEquals(session.getGrantAdminId(), applicationForm.getLastUpdateBy()); assertEquals(Integer.valueOf(2), applicationForm.getVersion()); } + @Test + void doesntCallSetLastUpdateByWhenIsLambdaEqualsTrue() { + Instant fiveSecondsAgo = Instant.now().minusSeconds(5); + Integer version = 1; + ApplicationFormEntity applicationForm = Mockito.spy(RandomApplicationFormGenerators + .randomApplicationFormEntity().lastUpdateBy(007).lastUpdated(fiveSecondsAgo).version(version).build()); + Mockito.verify(applicationForm, Mockito.times(0)).setLastUpdateBy(any()); + ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, null, true); + + assertThat(applicationForm.getLastUpdated()).isAfter(fiveSecondsAgo); + assertEquals(Integer.valueOf(2), applicationForm.getVersion()); + } + } From c6a31980b6513891235fc8789aa1fced55631e5a Mon Sep 17 00:00:00 2001 From: GavCookCO <99668051+GavCookCO@users.noreply.github.com> Date: Thu, 9 Nov 2023 18:26:06 +0000 Subject: [PATCH 4/4] Create DUMMY --- DUMMY | 1 + 1 file changed, 1 insertion(+) create mode 100644 DUMMY diff --git a/DUMMY b/DUMMY new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/DUMMY @@ -0,0 +1 @@ +