AzureAD / microsoft-identity-web

Helps creating protected web apps and web APIs with Microsoft identity platform and Azure AD B2C
MIT License
682 stars 213 forks source link

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

Closed jennyf19 closed 7 months ago

jennyf19 commented 8 months ago

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:

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()

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.

bgavrilMS commented 8 months ago

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 commented 8 months ago

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 ?
jmprieur commented 8 months ago
benjaminclewis commented 7 months ago

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

jmprieur commented 7 months ago

@benjaminclewis : when you use Authority, don't use Tenant. If you want to use Tenant, use Instance. See https://github.com/AzureAD/microsoft-identity-web/issues/2741

benjaminclewis commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

benjaminclewis commented 7 months ago
"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 commented 7 months ago
builder.Services
    .AddMicrosoftIdentityWebApiAuthentication(builder.Configuration, Constants.AzureAd, "Bearer")
    .EnableTokenAcquisitionToCallDownstreamApi()
    .AddInMemoryTokenCaches();
benjaminclewis commented 7 months ago

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 commented 7 months ago

would you have a small repro?

benjaminclewis commented 7 months ago

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

benjaminclewis commented 7 months ago

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 commented 6 months ago

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 commented 4 months ago

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?