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

[Feature Request] Id.Web supports CIAM custom user domains #2690

Closed
jennyf19 opened this issue Mar 4, 2024 · 16 comments · Fixed by #2696
Closed

[Feature Request] Id.Web supports CIAM custom user domains #2690

jennyf19 opened this issue Mar 4, 2024 · 16 comments · Fixed by #2696
Assignees
Labels
enhancement New feature or request feature request

Comments

@jennyf19
Copy link
Collaborator

jennyf19 commented Mar 4, 2024

API experience

Add support for CIAM CUD authorities. See https://microsoft-my.sharepoint-df.com/:w:/p/jmprieur/EbtMcuWkuyRKnWTR8Fg9EAsBMn22Sy5Kni6YWOxTfYWjtg?e=GGad0r for spec

"AzureAd": {
    "ClientId": "12345-cdd4-46d4-9e68-db03240b4baa",      
    "Authority": "https://cats.ciamextensibility.com/111111-43bb-4ff9-89af-30ed8fe31c6d/v2.0"
},

Technical details

In MergedOptions:

  • Add a new boolean property named PreserveAuthority
  • in MergedOptions.ParseAuthorityIfNecessary, only set the mergedOptions.TenantId if mergedOptions.PreserveAuthority is false (as MSAL.NET does not want a tenantId when .WithOidcAuthority is used)

In AuthorityHelper.BuildCiamAuthorityIfNeeded, have a new out bool parameter preserveAuthority, which will be set to false if the authority is a CiamLogin.com authority and otherwise to true

In MicrosoftIdentityWebApiAuthenticationBuilderExtensions.cs and WebAppExtensions\MicrosoftIdentityWebAppAuthenticationBuilderExtensions.cs, after calling AuthorityHelper.BuildCiamAuthorityIfNeeded, set mergedOptions.PreserveAuthority based on the value of the out parameter.

In TokenAcquisition.BuildConfidentialClientApplicationAsync()

  • if mergedOptions.PreserveAuthority is true, set the authority to mergedOptions.Authority and call builder.WithOidcAuthority(authority),
    otherwise do as today (WithAuthority, and WithB2CAuthority)

Need to MSAL 4.60.0-preview to get builder.WithOidcAuthority(authority)

Testing resources

MSAL 4.60.0-preview and a CIAM CUD test tenant can be found at https://microsofteur-my.sharepoint.com/:f:/g/personal/bogavril_microsoft_com/EoEwmcgN3oJAplznhkE-OosBAQc4xl7I2sNVC8TfDFR_JA?e=8M82R9

CIAM CUD is not currently available in the Lab.

@jennyf19 jennyf19 added enhancement New feature or request feature request labels Mar 4, 2024
@bgavrilMS
Copy link
Member

bgavrilMS commented Mar 4, 2024

Hi @jennyf19 - would you like MSAL to drop its restriction on setting the tenant for OIDC authorities? This could help ID.Web looks for the tid claim in the client token and uses WithTenantId when performing OBO. https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/main/tests/Microsoft.Identity.Test.Unit/ApiConfigTests/AuthorityTests.cs#L179

I think the web api use of WithTenantId only makes sense for AAD's authority which end in "common" or "organization" anyway.

@bgavrilMS
Copy link
Member

bgavrilMS commented Mar 4, 2024

I'd like to add a few acceptance tests to see if this matches my understanding. Can you please review this @jennyf19

Authority type Id.Web config OIDC document MSAL API MSAL example
AAD common Instance: https://login.microsoftonline.com Tenant: common https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration WithAuthority WithAuthority("https://login.microsoftonline.com/common")
AAD tenanted Instance: https://login.microsoftonline.com/ Tenant: 12345 https://login.microsoftonline.com/12345/v2.0/.well-known/openid-configuration WithAuthority WithAuthority("https://login.microsoftonline.com/12345")
non-public AAD common Instance: https://login.windows-ppe.net Tenant: common https://login.windows-ppe.net/common/v2.0/.well-known/openid-configuration WithAuthority WithAuthority("https://login.windows-ppe.net/common")
B2C ?? https://cats.b2clogin.com/cats.onmicrosoft.com/B2C_1_signupsignin1/v2.0/.well-known/openid-configuration WithB2CAuthority WithB2CAuthority("https://cats.b2clogin.com/cats.onmicrosoft.com/tfp/B2C_1_signupsignin1")
B2C custom authority ?? WithB2CAuthority
B2C custom authority with v2 ?? WithB2CAuthority
CIAM non-CUD Authority: https://idgciamdemo.ciamlogin.com https://idgciamdemo.ciamlogin.com/idgciamdemo.onmicrosoft.com/v2.0/.well-known/openid-configuration WithAuthority WithAuthority("https://idgciamdemo.ciamlogin.com/") or WithAuthority("https://idgciamdemo.ciamlogin.com/idgciamdemo.onmicrosoft.com")
CIAM CUD (new!) Authority: https://cats.ciamextensibility.com/111111-43bb-4ff9-89af-30ed8fe31c6d/v2.0 OIDC: https://cats.ciamextensibility.com/111111-43bb-4ff9-89af-30ed8fe31c6d/v2.0/.well-known/openid-configuration WithOidcAuthority WithOidcAuthority("https://cats.ciamextensibility.com/111111-43bb-4ff9-89af-30ed8fe31c6d/v2.0")
dSTS ? WithOidcAuthority ?
ADFS (not supported) ? WithOidcAuthority ?

@westin-m westin-m self-assigned this Mar 4, 2024
@bgavrilMS bgavrilMS changed the title [Feature Request] support CIAM custom user domains [Feature Request] Id.Web supports CIAM custom user domains Mar 6, 2024
@jmprieur
Copy link
Collaborator

jmprieur commented Mar 6, 2024

  • B2C uses Instance and Domain, plus the default policy
  • B2C custom authority are not supported
  • dSTS: See the MISE doc

@westin-m westin-m linked a pull request Mar 8, 2024 that will close this issue
@benjaminclewis
Copy link

This change is breaking my AAD-tenanted auth scenario, which now complains that it can't use TenantId with "Generic" authority.

@jmprieur
Copy link
Collaborator

jmprieur commented Apr 4, 2024

@benjaminclewis : when you use Authority, don't use Tenant. If you want to use Tenant, use Instance.
See #2741

@benjaminclewis
Copy link

I wasn't setting Authority, at least not in my appsettings or anywhere explicitly that I'm aware of. I can go back and double check when I have a moment but I worked around my issue by rolling back to 2.17.1 for now.

@benjaminclewis
Copy link

benjaminclewis commented Apr 4, 2024

I've confirmed I'm not setting Authority, it appears to be getting set in internal Microsoft.Identity.Web code, in MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityApplicationOptions, which gets it as (Instance?.TrimEnd('/') + "/" + TenantId + "/v2.0") if it's null in MicrosoftIdentityApplicationOptions.

@jmprieur
Copy link
Collaborator

jmprieur commented Apr 4, 2024

@benjaminclewis : would you mind sharing your appsettings.json or AddMicrosoftWebXXX code?

@benjaminclewis
Copy link

benjaminclewis commented Apr 4, 2024

"AzureAd": {
	"ClientId": "<CLIENT_ID>",
	"Instance": "https://login.microsoftonline.com/",
	"TenantId": "<TENANT_ID>",
	"Audience": "api://<TENANT_ID>/MyClientApp",
	"ClientSecret": "<CLIENT_SECRET>",
	"RedirectUri": "https://localhost:6001"
}

@benjaminclewis
Copy link

builder.Services
	.AddMicrosoftIdentityWebApiAuthentication(builder.Configuration, Constants.AzureAd, "Bearer")
	.EnableTokenAcquisitionToCallDownstreamApi()
	.AddInMemoryTokenCaches();

@benjaminclewis
Copy link

benjaminclewis commented Apr 4, 2024

I get the error when calling either tokenAcquisition.GetAccessTokenForAppAsync(scope) or tokenAcquisition.GetAccessTokenForUserAsync(scopes) from an endpoint handler.

(But I do NOT get the error when I call tokenAcquisition.GetAccessTokenForAppAsync(scope) in a hosted service that runs at startup)

@jmprieur
Copy link
Collaborator

jmprieur commented Apr 4, 2024

would you have a small repro?

@benjaminclewis
Copy link

Sorry, not immediately, but if I get some time I can try to cook one up.

@benjaminclewis
Copy link

benjaminclewis commented Apr 4, 2024

Okay, I've made a smaller project that reproduces the issues, and in doing so I've noticed that the issue is only occurring if I call ITokenAcquisition method(s) first in my hosted service (which works) and then in the endpoint handler (which fails). If I don't start the hosted service the error does not happen in the endpoint handler.
TokenAcquisitionIssueRepro.zip

If you'd prefer some other format let me know. Note this one requires you have azure app registrations set up for the web api and the downstream service, and needs to have appsettings updated accordingly.

@almostjulian
Copy link

almostjulian commented Apr 9, 2024

FWIW, I get the "can't use TenantId with "Generic" authority." when using a GraphSerivceClient via Microsoft.Identity.Web.GraphServiceClient (making calls with my app token)

e.g.

services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
    .AddMicrosoftIdentityWebApi(Configuration.GetSection("AzureAd"))
    .EnableTokenAcquisitionToCallDownstreamApi()
    .AddInMemoryTokenCaches();

    services.AddMicrosoftGraph(options =>
    {
        options.RequestAppToken = true;
    });

my appsettings AzureAD section:

"AzureAd": {
"ClientId": "[clientId]",
"TenantId": "[tenantid]",
"ClientSecret": "[clientsecret]",
"Instance": "https://login.microsoftonline.com/",
"Audience": "api://[myApiUri]",
}

2.17.1 does not have this issue. Removing tenantId and including it in the instance doesn't help either.

@benjaminclewis
Copy link

This problem is still occurring with latest versions, I still have to use the old version 2.17.1 for my project to run. I suppose I should file a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants