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 01/20] 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 02/20] 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 03/20] 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 04/20] 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 @@ + From ee6a59816fdc86e375a76749bc788919f57f434c Mon Sep 17 00:00:00 2001 From: a-lor-cab <107372333+a-lor-cab@users.noreply.github.com> Date: Wed, 6 Dec 2023 14:17:56 +0000 Subject: [PATCH 05/20] Tmi2 501/revert tmi2 496 changes (#129) (#130) * revert tmi2-496 changes * format --- .../GrantMandatoryQuestionsController.java | 3 +- .../GrantMandatoryQuestionRepository.java | 27 +++----- .../GrantMandatoryQuestionService.java | 30 ++++----- .../MandatoryQuestionsControllerTest.java | 6 +- .../GrantMandatoryQuestionServiceTest.java | 61 ++++++++----------- 5 files changed, 49 insertions(+), 78 deletions(-) diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/GrantMandatoryQuestionsController.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/GrantMandatoryQuestionsController.java index 305b68ea..bfa69218 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/GrantMandatoryQuestionsController.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/GrantMandatoryQuestionsController.java @@ -33,8 +33,7 @@ public class GrantMandatoryQuestionsController { @GetMapping("/scheme/{schemeId}/complete") public ResponseEntity hasCompletedMandatoryQuestions(@PathVariable Integer schemeId) { - return ResponseEntity.ok( - grantMandatoryQuestionService.hasCompletedMandatoryQuestionsWithSubmittedSubmissionStatus(schemeId)); + return ResponseEntity.ok(grantMandatoryQuestionService.hasCompletedMandatoryQuestions(schemeId)); } @GetMapping("/scheme/{schemeId}/spotlight-complete") diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GrantMandatoryQuestionRepository.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GrantMandatoryQuestionRepository.java index 6ce0156b..623fce11 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GrantMandatoryQuestionRepository.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/GrantMandatoryQuestionRepository.java @@ -10,31 +10,25 @@ public interface GrantMandatoryQuestionRepository extends JpaRepository { @Query("select g " + "from GrantMandatoryQuestions g " + "where g.schemeEntity.id = ?1 " - + "and g.status = gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionStatus.COMPLETED " - + "and g.submission.status = 'SUBMITTED'") - List findBySchemeEntity_IdAndCompletedStatusAndSubmittedSubmissionStatus(Integer id); + + "and g.status = gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionStatus.COMPLETED") + List findBySchemeEntity_IdAndCompletedStatus(Integer id); @Query("select g from GrantMandatoryQuestions g " + "where g.schemeEntity.id = ?1 " + "and g.status = gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionStatus.COMPLETED " + "and g.orgType in (gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.CHARITY, " + "gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.REGISTERED_CHARITY, " + "gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.UNREGISTERED_CHARITY, " - + "gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.LIMITED_COMPANY) " - + "and g.submission.status = 'SUBMITTED'") - List findCharitiesAndCompaniesBySchemeEntityIdAndCompletedStatusAndSubmittedSubmissionStatus( - Integer id); + + "gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.LIMITED_COMPANY)") + List findCharitiesAndCompaniesBySchemeEntityIdAndCompletedStatus(Integer id); @Query("select g from GrantMandatoryQuestions g " + "where g.schemeEntity.id = ?1 " + "and g.status = gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionStatus.COMPLETED " - + "and g.orgType = gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.NON_LIMITED_COMPANY " - + "and g.submission.status = 'SUBMITTED'") - List findNonLimitedCompaniesBySchemeEntityIdAndCompletedStatusAndSubmittedSubmissionStatus( - Integer id); + + "and g.orgType = gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.NON_LIMITED_COMPANY") + List findNonLimitedCompaniesBySchemeEntityIdAndCompletedStatus(Integer id); @Query("select (count(g) > 0) from GrantMandatoryQuestions g where g.schemeEntity.id = ?1 " - + "and g.status = gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionStatus.COMPLETED " - + "and g.submission.status = 'SUBMITTED'") - boolean existsBySchemeEntity_IdAndCompletedStatusAndSubmittedSubmission_Status(Integer id); + + "and g.status = gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionStatus.COMPLETED") + boolean existsBySchemeEntity_IdAndCompletedStatus(Integer id); @Query("select (count(g) > 0) from GrantMandatoryQuestions g " + "where g.schemeEntity.id = ?1 " + "and g.status = gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionStatus.COMPLETED " @@ -42,8 +36,7 @@ List findNonLimitedCompaniesBySchemeEntityIdAndComplete + "gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.REGISTERED_CHARITY, " + "gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.UNREGISTERED_CHARITY, " + "gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.LIMITED_COMPANY, " - + "gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.NON_LIMITED_COMPANY) " - + "and g.submission.status = 'SUBMITTED'") - boolean existsBySchemeEntityIdAndCompletedStatusAndRequiredOrgTypeAndSubmittedSubmissionStatus(Integer id); + + "gov.cabinetoffice.gap.adminbackend.enums.GrantMandatoryQuestionOrgType.NON_LIMITED_COMPANY)") + boolean existsBySchemeEntityIdAndCompleteStatusAndOrgType(Integer id); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantMandatoryQuestionService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantMandatoryQuestionService.java index efbb1173..22cf1aa0 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantMandatoryQuestionService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantMandatoryQuestionService.java @@ -35,27 +35,22 @@ public class GrantMandatoryQuestionService { private final ZipService zipService; - public List getGrantMandatoryQuestionBySchemeAndCompletedStatusAndSubmittedSubmissionStatus( - Integer schemeId) { - return grantMandatoryQuestionRepository - .findBySchemeEntity_IdAndCompletedStatusAndSubmittedSubmissionStatus(schemeId); + public List getGrantMandatoryQuestionBySchemeAndCompletedStatus(Integer schemeId) { + return grantMandatoryQuestionRepository.findBySchemeEntity_IdAndCompletedStatus(schemeId); } - public List getCharitiesAndCompaniesMandatoryQuestionsBySchemeAndCompletedStatusAndSubmittedSubmissionStatus( + public List getCharitiesAndCompaniesMandatoryQuestionsBySchemeAndCompletedStatus( Integer schemeId) { - return grantMandatoryQuestionRepository - .findCharitiesAndCompaniesBySchemeEntityIdAndCompletedStatusAndSubmittedSubmissionStatus(schemeId); + return grantMandatoryQuestionRepository.findCharitiesAndCompaniesBySchemeEntityIdAndCompletedStatus(schemeId); } - public List getNonLimitedCompaniesMandatoryQuestionsBySchemeAndCompletedStatusAndSubmittedSubmissionStatus( + public List getNonLimitedCompaniesMandatoryQuestionsBySchemeAndCompletedStatus( Integer schemeId) { - return grantMandatoryQuestionRepository - .findNonLimitedCompaniesBySchemeEntityIdAndCompletedStatusAndSubmittedSubmissionStatus(schemeId); + return grantMandatoryQuestionRepository.findNonLimitedCompaniesBySchemeEntityIdAndCompletedStatus(schemeId); } public boolean hasCompletedDataForSpotlight(Integer schemeId) { - return grantMandatoryQuestionRepository - .existsBySchemeEntityIdAndCompletedStatusAndRequiredOrgTypeAndSubmittedSubmissionStatus(schemeId); + return grantMandatoryQuestionRepository.existsBySchemeEntityIdAndCompleteStatusAndOrgType(schemeId); } @@ -75,9 +70,9 @@ public ByteArrayOutputStream getValidationErrorChecks(List companiesAndCharitiesQuestions = getCharitiesAndCompaniesMandatoryQuestionsBySchemeAndCompletedStatusAndSubmittedSubmissionStatus( + final List companiesAndCharitiesQuestions = getCharitiesAndCompaniesMandatoryQuestionsBySchemeAndCompletedStatus( schemeId); - final List nonLimitedCompanyQuestions = getNonLimitedCompaniesMandatoryQuestionsBySchemeAndCompletedStatusAndSubmittedSubmissionStatus( + final List nonLimitedCompanyQuestions = getNonLimitedCompaniesMandatoryQuestionsBySchemeAndCompletedStatus( schemeId); return generateZipFile(companiesAndCharitiesQuestions, nonLimitedCompanyQuestions, schemeId); @@ -99,7 +94,7 @@ private ByteArrayOutputStream generateZipFile(List comp } public ByteArrayOutputStream getDueDiligenceData(Integer schemeId) { - final List mandatoryQuestions = getGrantMandatoryQuestionBySchemeAndCompletedStatusAndSubmittedSubmissionStatus( + final List mandatoryQuestions = getGrantMandatoryQuestionBySchemeAndCompletedStatus( schemeId); final List> exportData = exportSpotlightChecks(schemeId, mandatoryQuestions, true); return XlsxGenerator.createResource(DueDiligenceHeaders.DUE_DILIGENCE_HEADERS, exportData); @@ -188,9 +183,8 @@ public String generateExportFileName(Integer schemeId, String orgType) { return dateString + "_" + ggisReference + "_" + schemeName + (orgType == null ? "" : "_" + orgType) + ".xlsx"; } - public boolean hasCompletedMandatoryQuestionsWithSubmittedSubmissionStatus(Integer schemeId) { - return grantMandatoryQuestionRepository - .existsBySchemeEntity_IdAndCompletedStatusAndSubmittedSubmission_Status(schemeId); + public boolean hasCompletedMandatoryQuestions(Integer schemeId) { + return grantMandatoryQuestionRepository.existsBySchemeEntity_IdAndCompletedStatus(schemeId); } } diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/MandatoryQuestionsControllerTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/MandatoryQuestionsControllerTest.java index a2bae670..ba931f9c 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/MandatoryQuestionsControllerTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/MandatoryQuestionsControllerTest.java @@ -55,16 +55,14 @@ class hasCompletedMandatoryQuestions { @Test void hasCompletedMandatoryQuestionsReturnsTrue() throws Exception { - when(grantMandatoryQuestionService.hasCompletedMandatoryQuestionsWithSubmittedSubmissionStatus(SCHEME_ID)) - .thenReturn(true); + when(grantMandatoryQuestionService.hasCompletedMandatoryQuestions(SCHEME_ID)).thenReturn(true); mockMvc.perform(get("/mandatory-questions/scheme/" + SCHEME_ID + "/complete")).andExpect(status().isOk()) .andExpect(content().string("true")); } @Test void hasCompletedMandatoryQuestionsReturnsFalse() throws Exception { - when(grantMandatoryQuestionService.hasCompletedMandatoryQuestionsWithSubmittedSubmissionStatus(SCHEME_ID)) - .thenReturn(false); + when(grantMandatoryQuestionService.hasCompletedMandatoryQuestions(SCHEME_ID)).thenReturn(false); mockMvc.perform(get("/mandatory-questions/scheme/" + SCHEME_ID + "/complete")).andExpect(status().isOk()) .andExpect(content().string("false")); } diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/GrantMandatoryQuestionServiceTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/GrantMandatoryQuestionServiceTest.java index c11d8286..0bcaafac 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/GrantMandatoryQuestionServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/GrantMandatoryQuestionServiceTest.java @@ -90,12 +90,11 @@ class GetGrantMandatoryQuestionBySchemeAndStatusTests { @Test void validSchemeId() { - when(grantMandatoryQuestionRepository - .findBySchemeEntity_IdAndCompletedStatusAndSubmittedSubmissionStatus(SCHEME_ID)) - .thenReturn(List.of(grantMandatoryQuestions)); + when(grantMandatoryQuestionRepository.findBySchemeEntity_IdAndCompletedStatus(SCHEME_ID)) + .thenReturn(List.of(grantMandatoryQuestions)); List result = grantMandatoryQuestionService - .getGrantMandatoryQuestionBySchemeAndCompletedStatusAndSubmittedSubmissionStatus(SCHEME_ID); + .getGrantMandatoryQuestionBySchemeAndCompletedStatus(SCHEME_ID); assertThat(result).isEqualTo(List.of(grantMandatoryQuestions)); @@ -105,13 +104,11 @@ void validSchemeId() { @Test void getCharitiesAndCompaniesMandatoryQuestionsBySchemeAndCompletedStatus() { - when(grantMandatoryQuestionRepository - .findCharitiesAndCompaniesBySchemeEntityIdAndCompletedStatusAndSubmittedSubmissionStatus(SCHEME_ID)) - .thenReturn(List.of(grantMandatoryQuestions)); + when(grantMandatoryQuestionRepository.findCharitiesAndCompaniesBySchemeEntityIdAndCompletedStatus(SCHEME_ID)) + .thenReturn(List.of(grantMandatoryQuestions)); List result = grantMandatoryQuestionService - .getCharitiesAndCompaniesMandatoryQuestionsBySchemeAndCompletedStatusAndSubmittedSubmissionStatus( - SCHEME_ID); + .getCharitiesAndCompaniesMandatoryQuestionsBySchemeAndCompletedStatus(SCHEME_ID); assertThat(result).isEqualTo(List.of(grantMandatoryQuestions)); @@ -119,13 +116,11 @@ void getCharitiesAndCompaniesMandatoryQuestionsBySchemeAndCompletedStatus() { @Test void getNonLimitedCompaniesMandatoryQuestionsBySchemeAndCompletedStatus() { - when(grantMandatoryQuestionRepository - .findNonLimitedCompaniesBySchemeEntityIdAndCompletedStatusAndSubmittedSubmissionStatus(SCHEME_ID)) - .thenReturn(List.of(grantMandatoryQuestionsNonLimitedCompany)); + when(grantMandatoryQuestionRepository.findNonLimitedCompaniesBySchemeEntityIdAndCompletedStatus(SCHEME_ID)) + .thenReturn(List.of(grantMandatoryQuestionsNonLimitedCompany)); List result = grantMandatoryQuestionService - .getNonLimitedCompaniesMandatoryQuestionsBySchemeAndCompletedStatusAndSubmittedSubmissionStatus( - SCHEME_ID); + .getNonLimitedCompaniesMandatoryQuestionsBySchemeAndCompletedStatus(SCHEME_ID); assertThat(result).isEqualTo(List.of(grantMandatoryQuestionsNonLimitedCompany)); @@ -170,11 +165,10 @@ class getSpotlightChecks { @Test void getSpotlightChecks() { when(grantMandatoryQuestionRepository - .findCharitiesAndCompaniesBySchemeEntityIdAndCompletedStatusAndSubmittedSubmissionStatus(SCHEME_ID)) + .findCharitiesAndCompaniesBySchemeEntityIdAndCompletedStatus(SCHEME_ID)) .thenReturn(List.of(grantMandatoryQuestions)); - when(grantMandatoryQuestionRepository - .findNonLimitedCompaniesBySchemeEntityIdAndCompletedStatusAndSubmittedSubmissionStatus(SCHEME_ID)) - .thenReturn(List.of(grantMandatoryQuestionsNonLimitedCompany)); + when(grantMandatoryQuestionRepository.findNonLimitedCompaniesBySchemeEntityIdAndCompletedStatus(SCHEME_ID)) + .thenReturn(List.of(grantMandatoryQuestionsNonLimitedCompany)); when(schemeService.getSchemeBySchemeId(SCHEME_ID)).thenReturn(schemeDTO); when(zipService.createZip(anyList(), anyList(), anyList())).thenReturn(new ByteArrayOutputStream()); doReturn(EXPECTED_SPOTLIGHT_ROW).when(grantMandatoryQuestionService) @@ -213,9 +207,8 @@ private static void assertRowIsAsExpected(Row actualRow, List expectedRo @Test void forSingleRowWithGoodData() throws IOException { - when(grantMandatoryQuestionRepository - .findBySchemeEntity_IdAndCompletedStatusAndSubmittedSubmissionStatus(SCHEME_ID)) - .thenReturn(List.of(grantMandatoryQuestions)); + when(grantMandatoryQuestionRepository.findBySchemeEntity_IdAndCompletedStatus(SCHEME_ID)) + .thenReturn(List.of(grantMandatoryQuestions)); doReturn(EXPECTED_DUE_DILIGENCE_ROW).when(grantMandatoryQuestionService) .buildSingleSpotlightRow(grantMandatoryQuestions, true); @@ -419,21 +412,17 @@ class doesSchemeHaveCompletedMandatoryQuestions { @Test void returnsTrue() { - when(grantMandatoryQuestionRepository - .existsBySchemeEntity_IdAndCompletedStatusAndSubmittedSubmission_Status(SCHEME_ID)) - .thenReturn(true); - boolean result = grantMandatoryQuestionService - .hasCompletedMandatoryQuestionsWithSubmittedSubmissionStatus(SCHEME_ID); + when(grantMandatoryQuestionRepository.existsBySchemeEntity_IdAndCompletedStatus(SCHEME_ID)) + .thenReturn(true); + boolean result = grantMandatoryQuestionService.hasCompletedMandatoryQuestions(SCHEME_ID); assertThat(result).isEqualTo(true); } @Test void returnFalse() { - when(grantMandatoryQuestionRepository - .existsBySchemeEntity_IdAndCompletedStatusAndSubmittedSubmission_Status(SCHEME_ID)) - .thenReturn(false); - boolean result = grantMandatoryQuestionService - .hasCompletedMandatoryQuestionsWithSubmittedSubmissionStatus(SCHEME_ID); + when(grantMandatoryQuestionRepository.existsBySchemeEntity_IdAndCompletedStatus(SCHEME_ID)) + .thenReturn(false); + boolean result = grantMandatoryQuestionService.hasCompletedMandatoryQuestions(SCHEME_ID); assertThat(result).isEqualTo(false); } @@ -444,18 +433,16 @@ class doesSchemeHaveCompletedDataForSpotlight { @Test void returnsTrue() { - when(grantMandatoryQuestionRepository - .existsBySchemeEntityIdAndCompletedStatusAndRequiredOrgTypeAndSubmittedSubmissionStatus(SCHEME_ID)) - .thenReturn(true); + when(grantMandatoryQuestionRepository.existsBySchemeEntityIdAndCompleteStatusAndOrgType(SCHEME_ID)) + .thenReturn(true); boolean result = grantMandatoryQuestionService.hasCompletedDataForSpotlight(SCHEME_ID); assertThat(result).isEqualTo(true); } @Test void returnFalse() { - when(grantMandatoryQuestionRepository - .existsBySchemeEntityIdAndCompletedStatusAndRequiredOrgTypeAndSubmittedSubmissionStatus(SCHEME_ID)) - .thenReturn(false); + when(grantMandatoryQuestionRepository.existsBySchemeEntityIdAndCompleteStatusAndOrgType(SCHEME_ID)) + .thenReturn(false); boolean result = grantMandatoryQuestionService.hasCompletedDataForSpotlight(SCHEME_ID); assertThat(result).isEqualTo(false); } From 3320f8275d845c7f6afb3041ab20ba327efdf758 Mon Sep 17 00:00:00 2001 From: Connor Macqueen <138442814+ConnorTCO@users.noreply.github.com> Date: Tue, 12 Dec 2023 09:04:00 +0000 Subject: [PATCH 06/20] Release/6.1 (#136) * Tmi2 501/revert tmi2 496 changes (#129) * revert tmi2-496 changes * format * Adding tables for completion_statistics.sql (#131) * Adding tables for completion_statistics.sql * Formatting... * Tn 154 calculations tables (#132) * Adding tables for completion_statistics.sql * Formatting... * Adding sequence for calculations table id * Tn 154 calculations tables (#133) * Adding tables for completion_statistics.sql * Formatting... * Adding sequence for calculations table id * Fixing incorrect format for migration script name. Combining 2 into 1 * Fixing issue with db migration script (#134) * Adding tables for completion_statistics.sql * Formatting... * Adding sequence for calculations table id * Fixing incorrect format for migration script name. Combining 2 into 1 * Fixing errors in db migration script --------- Co-authored-by: a-lor-cab <107372333+a-lor-cab@users.noreply.github.com> --- ..._adding_calculations_statistics_tables.sql | 39 ++++ .../services/EventLogServiceTest.java | 193 ++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 src/main/resources/db/migration/V1_80__adding_calculations_statistics_tables.sql diff --git a/src/main/resources/db/migration/V1_80__adding_calculations_statistics_tables.sql b/src/main/resources/db/migration/V1_80__adding_calculations_statistics_tables.sql new file mode 100644 index 00000000..e5446d3a --- /dev/null +++ b/src/main/resources/db/migration/V1_80__adding_calculations_statistics_tables.sql @@ -0,0 +1,39 @@ +CREATE TABLE IF NOT EXISTS event_stream.completion_statistics +( + id bigint NOT NULL, + user_sub character varying(128) COLLATE pg_catalog."default" NOT NULL, + funding_organisation_id bigint, + object_type character varying(128) COLLATE pg_catalog."default" NOT NULL, + object_id character varying COLLATE pg_catalog."default" NOT NULL, + total_alive_time bigint NOT NULL, + time_worked_on bigint NOT NULL, + object_completed timestamp(6) with time zone, + created timestamp(6) with time zone NOT NULL, + CONSTRAINT completion_statistics_pkey PRIMARY KEY (id) +); + +CREATE INDEX IF NOT EXISTS completion_statistics_funding_org_id_index + ON event_stream.completion_statistics USING btree + (funding_organisation_id ASC NULLS LAST); + +CREATE INDEX IF NOT EXISTS completion_statistics_object_id_index + ON event_stream.completion_statistics USING btree + (object_id COLLATE pg_catalog."default" ASC NULLS LAST); + +CREATE INDEX IF NOT EXISTS completion_statistics_object_id_user_sub_index + ON event_stream.completion_statistics USING btree + (object_id COLLATE pg_catalog."default" ASC NULLS LAST, user_sub COLLATE pg_catalog."default" ASC NULLS LAST); + +CREATE INDEX IF NOT EXISTS completion_statistics_user_sub_index + ON event_stream.completion_statistics USING btree + (user_sub COLLATE pg_catalog."default" ASC NULLS LAST); + + + +-- Sequence creation +CREATE SEQUENCE IF NOT EXISTS event_stream.completion_statistics_id_seq + INCREMENT 1 + START 1 + MINVALUE 1 + MAXVALUE 9223372036854775807 + CACHE 1; diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/EventLogServiceTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/EventLogServiceTest.java index 80d95f01..e7bb9903 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/EventLogServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/EventLogServiceTest.java @@ -245,4 +245,197 @@ public void queueDisabled(InfoLogCapture logCapture) { } + @Nested + class logApplicationCreatedEvent { + + @Test + public void success(InfoLogCapture logCapture) throws JsonProcessingException { + + String sessionId = "SessionId"; + String userSub = "UserSub"; + long fundingOrgId = 1L; + String objectId = "ObjectId"; + + when(objectMapper.writeValueAsString(eventLogArgumentCaptor.capture())).thenReturn(""); + when(amazonSQS.sendMessage(anyString(), any())).thenReturn(null); + + eventLogService.logApplicationCreatedEvent(sessionId, userSub, fundingOrgId, objectId); + + EventLog actualEventLog = eventLogArgumentCaptor.getValue(); + + assertThat(actualEventLog.getEventType()).isEqualTo(EventType.APPLICATION_CREATED); + assertThat(actualEventLog.getFundingOrganisationId()).isEqualTo(fundingOrgId); + assertThat(actualEventLog.getSessionId()).isEqualTo(sessionId); + assertThat(actualEventLog.getUserSub()).isEqualTo(userSub); + assertThat(actualEventLog.getObjectId()).isEqualTo(objectId); + assertThat(actualEventLog.getObjectType()).isEqualTo(ObjectType.APPLICATION); + assertThat(actualEventLog.getTimestamp()).isEqualTo(clock.instant()); + + assertThat(logCapture.getLoggingEventAt(1).getFormattedMessage()) + .isEqualTo(EventType.APPLICATION_CREATED + " Message sent successfully"); + } + + @Test + public void cantSendToSQS(ErrorLogCapture logCapture) throws JsonProcessingException { + String sessionId = "SessionId"; + String userSub = "UserSub"; + long fundingOrgId = 1L; + String objectId = "ObjectId"; + + String expectedMessageBody = "MessageBody"; + when(objectMapper.writeValueAsString(any(EventLog.class))).thenReturn(expectedMessageBody); + when(amazonSQS.sendMessage(anyString(), anyString())).thenThrow(AmazonSQSException.class); + + eventLogService.logApplicationCreatedEvent(sessionId, userSub, fundingOrgId, objectId); + + assertThat(logCapture.getLoggingEventAt(0).getFormattedMessage()) + .startsWith("Message failed to send for event log"); + + } + + @Test + public void queueDisabled(InfoLogCapture logCapture) { + eventLogService = new EventLogService("eventLogQueue", false, amazonSQS, objectMapper, clock); + String sessionId = "SessionId"; + String userSub = "UserSub"; + long fundingOrgId = 1L; + String objectId = "ObjectId"; + + eventLogService.logApplicationCreatedEvent(sessionId, userSub, fundingOrgId, objectId); + + assertThat(logCapture.getLoggingEventAt(0).getFormattedMessage()) + .isEqualTo("Event Service Queue is disabled. Returning without sending."); + verifyNoInteractions(amazonSQS, objectMapper); + } + + } + + @Nested + class logApplicationUpdatedEvent { + + @Test + public void success(InfoLogCapture logCapture) throws JsonProcessingException { + String sessionId = "SessionId"; + String userSub = "UserSub"; + long fundingOrgId = 1L; + String objectId = "ObjectId"; + + when(objectMapper.writeValueAsString(eventLogArgumentCaptor.capture())).thenReturn(""); + when(amazonSQS.sendMessage(anyString(), any())).thenReturn(null); + + eventLogService.logApplicationUpdatedEvent(sessionId, userSub, fundingOrgId, objectId); + + EventLog actualEventLog = eventLogArgumentCaptor.getValue(); + + assertThat(actualEventLog.getEventType()).isEqualTo(EventType.APPLICATION_UPDATED); + assertThat(actualEventLog.getFundingOrganisationId()).isEqualTo(fundingOrgId); + assertThat(actualEventLog.getSessionId()).isEqualTo(sessionId); + assertThat(actualEventLog.getUserSub()).isEqualTo(userSub); + assertThat(actualEventLog.getObjectId()).isEqualTo(objectId); + assertThat(actualEventLog.getObjectType()).isEqualTo(ObjectType.APPLICATION); + assertThat(actualEventLog.getTimestamp()).isEqualTo(clock.instant()); + + assertThat(logCapture.getLoggingEventAt(1).getFormattedMessage()) + .isEqualTo(EventType.APPLICATION_UPDATED + " Message sent successfully"); + } + + @Test + public void cantSendToSQS(ErrorLogCapture logCapture) throws JsonProcessingException { + String sessionId = "SessionId"; + String userSub = "UserSub"; + long fundingOrgId = 1L; + String objectId = "ObjectId"; + + String expectedMessageBody = "MessageBody"; + when(objectMapper.writeValueAsString(any(EventLog.class))).thenReturn(expectedMessageBody); + when(amazonSQS.sendMessage(anyString(), eq(expectedMessageBody))).thenThrow(AmazonSQSException.class); + + eventLogService.logApplicationUpdatedEvent(sessionId, userSub, fundingOrgId, objectId); + + assertThat(logCapture.getLoggingEventAt(0).getFormattedMessage()) + .startsWith("Message failed to send for event log"); + } + + @Test + public void queueDisabled(InfoLogCapture logCapture) { + ReflectionTestUtils.setField(eventLogService, "eventServiceQueueEnabled", false); + String sessionId = "SessionId"; + String userSub = "UserSub"; + long fundingOrgId = 1L; + String objectId = "ObjectId"; + + eventLogService.logApplicationUpdatedEvent(sessionId, userSub, fundingOrgId, objectId); + + assertThat(logCapture.getLoggingEventAt(0).getFormattedMessage()) + .isEqualTo("Event Service Queue is disabled. Returning without sending."); + + verifyNoInteractions(amazonSQS, objectMapper); + } + + } + + @Nested + class logApplicationPublishedEvent { + + @Test + public void success(InfoLogCapture logCapture) throws JsonProcessingException { + String sessionId = "SessionId"; + String userSub = "UserSub"; + long fundingOrgId = 1L; + String objectId = "ObjectId"; + + when(objectMapper.writeValueAsString(eventLogArgumentCaptor.capture())).thenReturn(""); + when(amazonSQS.sendMessage(anyString(), any())).thenReturn(null); + + eventLogService.logApplicationPublishedEvent(sessionId, userSub, fundingOrgId, objectId); + + EventLog actualEventLog = eventLogArgumentCaptor.getValue(); + + assertThat(actualEventLog.getEventType()).isEqualTo(EventType.APPLICATION_PUBLISHED); + assertThat(actualEventLog.getFundingOrganisationId()).isEqualTo(fundingOrgId); + assertThat(actualEventLog.getSessionId()).isEqualTo(sessionId); + assertThat(actualEventLog.getUserSub()).isEqualTo(userSub); + assertThat(actualEventLog.getObjectId()).isEqualTo(objectId); + assertThat(actualEventLog.getObjectType()).isEqualTo(ObjectType.APPLICATION); + assertThat(actualEventLog.getTimestamp()).isEqualTo(clock.instant()); + + assertThat(logCapture.getLoggingEventAt(1).getFormattedMessage()) + .isEqualTo(EventType.APPLICATION_PUBLISHED + " Message sent successfully"); + + } + + @Test + public void cantSendToSQS(ErrorLogCapture logCapture) throws JsonProcessingException { + String sessionId = "SessionId"; + String userSub = "UserSub"; + long fundingOrgId = 1L; + String objectId = "ObjectId"; + + String expectedMessageBody = "MessageBody"; + when(objectMapper.writeValueAsString(any(EventLog.class))).thenReturn(expectedMessageBody); + when(amazonSQS.sendMessage(anyString(), eq(expectedMessageBody))).thenThrow(AmazonSQSException.class); + + eventLogService.logApplicationPublishedEvent(sessionId, userSub, fundingOrgId, objectId); + + assertThat(logCapture.getLoggingEventAt(0).getFormattedMessage()) + .startsWith("Message failed to send for event log"); + } + + @Test + public void queueDisabled(InfoLogCapture logCapture) { + ReflectionTestUtils.setField(eventLogService, "eventServiceQueueEnabled", false); + String sessionId = "SessionId"; + String userSub = "UserSub"; + long fundingOrgId = 1L; + String objectId = "ObjectId"; + + eventLogService.logApplicationPublishedEvent(sessionId, userSub, fundingOrgId, objectId); + + assertThat(logCapture.getLoggingEventAt(0).getFormattedMessage()) + .isEqualTo("Event Service Queue is disabled. Returning without sending."); + verifyNoInteractions(amazonSQS, objectMapper); + } + + } + } \ No newline at end of file From fd69ec07d647d60a24d4a71dbec380b1f47f607b Mon Sep 17 00:00:00 2001 From: dominicwest <101722961+dominicwest@users.noreply.github.com> Date: Wed, 7 Feb 2024 18:19:54 +0000 Subject: [PATCH 07/20] Fix promoteToProd.yml (#179) --- .github/workflows/promoteToProd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/promoteToProd.yml b/.github/workflows/promoteToProd.yml index f70c95da..ce84da49 100644 --- a/.github/workflows/promoteToProd.yml +++ b/.github/workflows/promoteToProd.yml @@ -24,7 +24,7 @@ jobs: with: role-to-assume: ${{ secrets.AWS_ROLE_ARN }} role-session-name: github-gap-find-admin-backend - aws-region: ${{ vars.AWS_REGION }} + aws-region: ${{ env.AWS_REGION }} - name: Login to AWS ECR id: login-ecr From 4f5b3dd7eaabde83f51489fd2c3b1db3eeba35e1 Mon Sep 17 00:00:00 2001 From: dylanwrightCO <135221918+dylanwrightCO@users.noreply.github.com> Date: Mon, 26 Feb 2024 13:53:01 +0000 Subject: [PATCH 08/20] Adds query to find application with no ownership check to allow lambdas to call (#212) --- .../repositories/ApplicationFormRepository.java | 3 +++ .../services/ApplicationFormService.java | 2 +- .../services/ApplicationFormServiceTest.java | 15 +++++++++------ 3 files changed, 13 insertions(+), 7 deletions(-) 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 ce9238dc..50474802 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/ApplicationFormRepository.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/ApplicationFormRepository.java @@ -18,6 +18,9 @@ public interface ApplicationFormRepository extends JpaRepository findById(Integer applicationId); + @Query("SELECT af FROM ApplicationFormEntity af WHERE af.id = :applicationId") + Optional findByIdWithNoOwnershipCheck(Integer applicationId); + Optional findByGrantApplicationId(Integer applicationId); Optional findByGrantSchemeId(Integer grantSchemeId); 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 03076262..e46e02c8 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java @@ -340,7 +340,7 @@ public ApplicationFormQuestionDTO retrieveQuestion(Integer applicationId, String public void patchApplicationForm(Integer applicationId, ApplicationFormPatchDTO patchDTO, boolean isLambdaCall) { AdminSession session = null; - ApplicationFormEntity application = this.applicationFormRepository.findById(applicationId) + ApplicationFormEntity application = this.applicationFormRepository.findByIdWithNoOwnershipCheck(applicationId) .orElseThrow(() -> new NotFoundException("Application with id " + applicationId + " does not exist.")); if (!isLambdaCall) { 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 1effe672..69c429a3 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java @@ -388,7 +388,8 @@ void patchQuestionValuesOptionsHappyPathTest() { @Test void patchQuestionValuesApplicationNotFoundPathTest() { - Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository.findById(SAMPLE_APPLICATION_ID)) + Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository + .findByIdWithNoOwnershipCheck(SAMPLE_APPLICATION_ID)) .thenReturn(Optional.empty()); assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService.patchQuestionValues( @@ -754,7 +755,8 @@ void successfullyPatchApplicationForm() { final ApplicationFormEntity patchedApplicationFormEntity = randomApplicationFormEntity() .applicationStatus(ApplicationStatusEnum.PUBLISHED).build(); - Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository.findById(applicationId)) + Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository + .findByIdWithNoOwnershipCheck(applicationId)) .thenReturn(Optional.of(testApplicationFormEntity)); Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository.save(patchedApplicationFormEntity)) .thenReturn(patchedApplicationFormEntity); @@ -762,7 +764,7 @@ void successfullyPatchApplicationForm() { ApplicationFormServiceTest.this.applicationFormService.patchApplicationForm(applicationId, SAMPLE_PATCH_APPLICATION_DTO, false); - verify(ApplicationFormServiceTest.this.applicationFormRepository).findById(applicationId); + verify(ApplicationFormServiceTest.this.applicationFormRepository).findByIdWithNoOwnershipCheck(applicationId); verify(ApplicationFormServiceTest.this.applicationFormMapper) .updateApplicationEntityFromPatchDto(SAMPLE_PATCH_APPLICATION_DTO, testApplicationFormEntity); verify(ApplicationFormServiceTest.this.applicationFormRepository).save(patchedApplicationFormEntity); @@ -790,7 +792,8 @@ void attemptingToPatchApplicationFormUnableToSave() { final ApplicationFormEntity patchedApplicationFormEntity = randomApplicationFormEntity() .applicationStatus(ApplicationStatusEnum.PUBLISHED).build(); - Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository.findById(applicationId)) + Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository + .findByIdWithNoOwnershipCheck(applicationId)) .thenReturn(Optional.of(testApplicationFormEntity)); Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository.save(patchedApplicationFormEntity)) .thenThrow(new RuntimeException()); @@ -806,8 +809,8 @@ void attemptingToPatchApplicationForm_AccessDenied() { ApplicationFormEntity testApplicationEntity = randomApplicationFormEntity().createdBy(2).build(); Integer applicationId = testApplicationEntity.getGrantApplicationId(); - Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository.findById(applicationId)) - .thenReturn(Optional.of(testApplicationEntity)); + Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository + .findByIdWithNoOwnershipCheck(applicationId)).thenReturn(Optional.of(testApplicationEntity)); assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService .patchApplicationForm(applicationId, SAMPLE_PATCH_APPLICATION_DTO, false)) From f5edb8bf567e9ae644b1d8a5fd53f70d928e8320 Mon Sep 17 00:00:00 2001 From: dominicwest <101722961+dominicwest@users.noreply.github.com> Date: Wed, 27 Mar 2024 11:08:10 +0000 Subject: [PATCH 09/20] Fix unpublish application form from lambda (#268) (#269) --- .../gap/adminbackend/services/ApplicationFormService.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 6bca1540..0745d37d 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java @@ -25,6 +25,8 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.math.NumberUtils; import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; import org.odftoolkit.odfdom.doc.OdfTextDocument; @@ -63,9 +65,11 @@ public class ApplicationFormService { public ApplicationFormEntity save(ApplicationFormEntity applicationForm) { final ApplicationFormEntity savedApplicationForm = applicationFormRepository.save(applicationForm); - Optional.ofNullable(HelperUtils.getAdminSessionForAuthenticatedUser()) - .ifPresentOrElse(adminSession -> { + final Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + Optional.ofNullable(auth) + .ifPresentOrElse(authentication -> { if (!HelperUtils.isAnonymousSession()) { + final AdminSession adminSession = (AdminSession) authentication.getPrincipal(); final SchemeEntity grantScheme = schemeRepository.findById(applicationForm.getGrantSchemeId()) .orElseThrow(() -> new EntityNotFoundException("Could not find grant scheme with ID" + applicationForm.getGrantSchemeId())); From b3ef7527da60566f7d8257257565dd6282997249 Mon Sep 17 00:00:00 2001 From: dominicwest <101722961+dominicwest@users.noreply.github.com> Date: Mon, 1 Apr 2024 16:45:12 +0100 Subject: [PATCH 10/20] Updating release 10.0 with develop changes (#274) * Fix unpublish application form from lambda (#268) * GAP-2531: Cascade delete grant export tables createdby column (#270) * GAP-2533: Fix advert scheduler view for BST (#273) * use LocalDateTime for grant opening and closing times (#272) * use LocalDateTime for grant opening and closing times * fix test * fix more tests * Tmi2 740 - Handling new locations for MQ saving to grant_beneficiary table (#271) * Adding two new columns to grant_beneficiary for new locations --------- Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> Co-authored-by: Connor Macqueen <138442814+ConnorTCO@users.noreply.github.com> --- ...dvertPublishingInformationResponseDTO.java | 5 +++-- .../adminbackend/entities/GrantAdvert.java | 5 +++-- .../services/GrantAdvertService.java | 16 +++++++------- ..._100__adding_new_grant_benef_locations.sql | 3 +++ .../V1_98__cascade_delete_export_rows.sql | 15 +++++++++++++ ...eduled_publishing_daylight_savings_fix.sql | 21 +++++++++++++++++++ .../GrantAdvertControllerTest.java | 5 +++-- .../services/GrantAdvertServiceTest.java | 10 ++++----- 8 files changed, 59 insertions(+), 21 deletions(-) create mode 100644 src/main/resources/db/migration/V1_100__adding_new_grant_benef_locations.sql create mode 100644 src/main/resources/db/migration/V1_98__cascade_delete_export_rows.sql create mode 100644 src/main/resources/db/migration/V1_99__scheduled_publishing_daylight_savings_fix.sql diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/dtos/grantadvert/GetGrantAdvertPublishingInformationResponseDTO.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/dtos/grantadvert/GetGrantAdvertPublishingInformationResponseDTO.java index 130a5894..f6d124ec 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/dtos/grantadvert/GetGrantAdvertPublishingInformationResponseDTO.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/dtos/grantadvert/GetGrantAdvertPublishingInformationResponseDTO.java @@ -8,6 +8,7 @@ import java.time.Instant; +import java.time.LocalDateTime; import java.util.UUID; @Data @@ -18,9 +19,9 @@ public class GetGrantAdvertPublishingInformationResponseDTO { private UUID grantAdvertId; - private Instant openingDate; + private LocalDateTime openingDate; - private Instant closingDate; + private LocalDateTime closingDate; private Instant firstPublishedDate; diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/entities/GrantAdvert.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/entities/GrantAdvert.java index 6dc9d1be..f754a75b 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/entities/GrantAdvert.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/entities/GrantAdvert.java @@ -12,6 +12,7 @@ import javax.persistence.*; import java.time.Instant; +import java.time.LocalDateTime; import java.util.UUID; @EntityListeners(AuditingEntityListener.class) @@ -60,10 +61,10 @@ public class GrantAdvert extends BaseEntity { private GrantAdmin lastUpdatedBy; @Column(name = "opening_date") - private Instant openingDate; + private LocalDateTime openingDate; @Column(name = "closing_date") - private Instant closingDate; + private LocalDateTime closingDate; @Column(name = "first_published_date") private Instant firstPublishedDate; 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 5a76c0df..c7209213 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertService.java @@ -574,18 +574,16 @@ private void updateGrantAdvertApplicationDates(final GrantAdvert grantAdvert) { int[] closingResponse = Arrays.stream(closingDateQuestion.getMultiResponse()).mapToInt(Integer::parseInt) .toArray(); - // build LocalDateTimes, convert to Instant - Instant openingDateInstant = LocalDateTime - .of(openingResponse[2], openingResponse[1], openingResponse[0], openingResponse[3], openingResponse[4]) - .atZone(ZoneId.of("Z")).toInstant(); + // build LocalDateTimes + LocalDateTime openingDate = LocalDateTime + .of(openingResponse[2], openingResponse[1], openingResponse[0], openingResponse[3], openingResponse[4]); - Instant closingDateInstant = LocalDateTime - .of(closingResponse[2], closingResponse[1], closingResponse[0], closingResponse[3], closingResponse[4]) - .atZone(ZoneId.of("Z")).toInstant(); + LocalDateTime closingDate = LocalDateTime + .of(closingResponse[2], closingResponse[1], closingResponse[0], closingResponse[3], closingResponse[4]); // set dates on advert - grantAdvert.setOpeningDate(openingDateInstant); - grantAdvert.setClosingDate(closingDateInstant); + grantAdvert.setOpeningDate(openingDate); + grantAdvert.setClosingDate(closingDate); } public void unscheduleGrantAdvert(final UUID advertId) { diff --git a/src/main/resources/db/migration/V1_100__adding_new_grant_benef_locations.sql b/src/main/resources/db/migration/V1_100__adding_new_grant_benef_locations.sql new file mode 100644 index 00000000..3ff99f33 --- /dev/null +++ b/src/main/resources/db/migration/V1_100__adding_new_grant_benef_locations.sql @@ -0,0 +1,3 @@ +ALTER TABLE grant_beneficiary +ADD COLUMN location_lon boolean NULL, +ADD COLUMN location_out_uk boolean NULL; diff --git a/src/main/resources/db/migration/V1_98__cascade_delete_export_rows.sql b/src/main/resources/db/migration/V1_98__cascade_delete_export_rows.sql new file mode 100644 index 00000000..7924f6f6 --- /dev/null +++ b/src/main/resources/db/migration/V1_98__cascade_delete_export_rows.sql @@ -0,0 +1,15 @@ +alter table public.grant_export +drop constraint grant_export_created_by_fkey, +add constraint grant_export_created_by_fkey + foreign key (created_by) + references public.grant_admin (grant_admin_id) match simple + on delete cascade + on update no action; + +alter table public.grant_export_batch +drop constraint grant_export_batch_created_by_fkey, +add constraint grant_export_batch_created_by_fkey + foreign key (created_by) + references public.grant_admin (grant_admin_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_99__scheduled_publishing_daylight_savings_fix.sql b/src/main/resources/db/migration/V1_99__scheduled_publishing_daylight_savings_fix.sql new file mode 100644 index 00000000..befb053f --- /dev/null +++ b/src/main/resources/db/migration/V1_99__scheduled_publishing_daylight_savings_fix.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 'Europe/London') <= NOW() AT TIME ZONE 'Europe/London' + THEN 'PUBLISH' + WHEN + STATUS = 'PUBLISHED' + AND (CLOSING_DATE AT TIME ZONE 'Europe/London') <= NOW() AT TIME ZONE 'Europe/London' + THEN 'UNPUBLISH' + END AS ACTION +FROM GRANT_ADVERT +WHERE ( + STATUS = 'SCHEDULED' AND (OPENING_DATE AT TIME ZONE 'Europe/London') <= NOW() AT TIME ZONE 'Europe/London' +) OR ( + STATUS = 'PUBLISHED' AND (CLOSING_DATE AT TIME ZONE 'Europe/London') <= (NOW() AT TIME ZONE 'Europe/London') +); \ No newline at end of file diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/GrantAdvertControllerTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/GrantAdvertControllerTest.java index 70a48bc3..02ebd2a5 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/GrantAdvertControllerTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/controllers/GrantAdvertControllerTest.java @@ -34,6 +34,7 @@ import javax.validation.Validator; import java.time.Instant; +import java.time.LocalDateTime; import java.util.Collections; import java.util.Set; import java.util.UUID; @@ -448,9 +449,9 @@ class getAdvertPublishInformation { private final Instant dateTimeInput = Instant.parse("2022-01-01T00:00:00.00Z"); - private final Instant openingDate = dateTimeInput; + private final LocalDateTime openingDate = LocalDateTime.parse("2022-01-01T00:00:00.00"); - private final Instant closingDate = dateTimeInput; + private final LocalDateTime closingDate = LocalDateTime.parse("2022-01-01T00:00:00.00"); private final Instant firstPublishedDate = dateTimeInput; diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertServiceTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertServiceTest.java index 7ce06cb0..22c6359f 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertServiceTest.java @@ -738,14 +738,13 @@ class publishAdvert { .id("grantApplicationOpenDate").seen(true) .multiResponse(new String[] { "10", "12", "2022", "00", "01" }).build(); - final Instant openingDate = LocalDateTime.of(2022, 12, 10, 0, 1).atZone(ZoneId.of("Europe/London")).toInstant(); + final LocalDateTime openingDate = LocalDateTime.of(2022, 12, 10, 0, 1); final GrantAdvertQuestionResponse response5 = GrantAdvertQuestionResponse.builder() .id("grantApplicationCloseDate").seen(true) .multiResponse(new String[] { "10", "12", "2023", "23", "59" }).build(); - final Instant closingDate = LocalDateTime.of(2023, 12, 10, 23, 59).atZone(ZoneId.of("Europe/London")) - .toInstant(); + final LocalDateTime closingDate = LocalDateTime.of(2023, 12, 10, 23, 59); final GrantAdvertQuestionResponse response6 = GrantAdvertQuestionResponse.builder().id("grantTotalAwardAmount") .response("1000000").seen(true).build(); @@ -1224,14 +1223,13 @@ class scheduleGrantAdvert { .id("grantApplicationOpenDate").seen(true).multiResponse(new String[] { "10", "12", "2022", "0", "1" }) .build(); - final Instant openingDate = LocalDateTime.of(2022, 12, 10, 0, 1).atZone(ZoneId.of("Europe/London")).toInstant(); + final LocalDateTime openingDate = LocalDateTime.of(2022, 12, 10, 0, 1); final GrantAdvertQuestionResponse response5 = GrantAdvertQuestionResponse.builder() .id("grantApplicationCloseDate").seen(true) .multiResponse(new String[] { "10", "12", "2023", "23", "59" }).build(); - final Instant closingDate = LocalDateTime.of(2023, 12, 10, 23, 59).atZone(ZoneId.of("Europe/London")) - .toInstant(); + final LocalDateTime closingDate = LocalDateTime.of(2023, 12, 10, 23, 59); final GrantAdvertPageResponse datesPage = GrantAdvertPageResponse.builder().id("1") .status(GrantAdvertPageResponseStatus.COMPLETED).questions(List.of(response4, response5)).build(); From 54e1ef979cfba4485972d0ff68fae55096cb6846 Mon Sep 17 00:00:00 2001 From: dominicwest <101722961+dominicwest@users.noreply.github.com> Date: Tue, 2 Apr 2024 15:12:09 +0100 Subject: [PATCH 11/20] GAP-2533: Retrospectively fix BST ads in prod (#276) (#278) * GAP-2533: Retrospectively fix BST ads in prod * GAP-2533: Including release time * GAP-2533: Overriding UTC opening/closing dates as Europe/London time * GAP-2533: Update scheduled closing times too * GAP-2533: Fix syntax * GAP-2533: Hard code datetimes for BST --- .../V1_101__retrospectively_fix_bst_ads.sql | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/main/resources/db/migration/V1_101__retrospectively_fix_bst_ads.sql diff --git a/src/main/resources/db/migration/V1_101__retrospectively_fix_bst_ads.sql b/src/main/resources/db/migration/V1_101__retrospectively_fix_bst_ads.sql new file mode 100644 index 00000000..a593eac1 --- /dev/null +++ b/src/main/resources/db/migration/V1_101__retrospectively_fix_bst_ads.sql @@ -0,0 +1,13 @@ +UPDATE grant_advert + SET opening_date = opening_date - INTERVAL '1 hour' +WHERE opening_date > '2024-03-31 01:00:00'::TIMESTAMP + AND opening_date < '2024-10-27 02:00:00'::TIMESTAMP + AND created > '2023-09-28 12:00:00'::TIMESTAMP + AND status = 'SCHEDULED'; + +UPDATE grant_advert + SET closing_date = closing_date - INTERVAL '1 hour' +WHERE closing_date > '2024-03-31 01:00:00'::TIMESTAMP + AND closing_date < '2024-10-27 02:00:00'::TIMESTAMP + AND created > '2023-09-28 12:00:00'::TIMESTAMP + AND (status = 'PUBLISHED' OR status = 'SCHEDULED'); \ No newline at end of file From 535af9a5913bc03377c5566e127ad2ba3f45b365 Mon Sep 17 00:00:00 2001 From: dylanwrightCO <135221918+dylanwrightCO@users.noreply.github.com> Date: Tue, 9 Apr 2024 16:36:41 +0100 Subject: [PATCH 12/20] Improves logs around updating and deleting open search (#280) --- .../gap/adminbackend/services/OpenSearchService.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java index ce9a9284..3630124e 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java @@ -28,6 +28,8 @@ public class OpenSearchService { public void indexEntry(final CMAEntry contentfulEntry) { final String body = contentfulEntryToJsonString(contentfulEntry); + log.debug("Elastic search update json string: {}", body); + webClientBuilder.build().put() .uri(createUrl(contentfulEntry)) .body(Mono.just(body), String.class) @@ -35,12 +37,15 @@ public void indexEntry(final CMAEntry contentfulEntry) { .header(AUTHORIZATION, createAuthHeader()) .retrieve() .bodyToMono(void.class) - .doOnError(e -> log.error("Failed to create an index entry for ad " + contentfulEntry.getId() + "in open search: {}", e.getMessage())) + .doOnError(e -> log.error("Failed to create an index entry for ad " + contentfulEntry.getId() + + " in open search: ", e)) .block(); } public void removeIndexEntry(final CMAEntry contentfulEntry) { final String body = contentfulEntryToJsonString(contentfulEntry.getSystem()); + log.debug("Elastic search delete json string: {}", body); + webClientBuilder.build().method(HttpMethod.DELETE) .uri(createUrl(contentfulEntry)) .body(Mono.just(body), String.class) @@ -48,7 +53,8 @@ public void removeIndexEntry(final CMAEntry contentfulEntry) { .header(AUTHORIZATION, createAuthHeader()) .retrieve() .bodyToMono(void.class) - .doOnError(e -> log.error("Failed to delete an index entry for ad " + contentfulEntry.getId() + "in open search: {}", e.getMessage())) + .doOnError(e -> log.error("Failed to delete an index entry for ad " + contentfulEntry.getId() + + "in open search: ", e)) .block(); } From f72f6490c4f69f5efa53261887fc545fa5c99688 Mon Sep 17 00:00:00 2001 From: GavCookCO <99668051+GavCookCO@users.noreply.github.com> Date: Wed, 10 Apr 2024 15:44:27 +0100 Subject: [PATCH 13/20] Improving logging (#283) --- .../exceptions/IndexingException.java | 10 +++++++ .../services/GrantAdvertService.java | 2 +- .../services/OpenSearchService.java | 27 +++++++++++-------- .../services/OpenSearchServiceTest.java | 4 +++ 4 files changed, 31 insertions(+), 12 deletions(-) create mode 100644 src/main/java/gov/cabinetoffice/gap/adminbackend/exceptions/IndexingException.java diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/exceptions/IndexingException.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/exceptions/IndexingException.java new file mode 100644 index 00000000..83189cf2 --- /dev/null +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/exceptions/IndexingException.java @@ -0,0 +1,10 @@ +package gov.cabinetoffice.gap.adminbackend.exceptions; + +public class IndexingException extends RuntimeException { + + public IndexingException() {} + + public IndexingException(String message) { + super(message); + } +} 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 b8d1376c..ccf8a434 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/GrantAdvertService.java @@ -316,7 +316,7 @@ public GrantAdvert publishAdvert(UUID advertId) { @Override protected void onSuccess(CMAEntry result) { openSearchService.indexEntry(result); - log.debug("Took {} seconds to publish advert", Duration.between(now, Instant.now()).getSeconds()); + log.info("Took {} seconds to publish advert", Duration.between(now, Instant.now()).getSeconds()); } }); } diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java index 3630124e..868d12c9 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java @@ -3,9 +3,13 @@ import com.contentful.java.cma.model.CMAEntry; import com.fasterxml.jackson.databind.ObjectMapper; import gov.cabinetoffice.gap.adminbackend.config.OpenSearchConfig; +import gov.cabinetoffice.gap.adminbackend.exceptions.IndexingException; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import static org.apache.http.HttpHeaders.AUTHORIZATION; +import static org.apache.http.HttpHeaders.CONTENT_TYPE; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.stereotype.Service; import org.springframework.web.reactive.function.client.WebClient; @@ -14,9 +18,6 @@ import java.nio.charset.StandardCharsets; import java.util.Base64; -import static org.apache.http.HttpHeaders.AUTHORIZATION; -import static org.apache.http.HttpHeaders.CONTENT_TYPE; - @Service @RequiredArgsConstructor @Slf4j @@ -28,33 +29,37 @@ public class OpenSearchService { public void indexEntry(final CMAEntry contentfulEntry) { final String body = contentfulEntryToJsonString(contentfulEntry); - log.debug("Elastic search update json string: {}", body); - webClientBuilder.build().put() .uri(createUrl(contentfulEntry)) .body(Mono.just(body), String.class) .header(CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + "; " + StandardCharsets.UTF_8.name()) .header(AUTHORIZATION, createAuthHeader()) .retrieve() + .onStatus(HttpStatus::isError, clientResponse -> { + log.info("Elastic search update json string: {}", body); + log.error("response code from open search: {}", clientResponse.statusCode()); + log.error("response from open search: {}", clientResponse.bodyToMono(String.class)); + throw new IndexingException("failed to add CMA entry with ID " + contentfulEntry.getId() + " to index"); + }) .bodyToMono(void.class) - .doOnError(e -> log.error("Failed to create an index entry for ad " + contentfulEntry.getId() - + " in open search: ", e)) .block(); } public void removeIndexEntry(final CMAEntry contentfulEntry) { final String body = contentfulEntryToJsonString(contentfulEntry.getSystem()); - log.debug("Elastic search delete json string: {}", body); - webClientBuilder.build().method(HttpMethod.DELETE) .uri(createUrl(contentfulEntry)) .body(Mono.just(body), String.class) .header(CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + "; " + StandardCharsets.UTF_8.name()) .header(AUTHORIZATION, createAuthHeader()) .retrieve() + .onStatus(HttpStatus::isError, clientResponse -> { + log.info("Elastic search delete json string: {}", body); + log.error("response code from open search: {}", clientResponse.statusCode()); + log.error("response from open search: {}", clientResponse.bodyToMono(String.class)); + throw new IndexingException("failed to remove CMA entry with ID " + contentfulEntry.getId() + " from index"); + }) .bodyToMono(void.class) - .doOnError(e -> log.error("Failed to delete an index entry for ad " + contentfulEntry.getId() - + "in open search: ", e)) .block(); } diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchServiceTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchServiceTest.java index 2865f998..eab19c9e 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchServiceTest.java @@ -65,9 +65,11 @@ void indexEntry() { when(mockRequestHeadersSpec.header("Content-Type", "application/json; UTF-8")).thenReturn(mockRequestHeadersSpec); when(mockRequestHeadersSpec.header("Authorization", "Basic dGVzdFVzZXJuYW1lOnRlc3RQYXNzd29yZA==")).thenReturn(mockRequestHeadersSpec); when(mockRequestHeadersSpec.retrieve()).thenReturn(mockResponseSpec); + when(mockResponseSpec.onStatus(any(), any())).thenReturn(mockResponseSpec); when(mockResponseSpec.bodyToMono(void.class)).thenReturn(Mono.empty()); openSearchService.indexEntry(contentfulEntry); + } @Test @@ -87,8 +89,10 @@ void removeIndexEntry() { when(mockRequestHeadersSpec.header("Content-Type", "application/json; UTF-8")).thenReturn(mockRequestHeadersSpec); when(mockRequestHeadersSpec.header("Authorization", "Basic dGVzdFVzZXJuYW1lOnRlc3RQYXNzd29yZA==")).thenReturn(mockRequestHeadersSpec); when(mockRequestHeadersSpec.retrieve()).thenReturn(mockResponseSpec); + when(mockResponseSpec.onStatus(any(), any())).thenReturn(mockResponseSpec); when(mockResponseSpec.bodyToMono(void.class)).thenReturn(Mono.empty()); openSearchService.removeIndexEntry(contentfulEntry); + } } From 24d4b57b7e289bab17998416cd3cdf5ce975eef8 Mon Sep 17 00:00:00 2001 From: Gavin Cook Date: Wed, 10 Apr 2024 16:08:00 +0100 Subject: [PATCH 14/20] Improving logging II --- .../gap/adminbackend/services/OpenSearchService.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java index 868d12c9..ff057081 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/OpenSearchService.java @@ -38,7 +38,9 @@ public void indexEntry(final CMAEntry contentfulEntry) { .onStatus(HttpStatus::isError, clientResponse -> { log.info("Elastic search update json string: {}", body); log.error("response code from open search: {}", clientResponse.statusCode()); - log.error("response from open search: {}", clientResponse.bodyToMono(String.class)); + log.error("response headers from open search: {}", clientResponse.headers().asHttpHeaders()); + log.error("response body from open search: {}", clientResponse.bodyToMono(String.class)); + throw new IndexingException("failed to add CMA entry with ID " + contentfulEntry.getId() + " to index"); }) .bodyToMono(void.class) @@ -56,7 +58,9 @@ public void removeIndexEntry(final CMAEntry contentfulEntry) { .onStatus(HttpStatus::isError, clientResponse -> { log.info("Elastic search delete json string: {}", body); log.error("response code from open search: {}", clientResponse.statusCode()); - log.error("response from open search: {}", clientResponse.bodyToMono(String.class)); + log.error("response headers from open search: {}", clientResponse.headers().asHttpHeaders()); + log.error("response body from open search: {}", clientResponse.bodyToMono(String.class)); + throw new IndexingException("failed to remove CMA entry with ID " + contentfulEntry.getId() + " from index"); }) .bodyToMono(void.class) From 16b05991704dc05aa6238e401b36dc015e510909 Mon Sep 17 00:00:00 2001 From: JamieGunnCO Date: Wed, 1 May 2024 08:50:08 +0100 Subject: [PATCH 15/20] amend schemeeditorcontroller --- .../gap/adminbackend/controllers/SchemeEditorController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/SchemeEditorController.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/SchemeEditorController.java index 8ce2d2b9..0b7a8fec 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/SchemeEditorController.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/SchemeEditorController.java @@ -55,7 +55,7 @@ public ResponseEntity> getSchemeEditors(@PathVariable fin @PostMapping public ResponseEntity addEditorToScheme(@PathVariable final Integer schemeId, @RequestBody @Valid final SchemeEditorPostDTO newEditorDto, final HttpServletRequest request) { final String jwt = HelperUtils.getJwtFromCookies(request, userServiceConfig.getCookieName()); - schemeEditorService.addEditorToScheme(schemeId, newEditorDto.getEditorEmailAddress(), jwt); + schemeEditorService.addEditorToScheme(schemeId, newEditorDto.getEditorEmailAddress().toLowerCase(), jwt); return ResponseEntity.ok("Editor added to scheme successfully"); } From 54a13dee944f0ebfe7bf5ade3ef9b743086a1e33 Mon Sep 17 00:00:00 2001 From: Arulelango Jayaraman Date: Mon, 12 Aug 2024 12:49:02 +0100 Subject: [PATCH 16/20] Release/12.0 (#308) * amend schemeeditorcontroller (#302) * url validation updated to accept # symbol * existing unknown chane removed (#307) --------- Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> --- .../validation/validators/AdvertPageResponseValidator.java | 2 +- .../validation/validators/AdvertPageResponseValidatorTest.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/validation/validators/AdvertPageResponseValidator.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/validation/validators/AdvertPageResponseValidator.java index 642588e6..e6da6264 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/validation/validators/AdvertPageResponseValidator.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/validation/validators/AdvertPageResponseValidator.java @@ -216,7 +216,7 @@ private SimpleEntry validateURL(GrantAdvertQuestionResponse ques // (incl. .com) (repeating) | // (optional) slash | (optional) additional path | (optional) slash | // (optional) query params - String urlValidPattern = "^(http://|https://)(www.)?((?!www)[a-zA-Z0-9\\-]{2,}+)(\\.[a-zA-Z0-9\\-]{2,})+(/)?(/[a-z0-9\\-._~%!$&'()*+,;=:@]+)*?(/)?(\\?[a-z0-9\\-._~%!$&'()*+,;=:@/]*)?$"; + String urlValidPattern = "^(http://|https://)(www.)?((?!www)[a-zA-Z0-9\\-]{2,}+)(\\.[a-zA-Z0-9\\-]{2,})+(/)?(/[a-z0-9\\-._~%!$&'()*+,;=:@#]+)*?(/)?(\\?[a-z0-9\\-._~%!$&'()*+,;=:@#/]*)?$"; Pattern pattern = Pattern.compile(urlValidPattern, Pattern.CASE_INSENSITIVE); Matcher matcher = pattern.matcher(question.getResponse()); diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/validation/validators/AdvertPageResponseValidatorTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/validation/validators/AdvertPageResponseValidatorTest.java index 3d5e622b..a525ddb4 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/validation/validators/AdvertPageResponseValidatorTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/validation/validators/AdvertPageResponseValidatorTest.java @@ -928,7 +928,8 @@ private static Stream validUrls() { Arguments.of("https://www.google.co.uk/long/path?query=var&query2=var"), Arguments.of("http://www.google.co.uk/long/path?query=var&query2=var"), Arguments.of("https://google.co.uk/long/path?query=var&query2=var"), - Arguments.of("http://google.co.uk/long/path?query=var&query2=var")); + Arguments.of("http://google.co.uk/long/path?query=var&query2=var"), + Arguments.of("https://google.co.uk/competition/1989/overview/63441f1e-850b-4581-986d-cd2f6c6c226d#summary")); } // @formatter:on From 35616fa5cf3540593a9711db3415af2aa3432033 Mon Sep 17 00:00:00 2001 From: Arulelango Jayaraman Date: Thu, 9 Jan 2025 17:22:32 +0000 Subject: [PATCH 17/20] Release/12.3 (#310) * amend schemeeditorcontroller (#302) * url validation updated to accept # symbol (#305) * Fix/gap lss 431 (#306) * url validation updated to accept # symbol * existing unknown chane removed * Lss 701 ownership transfer case sensitive (#309) * 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 * Release/3.3 (#39) Release 3.3 * 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 * Create DUMMY * Tmi2 501/revert tmi2 496 changes (#129) (#130) * revert tmi2-496 changes * format * Release/6.1 (#136) * Tmi2 501/revert tmi2 496 changes (#129) * revert tmi2-496 changes * format * Adding tables for completion_statistics.sql (#131) * Adding tables for completion_statistics.sql * Formatting... * Tn 154 calculations tables (#132) * Adding tables for completion_statistics.sql * Formatting... * Adding sequence for calculations table id * Tn 154 calculations tables (#133) * Adding tables for completion_statistics.sql * Formatting... * Adding sequence for calculations table id * Fixing incorrect format for migration script name. Combining 2 into 1 * Fixing issue with db migration script (#134) * Adding tables for completion_statistics.sql * Formatting... * Adding sequence for calculations table id * Fixing incorrect format for migration script name. Combining 2 into 1 * Fixing errors in db migration script --------- Co-authored-by: a-lor-cab <107372333+a-lor-cab@users.noreply.github.com> * Fix promoteToProd.yml (#179) * Adds query to find application with no ownership check to allow lambdas to call (#212) * Fix unpublish application form from lambda (#268) (#269) * Updating release 10.0 with develop changes (#274) * Fix unpublish application form from lambda (#268) * GAP-2531: Cascade delete grant export tables createdby column (#270) * GAP-2533: Fix advert scheduler view for BST (#273) * use LocalDateTime for grant opening and closing times (#272) * use LocalDateTime for grant opening and closing times * fix test * fix more tests * Tmi2 740 - Handling new locations for MQ saving to grant_beneficiary table (#271) * Adding two new columns to grant_beneficiary for new locations --------- Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> Co-authored-by: Connor Macqueen <138442814+ConnorTCO@users.noreply.github.com> * GAP-2533: Retrospectively fix BST ads in prod (#276) (#278) * GAP-2533: Retrospectively fix BST ads in prod * GAP-2533: Including release time * GAP-2533: Overriding UTC opening/closing dates as Europe/London time * GAP-2533: Update scheduled closing times too * GAP-2533: Fix syntax * GAP-2533: Hard code datetimes for BST * Improves logs around updating and deleting open search (#280) * Improving logging (#283) * Improving logging II * amend schemeeditorcontroller * Release/12.0 (#308) * amend schemeeditorcontroller (#302) * url validation updated to accept # symbol * existing unknown chane removed (#307) --------- Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> * convert admin email to lowercase --------- Co-authored-by: dominicwest <101722961+dominicwest@users.noreply.github.com> Co-authored-by: dylanwrightCO <135221918+dylanwrightCO@users.noreply.github.com> 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: Gavin Cook Co-authored-by: JamieGunnCO Co-authored-by: a-lor-cab <107372333+a-lor-cab@users.noreply.github.com> Co-authored-by: Connor Macqueen <138442814+ConnorTCO@users.noreply.github.com> Co-authored-by: dylanwrightCO Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> --------- Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> Co-authored-by: dominicwest <101722961+dominicwest@users.noreply.github.com> Co-authored-by: dylanwrightCO <135221918+dylanwrightCO@users.noreply.github.com> 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: Gavin Cook Co-authored-by: JamieGunnCO Co-authored-by: a-lor-cab <107372333+a-lor-cab@users.noreply.github.com> Co-authored-by: Connor Macqueen <138442814+ConnorTCO@users.noreply.github.com> Co-authored-by: dylanwrightCO --- .../cabinetoffice/gap/adminbackend/services/UserService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 e3bc182d..e0bcd4d4 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/UserService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/UserService.java @@ -116,8 +116,10 @@ public Boolean verifyAdminRoles(final String emailAddress, final String roles) { @PreAuthorize("hasAnyRole('ADMIN', 'SUPER_ADMIN')") public GrantAdmin getGrantAdminIdFromUserServiceEmail(final String email, final String jwt) { try { + //UI might send camelcase. change it to lowercase + UserV2DTO response = webClientBuilder.build().get() - .uri(userServiceConfig.getDomain() + "/user/email/" + email) + .uri(userServiceConfig.getDomain() + "/user/email/" + email.toLowerCase()) .cookie(userServiceConfig.getCookieName(), jwt).retrieve().bodyToMono(UserV2DTO.class).block(); return grantAdminRepository.findByGapUserUserSub(response.sub()).orElseThrow(() -> new NotFoundException( From e8ec48f2a0ed221ca68ea41e3e0099fa2fa5d68e Mon Sep 17 00:00:00 2001 From: Ben Tadiar Date: Tue, 3 Jun 2025 12:56:51 +0100 Subject: [PATCH 18/20] release/12.4 (#313) * amend schemeeditorcontroller (#302) * url validation updated to accept # symbol (#305) * Fix/gap lss 431 (#306) * url validation updated to accept # symbol * existing unknown chane removed * Lss 701 ownership transfer case sensitive (#309) * 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 * Release/3.3 (#39) Release 3.3 * 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 * Create DUMMY * Tmi2 501/revert tmi2 496 changes (#129) (#130) * revert tmi2-496 changes * format * Release/6.1 (#136) * Tmi2 501/revert tmi2 496 changes (#129) * revert tmi2-496 changes * format * Adding tables for completion_statistics.sql (#131) * Adding tables for completion_statistics.sql * Formatting... * Tn 154 calculations tables (#132) * Adding tables for completion_statistics.sql * Formatting... * Adding sequence for calculations table id * Tn 154 calculations tables (#133) * Adding tables for completion_statistics.sql * Formatting... * Adding sequence for calculations table id * Fixing incorrect format for migration script name. Combining 2 into 1 * Fixing issue with db migration script (#134) * Adding tables for completion_statistics.sql * Formatting... * Adding sequence for calculations table id * Fixing incorrect format for migration script name. Combining 2 into 1 * Fixing errors in db migration script --------- Co-authored-by: a-lor-cab <107372333+a-lor-cab@users.noreply.github.com> * Fix promoteToProd.yml (#179) * Adds query to find application with no ownership check to allow lambdas to call (#212) * Fix unpublish application form from lambda (#268) (#269) * Updating release 10.0 with develop changes (#274) * Fix unpublish application form from lambda (#268) * GAP-2531: Cascade delete grant export tables createdby column (#270) * GAP-2533: Fix advert scheduler view for BST (#273) * use LocalDateTime for grant opening and closing times (#272) * use LocalDateTime for grant opening and closing times * fix test * fix more tests * Tmi2 740 - Handling new locations for MQ saving to grant_beneficiary table (#271) * Adding two new columns to grant_beneficiary for new locations --------- Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> Co-authored-by: Connor Macqueen <138442814+ConnorTCO@users.noreply.github.com> * GAP-2533: Retrospectively fix BST ads in prod (#276) (#278) * GAP-2533: Retrospectively fix BST ads in prod * GAP-2533: Including release time * GAP-2533: Overriding UTC opening/closing dates as Europe/London time * GAP-2533: Update scheduled closing times too * GAP-2533: Fix syntax * GAP-2533: Hard code datetimes for BST * Improves logs around updating and deleting open search (#280) * Improving logging (#283) * Improving logging II * amend schemeeditorcontroller * Release/12.0 (#308) * amend schemeeditorcontroller (#302) * url validation updated to accept # symbol * existing unknown chane removed (#307) --------- Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> * convert admin email to lowercase --------- Co-authored-by: dominicwest <101722961+dominicwest@users.noreply.github.com> Co-authored-by: dylanwrightCO <135221918+dylanwrightCO@users.noreply.github.com> 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: Gavin Cook Co-authored-by: JamieGunnCO Co-authored-by: a-lor-cab <107372333+a-lor-cab@users.noreply.github.com> Co-authored-by: Connor Macqueen <138442814+ConnorTCO@users.noreply.github.com> Co-authored-by: dylanwrightCO Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> * LSS-550 Update Apache Commons IO to version 2.19.0 (#311) * LSS-712 Updated upload-artifact action version to v4 (#312) --------- Co-authored-by: jgunnCO <135321532+jgunnCO@users.noreply.github.com> Co-authored-by: Arulelango Jayaraman Co-authored-by: dominicwest <101722961+dominicwest@users.noreply.github.com> Co-authored-by: dylanwrightCO <135221918+dylanwrightCO@users.noreply.github.com> 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: Gavin Cook Co-authored-by: JamieGunnCO Co-authored-by: a-lor-cab <107372333+a-lor-cab@users.noreply.github.com> Co-authored-by: Connor Macqueen <138442814+ConnorTCO@users.noreply.github.com> Co-authored-by: dylanwrightCO --- .github/workflows/feature.yml | 2 +- .github/workflows/pushImage.yml | 6 +++--- pom.xml | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/feature.yml b/.github/workflows/feature.yml index a67bf660..2a4863ae 100644 --- a/.github/workflows/feature.yml +++ b/.github/workflows/feature.yml @@ -42,7 +42,7 @@ jobs: --disableRetireJS true - name: Upload Test results - uses: actions/upload-artifact@master + uses: actions/upload-artifact@v4 with: name: DependencyCheck report path: ${{github.workspace}}/reports diff --git a/.github/workflows/pushImage.yml b/.github/workflows/pushImage.yml index c354fe84..06201c00 100644 --- a/.github/workflows/pushImage.yml +++ b/.github/workflows/pushImage.yml @@ -42,7 +42,7 @@ jobs: --enableRetired --disableOssIndex true - name: Upload Test results - uses: actions/upload-artifact@master + uses: actions/upload-artifact@v4 with: name: DependencyCheck report path: ${{github.workspace}}/reports @@ -110,7 +110,7 @@ jobs: run: docker save --output ${{ steps.docker-image-name.outputs.name }}.tar $ECR_REGISTRY/gap-apply-admin-backend:${{ env.BUILD_VERSION }} - name: Upload Docker image - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: ${{ steps.docker-image-name.outputs.name }} path: ${{ steps.docker-image-name.outputs.name }}.tar @@ -148,7 +148,7 @@ jobs: echo BUILD_VERSION is ${{ env.BUILD_VERSION }} - name: Download Docker image - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: ${{ needs.build.outputs.docker-image-name }} diff --git a/pom.xml b/pom.xml index a54dc101..3c908062 100644 --- a/pom.xml +++ b/pom.xml @@ -142,7 +142,7 @@ commons-io commons-io - 2.11.0 + 2.19.0 org.apache.poi From a4be0bc7696c23efeaacfd8fbda031cabc0df837 Mon Sep 17 00:00:00 2001 From: rmohammed-goaco Date: Wed, 19 Nov 2025 12:25:29 +0000 Subject: [PATCH 19/20] Release/test deployments main (#317) * Add health check endpoint and update security configuration to permit access * Undo extra indentation --- .../controllers/HealthController.java | 5 +++ .../security/WebSecurityConfig.java | 43 ++++++++++++------- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/HealthController.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/HealthController.java index 6d017579..92ae0e3e 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/HealthController.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/controllers/HealthController.java @@ -16,4 +16,9 @@ public ResponseEntity getHealthCheck() { return ResponseEntity.ok("Service up"); } + @GetMapping("/check") + public ResponseEntity getHealthCheck2() { + return ResponseEntity.ok("Admin API Service up"); + } + } 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 fd570b74..4f182a29 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/security/WebSecurityConfig.java @@ -31,34 +31,45 @@ public WebSecurityConfig(final UserService userService, final JwtTokenFilterConf @Bean public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { - //if you add a path which is hit by the lambda, remember to update also the paths in gov/cabinetoffice/gap/adminbackend/config/LambdasInterceptor.java + // if you add a path which is hit by the lambda, remember to update also the + // paths in gov/cabinetoffice/gap/adminbackend/config/LambdasInterceptor.java http.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED).and() .authorizeHttpRequests(auth -> auth .mvcMatchers("/login", "/health", + "/health/check", "/emails/sendLambdaConfirmationEmail", "/users/validateAdminSession", "/submissions/{submissionId:" + UUID_REGEX_STRING - + "}/export-batch/{batchExportId:" + UUID_REGEX_STRING + "}/submission", + + "}/export-batch/{batchExportId:" + + UUID_REGEX_STRING + "}/submission", "/submissions/*/export-batch/*/status", - "/submissions/{submissionId:" + UUID_REGEX_STRING + "}/export-batch/{batchExportId:" + "/submissions/{submissionId:" + UUID_REGEX_STRING + + "}/export-batch/{batchExportId:" + UUID_REGEX_STRING + "}/s3-object-key", - "/grant-export/{exportId:" + UUID_REGEX_STRING + "}/outstandingCount", - "/grant-export/{exportId:" + UUID_REGEX_STRING + "}/failedCount", - "/grant-export/{exportId:" + UUID_REGEX_STRING + "}/remainingCount", - "/grant-export/{exportId:" + UUID_REGEX_STRING + "}/completed", - "/grant-export/{exportId:" + UUID_REGEX_STRING + "}/batch/status", - "/grant-export/{exportId:" + UUID_REGEX_STRING + "}/batch/s3-object-key", - "/grant-advert/lambda/{grantAdvertId:" + UUID_REGEX_STRING + "}/publish", - "/grant-advert/lambda/{grantAdvertId:" + UUID_REGEX_STRING + "}/unpublish", + "/grant-export/{exportId:" + UUID_REGEX_STRING + + "}/outstandingCount", + "/grant-export/{exportId:" + UUID_REGEX_STRING + + "}/failedCount", + "/grant-export/{exportId:" + UUID_REGEX_STRING + + "}/remainingCount", + "/grant-export/{exportId:" + UUID_REGEX_STRING + + "}/completed", + "/grant-export/{exportId:" + UUID_REGEX_STRING + + "}/batch/status", + "/grant-export/{exportId:" + UUID_REGEX_STRING + + "}/batch/s3-object-key", + "/grant-advert/lambda/{grantAdvertId:" + + UUID_REGEX_STRING + "}/publish", + "/grant-advert/lambda/{grantAdvertId:" + + UUID_REGEX_STRING + "}/unpublish", "/users/migrate", "/users/delete", "/users/tech-support-user/**", "/users/admin-user/**", "/users/funding-organisation", "/application-forms/lambda/**", - "/feedback/add" - ) + "/feedback/add") .permitAll() .antMatchers("/v3/api-docs/**", "/swagger-ui/**", @@ -66,7 +77,8 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { "/swagger-ui.html", "/webjars/**") .permitAll() - .antMatchers("/spotlight-submissions/{spotlightSubmissionId:" + UUID_REGEX_STRING + "}") + .antMatchers("/spotlight-submissions/{spotlightSubmissionId:" + + UUID_REGEX_STRING + "}") .permitAll() .antMatchers("/spotlight-batch/status/**", "/spotlight-batch", @@ -77,7 +89,8 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { .anyRequest() .authenticated()) - .formLogin().disable().httpBasic().disable().logout().disable().csrf().disable().exceptionHandling() + .formLogin().disable().httpBasic().disable().logout().disable().csrf().disable() + .exceptionHandling() .authenticationEntryPoint(new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED)); http.addFilterAfter(jwtTokenFilter, UsernamePasswordAuthenticationFilter.class); From ff9697d8d01e28ae7dd6417e55049c08b704cf50 Mon Sep 17 00:00:00 2001 From: rmohammed-goaco Date: Thu, 8 Jan 2026 14:28:22 +0000 Subject: [PATCH 20/20] Implement idempotency check for spotlight submissions in SpotlightBatchService (#324) --- .../adminbackend/services/SpotlightBatchService.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/SpotlightBatchService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/SpotlightBatchService.java index 0be0c7f0..cd58e975 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/SpotlightBatchService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/SpotlightBatchService.java @@ -107,6 +107,18 @@ public SpotlightBatch addSpotlightSubmissionToSpotlightBatch(SpotlightSubmission UUID spotlightBatchId) { final SpotlightBatch spotlightBatch = getSpotlightBatch(spotlightBatchId); final List existingSpotlightSubmissions = spotlightBatch.getSpotlightSubmissions(); + + // Check if submission is already in the batch (idempotent operation) + boolean alreadyExists = existingSpotlightSubmissions != null + && existingSpotlightSubmissions.stream() + .anyMatch(s -> s.getId().equals(spotlightSubmission.getId())); + + if (alreadyExists) { + log.info("Submission {} is already in batch {}. Returning existing batch (idempotent).", + spotlightSubmission.getId(), spotlightBatchId); + return spotlightBatch; + } + final List existingSpotlightBatches = spotlightSubmission.getBatches(); existingSpotlightSubmissions.add(spotlightSubmission);