remove the redis grant store (#3757)

perf/MicroBenchmarks/Identity/IdentityServer/PersistedGrantStoreTests.cs

@@ -3,19 +3,15 @@ using Bit.Identity.IdentityServer;
 using Bit.Infrastructure.Dapper.Auth.Repositories;
 using Duende.IdentityServer.Models;
 using Duende.IdentityServer.Stores;
-using Microsoft.Extensions.Logging.Abstractions;
-using StackExchange.Redis;
 
 namespace Bit.MicroBenchmarks.Identity.IdentityServer;
 
 [MemoryDiagnoser]
-public class RedisPersistedGrantStoreTests
+public class PersistedGrantStoreTests
 {
     const string SQL = nameof(SQL);
-    const string Redis = nameof(Redis);
     const string Cosmos = nameof(Cosmos);
 
-    private readonly IPersistedGrantStore _redisGrantStore;
     private readonly IPersistedGrantStore _sqlGrantStore;
     private readonly IPersistedGrantStore _cosmosGrantStore;
     private readonly PersistedGrant _updateGrant;
@@ -39,14 +35,8 @@ public class RedisPersistedGrantStoreTests
     // 15) "ClientId"
     // 16) "web"
 
-    public RedisPersistedGrantStoreTests()
+    public PersistedGrantStoreTests()
     {
-        _redisGrantStore = new RedisPersistedGrantStore(
-            ConnectionMultiplexer.Connect("localhost"),
-            NullLogger<RedisPersistedGrantStore>.Instance,
-            new InMemoryPersistedGrantStore()
-        );
-
         var sqlConnectionString = "YOUR CONNECTION STRING HERE";
         _sqlGrantStore = new PersistedGrantStore(
             new GrantRepository(
@@ -78,17 +68,13 @@ public class RedisPersistedGrantStoreTests
         };
     }
 
-    [Params(Redis, SQL, Cosmos)]
+    [Params(SQL, Cosmos)]
     public string StoreType { get; set; } = null!;
 
     [GlobalSetup]
     public void Setup()
     {
-        if (StoreType == Redis)
+        if (StoreType == SQL)
-        {
-            _grantStore = _redisGrantStore;
-        }
-        else if (StoreType == SQL)
         {
             _grantStore = _sqlGrantStore;
         }

src/Identity/IdentityServer/PersistedGrantStore.cs

@@ -9,16 +9,13 @@ public class PersistedGrantStore : IPersistedGrantStore
 {
     private readonly IGrantRepository _grantRepository;
     private readonly Func<PersistedGrant, IGrant> _toGrant;
-    private readonly IPersistedGrantStore _fallbackGrantStore;
 
     public PersistedGrantStore(
         IGrantRepository grantRepository,
-        Func<PersistedGrant, IGrant> toGrant,
+        Func<PersistedGrant, IGrant> toGrant)
-        IPersistedGrantStore fallbackGrantStore = null)
     {
         _grantRepository = grantRepository;
         _toGrant = toGrant;
-        _fallbackGrantStore = fallbackGrantStore;
     }
 
     public async Task<PersistedGrant> GetAsync(string key)
@@ -26,11 +23,6 @@ public class PersistedGrantStore : IPersistedGrantStore
         var grant = await _grantRepository.GetByKeyAsync(key);
         if (grant == null)
         {
-            if (_fallbackGrantStore != null)
-            {
-                // It wasn't found, there is a chance is was instead stored in the fallback store
-                return await _fallbackGrantStore.GetAsync(key);
-            }
             return null;
         }
 

src/Identity/IdentityServer/RedisPersistedGrantStore.cs

@@ -1,181 +0,0 @@
-using System.Diagnostics;
-using Duende.IdentityServer.Models;
-using Duende.IdentityServer.Stores;
-using MessagePack;
-using StackExchange.Redis;
-
-namespace Bit.Identity.IdentityServer;
-
-/// <summary>
-/// A <see cref="IPersistedGrantStore"/> that persists its grants on a Redis DB
-/// </summary>
-/// <remarks>
-/// This store also allows a fallback to another store in the case that a key was not found
-/// in the Redis DB or the Redis DB happens to be down.
-/// </remarks>
-public class RedisPersistedGrantStore : IPersistedGrantStore
-{
-    private static readonly MessagePackSerializerOptions _options = MessagePackSerializerOptions.Standard;
-    private readonly IConnectionMultiplexer _connectionMultiplexer;
-    private readonly ILogger<RedisPersistedGrantStore> _logger;
-    private readonly IPersistedGrantStore _fallbackGrantStore;
-
-    public RedisPersistedGrantStore(
-        IConnectionMultiplexer connectionMultiplexer,
-        ILogger<RedisPersistedGrantStore> logger,
-        IPersistedGrantStore fallbackGrantStore)
-    {
-        _connectionMultiplexer = connectionMultiplexer;
-        _logger = logger;
-        _fallbackGrantStore = fallbackGrantStore;
-    }
-
-    public Task<IEnumerable<PersistedGrant>> GetAllAsync(PersistedGrantFilter filter)
-    {
-        _logger.LogWarning("Redis does not implement 'GetAllAsync', Skipping.");
-        return Task.FromResult(Enumerable.Empty<PersistedGrant>());
-    }
-    public async Task<PersistedGrant> GetAsync(string key)
-    {
-        try
-        {
-            if (!_connectionMultiplexer.IsConnected)
-            {
-                // Redis is down, fallback to using SQL table
-                _logger.LogWarning("This is not connected, using fallback store to execute 'GetAsync' with {Key}.", key);
-                return await _fallbackGrantStore.GetAsync(key);
-            }
-
-            var redisKey = CreateRedisKey(key);
-
-            var redisDb = _connectionMultiplexer.GetDatabase();
-            var redisValueAndExpiry = await redisDb.StringGetWithExpiryAsync(redisKey);
-
-            if (!redisValueAndExpiry.Value.HasValue)
-            {
-                // It wasn't found, there is a chance is was instead stored in the fallback store
-                _logger.LogWarning("Could not find grant in primary store, using fallback one.");
-                return await _fallbackGrantStore.GetAsync(key);
-            }
-
-            Debug.Assert(redisValueAndExpiry.Expiry.HasValue, "Redis entry is expected to have an expiry.");
-
-            var storablePersistedGrant = MessagePackSerializer.Deserialize<StorablePersistedGrant>(redisValueAndExpiry.Value, _options);
-
-            return new PersistedGrant
-            {
-                Key = key,
-                Type = storablePersistedGrant.Type,
-                SubjectId = storablePersistedGrant.SubjectId,
-                SessionId = storablePersistedGrant.SessionId,
-                ClientId = storablePersistedGrant.ClientId,
-                Description = storablePersistedGrant.Description,
-                CreationTime = storablePersistedGrant.CreationTime,
-                ConsumedTime = storablePersistedGrant.ConsumedTime,
-                Data = storablePersistedGrant.Data,
-                Expiration = storablePersistedGrant.CreationTime.Add(redisValueAndExpiry.Expiry.Value),
-            };
-        }
-        catch (Exception ex)
-        {
-            _logger.LogWarning(ex, "Failure in 'GetAsync' using primary grant store, falling back.");
-            return await _fallbackGrantStore.GetAsync(key);
-        }
-    }
-
-    public Task RemoveAllAsync(PersistedGrantFilter filter)
-    {
-        _logger.LogWarning("This does not implement 'RemoveAllAsync', Skipping.");
-        return Task.CompletedTask;
-    }
-
-    // This method is not actually expected to get called and instead redis will just get rid of the expired items
-    public async Task RemoveAsync(string key)
-    {
-        if (!_connectionMultiplexer.IsConnecting)
-        {
-            _logger.LogWarning("Redis is not connected, using fallback store to execute 'RemoveAsync', with {Key}", key);
-            await _fallbackGrantStore.RemoveAsync(key);
-        }
-
-        var redisDb = _connectionMultiplexer.GetDatabase();
-        await redisDb.KeyDeleteAsync(CreateRedisKey(key));
-    }
-
-    public async Task StoreAsync(PersistedGrant grant)
-    {
-        try
-        {
-            if (!_connectionMultiplexer.IsConnected)
-            {
-                _logger.LogWarning("Redis is not connected, using fallback store to execute 'StoreAsync', with {Key}", grant.Key);
-                await _fallbackGrantStore.StoreAsync(grant);
-            }
-
-            if (!grant.Expiration.HasValue)
-            {
-                throw new ArgumentException("A PersistedGrant is always expected to include an expiration time.");
-            }
-
-            var redisDb = _connectionMultiplexer.GetDatabase();
-
-            var redisKey = CreateRedisKey(grant.Key);
-
-            var serializedGrant = MessagePackSerializer.Serialize(new StorablePersistedGrant
-            {
-                Type = grant.Type,
-                SubjectId = grant.SubjectId,
-                SessionId = grant.SessionId,
-                ClientId = grant.ClientId,
-                Description = grant.Description,
-                CreationTime = grant.CreationTime,
-                ConsumedTime = grant.ConsumedTime,
-                Data = grant.Data,
-            }, _options);
-
-            await redisDb.StringSetAsync(redisKey, serializedGrant, grant.Expiration.Value - grant.CreationTime);
-        }
-        catch (Exception ex)
-        {
-            _logger.LogWarning(ex, "Failure in 'StoreAsync' using primary grant store, falling back.");
-            await _fallbackGrantStore.StoreAsync(grant);
-        }
-    }
-
-    private static RedisKey CreateRedisKey(string key)
-    {
-        return $"grant-{key}";
-    }
-
-    // This is a slimmer version of PersistedGrant that removes the Key since that will be used as the key in Redis
-    // it also strips out the ExpirationDate since we use that to store the TTL and read that out when we retrieve the
-    // object and can use that information to fill in the Expiration property on PersistedGrant
-    // TODO: .NET 8 Make all properties required
-    [MessagePackObject]
-    public class StorablePersistedGrant
-    {
-        [Key(0)]
-        public string Type { get; set; }
-
-        [Key(1)]
-        public string SubjectId { get; set; }
-
-        [Key(2)]
-        public string SessionId { get; set; }
-
-        [Key(3)]
-        public string ClientId { get; set; }
-
-        [Key(4)]
-        public string Description { get; set; }
-
-        [Key(5)]
-        public DateTime CreationTime { get; set; }
-
-        [Key(6)]
-        public DateTime? ConsumedTime { get; set; }
-
-        [Key(7)]
-        public string Data { get; set; }
-    }
-}

src/Identity/Utilities/ServiceCollectionExtensions.cs

@@ -7,7 +7,6 @@ using Bit.SharedWeb.Utilities;
 using Duende.IdentityServer.ResponseHandling;
 using Duende.IdentityServer.Services;
 using Duende.IdentityServer.Stores;
-using StackExchange.Redis;
 
 namespace Bit.Identity.Utilities;
 
@@ -54,56 +53,18 @@ public static class ServiceCollectionExtensions
 
         if (CoreHelpers.SettingHasValue(globalSettings.IdentityServer.CosmosConnectionString))
         {
-            services.AddSingleton<IPersistedGrantStore>(sp => BuildCosmosGrantStore(sp, globalSettings));
+            services.AddSingleton<IPersistedGrantStore>(sp =>
-        }
+                new PersistedGrantStore(sp.GetRequiredKeyedService<IGrantRepository>("cosmos"),
-        else if (CoreHelpers.SettingHasValue(globalSettings.IdentityServer.RedisConnectionString))
+                    g => new Core.Auth.Models.Data.GrantItem(g)));
-        {
-            services.AddSingleton<IPersistedGrantStore>(sp => BuildRedisGrantStore(sp, globalSettings));
         }
         else
         {
-            services.AddTransient<IPersistedGrantStore>(sp => BuildSqlGrantStore(sp));
+            services.AddTransient<IPersistedGrantStore>(sp =>
+                new PersistedGrantStore(sp.GetRequiredService<IGrantRepository>(),
+                    g => new Core.Auth.Entities.Grant(g)));
         }
 
         services.AddTransient<ICorsPolicyService, CustomCorsPolicyService>();
         return identityServerBuilder;
     }
-
-    private static PersistedGrantStore BuildCosmosGrantStore(IServiceProvider sp, GlobalSettings globalSettings)
-    {
-        if (!CoreHelpers.SettingHasValue(globalSettings.IdentityServer.CosmosConnectionString))
-        {
-            throw new ArgumentException("No cosmos config string available.");
-        }
-        return new PersistedGrantStore(
-            // TODO: Perhaps we want to evaluate moving this repo to DI as a keyed service singleton in .NET 8
-            new Core.Auth.Repositories.Cosmos.GrantRepository(globalSettings),
-            g => new Core.Auth.Models.Data.GrantItem(g),
-            fallbackGrantStore: BuildRedisGrantStore(sp, globalSettings, true));
-    }
-
-    private static RedisPersistedGrantStore BuildRedisGrantStore(IServiceProvider sp,
-        GlobalSettings globalSettings, bool allowNull = false)
-    {
-        if (!CoreHelpers.SettingHasValue(globalSettings.IdentityServer.RedisConnectionString))
-        {
-            if (allowNull)
-            {
-                return null;
-            }
-            throw new ArgumentException("No redis config string available.");
-        }
-
-        return new RedisPersistedGrantStore(
-            // TODO: .NET 8 create a keyed service for this connection multiplexer and even PersistedGrantStore
-            ConnectionMultiplexer.Connect(globalSettings.IdentityServer.RedisConnectionString),
-            sp.GetRequiredService<ILogger<RedisPersistedGrantStore>>(),
-            fallbackGrantStore: BuildSqlGrantStore(sp));
-    }
-
-    private static PersistedGrantStore BuildSqlGrantStore(IServiceProvider sp)
-    {
-        return new PersistedGrantStore(sp.GetRequiredService<IGrantRepository>(),
-            g => new Core.Auth.Entities.Grant(g));
-    }
 }

src/SharedWeb/Utilities/ServiceCollectionExtensions.cs

@@ -11,6 +11,7 @@ using Bit.Core.Auth.Identity;
 using Bit.Core.Auth.IdentityServer;
 using Bit.Core.Auth.LoginFeatures;
 using Bit.Core.Auth.Models.Business.Tokenables;
+using Bit.Core.Auth.Repositories;
 using Bit.Core.Auth.Services;
 using Bit.Core.Auth.Services.Implementations;
 using Bit.Core.Auth.UserFeatures;
@@ -120,6 +121,7 @@ public static class ServiceCollectionExtensions
         {
             services.AddSingleton<IEventRepository, TableStorageRepos.EventRepository>();
             services.AddSingleton<IInstallationDeviceRepository, TableStorageRepos.InstallationDeviceRepository>();
+            services.AddKeyedSingleton<IGrantRepository, Core.Auth.Repositories.Cosmos.GrantRepository>("cosmos");
         }
 
         return provider;