Skip to content

RoleManager.RoleExistsAsync role name normalization issue #35997

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

Merged
merged 1 commit into from
Sep 1, 2021

Conversation

faresamr
Copy link
Contributor

@faresamr faresamr commented Aug 31, 2021

PR Title
RoleManager.RoleExistsAsync role name normalization issue

PR Description
As described in the linked issue, role name gets normalized twice

  • Remove unnecessary normalization in RoleExistsAsync, since the value will be passed to FindByNameAsync and the normalization will be handled there.
  • Update the unit test by inject a custom normalizer which returns different value in case recursive calls
    Normalize(Normalize("foo")) != Normalize("foo") to cover this case in both RoleManager and UserManager.
  • Stabilizing both RoleManager and UserManager unit tests to cope with the new normalizer.

Fixes #35946

@faresamr faresamr requested a review from Pilchie as a code owner August 31, 2021 23:31
@ghost ghost added area-identity Includes: Identity and providers community-contribution Indicates that the PR has been added by a community member labels Aug 31, 2021
@dnfadmin
Copy link

dnfadmin commented Aug 31, 2021

CLA assistant check
All CLA requirements met.

@faresamr
Copy link
Contributor Author

faresamr commented Sep 1, 2021

There are 3 checks failed one of them required and I'm assuming that this is not expected, the three of them failed for the same reason "a file can not be accessed because it's being used by another process"
How can I rerun these checks?

@TanayParikh
Copy link
Contributor

How can I rerun these checks?

We're having some file contention issues in the CI. I triggered the failed tests to re-run.

@HaoK
Copy link
Member

HaoK commented Sep 1, 2021

Thanks, for doing this so quickly @faresamr !

@HaoK HaoK merged commit f594e21 into dotnet:main Sep 1, 2021
@ghost ghost added this to the 7.0-preview1 milestone Sep 1, 2021
@HaoK
Copy link
Member

HaoK commented Sep 1, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2021

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1191548482

@aarindam10
Copy link

I am using 6.0 version, _roleManager.RoleExistsAsync is still not working! throwing TaskCanceledException with message "A task was canceled in Microsoft.EntityFrameworkCore.Relational", but earlier thread about "roleManager.RoleExistsAsync not working" - is marked as closed by [msftbot] bot.

@ghost
Copy link

ghost commented Jul 18, 2023

Hi @aarindam10. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@aarindam10
Copy link

Hi @aarindam10. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

Yes, I could open an new issue, but I am trying to post on same thread, so people who contributed on this issue, will know what’s going in new version of Identity 6

@ghost
Copy link

ghost commented Jul 18, 2023

Hi @aarindam10. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@aarindam10
Copy link

await _roleManager.CreateAsync(role) throws error, but following code executed successfully.

_roleManager.CreateAsync(role).GetAwaiter().GetResult()

Though the work is carried out, but not convinced why the first line of code is throwing error? not happy! will be waiting for right solution.

@ghost
Copy link

ghost commented Jul 18, 2023

Hi @aarindam10. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-identity Includes: Identity and providers community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RoleExistsAsync method not working in Identity RoleManager
5 participants