From 5f5e2fb2a24258822c3cff3820e171947e7968a5 Mon Sep 17 00:00:00 2001 From: Christopher Scott Date: Fri, 14 May 2021 12:45:28 -0500 Subject: [PATCH 1/2] InteractiveBrowser LoginHint --- .../api/Azure.Identity.netstandard2.0.cs | 1 + .../src/InteractiveBrowserCredential.cs | 47 +++++++++++++------ .../InteractiveBrowserCredentialOptions.cs | 5 ++ .../Azure.Identity/src/MsalPublicClient.cs | 27 +++++++---- .../tests/AzureIdentityEventSourceTests.cs | 4 +- .../InteractiveBrowserCredentialTests.cs | 30 +++++++++--- .../tests/Mock/MockMsalPublicClient.cs | 15 ++++-- 7 files changed, 92 insertions(+), 37 deletions(-) diff --git a/sdk/identity/Azure.Identity/api/Azure.Identity.netstandard2.0.cs b/sdk/identity/Azure.Identity/api/Azure.Identity.netstandard2.0.cs index a21e3c673e811..b1e3ddcaf08b8 100644 --- a/sdk/identity/Azure.Identity/api/Azure.Identity.netstandard2.0.cs +++ b/sdk/identity/Azure.Identity/api/Azure.Identity.netstandard2.0.cs @@ -198,6 +198,7 @@ public InteractiveBrowserCredentialOptions() { } public Azure.Identity.AuthenticationRecord AuthenticationRecord { get { throw null; } set { } } public string ClientId { get { throw null; } set { } } public bool DisableAutomaticAuthentication { get { throw null; } set { } } + public string LoginHint { get { throw null; } set { } } public System.Uri RedirectUri { get { throw null; } set { } } public string TenantId { get { throw null; } set { } } public Azure.Identity.TokenCachePersistenceOptions TokenCachePersistenceOptions { get { throw null; } set { } } diff --git a/sdk/identity/Azure.Identity/src/InteractiveBrowserCredential.cs b/sdk/identity/Azure.Identity/src/InteractiveBrowserCredential.cs index b6bc5bee16831..7ea7e73e2c2d0 100644 --- a/sdk/identity/Azure.Identity/src/InteractiveBrowserCredential.cs +++ b/sdk/identity/Azure.Identity/src/InteractiveBrowserCredential.cs @@ -18,12 +18,14 @@ namespace Azure.Identity public class InteractiveBrowserCredential : TokenCredential { internal string ClientId { get; } - internal MsalPublicClient Client {get;} + internal string LoginHint { get; } + internal MsalPublicClient Client { get; } internal CredentialPipeline Pipeline { get; } internal bool DisableAutomaticAuthentication { get; } internal AuthenticationRecord Record { get; private set; } private const string AuthenticationRequiredMessage = "Interactive authentication is needed to acquire token. Call Authenticate to interactively authenticate."; + private const string NoDefaultScopeMessage = "Authenticating in this environment requires specifying a TokenRequestContext."; /// @@ -31,8 +33,7 @@ public class InteractiveBrowserCredential : TokenCredential /// public InteractiveBrowserCredential() : this(null, Constants.DeveloperSignOnClientId, null, null) - { - } + { } /// /// Creates a new with the specified options, which will authenticate users with the specified application. @@ -52,8 +53,7 @@ public InteractiveBrowserCredential(InteractiveBrowserCredentialOptions options) [EditorBrowsable(EditorBrowsableState.Never)] public InteractiveBrowserCredential(string clientId) : this(null, clientId, null, null) - { - } + { } /// /// Creates a new with the specified options, which will authenticate users with the specified application. @@ -64,23 +64,21 @@ public InteractiveBrowserCredential(string clientId) /// The client options for the newly created . [EditorBrowsable(EditorBrowsableState.Never)] public InteractiveBrowserCredential(string tenantId, string clientId, TokenCredentialOptions options = default) - : this(Validations.ValidateTenantId(tenantId, nameof(tenantId), allowNull:true), clientId, options, null, null) - { - } + : this(Validations.ValidateTenantId(tenantId, nameof(tenantId), allowNull: true), clientId, options, null, null) + { } internal InteractiveBrowserCredential(string tenantId, string clientId, TokenCredentialOptions options, CredentialPipeline pipeline) : this(tenantId, clientId, options, pipeline, null) - { - } + { } internal InteractiveBrowserCredential(string tenantId, string clientId, TokenCredentialOptions options, CredentialPipeline pipeline, MsalPublicClient client) { - ClientId = clientId ?? throw new ArgumentNullException(nameof(clientId)); + Argument.AssertNotNull(clientId, nameof(clientId)); + ClientId = clientId; Pipeline = pipeline ?? CredentialPipeline.GetInstance(options); - + LoginHint = (options as InteractiveBrowserCredentialOptions)?.LoginHint; var redirectUrl = (options as InteractiveBrowserCredentialOptions)?.RedirectUri?.AbsoluteUri ?? Constants.DefaultRedirectUrl; - Client = client ?? new MsalPublicClient(Pipeline, tenantId, clientId, redirectUrl, options as ITokenCacheOptions); } @@ -182,7 +180,13 @@ private async ValueTask GetTokenImplAsync(bool async, TokenRequestC { try { - AuthenticationResult result = await Client.AcquireTokenSilentAsync(requestContext.Scopes, requestContext.Claims, Record, async, cancellationToken).ConfigureAwait(false); + AuthenticationResult result = await Client.AcquireTokenSilentAsync( + requestContext.Scopes, + requestContext.Claims, + Record, + async, + cancellationToken) + .ConfigureAwait(false); return scope.Succeeded(new AccessToken(result.AccessToken, result.ExpiresOn)); } @@ -207,7 +211,20 @@ private async ValueTask GetTokenImplAsync(bool async, TokenRequestC private async Task GetTokenViaBrowserLoginAsync(TokenRequestContext context, bool async, CancellationToken cancellationToken) { - AuthenticationResult result = await Client.AcquireTokenInteractiveAsync(context.Scopes, context.Claims, Prompt.SelectAccount, async, cancellationToken).ConfigureAwait(false); + Prompt prompt = LoginHint switch + { + null => Prompt.SelectAccount, + _ => Prompt.NoPrompt + }; + + AuthenticationResult result = await Client.AcquireTokenInteractiveAsync( + context.Scopes, + context.Claims, + prompt, + LoginHint, + async, + cancellationToken) + .ConfigureAwait(false); Record = new AuthenticationRecord(result, ClientId); diff --git a/sdk/identity/Azure.Identity/src/InteractiveBrowserCredentialOptions.cs b/sdk/identity/Azure.Identity/src/InteractiveBrowserCredentialOptions.cs index 05b3d857abca7..9788e50ab97e4 100644 --- a/sdk/identity/Azure.Identity/src/InteractiveBrowserCredentialOptions.cs +++ b/sdk/identity/Azure.Identity/src/InteractiveBrowserCredentialOptions.cs @@ -48,5 +48,10 @@ public string TenantId /// The captured from a previous authentication. /// public AuthenticationRecord AuthenticationRecord { get; set; } + + /// + /// Avoids the account prompt and pre-populates the username of the account to login. + /// + public string LoginHint { get; set; } } } diff --git a/sdk/identity/Azure.Identity/src/MsalPublicClient.cs b/sdk/identity/Azure.Identity/src/MsalPublicClient.cs index 157defe8394d8..51f08fc01e170 100644 --- a/sdk/identity/Azure.Identity/src/MsalPublicClient.cs +++ b/sdk/identity/Azure.Identity/src/MsalPublicClient.cs @@ -100,7 +100,7 @@ protected virtual async ValueTask AcquireTokenSilentCoreAs .ConfigureAwait(false); } - public async ValueTask AcquireTokenInteractiveAsync(string[] scopes, string claims, Prompt prompt, bool async, CancellationToken cancellationToken) + public async ValueTask AcquireTokenInteractiveAsync(string[] scopes, string claims, Prompt prompt, string loginHint, bool async, CancellationToken cancellationToken) { #pragma warning disable AZC0109 // Misuse of 'async' parameter. if (!async && !IdentityCompatSwitches.DisableInteractiveBrowserThreadpoolExecution) @@ -114,24 +114,33 @@ public async ValueTask AcquireTokenInteractiveAsync(string AzureIdentityEventSource.Singleton.InteractiveAuthenticationExecutingOnThreadPool(); #pragma warning disable AZC0102 // Do not use GetAwaiter().GetResult(). - return Task.Run(async () => await AcquireTokenInteractiveCoreAsync(scopes, claims, prompt, true, cancellationToken).ConfigureAwait(false)).GetAwaiter().GetResult(); + return Task.Run(async () => await AcquireTokenInteractiveCoreAsync(scopes, claims, prompt, loginHint, true, cancellationToken).ConfigureAwait(false)).GetAwaiter().GetResult(); #pragma warning restore AZC0102 // Do not use GetAwaiter().GetResult(). } AzureIdentityEventSource.Singleton.InteractiveAuthenticationExecutingInline(); - return await AcquireTokenInteractiveCoreAsync(scopes, claims, prompt, async, cancellationToken).ConfigureAwait(false); + return await AcquireTokenInteractiveCoreAsync(scopes, claims, prompt, loginHint, async, cancellationToken).ConfigureAwait(false); } - protected virtual async ValueTask AcquireTokenInteractiveCoreAsync(string[] scopes, string claims, Prompt prompt, bool async, CancellationToken cancellationToken) + protected virtual async ValueTask AcquireTokenInteractiveCoreAsync(string[] scopes, string claims, Prompt prompt, string loginHint, bool async, CancellationToken cancellationToken) { IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false); - return await client.AcquireTokenInteractive(scopes) - .WithPrompt(prompt) - .WithClaims(claims) - .ExecuteAsync(async, cancellationToken) - .ConfigureAwait(false); + return loginHint switch + { + null => await client.AcquireTokenInteractive(scopes) + .WithPrompt(prompt) + .WithClaims(claims) + .ExecuteAsync(async, cancellationToken) + .ConfigureAwait(false), + _ => await client.AcquireTokenInteractive(scopes) + .WithPrompt(prompt) + .WithClaims(claims) + .WithLoginHint(loginHint) + .ExecuteAsync(async, cancellationToken) + .ConfigureAwait(false) + }; } public async ValueTask AcquireTokenByUsernamePasswordAsync(string[] scopes, string claims, string username, SecureString password, bool async, CancellationToken cancellationToken) diff --git a/sdk/identity/Azure.Identity/tests/AzureIdentityEventSourceTests.cs b/sdk/identity/Azure.Identity/tests/AzureIdentityEventSourceTests.cs index ffc89323ccc1c..ca9c1d568354e 100644 --- a/sdk/identity/Azure.Identity/tests/AzureIdentityEventSourceTests.cs +++ b/sdk/identity/Azure.Identity/tests/AzureIdentityEventSourceTests.cs @@ -87,7 +87,7 @@ public async Task ValidateDeviceCodeCredentialSucceededEvents() [Test] public async Task ValidateInteractiveBrowserCredentialSucceededEvents() { - var mockMsalClient = new MockMsalPublicClient() { InteractiveAuthFactory = (_) => { return AuthenticationResultFactory.Create(accessToken: Guid.NewGuid().ToString(), expiresOn: DateTimeOffset.UtcNow.AddMinutes(10)); } }; + var mockMsalClient = new MockMsalPublicClient() { AuthFactory = _ => { return AuthenticationResultFactory.Create(accessToken: Guid.NewGuid().ToString(), expiresOn: DateTimeOffset.UtcNow.AddMinutes(10)); } }; var credential = InstrumentClient(new InteractiveBrowserCredential(default, Guid.NewGuid().ToString(), default, default, mockMsalClient)); @@ -159,7 +159,7 @@ public async Task ValidateInteractiveBrowserCredentialFailedEvents() { var expExMessage = Guid.NewGuid().ToString(); - var mockMsalClient = new MockMsalPublicClient() { InteractiveAuthFactory = (_) => throw new MockClientException(expExMessage) }; + var mockMsalClient = new MockMsalPublicClient() { AuthFactory = _ => throw new MockClientException(expExMessage) }; var credential = InstrumentClient(new InteractiveBrowserCredential(Guid.NewGuid().ToString(), Guid.NewGuid().ToString(), default, default, mockMsalClient)); diff --git a/sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialTests.cs b/sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialTests.cs index 027ba227c9d2b..9d1631fb66fd8 100644 --- a/sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialTests.cs +++ b/sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialTests.cs @@ -23,7 +23,7 @@ public async Task InteractiveBrowserAcquireTokenInteractiveException() { string expInnerExMessage = Guid.NewGuid().ToString(); - var mockMsalClient = new MockMsalPublicClient() { InteractiveAuthFactory = (_) => { throw new MockClientException(expInnerExMessage); } }; + var mockMsalClient = new MockMsalPublicClient { AuthFactory = _ => { throw new MockClientException(expInnerExMessage); } }; var credential = InstrumentClient(new InteractiveBrowserCredential(default, "", default, default, mockMsalClient)); @@ -47,8 +47,8 @@ public async Task InteractiveBrowserAcquireTokenSilentException() var mockMsalClient = new MockMsalPublicClient { - InteractiveAuthFactory = (_) => { return AuthenticationResultFactory.Create(accessToken: expToken, expiresOn: expExpiresOn); }, - SilentAuthFactory = (_) => { throw new MockClientException(expInnerExMessage); } + AuthFactory = _ => { return AuthenticationResultFactory.Create(expToken, expiresOn: expExpiresOn); }, + SilentAuthFactory = _ => { throw new MockClientException(expInnerExMessage); } }; var credential = InstrumentClient(new InteractiveBrowserCredential(default, "", default, default, mockMsalClient)); @@ -79,7 +79,7 @@ public async Task InteractiveBrowserRefreshException() var mockMsalClient = new MockMsalPublicClient { - InteractiveAuthFactory = (_) => { return AuthenticationResultFactory.Create(accessToken: expToken, expiresOn: expExpiresOn); }, + AuthFactory = (_) => { return AuthenticationResultFactory.Create(expToken, expiresOn: expExpiresOn); }, SilentAuthFactory = (_) => { throw new MsalUiRequiredException("errorCode", "message"); } }; @@ -91,7 +91,7 @@ public async Task InteractiveBrowserRefreshException() Assert.AreEqual(expExpiresOn, token.ExpiresOn); - mockMsalClient.InteractiveAuthFactory = (_) => { throw new MockClientException(expInnerExMessage); }; + mockMsalClient.AuthFactory = (_) => { throw new MockClientException(expInnerExMessage); }; var ex = Assert.ThrowsAsync(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default))); @@ -143,6 +143,24 @@ public async Task InteractiveBrowserValidateSyncWorkaroundCompatSwitch() await ValidateSyncWorkaroundCompatSwitch(!IsAsync); } + [Test] + public async Task LoginHint([Values(null, "fring@contoso.com")] string loginHint) + { + var mockMsalClient = new MockMsalPublicClient + { + InteractiveAuthFactory = (_, _, prompt, hintArg, _, _) => + { + Assert.AreEqual(loginHint == null ? Prompt.SelectAccount : Prompt.NoPrompt, prompt); + Assert.AreEqual(loginHint, hintArg); + return AuthenticationResultFactory.Create(Guid.NewGuid().ToString(), expiresOn: DateTimeOffset.UtcNow.AddMinutes(5)); + } + }; + var options = new InteractiveBrowserCredentialOptions { LoginHint = loginHint }; + var credential = InstrumentClient(new InteractiveBrowserCredential(default, "", options, default, mockMsalClient)); + + await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)); + } + private async Task ValidateSyncWorkaroundCompatSwitch(bool expectedThreadPoolExecution) { bool threadPoolExec = false; @@ -162,7 +180,7 @@ private async Task ValidateSyncWorkaroundCompatSwitch(bool expectedThreadPoolExe var mockMsalClient = new MockMsalPublicClient { - InteractiveAuthFactory = (_) => { return AuthenticationResultFactory.Create(accessToken: Guid.NewGuid().ToString(), expiresOn: DateTimeOffset.UtcNow.AddMinutes(5)); } + AuthFactory = _ => { return AuthenticationResultFactory.Create(Guid.NewGuid().ToString(), expiresOn: DateTimeOffset.UtcNow.AddMinutes(5)); } }; var credential = InstrumentClient(new InteractiveBrowserCredential(default, "", default, default, mockMsalClient)); diff --git a/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs b/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs index b73fedd47b068..6ddf8afee8698 100644 --- a/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs +++ b/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs @@ -18,7 +18,7 @@ internal class MockMsalPublicClient : MsalPublicClient public Func UserPassAuthFactory { get; set; } - public Func InteractiveAuthFactory { get; set; } + public Func InteractiveAuthFactory { get; set; } public Func SilentAuthFactory { get; set; } @@ -45,13 +45,18 @@ protected override ValueTask AcquireTokenByUsernamePasswor throw new NotImplementedException(); } - protected override ValueTask AcquireTokenInteractiveCoreAsync(string[] scopes, string claims, Prompt prompt, bool async, CancellationToken cancellationToken) + protected override ValueTask AcquireTokenInteractiveCoreAsync(string[] scopes, string claims, Prompt prompt, string loginHint, bool async, CancellationToken cancellationToken) { - Func factory = InteractiveAuthFactory ?? AuthFactory; + var interactiveAuthFactory = InteractiveAuthFactory; + var authFactory = AuthFactory; - if (factory != null) + if (interactiveAuthFactory != null) { - return new ValueTask(factory(scopes)); + return new ValueTask(interactiveAuthFactory(scopes, claims, prompt, loginHint, async, cancellationToken)); + } + if (authFactory != null) + { + return new ValueTask(authFactory(scopes)); } throw new NotImplementedException(); From 8eacd3a1707b682506f306e12a42a043068876d0 Mon Sep 17 00:00:00 2001 From: Christopher Scott Date: Thu, 20 May 2021 09:03:39 -0500 Subject: [PATCH 2/2] add changelog entry --- sdk/identity/Azure.Identity/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/identity/Azure.Identity/CHANGELOG.md b/sdk/identity/Azure.Identity/CHANGELOG.md index a4b959aabee2b..91676622756ad 100644 --- a/sdk/identity/Azure.Identity/CHANGELOG.md +++ b/sdk/identity/Azure.Identity/CHANGELOG.md @@ -2,6 +2,9 @@ ## 1.5.0-beta.1 (Unreleased) +### Fixes and improvements + +- Added `LoginHint` property to `InteractiveBrowserCredentialOptions` which allows a user name to be pre-selected for interactive logins. Setting this option skips the account selection prompt and immediately attempts to login with the specified account. ## 1.4.0 (2021-05-12)