Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Apr 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/uk/gov/hmcts/ccd/ApplicationParams.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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());
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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)
);
}
Expand All @@ -141,7 +151,7 @@ void shouldConsiderCitizen() {
userRepository.getUserClassifications(JURISDICTION_ID);

assertAll(
() -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(asList(CITIZEN)),
() -> verify(caseDefinitionRepository).getClassificationsForUserRoleList(singletonList(CITIZEN)),
() -> verifyNoMoreInteractions(caseDefinitionRepository)
);
}
Expand All @@ -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)
);
}
Expand All @@ -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("[email protected]");
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("[email protected]"),
"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 = "[email protected]";
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 {
Expand All @@ -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);

Expand All @@ -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));
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/resources/mappings/user1-profile.json
Original file line number Diff line number Diff line change
@@ -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,
Expand Down