EIP1-11113-EIP1-11114 combined for review #124
Conversation
… API project that has merged db migrations already in it; resolve issues arising from address entity field differences between APIs
…m EMS integration API
…h sts:AssumeRole owing to 13-digit test account id causing integer overflow
…eck-configs' into EIP1-11113-proxy-main
…in-code' into EIP1-11113-proxy-main
…web that have been fixed in the current spring/SB version we are using EIP1-11113: upgrade spring boot to 3.3.8 in line with applications api; delete defunct owasp suppressions
gradlew
Outdated
| then | ||
| die "xargs is not available" | ||
| fi | ||
| # if ! command -v xargs >/dev/null 2>&1 |
There was a problem hiding this comment.
Any reason for commenting this out and not deleting it if it's not needed?
There was a problem hiding this comment.
I have restored this scripting in the next commit
| @@ -1,243 +1,3 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <suppressions xmlns="https://jeremylong.github.io/DependencyCheck/dependency-suppression.1.3.xsd"> | |||
There was a problem hiding this comment.
Are there no vulnribilties for this version?
There was a problem hiding this comment.
All the suppressions that were listed in this file have now been resolved by upgrading SB to 3.3.8. I haven't run a CVE detection tool though. I could do that, trivy or suchlike.
There was a problem hiding this comment.
I've scanned the EMS projct with trivy and it didn't flag up any vulnerabilities.
brew install trivy
trivy -d fs --scanners vuln,secret /~/dev/eip-ero-ems-integration-api
|
|
||
| /** | ||
| * Spring Boot application bootstrapping class. | ||
| */ | ||
| @SpringBootApplication | ||
| @ConfigurationPropertiesScan | ||
| @EnableJpaAuditing |
There was a problem hiding this comment.
This should stay as EmsIntegrationApiApplication as this is for the whole service and not Register Checker specific
There was a problem hiding this comment.
change applied in next commit
| @@ -23,7 +24,7 @@ class CachingConfiguration { | |||
| return CaffeineCacheManager() | |||
| .apply { | |||
| setCaffeine(Caffeine.newBuilder().expireAfterWrite(timeToLive)) | |||
| setCacheNames(listOf(ERO_CERTIFICATE_MAPPING_CACHE, ERO_GSS_CODE_BY_ERO_ID_CACHE)) | |||
| setCacheNames(listOf(IER_ELECTORAL_REGISTRATION_OFFICES_CACHE)) | |||
| } | |||
There was a problem hiding this comment.
Do we not still need the ERO_CERTIFICATE_MAPPING_CACHE and ERO_GSS_CODE_BY_ERO_ID_CACHE cache's?
There was a problem hiding this comment.
restored these cache names to the list on next commit
| * Class to bind configuration properties for the email client | ||
| */ | ||
| @ConfigurationProperties(prefix = "email.client") | ||
| @ConstructorBinding |
There was a problem hiding this comment.
Why did we get rid of the @ConstructorBinding?
There was a problem hiding this comment.
See @ConstructingBinding No Longer Needed at the Type Level" for the reason why this has been removed.
| @@ -72,8 +72,6 @@ class ProxyVoteApplication( | |||
| ) | |||
| var welshRejectedReasonItems: Set<RejectedReasonItem>? = mutableSetOf(), | |||
|
|
|||
| var isFromApplicationsApi: Boolean?, | |||
There was a problem hiding this comment.
It was needed by the applications api for interaction with the portal to identify applications that are on the OS code but the ems api doesn't have that role, it is a background api.
| @@ -39,7 +39,7 @@ class PendingDownloadsMonitoringService( | |||
| logPendingDownloads("proxy", pendingProxyDownloads) | |||
|
|
|||
| if (sendEmail) { | |||
| emailService.sendPendingDownloadMonitoringEmail( | |||
| emailService .sendPendingDownloadMonitoringEmail( | |||
There was a problem hiding this comment.
fixed in next commit
|
@SimPaySW Seems like alot of tests have been deleted with out being replaced was this intentinal? |
Hi @tomsdjohnson yes, see PR #120 (a bit of a deja vu from your previous questions) |
…figurations to CachingConfiguration; rename application to EmsIntegrationApiApplication; remove space char
Combination of four PRs to make reviewing easier.
Target is set as a proxy main branch so that further PRs can be reviewed against this without requiring a merge to the main branch.