Skip to content

Conversation

@JunTaoLuo
Copy link
Contributor

@JunTaoLuo JunTaoLuo commented Aug 13, 2020

Fixes https://github.com/aspnet/AspNetCore-ManualTests/issues/149.

Description

The scope used during validation in the WeatherForecast controller is hard coded to access_as_user. This is an issue for the blazor-wasm template since if IndividualB2C auth is configured, the scope specified by the default-scope option will could lead to a mismatch between the client's configuration and the validation logic in the controller of the server. This change removes the hardcoding and aligns the scopes so that the same scope is used in the client and the server.

Customer Impact

If the customer creates a hosted blazor wasm template (aka ComponentsWebAssembly) and selectes to use IndividualB2C auth and sets the default-scope option to anything other than access_as_user. The validation check in the WeatherForecast controller will fail.

Regression?

Yes, this is a regression that was introduced in 5.0 preview8

Risk

Very low, this template change affects a very narrow scenario (hosted blazor wasm with IndividualB2C enabled) and has been verified locally.

@JunTaoLuo JunTaoLuo requested a review from captainsafia August 13, 2020 23:41
@JunTaoLuo JunTaoLuo requested a review from a team as a code owner August 13, 2020 23:41
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Aug 13, 2020
@JunTaoLuo
Copy link
Contributor Author

@Pilchie should we try to make this fix in preview8? Or should we just note it as a known issue and have the users work around it by removing the check after generating the template?

@JunTaoLuo
Copy link
Contributor Author

Based on offline discussion, it was determined that we should match the scope used for validation to what was set here: https://github.com/dotnet/aspnetcore/blob/master/src/ProjectTemplates/Web.ProjectTemplates/content/ComponentsWebAssembly-CSharp/Client/Program.cs#L67 which is controlled by the default-scopes option. We also decided to change the default from user_impersonation to access_as_user.

Note that we should update the instructions at https://github.com/aspnet/Tooling-ManualTests/blob/rel/16.7/E2EWalkthroughs/VS/AspNetCore30/30_ClientBlazorWithAADB2C.md

@javiercn
Copy link
Member

Based on offline discussion, it was determined that we should match the scope used for validation to what was set here: https://github.com/dotnet/aspnetcore/blob/master/src/ProjectTemplates/Web.ProjectTemplates/content/ComponentsWebAssembly-CSharp/Client/Program.cs#L67 which is controlled by the default-scopes option. We also decided to change the default from user_impersonation to access_as_user.

Note that we should update the instructions at https://github.com/aspnet/Tooling-ManualTests/blob/rel/16.7/E2EWalkthroughs/VS/AspNetCore30/30_ClientBlazorWithAADB2C.md

Thanks @JunTaoLuo.

@guardrex can you take care of the doc update?

Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,

I'll assume you've done due diligence and tested the change manually.

Copy link
Contributor

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@guardrex
Copy link
Contributor

Nothing has been documented for the new APIs yet. I opened Microsoft.Identity.Web APIs for Blazor (dotnet/AspNetCore.Docs #19503), and I plan to work it when RC1 lands.

@Pilchie
Copy link
Member

Pilchie commented Aug 14, 2020

I think we should at least talk to Tactics about taking this for Preview 8. Can you retarget and send mail for it?

@JunTaoLuo JunTaoLuo changed the base branch from master to release/5.0-preview8 August 14, 2020 17:17
@JunTaoLuo JunTaoLuo requested a review from a team August 14, 2020 17:17
@JunTaoLuo JunTaoLuo changed the base branch from release/5.0-preview8 to master August 14, 2020 17:17
@JunTaoLuo JunTaoLuo force-pushed the johluo/fix-blazor-wasm-hosted-azureadb2c branch from c1ad6c4 to 9c7c489 Compare August 14, 2020 17:26
@JunTaoLuo JunTaoLuo changed the base branch from master to release/5.0-preview8 August 14, 2020 17:29
@JunTaoLuo
Copy link
Contributor Author

I'll send the email after I test a few more scenarios with @captainsafia

@dougbu dougbu removed their request for review August 14, 2020 18:18
@JunTaoLuo JunTaoLuo force-pushed the johluo/fix-blazor-wasm-hosted-azureadb2c branch from 9c7c489 to 7c23da1 Compare August 14, 2020 19:32
@JunTaoLuo JunTaoLuo added the Servicing-consider Shiproom approval is required for the issue label Aug 14, 2020
@ghost
Copy link

ghost commented Aug 14, 2020

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

@JunTaoLuo JunTaoLuo changed the title Remove accepted scope check for non-Identity.Web scenarios Align scope used by client and server in blazor wasm template Aug 14, 2020
@JunTaoLuo JunTaoLuo force-pushed the johluo/fix-blazor-wasm-hosted-azureadb2c branch from 7c23da1 to 251fe4e Compare August 14, 2020 19:43
@JunTaoLuo JunTaoLuo added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Aug 14, 2020
@JunTaoLuo JunTaoLuo merged commit 7acb96a into release/5.0-preview8 Aug 15, 2020
@JunTaoLuo JunTaoLuo deleted the johluo/fix-blazor-wasm-hosted-azureadb2c branch August 15, 2020 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Servicing-approved Shiproom has approved the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants