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.

Sign up for GitHub

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

Jump to bottom

Ignore region outside client_credentials flow #3023

Merged
merged 5 commits into from
Nov 18, 2021
Merged

Ignore region outside client_credentials flow #3023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
485b8a5
skip region when doing OBO
jennyf19 Nov 18, 2021
4954f99
only use for AT for client
jennyf19 Nov 18, 2021
db67200
set apiId to AcquireTokenForClient for regional tests
jennyf19 Nov 18, 2021
34be1f1
Consolidate some obo tests (no code change)
bgavrilMS Nov 18, 2021
1dd398a
Unit and integration
bgavrilMS Nov 18, 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
9 changes: 7 additions & 2 deletions src/client/Microsoft.Identity.Client/Instance/Discovery/RegionDiscoveryProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@ public RegionDiscoveryProvider(IHttpManager httpManager, bool clearCache)

public async Task<InstanceDiscoveryMetadataEntry> GetMetadataAsync(Uri authority, RequestContext requestContext)
{
string region = await _regionManager.GetAzureRegionAsync(requestContext).ConfigureAwait(false);
string region = null;
if (requestContext.ApiEvent?.ApiId == TelemetryCore.Internal.Events.ApiEvent.ApiIds.AcquireTokenForClient)
{
region = await _regionManager.GetAzureRegionAsync(requestContext).ConfigureAwait(false);
}

if (string.IsNullOrEmpty(region))
{
requestContext.Logger.Info("[Region discovery] Azure region was not configured or could not be discovered. Not using a regional authority.");
requestContext.Logger.Info("[Region discovery] Not using a regional authority. ");
return null;
}

Expand Down
7 changes: 3 additions & 4 deletions src/client/Microsoft.Identity.Client/TelemetryCore/Internal/Events/ApiEvent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
using Microsoft.Identity.Client.TelemetryCore.Internal.Constants;
using Microsoft.Identity.Client.PlatformsCommon.Interfaces;
using Microsoft.Identity.Client.Region;
using Microsoft.Identity.Client;
using System.Diagnostics;

namespace Microsoft.Identity.Client.TelemetryCore.Internal.Events
{
Expand Down Expand Up @@ -78,13 +76,14 @@ public ApiTelemetryId ApiTelemId

public ApiIds ApiId
{
get => (ApiIds)Enum.Parse(typeof(ApiIds), this[MsalTelemetryBlobEventNames.ApiIdConstStrKey]);
get => TryGetValue(MsalTelemetryBlobEventNames.ApiIdConstStrKey, out string apiIdString) ? (ApiIds)Enum.Parse(typeof(ApiIds), apiIdString) : ApiIds.None;

set => this[MsalTelemetryBlobEventNames.ApiIdConstStrKey] = ((int) value).ToString(CultureInfo.InvariantCulture);
}

public string ApiIdString
{
get => this.ContainsKey(MsalTelemetryBlobEventNames.ApiIdConstStrKey) ?
get => ContainsKey(MsalTelemetryBlobEventNames.ApiIdConstStrKey) ?
this[MsalTelemetryBlobEventNames.ApiIdConstStrKey] :
null;
}
Expand Down
12 changes: 8 additions & 4 deletions tests/Microsoft.Identity.Test.Integration.netfx/HeadlessTests/OboTests2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,6 @@ public void TestInitialize()
if (_keyVault == null)
{
_keyVault = new KeyVaultSecretsProvider();
// s_publicCloudCcaSecret = _keyVault.GetSecret(TestConstants.MsalCCAKeyVaultUri).Value;
// s_arlingtonCCASecret = _keyVault.GetSecret(TestConstants.MsalArlingtonCCAKeyVaultUri).Value;
}
}

Expand Down Expand Up @@ -333,17 +331,19 @@ private async Task RunOnBehalfOfTestAsync(LabResponse labResponse)

var msalPublicClient = PublicClientApplicationBuilder.Create(publicClientID)
.WithAuthority(authority)
.WithRedirectUri(TestConstants.RedirectUri)
.WithRedirectUri(TestConstants.RedirectUri)
.WithTestLogging()
.Build();

var authResult = await msalPublicClient.AcquireTokenByUsernamePassword(oboScope, user.Upn, securePassword)
.ExecuteAsync()
.ConfigureAwait(false);

var ccaAuthority = new Uri(oboHost + authResult.TenantId);
var confidentialApp = ConfidentialClientApplicationBuilder
.Create(confidentialClientID)
.WithAuthority(new Uri(oboHost + authResult.TenantId), true)
.WithAuthority(ccaAuthority, true)
.WithAzureRegion(TestConstants.Region) // should be ignored by OBO
.WithClientSecret(secret)
.WithTestLogging()
.Build();
Expand All @@ -360,6 +360,10 @@ private async Task RunOnBehalfOfTestAsync(LabResponse labResponse)

MsalAssert.AssertAuthResult(authResult, user);
Assert.AreEqual(atHash, userCacheRecorder.LastAfterAccessNotificationArgs.SuggestedCacheKey);
Assert.AreEqual(
ccaAuthority.ToString() + "/oauth2/v2.0/token",
authResult.AuthenticationResultMetadata.TokenEndpoint,
"OBO does not obey region");

#pragma warning disable CS0618 // Type or member is obsolete
await confidentialApp.GetAccountsAsync().ConfigureAwait(false);
Expand Down
1 change: 1 addition & 0 deletions tests/Microsoft.Identity.Test.Unit/CoreTests/RegionDiscoveryProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public override void TestInitialize()
_harness.ServiceBundle.ApplicationLogger,
_harness.ServiceBundle.PlatformProxy.CryptographyManager,
Guid.NewGuid().AsMatsCorrelationId());
_apiEvent.ApiId = ApiEvent.ApiIds.AcquireTokenForClient;
_testRequestContext.ApiEvent = _apiEvent;
_regionDiscoveryProvider = new RegionDiscoveryProvider(_httpManager, true);
}
Expand Down
30 changes: 29 additions & 1 deletion tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,34 @@ await app.AcquireTokenByAuthorizationCode(TestConstants.s_scope, TestConstants.D
}
}

[DataTestMethod]
[DataRow(true)]
[DataRow(false)]
public async Task AcquireTokenByAuthorizationCode_IgnoresRegion_Async(bool autodetectRegion)
{
using (var httpManager = new MockHttpManager())
{
// MSAL should not auto-detect, but if it does, this test should fail because a call to IMDS is configured
string region = autodetectRegion ? ConfidentialClientApplication.AttemptRegionDiscovery : TestConstants.Region;

var app = ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId)
.WithClientSecret(TestConstants.ClientSecret)
.WithAzureRegion(region)
.WithHttpManager(httpManager)
.BuildConcrete();

httpManager.AddInstanceDiscoveryMockHandler();
httpManager.AddSuccessTokenResponseMockHandlerForPost();

var result = await app.AcquireTokenByAuthorizationCode(TestConstants.s_scope, TestConstants.DefaultAuthorizationCode)
.ExecuteAsync()
.ConfigureAwait(false);

Assert.AreEqual(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource);
Assert.AreEqual("https://login.microsoftonline.com/common/oauth2/v2.0/token", result.AuthenticationResultMetadata.TokenEndpoint);
}
}

[TestMethod]
public async Task GetAuthorizationRequestUrlDuplicateParamsTestAsync()
{
Expand Down Expand Up @@ -1504,7 +1532,7 @@ public async Task ConfidentialClientSuggestedExpiryAsync()
CoreAssert.IsWithinRange(
DateTimeOffset.UtcNow + TimeSpan.FromSeconds(3600),
result.ExpiresOn,
TimeSpan.FromSeconds(5));
TimeSpan.FromSeconds(5));

// Add second token with shorter expiration time
httpManager.AddMockHandlerSuccessfulClientCredentialTokenResponseMessage(expiresIn: "1800");
Expand Down
Loading