Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemement OnBehalfOfCredential #22146

Merged
merged 31 commits into from
Sep 4, 2021
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
264bff6
SupportsCaching property, OBO credential
christothes Apr 28, 2021
1eaca9f
export
christothes Jun 24, 2021
3abc6a3
BAP test
christothes Jun 24, 2021
568908a
missing xml docs
christothes Jun 24, 2021
5287642
CredentialTestBase
christothes Jun 25, 2021
66adcea
add new virtual to UnsafeTokenCacheOptions
christothes Jun 29, 2021
ac08680
refactor UserAssertionScope
christothes Jun 29, 2021
0bb3d6a
merge
christothes Jun 29, 2021
1a7b5c1
export
christothes Jun 30, 2021
ae2b022
UnsafeTokenCacheOptions and TokenCacheNotificationDetails
christothes Jun 30, 2021
05fee10
tweaks
christothes Jun 30, 2021
cc4ac48
Merge remote-tracking branch 'upstream/main' into chriss/OnBehalfOf
christothes Jul 12, 2021
5029352
Merge remote-tracking branch 'upstream/main' into chriss/OnBehalfOf
christothes Jul 19, 2021
64d2788
fb
christothes Jul 19, 2021
4a68ef1
refactor using AccessToken.RefreshOn
christothes Aug 12, 2021
6be7493
export
christothes Aug 12, 2021
7f52c10
fb
christothes Aug 12, 2021
8a6be33
tweaks
christothes Aug 12, 2021
0440c00
remove comment
christothes Aug 12, 2021
52fe1b1
Merge remote-tracking branch 'upstream/main' into chriss/OnBehalfOf
christothes Aug 16, 2021
916eed1
Merge branch 'chriss/OnBehalfOf' of https://github.com/christothes/az…
christothes Aug 16, 2021
d7b582d
use DateTimeOffset.MinValue as no-cache indicator
christothes Aug 16, 2021
3b10559
protect against datetimeoffset underflow
christothes Aug 17, 2021
cecffe1
update RefreshOn doc comment
christothes Aug 18, 2021
cc7fd2f
Simple OBO
christothes Aug 27, 2021
bab4afc
revert core changes
christothes Aug 27, 2021
837d1d6
proj tweak
christothes Aug 27, 2021
ea9066f
merge
christothes Aug 27, 2021
baed5fe
more ctor overloads
christothes Aug 30, 2021
cf193ab
fb
christothes Sep 3, 2021
aea16af
merge
christothes Sep 3, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdk/identity/Azure.Identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Thank you to our developer community members who helped to make Azure Identity b
- Added support to `ManagedIdentityCredential` for Bridge to Kubernetes local development authentication.
- TenantId values returned from service challenge responses can now be used to request tokens from the correct tenantId. To support this feature, there is a new `AllowMultiTenantAuthentication` option on `TokenCredentialOptions`.
- By default, `AllowMultiTenantAuthentication` is false. When this option property is false and the tenant Id configured in the credential options differs from the tenant Id set in the `TokenRequestContext` sent to a credential, an `AuthorizationFailedException` will be thrown. This is potentially breaking change as it could be a different exception than what was thrown previously. This exception behavior can be overridden by either setting an `AppContext` switch named "Azure.Identity.EnableLegacyTenantSelection" to `true` or by setting the environment variable "AZURE_IDENTITY_ENABLE_LEGACY_TENANT_SELECTION" to "true". Note: AppContext switches can also be configured via configuration like below:
- Added `OnBehalfOfFlowCredential` which enables support for AAD On-Behalf-Of (OBO) flow. See the [Azure Active Directory documentation](https://docs.microsoft.com/azure/active-directory/develop/v2-oauth2-on-behalf-of-flow) to learn more about OBO flow scenarios.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this feature deserves a sample.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed - I plan to add one in a follow up PR


```xml
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,18 @@ public ManagedIdentityCredential(string clientId = null, Azure.Identity.TokenCre
public override Azure.Core.AccessToken GetToken(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public override System.Threading.Tasks.ValueTask<Azure.Core.AccessToken> GetTokenAsync(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
}
public partial class OnBehalfOfCredential : Azure.Core.TokenCredential
{
protected OnBehalfOfCredential() { }
public OnBehalfOfCredential(string tenantId, string clientId, string clientSecret, string userAssertion, Azure.Identity.OnBehalfOfCredentialOptions options = null) { }
public override Azure.Core.AccessToken GetToken(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken) { throw null; }
public override System.Threading.Tasks.ValueTask<Azure.Core.AccessToken> GetTokenAsync(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken) { throw null; }
}
public partial class OnBehalfOfCredentialOptions : Azure.Identity.TokenCredentialOptions
{
public OnBehalfOfCredentialOptions() { }
public Azure.Identity.TokenCachePersistenceOptions TokenCachePersistenceOptions { get { throw null; } set { } }
}
[System.Runtime.InteropServices.StructLayoutAttribute(System.Runtime.InteropServices.LayoutKind.Sequential)]
public readonly partial struct RegionalAuthority : System.IEquatable<Azure.Identity.RegionalAuthority>
{
Expand Down Expand Up @@ -326,6 +338,18 @@ public SharedTokenCacheCredentialOptions(Azure.Identity.TokenCachePersistenceOpt
public Azure.Identity.TokenCachePersistenceOptions TokenCachePersistenceOptions { get { throw null; } }
public string Username { get { throw null; } set { } }
}
[System.Runtime.InteropServices.StructLayoutAttribute(System.Runtime.InteropServices.LayoutKind.Sequential)]
public partial struct TokenCacheDetails
{
private object _dummy;
private int _dummyPrimitive;
public System.ReadOnlyMemory<byte> CacheBytes { get { throw null; } set { } }
}
public partial class TokenCacheNotificationDetails
{
internal TokenCacheNotificationDetails() { }
public string SuggestedCacheKey { get { throw null; } }
}
public partial class TokenCachePersistenceOptions
{
public TokenCachePersistenceOptions() { }
Expand All @@ -348,6 +372,7 @@ public abstract partial class UnsafeTokenCacheOptions : Azure.Identity.TokenCach
{
protected UnsafeTokenCacheOptions() { }
protected internal abstract System.Threading.Tasks.Task<System.ReadOnlyMemory<byte>> RefreshCacheAsync();
protected internal virtual System.Threading.Tasks.Task<Azure.Identity.TokenCacheDetails> RefreshCacheAsync(Azure.Identity.TokenCacheNotificationDetails details) { throw null; }
protected internal abstract System.Threading.Tasks.Task TokenCacheUpdatedAsync(Azure.Identity.TokenCacheUpdatedArgs tokenCacheUpdatedArgs);
}
public partial class UsernamePasswordCredential : Azure.Core.TokenCredential
Expand Down
20 changes: 20 additions & 0 deletions sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,25 @@ public virtual async ValueTask<AuthenticationResult> AcquireTokenByAuthorization
.ExecuteAsync(async, cancellationToken)
.ConfigureAwait(false);
}

public virtual async ValueTask<AuthenticationResult> AcquireTokenOnBehalfOf(
string[] scopes,
string tenantId,
UserAssertion userAssertionValue,
bool async,
CancellationToken cancellationToken)
{
IConfidentialClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);

var builder = client.AcquireTokenOnBehalfOf(scopes, userAssertionValue);

if (!string.IsNullOrEmpty(tenantId))
{
builder.WithAuthority(Pipeline.AuthorityHost.AbsoluteUri, tenantId);
}
return await builder
.ExecuteAsync(async, cancellationToken)
.ConfigureAwait(false);
}
}
}
99 changes: 99 additions & 0 deletions sdk/identity/Azure.Identity/src/OnBehalfOfCredential.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Threading;
using System.Threading.Tasks;
using Azure.Core;
using Azure.Core.Pipeline;
using Microsoft.Identity.Client;

namespace Azure.Identity
{
/// <summary>
/// Enables authentication to Azure Active Directory using an On-Behalf-Of flow.``
/// </summary>
public class OnBehalfOfCredential : TokenCredential
{
private readonly MsalConfidentialClient _client;
private readonly string _tenantId;
private readonly CredentialPipeline _pipeline;
private readonly bool _allowMultiTenantAuthentication;
private readonly string _clientId;
private readonly string _clientSecret;
private readonly UserAssertion _userAssertion;

/// <summary>
/// Protected constructor for mocking.
/// </summary>
protected OnBehalfOfCredential()
{ }

/// <summary>
/// Creates an instance of the <see cref="OnBehalfOfCredential"/> with the details needed to authenticate with Azure Active Directory.
/// </summary>
/// <param name="tenantId">The Azure Active Directory tenant (directory) Id of the service principal.</param>
/// <param name="clientId">The client (application) ID of the service principal</param>
/// <param name="clientSecret">A client secret that was generated for the App Registration used to authenticate the client.</param>
/// <param name="userAssertion">The access token that will be used by <see cref="OnBehalfOfCredential"/> as the user assertion when requesting On-Behalf-Of tokens.</param>
/// <param name="options">Options that allow to configure the management of the requests sent to the Azure Active Directory service.</param>
public OnBehalfOfCredential(
string tenantId,
string clientId,
string clientSecret,
string userAssertion,
OnBehalfOfCredentialOptions options = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why does this ctor use a default parameter where the ctor which takes an X509Cert has an explicit overload?

: this(tenantId, clientId, clientSecret, userAssertion, options, null, null)
{ }

internal OnBehalfOfCredential(
string tenantId,
string clientId,
string clientSecret,
string userAssertion,
OnBehalfOfCredentialOptions options,
CredentialPipeline pipeline,
MsalConfidentialClient client)
{
Argument.AssertNotNull(clientId, nameof(clientId));
Argument.AssertNotNull(clientSecret, nameof(clientSecret));

options ??= new OnBehalfOfCredentialOptions();
_pipeline = pipeline ?? CredentialPipeline.GetInstance(options);
_allowMultiTenantAuthentication = options.AllowMultiTenantAuthentication;
_tenantId = Validations.ValidateTenantId(tenantId, nameof(tenantId));
_clientId = clientId;
_clientSecret = clientSecret;
_userAssertion = new UserAssertion(userAssertion);
_client = client ?? new MsalConfidentialClient(_pipeline, _tenantId, _clientId, _clientSecret, options, default, options.IsLoggingPIIEnabled);
}

/// <inheritdoc />
public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) =>
GetTokenInternalAsync(requestContext, false, cancellationToken).EnsureCompleted();

/// <inheritdoc />
public override ValueTask<AccessToken> GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) =>
GetTokenInternalAsync(requestContext, true, cancellationToken);

internal async ValueTask<AccessToken> GetTokenInternalAsync(TokenRequestContext requestContext, bool async, CancellationToken cancellationToken)
{
using CredentialDiagnosticScope scope = _pipeline.StartGetTokenScope("OnBehalfOfCredential.GetToken", requestContext);

try
{
var tenantId = TenantIdResolver.Resolve(_tenantId, requestContext, _allowMultiTenantAuthentication);

AuthenticationResult result = await _client
.AcquireTokenOnBehalfOf(requestContext.Scopes, tenantId, _userAssertion, async, cancellationToken)
.ConfigureAwait(false);

return new AccessToken(result.AccessToken, result.ExpiresOn);
}
catch (Exception e)
{
throw scope.FailWrapAndThrow(e);
}
}
}
}
16 changes: 16 additions & 0 deletions sdk/identity/Azure.Identity/src/OnBehalfOfCredentialOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

namespace Azure.Identity
{
/// <summary>
///
/// </summary>
public class OnBehalfOfCredentialOptions : TokenCredentialOptions, ITokenCacheOptions
{
/// <summary>
/// The <see cref="TokenCachePersistenceOptions"/>.
/// </summary>
public TokenCachePersistenceOptions TokenCachePersistenceOptions { get; set; }
}
}
6 changes: 3 additions & 3 deletions sdk/identity/Azure.Identity/src/TokenCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Azure.Core.Pipeline;
using Microsoft.Identity.Client;
using Microsoft.Identity.Client.Extensions.Msal;

Expand Down Expand Up @@ -88,7 +87,7 @@ internal TokenCache(TokenCachePersistenceOptions options, MsalCacheHelperWrapper
/// <summary>
/// A delegate that will be called before the cache is accessed. The data returned will be used to set the current state of the cache.
/// </summary>
internal Func<Task<ReadOnlyMemory<byte>>> RefreshCacheFromOptionsAsync;
internal Func<TokenCacheNotificationDetails, Task<TokenCacheDetails>> RefreshCacheFromOptionsAsync;

internal virtual async Task RegisterCache(bool async, ITokenCache tokenCache, CancellationToken cancellationToken)
{
Expand Down Expand Up @@ -143,7 +142,8 @@ private async Task OnBeforeCacheAccessAsync(TokenCacheNotificationArgs args)
{
if (RefreshCacheFromOptionsAsync != null)
{
Data = (await RefreshCacheFromOptionsAsync().ConfigureAwait(false)).ToArray();
Data = (await RefreshCacheFromOptionsAsync(new TokenCacheNotificationDetails(args)).ConfigureAwait(false))
.CacheBytes.ToArray();
}
args.TokenCache.DeserializeMsalV3(Data, true);

Expand Down
18 changes: 18 additions & 0 deletions sdk/identity/Azure.Identity/src/TokenCacheDetails.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;

namespace Azure.Identity
{
/// <summary>
/// Details related to a <see cref="UnsafeTokenCacheOptions"/> cache delegate.
/// </summary>
public struct TokenCacheDetails
{
/// <summary>
/// The bytes representing the state of the token cache.
/// </summary>
public ReadOnlyMemory<byte> CacheBytes { get; set; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Microsoft.Identity.Client;

namespace Azure.Identity
{
/// <summary>
/// Args setnt to TokenCache OnBefore and OnAfter events.
/// </summary>
public class TokenCacheNotificationDetails
{
/// <summary>
/// A suggested token cache key, which can be used with general purpose storage mechanisms that allow
/// storing key-value pairs and key based retrieval. Useful in applications that store 1 token cache per user,
/// the recommended pattern for web apps.
///
/// The value is:
///
/// <list type="bullet">
/// <item>the homeAccountId for AcquireTokenSilent, GetAccount(homeAccountId), RemoveAccount and when writing tokens on confidential client calls</item>
/// <item>clientID + "_AppTokenCache" for AcquireTokenForClient</item>
/// <item>clientID_tenantID + "_AppTokenCache" for AcquireTokenForClient when tenant specific authority</item>
/// <item>the hash of the original token for AcquireTokenOnBehalfOf</item>
/// </list>
/// </summary>
public string SuggestedCacheKey { get; }

internal TokenCacheNotificationDetails(TokenCacheNotificationArgs args)
{
SuggestedCacheKey = args.SuggestedCacheKey;
}
}
}
13 changes: 12 additions & 1 deletion sdk/identity/Azure.Identity/src/UnsafeTokenCacheOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,20 @@ public abstract class UnsafeTokenCacheOptions : TokenCachePersistenceOptions
protected internal abstract Task TokenCacheUpdatedAsync(TokenCacheUpdatedArgs tokenCacheUpdatedArgs);

/// <summary>
/// The bytes used to initialize the token cache. This would most likely have come from the <see cref="TokenCacheUpdatedArgs"/>.
/// Returns the bytes used to initialize the token cache. This would most likely have come from the <see cref="TokenCacheUpdatedArgs"/>.
/// This implementation will get called by the default implementation of <see cref="RefreshCacheAsync(TokenCacheNotificationDetails)"/>.
/// It is recommended to provide an implementation for <see cref="RefreshCacheAsync(TokenCacheNotificationDetails)"/> rather than this method.
/// </summary>
/// <value></value>
protected internal abstract Task<ReadOnlyMemory<byte>> RefreshCacheAsync();

/// <summary>
/// Returns the bytes used to initialize the token cache. This would most likely have come from the <see cref="TokenCacheUpdatedArgs"/>.
/// It is recommended that if this method is overriden, there is no need to provide a duplicate implementation for the parameterless <see cref="RefreshCacheAsync()"/>.
/// </summary>
/// <param name="details"></param>
/// <returns></returns>
protected internal virtual async Task<TokenCacheDetails> RefreshCacheAsync(TokenCacheNotificationDetails details) =>
new() {CacheBytes = await RefreshCacheAsync().ConfigureAwait(false)};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,67 +4,20 @@
using System;
using System.Threading.Tasks;
using Azure.Core;
using Azure.Core.TestFramework;
using Azure.Identity.Tests.Mock;
using Microsoft.Identity.Client;
using NUnit.Framework;

namespace Azure.Identity.Tests
{
public class AuthorizationCodeCredentialTests : ClientTestBase
public class AuthorizationCodeCredentialTests : CredentialTestBase
{
private const string ClientId = "04b07795-8ddb-461a-bbee-02f9e1bf7b46";
private const string TenantId = "a0287521-e002-0026-7112-207c0c000000";
private const string TenantIdHint = "a0287521-e002-0026-7112-207c0c001234";
private const string ReplyUrl = "https://myredirect/";
private const string Scope = "https://vault.azure.net/.default";
private TokenCredentialOptions options;
private string authCode;
private string expectedToken;
private DateTimeOffset expiresOn;
private MockMsalConfidentialClient mockMsalClient;
private string expectedTenantId;
private string expectedReplyUri;
private string clientSecret = Guid.NewGuid().ToString();
private Func<string[], string, string, AuthenticationAccount, ValueTask<AuthenticationResult>> silentFactory;

public AuthorizationCodeCredentialTests(bool isAsync) : base(isAsync)
{ }

[SetUp]
public void TestSetup()
public void Setup()
{
TestSetup();
expectedTenantId = TenantId;
expectedReplyUri = null;
authCode = Guid.NewGuid().ToString();
expectedToken = Guid.NewGuid().ToString();
expiresOn = DateTimeOffset.Now.AddHours(1);
var result = new AuthenticationResult(
expectedToken,
false,
null,
expiresOn,
expiresOn,
TenantId,
new MockAccount("username"),
null,
new[] { Scope },
Guid.NewGuid(),
null,
"Bearer");
silentFactory = (_, _tenantId, _replyUri, _) =>
{
Assert.AreEqual(expectedTenantId, _tenantId);
Assert.AreEqual(expectedReplyUri, _replyUri);
return new ValueTask<AuthenticationResult>(result);
};
mockMsalClient = new MockMsalConfidentialClient(silentFactory);
mockMsalClient.AuthcodeFactory = (_, _tenantId, _replyUri, _) =>
{
Assert.AreEqual(expectedTenantId, _tenantId);
Assert.AreEqual(expectedReplyUri, _replyUri);
return result;
};
}

[Test]
Expand All @@ -79,7 +32,7 @@ public async Task AuthenticateWithAuthCodeHonorsReplyUrl([Values(null, ReplyUrl)
expectedReplyUri = replyUri;

AuthorizationCodeCredential cred = InstrumentClient(
new AuthorizationCodeCredential(TenantId, ClientId, clientSecret, authCode, options, mockMsalClient));
new AuthorizationCodeCredential(TenantId, ClientId, clientSecret, authCode, options, mockConfidentialMsalClient));

AccessToken token = await cred.GetTokenAsync(context);

Expand All @@ -97,7 +50,8 @@ public async Task AuthenticateWithAuthCodeHonorsTenantId([Values(null, TenantIdH
var context = new TokenRequestContext(new[] { Scope }, tenantId: tenantId);
expectedTenantId = TenantIdResolver.Resolve(TenantId, context, options.AllowMultiTenantAuthentication);

AuthorizationCodeCredential cred = InstrumentClient(new AuthorizationCodeCredential(TenantId, ClientId, clientSecret, authCode, options, mockMsalClient));
AuthorizationCodeCredential cred = InstrumentClient(
new AuthorizationCodeCredential(TenantId, ClientId, clientSecret, authCode, options, mockConfidentialMsalClient));

AccessToken token = await cred.GetTokenAsync(context);

Expand Down
Loading