From cfabefece5513888283cb4b62f3620381217789a Mon Sep 17 00:00:00 2001 From: Tim Jacomb <21194782+timja@users.noreply.github.com> Date: Wed, 4 Jan 2023 16:46:23 +0000 Subject: [PATCH] Fix building on recent Java and all build warnings (#119) --- .github/workflows/gradle.yml | 9 +-- README.md | 65 +++++++++---------- build.gradle | 13 ++-- config/pmd/ruleset.xml | 62 +++++++++--------- .../ServiceAuthorisationApiTest.java | 33 +++++----- .../config/IntegrationTestInitializer.java | 15 +---- .../resources/application-wiremock.yaml | 2 +- .../ServiceAuthAutoConfiguration.java | 14 ++-- .../ServiceAuthorisationApi.java | 12 ++-- .../ServiceAuthorisationHealthApi.java | 4 +- .../exceptions/InvalidTokenException.java | 2 + .../exceptions/JwtDecodingException.java | 2 + .../exceptions/ServiceException.java | 1 + .../filters/ServiceAuthFilter.java | 13 ++-- .../generators/ServiceAuthTokenGenerator.java | 8 +-- .../validators/AuthTokenValidator.java | 2 +- .../validators/ServiceAuthTokenValidator.java | 2 +- .../filters/ServiceAuthFilterTest.java | 7 +- ...torefreshingJwtAuthTokenGeneratorTest.java | 4 +- .../generators/BearerTokenGeneratorTest.java | 2 +- .../ServiceAuthTokenGeneratorTest.java | 4 +- .../ServiceExceptionValidatorTest.java | 34 ++++------ 22 files changed, 144 insertions(+), 166 deletions(-) diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 95fc9cf7..ef15b68d 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -15,10 +15,11 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v1 - - name: Set up JDK 11 - uses: actions/setup-java@v1 + - uses: actions/checkout@v3 + - uses: actions/setup-java@v3 with: - java-version: 11 + distribution: 'temurin' + java-version: '17' + cache: 'gradle' - name: Build run: ./gradlew check diff --git a/README.md b/README.md index 6e0ba7f1..1ef41539 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,3 @@ -[![Known Vulnerabilities](https://snyk.io/test/github/hmcts/service-auth-provider-java-client/badge.svg)](https://snyk.io/test/github/hmcts/service-auth-provider-java-client) -[ ![Download](https://api.bintray.com/packages/hmcts/hmcts-maven/service-auth-provider-client/images/download.svg) ](https://bintray.com/hmcts/hmcts-maven/service-auth-provider-client/_latestVersion) - # service-auth-provider-java-client This is the client library for the service-auth-provider api microservice. @@ -11,18 +8,18 @@ The tool provides a method to generate s2s auth token for a microservice and, op ### Prerequisites -- [JDK 8](https://www.oracle.com/java) +- [Java 17](https://adoptium.net/) - [Docker](https://www.docker.com) ### Building -The project uses [Gradle](https://gradle.org) as a build tool but you don't have install it locally since there is a +The project uses [Gradle](https://gradle.org) as a build tool, but you don't have to install it locally since there is a `./gradlew` wrapper script. -To build project execute the following command: +To build the project run the following command: ```bash - ./gradlew build +./gradlew build ``` ## Configuration The following values must be provided: @@ -36,19 +33,17 @@ idam: A spring bean: ```java - @Configuration - public class ServiceTokenGeneratorConfiguration { - - @Bean - public AuthTokenGenerator serviceAuthTokenGenerator( - @Value("${idam.s2s-auth.totp_secret}") final String secret, - @Value("${idam.s2s-auth.microservice}") final String microService, - final ServiceAuthorisationApi serviceAuthorisationApi - ) { - return AuthTokenGeneratorFactory.createDefaultGenerator(secret, microService, serviceAuthorisationApi); - } - +@Configuration +public class ServiceTokenGeneratorConfiguration { + @Bean + public AuthTokenGenerator serviceAuthTokenGenerator( + @Value("${idam.s2s-auth.totp_secret}") final String secret, + @Value("${idam.s2s-auth.microservice}") final String microService, + final ServiceAuthorisationApi serviceAuthorisationApi + ) { + return AuthTokenGeneratorFactory.createDefaultGenerator(secret, microService, serviceAuthorisationApi); } +} ``` ## Configuration for Service Authentication filter The following values must be provided to enable a ServiceAuthFilter bean: @@ -63,23 +58,25 @@ to approve the request. Any requests from services that are not in your authoris to your service and return an HTTP response status code 403 (forbidden) and for any other reasons if the token is missing, invalid or failure to verify will result in 401(unauthorized). -## Running in a non spring context +## Running without Sprint You might want to use this client when not running in a spring context, i.e. a scheduled job possibly. ```java -private static AuthTokenGenerator getAuthTokenGenerator(String s2sURL, String clientId, String clientSecret) { - HttpMessageConverter jsonConverter = new MappingJackson2HttpMessageConverter(new ObjectMapper()); - ObjectFactory converter = () -> new HttpMessageConverters(jsonConverter); - - ServiceAuthorisationApi serviceAuthorisationApi = Feign.builder() - .contract(new SpringMvcContract()) - .encoder(new SpringEncoder(converter)) - .decoder(new StringDecoder()) - .target(ServiceAuthorisationApi.class, s2sURL); - - return AuthTokenGeneratorFactory - .createDefaultGenerator(clientSecret, clientId, serviceAuthorisationApi); +class ServiceTokenGenerator { + private static AuthTokenGenerator getAuthTokenGenerator(String s2sURL, String clientId, String clientSecret) { + HttpMessageConverter jsonConverter = new MappingJackson2HttpMessageConverter(new ObjectMapper()); + ObjectFactory converter = () -> new HttpMessageConverters(jsonConverter); + + ServiceAuthorisationApi serviceAuthorisationApi = Feign.builder() + .contract(new SpringMvcContract()) + .encoder(new SpringEncoder(converter)) + .decoder(new StringDecoder()) + .target(ServiceAuthorisationApi.class, s2sURL); + + return AuthTokenGeneratorFactory + .createDefaultGenerator(clientSecret, clientId, serviceAuthorisationApi); + } } ``` @@ -90,7 +87,7 @@ private static AuthTokenGenerator getAuthTokenGenerator(String s2sURL, String cl To run all unit tests execute the following command: ```bash - ./gradlew test +./gradlew test ``` ### Coding style tests @@ -98,7 +95,7 @@ To run all unit tests execute the following command: To run all checks (including unit tests) execute the following command: ```bash - ./gradlew check +./gradlew check ``` ## Versioning diff --git a/build.gradle b/build.gradle index e6bdc04c..fbd1871d 100644 --- a/build.gradle +++ b/build.gradle @@ -41,8 +41,8 @@ checkstyle { } pmd { - toolVersion = "6.12.0" - ignoreFailures = true + toolVersion = "6.53.0" + ignoreFailures = false sourceSets = [sourceSets.main, sourceSets.test, sourceSets.integrationTest] reportsDir = file("$project.buildDir/reports/pmd") ruleSetFiles = files("config/pmd/ruleset.xml") @@ -105,10 +105,8 @@ task contract(type: Test) { } task runAndPublishConsumerPactTests(type: Test){ - logger.lifecycle("Runs pact Tests") testClassesDirs = sourceSets.contractTest.output.classesDirs classpath = sourceSets.contractTest.runtimeClasspath - } runAndPublishConsumerPactTests.dependsOn contract @@ -136,8 +134,8 @@ publishing { url = gitRepo licenses { license { - name = 'MIT License, Copyright (c) 2019 HM Courts & Tribunals Service' - url = "http://www.opensource.org/licenses/mit-license.php" + name = 'MIT License, Copyright (c) 2023 HM Courts & Tribunals Service' + url = "https://www.opensource.org/licenses/MIT" } } scm { @@ -158,7 +156,8 @@ dependencies { testImplementation group: 'junit', name: 'junit', version: '4.13.2' testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.23.1' - testImplementation group: 'org.mockito', name: 'mockito-all', version: '1.10.19' + testImplementation group: 'org.mockito', name: 'mockito-core', version: '4.11.0' + integrationTestImplementation sourceSets.main.runtimeClasspath integrationTestImplementation sourceSets.test.runtimeClasspath diff --git a/config/pmd/ruleset.xml b/config/pmd/ruleset.xml index 0b67287b..05b6e5eb 100644 --- a/config/pmd/ruleset.xml +++ b/config/pmd/ruleset.xml @@ -1,70 +1,70 @@ + xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd"> HMCTS PMD rule set - - + + - - - - - - + + + + + + + + - + - + - + - - - - + + + + - + - + - + - - - + + + - - - - - - + + + + - + \ No newline at end of file diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApiTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApiTest.java index b69ff697..1af525d6 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApiTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApiTest.java @@ -1,11 +1,8 @@ package uk.gov.hmcts.reform.authorisation; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.Matchers; -import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -13,7 +10,6 @@ import org.springframework.cloud.contract.wiremock.AutoConfigureWireMock; import org.springframework.context.annotation.ComponentScan; import org.springframework.test.context.ActiveProfiles; -import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; import uk.gov.hmcts.reform.authorisation.config.IntegrationTestInitializer; @@ -32,16 +28,17 @@ import static com.github.tomakehurst.wiremock.client.WireMock.givenThat; import static com.github.tomakehurst.wiremock.client.WireMock.status; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.http.HttpStatus.OK; @ActiveProfiles("wiremock") -@AutoConfigureWireMock -@ContextConfiguration(initializers = IntegrationTestInitializer.class) +@AutoConfigureWireMock(port = 0) @ComponentScan @EnableAutoConfiguration @EnableConfigurationProperties() @@ -50,8 +47,11 @@ }) @RunWith(SpringRunner.class) @SpringBootTest(classes = IntegrationTestInitializer.class) +@SuppressWarnings("PMD.ExcessiveImports") public class ServiceAuthorisationApiTest { + private static final String DETAILS_ENDPOINT = "/details"; + private static final String DEFAULT_SERVICE = "service"; @Autowired private ServiceAuthorisationApi s2sApi; @@ -64,7 +64,6 @@ public class ServiceAuthorisationApiTest { @Before public void before() { - Assert.assertNotNull(serviceAuthFilter); filterChain = spy(FilterChain.class); httpServletRequest = mock(HttpServletRequest.class); when(httpServletRequest.getHeader(ServiceAuthFilter.AUTHORISATION)).thenReturn("token"); @@ -73,32 +72,32 @@ public void before() { @Test public void should_get_service_name_providing_valid_token() { AuthTokenValidator validator = new ServiceAuthTokenValidator(s2sApi); - givenThat(get("/details").willReturn(status(OK.value()).withBody("service"))); - assertThat(validator.getServiceName("token")).isEqualTo("service"); + givenThat(get(DETAILS_ENDPOINT).willReturn(status(OK.value()).withBody(DEFAULT_SERVICE))); + assertThat(validator.getServiceName("token")).isEqualTo(DEFAULT_SERVICE); } @Test public void should_pass_serviceAuthFilter_with_authorized_access() throws ServletException, IOException { - givenThat(get("/details").willReturn(status(OK.value()).withBody("service1"))); + givenThat(get(DETAILS_ENDPOINT).willReturn(status(OK.value()).withBody("service1"))); serviceAuthFilter.doFilter(httpServletRequest, mock(HttpServletResponse.class), filterChain); - Mockito.verify(filterChain, times(1)).doFilter(Matchers.any(), Matchers.any()); + verify(filterChain, times(1)).doFilter(any(), any()); } @Test public void should_fail_serviceAuthFilter_with_Unauthorized_access() throws ServletException, IOException { - givenThat(get("/details").willReturn(status(OK.value()).withStatus(HttpStatus.GATEWAY_TIMEOUT_504))); + givenThat(get(DETAILS_ENDPOINT).willReturn(status(OK.value()).withStatus(HttpStatus.GATEWAY_TIMEOUT_504))); HttpServletResponse response = mock(HttpServletResponse.class); serviceAuthFilter.doFilter(httpServletRequest, response, filterChain); - Mockito.verify(response, times(1)).setStatus(HttpStatus.UNAUTHORIZED_401); - Mockito.verify(filterChain, never()).doFilter(Matchers.any(), Matchers.any()); + verify(response, times(1)).setStatus(HttpStatus.UNAUTHORIZED_401); + verify(filterChain, never()).doFilter(any(), any()); } @Test public void should_fail_serviceAuthFilter_with_Forbidden_access() throws ServletException, IOException { - givenThat(get("/details").willReturn(status(OK.value()).withBody("service"))); + givenThat(get(DETAILS_ENDPOINT).willReturn(status(OK.value()).withBody(DEFAULT_SERVICE))); HttpServletResponse response = mock(HttpServletResponse.class); serviceAuthFilter.doFilter(httpServletRequest, response, filterChain); - Mockito.verify(response, times(1)).setStatus(HttpStatus.FORBIDDEN_403); - Mockito.verify(filterChain, never()).doFilter(Matchers.any(), Matchers.any()); + verify(response, times(1)).setStatus(HttpStatus.FORBIDDEN_403); + verify(filterChain, never()).doFilter(any(), any()); } } diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/config/IntegrationTestInitializer.java b/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/config/IntegrationTestInitializer.java index ecda5f35..b7addc0d 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/config/IntegrationTestInitializer.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/config/IntegrationTestInitializer.java @@ -1,26 +1,13 @@ package uk.gov.hmcts.reform.authorisation.config; -import com.github.tomakehurst.wiremock.common.Slf4jNotifier; -import com.github.tomakehurst.wiremock.core.Options; -import com.github.tomakehurst.wiremock.core.WireMockConfiguration; -import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ConfigurableApplicationContext; -import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import static org.springframework.util.SocketUtils.findAvailableTcpPort; - @Configuration -public class IntegrationTestInitializer implements ApplicationContextInitializer { +public class IntegrationTestInitializer implements ApplicationContextInitializer { @Override public void initialize(ConfigurableApplicationContext applicationContext) { - System.setProperty("wiremock.port", Integer.toString(findAvailableTcpPort())); - } - - @Bean - public Options options(@Value("${wiremock.port}") int port) { - return WireMockConfiguration.options().port(port).notifier(new Slf4jNotifier(false)); } } diff --git a/src/integrationTest/resources/application-wiremock.yaml b/src/integrationTest/resources/application-wiremock.yaml index 35222ddd..4dbf114b 100644 --- a/src/integrationTest/resources/application-wiremock.yaml +++ b/src/integrationTest/resources/application-wiremock.yaml @@ -1,5 +1,5 @@ wiremock: - url: &wiremock "http://localhost:${wiremock.port}" + url: &wiremock "http://localhost:${wiremock.server.port}" idam: s2s-auth: diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthAutoConfiguration.java b/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthAutoConfiguration.java index 90490696..0e0bb01c 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthAutoConfiguration.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthAutoConfiguration.java @@ -27,21 +27,21 @@ public ServiceAuthHealthIndicator serviceAuthHealthIndicator(ServiceAuthorisatio @Bean @ConditionalOnProperty("idam.s2s-authorised.services") - public ServiceAuthFilter serviceAuthFiler(ServiceAuthorisationApi authorisationApi, - @Value("${idam.s2s-authorised.services}") List authorisedServices) { - + public ServiceAuthFilter serviceAuthFiler( + ServiceAuthorisationApi authorisationApi, + @Value("${idam.s2s-authorised.services}") List authorisedServices + ) { AuthTokenValidator authTokenValidator = new ServiceAuthTokenValidator(authorisationApi); return new ServiceAuthFilter(authTokenValidator, authorisedServices); - } @Bean @ConditionalOnProperty("idam.s2s-authorised.services") - public FilterRegistrationBean deRegisterServiceAuthFilter(ServiceAuthFilter serviceAuthFilter) { - FilterRegistrationBean filterRegistrationBean = new FilterRegistrationBean(); + public FilterRegistrationBean deRegisterServiceAuthFilter(ServiceAuthFilter serviceAuthFilter) { + FilterRegistrationBean filterRegistrationBean = new FilterRegistrationBean<>(); filterRegistrationBean.setFilter(serviceAuthFilter); filterRegistrationBean.setEnabled(false); - return filterRegistrationBean; + return filterRegistrationBean; } } diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApi.java b/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApi.java index 63dac0b9..b34f44c0 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApi.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApi.java @@ -29,16 +29,16 @@ public interface ServiceAuthorisationApi { String serviceToken(@RequestBody Map signIn); @SuppressWarnings("PMD.UseVarargs") - @GetMapping(value = "/authorisation-check") - void authorise(@RequestHeader(AUTHORIZATION) final String authHeader, - @RequestParam("role") final String[] roles); + @GetMapping("/authorisation-check") + void authorise(@RequestHeader(AUTHORIZATION) String authHeader, + @RequestParam("role") String[] roles); - @GetMapping(value = "/details") - String getServiceName(@RequestHeader(AUTHORIZATION) final String authHeader); + @GetMapping("/details") + String getServiceName(@RequestHeader(AUTHORIZATION) String authHeader); class Config { @Bean - Decoder stringDecoder() { + /* default */ Decoder stringDecoder() { return new StringDecoder(); } } diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationHealthApi.java b/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationHealthApi.java index 9c05d9fc..d22017c5 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationHealthApi.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationHealthApi.java @@ -4,7 +4,6 @@ import feign.jackson.JacksonDecoder; import org.springframework.cloud.openfeign.FeignClient; import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Primary; import org.springframework.context.annotation.Scope; import org.springframework.web.bind.annotation.GetMapping; import uk.gov.hmcts.reform.authorisation.healthcheck.InternalHealth; @@ -21,9 +20,8 @@ public interface ServiceAuthorisationHealthApi { class ServiceAuthConfiguration { @Bean - @Primary @Scope("prototype") - Decoder feignDecoder() { + /* default */ Decoder feignDecoder() { return new JacksonDecoder(); } } diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/InvalidTokenException.java b/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/InvalidTokenException.java index e00e87ec..55eea44d 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/InvalidTokenException.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/InvalidTokenException.java @@ -2,6 +2,8 @@ public class InvalidTokenException extends RuntimeException { + private static final long serialVersionUID = 1L; + public InvalidTokenException(String message) { super(message); } diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/JwtDecodingException.java b/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/JwtDecodingException.java index cfcb284f..2f1b9d46 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/JwtDecodingException.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/JwtDecodingException.java @@ -2,6 +2,8 @@ public class JwtDecodingException extends RuntimeException { + private static final long serialVersionUID = 1L; + public JwtDecodingException(String message, Throwable cause) { super(message, cause); } diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/ServiceException.java b/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/ServiceException.java index ab158a65..13ef6414 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/ServiceException.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/exceptions/ServiceException.java @@ -1,6 +1,7 @@ package uk.gov.hmcts.reform.authorisation.exceptions; public class ServiceException extends RuntimeException { + private static final long serialVersionUID = 1L; public ServiceException(String message, Throwable cause) { super(message, cause); diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/filters/ServiceAuthFilter.java b/src/main/java/uk/gov/hmcts/reform/authorisation/filters/ServiceAuthFilter.java index 31d7e3c1..e821dbfc 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/filters/ServiceAuthFilter.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/filters/ServiceAuthFilter.java @@ -27,7 +27,7 @@ public class ServiceAuthFilter extends OncePerRequestFilter { private final AuthTokenValidator authTokenValidator; public ServiceAuthFilter(AuthTokenValidator authTokenValidator, List authorisedServices) { - + super(); this.authTokenValidator = authTokenValidator; if (authorisedServices == null || authorisedServices.isEmpty()) { throw new IllegalArgumentException("Must have at least one service defined"); @@ -38,28 +38,29 @@ public ServiceAuthFilter(AuthTokenValidator authTokenValidator, List aut } @Override + @SuppressWarnings("PMD.LawOfDemeter") protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { try { String bearerToken = extractBearerToken(request); String serviceName = authTokenValidator.getServiceName(bearerToken); - if (!authorisedServices.contains(serviceName)) { + if (authorisedServices.contains(serviceName)) { LOG.debug( - "service forbidden {} for endpoint: {} method: {} ", + "service authorized {} for endpoint: {} method: {} ", serviceName, request.getRequestURI(), request.getMethod() ); - response.setStatus(HttpStatus.FORBIDDEN.value()); + filterChain.doFilter(request, response); } else { LOG.debug( - "service authorized {} for endpoint: {} method: {} ", + "service forbidden {} for endpoint: {} method: {} ", serviceName, request.getRequestURI(), request.getMethod() ); - filterChain.doFilter(request, response); + response.setStatus(HttpStatus.FORBIDDEN.value()); } } catch (InvalidTokenException | ServiceException exception) { LOG.warn("Unsuccessful service authentication", exception); diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/generators/ServiceAuthTokenGenerator.java b/src/main/java/uk/gov/hmcts/reform/authorisation/generators/ServiceAuthTokenGenerator.java index 4c1a70c1..f3f90eeb 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/generators/ServiceAuthTokenGenerator.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/generators/ServiceAuthTokenGenerator.java @@ -3,7 +3,6 @@ import com.warrenstrange.googleauth.GoogleAuthenticator; import uk.gov.hmcts.reform.authorisation.ServiceAuthorisationApi; -import java.util.HashMap; import java.util.Map; import static java.lang.String.format; @@ -31,9 +30,10 @@ public ServiceAuthTokenGenerator( public String generate() { final String oneTimePassword = format("%06d", googleAuthenticator.getTotpPassword(secret)); - Map signInDetails = new HashMap<>(); - signInDetails.put("microservice", this.microService); - signInDetails.put("oneTimePassword", oneTimePassword); + Map signInDetails = Map.of( + "microservice", this.microService, + "oneTimePassword", oneTimePassword + ); return serviceAuthorisationApi.serviceToken(signInDetails); } diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/validators/AuthTokenValidator.java b/src/main/java/uk/gov/hmcts/reform/authorisation/validators/AuthTokenValidator.java index e64bc4a2..c8e00aee 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/validators/AuthTokenValidator.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/validators/AuthTokenValidator.java @@ -22,5 +22,5 @@ public interface AuthTokenValidator { * @param token Bearer token * @return Service name */ - String getServiceName(final String token); + String getServiceName(String token); } diff --git a/src/main/java/uk/gov/hmcts/reform/authorisation/validators/ServiceAuthTokenValidator.java b/src/main/java/uk/gov/hmcts/reform/authorisation/validators/ServiceAuthTokenValidator.java index 7f7053ef..32442183 100644 --- a/src/main/java/uk/gov/hmcts/reform/authorisation/validators/ServiceAuthTokenValidator.java +++ b/src/main/java/uk/gov/hmcts/reform/authorisation/validators/ServiceAuthTokenValidator.java @@ -30,7 +30,7 @@ public void validate(final String token) { @Override public void validate(String token, final List roles) { try { - api.authorise(token, roles.toArray(new String[roles.size()])); + api.authorise(token, roles.toArray(new String[0])); } catch (FeignException exception) { throw parseFeignException(exception); } diff --git a/src/test/java/uk/gov/hmcts/reform/authorisation/filters/ServiceAuthFilterTest.java b/src/test/java/uk/gov/hmcts/reform/authorisation/filters/ServiceAuthFilterTest.java index 9e92b5bd..3ea8ada2 100644 --- a/src/test/java/uk/gov/hmcts/reform/authorisation/filters/ServiceAuthFilterTest.java +++ b/src/test/java/uk/gov/hmcts/reform/authorisation/filters/ServiceAuthFilterTest.java @@ -23,8 +23,11 @@ import static org.mockito.Mockito.when; @RunWith(JUnit4.class) +@SuppressWarnings("PMD.LawOfDemeter") public class ServiceAuthFilterTest { + private static final String AUTH_TOKEN = "Bearer @%%$DFGDFGDF"; + private static final String SERVICE_1 = "service1"; private static final String SERVICE_2 = "service2"; @@ -39,9 +42,7 @@ public class ServiceAuthFilterTest { private FilterChain filterChain; - private static String AUTH_TOKEN = "Bearer @%%$DFGDFGDF"; - - private List authorisedServices = Arrays.asList(SERVICE_1, SERVICE_2); + private final List authorisedServices = Arrays.asList(SERVICE_1, SERVICE_2); @Before diff --git a/src/test/java/uk/gov/hmcts/reform/authorisation/generators/AutorefreshingJwtAuthTokenGeneratorTest.java b/src/test/java/uk/gov/hmcts/reform/authorisation/generators/AutorefreshingJwtAuthTokenGeneratorTest.java index e31a988b..27d6825e 100644 --- a/src/test/java/uk/gov/hmcts/reform/authorisation/generators/AutorefreshingJwtAuthTokenGeneratorTest.java +++ b/src/test/java/uk/gov/hmcts/reform/authorisation/generators/AutorefreshingJwtAuthTokenGeneratorTest.java @@ -5,7 +5,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import uk.gov.hmcts.reform.authorisation.exceptions.JwtDecodingException; import java.time.Duration; @@ -105,7 +105,7 @@ public void should_request_a_new_token_if_delta_is_larger_than_time_left_to_expi ); // when - repeat(2, () -> jwtGenerator.generate()); + repeat(2, jwtGenerator::generate); // then // it should request a new token diff --git a/src/test/java/uk/gov/hmcts/reform/authorisation/generators/BearerTokenGeneratorTest.java b/src/test/java/uk/gov/hmcts/reform/authorisation/generators/BearerTokenGeneratorTest.java index 868cdec5..7113c663 100644 --- a/src/test/java/uk/gov/hmcts/reform/authorisation/generators/BearerTokenGeneratorTest.java +++ b/src/test/java/uk/gov/hmcts/reform/authorisation/generators/BearerTokenGeneratorTest.java @@ -4,7 +4,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; diff --git a/src/test/java/uk/gov/hmcts/reform/authorisation/generators/ServiceAuthTokenGeneratorTest.java b/src/test/java/uk/gov/hmcts/reform/authorisation/generators/ServiceAuthTokenGeneratorTest.java index 0bf3c5b1..699ac198 100644 --- a/src/test/java/uk/gov/hmcts/reform/authorisation/generators/ServiceAuthTokenGeneratorTest.java +++ b/src/test/java/uk/gov/hmcts/reform/authorisation/generators/ServiceAuthTokenGeneratorTest.java @@ -5,7 +5,7 @@ import uk.gov.hmcts.reform.authorisation.ServiceAuthorisationApi; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Matchers.anyMapOf; +import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.Mockito.when; public class ServiceAuthTokenGeneratorTest { @@ -18,7 +18,7 @@ public void shouldGenerateServiceAuthToken() { final String microService = "microservice"; final String serviceAuthToken = "service-auth-token"; - when(serviceAuthorisationApi.serviceToken(anyMapOf(String.class, String.class))) + when(serviceAuthorisationApi.serviceToken(anyMap())) .thenReturn(serviceAuthToken); //when diff --git a/src/test/java/uk/gov/hmcts/reform/authorisation/validators/ServiceExceptionValidatorTest.java b/src/test/java/uk/gov/hmcts/reform/authorisation/validators/ServiceExceptionValidatorTest.java index c48156b3..fe5304b4 100644 --- a/src/test/java/uk/gov/hmcts/reform/authorisation/validators/ServiceExceptionValidatorTest.java +++ b/src/test/java/uk/gov/hmcts/reform/authorisation/validators/ServiceExceptionValidatorTest.java @@ -10,15 +10,13 @@ import org.junit.runners.Parameterized; import org.springframework.http.HttpStatus; import uk.gov.hmcts.reform.authorisation.ServiceAuthorisationApi; -import uk.gov.hmcts.reform.authorisation.exceptions.InvalidTokenException; -import uk.gov.hmcts.reform.authorisation.exceptions.ServiceException; import java.util.Arrays; import java.util.Collections; import java.util.stream.Collectors; -import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.eq; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -29,32 +27,24 @@ public class ServiceExceptionValidatorTest { private final ServiceAuthTokenValidator validator = new ServiceAuthTokenValidator(api); - private final Class expectedException; - private final HttpStatus status; @Parameterized.Parameters(name = "Testing for HTTP_STATUS {1}") public static Iterable data() { return Arrays.stream(HttpStatus.values()) .filter(httpStatus -> httpStatus.is4xxClientError() || httpStatus.is5xxServerError()) - .flatMap(httpStatus -> { - Class expected = httpStatus.is4xxClientError() - ? InvalidTokenException.class - : ServiceException.class; - return Arrays.stream(new Object[][]{ - {expected, httpStatus} - }); - }) + .flatMap(httpStatus -> Arrays.stream(new Object[][]{ + {httpStatus} + })) .collect(Collectors.toList()); } - public ServiceExceptionValidatorTest(Class expectedException, - HttpStatus status) { - this.expectedException = expectedException; + public ServiceExceptionValidatorTest(HttpStatus status) { this.status = status; } @Before + @SuppressWarnings({"PMD.LawOfDemeter", "PMD.DataflowAnomalyAnalysis"}) public void setUp() { Request request = Request.create( Request.HttpMethod.GET, @@ -63,17 +53,17 @@ public void setUp() { Request.Body.empty(), new RequestTemplate() ); - Response feignResponse = Response + try (Response feignResponse = Response .builder() .request(request) .status(status.value()) .body(new byte[0]) .reason("i must fail") .headers(Collections.emptyMap()) - .build(); - FeignException exception = FeignException.errorStatus("oh no", feignResponse); - - doThrow(exception).when(api).authorise(anyString(), eq(new String[0])); + .build()) { + FeignException exception = FeignException.errorStatus("oh no", feignResponse); + doThrow(exception).when(api).authorise(anyString(), eq(new String[0])); + } } @Test(expected = RuntimeException.class)