RDM-4369: Support reserved characters in userId email address (#420)

Daniel-Lam · hemantt · commit bcd4a517b1fc · 2019-04-15T17:32:48.000+01:00
* 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. * 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 spring-projects/spring-framework#20750 (comment)). * Add missing tests to increase coverage to required level * 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`. * Fix "double encoding" problem introduced by previous commit * Fix missing import statement * Remove unused import
diff --git a/src/main/java/uk/gov/hmcts/ccd/ApplicationParams.java b/src/main/java/uk/gov/hmcts/ccd/ApplicationParams.java
@@ -103,7 +103,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
@@ -1,9 +1,12 @@
 package uk.gov.hmcts.ccd.data.user;
 
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.HashMap;
 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;
 
@@ -21,9 +24,10 @@
 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;
+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;
@@ -111,17 +115,22 @@ public UserDefault getUserDefaultSettings(final String userId) {
             LOG.debug("retrieving default user settings for user {}", userId);
             final HttpEntity requestEntity = new HttpEntity(securityUtils.authorizationHeaders());
             final Map<String, String> queryParams = new HashMap<>();
-            queryParams.put("uid", userId);
-            return restTemplate.exchange(applicationParams.userDefaultSettingsURL(),
-                HttpMethod.GET, requestEntity, UserDefault.class, queryParams).getBody();
-        } catch (HttpStatusCodeException e) {
+            queryParams.put("uid", ApplicationParams.encode(userId));
+            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<String> headerMessages = e.getResponseHeaders().get("Message");
+            final List<String> 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);
             }
             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
@@ -1,22 +1,25 @@
 package uk.gov.hmcts.ccd.data.user;
 
+import java.net.URI;
 import java.util.Arrays;
 import java.util.Collection;
 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.Matchers.any;
-import static org.mockito.Matchers.anyListOf;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.eq;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyList;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.same;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.verify;
@@ -31,20 +34,26 @@
 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;
 import org.springframework.security.core.Authentication;
 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.RestClientException;
+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;
 import uk.gov.hmcts.ccd.data.casedetails.SecurityClassification;
 import uk.gov.hmcts.ccd.data.definition.CaseDefinitionRepository;
 import uk.gov.hmcts.ccd.domain.model.aggregated.IdamUser;
+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;
 
@@ -123,7 +132,8 @@ void shouldOnlyConsiderRolesForJurisdiction() {
                 userRepository.getUserClassifications(JURISDICTION_ID);
 
                 assertAll(
-                    () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(asList(ROLE_CASEWORKER_CMC)),
+                    () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(
+                        singletonList(ROLE_CASEWORKER_CMC)),
                     () -> verifyNoMoreInteractions(caseDefinitionRepository)
                 );
             }
@@ -141,7 +151,7 @@ void shouldConsiderCitizen() {
                 userRepository.getUserClassifications(JURISDICTION_ID);
 
                 assertAll(
-                    () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(asList(CITIZEN)),
+                    () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(singletonList(CITIZEN)),
                     () -> verifyNoMoreInteractions(caseDefinitionRepository)
                 );
             }
@@ -154,7 +164,8 @@ void shouldConsiderLetterHolder() {
                 userRepository.getUserClassifications(JURISDICTION_ID);
 
                 assertAll(
-                    () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(asList(LETTER_HOLDER)),
+                    () -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(
+                        singletonList(LETTER_HOLDER)),
                     () -> verifyNoMoreInteractions(caseDefinitionRepository)
                 );
             }
@@ -169,6 +180,69 @@ 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(singletonList(jurisdiction));
+            final ResponseEntity<UserDefault> responseEntity = new ResponseEntity<>(userDefault, HttpStatus.OK);
+            when(restTemplate
+                .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(
+                any(URI.class), same(HttpMethod.GET), any(HttpEntity.class), (Class<?>)any(Class.class));
+        }
+
+        @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(any(URI.class), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class)))
+                .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(any(URI.class), eq(HttpMethod.GET), any(HttpEntity.class), eq(UserDefault.class)))
+                .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
     @DisplayName("getHighestUserClassification()")
     class GetHighestUserClassification {
@@ -184,7 +258,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);
 
@@ -195,7 +270,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
@@ -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,

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`{`
`2`	`2`	`"request": {`
`3`	`3`	`"method": "GET",`
`4`		`- "url": "/user-profile/users?uid=Cloud.Strife@test.com"`
	`4`	`+ "url": "/user-profile/users?uid=Cloud.Strife%40test.com"`
`5`	`5`	`},`
`6`	`6`	`"response": {`
`7`	`7`	`"status": 200,`