Skip to content

Commit

Permalink
[AK] bar-app needs extended user info from idam (#4)
Browse files Browse the repository at this point in the history
  • Loading branch information
attila-kiss-hmcts authored and alectronic0 committed Apr 13, 2018
1 parent cf5257a commit d92a7a3
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 36 deletions.
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ plugins {
}

group = 'uk.gov.hmcts.reform.auth'
version = '2.1.1'
version = '2.1.2'

sourceCompatibility = 1.8
targetCompatibility = 1.8
Expand All @@ -32,6 +32,7 @@ dependencies {
compile group: 'com.google.guava', name: 'guava', version:'21.0'
compile group: 'org.springframework.boot', name: 'spring-boot-autoconfigure', version:'1.5.6.RELEASE'
compile group: 'org.springframework.boot', name: 'spring-boot-starter-security', version:'1.5.6.RELEASE'
compile group: 'org.projectlombok', name: 'lombok', version:'1.16.10'
testCompile group: 'com.github.tomakehurst', name: 'wiremock', version:'2.5.1'
testCompile group: 'org.assertj', name: 'assertj-core', version:'3.5.2'
testCompile group: 'junit', name: 'junit', version:'4.12'
Expand All @@ -41,7 +42,6 @@ dependencies {
exclude(module: 'commons-logging')
}
compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version:'3.1.0'
compileOnly group: 'org.projectlombok', name: 'lombok', version:'1.16.10'
compileOnly group: 'net.sourceforge.findbugs', name: 'annotations', version:'1.3.2'
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenInvalidException;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenParsingException;

public class UserRequestAuthorizer implements RequestAuthorizer<User> {
public class UserRequestAuthorizer<T extends User> implements RequestAuthorizer<T> {
public static final String AUTHORISATION = "Authorization";

private final SubjectResolver<User> userResolver;
private final SubjectResolver<T> userResolver;
private final Function<HttpServletRequest, Optional<String>> userIdExtractor;
private final Function<HttpServletRequest, Collection<String>> authorizedRolesExtractor;

public UserRequestAuthorizer(SubjectResolver<User> userResolver,
public UserRequestAuthorizer(SubjectResolver<T> userResolver,
Function<HttpServletRequest, Optional<String>> userIdExtractor,
Function<HttpServletRequest, Collection<String>> authorizedRolesExtractor) {
this.userResolver = userResolver;
Expand All @@ -31,13 +31,13 @@ public UserRequestAuthorizer(SubjectResolver<User> userResolver,
}

@Override
public User authorise(HttpServletRequest request) throws UnauthorisedRoleException, UnauthorisedUserException {
public T authorise(HttpServletRequest request) throws UnauthorisedRoleException, UnauthorisedUserException {
String bearerToken = request.getHeader(AUTHORISATION);
if (bearerToken == null) {
throw new BearerTokenMissingException();
}

User user = getTokenDetails(bearerToken);
T user = getTokenDetails(bearerToken);

Collection<String> authorizedRoles = authorizedRolesExtractor.apply(request);
if (!authorizedRoles.isEmpty() && Collections.disjoint(authorizedRoles, user.getRoles())) {
Expand All @@ -49,7 +49,7 @@ public User authorise(HttpServletRequest request) throws UnauthorisedRoleExcepti
return user;
}

private User getTokenDetails(String bearerToken) {
private T getTokenDetails(String bearerToken) {
try {
return userResolver.getTokenDetails(bearerToken);
} catch (UserTokenInvalidException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenParser;

public class UserResolver implements SubjectResolver<User> {
private final UserTokenParser userTokenParser;
private final UserTokenParser<UserTokenDetails> userTokenParser;

public UserResolver(UserTokenParser userTokenParser) {
this.userTokenParser = userTokenParser;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import uk.gov.hmcts.reform.auth.checker.core.user.User;
import uk.gov.hmcts.reform.auth.checker.core.user.UserRequestAuthorizer;
import uk.gov.hmcts.reform.auth.checker.core.user.UserResolver;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenDetails;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenParser;
import uk.gov.hmcts.reform.auth.parser.idam.core.service.token.ServiceTokenParser;

Expand All @@ -38,23 +39,26 @@ public SubjectResolver<Service> serviceResolver(ServiceTokenParser serviceTokenP

@Bean
@ConditionalOnMissingBean(name = "userResolver")
public SubjectResolver<User> userResolver(UserTokenParser userTokenParser, AuthCheckerProperties properties) {
public SubjectResolver<User> userResolver(UserTokenParser<UserTokenDetails> userTokenParser, AuthCheckerProperties properties) {
return new CachingSubjectResolver<>(new UserResolver(userTokenParser), properties.getUser().getTtlInSeconds(), properties.getUser().getMaximumSize());
}

@Bean
@ConditionalOnMissingBean(name = "serviceRequestAuthorizer")
public ServiceRequestAuthorizer serviceRequestAuthorizer(SubjectResolver<Service> serviceResolver, Function<HttpServletRequest, Collection<String>> authorizedServicesExtractor) {
return new ServiceRequestAuthorizer(serviceResolver, authorizedServicesExtractor);
}

@Bean
@ConditionalOnMissingBean(name = "userRequestAuthorizer")
public UserRequestAuthorizer userRequestAuthorizer(SubjectResolver<User> userResolver,
Function<HttpServletRequest, Optional<String>> userIdExtractor,
Function<HttpServletRequest, Collection<String>> authorizedRolesExtractor) {
return new UserRequestAuthorizer(userResolver, userIdExtractor, authorizedRolesExtractor);
}

@Bean
@ConditionalOnMissingBean(name = "preAuthenticatedAuthenticationProvider")
public PreAuthenticatedAuthenticationProvider preAuthenticatedAuthenticationProvider() {
PreAuthenticatedAuthenticationProvider authenticationProvider = new PreAuthenticatedAuthenticationProvider();
authenticationProvider.setPreAuthenticatedUserDetailsService(new AuthCheckerUserDetailsService());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@


@Slf4j
public class AuthCheckerUserOnlyFilter extends AbstractPreAuthenticatedProcessingFilter {
public class AuthCheckerUserOnlyFilter<T extends User> extends AbstractPreAuthenticatedProcessingFilter {

private final RequestAuthorizer<User> userRequestAuthorizer;
private final RequestAuthorizer<T> userRequestAuthorizer;

public AuthCheckerUserOnlyFilter(RequestAuthorizer<User> userRequestAuthorizer) {
public AuthCheckerUserOnlyFilter(RequestAuthorizer<T> userRequestAuthorizer) {
this.userRequestAuthorizer = userRequestAuthorizer;
}

Expand All @@ -28,13 +28,12 @@ protected Object getPreAuthenticatedCredentials(HttpServletRequest request) {
return request.getHeader(UserRequestAuthorizer.AUTHORISATION);
}

private User authorizeUser(HttpServletRequest request) {
private T authorizeUser(HttpServletRequest request) {
try {
return userRequestAuthorizer.authorise(request);
} catch (AuthCheckerException e) {
log.warn("Unsuccessful user authentication", e);
log.warn("Unsuccessful user authentication");
return null;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,31 @@

import java.io.IOException;

public class HttpComponentsBasedUserTokenParser implements UserTokenParser {
public class HttpComponentsBasedUserTokenParser<T> implements UserTokenParser<T> {

private final HttpClient httpClient;
private final String baseUrl;
//bad
private final ObjectMapper objectMapper = new ObjectMapper();
private final Class<T> type;

public HttpComponentsBasedUserTokenParser(HttpClient httpClient, String baseUrl) {
public HttpComponentsBasedUserTokenParser(HttpClient httpClient, String baseUrl, Class<T> type) {
this.httpClient = httpClient;
this.baseUrl = baseUrl;
this.type = type;
}

@Override
@SuppressWarnings(value = "HTTP_PARAMETER_POLLUTION", justification = "baseUrl + /details is not based on user input")
public UserTokenDetails parse(String jwt) {
public T parse(String jwt) {
try {
String bearerJwt = jwt.startsWith("Bearer ") ? jwt : "Bearer " + jwt;
HttpGet request = new HttpGet(baseUrl + "/details");
request.addHeader("Authorization", bearerJwt);

return httpClient.execute(request, httpResponse -> {
checkStatusIs2xx(httpResponse);
return objectMapper.readValue(httpResponse.getEntity().getContent(), UserTokenDetails.class);
return objectMapper.readValue(httpResponse.getEntity().getContent(), type);
});
} catch (IOException e) {
throw new UserTokenParsingException(e);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package uk.gov.hmcts.reform.auth.parser.idam.core.user.token;

public interface UserTokenParser {
UserTokenDetails parse(String jwt) throws UserTokenParsingException;
public interface UserTokenParser<T> {
T parse(String jwt);
}

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Lazy;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.HttpComponentsBasedUserTokenParser;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenDetails;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenParser;

@Lazy
Expand All @@ -21,9 +22,9 @@ public HttpClient userTokenParserHttpClient() {
}

@Bean
public UserTokenParser userTokenParser(HttpClient userTokenParserHttpClient,
@Value("${auth.idam.client.baseUrl}") String baseUrl) {
return new HttpComponentsBasedUserTokenParser(userTokenParserHttpClient, baseUrl);
public UserTokenParser<UserTokenDetails> userTokenParser(HttpClient userTokenParserHttpClient,
@Value("${auth.idam.client.baseUrl}") String baseUrl) {
return new HttpComponentsBasedUserTokenParser<>(userTokenParserHttpClient, baseUrl, UserTokenDetails.class);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package uk.gov.hmcts.reform.auth.parser.idam.core.user.token;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;

import java.util.Set;

@Data
@AllArgsConstructor
@Builder
@JsonIgnoreProperties(ignoreUnknown = true)
public class CompleteUserTokenDetails {

private final String defaultService;
private final String email;
private final String forename;
private final String surname;
private final String id;
private final Set<String> roles;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,40 @@
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.HttpComponentsBasedUserTokenParser;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenDetails;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenInvalidException;
import uk.gov.hmcts.reform.auth.parser.idam.core.user.token.UserTokenParsingException;

import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
import static org.assertj.core.api.Assertions.assertThat;

public class HttpComponentsBasedUserTokenParserTest {

private static final String VALID_RESPONSE = "{ \"id\": \"UserId\", \"roles\": [\"citizen\"] }";
private static final String VALID_RESPONSE = "{\n" +
" \"defaultService\" : \"BAR\",\n" +
" \"email\" : \"post.clerk@hmcts.net\",\n" +
" \"forename\" : \"Chris\",\n" +
" \"id\" : 365750,\n" +
" \"roles\" : [\"bar-post-clerk\"],\n" +
" \"surname\" : \"Spencer\"\n" +
" }";

@Rule
public WireMockRule wireMockRule = new WireMockRule(wireMockConfig().dynamicPort());

private HttpComponentsBasedUserTokenParser client;
private HttpComponentsBasedUserTokenParser<UserTokenDetails> client;
private HttpComponentsBasedUserTokenParser<CompleteUserTokenDetails> fullClient;

// bad
@Before
public void setUp() throws Exception {
client = new HttpComponentsBasedUserTokenParser(
client = new HttpComponentsBasedUserTokenParser<>(
HttpClients.createMinimal(),
"http://localhost:" + wireMockRule.port()
"http://localhost:" + wireMockRule.port(),
UserTokenDetails.class
);

fullClient = new HttpComponentsBasedUserTokenParser<>(
HttpClients.createMinimal(),
"http://localhost:" + wireMockRule.port(),
CompleteUserTokenDetails.class
);
}

Expand All @@ -44,7 +54,7 @@ public void happyPath() {

UserTokenDetails userTokenDetails = client.parse("someJwt");

assertThat(userTokenDetails).isEqualTo(new UserTokenDetails("UserId", ImmutableSet.of("citizen")));
assertThat(userTokenDetails).isEqualTo(new UserTokenDetails("365750", ImmutableSet.of("bar-post-clerk")));
}

@Test
Expand All @@ -58,7 +68,7 @@ public void bearerShouldNotBePrependedIfItsAlreadyPresent() {

UserTokenDetails userTokenDetails = client.parse("Bearer someJwt");

assertThat(userTokenDetails).isEqualTo(new UserTokenDetails("UserId", ImmutableSet.of("citizen")));
assertThat(userTokenDetails).isEqualTo(new UserTokenDetails("365750", ImmutableSet.of("bar-post-clerk")));
}

@Test(expected = UserTokenParsingException.class)
Expand All @@ -84,4 +94,20 @@ public void status401ShouldResultInUserTokenInvalidException() {

client.parse("expiredJwt");
}

@Test
public void testFullUserRetrieval() {
stubFor(
get(urlEqualTo("/details")).withHeader("Authorization", matching("Bearer someJwt"))
.willReturn(aResponse()
.withStatus(200)
.withBody(VALID_RESPONSE))
);

CompleteUserTokenDetails userTokenDetails = fullClient.parse("someJwt");
assertThat(userTokenDetails).isEqualTo(
new CompleteUserTokenDetails("BAR", "post.clerk@hmcts.net", "Chris",
"Spencer", "365750", ImmutableSet.of("bar-post-clerk"))
);
}
}

0 comments on commit d92a7a3

Please sign in to comment.