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

Add claims to Identity using IUserClaimsPrincipalFactory - subtopic generates run-time errors. #18797

Closed
Gary255 opened this issue Jun 13, 2020 · 7 comments · Fixed by #18947 or #25317
Closed
Assignees
Labels
doc-enhancement Source - Docs.ms Docs Customer feedback via GitHub Issue

Comments

@Gary255
Copy link

Gary255 commented Jun 13, 2020

[Enter feedback here]
I am using Net Core 3.1.6.
I worked through this tutorial page successfully up to the topic cited in the title.

With the WebApp1 project built as instructed, runtime produces the error:
System.InvalidOperationException: 'Scheme already exists: Identity.Application'

I worked out that this was due to there being two calls to establish Identity: one in Startup.cs, and one in IdentityHostingStartup.cs.

Commenting out the one in IdentityHostingStartup.cs produces:
InvalidOperationException: No service for type 'Microsoft.AspNetCore.Identity.UserManager`1[WebApp1.Areas.Identity.Data.WebApp1User]' has been registered.

Commenting out the one in Startup.cs produces:
System.AggregateException: 'Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType: Microsoft.AspNetCore.Identity.IUserClaimsPrincipalFactory1[WebApp1.Areas.Identity.Data.ApplicationUser] Lifetime: Scoped ImplementationType: WebApp1.Areas.Identity.Data.AdditionalUserClaimsPrincipalFactory': Unable to resolve service for type 'Microsoft.AspNetCore.Identity.UserManager1[WebApp1.Areas.Identity.Data.ApplicationUser]' while attempting to activate 'WebApp1.Areas.Identity.Data.AdditionalUserClaimsPrincipalFactory'.)'

It seems I need to construct a call which uses some things from one of the calls, and some things from the other, but I can't find in the docs a clear list of the services which should be registered.

I realise this is not a "help" service. It would be nice, however, if the tutorial provided a working sample.


Document Details

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

@serpent5
Copy link
Contributor

I just went through this tutorial. I found that the code in the section cited is out-of-context for the rest of the topic. Up until that point, it's a step-by-step tutorial. In that section, it's more "example code" than it is a step in the tutorial. For example, the tutorial part is all about WebApp1User, but the "example code" is about ApplicationUser. It also shows calling AddIdentity in ConfigureServices, but the tutorial means the configuration is also done in IdentityHostingStartup.

Having followed the tutorial, what you should do is:

  1. Add only the following code, to either Startup.ConfigureServices or IdentityHostingStartup.Configure:

    services.AddScoped<IUserClaimsPrincipalFactory<WebApp1User>, AdditionalUserClaimsPrincipalFactory>();
  2. Update the call to AddDefaultIdentity in IdentityHostingStartup.Configure to also add roles:

    services.AddDefaultIdentity<WebApp1User>(options => options.SignIn.RequireConfirmedAccount = true)
        .AddRoles<IdentityRole>() // <-- Here
        .AddEntityFrameworkStores<WebApp1IdentityDbContext>();
  3. Instead of creating ApplicationUser, add the IsAdmin property to WebApp1User.

  4. Create a new migration and update the database.

  5. When creating AdditionalUserClaimsPrincipalFactory, use WebApp1User instead of ApplicationUser.

  6. Lastly, create an authz policy named IsAdmin that checks for the claim. This is a bit more involved but is covered in claims-based authz.

@Rick-Anderson Rick-Anderson removed the PU label Jun 18, 2020
@Rick-Anderson Rick-Anderson self-assigned this Jun 18, 2020
@Rick-Anderson Rick-Anderson added this to To do in Next sprint via automation Jun 18, 2020
@Gary255
Copy link
Author

Gary255 commented Jun 18, 2020

Serpent5 - Thanks for the confirmation that I haven't gone crazy. What I think I really need is separate roles, so I will forget the out-of-context stuff and move on.

@serpent5
Copy link
Contributor

@Rick-Anderson What should we do with this one? Here's a few ideas:

  1. Reframe the section as a set of tutorial steps that extend the existing tutorial.
  2. Be more explicit that it's not an extension of the tutorial.
  3. Leave it as it is, with this issue being available in the repo for anyone that gets stuck.

@Rick-Anderson
Copy link
Contributor

How about 2 and keep it simple buy adding, For more information, see `this GitHub issue

@br3nt
Copy link

br3nt commented Nov 18, 2020

This is just feedback of my experience of the chosen solution (2)...

It's really odd that Add claims to Identity using IUserClaimsPrincipalFactory doesn't follow on from the step-by-step tutorial. It's completely out of place and has nothing to do with the subject of the page.

The IsAdmin property isn't marked as [PersonalData], and therefore has no relevance to the subject of this page, which is Add, download, and delete custom user data to Identity.

I would expect this section to add either the Name or DOB property of WebApp1User as a claim

@Rick-Anderson Rick-Anderson reopened this Mar 15, 2022
@Rick-Anderson
Copy link
Contributor

@serpent5 please review previous comment. I think we should just remove this section.

@serpent5
Copy link
Contributor

I think we should just remove this section.

I agree. I'll do that now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-enhancement Source - Docs.ms Docs Customer feedback via GitHub Issue
Projects
None yet
5 participants