Skip to content

Security Review for the new Identity APIs #48433

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

Closed
halter73 opened this issue May 25, 2023 · 1 comment
Closed

Security Review for the new Identity APIs #48433

halter73 opened this issue May 25, 2023 · 1 comment
Assignees
Labels
Milestone

Comments

@halter73
Copy link
Member

@javiercn brought up some concerns with my PRs that removed Duende IdentityServer from the angular and react templates and asked me to file an issue to track improvements in this area. @javiercn please let me know if I'm missing anything with this issue.

Here was my response in the PR explaining the current state of individual auth and how it's lacking in some ways compared to the IdentityServer version on the project templates:

Is there no way to know when you are authenticated or not? Isn't it confusing that you log in, come back to the app, and you see no visual change?

Nope. Currently the best we can do with just cookie auth and the default Identity UI is detect the redirect. Later, MapIdentityApi will provide a way to detect this which we will provide samples for. Eventually it would be nice to update these project templates to use it and not use the default Identity UI razor pages at all, but it's not ready yet.

What happens with all the state the app has before we redirect? Do we throw it all out?

Yes.

Why are we removing the authorize guard? That's meant to ensure you are authenticated when you access some areas of the page.

Because we have no way to know if we're authenticated or not.

How do you log out in the template?

You click the "Account" link in the nav bar. And then click "Logout" on the /Identity/Account/Manage page.

Where are we wiring up Antiforgery?

It's not explicit. This is just the default Identity UI razor pages. I can confirm that the __RequestVerificationToken is rendered though.

Originally posted in dotnet/spa-templates#144 (comment)

Once we have completed work on MapIdentityApi which is tracked by https://github.com/dotnet/aspnetcore/issues?q=is%3Aopen+label%3Afeature-token-identity+sort%3Aupdated-desc, we can go back and potentially use these new endpoints to address these concerns.

With regard to antiforgery, the Identity Razor Pages do use anti-csrf tokens, but I now understand the concern is the security API endpoints that developers will add to template later.

It's plausible that some people may try to expose POST endpoints that accept a form POST without an ant-csrf token, possibly just by not expecting any request body at all. These can be secretly forged by an attacker submitting cross-domain <form method="post" action="https://www.example.org/... bypassing CORS restrictions.

We would recommend only accepting authenticated POST requests with a JSON body from API endpoints as a defense in depth measure. In practice, some people might stille accept authenticate POST requests without any parameters that could mutate important state, but even these people should be protected by our default secure; samesite=lax; httponly authentication cookies which:

Means that the cookie is not sent on cross-site requests, such as on requests to load images or frames, but is sent when a user is navigating to the origin site from an external site (for example, when following a link). This is the default behavior if the SameSite attribute is not specified.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#attributes

Ultimately, this will go through a threat model with the rest to the MapIdentityApi changes. @blowdart @mkArtak

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label May 25, 2023
@halter73 halter73 added area-identity Includes: Identity and providers and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels May 26, 2023
@mkArtakMSFT mkArtakMSFT changed the title Improve individual auth in spa templates Security Review for the new Identity APIs Jun 8, 2023
@mkArtakMSFT mkArtakMSFT added this to the .NET 8 Planning milestone Jun 8, 2023
@ghost
Copy link

ghost commented Jun 8, 2023

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
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 mkArtakMSFT modified the milestones: 8.0-preview7, 8.0-rc1 Jul 24, 2023
@danroth27 danroth27 modified the milestones: 8.0-rc1, 8.0-rc2 Aug 16, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants
@halter73 @danroth27 @mkArtakMSFT and others