Skip to content
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. Admin changes #8502

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

Mute or remove privileges from a user. Admin changes #8502

gaazkam opened this issue Sep 12, 2018 — with docs.microsoft.com · 12 comments
Assignees
Labels
Pri1 High priority, do before Pri2 and Pri3 Source - Docs.ms Docs Customer feedback via GitHub Issue

Comments

Copy link

gaazkam commented Sep 12, 2018

"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
Copy link
Contributor

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

@Rick-Anderson Rick-Anderson self-assigned this Sep 12, 2018
@Rick-Anderson Rick-Anderson added Pri2 Priority 2 Source - Docs.ms Docs Customer feedback via GitHub Issue question labels Sep 12, 2018
@Rick-Anderson Rick-Anderson added this to the Backlog milestone Sep 12, 2018
@HaoK
Copy link
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
Copy link
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.

@HaoK
Copy link
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
Copy link
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.

@HaoK
Copy link
Member

HaoK commented Sep 12, 2018

Sounds good to me

Copy link
Author

gaazkam commented 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?

@HaoK
Copy link
Member

HaoK commented Sep 12, 2018

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

@HaoK
Copy link
Member

HaoK commented Sep 12, 2018

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

Copy link
Author

gaazkam commented Sep 12, 2018

Okay, thank you!

Copy link
Author

gaazkam commented 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?

@HaoK
Copy link
Member

HaoK commented Sep 12, 2018

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

@Rick-Anderson Rick-Anderson added Pri1 High priority, do before Pri2 and Pri3 and removed Pri2 Priority 2 labels Sep 13, 2018
@Rick-Anderson Rick-Anderson modified the milestones: Backlog, Sprint 141 (9/3 to 9/21) Sep 13, 2018
@Rick-Anderson Rick-Anderson changed the title signInManager.RefreshSignIn(user) is really not enough to foce immediate effectiveness of adding a user to a role 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
@Rick-Anderson Rick-Anderson changed the title Mute or remove privileges from a user. Mute or remove privileges from a user. Admin changes Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pri1 High priority, do before Pri2 and Pri3 Source - Docs.ms Docs Customer feedback via GitHub Issue
Projects
None yet
Development

No branches or pull requests

3 participants