DuendeSoftware / Support

Support for Duende Software products
21 stars 0 forks source link

QuickStart.UI ServerSideSessions page unsecured #1416

Closed RandScullard closed 2 months ago

RandScullard commented 2 months ago

Which version of Duende IdentityServer are you using? 7.0.5

Which version of .NET are you using? 8.0.8

I have a question about this page in the QuickStart UI: https://github.com/DuendeSoftware/IdentityServer.Quickstart.UI/blob/main/Pages/ServerSideSessions/Index.cshtml.cs

It seems as if this page has no security, so any authenticated user can get a list of server side sessions and delete any session belonging to any user. Is that true, or am I missing something? I noticed that the Diagnostics page has some code to only allow access from the local PC - shouldn't the ServerSideSessions page have the same code?

RolandGuijt commented 2 months ago

This QuickStart UI only contains Razor pages and supporting files. We are assuming that you are mapping the Razor endpoints using this code:

app.MapRazorPages().RequireAuthorization();

This will require an authenticated user for all Razor endpoints, except when they have the [AllowAnonymous] attribute on. On the login page we use that attribute to let users log in. The code in the Diagnostics page you're seeing is there on top of that to ensure it is accessed from a local IP address.

For a step-by-step guide please see our quickstart here.

RandScullard commented 2 months ago

Yes, it does require authentication - that's not the point. The point is that any authenticated user can delete a session belonging to any other user. That is a privileged operation that should not be available to all users, isn't it?

RolandGuijt commented 2 months ago

You are correct. By default any authenticated user can see the list of sessions and delete one. There's no good way for the template to make a default policy that would be easy to use because we're just using the test user store and there's no way to know what claim/other user data to check for in a "real world" implementation.

When the actual user store is implemented users of the Quickstart UI template should create an additional authorization policy and apply it to the relevant endpoints.

RandScullard commented 2 months ago

OK, then going back to my original point: Shouldn't the same code in the Diagnostics page that checks for a local IP address also be included in the ServerSideSessions page? At least that way there is some security on that page by default, so users of the template won't inadvertently deploy a vulnerability.

RolandGuijt commented 2 months ago

Yes that makes total sense. I've created an issue for your suggestion here. Please track the progress from there. Thank you for the suggestion!

RolandGuijt commented 2 months ago

I've done a PR for this which was approved, so I'm closing the issue. Thanks again.