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

[Bug] WithAuthority override ignored for GetAuthorizationRequestUrl #2929

Closed
1 of 7 tasks
michaeltreynolds opened this issue Oct 2, 2021 · 3 comments · Fixed by #3946
Closed
1 of 7 tasks

[Bug] WithAuthority override ignored for GetAuthorizationRequestUrl #2929

michaeltreynolds opened this issue Oct 2, 2021 · 3 comments · Fixed by #3946

Comments

@michaeltreynolds
Copy link

michaeltreynolds commented Oct 2, 2021

Which version of MSAL.NET are you using?
Latest

Platform
.NET 4.7.2

What authentication flow has the issue?

  • Desktop / Mobile
    • Interactive
    • Integrated Windows Authentication
    • Username Password
    • Device code flow (browserless)
  • Web app
    • Authorization code
    • On-Behalf-Of
  • Daemon app
    • Service to Service calls

Other?

Is this a new or existing app?
Existing app new scenario within the app.

Repro
We setup an app to use the "common" endpoint. However, there is a scenario where we want to silently get their consumers* login and we create the following URL:

        Uri authorizationRequestUrl = confidentialClientApplication
            .GetAuthorizationRequestUrl(new List<string> { BaseScope})
            .WithExtraQueryParameters(extraQueryParameters)
            .WithLoginHint(identity.Email) // Note email is primary sign in name (email or phone)
            .WithAuthority(AzureCloudInstance.AzurePublic, "consumers")
            .WithRedirectUri(GetCompleteSignInUrl(authorizationContext.HttpContext.Request.Url.Host).ToString())
            .ExecuteAsync()
            .ConfigureAwait(false)
            .GetAwaiter()
            .GetResult();

Also tried:

        Uri authorizationRequestUrl = confidentialClientApplication
            .GetAuthorizationRequestUrl(new List<string> { BaseScope})
            .WithExtraQueryParameters(extraQueryParameters)
            .WithLoginHint(identity.Email) // Note email is primary sign in name (email or phone)
            .WithAuthority(AadAuthorityAudience.PersonalMicrosoftAccount)
            .WithRedirectUri(GetCompleteSignInUrl(authorizationContext.HttpContext.Request.Url.Host).ToString())
            .ExecuteAsync()
            .ConfigureAwait(false)
            .GetAwaiter()
            .GetResult();

Expected behavior
Expected this to generate a URL for the consumers endpoint.

Actual behavior
Generates URL for the common endpoint.

Possible solution
Perhaps the options specified here are not being used if provided originally in the app? Maybe support this or throw if it isn't supported.

Additional context / logs / screenshots
So the app was originally setup with 'common' via code similar to this:

	ConfidentialClientApplicationOptions applicationOptions;
	applicationOptions = new ConfidentialClientApplicationOptions();
	applicationOptions.ClientId = "fakeId";
	applicationOptions.AadAuthorityAudience = AadAuthorityAudience.AzureAdAndPersonalMicrosoftAccount;
	applicationOptions.Instance = "https://login.microsoftonline.com/common";
	applicationOptions.RedirectUri = "https://example.com";

	var confidentialClientApplicationBuilder = ConfidentialClientApplicationBuilder.CreateWithApplicationOptions(applicationOptions)
		.WithCertificate(someCert);
@bgavrilMS
Copy link
Member

Yes, good bug. MSAL ignores the WithAuthority at the request level for GetAuthorizationRequestUrl

The workaround for now is to specify WithAuthority at the app level, smth like:

 var app = ConfidentialClientApplicationBuilder
                   .Create(TestConstants.ClientId)
                   .WithClientSecret("secret")
                   .WithAuthority(AzureCloudInstance.AzurePublic, "consumers")
                   .Build();

@bgavrilMS bgavrilMS added this to the 4.38.0 milestone Oct 4, 2021
@bgavrilMS bgavrilMS changed the title [Bug] [Bug] WithAuthority override ignored for GetAuthorizationRequestUrl Oct 4, 2021
@michaeltreynolds
Copy link
Author

michaeltreynolds commented Oct 6, 2021

In response to:

The workaround for now is to specify WithAuthority at the app level, smth like:

 var app = ConfidentialClientApplicationBuilder
                   .Create(TestConstants.ClientId)
                   .WithClientSecret("secret")
                   .WithAuthority(AzureCloudInstance.AzurePublic, "consumers")
                   .Build();

This workaround does not seem to work for our case. We actually use the "Instance" property to integrate with the PPE version of this as well. I find that I cannot both use the "Instance" property and specify "consumers". I'll continue to use a string.Replace on the resulting URL for my workaround.

loginUri = loginUri.Replace("/common/oauth2/v2.0/authorize", "/consumers/oauth2/v2.0/authorize")

The following did not work for me:

            ConfidentialClientApplicationOptions applicationOptions;
            applicationOptions = new ConfidentialClientApplicationOptions();
            applicationOptions.ClientId = msalAuthenticationConfig.ClientId;
            applicationOptions.AadAuthorityAudience = AadAuthorityAudience.PersonalMicrosoftAccount;
            applicationOptions.Instance = msalAuthenticationConfig.STSInstance; // Can be "-ppe" or regular url.

            var confidentialClientApplicationBuilder = ConfidentialClientApplicationBuilder.CreateWithApplicationOptions(applicationOptions)
                    .WithCertificate(certificate);

@trwalke trwalke modified the milestones: 4.38.0, 4.39.0 Nov 19, 2021
@pmaytak pmaytak modified the milestones: 4.39.0, 4.40.0 Nov 29, 2021
@pmaytak pmaytak removed this from the 4.40.0 milestone Jan 11, 2022
@bgavrilMS bgavrilMS added this to the 4.41.0 milestone Jan 13, 2022
@gladjohn gladjohn self-assigned this Jan 28, 2022
@gladjohn gladjohn modified the milestones: 4.41.0, 4.42.0 Feb 8, 2022
@bgavrilMS bgavrilMS modified the milestones: Future Minor Version, 4.50.0 Feb 1, 2023
@SameerK-MSFT SameerK-MSFT self-assigned this Feb 1, 2023
@pmaytak pmaytak linked a pull request Feb 10, 2023 that will close this issue
1 task
@bgavrilMS
Copy link
Member

Fixed. Will release with MSAL 4.50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment