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

Extracted core library to use in existing setups #225

Merged
merged 27 commits into from
Apr 4, 2019

Conversation

Brice-xCIT
Copy link
Contributor

This pull request provides the ability to target Skoruba.IdentityServer4.Admin as a netstandard2.0 library. It also adds IServiceCollection and IApplicationBuilder façade extensions as well as an associated public options model in order to configure the app programmatically rather than using appsettings.json.

I took every possible effort to retain compatibility with other use cases that were previously supported (configuration read from appsettings.json) but further testing may be required.

Overall, I hope these changes will make it easier for people like me to use this great library in existing setups. Would Skoruba.IdentityServer4.Admin ship as a NuGet package, it would be now directly usable in an Asp.Net Core app in a similar fashion to how IdentityServer4 itself is used.

This PR helps fixing #55 and #28 as far as I'm concerned, but more testing is probably needed.

Notable changes

  • The privileged way to use the library is now to use services.AddIdentityServerAdminUI(...) and app.UseIdentityServerAdminUI().
    Methods in StartupHelper, although still public, are only used internally by these extension methods, and by test projects.

  • Configuration has been refactored to support programmatic options being passed to services.AddIdentityServerAdminUI(options => ...).
    IConfigurationRoot instances are no longer used internally, but can still be imported using IdentityServerAdminOptions.ApplyConfiguration(IConfigurationRoot)

  • The Asp.NET Core app previously called Skoruba.IdentityServer4.Admin is now called Skoruba.IdentityServer4.Admin.App and uses the same newly introduced extension methods.

  • Configuration interfaces have been removed because they were never really used anyway.

  • All internal code referencing single DbContexts has been removed, because unused.

…pport adding services using configuration from IConfigurationRoot or inline settings
….Add chain to use App assembly for migrations
…rilog configuration into builder extensions.
# Conflicts:
#	Skoruba.IdentityServer4.Admin.sln
#	src/Skoruba.IdentityServer4.Admin.App/appsettings.json
#	src/Skoruba.IdentityServer4.Admin/Configuration/AdminConfiguration.cs
#	src/Skoruba.IdentityServer4.Admin/Configuration/Interfaces/IAdminConfiguration.cs
#	src/Skoruba.IdentityServer4.Admin/Startup.cs
@Brice-xCIT
Copy link
Contributor Author

Aaaaaand it doesn't pass tests. Woops...

@Brice-xCIT
Copy link
Contributor Author

Pfew, all sorted out now 😅

@skoruba skoruba self-requested a review March 20, 2019 14:46
@skoruba
Copy link
Owner

skoruba commented Mar 20, 2019

Thank you @Brice-xCIT 👍
I'll check it. :)

@skoruba
Copy link
Owner

skoruba commented Mar 20, 2019

BTW: It is amazing 👍

@skoruba
Copy link
Owner

skoruba commented Mar 20, 2019

@Brice-xCIT - What about the use case - if you want to rewrite some View/Controller from Skoruba.IdentityServer4.Admin?
I tested the creation of HomeController in Skoruba.IdentityServer4.Admin.App - but it is not possible to rewrite the original HomeController from Skoruba.IdentityServer4.Admin - any idea? :)
Thanks!

@Brice-xCIT
Copy link
Contributor Author

Hmm. In what case would you want to do that?
Intuitively, this would call for extensibility of the base controllers, and the ability to add custom controllers in the options. Would that be only for the home controller or all controllers?

@skoruba
Copy link
Owner

skoruba commented Mar 20, 2019

Maybe I want to use my own implementation of Users view and some extra authorization stuff, so I need access to Controller as well. I want to be able to rewrite Users.cshtml and IdentityController.
So in general for all controllers.
Any idea?
Thanks.

@Brice-xCIT
Copy link
Contributor Author

I think that, for such a big amount of changes and customization, a user would have to download the library code and modify it directly. Which is exactly what they would have had to do before this PR anyway, isn't it? :)

@skoruba
Copy link
Owner

skoruba commented Mar 20, 2019

Yes, I am wondering about razor class library - move these views, controllers into Areas.
Currently is possible to install this project via dotnet new template - where you will have a UI (Views/Controllers/Resources) served in project after installation. :)
But you are right, everything is about compromises, because if you want to create massive changes, it is better download whole solution.
BTW: What do you think about razor class library above?

@Brice-xCIT
Copy link
Contributor Author

I haven't used razor class libraries yet, but it seems really nice and probably the right solution if you want the consuming app to be able to override the pages.

Regarding the dotnet new template command, is the use case again to be able to customize the pages? It seems kind of weird to me that a consuming app would want to change anything more than pure aesthetics, without cloning the source code and modifying it there.

This said, it does seem like RCLs may be the best way to support this use case as well. However, I'm not sure how much of an effort it would be to refactor the core library that way.

@skoruba skoruba changed the base branch from dev to feature/extract-ui-package April 4, 2019 06:29
@skoruba skoruba merged commit ad788f0 into skoruba:feature/extract-ui-package Apr 4, 2019
@skoruba
Copy link
Owner

skoruba commented Apr 4, 2019

Thank you @Brice-xCIT - I will continue on this branch with extracting UI as nuget package using Razor Class Library.

@Brice-xCIT
Copy link
Contributor Author

Awesome, thanks!

@sw-ms-pradipbaria
Copy link

@skoruba, first Big Thanks for this project.
I am looking for extracting UI as nuget package. Can i have approx timeline when it will be available? It will be grate if we use is as package.

@skoruba
Copy link
Owner

skoruba commented Jun 16, 2019

@sw-ms-pradipbaria - Thanks for your request - currently I am working on adding of new API. Separate UI package is in process. @Brice-xCIT did a lot of work on separation of UI into package and I will definetely continue on this, but later. :)

@sw-ms-pradipbaria
Copy link

sw-ms-pradipbaria commented Jun 17, 2019

Thanks for updates @skoruba , will keep eyes on PRs.

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

Successfully merging this pull request may close these issues.

3 participants