From 60bf5e792174cc0df536a261b17df7eb40fc357a Mon Sep 17 00:00:00 2001 From: "james.howes" Date: Fri, 3 Dec 2021 10:38:30 +0000 Subject: [PATCH] GOVSI-1161 - Update ID Token to have accurate VOT value - Now we have added LevelOfConfidence we need to ensure that the VectorOfClient assigned to the ClientSession at Authorize, is placed in the ID token. - Add additional tests to ensure the ID token is correctly populated. --- .../api/TokenIntegrationTest.java | 81 +++++++++++++++---- .../oidc/lambda/TokenHandler.java | 3 +- .../oidc/lambda/TokenHandlerTest.java | 36 +++++++-- .../sharedtest/extensions/RedisExtension.java | 6 +- .../shared/entity/CredentialTrustLevel.java | 23 ------ .../shared/entity/VectorOfTrust.java | 8 ++ .../shared/entity/VectorOfTrustTest.java | 23 ++++++ 7 files changed, 129 insertions(+), 51 deletions(-) diff --git a/integration-tests/src/test/java/uk/gov/di/authentication/api/TokenIntegrationTest.java b/integration-tests/src/test/java/uk/gov/di/authentication/api/TokenIntegrationTest.java index 0f2f68f478..fe81d8cfa0 100644 --- a/integration-tests/src/test/java/uk/gov/di/authentication/api/TokenIntegrationTest.java +++ b/integration-tests/src/test/java/uk/gov/di/authentication/api/TokenIntegrationTest.java @@ -25,14 +25,19 @@ import com.nimbusds.openid.connect.sdk.AuthenticationRequest; import com.nimbusds.openid.connect.sdk.Nonce; import com.nimbusds.openid.connect.sdk.OIDCScopeValue; +import com.nimbusds.openid.connect.sdk.OIDCTokenResponse; +import net.minidev.json.JSONArray; import net.minidev.json.JSONObject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import uk.gov.di.authentication.oidc.lambda.TokenHandler; import uk.gov.di.authentication.shared.entity.ClientConsent; import uk.gov.di.authentication.shared.entity.RefreshTokenStore; import uk.gov.di.authentication.shared.entity.ServiceType; import uk.gov.di.authentication.shared.entity.ValidScopes; +import uk.gov.di.authentication.shared.entity.VectorOfTrust; import uk.gov.di.authentication.sharedtest.basetest.ApiGatewayHandlerIntegrationTest; import uk.gov.di.authentication.sharedtest.helper.KeyPairHelper; @@ -51,9 +56,11 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.stream.Stream; import static java.util.Collections.singletonList; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static uk.gov.di.authentication.sharedtest.matchers.APIGatewayProxyResponseEventMatcher.hasStatus; @@ -71,15 +78,30 @@ void setup() { handler = new TokenHandler(TEST_CONFIGURATION_SERVICE); } - @Test - void shouldCallTokenResourceAndReturnAccessAndRefreshToken() - throws JOSEException, ParseException, JsonProcessingException { + private static Stream validVectorValues() { + return Stream.of( + Optional.of("Cl.Cm"), + Optional.of("Cl"), + Optional.of("Pm.Cl.Cm"), + Optional.of("Pm.Cl"), + Optional.of("Pl.Cl.Cm"), + Optional.of("Pl.Cl"), + Optional.of("Ph.Cl"), + Optional.of("Ph.Cl.Cm"), + Optional.empty()); + } + + @ParameterizedTest + @MethodSource("validVectorValues") + void shouldCallTokenResourceAndReturnAccessAndRefreshToken(Optional vtr) + throws JOSEException, ParseException, JsonProcessingException, + java.text.ParseException { KeyPair keyPair = KeyPairHelper.GENERATE_RSA_KEY_PAIR(); Scope scope = new Scope( OIDCScopeValue.OPENID.getValue(), OIDCScopeValue.OFFLINE_ACCESS.getValue()); setUpDynamo(keyPair, scope, new Subject()); - var response = generateTokenRequest(keyPair, scope); + var response = generateTokenRequest(keyPair, scope, vtr); assertThat(response, hasStatus(200)); JSONObject jsonResponse = JSONObjectUtils.parse(response.getBody()); @@ -93,6 +115,16 @@ void shouldCallTokenResourceAndReturnAccessAndRefreshToken() .toSuccessResponse() .getTokens() .getBearerAccessToken()); + if (vtr.isEmpty()) { + vtr = Optional.of(VectorOfTrust.getDefaults().getCredentialTrustLevel().getValue()); + } + assertThat( + OIDCTokenResponse.parse(jsonResponse) + .getOIDCTokens() + .getIDToken() + .getJWTClaimsSet() + .getClaim("vot"), + equalTo(vtr.get())); } @Test @@ -101,7 +133,7 @@ void shouldCallTokenResourceAndOnlyReturnAccessTokenWithoutOfflineAccessScope() KeyPair keyPair = KeyPairHelper.GENERATE_RSA_KEY_PAIR(); Scope scope = new Scope(OIDCScopeValue.OPENID.getValue()); setUpDynamo(keyPair, scope, new Subject()); - var response = generateTokenRequest(keyPair, scope); + var response = generateTokenRequest(keyPair, scope, Optional.empty()); assertThat(response, hasStatus(200)); JSONObject jsonResponse = JSONObjectUtils.parse(response.getBody()); @@ -206,21 +238,26 @@ private void setUpDynamo(KeyPair keyPair, Scope scope, Subject internalSubject) userStore.updateConsent(TEST_EMAIL, clientConsent); } - private AuthenticationRequest generateAuthRequest(Scope scope) { + private AuthenticationRequest generateAuthRequest(Scope scope, Optional vtr) { ResponseType responseType = new ResponseType(ResponseType.Value.CODE); State state = new State(); Nonce nonce = new Nonce(); - return new AuthenticationRequest.Builder( - responseType, - scope, - new ClientID(CLIENT_ID), - URI.create("http://localhost/redirect")) - .state(state) - .nonce(nonce) - .build(); + AuthenticationRequest.Builder builder = + new AuthenticationRequest.Builder( + responseType, + scope, + new ClientID(CLIENT_ID), + URI.create("http://localhost/redirect")) + .state(state) + .nonce(nonce); + + vtr.ifPresent(v -> builder.customParameter("vtr", v)); + + return builder.build(); } - private APIGatewayProxyResponseEvent generateTokenRequest(KeyPair keyPair, Scope scope) + private APIGatewayProxyResponseEvent generateTokenRequest( + KeyPair keyPair, Scope scope, Optional vtr) throws JOSEException, JsonProcessingException { PrivateKey privateKey = keyPair.getPrivate(); LocalDateTime localDateTime = LocalDateTime.now().plusMinutes(5); @@ -233,8 +270,20 @@ private APIGatewayProxyResponseEvent generateTokenRequest(KeyPair keyPair, Scope new PrivateKeyJWT( claimsSet, JWSAlgorithm.RS256, (RSAPrivateKey) privateKey, null, null); String code = new AuthorizationCode().toString(); + VectorOfTrust vectorOfTrust = VectorOfTrust.getDefaults(); + if (vtr.isPresent()) { + JSONArray jsonArray = new JSONArray(); + jsonArray.add(vtr.get()); + vectorOfTrust = + VectorOfTrust.parseFromAuthRequestAttribute( + singletonList(jsonArray.toJSONString())); + } redis.addAuthCodeAndCreateClientSession( - code, "a-client-session-id", TEST_EMAIL, generateAuthRequest(scope).toParameters()); + code, + "a-client-session-id", + TEST_EMAIL, + generateAuthRequest(scope, vtr).toParameters(), + vectorOfTrust); Map> customParams = new HashMap<>(); customParams.put( "grant_type", Collections.singletonList(GrantType.AUTHORIZATION_CODE.getValue())); diff --git a/oidc-api/src/main/java/uk/gov/di/authentication/oidc/lambda/TokenHandler.java b/oidc-api/src/main/java/uk/gov/di/authentication/oidc/lambda/TokenHandler.java index 180c96fc92..b32472a904 100644 --- a/oidc-api/src/main/java/uk/gov/di/authentication/oidc/lambda/TokenHandler.java +++ b/oidc-api/src/main/java/uk/gov/di/authentication/oidc/lambda/TokenHandler.java @@ -235,8 +235,7 @@ public APIGatewayProxyResponseEvent handleRequest( String vot = clientSession .getEffectiveVectorOfTrust() - .getCredentialTrustLevel() - .getValue(); + .retrieveVectorOfTrustForToken(); OIDCTokenResponse tokenResponse = tokenService.generateTokenResponse( diff --git a/oidc-api/src/test/java/uk/gov/di/authentication/oidc/lambda/TokenHandlerTest.java b/oidc-api/src/test/java/uk/gov/di/authentication/oidc/lambda/TokenHandlerTest.java index f70df89653..503d6f41a8 100644 --- a/oidc-api/src/test/java/uk/gov/di/authentication/oidc/lambda/TokenHandlerTest.java +++ b/oidc-api/src/test/java/uk/gov/di/authentication/oidc/lambda/TokenHandlerTest.java @@ -30,8 +30,11 @@ import com.nimbusds.openid.connect.sdk.OIDCScopeValue; import com.nimbusds.openid.connect.sdk.OIDCTokenResponse; import com.nimbusds.openid.connect.sdk.token.OIDCTokens; +import net.minidev.json.JSONArray; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import uk.gov.di.authentication.shared.entity.AuthCodeExchangeData; import uk.gov.di.authentication.shared.entity.ClientConsent; import uk.gov.di.authentication.shared.entity.ClientRegistry; @@ -67,6 +70,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.Stream; import static java.util.Collections.singletonList; import static org.hamcrest.MatcherAssert.assertThat; @@ -129,10 +133,16 @@ public void setUp() { redisConnectionService); } - @Test - public void shouldReturn200ForSuccessfulTokenRequest() throws JOSEException { - VectorOfTrust vot = mock(VectorOfTrust.class); - when(vot.getCredentialTrustLevel()).thenReturn(CredentialTrustLevel.MEDIUM_LEVEL); + private static Stream validVectorValues() { + return Stream.of( + "Cl.Cm", "Cl", "Pm.Cl.Cm", "Pm.Cl", "Pl.Cl.Cm", "Pl.Cl", "Ph.Cl", "Ph.Cl.Cm"); + } + + @ParameterizedTest + @MethodSource("validVectorValues") + public void shouldReturn200ForSuccessfulTokenRequest(String vectorValue) throws JOSEException { + JSONArray jsonArray = new JSONArray(); + jsonArray.add(vectorValue); KeyPair keyPair = generateRsaKeyPair(); UserProfile userProfile = generateUserProfile(); SignedJWT signedJWT = @@ -163,11 +173,14 @@ public void shouldReturn200ForSuccessfulTokenRequest() throws JOSEException { new AuthCodeExchangeData() .setEmail(TEST_EMAIL) .setClientSessionId(CLIENT_SESSION_ID))); - + AuthenticationRequest authenticationRequest = generateAuthRequest(jsonArray.toJSONString()); + VectorOfTrust vtr = + VectorOfTrust.parseFromAuthRequestAttribute( + authenticationRequest.getCustomParameter("vtr")); when(clientSessionService.getClientSession(CLIENT_SESSION_ID)) .thenReturn( new ClientSession( - generateAuthRequest().toParameters(), LocalDateTime.now(), vot)); + authenticationRequest.toParameters(), LocalDateTime.now(), vtr)); when(dynamoService.getUserProfileByEmail(eq(TEST_EMAIL))).thenReturn(userProfile); when(tokenService.generateTokenResponse( CLIENT_ID, @@ -175,7 +188,7 @@ public void shouldReturn200ForSuccessfulTokenRequest() throws JOSEException { SCOPES, Map.of("nonce", NONCE), PUBLIC_SUBJECT, - VOT, + vtr.retrieveVectorOfTrustForToken(), userProfile.getClientConsent(), clientRegistry.isInternalService())) .thenReturn(tokenResponse); @@ -486,6 +499,7 @@ private PrivateKeyJWT generatePrivateKeyJWT(PrivateKey privateKey) throws JOSEEx private ClientRegistry generateClientRegistry(KeyPair keyPair) { return new ClientRegistry() .setClientID(CLIENT_ID) + .setInternalService(false) .setClientName("test-client") .setRedirectUrls(singletonList(REDIRECT_URI)) .setScopes(SCOPES.toStringList()) @@ -547,6 +561,13 @@ private APIGatewayProxyResponseEvent generateApiGatewayRequest( } private AuthenticationRequest generateAuthRequest() { + JSONArray jsonArray = new JSONArray(); + jsonArray.add("Cl.Cm"); + jsonArray.add("Cl"); + return generateAuthRequest(jsonArray.toJSONString()); + } + + private AuthenticationRequest generateAuthRequest(String vtr) { ResponseType responseType = new ResponseType(ResponseType.Value.CODE); State state = new State(); return new AuthenticationRequest.Builder( @@ -556,6 +577,7 @@ private AuthenticationRequest generateAuthRequest() { URI.create(REDIRECT_URI)) .state(state) .nonce(NONCE) + .customParameter("vtr", vtr) .build(); } diff --git a/shared-test/src/main/java/uk/gov/di/authentication/sharedtest/extensions/RedisExtension.java b/shared-test/src/main/java/uk/gov/di/authentication/sharedtest/extensions/RedisExtension.java index ec239613f9..c3473264d0 100644 --- a/shared-test/src/main/java/uk/gov/di/authentication/sharedtest/extensions/RedisExtension.java +++ b/shared-test/src/main/java/uk/gov/di/authentication/sharedtest/extensions/RedisExtension.java @@ -165,7 +165,8 @@ public void addAuthCodeAndCreateClientSession( String authCode, String clientSessionId, String email, - Map> authRequest) + Map> authRequest, + VectorOfTrust vtr) throws JsonProcessingException { redis.saveWithExpiry( AUTH_CODE_PREFIX.concat(authCode), @@ -177,8 +178,7 @@ public void addAuthCodeAndCreateClientSession( redis.saveWithExpiry( CLIENT_SESSION_PREFIX.concat(clientSessionId), objectMapper.writeValueAsString( - new ClientSession( - authRequest, LocalDateTime.now(), VectorOfTrust.getDefaults())), + new ClientSession(authRequest, LocalDateTime.now(), vtr)), 300); } diff --git a/shared/src/main/java/uk/gov/di/authentication/shared/entity/CredentialTrustLevel.java b/shared/src/main/java/uk/gov/di/authentication/shared/entity/CredentialTrustLevel.java index 6ee840065a..c4ecb6de09 100644 --- a/shared/src/main/java/uk/gov/di/authentication/shared/entity/CredentialTrustLevel.java +++ b/shared/src/main/java/uk/gov/di/authentication/shared/entity/CredentialTrustLevel.java @@ -1,8 +1,6 @@ package uk.gov.di.authentication.shared.entity; import java.util.Arrays; -import java.util.HashSet; -import java.util.List; public enum CredentialTrustLevel { LOW_LEVEL("Cl"), @@ -18,27 +16,6 @@ public String getValue() { return value; } - public static CredentialTrustLevel retrieveCredentialTrustLevel(List vtrSets) { - - return Arrays.stream(values()) - .filter( - tl -> - vtrSets.stream() - .anyMatch( - set -> - new HashSet<>( - Arrays.asList( - set.split("\\."))) - .equals( - new HashSet<>( - Arrays.asList( - tl.getValue() - .split( - "\\.")))))) - .findFirst() - .orElseThrow(() -> new IllegalArgumentException("Invalid CredentialTrustLevel")); - } - public static CredentialTrustLevel retrieveCredentialTrustLevel(String vtrSets) { return Arrays.stream(values()) diff --git a/shared/src/main/java/uk/gov/di/authentication/shared/entity/VectorOfTrust.java b/shared/src/main/java/uk/gov/di/authentication/shared/entity/VectorOfTrust.java index 4af1bf0f9e..274ad434cb 100644 --- a/shared/src/main/java/uk/gov/di/authentication/shared/entity/VectorOfTrust.java +++ b/shared/src/main/java/uk/gov/di/authentication/shared/entity/VectorOfTrust.java @@ -72,6 +72,14 @@ public static VectorOfTrust getDefaults() { return new VectorOfTrust(CredentialTrustLevel.getDefault()); } + public String retrieveVectorOfTrustForToken() { + if (Objects.isNull(levelOfConfidence)) { + return credentialTrustLevel.getValue(); + } else { + return levelOfConfidence.getValue() + "." + credentialTrustLevel.getValue(); + } + } + private static VectorOfTrust parseVtrSet(JSONArray vtrJsonArray) { List vectorOfTrusts = new ArrayList<>(); for (Object obj : vtrJsonArray) { diff --git a/shared/src/test/java/uk/gov/di/authentication/shared/entity/VectorOfTrustTest.java b/shared/src/test/java/uk/gov/di/authentication/shared/entity/VectorOfTrustTest.java index 4eaa1fae17..bbbfbb763b 100644 --- a/shared/src/test/java/uk/gov/di/authentication/shared/entity/VectorOfTrustTest.java +++ b/shared/src/test/java/uk/gov/di/authentication/shared/entity/VectorOfTrustTest.java @@ -179,4 +179,27 @@ void shouldThrowIfEmptyListIsPresent() { IllegalArgumentException.class, () -> VectorOfTrust.parseFromAuthRequestAttribute(Collections.singletonList(""))); } + + @Test + void shouldReturnCorrectlyFormattedVectorOfTrustForTokenWhenIdentityValuesArePresent() { + String vectorString = "Pm.Cl.Cm"; + JSONArray jsonArray = new JSONArray(); + jsonArray.add(vectorString); + VectorOfTrust vectorOfTrust = + VectorOfTrust.parseFromAuthRequestAttribute( + Collections.singletonList(jsonArray.toJSONString())); + assertThat(vectorOfTrust.retrieveVectorOfTrustForToken(), equalTo(vectorString)); + } + + @Test + void + shouldReturnCorrectlyFormattedVectorOfTrustForTokenWhenOnlyCredentialTrustLevelIsPresent() { + String vectorString = "Cl.Cm"; + JSONArray jsonArray = new JSONArray(); + jsonArray.add(vectorString); + VectorOfTrust vectorOfTrust = + VectorOfTrust.parseFromAuthRequestAttribute( + Collections.singletonList(jsonArray.toJSONString())); + assertThat(vectorOfTrust.retrieveVectorOfTrustForToken(), equalTo(vectorString)); + } }