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

Make it easier to extend #203

Closed
aiscrim opened this issue Feb 26, 2019 · 6 comments
Closed

Make it easier to extend #203

aiscrim opened this issue Feb 26, 2019 · 6 comments
Assignees
Labels
task Task

Comments

@aiscrim
Copy link
Contributor

aiscrim commented Feb 26, 2019

Currently the library relies on a set of repositories that are not easy to extend: the repository classes don't have virtual methods and they have private members instead of protected ones, and some data come from public static classes like ClientConst.
That makes it very difficult to extend the functionalities: for example, I wanted to add SAML 2.0 to the supported Protocol Types in my application, and I had to implement again IClientRepository, copying the whole implementation of ClientRepository and replacing just the GetProtocolTypes() method with one that instead of calling the static ClientConsts.GetProtocolTypes() returns the two protocol types I need.

@skoruba
Copy link
Owner

skoruba commented Feb 26, 2019

@aiscrim - Good point, I agree with you - I will change it.
Thank you 👍

@skoruba
Copy link
Owner

skoruba commented Mar 5, 2019

@aiscrim - Done on dev branch.
Thanks

@CoskunSunali
Copy link

CoskunSunali commented Mar 8, 2019

The same goes for using custom data stores. You expect the custom AdminDbContext to inherit from IdentityDbContext but why? Even Asp.Net Core Identity supports custom data stores that doesn’t inherit from that base class. Identity Server 4 doesn’t have such requirement either. Would you consider implementing ability to support custom data stores instead of requiring to have it inherit from IdentityDbContext?

Edit: I just read the issue #169 and seems you are already aware of the issues. Cheers and thanks for your work.

@skoruba
Copy link
Owner

skoruba commented Mar 8, 2019

@CoskunSunali - Thank you for your suggestion, you are right, we have already done this issue. :)

@CoskunSunali
Copy link

@skoruba Great work, thanks!

@skoruba skoruba mentioned this issue Apr 4, 2019
@skoruba
Copy link
Owner

skoruba commented Apr 4, 2019

Done on master. Please check new release.
Thanks!

@skoruba skoruba closed this as completed Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task Task
Projects
None yet
Development

No branches or pull requests

3 participants