Validate Redis session store health on startup

2020-08-06 15:43:01 -07:00 · 2020-08-06 15:43:01 -07:00 · 6db1aeb9c6
parent 93870ec0ff
commit 6db1aeb9c6
5 changed files with 308 additions and 31 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -11,9 +11,12 @@

 ## Breaking Changes

+- [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) When a Redis session store is configured, OAuth2-Proxy will fail to start up unless connection and health checks to Redis pass
+
 ## Changes since v6.1.1

 - [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Stop accepting legacy SHA1 signed cookies (@NickMeves)
+- [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) Validate Redis configuration options at startup (@NickMeves)
 - [#764](https://github.com/oauth2-proxy/oauth2-proxy/pull/764) Document bcrypt encryption for htpasswd (and hide SHA) (@lentzi90)
 - [#616](https://github.com/oauth2-proxy/oauth2-proxy/pull/616) Add support to ensure user belongs in required groups when using the OIDC provider (@stefansedich)

--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@ -23,7 +23,7 @@ type SessionStore struct {
 // NewRedisSessionStore initialises a new instance of the SessionStore and wraps
 // it in a persistence.Manager
 func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessions.SessionStore, error) {
-	client, err := newRedisClient(opts.Redis)
+	client, err := NewRedisClient(opts.Redis)
 	if err != nil {
 		return nil, fmt.Errorf("error constructing redis client: %v", err)
 	}
@ -64,9 +64,9 @@ func (store *SessionStore) Clear(ctx context.Context, key string) error {
 	return nil
 }

-// newRedisClient makes a redis.Client (either standalone, sentinel aware, or
+// NewRedisClient makes a redis.Client (either standalone, sentinel aware, or
 // redis cluster)
-func newRedisClient(opts options.RedisStoreOptions) (Client, error) {
+func NewRedisClient(opts options.RedisStoreOptions) (Client, error) {
 	if opts.UseSentinel && opts.UseCluster {
 		return nil, fmt.Errorf("options redis-use-sentinel and redis-use-cluster are mutually exclusive")
 	}
--- a/pkg/validation/options.go
+++ b/pkg/validation/options.go
@ -28,6 +28,7 @@ import (
 func Validate(o *options.Options) error {
 	msgs := validateCookie(o.Cookie)
 	msgs = append(msgs, validateSessionCookieMinimal(o)...)
+	msgs = append(msgs, validateRedisSessionStore(o)...)

 	if o.SSLInsecureSkipVerify {
 		// InsecureSkipVerify is a configurable option we allow
--- a/pkg/validation/sessions.go
+++ b/pkg/validation/sessions.go
@ -1,9 +1,13 @@
 package validation

 import (
+	"context"
+	"fmt"
 	"time"

 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
+	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
+	"github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/redis"
 )

 func validateSessionCookieMinimal(o *options.Options) []string {
@ -30,3 +34,50 @@ func validateSessionCookieMinimal(o *options.Options) []string {
 	}
 	return msgs
 }
+
+// validateRedisSessionStore builds a Redis Client from the options and
+// attempts to connect, Set, Get and Del a random health check key
+func validateRedisSessionStore(o *options.Options) []string {
+	if o.Session.Type != options.RedisSessionStoreType {
+		return []string{}
+	}
+
+	client, err := redis.NewRedisClient(o.Session.Redis)
+	if err != nil {
+		return []string{fmt.Sprintf("unable to initialize a redis client: %v", err)}
+	}
+
+	nonce, err := encryption.Nonce()
+	if err != nil {
+		return []string{fmt.Sprintf("unable to generate a redis initialization test key: %v", err)}
+	}
+
+	key := fmt.Sprintf("%s-healthcheck-%s", o.Cookie.Name, nonce)
+	return sendRedisConnectionTest(client, key, nonce)
+}
+
+func sendRedisConnectionTest(client redis.Client, key string, val string) []string {
+	msgs := []string{}
+	ctx := context.Background()
+
+	err := client.Set(ctx, key, []byte(val), time.Duration(60)*time.Second)
+	if err != nil {
+		msgs = append(msgs, fmt.Sprintf("unable to set a redis initialization key: %v", err))
+	} else {
+		gval, err := client.Get(ctx, key)
+		if err != nil {
+			msgs = append(msgs,
+				fmt.Sprintf("unable to retrieve redis initialization key: %v", err))
+		}
+		if string(gval) != val {
+			msgs = append(msgs,
+				"the retrieved redis initialization key did not match the value we set")
+		}
+	}
+
+	err = client.Del(ctx, key)
+	if err != nil {
+		msgs = append(msgs, fmt.Sprintf("unable to delete the redis initialization key: %v", err))
+	}
+	return msgs
+}
--- a/pkg/validation/sessions_test.go
+++ b/pkg/validation/sessions_test.go
@ -1,14 +1,17 @@
 package validation

 import (
-	"testing"
 	"time"

+	"github.com/Bose/minisentinel"
+	"github.com/alicebob/miniredis/v2"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
 	. "github.com/onsi/gomega"
 )

-func Test_validateSessionCookieMinimal(t *testing.T) {
+var _ = Describe("Sessions", func() {
 	const (
 		passAuthorizationMsg = "pass_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set"
 		setAuthorizationMsg  = "set_authorization_header requires oauth tokens in sessions. session_cookie_minimal cannot be set"
@ -16,11 +19,16 @@ func Test_validateSessionCookieMinimal(t *testing.T) {
 		cookieRefreshMsg     = "cookie_refresh > 0 requires oauth tokens in sessions. session_cookie_minimal cannot be set"
 	)

-	testCases := map[string]struct {
+	type cookieMinimalTableInput struct {
 		opts       *options.Options
 		errStrings []string
-	}{
-		"No minimal cookie session": {
+	}
+
+	DescribeTable("validateSessionCookieMinimal",
+		func(o *cookieMinimalTableInput) {
+			Expect(validateSessionCookieMinimal(o.opts)).To(ConsistOf(o.errStrings))
+		},
+		Entry("No minimal cookie session", &cookieMinimalTableInput{
 			opts: &options.Options{
 				Session: options.SessionOptions{
 					Cookie: options.CookieStoreOptions{
@ -29,8 +37,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) {
 				},
 			},
 			errStrings: []string{},
-		},
-		"No minimal cookie session & passAuthorization": {
+		}),
+		Entry("No minimal cookie session & passAuthorization", &cookieMinimalTableInput{
 			opts: &options.Options{
 				Session: options.SessionOptions{
 					Cookie: options.CookieStoreOptions{
@ -40,8 +48,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) {
 				PassAuthorization: true,
 			},
 			errStrings: []string{},
-		},
-		"Minimal cookie session no conflicts": {
+		}),
+		Entry("Minimal cookie session no conflicts", &cookieMinimalTableInput{
 			opts: &options.Options{
 				Session: options.SessionOptions{
 					Cookie: options.CookieStoreOptions{
@ -50,8 +58,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) {
 				},
 			},
 			errStrings: []string{},
-		},
-		"PassAuthorization conflict": {
+		}),
+		Entry("PassAuthorization conflict", &cookieMinimalTableInput{
 			opts: &options.Options{
 				Session: options.SessionOptions{
 					Cookie: options.CookieStoreOptions{
@ -61,8 +69,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) {
 				PassAuthorization: true,
 			},
 			errStrings: []string{passAuthorizationMsg},
-		},
-		"SetAuthorization conflict": {
+		}),
+		Entry("SetAuthorization conflict", &cookieMinimalTableInput{
 			opts: &options.Options{
 				Session: options.SessionOptions{
 					Cookie: options.CookieStoreOptions{
@ -72,8 +80,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) {
 				SetAuthorization: true,
 			},
 			errStrings: []string{setAuthorizationMsg},
-		},
-		"PassAccessToken conflict": {
+		}),
+		Entry("PassAccessToken conflict", &cookieMinimalTableInput{
 			opts: &options.Options{
 				Session: options.SessionOptions{
 					Cookie: options.CookieStoreOptions{
@ -83,8 +91,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) {
 				PassAccessToken: true,
 			},
 			errStrings: []string{passAccessTokenMsg},
-		},
-		"CookieRefresh conflict": {
+		}),
+		Entry("CookieRefresh conflict", &cookieMinimalTableInput{
 			opts: &options.Options{
 				Cookie: options.Cookie{
 					Refresh: time.Hour,
@ -96,8 +104,8 @@ func Test_validateSessionCookieMinimal(t *testing.T) {
 				},
 			},
 			errStrings: []string{cookieRefreshMsg},
-		},
-		"Multiple conflicts": {
+		}),
+		Entry("Multiple conflicts", &cookieMinimalTableInput{
 			opts: &options.Options{
 				Session: options.SessionOptions{
 					Cookie: options.CookieStoreOptions{
@ -108,14 +116,228 @@ func Test_validateSessionCookieMinimal(t *testing.T) {
 				PassAccessToken:   true,
 			},
 			errStrings: []string{passAuthorizationMsg, passAccessTokenMsg},
-		},
+		}),
+	)
+
+	const (
+		clusterAndSentinelMsg     = "unable to initialize a redis client: options redis-use-sentinel and redis-use-cluster are mutually exclusive"
+		parseWrongSchemeMsg       = "unable to initialize a redis client: unable to parse redis url: invalid redis URL scheme: https"
+		parseWrongFormatMsg       = "unable to initialize a redis client: unable to parse redis url: invalid redis database number: \"wrong\""
+		invalidPasswordSetMsg     = "unable to set a redis initialization key: WRONGPASS invalid username-password pair"
+		invalidPasswordDelMsg     = "unable to delete the redis initialization key: WRONGPASS invalid username-password pair"
+		unreachableRedisSetMsg    = "unable to set a redis initialization key: dial tcp 127.0.0.1:65535: connect: connection refused"
+		unreachableRedisDelMsg    = "unable to delete the redis initialization key: dial tcp 127.0.0.1:65535: connect: connection refused"
+		unreachableSentinelSetMsg = "unable to set a redis initialization key: redis: all sentinels are unreachable"
+		unrechableSentinelDelMsg  = "unable to delete the redis initialization key: redis: all sentinels are unreachable"
+	)
+
+	type redisStoreTableInput struct {
+		// miniredis setup details
+		password        string
+		useSentinel     bool
+		setAddr         bool
+		setSentinelAddr bool
+		setMasterName   bool
+
+		opts       *options.Options
+		errStrings []string
 	}

-	for testName, tc := range testCases {
-		t.Run(testName, func(t *testing.T) {
-			errStrings := validateSessionCookieMinimal(tc.opts)
-			g := NewWithT(t)
-			g.Expect(errStrings).To(ConsistOf(tc.errStrings))
-		})
-	}
-}
+	DescribeTable("validateRedisSessionStore",
+		func(o *redisStoreTableInput) {
+			mr, err := miniredis.Run()
+			Expect(err).ToNot(HaveOccurred())
+			mr.RequireAuth(o.password)
+			defer mr.Close()
+
+			if o.setAddr && !o.useSentinel {
+				o.opts.Session.Redis.ConnectionURL = "redis://" + mr.Addr()
+			}
+
+			if o.useSentinel {
+				ms := minisentinel.NewSentinel(mr)
+				Expect(ms.Start()).To(Succeed())
+				defer ms.Close()
+
+				if o.setSentinelAddr {
+					o.opts.Session.Redis.SentinelConnectionURLs = []string{"redis://" + ms.Addr()}
+				}
+				if o.setMasterName {
+					o.opts.Session.Redis.SentinelMasterName = ms.MasterInfo().Name
+				}
+			}
+
+			Expect(validateRedisSessionStore(o.opts)).To(ConsistOf(o.errStrings))
+		},
+		Entry("cookie sessions are skipped", &redisStoreTableInput{
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.CookieSessionStoreType,
+				},
+			},
+			errStrings: []string{},
+		}),
+		Entry("connect successfully to pure redis", &redisStoreTableInput{
+			setAddr: true,
+
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+				},
+			},
+			errStrings: []string{},
+		}),
+		Entry("failed redis connection with wrong address", &redisStoreTableInput{
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						ConnectionURL: "redis://127.0.0.1:65535",
+					},
+				},
+			},
+			errStrings: []string{unreachableRedisSetMsg, unreachableRedisDelMsg},
+		}),
+		Entry("fail to parse an invalid connection URL with wrong scheme", &redisStoreTableInput{
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						ConnectionURL: "https://example.com",
+					},
+				},
+			},
+			errStrings: []string{parseWrongSchemeMsg},
+		}),
+		Entry("fail to parse an invalid connection URL with invalid format", &redisStoreTableInput{
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						ConnectionURL: "redis://127.0.0.1:6379/wrong",
+					},
+				},
+			},
+			errStrings: []string{parseWrongFormatMsg},
+		}),
+		Entry("connect successfully to pure redis with password", &redisStoreTableInput{
+			password: "abcdef123",
+			setAddr:  true,
+
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						Password: "abcdef123",
+					},
+				},
+			},
+			errStrings: []string{},
+		}),
+		Entry("failed connection with wrong password", &redisStoreTableInput{
+			password: "abcdef123",
+			setAddr:  true,
+
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						Password: "zyxwtuv987",
+					},
+				},
+			},
+			errStrings: []string{invalidPasswordSetMsg, invalidPasswordDelMsg},
+		}),
+		Entry("connect successfully to sentinel redis", &redisStoreTableInput{
+			useSentinel:     true,
+			setSentinelAddr: true,
+			setMasterName:   true,
+
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						UseSentinel: true,
+					},
+				},
+			},
+			errStrings: []string{},
+		}),
+		Entry("connect successfully to sentinel redis with 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,
+					},
+				},
+			},
+			errStrings: []string{},
+		}),
+		Entry("failed connection to sentinel redis with wrong password", &redisStoreTableInput{
+			password:        "abcdef123",
+			useSentinel:     true,
+			setSentinelAddr: true,
+			setMasterName:   true,
+
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						Password:    "zyxwtuv987",
+						UseSentinel: true,
+					},
+				},
+			},
+			errStrings: []string{invalidPasswordSetMsg, invalidPasswordDelMsg},
+		}),
+		Entry("failed connection to sentinel redis with wrong master name", &redisStoreTableInput{
+			useSentinel:     true,
+			setSentinelAddr: true,
+
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						UseSentinel:        true,
+						SentinelMasterName: "WRONG",
+					},
+				},
+			},
+			errStrings: []string{unreachableSentinelSetMsg, unrechableSentinelDelMsg},
+		}),
+		Entry("failed connection to sentinel redis with wrong address", &redisStoreTableInput{
+			useSentinel:   true,
+			setMasterName: true,
+
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						UseSentinel:            true,
+						SentinelConnectionURLs: []string{"redis://127.0.0.1:65535"},
+					},
+				},
+			},
+			errStrings: []string{unreachableSentinelSetMsg, unrechableSentinelDelMsg},
+		}),
+		Entry("sentinel and cluster both enabled fails", &redisStoreTableInput{
+			opts: &options.Options{
+				Session: options.SessionOptions{
+					Type: options.RedisSessionStoreType,
+					Redis: options.RedisStoreOptions{
+						UseSentinel: true,
+						UseCluster:  true,
+					},
+				},
+			},
+			errStrings: []string{clusterAndSentinelMsg},
+		}),
+	)
+})