From 058956de8cb44775a2fe7a3f90a16b4dee01afba Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Tue, 16 Apr 2024 09:26:12 -0700 Subject: [PATCH 01/17] Refactor silent flow and refresh logic to reduce complexity --- .../msal4j/AcquireTokenSilentSupplier.java | 117 +++++++++++------- 1 file changed, 72 insertions(+), 45 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java index 91e76821..73adab73 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java @@ -13,6 +13,8 @@ class AcquireTokenSilentSupplier extends AuthenticationResultSupplier { private SilentRequest silentRequest; + private boolean shouldRefresh; + private boolean afterRefreshOn; AcquireTokenSilentSupplier(AbstractApplicationBase clientApplication, SilentRequest silentRequest) { super(clientApplication, silentRequest); @@ -53,28 +55,10 @@ AuthenticationResult execute() throws Exception { clientApplication.serviceBundle().getServerSideTelemetry().incrementSilentSuccessfulCount(); } - //Determine if the current token needs to be refreshed according to the refresh_in value - long currTimeStampSec = new Date().getTime() / 1000; - boolean afterRefreshOn = res.refreshOn() != null && res.refreshOn() > 0 && - res.refreshOn() < currTimeStampSec && res.expiresOn() >= currTimeStampSec; - - if (silentRequest.parameters().forceRefresh() || afterRefreshOn || StringHelper.isBlank(res.accessToken())) { - - //As of version 3 of the telemetry schema, there is a field for collecting data about why a token was refreshed, - // so here we set the telemetry value based on the cause of the refresh - if (silentRequest.parameters().forceRefresh()) { - clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( - CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue); - } else if (afterRefreshOn) { - clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( - CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue); - } else if (res.expiresOn() < currTimeStampSec) { - clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( - CacheTelemetry.REFRESH_ACCESS_TOKEN_EXPIRED.telemetryValue); - } else if (StringHelper.isBlank(res.accessToken())) { - clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( - CacheTelemetry.REFRESH_NO_ACCESS_TOKEN.telemetryValue); - } + shouldRefresh = shouldRefresh(silentRequest.parameters(), res); + + if (shouldRefresh || afterRefreshOn) { + setRefreshTelemetry(res); if (!StringHelper.isBlank(res.refreshToken())) { //There are certain scenarios where the cached authority may differ from the client app's authority, @@ -84,29 +68,7 @@ AuthenticationResult execute() throws Exception { requestAuthority = Authority.createAuthority(new URL(requestAuthority.authority().replace(requestAuthority.host(), res.account().environment()))); } - - RefreshTokenRequest refreshTokenRequest = new RefreshTokenRequest( - RefreshTokenParameters.builder(silentRequest.parameters().scopes(), res.refreshToken()).build(), - silentRequest.application(), - silentRequest.requestContext(), - silentRequest); - - AcquireTokenByAuthorizationGrantSupplier acquireTokenByAuthorisationGrantSupplier = - new AcquireTokenByAuthorizationGrantSupplier(clientApplication, refreshTokenRequest, requestAuthority); - - try { - res = acquireTokenByAuthorisationGrantSupplier.execute(); - - res.metadata().tokenSource(TokenSource.IDENTITY_PROVIDER); - - log.info("Access token refreshed successfully."); - } catch (MsalServiceException ex) { - //If the token refresh attempt threw a MsalServiceException but the refresh attempt was done - // only because of refreshOn, then simply return the existing cached token - if (afterRefreshOn && !(silentRequest.parameters().forceRefresh() || StringHelper.isBlank(res.accessToken()))) { - return res; - } else throw ex; - } + res = makeRefreshRequest(res, requestAuthority); } else { res = null; } @@ -120,4 +82,69 @@ AuthenticationResult execute() throws Exception { return res; } + + private AuthenticationResult makeRefreshRequest(AuthenticationResult cachedResult, Authority requestAuthority) throws Exception { + RefreshTokenRequest refreshTokenRequest = new RefreshTokenRequest( + RefreshTokenParameters.builder(silentRequest.parameters().scopes(), cachedResult.refreshToken()).build(), + silentRequest.application(), + silentRequest.requestContext(), + silentRequest); + + AcquireTokenByAuthorizationGrantSupplier acquireTokenByAuthorisationGrantSupplier = + new AcquireTokenByAuthorizationGrantSupplier(clientApplication, refreshTokenRequest, requestAuthority); + + try { + AuthenticationResult refreshedResult = acquireTokenByAuthorisationGrantSupplier.execute(); + + refreshedResult.metadata().tokenSource(TokenSource.IDENTITY_PROVIDER); + + log.info("Access token refreshed successfully."); + return refreshedResult; + } catch (MsalServiceException ex) { + //If the token refresh attempt threw a MsalServiceException but the refresh attempt was done + // only because of refreshOn, then simply return the existing cached token + if (!afterRefreshOn && (silentRequest.parameters().forceRefresh() || StringHelper.isBlank(cachedResult.accessToken()))) { + throw ex; + } + return cachedResult; + } + } + + private void setRefreshTelemetry(AuthenticationResult cachedResult) { + //As of version 3 of the telemetry schema, there is a field for collecting data about why a token was refreshed, + // so here we set the telemetry value based on the cause of the refresh + if (silentRequest.parameters().forceRefresh()) { + clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( + CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue); + } else if (afterRefreshOn) { + clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( + CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue); + } else if (cachedResult.expiresOn() < new Date().getTime() / 1000) { + clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( + CacheTelemetry.REFRESH_ACCESS_TOKEN_EXPIRED.telemetryValue); + } else if (StringHelper.isBlank(cachedResult.accessToken())) { + clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( + CacheTelemetry.REFRESH_NO_ACCESS_TOKEN.telemetryValue); + } + } + + //Handles any logic to determine if a token should be refreshed, based on the request parameters and the status of cached tokens + private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult cachedResult) { + + //If there is a refresh token but no access token, we should use the refresh token to get the access token + if (StringHelper.isBlank(cachedResult.accessToken()) && !StringHelper.isBlank(cachedResult.refreshToken())) { + return true; + } + + //Certain long-lived tokens will have a 'refresh on' time that indicates a refresh should be attempted long before the token would expire + long currTimeStampSec = new Date().getTime() / 1000; + if (cachedResult.refreshOn() != null && cachedResult.refreshOn() > 0 && + cachedResult.refreshOn() < currTimeStampSec && cachedResult.expiresOn() >= currTimeStampSec){ + afterRefreshOn = true; + return true; + } + + //Finally, simply return the value of the request's forceRefresh parameter + return parameters.forceRefresh(); + } } \ No newline at end of file From f038470001e835a8e5b55a5051a6981780012222 Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Tue, 16 Apr 2024 09:26:31 -0700 Subject: [PATCH 02/17] Add claims parameter as a reason to refresh --- .../com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java index 73adab73..24ff355e 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java @@ -136,6 +136,11 @@ private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult return true; } + //If the request contains claims then the token should be refreshed, to ensure that the returned token has the correct claims + if (parameters.claims() != null) { + return true; + } + //Certain long-lived tokens will have a 'refresh on' time that indicates a refresh should be attempted long before the token would expire long currTimeStampSec = new Date().getTime() / 1000; if (cachedResult.refreshOn() != null && cachedResult.refreshOn() > 0 && From 54e9bb0de4e949be37e32fa5848b4a3aac7ed570 Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Thu, 2 May 2024 11:41:06 -0700 Subject: [PATCH 03/17] Change cloudshell request method from post to get --- .../aad/msal4j/AbstractManagedIdentitySource.java | 7 +++---- .../aad/msal4j/CloudShellManagedIdentitySource.java | 6 ++++-- .../com/microsoft/aad/msal4j/ManagedIdentityTests.java | 5 +++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java index a6160cf2..78e6bd34 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java @@ -49,12 +49,11 @@ public ManagedIdentityResponse getManagedIdentityResponse( IHttpResponse response; try { - - HttpRequest httpRequest = managedIdentityRequest.method.equals(HttpMethod.GET) ? - new HttpRequest(HttpMethod.GET, + HttpRequest httpRequest = StringHelper.isNullOrBlank(managedIdentityRequest.getBodyAsString()) ? + new HttpRequest(managedIdentityRequest.method, managedIdentityRequest.computeURI().toString(), managedIdentityRequest.headers) : - new HttpRequest(HttpMethod.POST, + new HttpRequest(managedIdentityRequest.method, managedIdentityRequest.computeURI().toString(), managedIdentityRequest.headers, managedIdentityRequest.getBodyAsString()); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CloudShellManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CloudShellManagedIdentitySource.java index 6e337d6c..f7a10008 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CloudShellManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CloudShellManagedIdentitySource.java @@ -20,15 +20,17 @@ class CloudShellManagedIdentitySource extends AbstractManagedIdentitySource{ @Override public void createManagedIdentityRequest(String resource) { managedIdentityRequest.baseEndpoint = msiEndpoint; - managedIdentityRequest.method = HttpMethod.POST; + managedIdentityRequest.method = HttpMethod.GET; managedIdentityRequest.headers = new HashMap<>(); managedIdentityRequest.headers.put("ContentType", "application/x-www-form-urlencoded"); managedIdentityRequest.headers.put("Metadata", "true"); - managedIdentityRequest.headers.put("resource", resource); managedIdentityRequest.bodyParameters = new HashMap<>(); managedIdentityRequest.bodyParameters.put("resource", Collections.singletonList(resource)); + + managedIdentityRequest.queryParameters = new HashMap<>(); + managedIdentityRequest.queryParameters.put("resource", Collections.singletonList(resource)); } private CloudShellManagedIdentitySource(MsalRequest msalRequest, ServiceBundle serviceBundle, URI msiEndpoint) diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java index 47f30e90..03a228c3 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java @@ -86,10 +86,11 @@ private HttpRequest expectedRequest(ManagedIdentitySourceType source, String res headers.put("ContentType", "application/x-www-form-urlencoded"); headers.put("Metadata", "true"); - headers.put("resource", resource); bodyParameters.put("resource", Collections.singletonList(resource)); - return new HttpRequest(HttpMethod.POST, computeUri(endpoint, queryParameters), headers, URLUtils.serializeParameters(bodyParameters)); + + queryParameters.put("resource", Collections.singletonList(resource)); + return new HttpRequest(HttpMethod.GET, computeUri(endpoint, queryParameters), headers, URLUtils.serializeParameters(bodyParameters)); } case IMDS: { endpoint = IMDS_ENDPOINT; From 8f7457bc9db9f5c58c85f24e3990c2e289d39c60 Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Fri, 3 May 2024 11:59:07 -0700 Subject: [PATCH 04/17] Address PR comments --- .../aad/msal4j/AcquireTokenSilentSupplier.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java index 24ff355e..e1a78770 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java @@ -131,12 +131,13 @@ private void setRefreshTelemetry(AuthenticationResult cachedResult) { //Handles any logic to determine if a token should be refreshed, based on the request parameters and the status of cached tokens private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult cachedResult) { - //If there is a refresh token but no access token, we should use the refresh token to get the access token - if (StringHelper.isBlank(cachedResult.accessToken()) && !StringHelper.isBlank(cachedResult.refreshToken())) { + //If forceRefresh is true, no reason to check any other option + if (parameters.forceRefresh()) { return true; } //If the request contains claims then the token should be refreshed, to ensure that the returned token has the correct claims + // Note: these are the types of claims found in (for example) a claims challenge, and do not include client capabilities if (parameters.claims() != null) { return true; } @@ -149,7 +150,11 @@ private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult return true; } - //Finally, simply return the value of the request's forceRefresh parameter - return parameters.forceRefresh(); + //If there is a refresh token but no access token, we should use the refresh token to get the access token + if (StringHelper.isBlank(cachedResult.accessToken()) && !StringHelper.isBlank(cachedResult.refreshToken())) { + return true; + } + + return false; } } \ No newline at end of file From f012b33b361a149ef48f4c92ec31cf52ccea4ef4 Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Sun, 5 May 2024 15:38:14 -0700 Subject: [PATCH 05/17] Address PR comments --- .../msal4j/AcquireTokenSilentSupplier.java | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java index e1a78770..0e69cf47 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java @@ -58,8 +58,6 @@ AuthenticationResult execute() throws Exception { shouldRefresh = shouldRefresh(silentRequest.parameters(), res); if (shouldRefresh || afterRefreshOn) { - setRefreshTelemetry(res); - if (!StringHelper.isBlank(res.refreshToken())) { //There are certain scenarios where the cached authority may differ from the client app's authority, // such as when a request is instance aware. Unless overridden by SilentParameters.authorityUrl, the @@ -110,51 +108,47 @@ private AuthenticationResult makeRefreshRequest(AuthenticationResult cachedResul } } - private void setRefreshTelemetry(AuthenticationResult cachedResult) { - //As of version 3 of the telemetry schema, there is a field for collecting data about why a token was refreshed, - // so here we set the telemetry value based on the cause of the refresh - if (silentRequest.parameters().forceRefresh()) { - clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( - CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue); - } else if (afterRefreshOn) { - clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( - CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue); - } else if (cachedResult.expiresOn() < new Date().getTime() / 1000) { - clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( - CacheTelemetry.REFRESH_ACCESS_TOKEN_EXPIRED.telemetryValue); - } else if (StringHelper.isBlank(cachedResult.accessToken())) { - clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo( - CacheTelemetry.REFRESH_NO_ACCESS_TOKEN.telemetryValue); - } - } - //Handles any logic to determine if a token should be refreshed, based on the request parameters and the status of cached tokens private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult cachedResult) { //If forceRefresh is true, no reason to check any other option if (parameters.forceRefresh()) { + setCacheTelemetry(CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue); return true; } //If the request contains claims then the token should be refreshed, to ensure that the returned token has the correct claims // Note: these are the types of claims found in (for example) a claims challenge, and do not include client capabilities if (parameters.claims() != null) { + setCacheTelemetry(CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue); return true; } - //Certain long-lived tokens will have a 'refresh on' time that indicates a refresh should be attempted long before the token would expire long currTimeStampSec = new Date().getTime() / 1000; + + if (cachedResult.expiresOn() < currTimeStampSec) { + setCacheTelemetry(CacheTelemetry.REFRESH_ACCESS_TOKEN_EXPIRED.telemetryValue); + return true; + } + + //Certain long-lived tokens will have a 'refresh on' time that indicates a refresh should be attempted long before the token would expire if (cachedResult.refreshOn() != null && cachedResult.refreshOn() > 0 && cachedResult.refreshOn() < currTimeStampSec && cachedResult.expiresOn() >= currTimeStampSec){ + setCacheTelemetry(CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue); afterRefreshOn = true; return true; } //If there is a refresh token but no access token, we should use the refresh token to get the access token if (StringHelper.isBlank(cachedResult.accessToken()) && !StringHelper.isBlank(cachedResult.refreshToken())) { + setCacheTelemetry(CacheTelemetry.REFRESH_NO_ACCESS_TOKEN.telemetryValue); return true; } return false; } + + private void setCacheTelemetry(int cacheInfoValue){ + clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo(cacheInfoValue); + } } \ No newline at end of file From fbca430b05574bed24e1d8a54e9658d8df0b7687 Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Sun, 5 May 2024 15:50:24 -0700 Subject: [PATCH 06/17] Add a test for claims triggering a refresh --- .../AcquireTokenSilentIT.java | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/AcquireTokenSilentIT.java b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/AcquireTokenSilentIT.java index cb0b2f3b..38aeec13 100644 --- a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/AcquireTokenSilentIT.java +++ b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/AcquireTokenSilentIT.java @@ -193,7 +193,7 @@ void acquireTokenSilent_ConfidentialClient_acquireTokenSilent(String environment } @Test - public void acquireTokenSilent_ConfidentialClient_acquireTokenSilentDifferentScopeThrowsException() + void acquireTokenSilent_ConfidentialClient_acquireTokenSilentDifferentScopeThrowsException() throws Exception { cfg = new Config(AzureEnvironment.AZURE); @@ -344,6 +344,48 @@ void acquireTokenSilent_emptyScopeSet(String environment) throws Exception { assertEquals(result.accessToken(), silentResult.accessToken()); } + @Test + public void acquireTokenSilent_ClaimsForceRefresh() throws Exception { + cfg = new Config(AzureEnvironment.AZURE); + User user = labUserProvider.getDefaultUser(AzureEnvironment.AZURE); + + Set scopes = new HashSet<>(); + PublicClientApplication pca = PublicClientApplication.builder( + user.getAppId()). + authority(cfg.organizationsAuthority()). + build(); + + IAuthenticationResult result = pca.acquireToken(UserNamePasswordParameters. + builder(scopes, + user.getUpn(), + user.getPassword().toCharArray()) + .build()) + .get(); + + assertResultNotNull(result); + + IAuthenticationResult silentResultWithoutClaims = pca.acquireTokenSilently(SilentParameters. + builder(scopes, result.account()) + .build()) + .get(); + + assertResultNotNull(silentResultWithoutClaims); + assertEquals(result.accessToken(), silentResultWithoutClaims.accessToken()); + + //If claims are added to a silent request, it should trigger the refresh flow and return a new token + ClaimsRequest cr = new ClaimsRequest(); + cr.requestClaimInAccessToken("email", null); + + IAuthenticationResult silentResultWithClaims = pca.acquireTokenSilently(SilentParameters. + builder(scopes, result.account()) + .claims(cr) + .build()) + .get(); + + assertResultNotNull(silentResultWithClaims); + assertNotEquals(result.accessToken(), silentResultWithClaims.accessToken()); + } + private IConfidentialClientApplication getConfidentialClientApplications() throws Exception { String clientId = cfg.appProvider.getOboAppId(); String password = cfg.appProvider.getOboAppPassword(); From 858795f79693c7c0ab1419711bade304478de4bd Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Mon, 6 May 2024 16:08:42 -0700 Subject: [PATCH 07/17] Update claims/capabilities format unit tests --- .../com/microsoft/aad/msal4j/ClaimsTest.java | 33 ++++++++----------- .../aad/msal4j/TestConfiguration.java | 10 +++--- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClaimsTest.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClaimsTest.java index 708ea306..791f7a53 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClaimsTest.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClaimsTest.java @@ -12,6 +12,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Set; @TestInstance(TestInstance.Lifecycle.PER_CLASS) class ClaimsTest { @@ -30,30 +31,25 @@ void testClaimsRequest_Format() { cr.requestClaimInIdToken("sub", new RequestedClaimAdditionalInfo(true, "248289761001", null)); cr.requestClaimInIdToken("auth_time", new RequestedClaimAdditionalInfo(false, null, null)); - assertEquals(cr.formatAsJSONString(), TestConfiguration.CLAIMS_REQUEST); + assertEquals(TestConfiguration.CLAIMS_REQUEST, cr.formatAsJSONString()); } @Test void testClaimsRequest_MergeWithClientCapabilitiesAndClaimsChallenge() throws URISyntaxException { - List values = new ArrayList<>(); - values.add("urn:mace:incommon:iap:silver"); - values.add("urn:mace:incommon:iap:bronze"); - ClaimsRequest cr = new ClaimsRequest(); - cr.requestClaimInAccessToken("given_name", new RequestedClaimAdditionalInfo(true, null, null)); - cr.requestClaimInAccessToken("email", null); - cr.requestClaimInIdToken("acr", new RequestedClaimAdditionalInfo(false, null, values)); - cr.requestClaimInIdToken("sub", new RequestedClaimAdditionalInfo(true, "248289761001", null)); - cr.requestClaimInIdToken("auth_time", new RequestedClaimAdditionalInfo(false, null, null)); + cr.requestClaimInAccessToken("nbf", new RequestedClaimAdditionalInfo(true, "1701477303", null)); + + Set capabilities = new HashSet<>(); + capabilities.add("cp1"); PublicClientApplication pca = PublicClientApplication.builder( "client_id"). - clientCapabilities(new HashSet<>(Collections.singletonList("llt"))). + clientCapabilities(capabilities). build(); InteractiveRequestParameters parameters = InteractiveRequestParameters.builder(new URI("http://localhost:8080")) - .claimsChallenge("{\"id_token\":{\"auth_time\":{\"essential\":true}},\"access_token\":{\"auth_time\":{\"essential\":true},\"xms_cc\":{\"values\":[\"abc\"]}}}") + .claimsChallenge(TestConfiguration.CLAIMS_CHALLENGE) .claims(cr) .scopes(Collections.singleton("")) .build(); @@ -65,18 +61,17 @@ void testClaimsRequest_MergeWithClientCapabilitiesAndClaimsChallenge() throws UR String mergedClaimsAndChallenge = JsonHelper.mergeJSONString(claimsChallenge, claimsRequest); String mergedAll = JsonHelper.mergeJSONString(claimsChallenge, mergedClaimsAndCapabilities); - assertEquals(clientCapabilities, TestConfiguration.CLIENT_CAPABILITIES); - assertEquals(claimsChallenge, TestConfiguration.CLAIMS_CHALLENGE); - assertEquals(claimsRequest, TestConfiguration.CLAIMS_REQUEST); - assertEquals(mergedClaimsAndCapabilities, TestConfiguration.MERGED_CLAIMS_AND_CAPABILITIES); - assertEquals(mergedClaimsAndChallenge, TestConfiguration.MERGED_CLAIMS_AND_CHALLENGE); - assertEquals(mergedAll, TestConfiguration.MERGED_CLAIMS_CAPABILITIES_AND_CHALLENGE); + assertEquals(TestConfiguration.CLIENT_CAPABILITIES, clientCapabilities); + assertEquals(TestConfiguration.CLAIMS_CHALLENGE, claimsChallenge); + assertEquals(TestConfiguration.MERGED_CLAIMS_AND_CAPABILITIES, mergedClaimsAndCapabilities); + assertEquals(TestConfiguration.MERGED_CLAIMS_AND_CHALLENGE, mergedClaimsAndChallenge); + assertEquals(TestConfiguration.MERGED_CLAIMS_CAPABILITIES_AND_CHALLENGE, mergedAll); } @Test void testClaimsRequest_StringToClaimsRequest() { ClaimsRequest cr = ClaimsRequest.formatAsClaimsRequest(TestConfiguration.CLAIMS_CHALLENGE); - assertEquals(cr.formatAsJSONString(), TestConfiguration.CLAIMS_CHALLENGE); + assertEquals(TestConfiguration.CLAIMS_CHALLENGE, cr.formatAsJSONString()); } } diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestConfiguration.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestConfiguration.java index f3a4d810..2ddaec54 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestConfiguration.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestConfiguration.java @@ -80,9 +80,9 @@ public final class TestConfiguration { "\"suberror\":\"basic_action\"}"; public final static String CLAIMS_REQUEST = "{\"id_token\":{\"acr\":{\"values\":[\"urn:mace:incommon:iap:silver\",\"urn:mace:incommon:iap:bronze\"]},\"sub\":{\"essential\":true,\"value\":\"248289761001\"},\"auth_time\":{}},\"access_token\":{\"given_name\":{\"essential\":true},\"email\":null}}"; - public final static String CLAIMS_CHALLENGE = "{\"id_token\":{\"auth_time\":{\"essential\":true}},\"access_token\":{\"auth_time\":{\"essential\":true},\"xms_cc\":{\"values\":[\"abc\"]}}}"; - public final static String CLIENT_CAPABILITIES = "{\"access_token\":{\"xms_cc\":{\"values\":[\"llt\"]}}}"; - public final static String MERGED_CLAIMS_AND_CAPABILITIES = "{\"id_token\":{\"acr\":{\"values\":[\"urn:mace:incommon:iap:silver\",\"urn:mace:incommon:iap:bronze\"]},\"sub\":{\"essential\":true,\"value\":\"248289761001\"},\"auth_time\":{}},\"access_token\":{\"given_name\":{\"essential\":true},\"email\":null,\"xms_cc\":{\"values\":[\"llt\"]}}}"; - public final static String MERGED_CLAIMS_AND_CHALLENGE = "{\"id_token\":{\"auth_time\":{\"essential\":true},\"acr\":{\"values\":[\"urn:mace:incommon:iap:silver\",\"urn:mace:incommon:iap:bronze\"]},\"sub\":{\"essential\":true,\"value\":\"248289761001\"}},\"access_token\":{\"auth_time\":{\"essential\":true},\"xms_cc\":{\"values\":[\"abc\"]},\"given_name\":{\"essential\":true},\"email\":null}}"; - public final static String MERGED_CLAIMS_CAPABILITIES_AND_CHALLENGE = "{\"id_token\":{\"auth_time\":{\"essential\":true},\"acr\":{\"values\":[\"urn:mace:incommon:iap:silver\",\"urn:mace:incommon:iap:bronze\"]},\"sub\":{\"essential\":true,\"value\":\"248289761001\"}},\"access_token\":{\"auth_time\":{\"essential\":true},\"xms_cc\":{\"values\":[\"llt\"]},\"given_name\":{\"essential\":true},\"email\":null}}"; + public final static String CLAIMS_CHALLENGE = "{\"access_token\":{\"nbf\":{\"essential\":true,\"value\":\"1701477303\"}}}"; + public final static String CLIENT_CAPABILITIES = "{\"access_token\":{\"xms_cc\":{\"values\":[\"cp1\"]}}}"; + public final static String MERGED_CLAIMS_AND_CAPABILITIES = "{\"access_token\":{\"nbf\":{\"essential\":true,\"value\":\"1701477303\"},\"xms_cc\":{\"values\":[\"cp1\"]}}}"; + public final static String MERGED_CLAIMS_AND_CHALLENGE = "{\"access_token\":{\"nbf\":{\"essential\":true,\"value\":\"1701477303\"}}}"; + public final static String MERGED_CLAIMS_CAPABILITIES_AND_CHALLENGE = "{\"access_token\":{\"nbf\":{\"essential\":true,\"value\":\"1701477303\"},\"xms_cc\":{\"values\":[\"cp1\"]}}}"; } From 2c8efc7102ea1907273f2b5e37b6b8f2ac666c95 Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Mon, 6 May 2024 18:11:37 -0700 Subject: [PATCH 08/17] Address PR comments --- .../msal4j/AcquireTokenSilentSupplier.java | 28 +++++++++++-------- .../com/microsoft/aad/msal4j/TokenCache.java | 2 -- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java index 0e69cf47..261487c6 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java @@ -13,8 +13,7 @@ class AcquireTokenSilentSupplier extends AuthenticationResultSupplier { private SilentRequest silentRequest; - private boolean shouldRefresh; - private boolean afterRefreshOn; + protected static final int ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC = 5 * 60; AcquireTokenSilentSupplier(AbstractApplicationBase clientApplication, SilentRequest silentRequest) { super(clientApplication, silentRequest); @@ -24,6 +23,7 @@ class AcquireTokenSilentSupplier extends AuthenticationResultSupplier { @Override AuthenticationResult execute() throws Exception { + boolean shouldRefresh; Authority requestAuthority = silentRequest.requestAuthority(); if (requestAuthority.authorityType != AuthorityType.B2C) { requestAuthority = @@ -57,7 +57,7 @@ AuthenticationResult execute() throws Exception { shouldRefresh = shouldRefresh(silentRequest.parameters(), res); - if (shouldRefresh || afterRefreshOn) { + if (shouldRefresh || clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo() == CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue) { if (!StringHelper.isBlank(res.refreshToken())) { //There are certain scenarios where the cached authority may differ from the client app's authority, // such as when a request is instance aware. Unless overridden by SilentParameters.authorityUrl, the @@ -100,11 +100,11 @@ private AuthenticationResult makeRefreshRequest(AuthenticationResult cachedResul return refreshedResult; } catch (MsalServiceException ex) { //If the token refresh attempt threw a MsalServiceException but the refresh attempt was done - // only because of refreshOn, then simply return the existing cached token - if (!afterRefreshOn && (silentRequest.parameters().forceRefresh() || StringHelper.isBlank(cachedResult.accessToken()))) { - throw ex; + // only because of refreshOn, then simply return the existing cached token rather than throw an exception + if (clientApplication.serviceBundle().getServerSideTelemetry().getCurrentRequest().cacheInfo() == CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue) { + return cachedResult; } - return cachedResult; + throw ex; } } @@ -114,6 +114,7 @@ private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult //If forceRefresh is true, no reason to check any other option if (parameters.forceRefresh()) { setCacheTelemetry(CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue); + log.debug("Refreshing access token because forceRefresh parameter is true."); return true; } @@ -121,27 +122,32 @@ private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult // Note: these are the types of claims found in (for example) a claims challenge, and do not include client capabilities if (parameters.claims() != null) { setCacheTelemetry(CacheTelemetry.REFRESH_FORCE_REFRESH.telemetryValue); + log.debug("Refreshing access token because the claims parameter is not null."); return true; } long currTimeStampSec = new Date().getTime() / 1000; - if (cachedResult.expiresOn() < currTimeStampSec) { + //If the access token is expired or within 5 minutes of becoming expired, refresh it + if (!StringHelper.isBlank(cachedResult.accessToken()) && cachedResult.expiresOn() < (currTimeStampSec - ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC)) { setCacheTelemetry(CacheTelemetry.REFRESH_ACCESS_TOKEN_EXPIRED.telemetryValue); + log.debug("Refreshing access token because it is expired."); return true; } //Certain long-lived tokens will have a 'refresh on' time that indicates a refresh should be attempted long before the token would expire - if (cachedResult.refreshOn() != null && cachedResult.refreshOn() > 0 && - cachedResult.refreshOn() < currTimeStampSec && cachedResult.expiresOn() >= currTimeStampSec){ + if (!StringHelper.isBlank(cachedResult.accessToken()) && + cachedResult.refreshOn() != null && cachedResult.refreshOn() > 0 && + cachedResult.refreshOn() < currTimeStampSec && cachedResult.expiresOn() >= (currTimeStampSec + ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC)){ setCacheTelemetry(CacheTelemetry.REFRESH_REFRESH_IN.telemetryValue); - afterRefreshOn = true; + log.debug("Attempting to refresh access token because it is after the refreshOn time."); return true; } //If there is a refresh token but no access token, we should use the refresh token to get the access token if (StringHelper.isBlank(cachedResult.accessToken()) && !StringHelper.isBlank(cachedResult.refreshToken())) { setCacheTelemetry(CacheTelemetry.REFRESH_NO_ACCESS_TOKEN.telemetryValue); + log.debug("Refreshing access token because it was missing from the cache."); return true; } 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 a5c50b9a..ebc1753c 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 @@ -474,13 +474,11 @@ private Optional getAccessTokenCacheEntity( Set scopes, String clientId, Set environmentAliases) { - long currTimeStampSec = new Date().getTime() / 1000; return accessTokens.values().stream().filter( accessToken -> accessToken.homeAccountId.equals(account.homeAccountId()) && environmentAliases.contains(accessToken.environment) && - Long.parseLong(accessToken.expiresOn()) > currTimeStampSec + MIN_ACCESS_TOKEN_EXPIRE_IN_SEC && accessToken.realm.equals(authority.tenant()) && accessToken.clientId.equals(clientId) && isMatchingScopes(accessToken, scopes) From 30a9eeeb0c881bc4a3e833503bf9cc1435f23bb5 Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Sun, 14 Jul 2024 08:46:49 -0700 Subject: [PATCH 09/17] Use SHA256 thumbprints in non-ADFS cert flows --- .../ClientCredentialsIT.java | 2 +- .../aad/msal4j/ClientCertificate.java | 15 ++++++++++++++- .../msal4j/ConfidentialClientApplication.java | 19 +++++++++++++++++-- .../com/microsoft/aad/msal4j/JwtHelper.java | 9 +++++++-- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/ClientCredentialsIT.java b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/ClientCredentialsIT.java index 1a5498b4..67a70f66 100644 --- a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/ClientCredentialsIT.java +++ b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/ClientCredentialsIT.java @@ -160,7 +160,7 @@ private ClientAssertion getClientAssertion(String clientId) { clientId, (ClientCertificate) certificate, "https://login.microsoftonline.com/common/oauth2/v2.0/token", - true); + true, false); } private void assertAcquireTokenCommon(String clientId, IClientCredential credential, String authority) throws Exception { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java index 1f73c6d8..2a214a8a 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java @@ -56,6 +56,13 @@ public String publicCertificateHash() .getHash(publicKeyCertificateChain.get(0).getEncoded())); } + public String publicCertificateHashSha1() + throws CertificateEncodingException, NoSuchAlgorithmException { + + return Base64.getEncoder().encodeToString(ClientCertificate + .getHashSha1(publicKeyCertificateChain.get(0).getEncoded())); + } + public List getEncodedPublicKeyCertificateChain() throws CertificateEncodingException { List result = new ArrayList<>(); @@ -119,9 +126,15 @@ static ClientCertificate create(final PrivateKey key, final X509Certificate publ return new ClientCertificate(key, Arrays.asList(publicKeyCertificate)); } - private static byte[] getHash(final byte[] inputBytes) throws NoSuchAlgorithmException { + private static byte[] getHashSha1(final byte[] inputBytes) throws NoSuchAlgorithmException { final MessageDigest md = MessageDigest.getInstance("SHA-1"); md.update(inputBytes); return md.digest(); } + + private static byte[] getHash(final byte[] inputBytes) throws NoSuchAlgorithmException { + final MessageDigest md = MessageDigest.getInstance("SHA-256"); + md.update(inputBytes); + return md.digest(); + } } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java index 0318cd66..8d6ffe26 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java @@ -101,7 +101,11 @@ private void initClientAuthentication(IClientCredential clientCredential) { } else if (clientCredential instanceof ClientCertificate) { this.clientCertAuthentication = true; this.clientCertificate = (ClientCertificate) clientCredential; - clientAuthentication = buildValidClientCertificateAuthority(); + if (Authority.detectAuthorityType(this.authenticationAuthority.canonicalAuthorityUrl()) == AuthorityType.ADFS) { + clientAuthentication = buildValidClientCertificateAuthorityLegacySha1(); + } else { + clientAuthentication = buildValidClientCertificateAuthority(); + } } else if (clientCredential instanceof ClientAssertion) { clientAuthentication = createClientAuthFromClientAssertion((ClientAssertion) clientCredential); } else { @@ -127,7 +131,18 @@ private ClientAuthentication buildValidClientCertificateAuthority() { clientId(), clientCertificate, this.authenticationAuthority.selfSignedJwtAudience(), - sendX5c); + sendX5c, + false); + return createClientAuthFromClientAssertion(clientAssertion); + } + + private ClientAuthentication buildValidClientCertificateAuthorityLegacySha1() { + ClientAssertion clientAssertion = JwtHelper.buildJwt( + clientId(), + clientCertificate, + this.authenticationAuthority.selfSignedJwtAudience(), + sendX5c, + true); return createClientAuthFromClientAssertion(clientAssertion); } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java index e6b5171e..a7c792dc 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java @@ -21,7 +21,8 @@ final class JwtHelper { static ClientAssertion buildJwt(String clientId, final ClientCertificate credential, - final String jwtAudience, boolean sendX5c) throws MsalClientException { + final String jwtAudience, boolean sendX5c, + boolean useLegacySha1) throws MsalClientException { if (StringHelper.isBlank(clientId)) { throw new IllegalArgumentException("clientId is null or empty"); } @@ -55,7 +56,11 @@ static ClientAssertion buildJwt(String clientId, final ClientCertificate credent builder.x509CertChain(certs); } - builder.x509CertThumbprint(new Base64URL(credential.publicCertificateHash())); + if (useLegacySha1) { + builder.x509CertThumbprint(new Base64URL(credential.publicCertificateHashSha1())); + } else { + builder.x509CertSHA256Thumbprint(new Base64URL(credential.publicCertificateHash())); + } jwt = new SignedJWT(builder.build(), claimsSet); final RSASSASigner signer = new RSASSASigner(credential.privateKey()); From cb7c22bb3f1392d8e3a0b973d186793b61695119 Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Tue, 16 Jul 2024 14:16:48 -0700 Subject: [PATCH 10/17] Address PR feedback --- .../java/com/microsoft/aad/msal4j/ClientCertificate.java | 7 ++----- .../aad/msal4j/ConfidentialClientApplication.java | 6 ++++-- .../src/main/java/com/microsoft/aad/msal4j/JwtHelper.java | 5 +++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java index 2a214a8a..81daf118 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java @@ -8,8 +8,6 @@ import java.io.IOException; import java.io.InputStream; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.MessageDigest; @@ -21,7 +19,6 @@ import java.security.cert.CertificateEncodingException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; -import java.security.interfaces.RSAPrivateKey; import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; @@ -53,7 +50,7 @@ public String publicCertificateHash() throws CertificateEncodingException, NoSuchAlgorithmException { return Base64.getEncoder().encodeToString(ClientCertificate - .getHash(publicKeyCertificateChain.get(0).getEncoded())); + .getHashSha256(publicKeyCertificateChain.get(0).getEncoded())); } public String publicCertificateHashSha1() @@ -132,7 +129,7 @@ private static byte[] getHashSha1(final byte[] inputBytes) throws NoSuchAlgorith return md.digest(); } - private static byte[] getHash(final byte[] inputBytes) throws NoSuchAlgorithmException { + private static byte[] getHashSha256(final byte[] inputBytes) throws NoSuchAlgorithmException { final MessageDigest md = MessageDigest.getInstance("SHA-256"); md.update(inputBytes); return md.digest(); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java index 8d6ffe26..c706db37 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java @@ -102,7 +102,7 @@ private void initClientAuthentication(IClientCredential clientCredential) { this.clientCertAuthentication = true; this.clientCertificate = (ClientCertificate) clientCredential; if (Authority.detectAuthorityType(this.authenticationAuthority.canonicalAuthorityUrl()) == AuthorityType.ADFS) { - clientAuthentication = buildValidClientCertificateAuthorityLegacySha1(); + clientAuthentication = buildValidClientCertificateAuthoritySha1(); } else { clientAuthentication = buildValidClientCertificateAuthority(); } @@ -136,7 +136,9 @@ private ClientAuthentication buildValidClientCertificateAuthority() { return createClientAuthFromClientAssertion(clientAssertion); } - private ClientAuthentication buildValidClientCertificateAuthorityLegacySha1() { + //The library originally used SHA-1 for thumbprints as other algorithms were not supported server-side, + // and while support for SHA-256 has been added certain flows still only allow SHA-1 + private ClientAuthentication buildValidClientCertificateAuthoritySha1() { ClientAssertion clientAssertion = JwtHelper.buildJwt( clientId(), clientCertificate, diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java index a7c792dc..7524addb 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java @@ -22,7 +22,7 @@ final class JwtHelper { static ClientAssertion buildJwt(String clientId, final ClientCertificate credential, final String jwtAudience, boolean sendX5c, - boolean useLegacySha1) throws MsalClientException { + boolean useSha1) throws MsalClientException { if (StringHelper.isBlank(clientId)) { throw new IllegalArgumentException("clientId is null or empty"); } @@ -56,7 +56,8 @@ static ClientAssertion buildJwt(String clientId, final ClientCertificate credent builder.x509CertChain(certs); } - if (useLegacySha1) { + //SHA-256 is preferred, however certain flows still require SHA-1 due to what is supported server-side + if (useSha1) { builder.x509CertThumbprint(new Base64URL(credential.publicCertificateHashSha1())); } else { builder.x509CertSHA256Thumbprint(new Base64URL(credential.publicCertificateHash())); From eeee81545ef630d85cfaa75766b5f76c9827f571 Mon Sep 17 00:00:00 2001 From: Avery-Dunn Date: Tue, 16 Jul 2024 15:50:13 -0700 Subject: [PATCH 11/17] Address PR feedback --- .../aad/msal4j/AbstractManagedIdentitySource.java | 9 ++------- .../aad/msal4j/CloudShellManagedIdentitySource.java | 3 --- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java index 09b2de43..2ae83f91 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java @@ -49,14 +49,9 @@ public ManagedIdentityResponse getManagedIdentityResponse( IHttpResponse response; try { - HttpRequest httpRequest = StringHelper.isNullOrBlank(managedIdentityRequest.getBodyAsString()) ? - new HttpRequest(managedIdentityRequest.method, + HttpRequest httpRequest = new HttpRequest(managedIdentityRequest.method, managedIdentityRequest.computeURI().toString(), - managedIdentityRequest.headers) : - new HttpRequest(managedIdentityRequest.method, - managedIdentityRequest.computeURI().toString(), - managedIdentityRequest.headers, - managedIdentityRequest.getBodyAsString()); + managedIdentityRequest.headers); response = serviceBundle.getHttpHelper().executeHttpRequest(httpRequest, managedIdentityRequest.requestContext(), serviceBundle); } catch (URISyntaxException e) { throw new RuntimeException(e); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CloudShellManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CloudShellManagedIdentitySource.java index d1800057..11d75bb3 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CloudShellManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CloudShellManagedIdentitySource.java @@ -26,9 +26,6 @@ public void createManagedIdentityRequest(String resource) { managedIdentityRequest.headers.put("ContentType", "application/x-www-form-urlencoded"); managedIdentityRequest.headers.put("Metadata", "true"); - managedIdentityRequest.bodyParameters = new HashMap<>(); - managedIdentityRequest.bodyParameters.put("resource", Collections.singletonList(resource)); - managedIdentityRequest.queryParameters = new HashMap<>(); managedIdentityRequest.queryParameters.put("resource", Collections.singletonList(resource)); } From 217097d7ee67aeb9de26a85b10604801f3c3063b Mon Sep 17 00:00:00 2001 From: Liv Fischer Date: Tue, 23 Jul 2024 11:45:17 +0200 Subject: [PATCH 12/17] fix: Maven enforcer plugin needs correct versions --- msal4j-persistence-extension/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal4j-persistence-extension/pom.xml b/msal4j-persistence-extension/pom.xml index a18d9173..3a131a18 100644 --- a/msal4j-persistence-extension/pom.xml +++ b/msal4j-persistence-extension/pom.xml @@ -38,7 +38,7 @@ com.microsoft.azure msal4j - 1.15.0 + 1.15.1 From 361ec5fad7ef8b7a8863657d954d1b99862b2218 Mon Sep 17 00:00:00 2001 From: papaya2k Date: Tue, 23 Jul 2024 16:27:39 -0400 Subject: [PATCH 13/17] Reduce logging level of CACHE_MISS Addresses GitHub Issue #833 by reducing the logging level of a CACHE_MISS in the AuthenticationResultSupplier to debug level. --- .../aad/msal4j/AuthenticationResultSupplier.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationResultSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationResultSupplier.java index 20d5f07e..1d8d1a43 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationResultSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationResultSupplier.java @@ -95,10 +95,17 @@ public IAuthenticationResult get() { msalRequest.requestContext().correlationId(), error); - clientApplication.log.warn( - LogHelper.createMessage( - String.format("Execution of %s failed: %s", this.getClass(), ex.getMessage()), - msalRequest.headers().getHeaderCorrelationIdValue())); + String logMessage = LogHelper.createMessage( + String.format("Execution of %s failed: %s", this.getClass(), ex.getMessage()), + msalRequest.headers().getHeaderCorrelationIdValue()); + if (ex instanceof MsalClientException) { + MsalClientException exception = (MsalClientException) ex; + if (exception.errorCode() != null && exception.errorCode().equalsIgnoreCase(AuthenticationErrorCode.CACHE_MISS)) { + clientApplication.log.debug(logMessage); + } + } else { + clientApplication.log.warn(logMessage); + } throw new CompletionException(ex); } From 56632610d28ba8927f4dfa2bf120275cea51c7b4 Mon Sep 17 00:00:00 2001 From: avdunn Date: Thu, 25 Jul 2024 12:29:09 -0700 Subject: [PATCH 14/17] Make ManagedIdentitySourceType enum public --- .../com/microsoft/aad/msal4j/ManagedIdentitySourceType.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentitySourceType.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentitySourceType.java index 14a3b6e0..8356184c 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentitySourceType.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentitySourceType.java @@ -3,7 +3,7 @@ package com.microsoft.aad.msal4j; -enum ManagedIdentitySourceType { +public enum ManagedIdentitySourceType { // Default. NONE, // The source used to acquire token for managed identity is IMDS. From c145539265bc2b7d4336dcbbf260d1cbad474892 Mon Sep 17 00:00:00 2001 From: avdunn Date: Fri, 26 Jul 2024 14:11:49 -0700 Subject: [PATCH 15/17] Version updates for 1.16.2 --- README.md | 6 +++--- changelog.txt | 6 ++++++ msal4j-sdk/README.md | 6 +++--- msal4j-sdk/bnd.bnd | 2 +- msal4j-sdk/pom.xml | 2 +- msal4j-sdk/src/samples/msal-b2c-web-sample/pom.xml | 2 +- msal4j-sdk/src/samples/msal-obo-sample/pom.xml | 2 +- msal4j-sdk/src/samples/msal-web-sample/pom.xml | 2 +- 8 files changed, 17 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 6ed7baad..bc1e7070 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Quick links: The library supports the following Java environments: - Java 8 (or higher) -Current version - 1.16.1 +Current version - 1.16.2 You can find the changes for each version in the [change log](https://github.com/AzureAD/microsoft-authentication-library-for-java/blob/main/msal4j-sdk/changelog.txt). @@ -28,13 +28,13 @@ Find [the latest package in the Maven repository](https://mvnrepository.com/arti com.microsoft.azure msal4j - 1.16.1 + 1.16.2 ``` ### Gradle ```gradle -implementation group: 'com.microsoft.azure', name: 'com.microsoft.aad.msal4j', version: '1.16.1' +implementation group: 'com.microsoft.azure', name: 'com.microsoft.aad.msal4j', version: '1.16.2' ``` ## Usage diff --git a/changelog.txt b/changelog.txt index 32c52352..c41527e8 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,3 +1,9 @@ +Version 1.16.2 +============= +- Use SHA256 thumbprints in non-ADFS cert flows (#840) +- Reduce logging level of cache miss messages (#844) +- Make ManagedIdentitySourceType enum public (#845) + Version 1.16.1 ============= - Add missing refreshOn metadata (#838) diff --git a/msal4j-sdk/README.md b/msal4j-sdk/README.md index ea724d2e..92dead87 100644 --- a/msal4j-sdk/README.md +++ b/msal4j-sdk/README.md @@ -16,7 +16,7 @@ Quick links: The library supports the following Java environments: - Java 8 (or higher) -Current version - 1.16.1 +Current version - 1.16.2 You can find the changes for each version in the [change log](https://github.com/AzureAD/microsoft-authentication-library-for-java/blob/master/changelog.txt). @@ -28,13 +28,13 @@ Find [the latest package in the Maven repository](https://mvnrepository.com/arti com.microsoft.azure msal4j - 1.16.1 + 1.16.2 ``` ### Gradle ```gradle -compile group: 'com.microsoft.azure', name: 'msal4j', version: '1.16.1' +compile group: 'com.microsoft.azure', name: 'msal4j', version: '1.16.2' ``` ## Usage diff --git a/msal4j-sdk/bnd.bnd b/msal4j-sdk/bnd.bnd index 4175441c..b55d229c 100644 --- a/msal4j-sdk/bnd.bnd +++ b/msal4j-sdk/bnd.bnd @@ -1,2 +1,2 @@ -Export-Package: com.microsoft.aad.msal4j;version="1.16.1" +Export-Package: com.microsoft.aad.msal4j;version="1.16.2" Automatic-Module-Name: com.microsoft.aad.msal4j diff --git a/msal4j-sdk/pom.xml b/msal4j-sdk/pom.xml index d0cc1a46..5cb2c7c4 100644 --- a/msal4j-sdk/pom.xml +++ b/msal4j-sdk/pom.xml @@ -3,7 +3,7 @@ 4.0.0 com.microsoft.azure msal4j - 1.16.1 + 1.16.2 jar msal4j diff --git a/msal4j-sdk/src/samples/msal-b2c-web-sample/pom.xml b/msal4j-sdk/src/samples/msal-b2c-web-sample/pom.xml index 26e4723c..6043b80f 100644 --- a/msal4j-sdk/src/samples/msal-b2c-web-sample/pom.xml +++ b/msal4j-sdk/src/samples/msal-b2c-web-sample/pom.xml @@ -23,7 +23,7 @@ com.microsoft.azure msal4j - 1.16.1 + 1.16.2 com.nimbusds diff --git a/msal4j-sdk/src/samples/msal-obo-sample/pom.xml b/msal4j-sdk/src/samples/msal-obo-sample/pom.xml index 2888306b..ec55778d 100644 --- a/msal4j-sdk/src/samples/msal-obo-sample/pom.xml +++ b/msal4j-sdk/src/samples/msal-obo-sample/pom.xml @@ -23,7 +23,7 @@ com.microsoft.azure msal4j - 1.16.1 + 1.16.2 com.nimbusds diff --git a/msal4j-sdk/src/samples/msal-web-sample/pom.xml b/msal4j-sdk/src/samples/msal-web-sample/pom.xml index 641f889a..5e2958ad 100644 --- a/msal4j-sdk/src/samples/msal-web-sample/pom.xml +++ b/msal4j-sdk/src/samples/msal-web-sample/pom.xml @@ -23,7 +23,7 @@ com.microsoft.azure msal4j - 1.16.1 + 1.16.2 com.nimbusds From b581622a59f174345d627f4aac10c52021502260 Mon Sep 17 00:00:00 2001 From: avdunn Date: Tue, 13 Aug 2024 18:30:15 -0700 Subject: [PATCH 16/17] Add tests --- .../aad/msal4j/AcquireTokenSilentlyTest.java | 81 ++++++++++++++++++- .../resources/AAD_cache_data/full_cache.json | 48 +++++++++++ 2 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 msal4j-sdk/src/test/resources/AAD_cache_data/full_cache.json diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/AcquireTokenSilentlyTest.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/AcquireTokenSilentlyTest.java index 5d0265c0..79cf06f9 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/AcquireTokenSilentlyTest.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/AcquireTokenSilentlyTest.java @@ -5,9 +5,17 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; + import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertThrows; - +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import java.io.IOException; +import java.net.URISyntaxException; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.Collections; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -16,6 +24,9 @@ @TestInstance(TestInstance.Lifecycle.PER_CLASS) class AcquireTokenSilentlyTest { + Account basicAccount = new Account("home_account_id", "login.windows.net", "username", null); + String cache = readResource("/AAD_cache_data/full_cache.json"); + @Test void publicAppAcquireTokenSilently_emptyCache_MsalClientException() throws Throwable { @@ -29,7 +40,7 @@ void publicAppAcquireTokenSilently_emptyCache_MsalClientException() throws Throw ExecutionException ex = assertThrows(ExecutionException.class, future::get); - assertTrue(ex.getCause() instanceof MsalClientException); + assertInstanceOf(MsalClientException.class, ex.getCause()); assertTrue(ex.getMessage().contains(AuthenticationErrorMessage.NO_TOKEN_IN_CACHE)); } @@ -45,7 +56,71 @@ void confidentialAppAcquireTokenSilently_emptyCache_MsalClientException() throws ExecutionException ex = assertThrows(ExecutionException.class, future::get); - assertTrue(ex.getCause() instanceof MsalClientException); + assertInstanceOf(MsalClientException.class, ex.getCause()); assertTrue(ex.getMessage().contains(AuthenticationErrorMessage.NO_TOKEN_IN_CACHE)); } + + @Test + void publicAppAcquireTokenSilently_claimsSkipCache() throws Throwable { + + PublicClientApplication application = PublicClientApplication.builder("client_id") + .instanceDiscovery(false) + .authority("https://some.authority.com/realm") + .build(); + + application.tokenCache.deserialize(cache); + + SilentParameters parameters = SilentParameters.builder(Collections.singleton("scopes"), basicAccount).build(); + + IAuthenticationResult result = application.acquireTokenSilently(parameters).get(); + + //Confirm cached dummy token returned from silent request + assertNotNull(result); + assertEquals("token", result.accessToken()); + + ClaimsRequest cr = new ClaimsRequest(); + cr.requestClaimInAccessToken("something", null); + + parameters = SilentParameters.builder(Collections.singleton("scopes"), basicAccount).claims(cr).build(); + CompletableFuture future = application.acquireTokenSilently(parameters); + + //Confirm cached dummy token ignored when claims are part of request + ExecutionException ex = assertThrows(ExecutionException.class, future::get); + assertInstanceOf(MsalInteractionRequiredException.class, ex.getCause()); + } + + @Test + void confidentialAppAcquireTokenSilently_claimsSkipCache() throws Throwable { + + ConfidentialClientApplication application = ConfidentialClientApplication + .builder("client_id", ClientCredentialFactory.createFromSecret(TestConfiguration.AAD_CLIENT_DUMMYSECRET)) + .instanceDiscovery(false) + .authority("https://some.authority.com/realm").build(); + + application.tokenCache.deserialize(cache); + + SilentParameters parameters = SilentParameters.builder(Collections.singleton("scopes"), basicAccount).build(); + + IAuthenticationResult result = application.acquireTokenSilently(parameters).get(); + + assertNotNull(result); + assertEquals("token", result.accessToken()); + + ClaimsRequest cr = new ClaimsRequest(); + cr.requestClaimInAccessToken("something", null); + + parameters = SilentParameters.builder(Collections.singleton("scopes"), basicAccount).claims(cr).build(); + CompletableFuture future = application.acquireTokenSilently(parameters); + + ExecutionException ex = assertThrows(ExecutionException.class, future::get); + assertInstanceOf(MsalInteractionRequiredException.class, ex.getCause()); + } + + String readResource(String resource) { + try { + return new String(Files.readAllBytes(Paths.get(getClass().getResource(resource).toURI()))); + } catch (IOException | URISyntaxException e) { + throw new RuntimeException(e); + } + } } diff --git a/msal4j-sdk/src/test/resources/AAD_cache_data/full_cache.json b/msal4j-sdk/src/test/resources/AAD_cache_data/full_cache.json new file mode 100644 index 00000000..c206cea7 --- /dev/null +++ b/msal4j-sdk/src/test/resources/AAD_cache_data/full_cache.json @@ -0,0 +1,48 @@ +{ + "AccessToken": { + "home_account_id-login.windows.net-accesstoken-client_id-realm-scopes": { + "home_account_id": "home_account_id", + "environment": "login.windows.net", + "client_id": "client_id", + "secret": "token", + "credential_type": "AccessToken", + "realm": "realm", + "target": "scopes", + "cached_at": "9999999999999", + "expires_on": "9999999999999", + "extended_expires_on": "9999999999999" + } + }, + "RefreshToken": { + "home_account_id-login.windows.net-refreshtoken-client_id--": { + "home_account_id": "home_account_id", + "environment": "login.windows.net", + "client_id": "client_id", + "secret": "token", + "credential_type": "RefreshToken" + } + }, + "IdToken": { + "home_account_id-login.windows.net-idtoken-client_id-realm-": { + "home_account_id": "home_account_id", + "environment": "login.windows.net", + "client_id": "client_id", + "secret": "token", + "credential_type": "IdToken", + "realm": "realm" + } + }, + "Account": { + "home_account_id-login.windows.net-realm": { + "home_account_id": "home_account_id", + "environment": "login.windows.net", + "realm": "realm", + "local_account_id": "local_account_id", + "username": "username", + "name": "name", + "client_info": "client_info", + "authority_type": "authority_type" + } + }, + "AppMetadata": {} +} From 51a0101f27256d03e1c82f8c9c29a26cc7b6b730 Mon Sep 17 00:00:00 2001 From: avdunn Date: Mon, 19 Aug 2024 16:24:43 -0700 Subject: [PATCH 17/17] Add object ID option for Managed Identity --- .../msal4j/AbstractManagedIdentitySource.java | 5 ++++- .../AppServiceManagedIdentitySource.java | 13 +++-------- .../com/microsoft/aad/msal4j/Constants.java | 1 + .../aad/msal4j/IMDSManagedIdentitySource.java | 17 ++++---------- .../aad/msal4j/ManagedIdentityClient.java | 14 +++--------- .../aad/msal4j/ManagedIdentityId.java | 17 ++++++++++++++ .../aad/msal4j/ManagedIdentityIdType.java | 3 ++- .../aad/msal4j/ManagedIdentityRequest.java | 22 +++++++++++++++++++ .../ServiceFabricManagedIdentitySource.java | 14 +++--------- .../ManagedIdentityTestDataProvider.java | 9 +++++++- .../aad/msal4j/ManagedIdentityTests.java | 3 +++ 11 files changed, 70 insertions(+), 48 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java index 2ae83f91..8df12efb 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java @@ -15,13 +15,14 @@ //base class for all sources that support managed identity abstract class AbstractManagedIdentitySource { - protected static final String TIMEOUT_ERROR = "[Managed Identity] Authentication unavailable. The request to the managed identity endpoint timed out."; private static final Logger LOG = LoggerFactory.getLogger(AbstractManagedIdentitySource.class); private static final String MANAGED_IDENTITY_NO_RESPONSE_RECEIVED = "[Managed Identity] Authentication unavailable. No response received from the managed identity endpoint."; protected final ManagedIdentityRequest managedIdentityRequest; protected final ServiceBundle serviceBundle; ManagedIdentitySourceType managedIdentitySourceType; + ManagedIdentityIdType idType; + String userAssignedId; @Getter @Setter @@ -40,6 +41,8 @@ public AbstractManagedIdentitySource(MsalRequest msalRequest, ServiceBundle serv this.managedIdentityRequest = (ManagedIdentityRequest) msalRequest; this.managedIdentitySourceType = sourceType; this.serviceBundle = serviceBundle; + this.idType = ((ManagedIdentityApplication) msalRequest.application()).getManagedIdentityId().getIdType(); + this.userAssignedId = ((ManagedIdentityApplication) msalRequest.application()).getManagedIdentityId().getUserAssignedId(); } public ManagedIdentityResponse getManagedIdentityResponse( diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AppServiceManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AppServiceManagedIdentitySource.java index 2b26b109..bb0f3112 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AppServiceManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AppServiceManagedIdentitySource.java @@ -34,16 +34,9 @@ public void createManagedIdentityRequest(String resource) { managedIdentityRequest.queryParameters.put("api-version", Collections.singletonList(APP_SERVICE_MSI_API_VERSION)); managedIdentityRequest.queryParameters.put("resource", Collections.singletonList(resource)); - if (!StringHelper.isNullOrBlank(getManagedIdentityUserAssignedClientId())) - { - LOG.info("[Managed Identity] Adding user assigned client id to the request."); - managedIdentityRequest.queryParameters.put(Constants.MANAGED_IDENTITY_CLIENT_ID, Collections.singletonList(getManagedIdentityUserAssignedClientId())); - } - - if (!StringHelper.isNullOrBlank(getManagedIdentityUserAssignedResourceId())) - { - LOG.info("[Managed Identity] Adding user assigned resource id to the request."); - managedIdentityRequest.queryParameters.put(Constants.MANAGED_IDENTITY_RESOURCE_ID, Collections.singletonList(getManagedIdentityUserAssignedResourceId())); + if (this.idType != null && !StringHelper.isNullOrBlank(this.userAssignedId)) { + LOG.info("[Managed Identity] Adding user assigned ID to the request for App Service Managed Identity."); + managedIdentityRequest.addUserAssignedIdToQuery(this.idType, this.userAssignedId); } } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/Constants.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/Constants.java index aec0e880..07e99345 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/Constants.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/Constants.java @@ -13,6 +13,7 @@ final class Constants { public static final String MANAGED_IDENTITY_CLIENT_ID = "client_id"; public static final String MANAGED_IDENTITY_RESOURCE_ID = "mi_res_id"; + public static final String MANAGED_IDENTITY_OBJECT_ID = "object_id"; public static final String MANAGED_IDENTITY_DEFAULT_TENTANT = "managed_identity"; public static final String IDENTITY_ENDPOINT = "IDENTITY_ENDPOINT"; diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IMDSManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IMDSManagedIdentitySource.java index 0dc2d2b1..c1973ee1 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IMDSManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IMDSManagedIdentitySource.java @@ -34,8 +34,8 @@ class IMDSManagedIdentitySource extends AbstractManagedIdentitySource{ public IMDSManagedIdentitySource(MsalRequest msalRequest, ServiceBundle serviceBundle) { super(msalRequest, serviceBundle, ManagedIdentitySourceType.IMDS); - ManagedIdentityParameters parameters = (ManagedIdentityParameters) msalRequest.requestContext().apiParameters(); IEnvironmentVariables environmentVariables = getEnvironmentVariables(); + if (!StringHelper.isNullOrBlank(environmentVariables.getEnvironmentVariable(Constants.AZURE_POD_IDENTITY_AUTHORITY_HOST))){ LOG.info(String.format("[Managed Identity] Environment variable AZURE_POD_IDENTITY_AUTHORITY_HOST for IMDS returned endpoint: %s", environmentVariables.getEnvironmentVariable(Constants.AZURE_POD_IDENTITY_AUTHORITY_HOST))); try { @@ -77,18 +77,9 @@ public void createManagedIdentityRequest(String resource) { managedIdentityRequest.queryParameters.put("api-version", Collections.singletonList(IMDS_API_VERSION)); managedIdentityRequest.queryParameters.put("resource", Collections.singletonList(resource)); - String clientId = getManagedIdentityUserAssignedClientId(); - String resourceId = getManagedIdentityUserAssignedResourceId(); - if (!StringHelper.isNullOrBlank(clientId)) - { - LOG.info("[Managed Identity] Adding user assigned client id to the request."); - managedIdentityRequest.queryParameters.put(Constants.MANAGED_IDENTITY_CLIENT_ID, Collections.singletonList(clientId)); - } - - if (!StringHelper.isNullOrBlank(resourceId)) - { - LOG.info("[Managed Identity] Adding user assigned resource id to the request."); - managedIdentityRequest.queryParameters.put(Constants.MANAGED_IDENTITY_RESOURCE_ID, Collections.singletonList(resourceId)); + if (this.idType != null && !StringHelper.isNullOrBlank(this.userAssignedId)) { + LOG.info("[Managed Identity] Adding user assigned ID to the request for IMDS Managed Identity."); + managedIdentityRequest.addUserAssignedIdToQuery(this.idType, this.userAssignedId); } } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityClient.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityClient.java index 334abec8..fbcd8386 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityClient.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityClient.java @@ -3,8 +3,6 @@ package com.microsoft.aad.msal4j; -import lombok.AccessLevel; -import lombok.Getter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,8 +29,9 @@ static ManagedIdentitySourceType getManagedIdentitySource() { !StringHelper.isNullOrBlank(environmentVariables.getEnvironmentVariable(Constants.IDENTITY_HEADER))) { if (!StringHelper.isNullOrBlank(environmentVariables.getEnvironmentVariable(Constants.IDENTITY_SERVER_THUMBPRINT))) { managedIdentitySourceType = ManagedIdentitySourceType.SERVICE_FABRIC; - } else - managedIdentitySourceType = ManagedIdentitySourceType.APP_SERVICE; + } else { + managedIdentitySourceType = ManagedIdentitySourceType.APP_SERVICE; + } } else if (!StringHelper.isNullOrBlank(environmentVariables.getEnvironmentVariable(Constants.MSI_ENDPOINT))) { managedIdentitySourceType = ManagedIdentitySourceType.CLOUD_SHELL; } else if (!StringHelper.isNullOrBlank(environmentVariables.getEnvironmentVariable(Constants.IDENTITY_ENDPOINT)) && @@ -54,12 +53,6 @@ static ManagedIdentitySourceType getManagedIdentitySource() { ManagedIdentityIdType identityIdType = managedIdentityApplication.getManagedIdentityId().getIdType(); if (!identityIdType.equals(ManagedIdentityIdType.SYSTEM_ASSIGNED)) { managedIdentitySource.setUserAssignedManagedIdentity(true); - String userAssignedId = managedIdentityApplication.getManagedIdentityId().getUserAssignedId(); - if (identityIdType.equals(ManagedIdentityIdType.CLIENT_ID)) { - managedIdentitySource.setManagedIdentityUserAssignedClientId(userAssignedId); - } else if (identityIdType.equals(ManagedIdentityIdType.RESOURCE_ID)) { - managedIdentitySource.setManagedIdentityUserAssignedResourceId(userAssignedId); - } } } @@ -70,7 +63,6 @@ ManagedIdentityResponse getManagedIdentityResponse(ManagedIdentityParameters par // This method tries to create managed identity source for different sources, if none is created then defaults to IMDS. private static AbstractManagedIdentitySource createManagedIdentitySource(MsalRequest msalRequest, ServiceBundle serviceBundle) { - AbstractManagedIdentitySource managedIdentitySource; if (managedIdentitySourceType == null || managedIdentitySourceType == ManagedIdentitySourceType.NONE) { managedIdentitySourceType = getManagedIdentitySource(); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityId.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityId.java index 20c28728..5f2f18b3 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityId.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityId.java @@ -61,4 +61,21 @@ public static ManagedIdentityId userAssignedResourceId(String resourceId) return new ManagedIdentityId(ManagedIdentityIdType.RESOURCE_ID, resourceId); } + + /** + * Create an instance of ManagedIdentityId for a user assigned managed identity from an object id. + * + * @param objectId Object ID of the user assigned managed identity assigned to azure resource. + * @return Instance of ManagedIdentityId + * @exception NullPointerException Indicates the resourceId param is null or blank + */ + public static ManagedIdentityId userAssignedObjectId(String objectId) + { + if (StringHelper.isNullOrBlank(objectId)) + { + throw new NullPointerException(objectId); + } + + return new ManagedIdentityId(ManagedIdentityIdType.OBJECT_ID, objectId); + } } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityIdType.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityIdType.java index 8f7fbe44..6040f999 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityIdType.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityIdType.java @@ -7,5 +7,6 @@ enum ManagedIdentityIdType { SYSTEM_ASSIGNED, CLIENT_ID, - RESOURCE_ID + RESOURCE_ID, + OBJECT_ID } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityRequest.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityRequest.java index bf77b65c..2076c37b 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityRequest.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityRequest.java @@ -4,16 +4,21 @@ package com.microsoft.aad.msal4j; import com.nimbusds.oauth2.sdk.util.URLUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.util.Collections; import java.util.List; import java.util.Map; class ManagedIdentityRequest extends MsalRequest { + private static final Logger LOG = LoggerFactory.getLogger(ManagedIdentityRequest.class); + URI baseEndpoint; HttpMethod method; @@ -53,4 +58,21 @@ private String appendQueryParametersToBaseEndpoint() { return baseEndpoint.toString() + "?" + queryString; } + + void addUserAssignedIdToQuery(ManagedIdentityIdType idType, String userAssignedId) { + switch (idType) { + case CLIENT_ID: + LOG.info("[Managed Identity] Adding user assigned client id to the request."); + queryParameters.put(Constants.MANAGED_IDENTITY_CLIENT_ID, Collections.singletonList(userAssignedId)); + break; + case RESOURCE_ID: + LOG.info("[Managed Identity] Adding user assigned resource id to the request."); + queryParameters.put(Constants.MANAGED_IDENTITY_RESOURCE_ID, Collections.singletonList(userAssignedId)); + break; + case OBJECT_ID: + LOG.info("[Managed Identity] Adding user assigned object id to the request."); + queryParameters.put(Constants.MANAGED_IDENTITY_OBJECT_ID, Collections.singletonList(userAssignedId)); + break; + } + } } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java index 804eb486..6c20cf9c 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java @@ -20,8 +20,6 @@ class ServiceFabricManagedIdentitySource extends AbstractManagedIdentitySource { private final URI msiEndpoint; private final String identityHeader; - private final ManagedIdentityIdType idType; - private final String userAssignedId; //Service Fabric requires a special check for an environment variable containing a certificate thumbprint used for validating requests. //No other flow need this and an app developer may not be aware of it, so it was decided that for the Service Fabric flow we will simply override @@ -41,12 +39,9 @@ public void createManagedIdentityRequest(String resource) { managedIdentityRequest.queryParameters.put("resource", Collections.singletonList(resource)); managedIdentityRequest.queryParameters.put("api-version", Collections.singletonList(SERVICE_FABRIC_MSI_API_VERSION)); - if (idType == ManagedIdentityIdType.CLIENT_ID) { - LOG.info("[Managed Identity] Adding user assigned client id to the request for Service Fabric Managed Identity."); - managedIdentityRequest.queryParameters.put(Constants.MANAGED_IDENTITY_CLIENT_ID, Collections.singletonList(userAssignedId)); - } else if (idType == ManagedIdentityIdType.RESOURCE_ID) { - LOG.info("[Managed Identity] Adding user assigned resource id to the request for Service Fabric Managed Identity."); - managedIdentityRequest.queryParameters.put(Constants.MANAGED_IDENTITY_RESOURCE_ID, Collections.singletonList(userAssignedId)); + if (this.idType != null && !StringHelper.isNullOrBlank(this.userAssignedId)) { + LOG.info("[Managed Identity] Adding user assigned ID to the request for Service Fabric Managed Identity."); + managedIdentityRequest.addUserAssignedIdToQuery(this.idType, this.userAssignedId); } } @@ -55,9 +50,6 @@ private ServiceFabricManagedIdentitySource(MsalRequest msalRequest, ServiceBundl super(msalRequest, serviceBundle, ManagedIdentitySourceType.SERVICE_FABRIC); this.msiEndpoint = msiEndpoint; this.identityHeader = identityHeader; - - this.idType = ((ManagedIdentityApplication) msalRequest.application()).getManagedIdentityId().getIdType(); - this.userAssignedId = ((ManagedIdentityApplication) msalRequest.application()).getManagedIdentityId().getUserAssignedId(); } @Override diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTestDataProvider.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTestDataProvider.java index 5132bc83..d5aedabf 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTestDataProvider.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTestDataProvider.java @@ -10,6 +10,7 @@ class ManagedIdentityTestDataProvider { private static final String CLIENT_ID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"; private static final String RESOURCE_ID = "/subscriptions/ffa4aaa2-4444-4444-5555-e3ccedd3d046/resourcegroups/UAMI_group/providers/Microsoft.ManagedIdentityClient/userAssignedIdentities/UAMI"; + private static final String OBJECT_ID = "593b2662-5af7-4a90-a9cb-5a9de615b82f"; public static Stream createData() { return Stream.of( @@ -43,14 +44,20 @@ public static Stream createDataUserAssigned() { ManagedIdentityId.userAssignedClientId(CLIENT_ID)), Arguments.of(ManagedIdentitySourceType.APP_SERVICE, ManagedIdentityTests.appServiceEndpoint, ManagedIdentityId.userAssignedResourceId(RESOURCE_ID)), + Arguments.of(ManagedIdentitySourceType.APP_SERVICE, ManagedIdentityTests.appServiceEndpoint, + ManagedIdentityId.userAssignedObjectId(OBJECT_ID)), Arguments.of(ManagedIdentitySourceType.IMDS, null, ManagedIdentityId.userAssignedClientId(CLIENT_ID)), Arguments.of(ManagedIdentitySourceType.IMDS, null, ManagedIdentityId.userAssignedResourceId(RESOURCE_ID)), + Arguments.of(ManagedIdentitySourceType.IMDS, null, + ManagedIdentityId.userAssignedObjectId(OBJECT_ID)), Arguments.of(ManagedIdentitySourceType.SERVICE_FABRIC, ManagedIdentityTests.serviceFabricEndpoint, ManagedIdentityId.userAssignedResourceId(CLIENT_ID)), Arguments.of(ManagedIdentitySourceType.SERVICE_FABRIC, ManagedIdentityTests.serviceFabricEndpoint, - ManagedIdentityId.userAssignedResourceId(RESOURCE_ID))); + ManagedIdentityId.userAssignedResourceId(RESOURCE_ID)), + Arguments.of(ManagedIdentitySourceType.SERVICE_FABRIC, ManagedIdentityTests.serviceFabricEndpoint, + ManagedIdentityId.userAssignedObjectId(OBJECT_ID))); } public static Stream createDataUserAssignedNotSupported() { diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java index 08fc7b61..3b99085f 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java @@ -124,6 +124,9 @@ private HttpRequest expectedRequest(ManagedIdentitySourceType source, String res case RESOURCE_ID: queryParameters.put("mi_res_id", Collections.singletonList(id.getUserAssignedId())); break; + case OBJECT_ID: + queryParameters.put("object_id", Collections.singletonList(id.getUserAssignedId())); + break; } return new HttpRequest(HttpMethod.GET, computeUri(endpoint, queryParameters), headers);