[BUG] User remains logged in even after password change or account-disable

[iJungleboy]

Oqtane Info

Version - 5.1.2
Render Mode - Static
Interactivity - Server
Database - SQL Server

Describe the bug

My understanding is that when a host changes another users password or sets an account to Deleted = true should invalidate / lock-out a user who is currently logged on.

This does not happen. The user can continue to use the site if he doesn't actively log out. I believe this even remains this way for (default) 14 days after the initial login.

Expected Behavior

• Changing a users password should make any sessions of that user invalid and force him to log on again
• Disabling the user Deleted = true should make any sessions of that user invalid and of prevent him from logging on again

Steps To Reproduce

1. Create a user, give it some simple password

2. Log in the user in another browser

3. change the users password

4. user in other browser still works

5. Create a user, give it some simple password

6. Log in the user in another browser

7. set user to deleted

8. user in other browser still works

Anything else

may be related to #3108

[sbwalker]

#3108 only handled Interactive Blazor scenarios - Static Blazor will require a different approach

[sbwalker]

As part of the migration to .NET 8 and SSR, Oqtane is using the RevalidatingAuthenticationStateProvider. However by default it only revalidates every 30 minutes (this is the default specified by Microsoft I assume to avoid scalability issues). The time could be reduced but it's not real-time.

"Blazor differs from a traditional server-rendered web apps that make new HTTP requests with cookies on every page navigation. Authentication is checked during navigation events. However, cookies aren't involved. Cookies are only sent when making an HTTP request to a server, which isn't what happens when the user navigates in a Blazor app. During navigation, the user's authentication state is checked within the Blazor circuit, which you can update at any time on the server using the RevalidatingAuthenticationStateProvider abstraction."

[marcdrexel]

this issue is caused by "SecurityStamp" not being updated / present in cookie and not checked in AutenticationStateProvider or in PrincipalValidator.

i implemented a possible fix in our fork
https://github.com/2sic/oqtane.framework/tree/bugfix/issue-4580

it's not ready for a pull-request yet as there is still a bug if a user changes their own password; they are logged out because of this. reason for this is that the cookie is not correctly reissued in this case.

[sbwalker]

@marcdrexel thank you for sharing - this is very helpful

[sbwalker]

@marcdrexel I believe the issue you described can be addressed with some logic in Oqtane's UpdateUser method -

ie.

if the user object being updated matches the principal (the user is updating their own password) then ...

// update the password without changing the SecurityStamp
identityuser.PasswordHash = _identityUserManager.PasswordHasher.HashPassword(identityuser, user.Password);
await _identityUserManager.UpdateAsync(identityuser);

else

// update the user password and change the SecurityStamp
var user = _userManager.ChangePasswordAsync(userId, oldPassword, password);

[marcdrexel]

@sbwalker

yes, that would work for this special case, but it has some down sides

• various other methods in UserManager class implicitly update security stamp like SetEmailAsync, ChangeEmailAsync etc. causing the very same problem.
• It would create a security issue, a user with a compromised account could not lock out a bad actor by changing their password cause the security stamp would not update and the attacker while not being able to login again would still be in possession of a valid cookie.

Supplying a new updated cookie after every security relevant change to a user after they updated their own account is essential. Unfortunately, I can’t get this to work in theory a simple check and call to SignInManager.RefreshSignInAsync() should have done the trick e.g.

// check if updated user is logged-in user
if (_identitySignInManager.Context.User.Identity?.Name != null && (_identitySignInManager.Context.User.Identity?.Name.Equals(identityuser.UserName) ?? false))
{
    // did security stamp change
    var securityStampClaim =
        _identitySignInManager.Context.User.Claims.FirstOrDefault(c => c.Type == _identitySignInManager.Options.ClaimsIdentity.SecurityStampClaimType);

    if (securityStampClaim != null &&
        !securityStampClaim.Value.Equals(identityuser.SecurityStamp))
    {

        // refresh sign in -> this does not work cookies is not reissued
        await _identitySignInManager.RefreshSignInAsync(identityuser);
    }
}

This is basically the same approach you get when creating a new BlazorApp in VS and set “Authentication type” to “Individual Accounts”. Doesn’t work within Oqtane and I can’t figure out way, my knowledge about Blazor/Oqtane is too limited. Maybe you have an idea?

[sbwalker]

@marcdrexel thank you for the explanation... I had not considered the scenario you described above. You are correct that a new cookie is needed after every security relevant change to a user.

In regards to the other issue you described.... when you are running a Blazor application using Static Rendering and you have some Interactive components rendered using Blazor Server, there is a SignalR connection between the server and browser. This SignalR circuit contains cookies which are maintained independently from the standard browser cookies. So when cookies are changed, the circuit needs to be refreshed as well. I believe this is the reason why simply calling RefreshSignInAsync() is not working - because it is not refreshing the SignalR circuit. In order to refresh the cookies you need to implement a server-side redirect-style flow since it requires a new HTTP request. And the other (undocumented) aspect to this redirect-style flow is that it must be a POST request - GET requests will not refresh the circuit. Oqtane does have a mechanism for handling this which is exactly how it handles external login providers (ie. OIDC). Basically you can include a querystring parameter of ?reload=post to a Url and Oqtane will send a POST request to the server which does nothing but redirect back to the client... however this redirect-style flow will refresh the cookies in the circuit. You can try this for yourself with your custom modifications you have made, and I am guessing that after RefreshSignInAsync() is called if you manually modify the Url in your browser to include ?reload=post and hit Enter it will refresh the page and the user will remain logged in. Obviously, the actual solution will need to be automated - which could be done with events or a Navigation.NavigateTo() call in the component (including reload=post) after the password change has been processed.

[sbwalker]

And the reason why it "works" in the default Blazor Web App template is because the scaffolded Auth pages are using static components - not interactive components - so they reference HttpContext.User directly (and although this works in the default scenario it creates a lot of challenges in other Blazor scenarios)

[sbwalker]

@marcdrexel the one major concern I have with https://github.com/2sic/oqtane.framework/tree/bugfix/issue-4580 is that the ValidateAsync() method now calls SecurityStampValidator which makes 2 additional database calls each time ValidateAsync() is invoked (and it is invoked a lot). So this would be a MAJOR performance impact to the framework. So there would probably need to be an option to toggle the SupportsUserSecurityStamp per site or installation so that performance is only impacted if you truly need this security feature.

I am guessing the code changes you made is how the default .NET Core Identity handles the scenario. I find this to be quite interesting as most developers blindly adopt libraries such as .NET Core Identity and do not understand that certain configurations (ie. real-time account lockout) can actually cause major performance issues in their application. There are always tradeoffs in software - but with a black box you don't always know the impacts.

[sbwalker]

#4618 adds support for SecurityStamp. The integration has no impact on performance as it is uses the cached user object rather than making database queries for each validation. The expected behavior is that the user is logged out if their account is deleted, their password is changed, or their role assignments are modified.