From 06fcbda8473257dc2cc5ef14737e80549e88e17a Mon Sep 17 00:00:00 2001 From: Lam Date: Mon, 18 Mar 2019 21:00:01 +0000 Subject: [PATCH 1/7] RDM-4369: Support reserved characters in userId email address Ensure `GET` request to `/user-profile/users` endpoint uses a fully encoded URL, to allow reserved characters in the `uid` query parameter. --- .../ccd/data/user/DefaultUserRepository.java | 13 ++++-- .../data/user/DefaultUserRepositoryTest.java | 41 +++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java index 9141058414..f4be9fd899 100644 --- a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java +++ b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java @@ -18,6 +18,7 @@ import org.springframework.stereotype.Repository; import org.springframework.web.client.HttpStatusCodeException; import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; import uk.gov.hmcts.ccd.ApplicationParams; import uk.gov.hmcts.ccd.AuthCheckerConfiguration; import uk.gov.hmcts.ccd.data.SecurityUtils; @@ -91,13 +92,17 @@ public UserDefault getUserDefaultSettings(final String userId) { try { LOG.debug("retrieving default user settings for user {}", userId); final HttpEntity requestEntity = new HttpEntity(securityUtils.authorizationHeaders()); - final Map queryParams = new HashMap<>(); + final Map queryParams = new HashMap<>(); queryParams.put("uid", userId); - return restTemplate.exchange(applicationParams.userDefaultSettingsURL(), - HttpMethod.GET, requestEntity, UserDefault.class, queryParams).getBody(); + // The toUriString() method ensures the whole URL is encoded, including the query string variables + final String encodedUrl = UriComponentsBuilder.fromHttpUrl(applicationParams.userDefaultSettingsURL()) + .uriVariables(queryParams) + .toUriString(); + return restTemplate.exchange(encodedUrl, HttpMethod.GET, requestEntity, UserDefault.class).getBody(); } catch (HttpStatusCodeException e) { LOG.error("Failed to retrieve user profile", e); - final List headerMessages = e.getResponseHeaders().get("Message"); + final List headerMessages = Optional.ofNullable(e.getResponseHeaders()) + .map(headers -> headers.get("Message")).orElse(null); final String message = headerMessages != null ? headerMessages.get(0) : e.getMessage(); if (message != null) { throw new BadRequestException(message); diff --git a/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java b/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java index 38ff03f32d..8a41beb911 100644 --- a/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java @@ -2,6 +2,7 @@ import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Set; import java.util.stream.Collectors; @@ -13,8 +14,11 @@ import static org.hamcrest.Matchers.hasSize; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyListOf; import static org.mockito.Mockito.doReturn; +import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.same; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -23,9 +27,14 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.context.SecurityContext; @@ -35,6 +44,8 @@ import uk.gov.hmcts.ccd.data.SecurityUtils; import uk.gov.hmcts.ccd.data.casedetails.SecurityClassification; import uk.gov.hmcts.ccd.data.definition.CaseDefinitionRepository; +import uk.gov.hmcts.ccd.domain.model.aggregated.UserDefault; +import uk.gov.hmcts.ccd.domain.model.definition.Jurisdiction; import uk.gov.hmcts.ccd.domain.model.definition.UserRole; import uk.gov.hmcts.ccd.endpoint.exceptions.ServiceException; import uk.gov.hmcts.reform.auth.checker.spring.serviceanduser.ServiceAndUserDetails; @@ -160,6 +171,36 @@ void shouldExcludeOtherRoles() { } } + @Nested + @DisplayName("getUserDefaultSettings()") + class GetUserDefaultSettings { + + @Test + @DisplayName("should return the User Profile defaults for the user") + void shouldReturnUserProfileDefaultsForUser() { + when(applicationParams.userDefaultSettingsURL()).thenReturn("http://test.hmcts.net/users?uid={uid}"); + final Jurisdiction jurisdiction = new Jurisdiction(); + jurisdiction.setId("TEST"); + jurisdiction.setName("Test"); + jurisdiction.setDescription("Test Jurisdiction"); + final UserDefault userDefault = new UserDefault(); + userDefault.setJurisdictions(Collections.singletonList(jurisdiction)); + final ResponseEntity responseEntity = new ResponseEntity<>(userDefault, HttpStatus.OK); + when(restTemplate + .exchange(any(String.class), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class))) + .thenReturn(responseEntity); + + final UserDefault result = userRepository.getUserDefaultSettings("ccd+test@hmcts.net"); + assertThat(result, is(userDefault)); + + // Check that the URL used in the request is encoded correctly, specifically the query string + ArgumentCaptor requestUrl = ArgumentCaptor.forClass(String.class); + verify(restTemplate).exchange( + requestUrl.capture(), same(HttpMethod.GET), any(HttpEntity.class), (Class)any(Class.class)); + assertThat(requestUrl.getValue(), is("http://test.hmcts.net/users?uid=ccd%2Btest%40hmcts.net")); + } + } + @Nested @DisplayName("getHighestUserClassification()") class GetHighestUserClassification { From 5929239317e7fb25d66a644a9466bbf31d17e201 Mon Sep 17 00:00:00 2001 From: Lam Date: Tue, 19 Mar 2019 12:00:05 +0000 Subject: [PATCH 2/7] Change method of encoding request URL Switch to using `DefaultUriBuilderFactory` in Spring Boot 2 to encode the request URL, to fix a problem with "double encoding" of query parameters (see https://github.com/spring-projects/spring-framework/issues/20750#issuecomment-453463983). --- .../ccd/data/user/DefaultUserRepository.java | 16 +++++----- .../data/user/DefaultUserRepositoryTest.java | 30 +++++++++---------- .../resources/mappings/user1-profile.json | 2 +- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java index f4be9fd899..273df1e983 100644 --- a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java +++ b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java @@ -18,7 +18,8 @@ import org.springframework.stereotype.Repository; import org.springframework.web.client.HttpStatusCodeException; import org.springframework.web.client.RestTemplate; -import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.web.util.DefaultUriBuilderFactory; +import org.springframework.web.util.DefaultUriBuilderFactory.EncodingMode; import uk.gov.hmcts.ccd.ApplicationParams; import uk.gov.hmcts.ccd.AuthCheckerConfiguration; import uk.gov.hmcts.ccd.data.SecurityUtils; @@ -92,13 +93,14 @@ public UserDefault getUserDefaultSettings(final String userId) { try { LOG.debug("retrieving default user settings for user {}", userId); final HttpEntity requestEntity = new HttpEntity(securityUtils.authorizationHeaders()); - final Map queryParams = new HashMap<>(); + final Map queryParams = new HashMap<>(); queryParams.put("uid", userId); - // The toUriString() method ensures the whole URL is encoded, including the query string variables - final String encodedUrl = UriComponentsBuilder.fromHttpUrl(applicationParams.userDefaultSettingsURL()) - .uriVariables(queryParams) - .toUriString(); - return restTemplate.exchange(encodedUrl, HttpMethod.GET, requestEntity, UserDefault.class).getBody(); + final DefaultUriBuilderFactory builderFactory = new DefaultUriBuilderFactory(); + // The EncodingMode.VALUES_ONLY mode ensures the query string variables are encoded + builderFactory.setEncodingMode(EncodingMode.VALUES_ONLY); + restTemplate.setUriTemplateHandler(builderFactory); + return restTemplate.exchange(applicationParams.userDefaultSettingsURL(), + HttpMethod.GET, requestEntity, UserDefault.class, queryParams).getBody(); } catch (HttpStatusCodeException e) { LOG.error("Failed to retrieve user profile", e); final List headerMessages = Optional.ofNullable(e.getResponseHeaders()) diff --git a/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java b/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java index 8a41beb911..8776e83dff 100644 --- a/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java @@ -2,20 +2,22 @@ import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.Set; import java.util.stream.Collectors; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasSize; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyListOf; +import static org.mockito.Matchers.anyList; import static org.mockito.Mockito.doReturn; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.same; @@ -27,7 +29,6 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -125,7 +126,8 @@ void shouldOnlyConsiderRolesForJurisdiction() { userRepository.getUserClassifications(JURISDICTION_ID); assertAll( - () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(asList(ROLE_CASEWORKER_CMC)), + () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList( + singletonList(ROLE_CASEWORKER_CMC)), () -> verifyNoMoreInteractions(caseDefinitionRepository) ); } @@ -143,7 +145,7 @@ void shouldConsiderCitizen() { userRepository.getUserClassifications(JURISDICTION_ID); assertAll( - () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(asList(CITIZEN)), + () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(singletonList(CITIZEN)), () -> verifyNoMoreInteractions(caseDefinitionRepository) ); } @@ -156,7 +158,8 @@ void shouldConsiderLetterHolder() { userRepository.getUserClassifications(JURISDICTION_ID); assertAll( - () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(asList(LETTER_HOLDER)), + () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList( + singletonList(LETTER_HOLDER)), () -> verifyNoMoreInteractions(caseDefinitionRepository) ); } @@ -184,20 +187,16 @@ void shouldReturnUserProfileDefaultsForUser() { jurisdiction.setName("Test"); jurisdiction.setDescription("Test Jurisdiction"); final UserDefault userDefault = new UserDefault(); - userDefault.setJurisdictions(Collections.singletonList(jurisdiction)); + userDefault.setJurisdictions(singletonList(jurisdiction)); final ResponseEntity responseEntity = new ResponseEntity<>(userDefault, HttpStatus.OK); when(restTemplate - .exchange(any(String.class), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class))) + .exchange(anyString(), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class), anyMap())) .thenReturn(responseEntity); final UserDefault result = userRepository.getUserDefaultSettings("ccd+test@hmcts.net"); assertThat(result, is(userDefault)); - - // Check that the URL used in the request is encoded correctly, specifically the query string - ArgumentCaptor requestUrl = ArgumentCaptor.forClass(String.class); verify(restTemplate).exchange( - requestUrl.capture(), same(HttpMethod.GET), any(HttpEntity.class), (Class)any(Class.class)); - assertThat(requestUrl.getValue(), is("http://test.hmcts.net/users?uid=ccd%2Btest%40hmcts.net")); + anyString(), same(HttpMethod.GET), any(HttpEntity.class), (Class)any(Class.class), anyMap()); } } @@ -216,7 +215,8 @@ void shouldReturnHighestClassification() { userRole2.setSecurityClassification(SecurityClassification.PUBLIC.name()); UserRole userRole3 = new UserRole(); userRole3.setSecurityClassification(SecurityClassification.RESTRICTED.name()); - when(caseDefinitionRepository.getClassificationsForUserRoleList(anyListOf(String.class))).thenReturn(asList(userRole1, userRole2, userRole3)); + when(caseDefinitionRepository.getClassificationsForUserRoleList(anyList())) + .thenReturn(asList(userRole1, userRole2, userRole3)); SecurityClassification result = userRepository.getHighestUserClassification(JURISDICTION_ID); @@ -227,7 +227,7 @@ void shouldReturnHighestClassification() { @DisplayName("should throw exception when no user roles returned") void shouldThrowExceptionWhenNoUserRolesReturned() { asCaseworker(); - when(caseDefinitionRepository.getClassificationsForUserRoleList(anyListOf(String.class))).thenReturn(emptyList()); + when(caseDefinitionRepository.getClassificationsForUserRoleList(anyList())).thenReturn(emptyList()); assertThrows(ServiceException.class, () -> userRepository.getHighestUserClassification(JURISDICTION_ID)); } diff --git a/src/test/resources/mappings/user1-profile.json b/src/test/resources/mappings/user1-profile.json index d8108466d1..2d784ad42a 100644 --- a/src/test/resources/mappings/user1-profile.json +++ b/src/test/resources/mappings/user1-profile.json @@ -1,7 +1,7 @@ { "request": { "method": "GET", - "url": "/user-profile/users?uid=Cloud.Strife@test.com" + "url": "/user-profile/users?uid=Cloud.Strife%40test.com" }, "response": { "status": 200, From 963680478e60fa98c5aa5dfbb4ee5073331959f1 Mon Sep 17 00:00:00 2001 From: Lam Date: Tue, 19 Mar 2019 15:00:01 +0000 Subject: [PATCH 3/7] Add missing tests to increase coverage to required level --- .../ccd/data/user/DefaultUserRepository.java | 4 +- .../data/user/DefaultUserRepositoryTest.java | 40 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java index 273df1e983..fa8f982a1c 100644 --- a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java +++ b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java @@ -16,7 +16,7 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Repository; -import org.springframework.web.client.HttpStatusCodeException; +import org.springframework.web.client.RestClientResponseException; import org.springframework.web.client.RestTemplate; import org.springframework.web.util.DefaultUriBuilderFactory; import org.springframework.web.util.DefaultUriBuilderFactory.EncodingMode; @@ -101,7 +101,7 @@ public UserDefault getUserDefaultSettings(final String userId) { restTemplate.setUriTemplateHandler(builderFactory); return restTemplate.exchange(applicationParams.userDefaultSettingsURL(), HttpMethod.GET, requestEntity, UserDefault.class, queryParams).getBody(); - } catch (HttpStatusCodeException e) { + } catch (RestClientResponseException e) { LOG.error("Failed to retrieve user profile", e); final List headerMessages = Optional.ofNullable(e.getResponseHeaders()) .map(headers -> headers.get("Message")).orElse(null); diff --git a/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java b/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java index 8776e83dff..11f689508b 100644 --- a/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java @@ -33,6 +33,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -40,6 +41,7 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.web.client.RestClientResponseException; import org.springframework.web.client.RestTemplate; import uk.gov.hmcts.ccd.ApplicationParams; import uk.gov.hmcts.ccd.data.SecurityUtils; @@ -48,6 +50,7 @@ import uk.gov.hmcts.ccd.domain.model.aggregated.UserDefault; import uk.gov.hmcts.ccd.domain.model.definition.Jurisdiction; import uk.gov.hmcts.ccd.domain.model.definition.UserRole; +import uk.gov.hmcts.ccd.endpoint.exceptions.BadRequestException; import uk.gov.hmcts.ccd.endpoint.exceptions.ServiceException; import uk.gov.hmcts.reform.auth.checker.spring.serviceanduser.ServiceAndUserDetails; @@ -198,6 +201,43 @@ void shouldReturnUserProfileDefaultsForUser() { verify(restTemplate).exchange( anyString(), same(HttpMethod.GET), any(HttpEntity.class), (Class)any(Class.class), anyMap()); } + + @Test + @DisplayName("should throw a BadRequestException if the User Profile defaults cannot be retrieved") + void shouldThrowExceptionIfUserProfileCannotBeRetrieved() { + when(applicationParams.userDefaultSettingsURL()).thenReturn("http://test.hmcts.net/users?uid={uid}"); + final HttpHeaders headers = new HttpHeaders(); + headers.add("Message", "User Profile data could not be retrieved"); + final RestClientResponseException exception = + new RestClientResponseException("Error on GET", 400, "Bad Request", headers, null, null); + when(restTemplate + .exchange(anyString(), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class), anyMap())) + .thenThrow(exception); + + final BadRequestException badRequestException = + assertThrows(BadRequestException.class, + () -> userRepository.getUserDefaultSettings("ccd+test@hmcts.net"), + "Expected getUserDefaultSettings() to throw, but it didn't"); + assertThat(badRequestException.getMessage(), is(headers.getFirst("Message"))); + } + + @Test + @DisplayName("should throw a ServiceException if an error occurs retrieving the User Profile defaults") + void shouldThrowExceptionIfErrorOnRetrievingUserProfile() { + when(applicationParams.userDefaultSettingsURL()).thenReturn("http://test.hmcts.net/users?uid={uid}"); + final RestClientResponseException exception = + new RestClientResponseException(null, 500, "Internal Server Error", null, null, null); + when(restTemplate + .exchange(anyString(), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class), anyMap())) + .thenThrow(exception); + + final String userId = "ccd+test@hmcts.net"; + final ServiceException serviceException = + assertThrows(ServiceException.class, + () -> userRepository.getUserDefaultSettings(userId), + "Expected getUserDefaultSettings() to throw, but it didn't"); + assertThat(serviceException.getMessage(), is("Problem getting user default settings for " + userId)); + } } @Nested From 23a25b3e5b46b8da825dadd87d5143644816652d Mon Sep 17 00:00:00 2001 From: Lam Date: Wed, 20 Mar 2019 12:20:01 +0000 Subject: [PATCH 4/7] Switch to use ApplicationParams.encode() to encode the userId query string parameter Avoid setting the encoding mode on `restTemplate` because this is an autowired singleton bean, shared across the application. A future ticket will address encoding query string parameters for *all* requests, using `restTemplate`. --- src/main/java/uk/gov/hmcts/ccd/ApplicationParams.java | 2 +- .../uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/ccd/ApplicationParams.java b/src/main/java/uk/gov/hmcts/ccd/ApplicationParams.java index b9c41050ce..a78b824113 100644 --- a/src/main/java/uk/gov/hmcts/ccd/ApplicationParams.java +++ b/src/main/java/uk/gov/hmcts/ccd/ApplicationParams.java @@ -97,7 +97,7 @@ public class ApplicationParams { @Value("${search.elastic.nodes.discovery.filter}") private String elasticsearchNodeDiscoveryFilter; - private static String encode(final String stringToEncode) { + public static String encode(final String stringToEncode) { try { return URLEncoder.encode(stringToEncode, "UTF-8"); } catch (UnsupportedEncodingException e) { diff --git a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java index fa8f982a1c..1ca0784af1 100644 --- a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java +++ b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java @@ -18,8 +18,6 @@ import org.springframework.stereotype.Repository; import org.springframework.web.client.RestClientResponseException; import org.springframework.web.client.RestTemplate; -import org.springframework.web.util.DefaultUriBuilderFactory; -import org.springframework.web.util.DefaultUriBuilderFactory.EncodingMode; import uk.gov.hmcts.ccd.ApplicationParams; import uk.gov.hmcts.ccd.AuthCheckerConfiguration; import uk.gov.hmcts.ccd.data.SecurityUtils; @@ -94,11 +92,7 @@ public UserDefault getUserDefaultSettings(final String userId) { LOG.debug("retrieving default user settings for user {}", userId); final HttpEntity requestEntity = new HttpEntity(securityUtils.authorizationHeaders()); final Map queryParams = new HashMap<>(); - queryParams.put("uid", userId); - final DefaultUriBuilderFactory builderFactory = new DefaultUriBuilderFactory(); - // The EncodingMode.VALUES_ONLY mode ensures the query string variables are encoded - builderFactory.setEncodingMode(EncodingMode.VALUES_ONLY); - restTemplate.setUriTemplateHandler(builderFactory); + queryParams.put("uid", ApplicationParams.encode(userId)); return restTemplate.exchange(applicationParams.userDefaultSettingsURL(), HttpMethod.GET, requestEntity, UserDefault.class, queryParams).getBody(); } catch (RestClientResponseException e) { From 62c6d5b08f5da041989d067e029a416651d8adb0 Mon Sep 17 00:00:00 2001 From: Lam Date: Thu, 21 Mar 2019 11:00:31 +0000 Subject: [PATCH 5/7] Fix "double encoding" problem introduced by previous commit --- .../ccd/data/user/DefaultUserRepository.java | 11 +++++++++-- .../data/user/DefaultUserRepositoryTest.java | 19 +++++++++---------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java index 1ca0784af1..472cebd4af 100644 --- a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java +++ b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java @@ -1,5 +1,7 @@ package uk.gov.hmcts.ccd.data.user; +import java.net.URI; +import java.net.URISyntaxException; import java.util.*; import java.util.stream.Collectors; @@ -18,6 +20,7 @@ import org.springframework.stereotype.Repository; import org.springframework.web.client.RestClientResponseException; import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; import uk.gov.hmcts.ccd.ApplicationParams; import uk.gov.hmcts.ccd.AuthCheckerConfiguration; import uk.gov.hmcts.ccd.data.SecurityUtils; @@ -93,8 +96,10 @@ public UserDefault getUserDefaultSettings(final String userId) { final HttpEntity requestEntity = new HttpEntity(securityUtils.authorizationHeaders()); final Map queryParams = new HashMap<>(); queryParams.put("uid", ApplicationParams.encode(userId)); - return restTemplate.exchange(applicationParams.userDefaultSettingsURL(), - HttpMethod.GET, requestEntity, UserDefault.class, queryParams).getBody(); + final String encodedUrl = UriComponentsBuilder.fromHttpUrl(applicationParams.userDefaultSettingsURL()) + .buildAndExpand(queryParams).toUriString(); + return restTemplate.exchange(new URI(encodedUrl), HttpMethod.GET, requestEntity, UserDefault.class) + .getBody(); } catch (RestClientResponseException e) { LOG.error("Failed to retrieve user profile", e); final List headerMessages = Optional.ofNullable(e.getResponseHeaders()) @@ -104,6 +109,8 @@ public UserDefault getUserDefaultSettings(final String userId) { throw new BadRequestException(message); } throw new ServiceException("Problem getting user default settings for " + userId); + } catch (URISyntaxException e) { + throw new BadRequestException(e.getMessage()); } } diff --git a/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java b/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java index 11f689508b..906425a362 100644 --- a/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepositoryTest.java @@ -1,5 +1,6 @@ package uk.gov.hmcts.ccd.data.user; +import java.net.URI; import java.util.Arrays; import java.util.Collection; import java.util.Set; @@ -14,13 +15,11 @@ import static org.hamcrest.Matchers.hasSize; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.anyMap; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyList; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.doReturn; -import static org.mockito.Matchers.eq; -import static org.mockito.Matchers.same; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -193,13 +192,13 @@ void shouldReturnUserProfileDefaultsForUser() { userDefault.setJurisdictions(singletonList(jurisdiction)); final ResponseEntity responseEntity = new ResponseEntity<>(userDefault, HttpStatus.OK); when(restTemplate - .exchange(anyString(), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class), anyMap())) + .exchange(any(URI.class), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class))) .thenReturn(responseEntity); final UserDefault result = userRepository.getUserDefaultSettings("ccd+test@hmcts.net"); assertThat(result, is(userDefault)); verify(restTemplate).exchange( - anyString(), same(HttpMethod.GET), any(HttpEntity.class), (Class)any(Class.class), anyMap()); + any(URI.class), same(HttpMethod.GET), any(HttpEntity.class), (Class)any(Class.class)); } @Test @@ -211,7 +210,7 @@ void shouldThrowExceptionIfUserProfileCannotBeRetrieved() { final RestClientResponseException exception = new RestClientResponseException("Error on GET", 400, "Bad Request", headers, null, null); when(restTemplate - .exchange(anyString(), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class), anyMap())) + .exchange(any(URI.class), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class))) .thenThrow(exception); final BadRequestException badRequestException = @@ -228,7 +227,7 @@ void shouldThrowExceptionIfErrorOnRetrievingUserProfile() { final RestClientResponseException exception = new RestClientResponseException(null, 500, "Internal Server Error", null, null, null); when(restTemplate - .exchange(anyString(), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class), anyMap())) + .exchange(any(URI.class), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class))) .thenThrow(exception); final String userId = "ccd+test@hmcts.net"; From 7f0b261d1e4ba6094b056a031c62344def52df50 Mon Sep 17 00:00:00 2001 From: Lam Date: Tue, 9 Apr 2019 16:45:31 +0100 Subject: [PATCH 6/7] Fix missing import statement --- .../java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java index 55f559717f..c9f45d2124 100644 --- a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java +++ b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; From 4fec23fe3b53e796b72cd54af5a6041527282358 Mon Sep 17 00:00:00 2001 From: Lam Date: Wed, 10 Apr 2019 11:00:01 +0100 Subject: [PATCH 7/7] Remove unused import --- .../java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java index c9f45d2124..641fc33eb4 100644 --- a/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java +++ b/src/main/java/uk/gov/hmcts/ccd/data/user/DefaultUserRepository.java @@ -24,7 +24,6 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Repository; -import org.springframework.web.client.HttpStatusCodeException; import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestClientResponseException; import org.springframework.web.client.RestTemplate;