Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

Identity UI Unsuitable #1825

Closed
tstivers1990 opened this issue Jun 11, 2018 · 13 comments
Closed

Identity UI Unsuitable #1825

tstivers1990 opened this issue Jun 11, 2018 · 13 comments

Comments

@tstivers1990
Copy link
Contributor

tstivers1990 commented Jun 11, 2018

The new Identity UI library system introduced with 2.1 is unacceptable, in my eyes. There is too much background magic going on that isn't clear even after scaffolding all the pages. Not all users want to have an entire Areas folder with nothing but Identity in it while the rest of their project lives in the project root.

After scaffolding all the pages, I've noticed some pages have [AllowAnonymous] yet no pages have [Authorize], implying requiring authorization is the default. Yet, nowhere in the code is this made clear. Not in the Startup configuration. Not in Program.cs. Not in any files under the Identity folder. Nowhere. Leaving me to believe there's just some background magic going on that is being deliberately hidden from us.

The only current option also seems to be using Razor Pages for the Identity UI. Some of us want full control over how we use Identity so that we can customize it to our needs. The current setup is simply unacceptable. If my entire project is using MVC, I don't want Identity living in its own folder off in la-la land as Razor Pages. It makes the project structure a mess, and there's just no reason for it.

Please either fix the options so that those who want it can take full control of Identity and use it as they wish, whether that's the MVC approach or the Razor Pages approach. Or provide us with sufficient documentation so that we may add Identity to a blank project without using the Identity UI library or some magic scaffolding voodoo. As it stands now, there doesn't appear to be any documentation on Identity that doesn't rely on the new scaffolding system and Identity UI.

@gpcottle
Copy link

I have to say I agree. I have just been looking at the same thing, thinking I could make use of it in an application I am starting. At the moment, I am not willing to consider it as I cannot control it. When I get it back in a MVC form, where I can see the models and the controllers, then I will reconsider. In the meantime, it is dead in the water and I will go back to 2.0 for the time being.

@blowdart
Copy link
Member

@blowdart blowdart added this to the Discussions milestone Jun 11, 2018
@brockallen
Copy link

Perhaps the comment I can add here is that it's very rare that in a real production app anyone would really use the default ASP.NET Identity UI/templates. They will certainly be a starting point, but they always need tweaking/customizing. This is certainly the case for every identity project I work on or that I come across in my work.

So I think I agree that the more the UI is encapsulated the harder it is to use as that template/starting point. And the more there are tacit behaviors/features, then the harder it is to understand and take control of those.

@javiercn
Copy link
Member

@tstivers1990 Thanks for getting in touch with us.

The new Identity UI library system introduced with 2.1 is unacceptable, in my eyes. There is too much background magic going on that isn't clear even after scaffolding all the pages. Not all users want to have an entire Areas folder with nothing but Identity in it while the rest of their project lives in the project root.

The default UI is a starting point suitable for applications where there's not a lot of need to fully customize the UI. I'm not sure what background magic you are referring too; specially after scaffolding the pages, the mechanics of how it works are pretty similar to the 2.0 version in the templates, with the main exception that all the code is now in its own area. We put Identity in an area to isolate the code from the rest of the application and make sure that it doesn't negatively impact the rest of your application in a way that is not obvious.

After scaffolding all the pages, I've noticed some pages have [AllowAnonymous] yet no pages have [Authorize], implying requiring authorization is the default. Yet, nowhere in the code is this made clear. Not in the Startup configuration. Not in Program.cs. Not in any files under the Identity folder. Nowhere. Leaving me to believe there's just some background magic going on that is being deliberately hidden from us.

This is not the case. AllowAnonymous doesn't imply authorization, it only implies that no matter what, that page should be accessible to unauthenticated users. Previous versions of the templates would break when you added authorize as a global filter, hence we added [AllowAnonymous] to the pages where it applies.

The only current option also seems to be using Razor Pages for the Identity UI. Some of us want full control over how we use Identity so that we can customize it to our needs. The current setup is simply unacceptable. If my entire project is using MVC, I don't want Identity living in its own folder off in la-la land as Razor Pages. It makes the project structure a mess, and there's just no reason for it.

Maintaining two versions of the same codebase (MVC and Razor Pages) is very expensive for us and there are no real benefits from having an MVC implementation compared to a Razor Pages version. Both flavors are fully in ASP.NET. Moving the code out of the area should be relatively straight forward, as well as converting it to MVC from Razor Pages. It probably simply involves moving the pages from the area into your main app Pages folder and calling AddIdentity instead of AddDefaultIdentity.

Please either fix the options so that those who want it can take full control of Identity and use it as they wish, whether that's the MVC approach or the Razor Pages approach. Or provide us with sufficient documentation so that we may add Identity to a blank project without using the Identity UI library or some magic scaffolding voodoo. As it stands now, there doesn't appear to be any documentation on Identity that doesn't rely on the new scaffolding system and Identity UI.

The default UI is completely optional. You should be able to just scaffold the pages into your project and do whatever you want from there, whether that's convert them to an MVC flavor or move them out of the area into your main project.
@Rick-Anderson Is this something we can get documented?

Perhaps the comment I can add here is that it's very rare that in a real production app anyone would really use the default ASP.NET Identity UI/templates. They will certainly be a starting point, but they always need tweaking/customizing. This is certainly the case for every identity project I work on or that I come across in my work.

So I think I agree that the more the UI is encapsulated the harder it is to use as that template/starting point. And the more there are tacit behaviors/features, then the harder it is to understand and take control of those.

@brockallen The default UI is a starting point and a suitable option when you need to do simple customizations. For full control we recommend you scaffold the pages and change them to suit your needs.

Hope this helps.

@tstivers1990
Copy link
Contributor Author

This is not the case. AllowAnonymous doesn't imply authorization, it only implies that no matter what, that page should be accessible to unauthenticated users. Previous versions of the templates would break when you added authorize as a global filter, hence we added [AllowAnonymous] to the pages where it applies.

What I meant by this is that it's not obvious where authorization is required, and where it isn't. I'm guessing this is caused by the fact that configuration is now hidden behind services.AddDefaultIdentity<T>() which isn't very well documented. Probably the largest chunk of the frustration being caused by the whole thing is the lack of solid documentation.

I don't mind having to write my own implementation from scratch. Just document the Identity library so we know how to use it is all I ask.

@Rick-Anderson
Copy link
Contributor

Rick-Anderson commented Jun 11, 2018

The default UI is completely optional. You should be able to just scaffold the pages into your project and do whatever you want from there, whether that's convert them to an MVC flavor or move them out of the area into your main project.
@Rick-Anderson Is this something we can get documented?

Scaffold Identity
Applications that include Identity can apply the scaffolder to selectively add the source code contained in the Identity Razor Class Library (RCL). You might want to generate source code so you can modify the code and change the behavior. For example, you could instruct the scaffolder to generate the code used in registration. Generated code takes precedence over the same code in the Identity RCL.

@tstivers1990
Copy link
Contributor Author

Scaffold Identity
Applications that include Identity can apply the scaffolder to selectively add the source code contained in the Identity Razor Class Library (RCL). You might want to generate source code so you can modify the code and change the behavior. For example, you could instruct the scaffolder to generate the code used in registration. Generated code takes precedence over the same code in the Identity RCL.

This documentation is absolutely useless. It doesn't document how Identity works as a library. We need clear documentation that outlines how Identity works. Scaffolding only tells you how the templates use Identity. It doesn't tell you how Identity can be used, or what your options are if you're looking to customize it. Identity itself really needs to be documented, imo.

@Rick-Anderson
Copy link
Contributor

Intro to Identity
Identity is enabled for the application by calling UseAuthentication in the Configure method. UseAuthentication adds authentication middleware to the request pipeline.

@Rick-Anderson
Copy link
Contributor

@tstivers1990

This documentation is absolutely useless. It doesn't document how Identity works as a library.

Thanks for your candid appraisal of my document. That document is focused on scaffolding Identity, not explaining Identity. See Intro to Identity and associated docs.

@tstivers1990
Copy link
Contributor Author

tstivers1990 commented Jun 11, 2018

How is one supposed to determine what parts of the website require authorization and what parts don't, when the default project template leaves no code making this clear? A lot seems to be hidden behind services.AddDefaultIdentity<IdentityUser>() in the interest of making things easier for new users. But, in doing so, things have become more magic and less obvious. There's already an issue (dotnet/AspNetCore.Docs#6367) open for this for almost a month with no update.

Add custom user data to Identity relies entirely on the scaffolding system. If you're not looking to use this system, you're out of luck. It fails to explain how customizing user data actually works. It doesn't explain the fact that you need to modify the call to AddDefaultIdentity<T>(). It doesn't explain the fact that you need to modify your DbContext to inherit from IdentityDbContext<T>.

I apologize if you take offense at my bluntness when it comes to the documentation. It's not intended to offend. But the theme I've noticed with the new documentation is that it relies very heavily on the scaffolding system. I think that's a bad approach, and I think that because as a user trying to familiarize myself with all of the changes that have happened since the pre-release days, a lot of questions have gone un-answered and a lot of time has been spent digging through GitHub repos to try and figure out how things work.

@javiercn
Copy link
Member

@Rick-Anderson I think that what @tstivers1990 wants is better documentation on how to plug-in identity without relying on the scaffolder.

@tstivers1990 The scaffolder is just a vehicle to delivering the code into your application. If you want full control, scaffold once and remove everything you don't need.

@Rick-Anderson These are the important bits to know if you have scaffolded the code into your app and don't want to use default identity at all.

Configure MVC to authorize the Identity pages that require authorization:

services.AddMvc()
   .AddRazorPages(options =>
       {
           options.AllowAreas = true;
           options.Conventions.AuthorizeAreaFolder("Identity", "/Account/Manage");
           options.Conventions.AuthorizeAreaPage("Identity", "/Account/Logout");
      });

Configure the cookie to point to the identity pages paths

services.ConfigureNamedOptions<CookieAuthenticationOptions>(options =>
{
    options.LoginPath = $"/Identity/Account/Login";
    options.LogoutPath = $"/Identity/Account/Logout";
    options.AccessDeniedPath = $"/Identity/Account/AccessDenied";
});

Regarding the fact that we rely on the scaffolder in our docs, its totally fine. The alternative is to tell you to copy paste a bunch of code in the docs into your project which is inevitably going to go out of sync. The scaffolder is the source of truth.

Hope this helps clarify things a bit.

@javiercn
Copy link
Member

Closing this issue as we are taking action on dotnet/AspNetCore.Docs#6990.
Thanks for bringing this issue to our attention.

@Ponant
Copy link
Contributor

Ponant commented Jul 19, 2018

Same comment as @brockallen here dotnet/AspNetCore.Docs#6146 and here dotnet/AspNetCore.Docs#6301 . It is creating too much confusion for only getting unnecessary aesthetics and makes it more difficult for people to get work done because they will have to modify the code anyway to meet compliance.
I can only strongly suggest to:

  1. Remove the ui library from the default template and make docs to instruct the user on how put it all in a library (i.e. we basically reverse the process)
  2. Remove AddDefaultIdentity from the universe, or the older one. In the case where the AddDefaultIdentity is chosen, clarify in the docs what is meant by Default by enumarating default values or behavior.

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

No branches or pull requests

7 participants