Double scheme for App ID URI with Blazor WASM template (hosted, single org)

guardrex commented 3 years ago

Describe the bug

The Blazor WebAssembly template for a hosted, single org solution creates the default access token scope with an api:// scheme, but this results in a double scheme because devs provide the scheme when they pass the App ID URI argument to the dotnet new command.

The symbol :point_right: https://github.com/dotnet/aspnetcore/blob/master/src/ProjectTemplates/Web.ProjectTemplates/content/ComponentsWebAssembly-CSharp/.template.config/template.json#L317

The hard-coded scheme :point_right: https://github.com/dotnet/aspnetcore/blob/master/src/ProjectTemplates/Web.ProjectTemplates/content/ComponentsWebAssembly-CSharp/Client/Program.cs#L67

To Reproduce

Execute the dotnet new command with the blazorwasm template for a single organization hosted app with the App ID URI that includes the scheme.

For example ...

dotnet new blazorwasm --auth SingleOrg --hosted --app-id-uri "api://41451fa7-82d9-4673-8fa5-69eff5a761fd" --default-scope "API.Access"

Result (double api://) ...

options.ProviderOptions.DefaultAccessTokenScopes.Add("api://api://41451fa7-82d9-4673-8fa5-69eff5a761fd/API.Access");

It's also an issue if the directory has an unverified tenant domain, where the portal locks you into a fixed App ID URI with the tenant domain (e.g., https://contoso.onmicrosoft.com/...).

For example ...

dotnet new blazorwasm --auth SingleOrg --hosted --app-id-uri "https://contoso.onmicrosoft.com/41451fa7-82d9-4673-8fa5-69eff5a761fd" --default-scope "API.Access"

Result (api:// followed by the fixed, unverified publisher domain and its scheme) ...

options.ProviderOptions.DefaultAccessTokenScopes.Add("api://https://contoso.onmicrosoft.com/41451fa7-82d9-4673-8fa5-69eff5a761fd/API.Access");

Exceptions (if any)

None

Further technical details

ASP.NET Core version: >=3.1

Output of dotnet --info

.NET SDK (reflecting any global.json): Version: 5.0.100-rc.2.20479.15 Commit: da7dfa8840

Runtime Environment: OS Name: Windows OS Version: 10.0.18363 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\5.0.100-rc.2.20479.15\

Host (useful for support): Version: 5.0.0-rc.2.20475.5 Commit: c5a3f49c88

.NET SDKs installed: 3.1.300 [C:\Program Files\dotnet\sdk] 3.1.302 [C:\Program Files\dotnet\sdk] 3.1.402 [C:\Program Files\dotnet\sdk] 5.0.100-rc.2.20479.15 [C:\Program Files\dotnet\sdk]

The IDE (VS / VS Code/ VS4Mac) you're running on, and it's version: .NET Core CLI

cc: @scdehmlow

Docs cross-reference: Current dotnet new cli example adds an extra api:// to api uri (dotnet/AspNetCore.Docs #20381)

I've called this out in the docs to cover it for readers. I'll open a docs tracking issue to update the content based on this issue's outcome.

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

mkArtakMSFT commented 3 years ago

@guardrex can you still repro this?

guardrex commented 3 years ago

@mkArtakMSFT ... Yes. With 5.0.102, I just had it create a few apps to check, and I get the following in the first case ...

options.ProviderOptions.DefaultAccessTokenScopes.Add(
    "api://api://41451fa7-82d9-4673-8fa5-69eff5a761fd/API.Access");

... and the following the second case ...

options.ProviderOptions.DefaultAccessTokenScopes.Add(
    "api://https://contoso.onmicrosoft.com/41451fa7-82d9-4673-8fa5-69eff5a761fd/API.Access");

The automatic additional api:// still appears when the --app-id-uri is supplied.

adityamandaleeka commented 3 years ago

Triage: the input isn't supposed to have the prefix.

guardrex commented 3 years ago

Hello @adityamandaleeka ... Could you justify that a bit further? What you said seems to be at odds with the spec ...

Each URI begins with a scheme name that refers to a specification for assigning identifiers within that scheme.

Cross-reference: https://datatracker.ietf.org/doc/html/rfc3986#section-3.1

... and what the Azure portal shows/uses (e.g., the portal's copy feature grabs the value with the scheme).

Taken at face value, the "App Id URI" (--app-id-uri) is a URI, so it has to have a scheme provided by the dev to the option.

Note further that ~the framework is even adding the wrong scheme for an untrusted publisher domain ... it's not api: in that case ... it's https:.~ UPDATE (5/15): I've come to the conclusion that this is based on tenant type, AAD or AAD B2C, not publisher domain trust.

guardrex commented 1 year ago

@mkArtakMSFT ... This didn't make the cut for 6.0 or 7.0. We're still having to tell devs in the doc to address it because ...

AFAICT, a "URI" by spec definition must include the scheme, so devs would supply a scheme if asked for a URI.
There are (at least) two valid URI schemes for this parameter ... at least for Azure.
- If they supply no scheme (wrong anyway AFAICT, but ok ... they supply no scheme), it resolves to the wrong scheme of api:// in the generated app for when they're locked into https:// by Azure for an onmicrosoft.com domain. They must manually change it in the generated app.
- If they supply the scheme (https:// for example), which is consistent with what the Azure portal shows for the App Id URI, they end up with a double scheme of api://https://, which is also wrong and requires a manual update to the generated app.

Besides all of that, the URI for the app Id doesn't seem to be locked even to either api:// or https:// per the spec. Can't any scheme be used? ... even nonsensical schemes ... mmmm 🤔 .... rex:// 😄. Isn't that allowed? Idk ... I just think if that's correct that having the framework hard-code it makes even less sense.

Just for reference, it might be here 👉 https://github.com/dotnet/Scaffolding/blob/main/src/MSIdentityScaffolding/Microsoft.DotNet.MSIdentity/Tool/AppProvisioningTool.cs#L294

I'm mentioning this because of the planned work EOY for the Blazor security node. If this were to be scheduled and addressed for .NET 8, I can place a tracking note on it for Blazor docs for next year.

OH! ... one more thing ... This is marked as a breaking change, but that seems strange to me. This would be for scaffolding new apps from the project template, so it doesn't seem like it would break existing apps.

guardrex commented 1 year ago

We might not need this issue any longer† following the most recent round of security updates. We now explicitly instruct devs that they'll be using an "Azure Active Directory" tenant type for our AAD hosted WASM topic with a CLI command generating the app with Single Org authentication (-au SingleOrg). We now also explicitly instruct devs that they'll be using an "Azure Active Directory B2C" tenant type for our AAD B2C hosted WASM topic with a CLI command generating the app with Individual B2C authentication (-au IndividualB2C). Because we're explicitly instructing them to match the Azure tenant type to the command and we explicitly tell them not to use what Azure says is the App ID URI for the CLI command but to only use the GUID portion of what Azure shows, they won't end up with a mismatch between the App ID URI in the client app's token scope and the App ID URI of the token scope in Azure.

Still tho, I felt that at least in the hosted WASM with AAD case that we should call out how to use an AAD B2C tenant type with a SingleOrg auth hosted WASM app. I include a full section dealing with that scenario that explains what they'll need to change in their app.

†might not need this issue any longer ... The framework design still seems a bit off on this point: It is possible that a dev has mixed 🍸 the tenant type/app auth on purpose, and this scenario is supported by Azure and Blazor. Instead of the CLI command accepting a "URI" without a scheme (not really legal in terms of specification), I still feel that the CLI command should take the literal App ID URI (i.e., with the scheme, exactly as the Azure portal shows it) and not guess that that the dev is using a matching tenant type between SingleOrg and IndividualB2C. If that were the case, the topic wouldn't need to say, 'pass the GUID of the App ID URI shown by the portal.' We probably could drop the section that I have on how to mix tenant type and app authentication type and just say, 'use the exact App ID URI that the Azure portal shows (i.e., with whatever scheme it shows)' because Azure will show the correct App ID URI value either way, whether they mix or not. They can just copy what Azure shows directly into their CLI command placeholder. I would only need a NOTE to explain that there is a difference based on Azure tenant type on what Azure defaults to and leave it at that.

Also, I had to add a section on a custom App ID URI because the template is guessing that they won't do that. The framework guess on the scheme will be wrong, and this issue is still a concern. The scenario is that the dev tries to use something like urn://custom-app-id-uri. If they only supply the custom-app-id-uri part, they will get either https://custom-app-id-uri/... or api://custom-app-id-uri/... depending on auth type of the app ... both being wrong. If they try passing urn://custom-app-id-uri, then they'll get the double scheme of https://urn://custom-app-id-uri or api://urn://custom-app-id-uri ... obviously not good either way. If the command would just take the URI as whatever the URI is and as the portal shows and not guess, these problems can be avoided.

If you want to leave the design as it is, we can close this and let the doc cover it. If you still want to entertain the idea that the CLI should be taking a correct App ID URI per HTML spec with a scheme that matches exactly what the portal shows, then this issue should remain open.

guardrex commented 1 year ago

We can close this for sure now: With hosted WASM going away, WASM apps will be left with standalone/static WASM and a web API server app that they'll create separately outside of a Blazor template (command). We may carry replacement guidance for such a scenario in the future, but that's TBD at this point. If we do, I'll be sure to call out the relevance of the Azure directory tenant type as it pertains to the App ID URI in any new guidance that we write for Blazor security articles. Guidance for <8.0 is in good shape AFAIK.

@mkArtakMSFT ... I just leave the one thought with you on the URI for future templates. I still feel that templates shouldn't try to guess the scheme for App ID URIs. I still recommend taking the URI as a complete, valid URI with a scheme and using it exactly as supplied for scope configuration. That's simpler (and cheaper💰💰💰😆) to document and probably less confusing to devs given what the Azure portal shows during Azure app registration/config.

dotnet / aspnetcore