From bded36d88f76414040d5a223a2a3f351ac5468e0 Mon Sep 17 00:00:00 2001 From: Shakar Bakr <5h4k4r.b4kr@gmail.com> Date: Mon, 25 Aug 2025 15:03:59 +0300 Subject: [PATCH 1/2] Add db param to sentinel configurations Signed-off-by: Shakar Bakr <5h4k4r.b4kr@gmail.com> --- docs/docs/configuration/sessions.md | 3 +- pkg/apis/options/options.go | 1 + pkg/apis/options/sessions.go | 1 + pkg/sessions/redis/redis_store.go | 1 + pkg/sessions/redis/redis_store_test.go | 157 +++++++++++++++++++++++++ pkg/validation/sessions_test.go | 50 ++++++++ 6 files changed, 211 insertions(+), 2 deletions(-) diff --git a/docs/docs/configuration/sessions.md b/docs/docs/configuration/sessions.md index e2037817..a2aa76b0 100644 --- a/docs/docs/configuration/sessions.md +++ b/docs/docs/configuration/sessions.md @@ -86,8 +86,7 @@ When using the redis store, specify `--session-store-type=redis` as well as the `--redis-connection-url=redis://host[:port][/db-number]`. You may also configure the store for Redis Sentinel. In this case, you will want to use the -`--redis-use-sentinel=true` flag, as well as configure the flags `--redis-sentinel-master-name` -and `--redis-sentinel-connection-urls` appropriately. +`--redis-use-sentinel=true` flag, as well as configure the flags `--redis-sentinel-master-name`, `--redis-sentinel-db` and `--redis-sentinel-connection-urls` appropriately. Redis Cluster is available to be the backend store as well. To leverage it, you will need to set the `--redis-use-cluster=true` flag, and configure the flags `--redis-cluster-connection-urls` appropriately. diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 8fa72c7c..0d7e49a4 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -151,6 +151,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.String("redis-username", "", "Redis username. Applicable for Redis configurations where ACL has been configured. Will override any username set in `--redis-connection-url`") flagSet.String("redis-password", "", "Redis password. Applicable for all Redis configurations. Will override any password set in `--redis-connection-url`") flagSet.Bool("redis-use-sentinel", false, "Connect to redis via sentinels. Must set --redis-sentinel-master-name and --redis-sentinel-connection-urls to use this feature") + flagSet.Int("redis-sentinel-db", 0, "Redis sentinel database number. Used in conjunction with --redis-use-sentinel") flagSet.String("redis-sentinel-password", "", "Redis sentinel password. Used only for sentinel connection; any redis node passwords need to use `--redis-password`") flagSet.String("redis-sentinel-master-name", "", "Redis sentinel master name. Used in conjunction with --redis-use-sentinel") flagSet.String("redis-ca-path", "", "Redis custom CA path") diff --git a/pkg/apis/options/sessions.go b/pkg/apis/options/sessions.go index c90c0ac2..f8879e41 100644 --- a/pkg/apis/options/sessions.go +++ b/pkg/apis/options/sessions.go @@ -26,6 +26,7 @@ type RedisStoreOptions struct { Username string `flag:"redis-username" cfg:"redis_username"` Password string `flag:"redis-password" cfg:"redis_password"` UseSentinel bool `flag:"redis-use-sentinel" cfg:"redis_use_sentinel"` + SentinelDB int `flag:"redis-sentinel-db" cfg:"redis_sentinel_db"` SentinelPassword string `flag:"redis-sentinel-password" cfg:"redis_sentinel_password"` SentinelMasterName string `flag:"redis-sentinel-master-name" cfg:"redis_sentinel_master_name"` SentinelConnectionURLs []string `flag:"redis-sentinel-connection-urls" cfg:"redis_sentinel_connection_urls"` diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index 4e846e9b..76885ea2 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/pkg/sessions/redis/redis_store.go @@ -119,6 +119,7 @@ func buildSentinelClient(opts options.RedisStoreOptions) (Client, error) { SentinelAddrs: addrs, SentinelPassword: opts.SentinelPassword, Username: opts.Username, + DB: opts.SentinelDB, Password: opts.Password, TLSConfig: opt.TLSConfig, ConnMaxIdleTime: time.Duration(opts.IdleTimeout) * time.Second, diff --git a/pkg/sessions/redis/redis_store_test.go b/pkg/sessions/redis/redis_store_test.go index 1bff6855..6b90a7a9 100644 --- a/pkg/sessions/redis/redis_store_test.go +++ b/pkg/sessions/redis/redis_store_test.go @@ -93,6 +93,29 @@ var _ = Describe("Redis SessionStore Tests", func() { return nil }, ) + + Context("with custom sentinel DB", func() { + tests.RunSessionStoreTests( + func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) { + // Set the sentinel connection URL with custom DB + sentinelAddr := redisProtocol + ms.Addr() + opts.Type = options.RedisSessionStoreType + opts.Redis.SentinelConnectionURLs = []string{sentinelAddr} + opts.Redis.UseSentinel = true + opts.Redis.SentinelMasterName = ms.MasterInfo().Name + opts.Redis.SentinelDB = 1 + + // Capture the session store so that we can close the client + var err error + ss, err = NewRedisSessionStore(opts, cookieOpts) + return ss, err + }, + func(d time.Duration) error { + mr.FastForward(d) + return nil + }, + ) + }) }) Context("with cluster", func() { @@ -170,6 +193,30 @@ var _ = Describe("Redis SessionStore Tests", func() { return nil }, ) + + Context("with custom sentinel DB and password", func() { + tests.RunSessionStoreTests( + func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) { + // Set the sentinel connection URL with custom DB and password + sentinelAddr := redisProtocol + ms.Addr() + opts.Type = options.RedisSessionStoreType + opts.Redis.SentinelConnectionURLs = []string{sentinelAddr} + opts.Redis.UseSentinel = true + opts.Redis.SentinelMasterName = ms.MasterInfo().Name + opts.Redis.Password = redisPassword + opts.Redis.SentinelDB = 2 + + // Capture the session store so that we can close the client + var err error + ss, err = NewRedisSessionStore(opts, cookieOpts) + return ss, err + }, + func(d time.Duration) error { + mr.FastForward(d) + return nil + }, + ) + }) }) Context("with cluster", func() { @@ -271,4 +318,114 @@ var _ = Describe("Redis SessionStore Tests", func() { Expect(opts).To(BeNil()) }) }) + + Describe("Sentinel DB Configuration", func() { + var mr *miniredis.Miniredis + var ms *minisentinel.Sentinel + + BeforeEach(func() { + var err error + mr, err = miniredis.Run() + Expect(err).ToNot(HaveOccurred()) + ms = minisentinel.NewSentinel(mr) + Expect(ms.Start()).To(Succeed()) + }) + + AfterEach(func() { + mr.Close() + }) + + It("should use default DB 0 when SentinelDB is not set", func() { + opts := options.RedisStoreOptions{ + SentinelConnectionURLs: []string{"redis://" + ms.Addr()}, + UseSentinel: true, + SentinelMasterName: ms.MasterInfo().Name, + SentinelDB: 0, // Default value + } + + client, err := NewRedisClient(opts) + Expect(err).ToNot(HaveOccurred()) + Expect(client).ToNot(BeNil()) + + // Verify we can create a session store successfully + sessionStore := &SessionStore{Client: client} + Expect(sessionStore).ToNot(BeNil()) + + // Clean up + if closer, ok := client.(interface{ Close() error }); ok { + Expect(closer.Close()).To(Succeed()) + } + }) + + It("should use custom SentinelDB value when set", func() { + testDB := 5 + opts := options.RedisStoreOptions{ + SentinelConnectionURLs: []string{"redis://" + ms.Addr()}, + UseSentinel: true, + SentinelMasterName: ms.MasterInfo().Name, + SentinelDB: testDB, + } + + client, err := NewRedisClient(opts) + Expect(err).ToNot(HaveOccurred()) + Expect(client).ToNot(BeNil()) + + // Verify we can create a session store successfully + sessionStore := &SessionStore{Client: client} + Expect(sessionStore).ToNot(BeNil()) + + // Clean up + if closer, ok := client.(interface{ Close() error }); ok { + Expect(closer.Close()).To(Succeed()) + } + }) + + It("should work with maximum valid DB number", func() { + maxDB := 15 // Redis supports DB 0-15 by default + opts := options.RedisStoreOptions{ + SentinelConnectionURLs: []string{"redis://" + ms.Addr()}, + UseSentinel: true, + SentinelMasterName: ms.MasterInfo().Name, + SentinelDB: maxDB, + } + + client, err := NewRedisClient(opts) + Expect(err).ToNot(HaveOccurred()) + Expect(client).ToNot(BeNil()) + + // Verify we can create a session store successfully + sessionStore := &SessionStore{Client: client} + Expect(sessionStore).ToNot(BeNil()) + + // Clean up + if closer, ok := client.(interface{ Close() error }); ok { + Expect(closer.Close()).To(Succeed()) + } + }) + + It("should handle SentinelDB with authentication", func() { + testDB := 3 + testPassword := "test-password" + opts := options.RedisStoreOptions{ + SentinelConnectionURLs: []string{"redis://" + ms.Addr()}, + UseSentinel: true, + SentinelMasterName: ms.MasterInfo().Name, + SentinelDB: testDB, + Password: testPassword, + } + + client, err := NewRedisClient(opts) + Expect(err).ToNot(HaveOccurred()) + Expect(client).ToNot(BeNil()) + + // Verify we can create a session store successfully + sessionStore := &SessionStore{Client: client} + Expect(sessionStore).ToNot(BeNil()) + + // Clean up + if closer, ok := client.(interface{ Close() error }); ok { + Expect(closer.Close()).To(Succeed()) + } + }) + }) }) diff --git a/pkg/validation/sessions_test.go b/pkg/validation/sessions_test.go index 6f590ac5..b5696958 100644 --- a/pkg/validation/sessions_test.go +++ b/pkg/validation/sessions_test.go @@ -346,6 +346,56 @@ var _ = Describe("Sessions", func() { }, errStrings: []string{}, }), + Entry("connect successfully to sentinel redis with custom DB", &redisStoreTableInput{ + useSentinel: true, + setSentinelAddr: true, + setMasterName: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + UseSentinel: true, + SentinelDB: 1, + }, + }, + }, + errStrings: []string{}, + }), + Entry("connect successfully to sentinel redis with custom DB and password", &redisStoreTableInput{ + password: "abcdef123", + useSentinel: true, + setSentinelAddr: true, + setMasterName: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + Password: "abcdef123", + UseSentinel: true, + SentinelDB: 5, + }, + }, + }, + errStrings: []string{}, + }), + Entry("connect successfully to sentinel redis with maximum DB number", &redisStoreTableInput{ + useSentinel: true, + setSentinelAddr: true, + setMasterName: true, + + opts: &options.Options{ + Session: options.SessionOptions{ + Type: options.RedisSessionStoreType, + Redis: options.RedisStoreOptions{ + UseSentinel: true, + SentinelDB: 15, + }, + }, + }, + errStrings: []string{}, + }), Entry("failed connection to sentinel redis with wrong password", &redisStoreTableInput{ password: "abcdef123", useSentinel: true, From 5c598577f4ef540403e56c0d246f3e13da73a183 Mon Sep 17 00:00:00 2001 From: Shakar Bakr <5h4k4r.b4kr@gmail.com> Date: Mon, 25 Aug 2025 15:06:49 +0300 Subject: [PATCH 2/2] chore: update CHANGELOG.md to include comprehensive tests for Redis sentinel DB configuration Signed-off-by: Shakar Bakr <5h4k4r.b4kr@gmail.com> --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2c9b441..31e14009 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.12.0 +- [#3175](https://github.com/oauth2-proxy/oauth2-proxy/pull/3175) test: add comprehensive tests for Redis sentinel DB configuration (@5h4k4r) + # V7.12.0 ## Release Highlights @@ -119,7 +121,7 @@ For detailed information, migration guidance, and security implications, see the - 🕵️‍♀️ Vulnerabilities have been addressed - [CVE-2025-22871](https://github.com/advisories/GHSA-g9pc-8g42-g6vq) - 🐛 Squashed some bugs - + ## Important Notes ## Breaking Changes