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

Revocation in IdentityServer not revoking all instances of a subject #4

Closed ghstahl closed 4 years ago

ghstahl commented 6 years ago

IdentityServer4 Issue

I have a case, through my use of Extension Grants, where I can mint multiple tokens with the same "subject" using different clientIds;

ClientId Subject
client_one subject_one
client_one subject_one
client_two subject_one
client_two subject_one

When a revocation happens, the expected behavior is that all refresh_tokens that reference the subject must be revoked. At the moment IdentityServer4's IStores only account for a single revocation, but we need full revocation across all clients.

I have an implementation that delegates everything to your dentityServer4.Contrib.RedisStore.Stores.PersistedGrantStore, except for;

        public async Task RemoveAllAsync(string subjectId, string _, string type)
        {
            var clientIds = await _clientStoreExtra.GetAllClientIdsAsync();
            foreach (var clientId in clientIds)
            {
                _persistedGrantStoreImplementation.RemoveAllAsync(subjectId, clientId, type);
            }
        }

You may want to account for this in future releases. Here is my PersistedGrantStoreExtra implementation.

AliBazzi commented 6 years ago

I will see how it goes for Identity server 4 contract, if it's going to introduce new method there to cater this case. Later I will add implementation for it maybe with additional Redis Set.

Thanks for your input !

AliBazzi commented 4 years ago

This is implemented already with Version 4 of the library, I will close it