Concrete Dependency in SigningKeyStore

[BenWolstencroft]

As I’m already using EF, I’ve decide to go down that route for the keystore, but I’m getting dependency injection issues.

Our implementation is pretty complex (we extend your EF interfaces and DbContexts with our own in order to add additional functionality), the error I am seeing is lots of the following (for various ServiceTypes and ImplementationTypes, but always while attempting to activate Duende.IdentityServer.EntityFramework.Stores.SigningKeyStore

Error while validating the service descriptor ' ServiceType: Microsoft.AspNetCore.Authentication.IAuthenticationService Lifetime: Transient ImplementationType: Duende.IdentityServer.Hosting.IdentityServerAuthenticationService ': Unable to resolve service for type 'Duende.IdentityServer.EntityFramework.DbContexts.PersistedGrantDbContext' while attempting to activate 'Duende.IdentityServer.EntityFramework.Stores.SigningKeyStore'.

I believe that the issue is due to the fact that the constructor for the SigningKeyStore is expecting a concrete type of PersistedGrantDbContext, rather than the Interface type IPersistedGrantDbContext in https://github.com/DuendeSoftware/IdentityServer/blob/main/src/EntityFramework.Storage/Stores/SigningKeyStore.cs lines 29 and 42

I may be completely wrong here, but this is the most obvious thing I can see that would break it in a scenario where you aren’t simply using the ‘out of the box’ dbContexts

[BenWolstencroft]

Referenced pull request fixes this issue.

[brockallen]

Hi @BenWolstencroft -- The reason I added the concrete dependency was because we needed to handle the concurrency scenarios, but perhaps we already had such checks in the interface base abstraction (tho I doubt it). I'll have a look at the PR.

[BenWolstencroft]

Hi @brockallen - thanks for the response.. if that's the case then I guess I can try adding a transient service to the builder. I'll be honest I thought that the concrete dependency was accidental - the other repositories in the same folder all have the IPersitedGrantDbContext - you guys know the detail better than I do!

[brockallen]

Yea, I know the other locations use the abstraction... this was a first cut, so I can think about it some more. To be honest I don't think I ever liked the DbContext abstraction. Do you use it at all (for testing), by any chance?

[BenWolstencroft]

@brockallen - looking through our codebase, i'll be honest it looks like we've implemented all of our extension methods etc using the IPersistedGrantDbContext as the type parameter, but only because it's the 'right thing to do' given there's an interface available. I don't think we would actually miss it (in our implementation) if it were gone.

For unit testing we simply switch our EF provider over from the PostgreSql to the InMemory Implementation, rather than making any replacements of the DbContext service itself.

[brockallen]

Ok, thanks for that feedback on your usage.

[brockallen]

Thanks Ben. I did a fix and we are publishing an updated preview now.

[BenWolstencroft]

@brockallen - just to confirm, should the change be in preview.2.12 ? looking at the code in the dll it appears to still be the concrete dependency, i'm still getting the service resolution exceptions thrown.

[leastprivilege]

Seems there was a timing issue...preview.2.13 on the way...

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue.