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

Problem with normalized roles #29

Closed
SebastianStehle opened this issue Feb 19, 2017 · 4 comments
Closed

Problem with normalized roles #29

SebastianStehle opened this issue Feb 19, 2017 · 4 comments

Comments

@SebastianStehle
Copy link

Hi,

I have a problem with this comment:

// todo might have issue, I'm just storing Normalized only now, so I'm returning normalized here instead of not normalized.
// EF provider returns not noramlized here
// however, the rest of the API uses normalized (add/remove/isinrole) so maybe this approach is better anyways
// note: could always map normalized to not if people complain
public virtual async Task<IList<string>> GetRolesAsync(TUser user, CancellationToken token)
=> user.Roles;

The problem is, that my javascript client gets the claims including the role and the role is in upper case. A little bit annoying.

@g0t4
Copy link
Owner

g0t4 commented Feb 20, 2017

IIRC the tradeoff was between being surprised by hard to track down runtime errors ... or being annoyed when people look at uppercase.

Regardless, annoyance is perception, one that is within your ability to control. This is a wonderful opportunity to see how powerful it can be to prove yourself wrong (prove that you don't have to be annoyed) which is a powerful tool to keep in your toolbelt.

I have a bit more about how to change annoyance into at least disinteresting, perhaps even liking: http://www.weshigbee.com/being-happy-without-compromising-when-you-dont-get-your-way/

At first, when I saw what the new API was doing with normalization, I didn't like it either. You can see this in my comments in the code. But, I reminded myself that it works still, so I actually don't mind.

@g0t4 g0t4 closed this as completed Feb 20, 2017
@SebastianStehle
Copy link
Author

What I mean is, that it is not consistent. E.g. you introduce a role with AddRoleAsync((new IdentityRole("admin")) and then you cannot use this role name directly. Of course it is best practice to use case invariant string comparisons, but this also a "problem" for debugging or for the UI's in my opionion.. And why is there a normalized name and a normal name?

@g0t4
Copy link
Owner

g0t4 commented Feb 21, 2017

The normalized name/email stuff was added in the last update to the framework, I haven't been able to find a reason why.

If you want the names unaltered you can query the underlying mongo collection. I designed that GetRolesAsync to plug back into the existing methods which work with a normalized form. I never saw much value in that GetRolesAsync method because it's just firing off a simple query.

@SebastianStehle
Copy link
Author

I just uppercased all role names :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants