git.iko.soy/typeable/oauth2-proxy
on master help sign in
fix: return error for empty Redis URL list (#3101)

* fix: return error for empty Redis URL list * add changelog entry Signed-off-by: Jan Larwig <jan@larwig.com> --------- Signed-off-by: Jan Larwig <jan@larwig.com> Co-authored-by: Jan Larwig <jan@larwig.com>

commit0e1dc9bb84a63c67d06d19e03779f74e213c0cf0 authorDaniel Givens <1581675+dgivens@users.noreply.github.com> dateThu 17 Jul 2025, 02:23 (1 year ago) parent6c30a3c
3 files changed+159−125
CHANGELOG.md+1 pkg/sessions/redis/redis_store.go+4 pkg/sessions/redis/redis_store_test.go+154−125
CHANGELOG.md+1
@@ -11,6 +11,7 @@
1111- [#3072](https://github.com/oauth2-proxy/oauth2-proxy/pull/3072) feat: support for multiple github orgs #3072 (@daniel-mersch)
1212- [#3116](https://github.com/oauth2-proxy/oauth2-proxy/pull/3116) feat: bump to go1.24.5 and full dependency update (@wardviaene / @dolmen)
1313- [#3097](https://github.com/oauth2-proxy/oauth2-proxy/pull/3097) chore(deps): update alpine base image to v3.22.0
14+- [#3101](https://github.com/oauth2-proxy/oauth2-proxy/pull/3101) fix: return error for empty Redis URL list (@dgivens)
1415
1516# V7.9.0
1617
pkg/sessions/redis/redis_store.go+4
@@ -218,6 +218,10 @@ func setupTLSConfig(opts options.RedisStoreOptions, opt *redis.Options) error {
218218// parseRedisURLs parses a list of redis urls and returns a list
219219// of addresses in the form of host:port and redis.Options that can be used to connect to Redis
220220func parseRedisURLs(urls []string) ([]string, *redis.Options, error) {
221+ if len(urls) == 0 {
222+ return nil, nil, fmt.Errorf("unable to parse redis urls: no redis urls provided")
223+ }
224+
221225 addrs := []string{}
222226 var redisOptions *redis.Options
223227 for _, u := range urls {
pkg/sessions/redis/redis_store_test.go+154−125
@@ -19,116 +19,41 @@ const (
1919)
2020
2121var _ = Describe("Redis SessionStore Tests", func() {
22− // helper interface to allow us to close client connections
23− // All non-nil redis clients should implement this
24− type closer interface {
25− Close() error
26− }
27−
28− var mr *miniredis.Miniredis
29− var ss sessionsapi.SessionStore
30−
31− BeforeEach(func() {
32− var err error
33− mr, err = miniredis.Run()
34− Expect(err).ToNot(HaveOccurred())
35− })
36−
37− AfterEach(func() {
38− mr.Close()
39− })
40−
41− JustAfterEach(func() {
42− // Release any connections immediately after the test ends
43− if redisManager, ok := ss.(*persistence.Manager); ok {
44− if redisManager.Store.(*SessionStore).Client != nil {
45− Expect(redisManager.Store.(*SessionStore).Client.(closer).Close()).To(Succeed())
46− }
22+ Describe("Redis SessionStore Creation", func() {
23+ // helper interface to allow us to close client connections
24+ // All non-nil redis clients should implement this
25+ type closer interface {
26+ Close() error
4727 }
48− })
4928
50− const redisProtocol = "redis://"
51− tests.RunSessionStoreTests(
52− func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
53− // Set the connection URL
54− opts.Type = options.RedisSessionStoreType
55− opts.Redis.ConnectionURL = redisProtocol + mr.Addr()
56−
57− // Capture the session store so that we can close the client
58− var err error
59− ss, err = NewRedisSessionStore(opts, cookieOpts)
60− return ss, err
61− },
62− func(d time.Duration) error {
63− mr.FastForward(d)
64− return nil
65− },
66− )
67−
68− Context("with sentinel", func() {
69− var ms *minisentinel.Sentinel
29+ var mr *miniredis.Miniredis
30+ var ss sessionsapi.SessionStore
7031
7132 BeforeEach(func() {
72− ms = minisentinel.NewSentinel(mr)
73− Expect(ms.Start()).To(Succeed())
33+ var err error
34+ mr, err = miniredis.Run()
35+ Expect(err).ToNot(HaveOccurred())
7436 })
7537
76− tests.RunSessionStoreTests(
77− func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
78− // Set the sentinel connection URL
79− sentinelAddr := redisProtocol + ms.Addr()
80− opts.Type = options.RedisSessionStoreType
81− opts.Redis.SentinelConnectionURLs = []string{sentinelAddr}
82− opts.Redis.UseSentinel = true
83− opts.Redis.SentinelMasterName = ms.MasterInfo().Name
84−
85− // Capture the session store so that we can close the client
86− var err error
87− ss, err = NewRedisSessionStore(opts, cookieOpts)
88− return ss, err
89− },
90− func(d time.Duration) error {
91− mr.FastForward(d)
92− return nil
93− },
94− )
95− })
96−
97− Context("with cluster", func() {
98− tests.RunSessionStoreTests(
99− func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
100− clusterAddr := redisProtocol + mr.Addr()
101− opts.Type = options.RedisSessionStoreType
102− opts.Redis.ClusterConnectionURLs = []string{clusterAddr}
103− opts.Redis.UseCluster = true
104−
105− // Capture the session store so that we can close the client
106− var err error
107− ss, err = NewRedisSessionStore(opts, cookieOpts)
108− return ss, err
109− },
110− func(d time.Duration) error {
111− mr.FastForward(d)
112− return nil
113− },
114− )
115− })
116−
117− Context("with a redis password", func() {
118− BeforeEach(func() {
119− mr.RequireAuth(redisPassword)
38+ AfterEach(func() {
39+ mr.Close()
12040 })
12141
122− AfterEach(func() {
123− mr.RequireAuth("")
42+ JustAfterEach(func() {
43+ // Release any connections immediately after the test ends
44+ if redisManager, ok := ss.(*persistence.Manager); ok {
45+ if redisManager.Store.(*SessionStore).Client != nil {
46+ Expect(redisManager.Store.(*SessionStore).Client.(closer).Close()).To(Succeed())
47+ }
48+ }
12449 })
12550
51+ const redisProtocol = "redis://"
12652 tests.RunSessionStoreTests(
12753 func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
12854 // Set the connection URL
12955 opts.Type = options.RedisSessionStoreType
13056 opts.Redis.ConnectionURL = redisProtocol + mr.Addr()
131− opts.Redis.Password = redisPassword
13257
13358 // Capture the session store so that we can close the client
13459 var err error
@@ -157,7 +82,6 @@ var _ = Describe("Redis SessionStore Tests", func() {
15782 opts.Redis.SentinelConnectionURLs = []string{sentinelAddr}
15883 opts.Redis.UseSentinel = true
15984 opts.Redis.SentinelMasterName = ms.MasterInfo().Name
160− opts.Redis.Password = redisPassword
16185
16286 // Capture the session store so that we can close the client
16387 var err error
@@ -178,7 +102,6 @@ var _ = Describe("Redis SessionStore Tests", func() {
178102 opts.Type = options.RedisSessionStoreType
179103 opts.Redis.ClusterConnectionURLs = []string{clusterAddr}
180104 opts.Redis.UseCluster = true
181− opts.Redis.Password = redisPassword
182105
183106 // Capture the session store so that we can close the client
184107 var err error
@@ -191,43 +114,100 @@ var _ = Describe("Redis SessionStore Tests", func() {
191114 },
192115 )
193116 })
194− })
195117
196− Context("with a redis username and password", func() {
197− BeforeEach(func() {
198− mr.RequireUserAuth(redisUsername, redisPassword)
199− })
118+ Context("with a redis password", func() {
119+ BeforeEach(func() {
120+ mr.RequireAuth(redisPassword)
121+ })
200122
201− AfterEach(func() {
202− mr.RequireUserAuth("", "")
123+ AfterEach(func() {
124+ mr.RequireAuth("")
125+ })
126+
127+ tests.RunSessionStoreTests(
128+ func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
129+ // Set the connection URL
130+ opts.Type = options.RedisSessionStoreType
131+ opts.Redis.ConnectionURL = redisProtocol + mr.Addr()
132+ opts.Redis.Password = redisPassword
133+
134+ // Capture the session store so that we can close the client
135+ var err error
136+ ss, err = NewRedisSessionStore(opts, cookieOpts)
137+ return ss, err
138+ },
139+ func(d time.Duration) error {
140+ mr.FastForward(d)
141+ return nil
142+ },
143+ )
144+
145+ Context("with sentinel", func() {
146+ var ms *minisentinel.Sentinel
147+
148+ BeforeEach(func() {
149+ ms = minisentinel.NewSentinel(mr)
150+ Expect(ms.Start()).To(Succeed())
151+ })
152+
153+ tests.RunSessionStoreTests(
154+ func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
155+ // Set the sentinel connection URL
156+ sentinelAddr := redisProtocol + ms.Addr()
157+ opts.Type = options.RedisSessionStoreType
158+ opts.Redis.SentinelConnectionURLs = []string{sentinelAddr}
159+ opts.Redis.UseSentinel = true
160+ opts.Redis.SentinelMasterName = ms.MasterInfo().Name
161+ opts.Redis.Password = redisPassword
162+
163+ // Capture the session store so that we can close the client
164+ var err error
165+ ss, err = NewRedisSessionStore(opts, cookieOpts)
166+ return ss, err
167+ },
168+ func(d time.Duration) error {
169+ mr.FastForward(d)
170+ return nil
171+ },
172+ )
173+ })
174+
175+ Context("with cluster", func() {
176+ tests.RunSessionStoreTests(
177+ func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
178+ clusterAddr := redisProtocol + mr.Addr()
179+ opts.Type = options.RedisSessionStoreType
180+ opts.Redis.ClusterConnectionURLs = []string{clusterAddr}
181+ opts.Redis.UseCluster = true
182+ opts.Redis.Password = redisPassword
183+
184+ // Capture the session store so that we can close the client
185+ var err error
186+ ss, err = NewRedisSessionStore(opts, cookieOpts)
187+ return ss, err
188+ },
189+ func(d time.Duration) error {
190+ mr.FastForward(d)
191+ return nil
192+ },
193+ )
194+ })
203195 })
204196
205− tests.RunSessionStoreTests(
206− func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
207− // Set the connection URL
208− opts.Type = options.RedisSessionStoreType
209− opts.Redis.ConnectionURL = "redis://" + redisUsername + "@" + mr.Addr()
210− opts.Redis.Password = redisPassword
197+ Context("with a redis username and password", func() {
198+ BeforeEach(func() {
199+ mr.RequireUserAuth(redisUsername, redisPassword)
200+ })
211201
212− // Capture the session store so that we can close the client
213− var err error
214− ss, err = NewRedisSessionStore(opts, cookieOpts)
215− return ss, err
216− },
217− func(d time.Duration) error {
218− mr.FastForward(d)
219− return nil
220− },
221− )
202+ AfterEach(func() {
203+ mr.RequireUserAuth("", "")
204+ })
222205
223− Context("with cluster", func() {
224206 tests.RunSessionStoreTests(
225207 func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
226− clusterAddr := "redis://" + redisUsername + "@" + mr.Addr()
208+ // Set the connection URL
227209 opts.Type = options.RedisSessionStoreType
228− opts.Redis.ClusterConnectionURLs = []string{clusterAddr}
229− opts.Redis.UseCluster = true
230− opts.Redis.Username = redisUsername
210+ opts.Redis.ConnectionURL = "redis://" + redisUsername + "@" + mr.Addr()
231211 opts.Redis.Password = redisPassword
232212
233213 // Capture the session store so that we can close the client
@@ -240,6 +220,55 @@ var _ = Describe("Redis SessionStore Tests", func() {
240220 return nil
241221 },
242222 )
223+
224+ Context("with cluster", func() {
225+ tests.RunSessionStoreTests(
226+ func(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessionsapi.SessionStore, error) {
227+ clusterAddr := "redis://" + redisUsername + "@" + mr.Addr()
228+ opts.Type = options.RedisSessionStoreType
229+ opts.Redis.ClusterConnectionURLs = []string{clusterAddr}
230+ opts.Redis.UseCluster = true
231+ opts.Redis.Username = redisUsername
232+ opts.Redis.Password = redisPassword
233+
234+ // Capture the session store so that we can close the client
235+ var err error
236+ ss, err = NewRedisSessionStore(opts, cookieOpts)
237+ return ss, err
238+ },
239+ func(d time.Duration) error {
240+ mr.FastForward(d)
241+ return nil
242+ },
243+ )
244+ })
245+ })
246+ })
247+
248+ Describe("Redis URL Parsing", func() {
249+ It("should parse valid redis URL", func() {
250+ addrs, opts, err := parseRedisURLs([]string{"redis://localhost:6379"})
251+ Expect(err).ToNot(HaveOccurred())
252+ Expect(addrs).To(Equal([]string{"localhost:6379"}))
253+ Expect(opts).ToNot(BeNil())
254+ Expect(opts.Addr).To(Equal("localhost:6379"))
255+ })
256+
257+ It("should return error for invalid redis URL", func() {
258+ addrs, opts, err := parseRedisURLs([]string{"invalid://url"})
259+ Expect(err).To(HaveOccurred())
260+ Expect(err.Error()).To(ContainSubstring("unable to parse redis url"))
261+ Expect(err.Error()).To(Not(ContainSubstring("no redis urls provided")))
262+ Expect(addrs).To(BeNil())
263+ Expect(opts).To(BeNil())
264+ })
265+
266+ It("should return error when no URLs provided", func() {
267+ addrs, opts, err := parseRedisURLs([]string{})
268+ Expect(err).To(HaveOccurred())
269+ Expect(err.Error()).To(Equal("unable to parse redis urls: no redis urls provided"))
270+ Expect(addrs).To(BeNil())
271+ Expect(opts).To(BeNil())
243272 })
244273 })
245274})