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

Mute or remove privileges from a user. #8502

Closed
gaazkam opened this Issue Sep 12, 2018 — with docs.microsoft.com · 12 comments

Comments

Projects
None yet
3 participants

gaazkam commented Sep 12, 2018 — with docs.microsoft.com

"Roles are stored in the Identity cookie. Changes made to user roles are not persisted to the cookie until the cookie is regenerated or the user signs out and signs in. Applications that add users to a role should call SignInManager.RefreshSignInAsync(user) to update the cookie."

I am dreadfully sorry but this really doesn't seem to change the observable behavior with regards to issue #8474

I have created an extremely simplistic project to show this, the code is accessible at https://github.com/gaazkam/RefreshSignIn

The project contains a page "Ban" that both allows users to ban other user via adding them to the role "Banned" and should be inaccessible to banned users (for the sake of simplicity)

This is the banning C# code from this page:

            var bannedUser = await userManager.FindByNameAsync(Input.UsernameToBan);
            if (bannedUser != null)
            {
                await userManager.AddToRoleAsync(bannedUser, "Banned");
                await signInManager.RefreshSignInAsync(bannedUser);
                Message = "Succesfully banned user " + Input.UsernameToBan + ".";
            }

As you can see SignInManager.RefreshSignInAsync IS called here

In spite of this the problematic observable behavior persists: if UserA bans UserB while UserB is logged in, UserB STILL can successfully access the Ban page they should not be able to access, at least until they log out and log in again. Or until enough time has passed, as you have mentioned waiting 30mins indeed makes UserB effectivelly banned, but I feel this still should happen immediatelly, not after 30 mins.


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@Rick-Anderson

This comment has been minimized.

Show comment
Hide comment
@Rick-Anderson

Rick-Anderson Sep 12, 2018

Contributor

@HaoK is signInManager.RefreshSignInAsync(User); enough to refresh the Identity cookie?

Contributor

Rick-Anderson commented Sep 12, 2018

@HaoK is signInManager.RefreshSignInAsync(User); enough to refresh the Identity cookie?

@HaoK

This comment has been minimized.

Show comment
Hide comment
@HaoK

HaoK Sep 12, 2018

Member

RefreshSignInCookie refreshes your cookie, it doesn't refresh someone elses cookie. You need to UpdateSecurityStamp on banned user, and that only triggers a refresh after the default validation interval, there is no immediate revocation by default

Member

HaoK commented Sep 12, 2018

RefreshSignInCookie refreshes your cookie, it doesn't refresh someone elses cookie. You need to UpdateSecurityStamp on banned user, and that only triggers a refresh after the default validation interval, there is no immediate revocation by default

@Rick-Anderson

This comment has been minimized.

Show comment
Hide comment
@Rick-Anderson

Rick-Anderson Sep 12, 2018

Contributor

@HaoK Adding roles is a rare event, perhaps we should suggest they send a message to the user in the new role to sign-out/sign. It sounds like the following advice I added is incomplete:

Add a user to a role

Roles are stored in the Identity cookie. Changes made to user roles are not persisted to the cookie until the cookie is regenerated or the user signs out and signs in. Applications that add users to a role should call SignInManager.RefreshSignInAsync(user) to update the cookie.

Contributor

Rick-Anderson commented Sep 12, 2018

@HaoK Adding roles is a rare event, perhaps we should suggest they send a message to the user in the new role to sign-out/sign. It sounds like the following advice I added is incomplete:

Add a user to a role

Roles are stored in the Identity cookie. Changes made to user roles are not persisted to the cookie until the cookie is regenerated or the user signs out and signs in. Applications that add users to a role should call SignInManager.RefreshSignInAsync(user) to update the cookie.

@HaoK

This comment has been minimized.

Show comment
Hide comment
@HaoK

HaoK Sep 12, 2018

Member

Well the typical scenario for RefreshSignIn is when they are on their manage page and doing something on them. Admin scenarios are not something identity supports generally at this time. So changing someone elses roles is not something we typically deal with as its an admin scenario

Member

HaoK commented Sep 12, 2018

Well the typical scenario for RefreshSignIn is when they are on their manage page and doing something on them. Admin scenarios are not something identity supports generally at this time. So changing someone elses roles is not something we typically deal with as its an admin scenario

@Rick-Anderson

This comment has been minimized.

Show comment
Hide comment
@Rick-Anderson

Rick-Anderson Sep 12, 2018

Contributor

Supporting real-time admin pages is ultra-low priority. I think it's reasonable for the admin to send a message to the user: "If you are currently signed in, sign-out and sign in again to use your new privilege - or wait approximately 30 minutes until the Identity cookie is refreshed.

Contributor

Rick-Anderson commented Sep 12, 2018

Supporting real-time admin pages is ultra-low priority. I think it's reasonable for the admin to send a message to the user: "If you are currently signed in, sign-out and sign in again to use your new privilege - or wait approximately 30 minutes until the Identity cookie is refreshed.

@HaoK

This comment has been minimized.

Show comment
Hide comment
@HaoK

HaoK Sep 12, 2018

Member

Sounds good to me

Member

HaoK commented Sep 12, 2018

Sounds good to me

This comment has been minimized.

Show comment
Hide comment
@gaazkam

gaazkam Sep 12, 2018

For adding new privileges, that may be so.
But what about banning? Muting?
https://docs.microsoft.com/en-us/aspnet/core/tutorials/signalr?view=aspnetcore-2.2&tabs=visual-studio
Tutorials include a chat app, but those require muting functionality. When someone spams nonsense in chat, I don't think telling them "We've just muted you, please log out so that you may stop spamming chat" is appropriate.
And what about forums? Wikis? All those require banning.
If I might ask, if and how (if yes) does the framework support this?

gaazkam commented Sep 12, 2018 — with docs.microsoft.com

For adding new privileges, that may be so.
But what about banning? Muting?
https://docs.microsoft.com/en-us/aspnet/core/tutorials/signalr?view=aspnetcore-2.2&tabs=visual-studio
Tutorials include a chat app, but those require muting functionality. When someone spams nonsense in chat, I don't think telling them "We've just muted you, please log out so that you may stop spamming chat" is appropriate.
And what about forums? Wikis? All those require banning.
If I might ask, if and how (if yes) does the framework support this?

@HaoK

This comment has been minimized.

Show comment
Hide comment
@HaoK

HaoK Sep 12, 2018

Member

Check on the server whether they have privileges rather than trusting the cookie

Member

HaoK commented Sep 12, 2018

Check on the server whether they have privileges rather than trusting the cookie

@HaoK

This comment has been minimized.

Show comment
Hide comment
@HaoK

HaoK Sep 12, 2018

Member

Cookies are by definition stale, hit your db if you want to ensure they have permissions

Member

HaoK commented Sep 12, 2018

Cookies are by definition stale, hit your db if you want to ensure they have permissions

This comment has been minimized.

Show comment
Hide comment
@gaazkam

gaazkam Sep 12, 2018

Okay, thank you!

gaazkam commented Sep 12, 2018 — with docs.microsoft.com

Okay, thank you!

This comment has been minimized.

Show comment
Hide comment
@gaazkam

gaazkam Sep 12, 2018

Just to make certain though, do I understand You correctly that I should use UserManager.IsInRoleAsync https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.identity.usermanager-1.isinroleasync?view=aspnetcore-2.1

instead of ClaimsPrincipal.IsInRole?

gaazkam commented Sep 12, 2018 — with docs.microsoft.com

Just to make certain though, do I understand You correctly that I should use UserManager.IsInRoleAsync https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.identity.usermanager-1.isinroleasync?view=aspnetcore-2.1

instead of ClaimsPrincipal.IsInRole?

@HaoK

This comment has been minimized.

Show comment
Hide comment
@HaoK

HaoK Sep 12, 2018

Member

That hits the database yes so it is guaranteed to be correct

Member

HaoK commented Sep 12, 2018

That hits the database yes so it is guaranteed to be correct

@Rick-Anderson Rick-Anderson added P1 and removed P2 labels Sep 13, 2018

@Rick-Anderson Rick-Anderson changed the title from signInManager.RefreshSignIn(user) is really not enough to foce immediate effectiveness of adding a user to a role to Mute or remove privileges from a user. Sep 14, 2018

Rick-Anderson added a commit that referenced this issue Sep 14, 2018

Rick-Anderson added a commit that referenced this issue Sep 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment