From 510dc279aae807d472409eed82f89ff6e8e71659 Mon Sep 17 00:00:00 2001 From: Jianghao Lu Date: Wed, 4 Mar 2020 17:15:57 -0800 Subject: [PATCH 1/5] Add configuration for refresh token before expiry --- .../azure/identity/CredentialBuilderBase.java | 13 ++++++++++ .../implementation/IdentityClient.java | 26 +++++++++---------- .../implementation/IdentityClientOptions.java | 19 ++++++++++++++ .../identity/implementation/MsalToken.java | 7 ++--- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java index 73da65dea943..50bc9eb1480a 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java @@ -68,4 +68,17 @@ public T httpPipeline(HttpPipeline httpPipeline) { this.identityClientOptions.setHttpPipeline(httpPipeline); return (T) this; } + + /** + * Sets the duration before the actual expiry of a token to refresh it. + * This is useful when network is congested and a request containing the + * token takes longer than normal to get to the server. + * + * @param refreshBeforeExpiry the duration before the actual expiry of a token to refresh it + */ + @SuppressWarnings("unchecked") + public T setRefreshBeforeExpiry(Duration refreshBeforeExpiry) { + this.identityClientOptions.setRefreshBeforeExpiry(refreshBeforeExpiry); + return (T) this; + } } diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java index 606fffbacae8..af7d26d8b48a 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java @@ -38,7 +38,6 @@ import java.nio.file.Paths; import java.time.Duration; import java.time.OffsetDateTime; -import java.time.ZoneOffset; import java.util.HashSet; import java.util.Locale; import java.util.Random; @@ -121,8 +120,7 @@ public Mono authenticateWithClientSecret(String clientSecret, Token return Mono.fromFuture(application.acquireToken( ClientCredentialParameters.builder(new HashSet<>(request.getScopes())) .build())) - .map(ar -> new AccessToken(ar.accessToken(), OffsetDateTime.ofInstant(ar.expiresOnDate().toInstant(), - ZoneOffset.UTC))); + .map(ar -> new MsalToken(ar, options)); } catch (MalformedURLException e) { return Mono.error(e); } @@ -153,8 +151,7 @@ public Mono authenticateWithPfxCertificate(String pfxCertificatePat return applicationBuilder.build(); }).flatMap(application -> Mono.fromFuture(application.acquireToken( ClientCredentialParameters.builder(new HashSet<>(request.getScopes())).build()))) - .map(ar -> new AccessToken(ar.accessToken(), OffsetDateTime.ofInstant(ar.expiresOnDate().toInstant(), - ZoneOffset.UTC))); + .map(ar -> new MsalToken(ar, options)); } /** @@ -183,8 +180,7 @@ public Mono authenticateWithPemCertificate(String pemCertificatePat return Mono.fromFuture(application.acquireToken( ClientCredentialParameters.builder(new HashSet<>(request.getScopes())) .build())) - .map(ar -> new AccessToken(ar.accessToken(), OffsetDateTime.ofInstant(ar.expiresOnDate().toInstant(), - ZoneOffset.UTC))); + .map(ar -> new MsalToken(ar, options)); } catch (IOException e) { return Mono.error(e); } @@ -203,7 +199,7 @@ public Mono authenticateWithUsernamePassword(TokenRequestContext requ return Mono.fromFuture(publicClientApplication.acquireToken( UserNamePasswordParameters.builder(new HashSet<>(request.getScopes()), username, password.toCharArray()) .build())) - .map(MsalToken::new); + .map(ar -> new MsalToken(ar, options)); } /** @@ -221,7 +217,7 @@ public Mono authenticateWithUserRefreshToken(TokenRequestContext requ } return Mono.defer(() -> { try { - return Mono.fromFuture(publicClientApplication.acquireTokenSilently(parameters)).map(MsalToken::new); + return Mono.fromFuture(publicClientApplication.acquireTokenSilently(parameters)).map(ar -> new MsalToken(ar, options)); } catch (MalformedURLException e) { return Mono.error(e); } @@ -245,7 +241,7 @@ public Mono authenticateWithDeviceCode(TokenRequestContext request, dc -> deviceCodeConsumer.accept(new DeviceCodeInfo(dc.userCode(), dc.deviceCode(), dc.verificationUri(), OffsetDateTime.now().plusSeconds(dc.expiresIn()), dc.message()))).build(); return publicClientApplication.acquireToken(parameters); - }).map(MsalToken::new); + }).map(ar -> new MsalToken(ar, options)); } /** @@ -262,7 +258,7 @@ public Mono authenticateWithAuthorizationCode(TokenRequestContext req AuthorizationCodeParameters.builder(authorizationCode, redirectUrl) .scopes(new HashSet<>(request.getScopes())) .build())) - .map(MsalToken::new); + .map(ar -> new MsalToken(ar, options)); } /** @@ -350,7 +346,9 @@ public Mono authenticateToManagedIdentityEndpoint(String msiEndpoin Scanner s = new Scanner(connection.getInputStream(), StandardCharsets.UTF_8.name()).useDelimiter("\\A"); String result = s.hasNext() ? s.next() : ""; - return Mono.just(SERIALIZER_ADAPTER.deserialize(result, MSIToken.class, SerializerEncoding.JSON)); + MSIToken msiToken = SERIALIZER_ADAPTER.deserialize(result, MSIToken.class, SerializerEncoding.JSON); + return Mono.just(new AccessToken(msiToken.getToken(), + msiToken.getExpiresAt().plusMinutes(2).minus(options.getRefreshBeforeExpiry()))); } catch (IOException e) { return Mono.error(e); } finally { @@ -403,7 +401,9 @@ public Mono authenticateToIMDSEndpoint(TokenRequestContext request) .useDelimiter("\\A"); String result = s.hasNext() ? s.next() : ""; - return SERIALIZER_ADAPTER.deserialize(result, MSIToken.class, SerializerEncoding.JSON); + MSIToken msiToken = SERIALIZER_ADAPTER.deserialize(result, MSIToken.class, SerializerEncoding.JSON); + return new AccessToken(msiToken.getToken(), + msiToken.getExpiresAt().plusMinutes(2).minus(options.getRefreshBeforeExpiry())); } catch (IOException exception) { if (connection == null) { throw logger.logExceptionAsError(new RuntimeException( diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java index fb8db6f635eb..8d14bb884d33 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java @@ -21,6 +21,7 @@ public final class IdentityClientOptions { private Function retryTimeout; private ProxyOptions proxyOptions; private HttpPipeline httpPipeline; + private Duration refreshBeforeExpiry; /** * Creates an instance of IdentityClientOptions with default settings. @@ -115,4 +116,22 @@ public IdentityClientOptions setHttpPipeline(HttpPipeline httpPipeline) { this.httpPipeline = httpPipeline; return this; } + + /** + * @return the duration before the actual expiry of a token to refresh it. + */ + public Duration getRefreshBeforeExpiry() { + return refreshBeforeExpiry; + } + + /** + * Sets the duration before the actual expiry of a token to refresh it. + * This is useful when network is congested and a request containing the + * token takes longer than normal to get to the server. + * + * @param refreshBeforeExpiry the duration before the actual expiry of a token to refresh it + */ + public void setRefreshBeforeExpiry(Duration refreshBeforeExpiry) { + this.refreshBeforeExpiry = refreshBeforeExpiry; + } } diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/MsalToken.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/MsalToken.java index e00e699709bf..291671435aa3 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/MsalToken.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/MsalToken.java @@ -22,9 +22,10 @@ public final class MsalToken extends AccessToken { * * @param msalResult the raw authentication result returned by MSAL */ - public MsalToken(IAuthenticationResult msalResult) { - super(msalResult.accessToken(), OffsetDateTime.ofInstant(msalResult.expiresOnDate().toInstant(), - ZoneOffset.UTC)); + public MsalToken(IAuthenticationResult msalResult, IdentityClientOptions options) { + super(msalResult.accessToken(), OffsetDateTime.ofInstant( + msalResult.expiresOnDate().toInstant().minus(options.getRefreshBeforeExpiry()), ZoneOffset.UTC) + .plusMinutes(2)); this.account = msalResult.account(); } From 1217579f5f1dcbaa2723d0d56560edce1ac6f1fa Mon Sep 17 00:00:00 2001 From: Jianghao Lu Date: Wed, 4 Mar 2020 17:17:47 -0800 Subject: [PATCH 2/5] Fix test utils in identity --- .../azure/identity/implementation/IdentityClientOptions.java | 2 +- .../src/test/java/com/azure/identity/util/TestUtils.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java index 8d14bb884d33..d510eb124ed4 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java @@ -21,7 +21,7 @@ public final class IdentityClientOptions { private Function retryTimeout; private ProxyOptions proxyOptions; private HttpPipeline httpPipeline; - private Duration refreshBeforeExpiry; + private Duration refreshBeforeExpiry = Duration.ofMinutes(2); /** * Creates an instance of IdentityClientOptions with default settings. diff --git a/sdk/identity/azure-identity/src/test/java/com/azure/identity/util/TestUtils.java b/sdk/identity/azure-identity/src/test/java/com/azure/identity/util/TestUtils.java index 78545f83ab39..b836b227792e 100644 --- a/sdk/identity/azure-identity/src/test/java/com/azure/identity/util/TestUtils.java +++ b/sdk/identity/azure-identity/src/test/java/com/azure/identity/util/TestUtils.java @@ -4,6 +4,7 @@ package com.azure.identity.util; import com.azure.core.credential.AccessToken; +import com.azure.identity.implementation.IdentityClientOptions; import com.azure.identity.implementation.MsalToken; import com.microsoft.aad.msal4j.IAccount; import com.microsoft.aad.msal4j.IAuthenticationResult; @@ -82,7 +83,7 @@ public Date expiresOnDate() { */ public static Mono getMockMsalToken(String accessToken, OffsetDateTime expiresOn) { return Mono.fromFuture(getMockAuthenticationResult(accessToken, expiresOn)) - .map(MsalToken::new); + .map(ar -> new MsalToken(ar, new IdentityClientOptions())); } /** From a934984f22895336c51fbe443c69443ee1494353 Mon Sep 17 00:00:00 2001 From: Jianghao Lu Date: Thu, 5 Mar 2020 11:56:05 -0800 Subject: [PATCH 3/5] Address feedback --- .../azure/identity/CredentialBuilderBase.java | 11 ++-- .../implementation/IdentityClient.java | 59 +++++++++---------- .../implementation/IdentityClientOptions.java | 19 +++--- .../implementation/IdentityToken.java | 24 ++++++++ .../identity/implementation/MsalToken.java | 9 ++- 5 files changed, 74 insertions(+), 48 deletions(-) create mode 100644 sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityToken.java diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java index 50bc9eb1480a..ec5e91a059eb 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java @@ -70,15 +70,18 @@ public T httpPipeline(HttpPipeline httpPipeline) { } /** - * Sets the duration before the actual expiry of a token to refresh it. + * Sets how long before the actual token expiry to refresh the token. The + * token will be considered expired at and after the time of (actual + * expiry - token refresh offset). The default offset is 2 minutes. + * * This is useful when network is congested and a request containing the * token takes longer than normal to get to the server. * - * @param refreshBeforeExpiry the duration before the actual expiry of a token to refresh it + * @param tokenRefreshOffset the duration before the actual expiry of a token to refresh it */ @SuppressWarnings("unchecked") - public T setRefreshBeforeExpiry(Duration refreshBeforeExpiry) { - this.identityClientOptions.setRefreshBeforeExpiry(refreshBeforeExpiry); + public T setTokenRefreshOffset(Duration tokenRefreshOffset) { + this.identityClientOptions.setTokenRefreshOffset(tokenRefreshOffset); return (T) this; } } diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java index af7d26d8b48a..a1ca20273201 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java @@ -217,7 +217,8 @@ public Mono authenticateWithUserRefreshToken(TokenRequestContext requ } return Mono.defer(() -> { try { - return Mono.fromFuture(publicClientApplication.acquireTokenSilently(parameters)).map(ar -> new MsalToken(ar, options)); + return Mono.fromFuture(publicClientApplication.acquireTokenSilently(parameters)) + .map(ar -> new MsalToken(ar, options)); } catch (MalformedURLException e) { return Mono.error(e); } @@ -315,11 +316,11 @@ public Mono authenticateWithBrowserInteraction(TokenRequestContext re */ public Mono authenticateToManagedIdentityEndpoint(String msiEndpoint, String msiSecret, TokenRequestContext request) { - String resource = ScopeUtil.scopesToResource(request.getScopes()); - HttpURLConnection connection = null; - StringBuilder payload = new StringBuilder(); + return Mono.fromCallable(() -> { + String resource = ScopeUtil.scopesToResource(request.getScopes()); + HttpURLConnection connection = null; + StringBuilder payload = new StringBuilder(); - try { payload.append("resource="); payload.append(URLEncoder.encode(resource, "UTF-8")); payload.append("&api-version="); @@ -328,34 +329,30 @@ public Mono authenticateToManagedIdentityEndpoint(String msiEndpoin payload.append("&clientid="); payload.append(URLEncoder.encode(clientId, "UTF-8")); } - } catch (IOException exception) { - return Mono.error(exception); - } - try { - URL url = new URL(String.format("%s?%s", msiEndpoint, payload)); - connection = (HttpURLConnection) url.openConnection(); + try { + URL url = new URL(String.format("%s?%s", msiEndpoint, payload)); + connection = (HttpURLConnection) url.openConnection(); - connection.setRequestMethod("GET"); - if (msiSecret != null) { - connection.setRequestProperty("Secret", msiSecret); - } - connection.setRequestProperty("Metadata", "true"); + connection.setRequestMethod("GET"); + if (msiSecret != null) { + connection.setRequestProperty("Secret", msiSecret); + } + connection.setRequestProperty("Metadata", "true"); - connection.connect(); + connection.connect(); - Scanner s = new Scanner(connection.getInputStream(), StandardCharsets.UTF_8.name()).useDelimiter("\\A"); - String result = s.hasNext() ? s.next() : ""; + Scanner s = new Scanner(connection.getInputStream(), StandardCharsets.UTF_8.name()) + .useDelimiter("\\A"); + String result = s.hasNext() ? s.next() : ""; - MSIToken msiToken = SERIALIZER_ADAPTER.deserialize(result, MSIToken.class, SerializerEncoding.JSON); - return Mono.just(new AccessToken(msiToken.getToken(), - msiToken.getExpiresAt().plusMinutes(2).minus(options.getRefreshBeforeExpiry()))); - } catch (IOException e) { - return Mono.error(e); - } finally { - if (connection != null) { - connection.disconnect(); + MSIToken msiToken = SERIALIZER_ADAPTER.deserialize(result, MSIToken.class, SerializerEncoding.JSON); + return new IdentityToken(msiToken.getToken(), msiToken.getExpiresAt(), options); + } finally { + if (connection != null) { + connection.disconnect(); + } } - } + }); } /** @@ -401,9 +398,9 @@ public Mono authenticateToIMDSEndpoint(TokenRequestContext request) .useDelimiter("\\A"); String result = s.hasNext() ? s.next() : ""; - MSIToken msiToken = SERIALIZER_ADAPTER.deserialize(result, MSIToken.class, SerializerEncoding.JSON); - return new AccessToken(msiToken.getToken(), - msiToken.getExpiresAt().plusMinutes(2).minus(options.getRefreshBeforeExpiry())); + MSIToken msiToken = SERIALIZER_ADAPTER.deserialize(result, + MSIToken.class, SerializerEncoding.JSON); + return new IdentityToken(msiToken.getToken(), msiToken.getExpiresAt(), options); } catch (IOException exception) { if (connection == null) { throw logger.logExceptionAsError(new RuntimeException( diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java index d510eb124ed4..3cf071b8a68b 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java @@ -21,7 +21,7 @@ public final class IdentityClientOptions { private Function retryTimeout; private ProxyOptions proxyOptions; private HttpPipeline httpPipeline; - private Duration refreshBeforeExpiry = Duration.ofMinutes(2); + private Duration tokenRefreshOffset = Duration.ofMinutes(2); /** * Creates an instance of IdentityClientOptions with default settings. @@ -118,20 +118,23 @@ public IdentityClientOptions setHttpPipeline(HttpPipeline httpPipeline) { } /** - * @return the duration before the actual expiry of a token to refresh it. + * @return how long before the actual token expiry to refresh the token. */ - public Duration getRefreshBeforeExpiry() { - return refreshBeforeExpiry; + public Duration getTokenRefreshOffset() { + return tokenRefreshOffset; } /** - * Sets the duration before the actual expiry of a token to refresh it. + * Sets how long before the actual token expiry to refresh the token. The + * token will be considered expired at and after the time of (actual + * expiry - token refresh offset). The default offset is 2 minutes. + * * This is useful when network is congested and a request containing the * token takes longer than normal to get to the server. * - * @param refreshBeforeExpiry the duration before the actual expiry of a token to refresh it + * @param tokenRefreshOffset the duration before the actual expiry of a token to refresh it */ - public void setRefreshBeforeExpiry(Duration refreshBeforeExpiry) { - this.refreshBeforeExpiry = refreshBeforeExpiry; + public void setTokenRefreshOffset(Duration tokenRefreshOffset) { + this.tokenRefreshOffset = tokenRefreshOffset; } } diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityToken.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityToken.java new file mode 100644 index 000000000000..c6973b3d99a8 --- /dev/null +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityToken.java @@ -0,0 +1,24 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.identity.implementation; + +import com.azure.core.credential.AccessToken; + +import java.time.OffsetDateTime; + +/** + * Type representing authentication result from the azure-identity client. + */ +public class IdentityToken extends AccessToken { + /** + * Creates an identity token instance. + * + * @param token the token string. + * @param expiresAt the expiration time. + * @param options the identity client options. + */ + public IdentityToken(String token, OffsetDateTime expiresAt, IdentityClientOptions options) { + super(token, expiresAt.plusMinutes(2).minus(options.getTokenRefreshOffset())); + } +} diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/MsalToken.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/MsalToken.java index 291671435aa3..5ccc94892a49 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/MsalToken.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/MsalToken.java @@ -3,7 +3,6 @@ package com.azure.identity.implementation; -import com.azure.core.credential.AccessToken; import com.microsoft.aad.msal4j.IAccount; import com.microsoft.aad.msal4j.IAuthenticationResult; @@ -13,7 +12,7 @@ /** * Type representing authentication result from the MSAL (Microsoft Authentication Library). */ -public final class MsalToken extends AccessToken { +public final class MsalToken extends IdentityToken { private IAccount account; @@ -23,9 +22,9 @@ public final class MsalToken extends AccessToken { * @param msalResult the raw authentication result returned by MSAL */ public MsalToken(IAuthenticationResult msalResult, IdentityClientOptions options) { - super(msalResult.accessToken(), OffsetDateTime.ofInstant( - msalResult.expiresOnDate().toInstant().minus(options.getRefreshBeforeExpiry()), ZoneOffset.UTC) - .plusMinutes(2)); + super(msalResult.accessToken(), + OffsetDateTime.ofInstant(msalResult.expiresOnDate().toInstant(), ZoneOffset.UTC), + options); this.account = msalResult.account(); } From 3e0664454f86a2cc762bf6ce5dfbdc0ef0f6c53c Mon Sep 17 00:00:00 2001 From: Jianghao Lu Date: Thu, 5 Mar 2020 12:00:00 -0800 Subject: [PATCH 4/5] Builder style setter --- .../src/main/java/com/azure/identity/CredentialBuilderBase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java index ec5e91a059eb..80146df1d0dc 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java @@ -80,7 +80,7 @@ public T httpPipeline(HttpPipeline httpPipeline) { * @param tokenRefreshOffset the duration before the actual expiry of a token to refresh it */ @SuppressWarnings("unchecked") - public T setTokenRefreshOffset(Duration tokenRefreshOffset) { + public T tokenRefreshOffset(Duration tokenRefreshOffset) { this.identityClientOptions.setTokenRefreshOffset(tokenRefreshOffset); return (T) this; } From 07a5ce9c0ae694d1e9bf34bfd90ce6527a28aec0 Mon Sep 17 00:00:00 2001 From: Jianghao Lu Date: Fri, 6 Mar 2020 12:24:01 -0800 Subject: [PATCH 5/5] Add tests and address feedback --- .../azure/identity/CredentialBuilderBase.java | 1 + .../implementation/IdentityClientOptions.java | 5 +- .../identity/ClientSecretCredentialTest.java | 35 ++++++++++ .../ManagedIdentityCredentialTest.java | 67 ++++++++++++++++++- .../com/azure/identity/util/TestUtils.java | 12 ++++ 5 files changed, 115 insertions(+), 5 deletions(-) diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java index ec16b95a967e..51b428079412 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java @@ -94,6 +94,7 @@ public T httpClient(HttpClient client) { * token takes longer than normal to get to the server. * * @param tokenRefreshOffset the duration before the actual expiry of a token to refresh it + * @return An updated instance of this builder with the token refresh offset set as specified. */ @SuppressWarnings("unchecked") public T tokenRefreshOffset(Duration tokenRefreshOffset) { diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java index b2f0b7adcb95..15fe75bf9dd3 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java @@ -127,7 +127,6 @@ public IdentityClientOptions setHttpPipeline(HttpPipeline httpPipeline) { } /** -<<<<<<< HEAD * @return how long before the actual token expiry to refresh the token. */ public Duration getTokenRefreshOffset() { @@ -145,7 +144,9 @@ public Duration getTokenRefreshOffset() { * @param tokenRefreshOffset the duration before the actual expiry of a token to refresh it */ public IdentityClientOptions setTokenRefreshOffset(Duration tokenRefreshOffset) { - this.tokenRefreshOffset = tokenRefreshOffset; + if (tokenRefreshOffset != null) { + this.tokenRefreshOffset = tokenRefreshOffset; + } return this; } diff --git a/sdk/identity/azure-identity/src/test/java/com/azure/identity/ClientSecretCredentialTest.java b/sdk/identity/azure-identity/src/test/java/com/azure/identity/ClientSecretCredentialTest.java index b43feadb7027..3cb4359cb004 100644 --- a/sdk/identity/azure-identity/src/test/java/com/azure/identity/ClientSecretCredentialTest.java +++ b/sdk/identity/azure-identity/src/test/java/com/azure/identity/ClientSecretCredentialTest.java @@ -17,6 +17,7 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import java.time.Duration; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.UUID; @@ -61,6 +62,40 @@ public void testValidSecrets() throws Exception { .verifyComplete(); } + @Test + public void testValidSecretsWithTokenRefreshOffset() throws Exception { + // setup + String secret = "secret"; + String token1 = "token1"; + String token2 = "token2"; + TokenRequestContext request1 = new TokenRequestContext().addScopes("https://management.azure.com"); + TokenRequestContext request2 = new TokenRequestContext().addScopes("https://vault.azure.net"); + OffsetDateTime expiresAt = OffsetDateTime.now(ZoneOffset.UTC).plusHours(1); + Duration offset = Duration.ofMinutes(10); + + // mock + IdentityClient identityClient = PowerMockito.mock(IdentityClient.class); + when(identityClient.authenticateWithClientSecret(secret, request1)).thenReturn(TestUtils.getMockAccessToken(token1, expiresAt, offset)); + when(identityClient.authenticateWithClientSecret(secret, request2)).thenReturn(TestUtils.getMockAccessToken(token2, expiresAt, offset)); + PowerMockito.whenNew(IdentityClient.class).withAnyArguments().thenReturn(identityClient); + + // test + ClientSecretCredential credential = new ClientSecretCredentialBuilder() + .tenantId(tenantId) + .clientId(clientId) + .clientSecret(secret) + .tokenRefreshOffset(offset) + .build(); + StepVerifier.create(credential.getToken(request1)) + .expectNextMatches(accessToken -> token1.equals(accessToken.getToken()) + && expiresAt.getSecond() == accessToken.getExpiresAt().getSecond()) + .verifyComplete(); + StepVerifier.create(credential.getToken(request2)) + .expectNextMatches(accessToken -> token2.equals(accessToken.getToken()) + && expiresAt.getSecond() == accessToken.getExpiresAt().getSecond()) + .verifyComplete(); + } + @Test public void testInvalidSecrets() throws Exception { // setup diff --git a/sdk/identity/azure-identity/src/test/java/com/azure/identity/ManagedIdentityCredentialTest.java b/sdk/identity/azure-identity/src/test/java/com/azure/identity/ManagedIdentityCredentialTest.java index 54c9c8b9c37b..ebaf988af222 100644 --- a/sdk/identity/azure-identity/src/test/java/com/azure/identity/ManagedIdentityCredentialTest.java +++ b/sdk/identity/azure-identity/src/test/java/com/azure/identity/ManagedIdentityCredentialTest.java @@ -16,6 +16,7 @@ import org.powermock.modules.junit4.PowerMockRunner; import reactor.test.StepVerifier; +import java.time.Duration; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.UUID; @@ -83,8 +84,68 @@ public void testIMDS() throws Exception { // test ManagedIdentityCredential credential = new ManagedIdentityCredentialBuilder().clientId(clientId).build(); StepVerifier.create(credential.getToken(request)) - .expectNextMatches(token -> token1.equals(token.getToken()) - && expiresOn.getSecond() == token.getExpiresAt().getSecond()) - .verifyComplete(); + .expectNextMatches(token -> token1.equals(token.getToken()) + && expiresOn.getSecond() == token.getExpiresAt().getSecond()) + .verifyComplete(); + } + + @Test + public void testMSIEndpointWithTokenRefreshOffset() throws Exception { + Configuration configuration = Configuration.getGlobalConfiguration(); + + try { + // setup + String endpoint = "http://localhost"; + String secret = "secret"; + String token1 = "token1"; + TokenRequestContext request1 = new TokenRequestContext().addScopes("https://management.azure.com"); + OffsetDateTime expiresAt = OffsetDateTime.now(ZoneOffset.UTC).plusHours(1); + configuration.put("MSI_ENDPOINT", endpoint); + configuration.put("MSI_SECRET", secret); + Duration offset = Duration.ofMinutes(10); + + // mock + IdentityClient identityClient = PowerMockito.mock(IdentityClient.class); + when(identityClient.authenticateToManagedIdentityEndpoint(endpoint, secret, request1)).thenReturn(TestUtils.getMockAccessToken(token1, expiresAt, offset)); + PowerMockito.whenNew(IdentityClient.class).withAnyArguments().thenReturn(identityClient); + + // test + ManagedIdentityCredential credential = new ManagedIdentityCredentialBuilder() + .clientId(clientId) + .tokenRefreshOffset(offset) + .build(); + StepVerifier.create(credential.getToken(request1)) + .expectNextMatches(token -> token1.equals(token.getToken()) + && expiresAt.getSecond() == token.getExpiresAt().getSecond()) + .verifyComplete(); + } finally { + // clean up + configuration.remove("MSI_ENDPOINT"); + configuration.remove("MSI_SECRET"); + } + } + + @Test + public void testIMDSWithTokenRefreshOffset() throws Exception { + // setup + String token1 = "token1"; + TokenRequestContext request = new TokenRequestContext().addScopes("https://management.azure.com"); + OffsetDateTime expiresOn = OffsetDateTime.now(ZoneOffset.UTC).plusHours(1); + Duration offset = Duration.ofMinutes(10); + + // mock + IdentityClient identityClient = PowerMockito.mock(IdentityClient.class); + when(identityClient.authenticateToIMDSEndpoint(request)).thenReturn(TestUtils.getMockAccessToken(token1, expiresOn, offset)); + PowerMockito.whenNew(IdentityClient.class).withAnyArguments().thenReturn(identityClient); + + // test + ManagedIdentityCredential credential = new ManagedIdentityCredentialBuilder() + .clientId(clientId) + .tokenRefreshOffset(offset) + .build(); + StepVerifier.create(credential.getToken(request)) + .expectNextMatches(token -> token1.equals(token.getToken()) + && expiresOn.getSecond() == token.getExpiresAt().getSecond()) + .verifyComplete(); } } diff --git a/sdk/identity/azure-identity/src/test/java/com/azure/identity/util/TestUtils.java b/sdk/identity/azure-identity/src/test/java/com/azure/identity/util/TestUtils.java index b836b227792e..a041b0977c43 100644 --- a/sdk/identity/azure-identity/src/test/java/com/azure/identity/util/TestUtils.java +++ b/sdk/identity/azure-identity/src/test/java/com/azure/identity/util/TestUtils.java @@ -10,6 +10,7 @@ import com.microsoft.aad.msal4j.IAuthenticationResult; import reactor.core.publisher.Mono; +import java.time.Duration; import java.time.OffsetDateTime; import java.util.Date; import java.util.UUID; @@ -96,6 +97,17 @@ public static Mono getMockAccessToken(String accessToken, OffsetDat return Mono.just(new AccessToken(accessToken, expiresOn.plusMinutes(2))); } + /** + * Creates a mock {@link AccessToken} instance. + * @param accessToken the access token to return + * @param expiresOn the expiration time + * @param tokenRefreshOffset how long before the actual expiry to refresh the token + * @return a Mono publisher of the result + */ + public static Mono getMockAccessToken(String accessToken, OffsetDateTime expiresOn, Duration tokenRefreshOffset) { + return Mono.just(new AccessToken(accessToken, expiresOn.plusMinutes(2).minus(tokenRefreshOffset))); + } + private TestUtils() { } }