AliBazzi / IdentityServer4.Contrib.RedisStore

A persistence layer using Redis DB for operational data and for caching capability for Identity Server 4
https://www.nuget.org/packages/IdentityServer4.Contrib.RedisStore
MIT License
137 stars 48 forks source link

IPersistedGrantStore registered twice #14

Closed SeriousM closed 5 years ago

SeriousM commented 5 years ago

In line https://github.com/AliBazzi/IdentityServer4.Contrib.RedisStore/blob/334f4ba7d7622e8cb71bf503328b444fcf4f4120/IdentityServer4.Contrib.RedisStore/Extensions/IdentityServerRedisBuilderExtensions.cs#L26 the service PersistedGrantStore as IPersistedGrantStore is added to the services which leads to the issue that the wrong IPersistedGrantStore is resolved - in my case the already registered InMemoryPersistedGrantStore. It would be good to remove the existing service first and add then the IPersistedGrantStore.

builder.Services.RemoveAll<IPersistedGrantStore>();
builder.Services.AddTransient<IPersistedGrantStore, PersistedGrantStore>();
AliBazzi commented 5 years ago

Hi @SeriousM Not sure about the context of this, I would ask why you have InMemoryPersistedGrantStore registered in the first place ?

I'm following this implementation as a guideline, and the assumption is that the InMemoryPersistedGrantStore or any similar alternative is not registered in the first place to replace it later.

SeriousM commented 5 years ago

The InMemoryPersistedGrantStore is added by AddIdentityServer which returns the builder that your extension is extending. There is no way to avoid adding the memory implementation.

AliBazzi commented 5 years ago

Again, based on this implementation, I don't see there is a removal for all registeredIPersistedGrantStore before registering the Entity Framework Grant Store.

So, unless I understand what is the issue you are facing particularity, and being convinced why I should not follow the referenced implementation up, I can't proceed into merging this change.

I would suggest to define your own extension method, to remove whatever you need to remove like:

public static IIdentityServerBuilder RemovePersistedGrantStore(this IIdentityServerBuilder builder)
        {
            builder.Services.RemoveAll<IPersistedGrantStore>();
            return builder;
}

That's what I can help you with, until I understand the context.

AliBazzi commented 5 years ago

Looking to hear from you @SeriousM

AliBazzi commented 5 years ago

I will close the issue since no response from your side @SeriousM

SeriousM commented 5 years ago

I want to inform you that I found the root cause of my issue: https://github.com/dadhi/DryIoc/pull/76 Thanks.

AliBazzi commented 5 years ago

Welcome @SeriousM