diff --git a/sdk/identity/Azure.Identity/CHANGELOG.md b/sdk/identity/Azure.Identity/CHANGELOG.md index 4560ede8f3d36..eec46bff925e0 100644 --- a/sdk/identity/Azure.Identity/CHANGELOG.md +++ b/sdk/identity/Azure.Identity/CHANGELOG.md @@ -1,9 +1,9 @@ # Release History -## 1.12.0-beta.1 (2024-04-17) +## 1.11.2 (2024-04-19) -### Other Changes -- An experimental overload `Authenticate` method on `InteractiveBrowserCredential` now supports the experimental `PopTokenRequestContext` parameter. +### Bugs Fixed +- Fixed an issue which caused claims to be incorrectly added to confidential client credentials such as `DeviceCodeCredential` [#43468](https://github.com/Azure/azure-sdk-for-net/issues/43468) ## 1.11.1 (2024-04-16) 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 ef4c6170e0ba5..c6a9457b46950 100644 --- a/sdk/identity/Azure.Identity/api/Azure.Identity.netstandard2.0.cs +++ b/sdk/identity/Azure.Identity/api/Azure.Identity.netstandard2.0.cs @@ -261,7 +261,7 @@ public static partial class IdentityModelFactory public static Azure.Identity.AuthenticationRecord AuthenticationRecord(string username, string authority, string homeAccountId, string tenantId, string clientId) { throw null; } public static Azure.Identity.DeviceCodeInfo DeviceCodeInfo(string userCode, string deviceCode, System.Uri verificationUri, System.DateTimeOffset expiresOn, string message, string clientId, System.Collections.Generic.IReadOnlyCollection scopes) { throw null; } } - public partial class InteractiveBrowserCredential : Azure.Core.TokenCredential, Azure.Core.ISupportsProofOfPossession + public partial class InteractiveBrowserCredential : Azure.Core.TokenCredential { public InteractiveBrowserCredential() { } public InteractiveBrowserCredential(Azure.Identity.InteractiveBrowserCredentialOptions options) { } @@ -269,15 +269,11 @@ public InteractiveBrowserCredential(Azure.Identity.InteractiveBrowserCredentialO public InteractiveBrowserCredential(string clientId) { } [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] public InteractiveBrowserCredential(string tenantId, string clientId, Azure.Identity.TokenCredentialOptions options = null) { } - public virtual Azure.Identity.AuthenticationRecord Authenticate(Azure.Core.PopTokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public virtual Azure.Identity.AuthenticationRecord Authenticate(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public virtual Azure.Identity.AuthenticationRecord Authenticate(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } - public virtual System.Threading.Tasks.Task AuthenticateAsync(Azure.Core.PopTokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public virtual System.Threading.Tasks.Task AuthenticateAsync(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public virtual System.Threading.Tasks.Task AuthenticateAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } - public Azure.Core.AccessToken GetToken(Azure.Core.PopTokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public override Azure.Core.AccessToken GetToken(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } - public System.Threading.Tasks.ValueTask GetTokenAsync(Azure.Core.PopTokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } public override System.Threading.Tasks.ValueTask GetTokenAsync(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } } public partial class InteractiveBrowserCredentialOptions : Azure.Identity.TokenCredentialOptions diff --git a/sdk/identity/Azure.Identity/src/Azure.Identity.csproj b/sdk/identity/Azure.Identity/src/Azure.Identity.csproj index 60878553a13ca..2c56219422ebf 100644 --- a/sdk/identity/Azure.Identity/src/Azure.Identity.csproj +++ b/sdk/identity/Azure.Identity/src/Azure.Identity.csproj @@ -2,7 +2,7 @@ This is the implementation of the Azure SDK Client Library for Azure Identity Microsoft Azure.Identity Component - 1.12.0-beta.1 + 1.11.2 1.11.1 Microsoft Azure Identity;$(PackageCommonTags) diff --git a/sdk/identity/Azure.Identity/src/Credentials/AuthorizationCodeCredential.cs b/sdk/identity/Azure.Identity/src/Credentials/AuthorizationCodeCredential.cs index 1b51b3f721319..cfa97718e60aa 100644 --- a/sdk/identity/Azure.Identity/src/Credentials/AuthorizationCodeCredential.cs +++ b/sdk/identity/Azure.Identity/src/Credentials/AuthorizationCodeCredential.cs @@ -181,7 +181,15 @@ private async ValueTask GetTokenImplAsync(bool async, TokenRequestC private async Task AcquireTokenWithCode(bool async, TokenRequestContext requestContext, AccessToken token, string tenantId, CancellationToken cancellationToken) { AuthenticationResult result = await Client - .AcquireTokenByAuthorizationCodeAsync(requestContext.Scopes, _authCode, tenantId, _redirectUri, requestContext.Claims, requestContext.IsCaeEnabled, async, cancellationToken) + .AcquireTokenByAuthorizationCodeAsync( + scopes: requestContext.Scopes, + code: _authCode, + tenantId: tenantId, + redirectUri: _redirectUri, + claims: requestContext.Claims, + enableCae: requestContext.IsCaeEnabled, + async, + cancellationToken) .ConfigureAwait(false); _record = new AuthenticationRecord(result, _clientId); token = new AccessToken(result.AccessToken, result.ExpiresOn); diff --git a/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs b/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs index 392e824d316b5..692789472f023 100644 --- a/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs +++ b/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs @@ -220,7 +220,7 @@ public virtual async ValueTask AcquireTokenSilentCoreAsync }; builder.WithTenantIdFromAuthority(uriBuilder.Uri); } - if (string.IsNullOrEmpty(claims)) + if (!string.IsNullOrEmpty(claims)) { builder.WithClaims(claims); } @@ -239,7 +239,7 @@ public virtual async ValueTask AcquireTokenByAuthorization bool async, CancellationToken cancellationToken) { - var result = await AcquireTokenByAuthorizationCodeCoreAsync(scopes, code, tenantId, redirectUri, claims, enableCae, async, cancellationToken).ConfigureAwait(false); + var result = await AcquireTokenByAuthorizationCodeCoreAsync(scopes: scopes, code: code, tenantId: tenantId, redirectUri: redirectUri, claims: claims, enableCae: enableCae, async, cancellationToken).ConfigureAwait(false); LogAccountDetails(result); return result; } @@ -248,8 +248,8 @@ public virtual async ValueTask AcquireTokenByAuthorization string[] scopes, string code, string tenantId, - string claims, string redirectUri, + string claims, bool enableCae, bool async, CancellationToken cancellationToken) @@ -312,6 +312,10 @@ public virtual async ValueTask AcquireTokenOnBehalfOfCoreA }; builder.WithTenantIdFromAuthority(uriBuilder.Uri); } + if (!string.IsNullOrEmpty(claims)) + { + builder.WithClaims(claims); + } return await builder .ExecuteAsync(async, cancellationToken) .ConfigureAwait(false); diff --git a/sdk/identity/Azure.Identity/tests/AuthorizationCodeCredentialTests.cs b/sdk/identity/Azure.Identity/tests/AuthorizationCodeCredentialTests.cs index e678f9a715471..85c8912ee6eab 100644 --- a/sdk/identity/Azure.Identity/tests/AuthorizationCodeCredentialTests.cs +++ b/sdk/identity/Azure.Identity/tests/AuthorizationCodeCredentialTests.cs @@ -33,6 +33,7 @@ public override TokenCredential GetTokenCredential(CommonCredentialTestConfig co DisableInstanceDiscovery = config.DisableInstanceDiscovery, AdditionallyAllowedTenants = config.AdditionallyAllowedTenants, IsUnsafeSupportLoggingEnabled = config.IsUnsafeSupportLoggingEnabled, + RedirectUri = config.RedirectUri, }; if (config.Transport != null) { diff --git a/sdk/identity/Azure.Identity/tests/Azure.Identity.Tests.csproj b/sdk/identity/Azure.Identity/tests/Azure.Identity.Tests.csproj index 6247e5a4a3432..df39b2059c467 100644 --- a/sdk/identity/Azure.Identity/tests/Azure.Identity.Tests.csproj +++ b/sdk/identity/Azure.Identity/tests/Azure.Identity.Tests.csproj @@ -16,9 +16,9 @@ - + diff --git a/sdk/identity/Azure.Identity/tests/CredentialTestBase.cs b/sdk/identity/Azure.Identity/tests/CredentialTestBase.cs index 5f3d73ddbb639..d64f1d84a36ca 100644 --- a/sdk/identity/Azure.Identity/tests/CredentialTestBase.cs +++ b/sdk/identity/Azure.Identity/tests/CredentialTestBase.cs @@ -355,6 +355,92 @@ public async Task EnableCae() Assert.True(observedNoCae); } + [Test] + public async Task ClaimsSetCorrectlyOnRequest() + { + // Configure the transport + var token = Guid.NewGuid().ToString(); + var idToken = CredentialTestHelpers.CreateMsalIdToken(Guid.NewGuid().ToString(), "userName", TenantId); + bool calledDiscoveryEndpoint = false; + bool isPubClient = false; + const string Claims = "myClaims"; + + var mockTransport = new MockTransport(req => + { + calledDiscoveryEndpoint |= req.Uri.Path.Contains("discovery/instance"); + + MockResponse response = new(200); + if (req.Uri.Path.EndsWith("/devicecode")) + { + response = CredentialTestHelpers.CreateMockMsalDeviceCodeResponse(); + } + else if (req.Uri.Path.Contains("/userrealm/")) + { + response.SetContent(UserrealmResponse); + } + else + { + if (isPubClient || typeof(TCredOptions) == typeof(AuthorizationCodeCredentialOptions) || typeof(TCredOptions) == typeof(OnBehalfOfCredentialOptions)) + { + response = CredentialTestHelpers.CreateMockMsalTokenResponse(200, token, TenantId, ExpectedUsername, ObjectId); + } + else + { + response.SetContent($"{{\"token_type\": \"Bearer\",\"expires_in\": 9999,\"ext_expires_in\": 9999,\"access_token\": \"{token}\" }}"); + } + if (req.Content != null) + { + var stream = new MemoryStream(); + req.Content.WriteTo(stream, default); + var content = new BinaryData(stream.ToArray()).ToString(); + var queryString = Uri.UnescapeDataString(content) + .Split('&') + .Select(q => q.Split('=')) + .ToDictionary(kvp => kvp[0], kvp => kvp[1]); + bool containsClaims = queryString.TryGetValue("claims", out var claimsJson); + + if (req.ClientRequestId == "NoClaims") + { + Assert.False(containsClaims, "(NoClaims) Claims should not be present. Claims=" + claimsJson); + } + if (req.ClientRequestId == "WithClaims") + { + Assert.True(containsClaims, "(WithClaims) Claims should be present"); + Assert.AreEqual(Claims, claimsJson, "(WithClaims) Claims should match"); + } + } + } + + return response; + }); + + var config = new CommonCredentialTestConfig() + { + Transport = mockTransport, + TenantId = TenantId, + RedirectUri = new Uri("http://localhost:8400/") + }; + var credential = GetTokenCredential(config); + if (!CredentialTestHelpers.IsMsalCredential(credential)) + { + Assert.Ignore("EnableCAE tests do not apply to the non-MSAL credentials."); + } + isPubClient = CredentialTestHelpers.IsCredentialTypePubClient(credential); + + using (HttpPipeline.CreateClientRequestIdScope("NoClaims")) + { + // First call to populate the account record for confidential client creds + await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default), default); + var actualToken = await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Alternate), default); + Assert.AreEqual(token, actualToken.Token); + } + using (HttpPipeline.CreateClientRequestIdScope("WithClaims")) + { + var actualToken = await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Alternate2, claims: Claims), default); + Assert.AreEqual(token, actualToken.Token); + } + } + [Test] public async Task TokenRequestContextClaimsPassedToMSAL() { @@ -624,6 +710,7 @@ public class CommonCredentialTestConfig : TokenCredentialOptions, ISupportsAddit public TokenRequestContext RequestContext { get; set; } public string TenantId { get; set; } public IList AdditionallyAllowedTenants { get; set; } = new List(); + public Uri RedirectUri { get; set; } internal TenantIdResolverBase TestTentantIdResolver { get; set; } internal MockMsalConfidentialClient MockConfidentialMsalClient { get; set; } internal MockMsalPublicClient MockPublicMsalClient { get; set; } diff --git a/sdk/identity/Azure.Identity/tests/Mock/MockScopes.cs b/sdk/identity/Azure.Identity/tests/Mock/MockScopes.cs index 710f42a72faa0..6256b3ce8637a 100644 --- a/sdk/identity/Azure.Identity/tests/Mock/MockScopes.cs +++ b/sdk/identity/Azure.Identity/tests/Mock/MockScopes.cs @@ -19,6 +19,7 @@ private MockScopes(string[] scopes) public static MockScopes Default = new MockScopes(new string[] { "https://default.mock.auth.scope/.default" }); public static MockScopes Alternate = new MockScopes(new string[] { "https://alternate.mock.auth.scope/.default" }); + public static MockScopes Alternate2 = new MockScopes(new string[] { "https://alternate2.mock.auth.scope/.default" }); public override string ToString() {