Skip to content

Blazor WASM security updates for 5.0#20138

Merged
guardrex merged 2 commits intomasterfrom
guardrex/blazor-wasm-security
Oct 8, 2020
Merged

Blazor WASM security updates for 5.0#20138
guardrex merged 2 commits intomasterfrom
guardrex/blazor-wasm-security

Conversation

@guardrex
Copy link
Copy Markdown
Collaborator

@guardrex guardrex commented Oct 7, 2020

Addresses #19503

Internal Review Topics ... these are the focus topics of this PR ...

... there are some additional updates to other topics, but they can be somewhat bypassed given that they aren't receiving their full updates here today. They will be the focus of additional PRs in the coming days.

  • All of the updates for the standalone guidance.
  • Part of the updates for the hosted guidance. The AAD and AAD B2C hosted topics will be the focus of a PR tomorrow to finish their 5.0 updates.
  • Consistency and correctness nits.
    • Code-fencing "Client" and "Server" when referring to the projects created by the template, which will only be in English.
    • Only using "clear" and "selected" when referring to check boxes per the MS style guidelines.

You'll see that I've temporarily left the NOTE on If the Azure portal provides the scope URI for the app and the app throws an unhandled exception when it receives a 401 Unauthorized response from the API, .... Let's leave that for now. I need to work on that a bit more before I determine if we can safely version it only for 3.x. I'll probably have an answer on it for tomorrow's PR.

I guessed correctly on issue Questions 1-3, so I didn't need to make changes. I already had some general text for DefaultAccessTokenScopes with AdditionalScopesToConsent added on this PR, but we still don't have anything specific yet. If you want to float some content ... a sentence or two ... or more ... on review, let's do that. Otherwise, we can go with this coverage now, and I can ask for further feedback later on it.

If possible, I'd like to get this merged by EOD so that I'm unblocked to pick up with the AAD-based hosted topics in the morning. Idk if we can pull it off, but I hope we can try. Sorry for the rush! 🏃🏃🏃🏃🏃🏃 I don't plan on pinging for additional comments, so I yield to you if you think we should.

@guardrex guardrex requested a review from captainsafia October 7, 2020 19:05
Copy link
Copy Markdown
Contributor

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

Pretty solid overall! Left some comments inline about clarifying the scopes that are set.

@guardrex
Copy link
Copy Markdown
Collaborator Author

guardrex commented Oct 7, 2020

@captainsafia ... Ok ... thx for that ... I wish I would've drilled down a bit more this morning.

We now have -AAD (for AAD) and -nonAAD (for MS Accounts and B2C) INCLUDES for both standalone and hosted default scope sections. They're separate for standalone and hosted because the hosted one has to say 'do this in the Client app.'

the config shouldn't be needed for hosted apps

I thought that the Client app of a hosted solution would be configured like a standalone app ... at least to avoid this new error for an app that doesn't specify default scopes. That's a wrong guess? I would find out the hard way in the morning anyway ... I'll be working with the hosted scenarios tomorrow.

@guardrex
Copy link
Copy Markdown
Collaborator Author

guardrex commented Oct 8, 2020

@captainsafia ... I'll go ahead and merge now so that I can pick up with the next round of updates. If there's more to update from this PR, I'll place the changes on the next one.

@guardrex guardrex merged commit 6d22c8d into master Oct 8, 2020
@guardrex guardrex deleted the guardrex/blazor-wasm-security branch October 8, 2020 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants