From e3558834dedf8a772e6b64b32dbdef5a99a729c9 Mon Sep 17 00:00:00 2001 From: avdunn Date: Mon, 30 Dec 2024 16:13:47 -0800 Subject: [PATCH 1/4] Fix null pointer exception --- .../com/microsoft/aad/msal4j/TokenCache.java | 4 +- .../aad/msal4j/TokenRequestExecutor.java | 11 ++- .../com/microsoft/aad/msal4j/CacheTests.java | 70 +++++++++++++++++++ .../com/microsoft/aad/msal4j/TestHelper.java | 36 ++++++++-- 4 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/CacheTests.java diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java index ebc1753c..2c39d67e 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java @@ -477,7 +477,8 @@ private Optional getAccessTokenCacheEntity( return accessTokens.values().stream().filter( accessToken -> - accessToken.homeAccountId.equals(account.homeAccountId()) && + accessToken.homeAccountId != null && + accessToken.homeAccountId.equals(account.homeAccountId()) && environmentAliases.contains(accessToken.environment) && accessToken.realm.equals(authority.tenant()) && accessToken.clientId.equals(clientId) && @@ -552,6 +553,7 @@ private Optional getRefreshTokenCacheEntity( return refreshTokens.values().stream().filter( refreshToken -> + refreshToken.homeAccountId != null && refreshToken.homeAccountId.equals(account.homeAccountId()) && environmentAliases.contains(refreshToken.environment) && refreshToken.clientId.equals(clientId) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java index 815ebfcc..981b9b18 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.net.MalformedURLException; +import java.nio.charset.StandardCharsets; import java.util.*; @Getter(AccessLevel.PACKAGE) @@ -118,9 +119,15 @@ private AuthenticationResult createAuthenticationResultFromOauthHttpResponse( } AccountCacheEntity accountCacheEntity = null; + if (!StringHelper.isNullOrBlank(tokens.getIDTokenString())) { + String idTokenJson; + try { + idTokenJson = new String(Base64.getDecoder().decode(tokens.getIDTokenString().split("\\.")[1]), StandardCharsets.UTF_8); + } catch (ArrayIndexOutOfBoundsException e) { + throw new MsalServiceException("Error parsing ID token, missing payload section. Ensure that the ID token is following the JWT format.", + AuthenticationErrorCode.INVALID_JWT); + } - if (tokens.getIDToken() != null) { - String idTokenJson = tokens.getIDToken().getParsedParts()[1].decodeToString(); IdToken idToken = JsonHelper.convertJsonToObject(idTokenJson, IdToken.class); AuthorityType type = msalRequest.application().authenticationAuthority.authorityType; diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/CacheTests.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/CacheTests.java new file mode 100644 index 00000000..856b10d6 --- /dev/null +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/CacheTests.java @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.HashMap; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class CacheTests { + + @Test + void cacheLookup_MixAccountBasedAndAssertionBasedSilentFlows() throws Exception { + DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + + ConfidentialClientApplication cca = + ConfidentialClientApplication.builder("clientId", ClientCredentialFactory.createFromSecret("password")) + .authority("https://login.microsoftonline.com/tenant/") + .instanceDiscovery(false) + .validateAuthority(false) + .httpClient(httpClientMock) + .build(); + + HashMap responseParameters = new HashMap<>(); + //Acquire a token with no ID token/account associated with it + responseParameters.put("access_token", "accessTokenNoAccount"); + + ClientCredentialParameters clientCredentialParameters = ClientCredentialParameters.builder(Collections.singleton("someScopes")).build(); + when(httpClientMock.send(any(HttpRequest.class))).thenReturn(TestHelper.expectedResponse(200, TestHelper.getSuccessfulTokenResponse(responseParameters))); + IAuthenticationResult resultNoAccount = cca.acquireToken(clientCredentialParameters).get(); + + //Ensure there is one token in the cache, and the result had no account + assertEquals(1, cca.tokenCache.accessTokens.size()); + assertNull(resultNoAccount.account()); + verify(httpClientMock, times(1)).send(any()); + + //Acquire a second token, this time with an ID token/account + responseParameters.put("access_token", "accessTokenWithAccount"); + responseParameters.put("id_token", TestHelper.createIdToken(new HashMap<>())); + + when(httpClientMock.send(any(HttpRequest.class))).thenReturn(TestHelper.expectedResponse(200, TestHelper.getSuccessfulTokenResponse(responseParameters))); + OnBehalfOfParameters onBehalfOfParametersarameters = OnBehalfOfParameters.builder(Collections.singleton("someOtherScopes"), new UserAssertion(TestHelper.signedAssertion)).build(); + IAuthenticationResult resultWithAccount = cca.acquireToken(onBehalfOfParametersarameters).get(); + + //Ensure there are now two tokens in the cache, and the result has an account + assertEquals(2, cca.tokenCache.accessTokens.size()); + assertNull(resultNoAccount.account()); + verify(httpClientMock, times(2)).send(any()); + + //Make two silent calls, one with the account and one without + SilentParameters silentParametersNoAccount = SilentParameters.builder(Collections.singleton("someScopes")).build(); + SilentParameters silentParametersWithAccount = SilentParameters.builder(Collections.singleton("someOtherScopes"), resultWithAccount.account()).build(); + + resultNoAccount = cca.acquireTokenSilently(silentParametersNoAccount).get(); + resultWithAccount = cca.acquireTokenSilently(silentParametersWithAccount).get(); + + //Ensure the correct access tokens were returned from each silent call + assertEquals("accessTokenNoAccount", resultNoAccount.accessToken()); + assertEquals("accessTokenWithAccount", resultWithAccount.accessToken()); + } +} diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestHelper.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestHelper.java index 70774a09..770f60ba 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestHelper.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestHelper.java @@ -7,13 +7,13 @@ import com.nimbusds.jose.crypto.RSASSASigner; import com.nimbusds.jose.jwk.RSAKey; import com.nimbusds.jose.jwk.gen.RSAKeyGenerator; - import java.io.File; import java.io.FileWriter; import java.io.IOException; import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.Base64; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -29,6 +29,17 @@ class TestHelper { "\"expires_on\": %d ,\"expires_in\": %d," + "\"token_type\":\"Bearer\"}"; + static final String idTokenFormat = "{\"aud\": \"%s\"," + + "\"iss\": \"%s\"," + + "\"iat\": 1455833828," + "\"nbf\": 1455833828," + "\"exp\": 1455837728," + + "\"ipaddr\": \"131.107.159.117\"," + + "\"name\": \"%s\"," + + "\"oid\": \"%s\"," + + "\"preferred_username\": \"%s\"," + + "\"sub\": \"%s\"," + + "\"tid\": \"%s\"," + + "\"ver\": \"2.0\"}"; + static String readResource(Class classInstance, String resource) { try { return new String(Files.readAllBytes(Paths.get(classInstance.getResource(resource).toURI()))); @@ -76,10 +87,10 @@ static String getSuccessfulTokenResponse(HashMap responseValues) return String.format(successfulResponseFormat, responseValues.getOrDefault("access_token", "access_token"), - responseValues.getOrDefault("id_token", "id_token"), + responseValues.getOrDefault("id_token", ""), responseValues.getOrDefault("refresh_token", "refresh_token"), responseValues.getOrDefault("client_id", "client_id"), - responseValues.getOrDefault("client_info", "client_info"), + responseValues.getOrDefault("client_info", "eyJ1aWQiOiI1OTdmODZjZC0xM2YzLTQ0YzAtYmVjZS1hMWU3N2JhNDMyMjgiLCJ1dGlkIjoiZjY0NWFkOTItZTM4ZC00ZDFhLWI1MTAtZDFiMDlhNzRhOGNhIn0"), expiresOn, expiresIn ); @@ -97,4 +108,21 @@ static HttpResponse expectedResponse(int statusCode, String response) { return httpResponse; } -} + + //Maps various values to the idTokenFormat string + static String createIdToken(HashMap idTokenValues) { + String tokenValues = String.format(idTokenFormat, + idTokenValues.getOrDefault("aud", "e854a4a7-6c34-449c-b237-fc7a28093d84"), + idTokenValues.getOrDefault("iss", "https://login.microsoftonline.com/6c3d51dd-f0e5-4959-b4ea-a80c4e36fe5e/v2.0/"), + idTokenValues.getOrDefault("name", "name"), + idTokenValues.getOrDefault("oid", "oid"), + idTokenValues.getOrDefault("preferred_username", "preferred_username"), + idTokenValues.getOrDefault("sub", "K4_SGGxKqW1SxUAmhg6C1F6VPiFzcx-Qd80ehIEdFus"), + idTokenValues.getOrDefault("client_info", "eyJ1aWQiOiI1OTdmODZjZC0xM2YzLTQ0YzAtYmVjZS1hMWU3N2JhNDMyMjgiLCJ1dGlkIjoiZjY0NWFkOTItZTM4ZC00ZDFhLWI1MTAtZDFiMDlhNzRhOGNhIn0") + ); + + String encodedTokenValues = Base64.getUrlEncoder().encodeToString(tokenValues.getBytes()); + + return String.format("someheader.%s.somesignature", encodedTokenValues); + } +} \ No newline at end of file From c81e599e9fa3a302f47413b9bf76e8e384d66fed Mon Sep 17 00:00:00 2001 From: avdunn Date: Tue, 14 Jan 2025 16:02:48 -0800 Subject: [PATCH 2/4] Update lombok-maven-plugin --- msal4j-sdk/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal4j-sdk/pom.xml b/msal4j-sdk/pom.xml index 9dd0d24e..a19de30c 100644 --- a/msal4j-sdk/pom.xml +++ b/msal4j-sdk/pom.xml @@ -207,7 +207,7 @@ org.projectlombok lombok-maven-plugin - 1.18.2.0 + 1.18.20.0 From 240655b20397bb9b1a04016eb6ae642a588e8d1b Mon Sep 17 00:00:00 2001 From: avdunn Date: Wed, 15 Jan 2025 10:40:17 -0800 Subject: [PATCH 3/4] Update more plugins --- msal4j-sdk/pom.xml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/msal4j-sdk/pom.xml b/msal4j-sdk/pom.xml index a19de30c..723c55d9 100644 --- a/msal4j-sdk/pom.xml +++ b/msal4j-sdk/pom.xml @@ -57,7 +57,7 @@ org.projectlombok lombok - 1.18.6 + 1.18.36 provided @@ -208,6 +208,13 @@ org.projectlombok lombok-maven-plugin 1.18.20.0 + + + org.projectlombok + lombok + 1.18.36 + + @@ -323,7 +330,7 @@ biz.aQute.bnd bnd-maven-plugin - 4.3.1 + 5.2.0 From 85d372c4986973e1172ca5385e05ccc43e385a20 Mon Sep 17 00:00:00 2001 From: avdunn Date: Wed, 15 Jan 2025 12:35:14 -0800 Subject: [PATCH 4/4] Update plugins in broker package --- msal4j-brokers/pom.xml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/msal4j-brokers/pom.xml b/msal4j-brokers/pom.xml index b1e16c7f..df0570e0 100644 --- a/msal4j-brokers/pom.xml +++ b/msal4j-brokers/pom.xml @@ -44,7 +44,7 @@ org.projectlombok lombok - 1.18.6 + 1.18.36 provided @@ -126,7 +126,14 @@ org.projectlombok lombok-maven-plugin - 1.18.2.0 + 1.18.20.0 + + + org.projectlombok + lombok + 1.18.36 + +