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

Use in existing IdentityServer setup #55

Open
ruant opened this issue Aug 15, 2018 · 45 comments
Open

Use in existing IdentityServer setup #55

ruant opened this issue Aug 15, 2018 · 45 comments
Assignees
Labels
enhancement New feature or request priority: high

Comments

@ruant
Copy link

ruant commented Aug 15, 2018

For people having identity server running already, any plans on making a guide on how to get this awesome Admin UI to work?

Maybe there has to be some work done with exposing setup options like naming, using existing dbcontexts etc.

@skoruba skoruba self-assigned this Aug 16, 2018
@skoruba skoruba added the enhancement New feature or request label Aug 16, 2018
@ruant
Copy link
Author

ruant commented Aug 16, 2018

Especially if it's planned to release this UI as a simple injectable middleware as issue #28 is about.

@skoruba
Copy link
Owner

skoruba commented Aug 16, 2018

@ruant Sure, I will add this use case to the upcoming video tutorial. It's a great idea. 👍

We are currently working on a project template for dotnet cli. It will be possible to use like this:
dotnet new skoruba.is4admin. #51
I think - it might be useful for the better customization. You can define your administration name, default admin role name etc.

After that I want to create the nuget packages with UI and BL & EF layers, but I think that takes more time to complete. :)

Thank you for your feedback.

@Brice-xCIT
Copy link
Contributor

Hi! This issue to me should be priority number one. After spending several hours in the code I still don't see how to use this with an existing installation. Somehow the code forces consumers to use its own EF migration and database schema, which bundles ASP.NET Core Identity and IdentityServer4.EntityFramework models into one single DB and DbContext. I tried to decouple both of these things to bind them to two different EF connections, without any success. Similarly, using this single DB as operational store, config store and identity store in vanilla IS4 fails because (from what I understand) migrations are not compatible.

I would really love some doc about how to use this nice admin UI with the plain old IS4 quickstart at http://docs.identityserver.io/en/release/quickstarts/8_entity_framework.html.
At least, is it even possible?

@skoruba
Copy link
Owner

skoruba commented Oct 2, 2018

Hi,
I absolutely understand your requirements, I will create a documentation for that.
Please for now - take a look at this branch - feature/extensible-aspnet-identity -
https://github.com/skoruba/IdentityServer4.Admin/tree/feature/extensible-aspnet-identity
Currently, I am working on better using of this administration with the existing instance of the IS4.
Do you want to use the administration only for IS4 or with Asp.Net Core Identity?
Thanks!

@Brice-xCIT
Copy link
Contributor

Thanks for your message and responsiveness, @skoruba!
I found out about the branch right after writing the message, and although it definitely goes into the right direction, as far as I can see AdminDbContext still wraps the three stores.
IS4+Asp.Net Core Identity is what I'm after, and truthfully I could even consider using AdminDbContext's integrated DB migration instead of the IS4 quickstart's suggested design, if only I could figure out how IS4 is supposed to connect to this integrated DB at all, either through Core Identity as an identity store, or through EF as operational or config store. I guess, that's what Skoruba.IdentityServer4.AspNetCoreIdentity is doing?

@skoruba
Copy link
Owner

skoruba commented Oct 2, 2018

It’s absolutely possible to split the AdminDbContext into more DbContexts like you mentioned with IS4 quickstart sample.
The repositories/services are designed for generic DbContext and you can choose an approach of one or more dbcontexts.
Please take a look at Startup class for extension method that added into DI the repositories and services - AddAdminServices - there you can specify one or more dbcontexts.
BTW: I know that it might be probable unclear, therefore I will prepare a doc.
I am very happy for your points!
Thanks!

@patrick-heuer
Copy link

I have the same requirement: I want to enhance my existing SSO service (C#/Core 2.1, Identity Server 4, ASP.NET Identity, Sqlite, Kestrel webserver) with your great Admin UI.

But I don´t know how to do!

I have seen in the "official" IdentityServer4 - AdminUI tool from Rock Solid, that you can add "UseAdminUI" in the startup.cs and then an additional admin-url is available... that is very easy integration, but I am not sure if I like their tool and also not their licences - so my hope is on your solution :)

@skoruba
Copy link
Owner

skoruba commented Oct 17, 2018

@PKrause79 I will update the README with getting started guide - probable this week :)
Thanks!

@patrick-heuer
Copy link

I am thinking, why you have separated (standalone) services for IDS4, ASP.NET Identity and your Admin-service? What are the reasons for that? Maybe your appraoch is better and its not a good design to integrate all the 3 artefacts as a "SSO-application" in only one service - what do you think?

@skoruba
Copy link
Owner

skoruba commented Oct 21, 2018

I have created two separated web apps - one for IS4 with Asp.Net Core Identity and AdminUI - I think it is up to you - how you will design this project structure - I prefer this approach. :)

@skoruba
Copy link
Owner

skoruba commented Oct 24, 2018

@PKrause79 - I have updated the master branch with some information how to use current solution with existing IdentityServer4 instance.
Please take a look, thanks!

@OculiViridi
Copy link

OculiViridi commented Nov 8, 2018

@skoruba Hi! I'm also trying to add your AdminUI on my existing IS4 instance, but by following the readme I'm not really sure about how to make them works together...

This is my actual IS4 configuration.

DbContext

public class AuthDbContext : IdentityDbContext<User>
{
	public AuthDbContext(DbContextOptions<AuthDbContext> options)
		: base(options)
	{ }

	protected override void OnModelCreating(ModelBuilder builder)
	{
		base.OnModelCreating(builder);
	}
}

Startup class

public class Startup
{
	public void ConfigureServices(IServiceCollection services)
	{
		services.AddDbContext<AuthDbContext>(options =>
			options.UseSqlServer(connectionString,
				sqlServerOptions => sqlServerOptions
					.MigrationsAssembly(assemblyName)));

		services.AddIdentity<User, IdentityRole>(options =>
			{
				options.Password.RequireDigit = true;
				options.Password.RequiredLength = 10;
				options.Password.RequireLowercase = true;
				options.Password.RequireUppercase = true;
			})
			.AddEntityFrameworkStores<AuthDbContext>()
			.AddDefaultTokenProviders();

		services.AddMvc(config =>
			{
				// HTTP 406 when not supported format is requested by client
				config.ReturnHttpNotAcceptable = true;
			})
			.SetCompatibilityVersion(CompatibilityVersion.Version_2_1);

		services.Configure<IISOptions>(iis =>
			{
				iis.AuthenticationDisplayName = "Windows";
				iis.AutomaticAuthentication = false;
			});

		var builder = services.AddIdentityServer(options =>
			{
				options.Events.RaiseErrorEvents = true;
				options.Events.RaiseInformationEvents = true;
				options.Events.RaiseFailureEvents = true;
				options.Events.RaiseSuccessEvents = true;
			})

			// Configuration data store
			.AddConfigurationStore(configurationDb =>
				{
					configurationDb.ConfigureDbContext = db => db.UseSqlServer(connectionString, sql => sql.MigrationsAssembly(assemblyName));
				})

			// Operational data store
			.AddOperationalStore(operationalDb =>
				{
					operationalDb.ConfigureDbContext = db => db.UseSqlServer(connectionString, sql => sql.MigrationsAssembly(assemblyName));
				})

			.AddAspNetIdentity<User>();
		}
	}
}

What am I supposed to do on MyProject.AuthAdminUI.STS.Identity and MyProject.AuthAdminUI.Admin projects?

Also, do I need to create a separate database for the Admin UI or can I add the required tables to the current one used by IS4? Actually, all my IS4 DbContexts uses the same database, so .NET IdentityUser, Persisted and Configuration are different DbContexts but all tables are on the same database.
Is there any possible conflict adding the Admin UI ones?

@andy-cheetham
Copy link

Was there any progress made on a document detailing how to add this very nice UI to an existing IS4 project?

Cheers

@ghost
Copy link

ghost commented Feb 7, 2019

I tried to use this AdminUI with my existing DbContext & IS4, but after spending 8 hours I decided to just write my own admin panel with the help of cshtms in skoruba. I wish it could be much more easy to setup

@skoruba
Copy link
Owner

skoruba commented Feb 7, 2019

@elyasnew Did you try using this Admin via dotnew template or clone whole solution?

Please, can you give me some points for improvements? I am happy for any suggestions.

Thank you for your feedback!

@ghost
Copy link

ghost commented Feb 10, 2019

I don't know why I'm redirecting to Access Denied Page while I had set the role and user role properly

@Brice-xCIT
Copy link
Contributor

@elyasnew I had this issue also during integration. Did you add the "roles" identity resources? Checklist: you need in DB 1) the role 2) an admin user with the right role 3) the roles identity resources with the role scope (@skoruba why is this even needed? I can't find mention of it in the code) 4) the right client with allowed scopes

@Brice-xCIT
Copy link
Contributor

Brice-xCIT commented Feb 11, 2019

I finally managed to integrate this project with my IdentityServer4 install. The core issue I was trying to solve was to separate the data contexts for configuration, persisted grants, logs and identity. During my early explorations, I discovered that IdentityServer4.Admin assumes all of these things should be in the same data context (i.e. the same database) and that, considering the amount of work put into the STS app, this project seemed aiming at creating a complete, integrated administrable IdentityServer solution rather than “just” an Admin UI.

This said, I really needed an admin UI. So I set myself to dive in the code and change everything needed to support my use case, while still staying compliant with the original design intent. And boy was it not straightforward. In total, from the moment I first discovered this project to the moment I got everything working, it took me about an aggregated week of work.

Here’s what I would have liked to do:

  1. Create a new ASP.NET Core application
  2. Install Skoruba.IdentityServer4.Admin NuGet package into the project.
  3. Add services.AddIdentityServerAdminUI(...) with a bunch of options regarding data contexts and user types.
  4. Add app.UseIdentityServerAdminUI().
  5. Add some users and resources into IS4’s database.
  6. Click run and marvel.

Here’s what I actually had to do:

  1. Clone the IdentityServer4.Admin repo

  2. Open the IdentityServer4.Admin solution. We’re going to change lots of stuff in almost all projects...

  3. To bind to existing IdentityServer config and operation database...

    1. Write 3 new DbContexts classes that split AdminDbContext in 3 to bind to 3 different connections.
    2. Add new configuration keys for 3 different connections and migration assemblies.
    3. Write new startup helper AddDbContexts and RegisterDbContexts with 3 context type variables to bind each DbContext to a different connection and migrations assembly from configuration.
    4. Use startup helper variant AddAdminServices with 3 context type variables
  4. To bind to existing ASP.Net Core Identity user database

    1. Reference own IS instance project into the code.
    2. Use variant of startup helper AddAuthenticationServices with own IdentityDbContext and user types (and add the custom DbContext with the right connection string and migrations assembly before that)
    3. Write a variant of AddAdminAspNetIdentityServices that takes a custom data context for identities and another one for grants
    4. Change the signatures of IPersistedGrantAspNetIdentityRepository and IPersistedGrantAspNetIdentityService and their implementations to separate the identity context from the grant context
    5. Fix queries and methods in the aforementioned implementations by using either the identity context or the grant context instead of the common one.
    6. Remove IAdminPersistedGrantIdentityDbContext since it's not needed anymore.
    7. Refactor IPersistedGrantAspNetIdentityRepository and IPersistedGrantAspNetIdentityService again to remove unused generics, so that GrantController can work with custom DbContexts.
    8. Do the same thing for pretty much every single service type whose interface is generic (and doesn't need to be), so that controllers can work without referencing AdminDbContext.
    9. Struggle to remove generics from IdentityController and realize that the entire "Dto" model forbids it. Get rid of IdentityController and write a Mvc feature provider (and name convention) that instanciates a generic BaseIdentityController using own user type and derived Dto types. Realize it's not working either because generic user "Dtos" are everywhere in views.
    10. Change all user Dto generic types to string instead of int in views. Realize that this makes the whole code not reusable for anybody else who does not have the same use case.
    11. Fix all internal tests and other code that references all these types that changed.
  5. Making it all work

    1. Add “role” identity resource (not sure why- no references in the code other than in OIDC auth requirements), add client for the admin UI, add admin user with correct administrative role (see source code) into the existing IdentityServer database.
    2. Generate a migration for the logs database.
    3. Run and marvel.

I’m sure you will understand that it is very hard for developers to spend this amount of effort to refactor – let alone to understand – a library’s code to suit their needs. I had to refactor interfaces and implementation signatures in almost all layers (and most of the time I ended up simplifying the code rather than complexifying it...).

I forked this repo and commited my changes at https://github.com/xcit-lab/IdentityServer4.Admin/tree/feature/db-context-separation
There people encountering the same issue as me can get bootstraped. I even added Startup and config comments to help understand the differences between this code and the previous one. In this state, the fork is not fully generic yet, because the views and the identity services still rely heavily on Dto<> types. This dependency makes the generic types bleed up until Startup, which is probably reasonable for services, but super weird for views. In the future, to really make this lib fully reusable out of the box, the views should in my opinion be completely separated from generic Dtos (probably with a ViewModel computed from the controllers and fed to the views).

Having now spent a good bunch of time in the internals of this lib, I feel like it’s really close to be reusable and deployable in various settings. However before this happens, I see major hinderances that will need to be resolved:

  • Completely separate all code from AdminDbContext. Any method that has AdminDbContext as a generic parameter is a design issue. It means “I assume you will use one context”. In the current code, most such methods do not offer a variant for separated contexts. All these methods should either offer such a variant, or be rewritten to use the interfaces atomically. It’s okay to offer variants to support the use case where people want AdminDbContext, but it shouldn’t be a requirement. (* This is the main contribution of the fork, and in the process, the below problems emerged)
  • Completely separate all controllers from user-defined configuration. This means no generic type in any dependency of any controller, or if generics are unavoidable, use Mvc mechanisms to generate generic controller instances at runtime. (* This was done in the fork)
  • Completely separate all views from user-defined configuration. This means no more Dtos in views, or make Dtos inherit non-generic types that expose invariant representations on their data. More likely, compute view models in controllers by calling on user-customizable services that do provide the configuration or conversions (e.g. defining key type and converting it to string). (* not done in the fork because it seemed like a massive refactor)
  • Provide implementations of separated data contexts. I had to write new adapter classes to bind to vanilla IS data contexts. That’s weird for such a basic use case. (* This was done in the fork)
  • Provide more helpful startup helpers, or a builder class to help configurate. The ones that exist are very tied to AdminDbContext. I had to write many variants to support my use case. A builder with nice config methods would also go a long way.
  • Extract base lib project and provide NuGet package. Last but not least: most of the .Admin code should be extracted into a library project, so that it could be used easily from an app. And then there would be a .Admin.App that would use this extracted lib as well. This way the code could be usable right away in any ASP.Net Core app. (Issue encapsulate project to injectable parts #28 ftw!)

I hope I provided enough details to help those who would want to integrate the app in its current state into their own IS install, and that this refactor will be considered for an upcoming release :) I don’t intend to maintain the fork synced with upstream, so if the changes make sense to you @skoruba I can maybe PR them into a dev branch.

Sorry for the lengthy message, I hope this is useful.

@ghost
Copy link

ghost commented Feb 11, 2019

Thank you @Brice-xCIT for your reply, I absolutely agree with you that the configuration should be as simple as just writing app.UseIdentityServerAdminUI().
I have modified several IS4 classes and for using AdminUI I should modify all the Dto, Entity classes and services again, and as I mentioned before, after 2 days messing around with it, I decided just to get help of the cshtmls and write my own dashboard.

@xmichaelx
Copy link
Contributor

@Brice-xCIT Before we get to business - from the bottom of our hearts - thank you for spending a week of your working time on IS4.Admin. We're both humbled and honored that you're willing to contribute your improvements back to the project.

I think that all of your suggestions are valid and should be included in the project. Lack of manpower to add everything as fast as we would want to is the only reason why it's not already there. Thank you for providing that additional push.

Now to business - please open PR to the dev branch (and please preferably merge back any commits that are already in the dev branch). Currently we're working on password resetting, 2FA and profile management and audit logging on the dev branch - just so you know in which areas progress is happening right now.

I'll alocate some of my time this week for reviewing any new PRs.

@skoruba
Copy link
Owner

skoruba commented Feb 11, 2019

Thank you so much for you great feedback @Brice-xCIT
I definitely agree with @xmichaelx - it would be great if you can send a PR like mentioned @xmichaelx - from latest dev branch.

I know that these things need some improvements for easier setup. :)

@Brice-xCIT
Copy link
Contributor

Hello @skoruba and @xmichaelx, sorry for coming late to the party! Thanks for your messages, I'm glad. I haven't had the time to PR yet, and I see that @skoruba beat me to integrating the changes, which is totally okay 😄. I will update the dev branch and reintegrate with my setup to see if it still works. I feel like we're getting very close to something that works out of the box, so I might work a bit more on Startup configuration to that end.

@skoruba
Copy link
Owner

skoruba commented Feb 19, 2019

Hi @Brice-xCIT - thank you for your response, I will be happy for your PR. Feel free to contribute. :)

@amideveloper
Copy link

Hi @skoruba,

I was able to configure and run skoruba admin for my application and now am trying extend few classes in Admin Console to display few additional information. On this note i would like to add a new property to IdentityUser class. What i thought is to extend the dbcontext but after discussions with team now trying with extending the IdentityClass. Please help with a correct approach. Tried to refer this url https://github.com/skoruba/IdentityServer4.Admin/tree/feature/extensible-aspnet-identity but got response as 404

@skoruba
Copy link
Owner

skoruba commented May 13, 2019

HI @amideveloper

you can extend this object - https://github.com/skoruba/IdentityServer4.Admin/blob/master/src/Skoruba.IdentityServer4.Admin.EntityFramework.Identity/Entities/Identity/UserIdentity.cs and according new properties you should extend your UserDto object as well.

Then you need solve mapping between these new properties (if needed):
https://github.com/skoruba/IdentityServer4.Admin/blob/master/src/Skoruba.IdentityServer4.Admin.BusinessLogic.Identity/Mappers/IdentityMapperProfile.cs#L45

Thanks!

@amideveloper
Copy link

@skoruba Thanks for the solution.

@narbit
Copy link

narbit commented Jan 22, 2020

Hi Jan (@skoruba),

What is the current recommended way to integrate with existing instance of IdP (backed by Asp.Net Identity) using rc1?

I've been going over @Brice-xCIT comments and your joint PR activity but can't figure out where to start. In particular, I really liked Brice's "what I would have liked to do" steps that mention pulling NuGet package and registering services.AddIdentityServerAdminUI(...) but it seems it hasn't been implemented that way in rc1? Do I need to implement some flavor of "what I actually had to do" steps?

Thank you!

@Brice-xCIT
Copy link
Contributor

Brice-xCIT commented Feb 7, 2020

Hi @narbit, the easy service registration you mention is available in the branch extract-ui-package. Sadly it had not been merged into rc1 yet (and is now lagging far behind master). This being said it works and this is how one can use it:

string mainAssemblyName = typeof(MyIdentityServer.Startup).Assembly.GetName().Name;
string adminAssemblyName = typeof(MyIdentityServerAdminStartup).Assembly.GetName().Name;
services.AddIdentityServerAdminUI<MyApplicationDbContext, MyApplicationUser>(options =>
{
	options.ApplyConfiguration(configuration);

	options.ConnectionStrings.SetMigrationsAssemblies(mainAssemblyName, adminAssemblyName);
});

and

app.UseIdentityServerAdminUI();

with in appsettings.json:

"ConnectionStrings": {
	"IdentityDbConnection": "Data Source=(LocalDb)\\MSSQLLocalDB;database=Id-UserData;trusted_connection=yes;",
	"ConfigurationDbConnection": "Data Source=(LocalDb)\\MSSQLLocalDB;database=Id-Configuration;trusted_connection=yes;",
	"PersistedGrantDbConnection": "Data Source=(LocalDb)\\MSSQLLocalDB;database=Id-PersistedGrant;trusted_connection=yes;",
	"AdminLogDbConnection": "Data Source=(LocalDb)\\MSSQLLocalDB;database=Id.Admin-Log;trusted_connection=yes;"
},
"AdminConfiguration": {
	"IdentityServerBaseUrl": "http://idsrv",
	"IdentityAdminBaseUrl": "http://idadmin",
	"IdentityAdminRedirectUri": "http://idadmin/signin-oidc"
}

with MyApplicationDbContext and MyApplicationUser being the extended ASP.Net Core Identity context and user.

@narbit
Copy link

narbit commented Feb 7, 2020

Hi @Brice-xCIT , thank you for sharing! Unfortunately after spending couple of days on this and not making progress I had to get moving and went with this project https://github.com/brunohbrito/JPProject.IdentityServer4.AdminUI instead. Which had its own challenges but overall was much easier to adapt to with an existing IdP setup.

@skoruba
Copy link
Owner

skoruba commented Feb 7, 2020

Hi guys, thanks for feedback on this. I will improve docs for this use case - how to connect to existing IS4.
@Brice-xCIT - unfortunately I did not have a time to adapt your PR with latest version. I will review your PR and consider how to continue on your great work.

@jmatheti
Copy link

jmatheti commented Jul 1, 2020

Greatly appreciate your efforts and time on this project.

I was also evaluating JPProject alongside this admin UI project. We are decided to extend the existing IDM/STS to include the Admin area (along with the User Profile). One advantage I see is less maintenance as there is no need of additional client, works with the existing authorization policies (I have broken the parts into permissions base by extending the existing asp net identity framework).

I was really wondering why would't that be considered in this project.!! I know security wide, its recommended to add few pages in the central IDM. But again if we have this as an another client, its backing with STS for security. Server pages are more secure than the javascript application.

Admin UI as a razor library , then register the services in the startup. I know the complete integration can not be smooth as the product needs to be integrated. Perhaps register the existing dbContext (ConfigurationDbContext, IdentityDbContext, PersistenseDbContext). So the Admin UI uses its own additional tables required for audit.

We are reusing the IDM for many of our applications by skinning the client specific styles. So it works out best going with the permissions based and having a role and permissions management pages.

@jmatheti
Copy link

jmatheti commented Jul 6, 2020

@skoruba do you mind taking a minute on the above query. I just want to make sure about the approach I am going.

@Brice-xCIT
Copy link
Contributor

Hi @skoruba and congrats for the v1.0 release. Now that the lib is out there, would there be a way for you to revisit this issue? What are the current plans to simplify integration of the lib?

@skoruba
Copy link
Owner

skoruba commented Nov 10, 2020

Hi @Brice-xCIT - I am glad to see you for long time again. 😊 I agree with you, this is still little-bit harder to upgrade to the newest version, especially UI part. I think - currently good way for some cases could be a docker image which is available. But I will wonder about some another approach. What do you think?

@Brice-xCIT
Copy link
Contributor

From what I can see, in its current state this project only supports using the app as-is, in a standalone deployment. For this, it's great. What is lacking however is supporting using the app as a library module, not as an app. I believe that is what the various people who have commented on this and other similar issues were going for. In a nutshell, we want to be able to inject the Admin UI into our projects, without having to change a single line of code in any Skoruba.IdentityServer4.* project. To be fair, why else would you distribute NuGet packages, if not to integrate them into existing projects?

From that perspective, I believe the way to go is to just follow ASP.NET Core ecosystem conventions. In all libs I've integrated so far, the pattern is always the same: a single call in ConfigureServices with options, and a single call in Configure to bootstrap middlewares. Anything that will require me to perform several calls in Startup is already too low-level.

I don't think it would require a big effort to slightly reorganize the library code to support that. As we've previously discussed, turning the current project .Admin (and .Admin.Api?) into Razor libraries is quite fast and efficient. And providing a configuration builder that allows integrators to inject the services is quite standard.

Both of these things have already been done and are currently in the extract-ui-package branch, but sadly it is lagging so far behind dev that it maybe even should be entirely re-done. Although I would really need this if I were to upgrade my current install of .Admin, I'm not sure I have the courage to do this work if it doesn't ever get merged.

That's why I wonder what your strategy is :)

@skoruba
Copy link
Owner

skoruba commented Nov 11, 2020

I would like to transfer UI part into Razor Class Library, but I remember that I had some issue if I wanted to rewrite existing view, this did not work properly in the past in my case. Currently I am almost done with version 2.0.0 - which will be published as 2.0.0-beta1, because there was a lot of changes and it needs to be more tested. So if you have a time to help me transfer 2.0.0 to Razor Class Library, it would be great and I am definitelly open to rework it.

Thanks!

@Brice-xCIT
Copy link
Contributor

Thanks @skoruba for your message. I'd be willing to help early next year. However I'm not sure transferring the UI part to an RCL will be enough to solve this issue completely. There also needs to be some easier configuration model for DI (like #255) and possibly some separation of the .Admin and .Admin.API core code into their own libraries (like #225). Does this make sense to you?

@skoruba
Copy link
Owner

skoruba commented Dec 19, 2020

Hey @Brice-xCIT - yes, I really appreciate your help on this feature. I would like to transfer the UI part into RCL.
My idea is something very similar to your original PR (#225) - Create package - Skoruba.IdentityServer4.Admin.UI - which was contains the views/controllers/styles/js. And in existing project called something like - services.AddIdentityServer4AdminUI(settings => ...).

@Brice-xCIT
Copy link
Contributor

Hi @skoruba. I'm starting to take a closer look at this issue, and to double down on my previous comment, I'm struggling to understand how just separating the UI into its own package will be enough to solve this issue. Indeed, an existing application would also need to replicate the startup protocol in Admin:Startup.cs, isn't that right? And in order to do that, it would also need to have access to Admin:StartupHelpers.cs, which in turn would have to be bundled into the NuGet package.

Do you believe StartupHelpers and its dependencies should be in package Admin.UI? If you do, that would mean that a user wanting to use this in an existing setup would only need to include a ref to the Admin.UI package to their project. If you don't, it means that there needs to be another NuGet package (Admin.Core? Admin?) that lets users access the helpers and all the code that is necessary to bootstrap admin UI and services during an arbitrary app's startup.

What do you think?

@skoruba
Copy link
Owner

skoruba commented Jan 4, 2021

Hey @Brice-xCIT - My idea is following:

  • Create package Skoruba.IdentityServer4.Admin.UI, which will contains: (definitely this should be created from branch release/2.0.0-beta1)
    • Copy folders: Controllers, Views, Resources, wwwroot
    • Modify StartupHelpers which will be part of UI package
      • I would definitely prefer ship all dependencies with UI and via line services.AddIdentityServer4AdminUI(settings => ...) add everything (BusinessLogic, DAL etc.)

What do you think?

@Brice-xCIT
Copy link
Contributor

Thanks @skoruba for the clarification. It makes sense to me now. At first glance I think that some other folders may have to move too (Configuration, Scripts?, Styles?). Do you see anything that specifically should not move to UI?

@skoruba
Copy link
Owner

skoruba commented Jan 6, 2021

@Brice-xCIT - I would also move the Configuration, scripts and style into UI package. 👍

@Brice-xCIT
Copy link
Contributor

I think I did most of the work at https://github.com/xcit-lab/IdentityServer4.Admin/tree/feature/extract-ui-package-v2 but I don't think it's completely ready for a PR yet, although all tests pass. (Left for me to do is the possibility to set the migration assemblies.) I had to move a lot of things and did quite a few refactors there and there. In particular, I'm not too sure about some parts of Startup I moved away (e.g. HSTS).
Would you mind taking a look @skoruba?

@skoruba
Copy link
Owner

skoruba commented Jan 7, 2021

@Brice-xCIT - I have checked your changes and it looks really good. 👌 I think some of things from Startup we will remove from UI package (not sure now) but for start it is more than enough - I am really happy for your work.

@Brice-xCIT
Copy link
Contributor

Thanks for the review! I will tie a few loose ends and submit a PR soon, then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority: high
Projects
None yet
Development

No branches or pull requests

10 participants