dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.59k stars 10.06k forks source link

Security Review for the new Identity APIs #48433

Closed halter73 closed 1 year ago

halter73 commented 1 year ago

@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 https://github.com/dotnet/spa-templates/pull/144#issuecomment-1561465615

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 commented 1 year ago

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.