Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ATO-595: Add journey ID to AUTH_USER_INFO_RETURNED audit event #4435

Closed
wants to merge 2 commits into from
Closed

ATO-595: Add journey ID to AUTH_USER_INFO_RETURNED audit event #4435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b719af6
ATO-595: Add journey ID (aka client session ID) to AUTH_USER_INFO_RET…
billrdunn May 9, 2024
a850b1b
ATO-595: Use `JOURNEY_ID` static member variable in TokenServiceTest
billrdunn May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions integration-tests/src/test/java/uk/gov/di/authentication/api/UserInfoIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public class UserInfoIntegrationTest extends ApiGatewayHandlerIntegrationTest {
private static final Subject PUBLIC_SUBJECT = new Subject();
private static final Subject INTERNAL_SUBJECT = new Subject();
private static final Subject INTERNAL_PAIRWISE_SUBJECT = new Subject();
private static final String JOURNEY_ID = "client-session-id";
private static final Scope DOC_APP_SCOPES =
new Scope(OIDCScopeValue.OPENID, CustomScopeValue.DOC_CHECKING_APP);
private static final Subject DOC_APP_PUBLIC_SUBJECT = new Subject();
Expand Down Expand Up @@ -149,7 +150,8 @@ void shouldCallUserInfoWithAccessTokenAndReturn200() throws Json.JsonException,
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenStoreString = objectMapper.writeValueAsString(accessTokenStore);
redis.addToRedis(
ACCESS_TOKEN_PREFIX + CLIENT_ID + "." + PUBLIC_SUBJECT,
Expand Down Expand Up @@ -395,7 +397,8 @@ private APIGatewayProxyResponseEvent makeIdentityUserinfoRequest() throws Json.J
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
redis.addToRedis(
ACCESS_TOKEN_PREFIX + CLIENT_ID + "." + PUBLIC_SUBJECT,
objectMapper.writeValueAsString(accessTokenStore),
Expand Down Expand Up @@ -424,7 +427,8 @@ private APIGatewayProxyResponseEvent makeDocAppUserinfoRequest() throws Json.Jso
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenStoreString = objectMapper.writeValueAsString(accessTokenStore);
redis.addToRedis(
ACCESS_TOKEN_PREFIX + APP_CLIENT_ID + "." + DOC_APP_PUBLIC_SUBJECT,
Expand Down
5 changes: 4 additions & 1 deletion oidc-api/src/main/java/uk/gov/di/authentication/oidc/lambda/UserInfoHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ public APIGatewayProxyResponseEvent userInfoRequestHandler(
auditService.submitAuditEvent(
OidcAuditableEvent.USER_INFO_RETURNED,
accessTokenInfo.getClientID(),
TxmaAuditUser.user().withUserId(subjectForAudit),
TxmaAuditUser.user()
.withUserId(subjectForAudit)
.withGovukSigninJourneyId(
accessTokenInfo.getAccessTokenStore().getJourneyId()),
metadataPairs);

return generateApiGatewayProxyResponse(200, userInfo.toJSONString());
Expand Down
17 changes: 15 additions & 2 deletions oidc-api/src/test/java/uk/gov/di/authentication/oidc/lambda/UserInfoHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import uk.gov.di.authentication.oidc.services.AccessTokenService;
import uk.gov.di.authentication.oidc.services.UserInfoService;
import uk.gov.di.orchestration.audit.TxmaAuditUser;
import uk.gov.di.orchestration.shared.entity.AccessTokenStore;
import uk.gov.di.orchestration.shared.entity.ValidClaims;
import uk.gov.di.orchestration.shared.exceptions.AccessTokenException;
import uk.gov.di.orchestration.shared.exceptions.ClientNotFoundException;
Expand All @@ -42,6 +43,10 @@ class UserInfoHandlerTest {
private static final String EMAIL_ADDRESS = "joe.bloggs@digital.cabinet-office.gov.uk";
private static final String PHONE_NUMBER = "01234567890";
private static final Subject SUBJECT = new Subject();
private static final String TOKEN = "token";
private static final String INTERNAL_SUBJECT_ID = "internal-subject-id";
private static final String INTERNAL_PAIRWISE_ID = "internal-pairwise-subject-id";
private static final String JOURNEY_ID = "client-session-id";
private static final Subject AUDIT_SUBJECT_ID = new Subject();
private final Context context = mock(Context.class);
private final ConfigurationService configurationService = mock(ConfigurationService.class);
Expand All @@ -62,6 +67,10 @@ void setUp() {
when(context.getAwsRequestId()).thenReturn("aws-request-id");
when(accessTokenInfo.getClientID()).thenReturn("client-id");
when(accessTokenInfo.getSubject()).thenReturn(SUBJECT.getValue());
when(accessTokenInfo.getAccessTokenStore())
.thenReturn(
new AccessTokenStore(
TOKEN, INTERNAL_SUBJECT_ID, INTERNAL_PAIRWISE_ID, JOURNEY_ID));
}

@Test
Expand Down Expand Up @@ -95,7 +104,9 @@ void shouldReturn200WithUserInfoBasedOnScopesForSuccessfulRequest()
.submitAuditEvent(
OidcAuditableEvent.USER_INFO_RETURNED,
"client-id",
TxmaAuditUser.user().withUserId(AUDIT_SUBJECT_ID.getValue()));
TxmaAuditUser.user()
.withUserId(AUDIT_SUBJECT_ID.getValue())
.withGovukSigninJourneyId(JOURNEY_ID));
}

@Test
Expand All @@ -118,7 +129,9 @@ void shouldAuditReturnCodeWhenReturnCodeClaimIsPresent()
.submitAuditEvent(
OidcAuditableEvent.USER_INFO_RETURNED,
"client-id",
TxmaAuditUser.user().withUserId(AUDIT_SUBJECT_ID.getValue()),
TxmaAuditUser.user()
.withUserId(AUDIT_SUBJECT_ID.getValue())
.withGovukSigninJourneyId(JOURNEY_ID),
AuditService.MetadataPair.pair("return-code", RETURN_CODE));
}

Expand Down
15 changes: 11 additions & 4 deletions oidc-api/src/test/java/uk/gov/di/authentication/oidc/services/AccessTokenServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class AccessTokenServiceTest {
private static final Subject INTERNAL_SUBJECT = new Subject("internal-subject");
private static final Subject INTERNAL_PAIRWISE_SUBJECT = new Subject();
private static final Subject SUBJECT = new Subject("some-subject");
private static final String JOURNEY_ID = "client-session-id";
private static final List<String> SCOPES =
List.of(
OIDCScopeValue.OPENID.getValue(),
Expand Down Expand Up @@ -123,7 +124,8 @@ void shouldReturnAccessTokenInfoWhenAccessTokenIsValid(boolean identityEnabled)
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue())));
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID)));

var accessTokenInfo =
validationService.parse(accessToken.toAuthorizationHeader(), identityEnabled);
Expand All @@ -133,6 +135,7 @@ void shouldReturnAccessTokenInfoWhenAccessTokenIsValid(boolean identityEnabled)
assertThat(
accessTokenInfo.getAccessTokenStore().getInternalSubjectId(),
equalTo(INTERNAL_SUBJECT.getValue()));
assertThat(accessTokenInfo.getAccessTokenStore().getJourneyId(), equalTo(JOURNEY_ID));
assertThat(accessTokenInfo.getSubject(), equalTo(SUBJECT.getValue()));
assertThat(accessTokenInfo.getScopes(), equalTo(SCOPES));
assertThat(accessTokenInfo.getIdentityClaims(), equalTo(expectedIdentityClaims));
Expand All @@ -151,7 +154,8 @@ void shouldNotReturnIdentityClaimsWhenClientIsNotConfiguredForIdentity()
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue())));
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID)));

var accessTokenInfo = validationService.parse(accessToken.toAuthorizationHeader(), true);

Expand All @@ -160,6 +164,7 @@ void shouldNotReturnIdentityClaimsWhenClientIsNotConfiguredForIdentity()
assertThat(
accessTokenInfo.getAccessTokenStore().getInternalSubjectId(),
equalTo(INTERNAL_SUBJECT.getValue()));
assertThat(accessTokenInfo.getAccessTokenStore().getJourneyId(), equalTo(JOURNEY_ID));
assertThat(accessTokenInfo.getSubject(), equalTo(SUBJECT.getValue()));
assertThat(accessTokenInfo.getScopes(), equalTo(SCOPES));
assertThat(accessTokenInfo.getIdentityClaims(), equalTo(null));
Expand Down Expand Up @@ -255,7 +260,8 @@ void shouldThrowExceptionWhenIdentityClaimsAreInvalid() throws Json.JsonExceptio
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue())));
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID)));

var accessTokenException =
assertThrows(
Expand Down Expand Up @@ -307,7 +313,8 @@ void shouldThrowExceptionWhenAccessTokenSentIsNotTheSameAsInRedis(boolean identi
new AccessTokenStore(
createSignedAccessToken(null, false).getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue())));
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID)));

var accessTokenException =
assertThrows(
Expand Down
31 changes: 21 additions & 10 deletions oidc-api/src/test/java/uk/gov/di/authentication/oidc/services/UserInfoServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class UserInfoServiceTest {
private static final String INTERNAL_SECTOR_URI = "https://test.account.gov.uk";
private static final Subject INTERNAL_SUBJECT = new Subject("internal-subject");
private static final Subject INTERNAL_PAIRWISE_SUBJECT = new Subject("test-subject");
private static final String JOURNEY_ID = "client-session-id";
private static final Subject SUBJECT = new Subject("some-subject");
private static final Subject DOC_APP_SUBJECT = new Subject("some-subject");
private static final List<String> SCOPES =
Expand Down Expand Up @@ -139,7 +140,8 @@ void shouldJustPopulateUserInfoWhenIdentityNotEnabled()
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(accessTokenStore, SUBJECT.getValue(), SCOPES, null, CLIENT_ID);
givenThereIsUserInfo();
Expand Down Expand Up @@ -183,7 +185,8 @@ void shouldJustPopulateWalletSubjectIdClaimWhenWalletSubjectIdScopeIsPresent()
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(accessTokenStore, SUBJECT.getValue(), scopes, null, CLIENT_ID);

Expand Down Expand Up @@ -214,7 +217,8 @@ void shouldJustPopulateUserInfoWhenIdentityEnabledButNoIdentityClaimsPresent()
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(
accessTokenStore, SUBJECT.getValue(), SCOPES, null, CLIENT_ID);
Expand Down Expand Up @@ -263,7 +267,8 @@ void shouldPopulateIdentityClaimsWhenClaimsArePresentAndIdentityIsEnabled()
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(
accessTokenStore,
Expand Down Expand Up @@ -319,7 +324,8 @@ void shouldJustPopulateEmailClaimWhenOnlyEmailScopeIsPresentAndIdentityNotEnable
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(
accessTokenStore, SUBJECT.getValue(), scopes, null, CLIENT_ID);
Expand Down Expand Up @@ -349,7 +355,8 @@ void shouldJustPopulateEmailClaimWhenOnlyEmailScopeIsPresentAndIdentity()
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(
accessTokenStore, SUBJECT.getValue(), scopes, null, CLIENT_ID);
Expand Down Expand Up @@ -386,7 +393,8 @@ void shouldPopulateIdentityClaimsWhenClaimsArePresentButNoAdditionalClaimsArePre
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(
accessTokenStore,
Expand Down Expand Up @@ -439,7 +447,8 @@ void shouldPopulateUserInfoWithDocAppCredentialWhenDocAppScopeIsPresent()
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(
accessTokenStore, SUBJECT.getValue(), docAppScope, null, CLIENT_ID);
Expand All @@ -462,7 +471,8 @@ void shouldReturnDocAppSubjectIdWhenDocAppScopeIsPresent() throws JOSEException
new AccessTokenStore(
accessToken.getValue(),
DOC_APP_SUBJECT.getValue(),
DOC_APP_SUBJECT.getValue());
DOC_APP_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(
accessTokenStore,
Expand Down Expand Up @@ -492,7 +502,8 @@ void shouldReturnInternalCommonSubjectIdentifierWhenDocAppScopeIsNotPresent()
new AccessTokenStore(
accessToken.getValue(),
INTERNAL_SUBJECT.getValue(),
INTERNAL_PAIRWISE_SUBJECT.getValue());
INTERNAL_PAIRWISE_SUBJECT.getValue(),
JOURNEY_ID);
var accessTokenInfo =
new AccessTokenInfo(
accessTokenStore, SUBJECT.getValue(), SCOPES, null, CLIENT_ID);
Expand Down
12 changes: 11 additions & 1 deletion ...stration-shared/src/main/java/uk/gov/di/orchestration/shared/entity/AccessTokenStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@ public class AccessTokenStore {

@Expose private String internalPairwiseSubjectId = "missing";

@Expose private String journeyId;

public AccessTokenStore() {}

public AccessTokenStore(
String token, String internalSubjectId, String internalPairwiseSubjectId) {
String token,
String internalSubjectId,
String internalPairwiseSubjectId,
String journeyId) {
this.token = token;
this.internalSubjectId = internalSubjectId;
this.internalPairwiseSubjectId = internalPairwiseSubjectId;
this.journeyId = journeyId;
}

public String getToken() {
Expand All @@ -30,4 +36,8 @@ public String getInternalSubjectId() {
public String getInternalPairwiseSubjectId() {
return internalPairwiseSubjectId;
}

public String getJourneyId() {
return journeyId;
}
}
13 changes: 9 additions & 4 deletions orchestration-shared/src/main/java/uk/gov/di/orchestration/shared/services/TokenService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ public OIDCTokenResponse generateTokenResponse(
rpPairwiseSubject,
internalPairwiseSubject,
claimsRequest,
signingAlgorithm));
signingAlgorithm,
journeyId));
AccessTokenHash accessTokenHash =
segmentedFunctionCall(
"AccessTokenHash.compute",
Expand Down Expand Up @@ -165,7 +166,8 @@ public OIDCTokenResponse generateRefreshTokenResponse(
rpPaiwiseSubject,
internalPairwiseSubject,
null,
signingAlgorithm);
signingAlgorithm,
null);
RefreshToken refreshToken =
generateAndStoreRefreshToken(
clientID,
Expand Down Expand Up @@ -322,7 +324,8 @@ private AccessToken generateAndStoreAccessToken(
Subject rpPairwiseSubject,
Subject internalPairwiseSubject,
OIDCClaimsRequest claimsRequest,
JWSAlgorithm signingAlgorithm) {
JWSAlgorithm signingAlgorithm,
String journeyId) {

LOG.info("Generating AccessToken");
Date expiryDate =
Expand All @@ -338,6 +341,7 @@ private AccessToken generateAndStoreAccessToken(
.expirationTime(expiryDate)
.issueTime(NowHelper.now())
.claim("client_id", clientId)
.claim("sid", journeyId)
.subject(rpPairwiseSubject.getValue())
.jwtID(jwtID);

Expand Down Expand Up @@ -366,7 +370,8 @@ private AccessToken generateAndStoreAccessToken(
new AccessTokenStore(
accessToken.getValue(),
internalSubject.getValue(),
internalPairwiseSubject.getValue())),
internalPairwiseSubject.getValue(),
journeyId)),
configService.getAccessTokenExpiry());
} catch (JsonException e) {
LOG.error("Unable to save access token to Redis");
Expand Down