Parse Redis cluster and sentinel urls (#573)

* Parse Redis cluster and sentinel urls

* Add changelog entry for #573

* Add unit tests for redis session store

* Use %v for error fmt

Co-authored-by: Amnay Mokhtari <amnay.mokhtari@adevinta.com>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

CHANGELOG.md

@@ -51,6 +51,7 @@
 
 ## Changes since v5.1.1
 
+- [#573](https://github.com/oauth2-proxy/oauth2-proxy/pull/573) Properly parse redis urls for cluster and sentinel connections (@amnay-mo)
 - [#574](https://github.com/oauth2-proxy/oauth2-proxy/pull/574) render error page on 502 proxy status (@amnay-mo)
 - [#559](https://github.com/oauth2-proxy/oauth2-proxy/pull/559) Rename cookie-domain config to cookie-domains (@JoelSpeed)
 - [#569](https://github.com/oauth2-proxy/oauth2-proxy/pull/569) Updated autocompletion for `--` long options. (@Izzette)

go.mod

@@ -3,6 +3,8 @@ module github.com/oauth2-proxy/oauth2-proxy
 go 1.14
 
 require (
+	github.com/Bose/minisentinel v0.0.0-20200130220412-917c5a9223bb
+	github.com/alicebob/miniredis v2.5.0+incompatible // indirect
 	github.com/alicebob/miniredis/v2 v2.11.2
 	github.com/bitly/go-simplejson v0.5.0
 	github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 // indirect

go.sum

@@ -2,13 +2,19 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMT
 cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 cloud.google.com/go v0.38.0 h1:ROfEUZz+Gh5pa62DJWXSaonyu3StP6EA6lPEXPI6mCo=
 cloud.google.com/go v0.38.0/go.mod h1:990N+gfupTy94rShfmMCWGDn0LpTmnzTp2qbd1dvSRU=
+github.com/Bose/minisentinel v0.0.0-20200130220412-917c5a9223bb h1:ZVN4Iat3runWOFLaBCDVU5a9X/XikSRBosye++6gojw=
+github.com/Bose/minisentinel v0.0.0-20200130220412-917c5a9223bb/go.mod h1:WsAABbY4HQBgd3mGuG4KMNTbHJCPvx9IVBHzysbknss=
 github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
+github.com/FZambia/sentinel v1.0.0/go.mod h1:ytL1Am/RLlAoAXG6Kj5LNuw/TRRQrv2rt2FT26vP5gI=
 github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6 h1:45bxf7AZMwWcqkLzDAQugVEwedisr5nRJ1r+7LYnv0U=
 github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc=
+github.com/alicebob/miniredis v2.5.0+incompatible h1:yBHoLpsyjupjz3NL3MhKMVkR41j82Yjf3KFv7ApYzUI=
+github.com/alicebob/miniredis v2.5.0+incompatible/go.mod h1:8HZjEj4yU0dwhYHky+DxYx+6BMjkBbe5ONFIF1MXffk=
+github.com/alicebob/miniredis/v2 v2.11.1/go.mod h1:UA48pmi7aSazcGAvcdKcBB49z521IC9VjTTRz2nIaJE=
 github.com/alicebob/miniredis/v2 v2.11.2 h1:OtWO7akm5otuhssnE/sNfsWxG4gZ8DbGhShDtRrByJs=
 github.com/alicebob/miniredis/v2 v2.11.2/go.mod h1:VL3UDEfAH59bSa7MuHMuFToxkqyHh69s/WUbYlOAuyg=
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
@@ -72,6 +78,8 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
 github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc=
+github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY=
+github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
 github.com/googleapis/gax-go/v2 v2.0.5 h1:sjZBwGj9Jlw33ImPtvFviGYvseOtDM7hkSKB7+Tv3SM=
 github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
@@ -106,6 +114,7 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/magiconair/properties v1.8.1 h1:ZC2Vc7/ZFkGmsVC9KvOjumD+G5lXy2RtTKyzRKO2BQ4=
 github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
+github.com/matryer/is v1.2.0/go.mod h1:2fLPjFQM9rhQ15aVEtbuwhJinnOqrmgXPNdZsdwlWXA=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/mbland/hmacauth v0.0.0-20170912233209-44256dfd4bfa h1:hI1uC2A3vJFjwvBn0G0a7QBRdBUp6Y048BtLAHRTKPo=
 github.com/mbland/hmacauth v0.0.0-20170912233209-44256dfd4bfa/go.mod h1:8vxFeeg++MqgCHwehSuwTlYCF0ALyDJbYJ1JsKi7v6s=
@@ -172,6 +181,8 @@ github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q
 github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
 github.com/yhat/wsutil v0.0.0-20170731153501-1d66fa95c997 h1:1+FQ4Ns+UZtUiQ4lP0sTCyKSQ0EXoiwAdHZB0Pd5t9Q=
 github.com/yhat/wsutil v0.0.0-20170731153501-1d66fa95c997/go.mod h1:DIGbh/f5XMAessMV/uaIik81gkDVjUeQ9ApdaU7wRKE=
+github.com/yuin/gopher-lua v0.0.0-20190206043414-8bfc7677f583/go.mod h1:gqRgreBUhTSL0GeU64rtZ3Uq3wtjOa/TB2YfrtkCbVQ=
+github.com/yuin/gopher-lua v0.0.0-20191213034115-f46add6fdb5c/go.mod h1:gqRgreBUhTSL0GeU64rtZ3Uq3wtjOa/TB2YfrtkCbVQ=
 github.com/yuin/gopher-lua v0.0.0-20191220021717-ab39c6098bdb h1:ZkM6LRnq40pR1Ox0hTHlnpkcOTuFIDQpZ1IN8rKKhX0=
 github.com/yuin/gopher-lua v0.0.0-20191220021717-ab39c6098bdb/go.mod h1:gqRgreBUhTSL0GeU64rtZ3Uq3wtjOa/TB2YfrtkCbVQ=
 go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=

pkg/sessions/redis/redis_store.go

@@ -60,16 +60,24 @@ func newRedisCmdable(opts options.RedisStoreOptions) (Client, error) {
 	}
 
 	if opts.UseSentinel {
+		addrs, err := parseRedisURLs(opts.SentinelConnectionURLs)
+		if err != nil {
+			return nil, fmt.Errorf("could not parse redis urls: %v", err)
+		}
 		client := redis.NewFailoverClient(&redis.FailoverOptions{
 			MasterName:    opts.SentinelMasterName,
-			SentinelAddrs: opts.SentinelConnectionURLs,
+			SentinelAddrs: addrs,
 		})
 		return newClient(client), nil
 	}
 
 	if opts.UseCluster {
+		addrs, err := parseRedisURLs(opts.ClusterConnectionURLs)
+		if err != nil {
+			return nil, fmt.Errorf("could not parse redis urls: %v", err)
+		}
 		client := redis.NewClusterClient(&redis.ClusterOptions{
-			Addrs: opts.ClusterConnectionURLs,
+			Addrs: addrs,
 		})
 		return newClusterClient(client), nil
 	}
@@ -108,6 +116,20 @@ func newRedisCmdable(opts options.RedisStoreOptions) (Client, error) {
 	return newClient(client), nil
 }
 
+// parseRedisURLs parses a list of redis urls and returns a list
+// of addresses in the form of host:port that can be used to connnect to Redis
+func parseRedisURLs(urls []string) ([]string, error) {
+	addrs := []string{}
+	for _, u := range urls {
+		parsedURL, err := redis.ParseURL(u)
+		if err != nil {
+			return nil, fmt.Errorf("unable to parse redis url: %v", err)
+		}
+		addrs = append(addrs, parsedURL.Addr)
+	}
+	return addrs, nil
+}
+
 // Save takes a sessions.SessionState and stores the information from it
 // to redies, and adds a new ticket cookie on the HTTP response writer
 func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *sessions.SessionState) error {

pkg/sessions/redis/redis_store_test.go

@@ -0,0 +1,60 @@
+package redis
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/Bose/minisentinel"
+	"github.com/alicebob/miniredis/v2"
+	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
+	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRedisStore(t *testing.T) {
+	t.Run("save session on redis standalone", func(t *testing.T) {
+		redisServer, err := miniredis.Run()
+		require.NoError(t, err)
+		defer redisServer.Close()
+		opts := options.NewOptions()
+		redisURL := url.URL{
+			Scheme: "redis",
+			Host:   redisServer.Addr(),
+		}
+		opts.Session.Redis.ConnectionURL = redisURL.String()
+		redisStore, err := NewRedisSessionStore(&opts.Session, &opts.Cookie)
+		require.NoError(t, err)
+		err = redisStore.Save(
+			httptest.NewRecorder(),
+			httptest.NewRequest(http.MethodGet, "/", nil),
+			&sessions.SessionState{})
+		assert.NoError(t, err)
+	})
+	t.Run("save session on redis sentinel", func(t *testing.T) {
+		redisServer, err := miniredis.Run()
+		require.NoError(t, err)
+		defer redisServer.Close()
+		sentinel := minisentinel.NewSentinel(redisServer)
+		err = sentinel.Start()
+		require.NoError(t, err)
+		defer sentinel.Close()
+		opts := options.NewOptions()
+		sentinelURL := url.URL{
+			Scheme: "redis",
+			Host:   sentinel.Addr(),
+		}
+		opts.Session.Redis.SentinelConnectionURLs = []string{sentinelURL.String()}
+		opts.Session.Redis.UseSentinel = true
+		opts.Session.Redis.SentinelMasterName = sentinel.MasterInfo().Name
+		redisStore, err := NewRedisSessionStore(&opts.Session, &opts.Cookie)
+		require.NoError(t, err)
+		err = redisStore.Save(
+			httptest.NewRecorder(),
+			httptest.NewRequest(http.MethodGet, "/", nil),
+			&sessions.SessionState{})
+		assert.NoError(t, err)
+	})
+}