d9a45a3 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Merge pull request #577 from oauth2-proxy/session-store-cipher

Move Cipher and Session Store initialisation out of Validation

commitd9a45a3b47f0b7e5cf004a1573fb732c59cb7c7d authorJoel Speed <Joel.speed@hotmail.co.uk> dateSun 28 Jun 2020, 18:29 (6 years ago) parentd9af3ff 6e1b3b9

12 files changed+180−98

compare with master

unifiedsplit

CHANGELOG.md+2 go.sum+2 main.go+5−1 oauthproxy.go+9−3 oauthproxy_test.go+105−52 pkg/apis/options/options.go−4 pkg/apis/options/sessions.go+2−5 pkg/sessions/cookie/session_store.go+6−1 pkg/sessions/redis/redis_store.go+6−1 pkg/sessions/redis/redis_store_test.go+9 pkg/sessions/session_store_test.go+34−14 pkg/validation/options.go−17

CHANGELOG.md+2

@@ -8,6 +8,8 @@

99## Changes since v6.0.0

1010

11+- [#577](https://github.com/oauth2-proxy/oauth2-proxy/pull/577) Move Cipher and Session Store initialisation out of Validation (@JoelSpeed)

12+

1113# v6.0.0

1214

1315## Release Highlights

go.sum+2

@@ -202,6 +202,7 @@ go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=

202202golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=

203203golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M=

204204golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=

205+golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8=

205206golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=

206207golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=

207208golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=

@@ -225,6 +226,7 @@ golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR

225226golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=

226227golang.org/x/net v0.0.0-20190923162816-aa69164e4478 h1:l5EDrHhldLYb3ZRHDUhXF7Om7MvYXnkV9/iQNo1lX6g=

227228golang.org/x/net v0.0.0-20190923162816-aa69164e4478/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=

229+golang.org/x/net v0.0.0-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZVDP2S5ou6y0gSgXHu8=

228230golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=

229231golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=

230232golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=

main.go+5−1

@@ -45,7 +45,11 @@ func main() {

4545 }

4646

4747 validator := NewValidator(opts.EmailDomains, opts.AuthenticatedEmailsFile)

48− oauthproxy := NewOAuthProxy(opts, validator)

48+ oauthproxy, err := NewOAuthProxy(opts, validator)

49+ if err != nil {

50+ logger.Printf("ERROR: Failed to initialise OAuth2 Proxy: %v", err)

51+ os.Exit(1)

52+ }

4953

5054 if len(opts.Banner) >= 1 {

5155 if opts.Banner == "-" {

oauthproxy.go+9−3

@@ -26,6 +26,7 @@ import (

2626 "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"

2727 "github.com/oauth2-proxy/oauth2-proxy/pkg/ip"

2828 "github.com/oauth2-proxy/oauth2-proxy/pkg/logger"

29+ "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions"

2930 "github.com/oauth2-proxy/oauth2-proxy/providers"

3031 "github.com/yhat/wsutil"

3132)

@@ -231,7 +232,12 @@ func NewWebSocketOrRestReverseProxy(u *url.URL, opts *options.Options, auth hmac

231232}

232233

233234// NewOAuthProxy creates a new instance of OAuthProxy from the options provided

234−func NewOAuthProxy(opts *options.Options, validator func(string) bool) *OAuthProxy {

235+func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthProxy, error) {

236+ sessionStore, err := sessions.NewSessionStore(&opts.Session, &opts.Cookie)

237+ if err != nil {

238+ return nil, fmt.Errorf("error initialising session store: %v", err)

239+ }

240+

235241 serveMux := http.NewServeMux()

236242 var auth hmacauth.HmacAuth

237243 if sigData := opts.GetSignatureData(); sigData != nil {

@@ -321,7 +327,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) *OAuthPro

321327 ProxyPrefix: opts.ProxyPrefix,

322328 provider: opts.GetProvider(),

323329 providerNameOverride: opts.ProviderName,

324− sessionStore: opts.GetSessionStore(),

330+ sessionStore: sessionStore,

325331 serveMux: serveMux,

326332 redirectURL: redirectURL,

327333 whitelistDomains: opts.WhitelistDomains,

@@ -345,7 +351,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) *OAuthPro

345351 templates: loadTemplates(opts.CustomTemplatesDir),

346352 Banner: opts.Banner,

347353 Footer: opts.Footer,

348− }

354+ }, nil

349355}

350356

351357// GetRedirectURI returns the redirectURL that the upstream OAuth Provider will

oauthproxy_test.go+105−52

@@ -35,7 +35,7 @@ const (

3535 // The rawCookieSecret is 32 bytes and the base64CookieSecret is the base64

3636 // encoded version of this.

3737 rawCookieSecret = "secretthirtytwobytes+abcdefghijk"

38− base64CookieSecret = "c2VjcmV0dGhpcnR5dHdvYnl0ZXMrYWJjZGVmZ2hpamsK"

38+ base64CookieSecret = "c2VjcmV0dGhpcnR5dHdvYnl0ZXMrYWJjZGVmZ2hpams"

3939)

4040

4141func init() {

@@ -82,10 +82,10 @@ func TestWebSocketProxy(t *testing.T) {

8282

8383 backendURL, _ := url.Parse(backend.URL)

8484

85− options := options.NewOptions()

85+ opts := baseTestOptions()

8686 var auth hmacauth.HmacAuth

87− options.PassHostHeader = true

88− proxyHandler := NewWebSocketOrRestReverseProxy(backendURL, options, auth)

87+ opts.PassHostHeader = true

88+ proxyHandler := NewWebSocketOrRestReverseProxy(backendURL, opts, auth)

8989 frontend := httptest.NewServer(proxyHandler)

9090 defer frontend.Close()

9191

@@ -172,13 +172,14 @@ func TestEncodedSlashes(t *testing.T) {

172172}

173173

174174func TestRobotsTxt(t *testing.T) {

175− opts := options.NewOptions()

175+ opts := baseTestOptions()

176176 opts.ClientID = "asdlkjx"

177177 opts.ClientSecret = "alkgks"

178178 opts.Cookie.Secret = rawCookieSecret

179179 validation.Validate(opts)

180180

181− proxy := NewOAuthProxy(opts, func(string) bool { return true })

181+ proxy, err := NewOAuthProxy(opts, func(string) bool { return true })

182+ assert.NoError(t, err)

182183 rw := httptest.NewRecorder()

183184 req, _ := http.NewRequest("GET", "/robots.txt", nil)

184185 proxy.ServeHTTP(rw, req)

@@ -187,7 +188,7 @@ func TestRobotsTxt(t *testing.T) {

187188}

188189

189190func TestIsValidRedirect(t *testing.T) {

190− opts := options.NewOptions()

191+ opts := baseTestOptions()

191192 opts.ClientID = "skdlfj"

192193 opts.ClientSecret = "fgkdsgj"

193194 opts.Cookie.Secret = base64CookieSecret

@@ -202,7 +203,8 @@ func TestIsValidRedirect(t *testing.T) {

202203 }

203204 validation.Validate(opts)

204205

205− proxy := NewOAuthProxy(opts, func(string) bool { return true })

206+ proxy, err := NewOAuthProxy(opts, func(string) bool { return true })

207+ assert.NoError(t, err)

206208

207209 testCases := []struct {

208210 Desc, Redirect string

@@ -453,11 +455,10 @@ func TestOpenRedirects(t *testing.T) {

453455 "www.whitelisteddomain.tld",

454456 }

455457 err := validation.Validate(opts)

456− if err != nil {

457− t.Fatal(err)

458− }

458+ assert.NoError(t, err)

459459

460− proxy := NewOAuthProxy(opts, func(string) bool { return true })

460+ proxy, err := NewOAuthProxy(opts, func(string) bool { return true })

461+ assert.NoError(t, err)

461462

462463 file, err := os.Open("./test/openredirects.txt")

463464 if err != nil {

@@ -545,7 +546,7 @@ func TestBasicAuthPassword(t *testing.T) {

545546 w.WriteHeader(200)

546547 w.Write([]byte(payload))

547548 }))

548− opts := options.NewOptions()

549+ opts := baseTestOptions()

549550 opts.Upstreams = append(opts.Upstreams, providerServer.URL)

550551 // The CookieSecret must be 32 bytes in order to create the AES

551552 // cipher.

@@ -564,9 +565,10 @@ func TestBasicAuthPassword(t *testing.T) {

564565 const emailAddress = "john.doe@example.com"

565566

566567 opts.SetProvider(NewTestProvider(providerURL, emailAddress))

567− proxy := NewOAuthProxy(opts, func(email string) bool {

568+ proxy, err := NewOAuthProxy(opts, func(email string) bool {

568569 return email == emailAddress

569570 })

571+ assert.NoError(t, err)

570572

571573 rw := httptest.NewRecorder()

572574 req, _ := http.NewRequest("GET", "/oauth2/callback?code=callback_code&state=nonce:", strings.NewReader(""))

@@ -611,11 +613,12 @@ func TestBasicAuthPassword(t *testing.T) {

611613}

612614

613615func TestBasicAuthWithEmail(t *testing.T) {

614− opts := options.NewOptions()

616+ opts := baseTestOptions()

615617 opts.PassBasicAuth = true

616618 opts.PassUserHeaders = false

617619 opts.PreferEmailToUser = false

618620 opts.BasicAuthPassword = "This is a secure password"

621+ opts.Cookie.Secret = rawCookieSecret

619622 validation.Validate(opts)

620623

621624 const emailAddress = "john.doe@example.com"

@@ -635,9 +638,10 @@ func TestBasicAuthWithEmail(t *testing.T) {

635638 {

636639 rw := httptest.NewRecorder()

637640 req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase0", nil)

638− proxy := NewOAuthProxy(opts, func(email string) bool {

641+ proxy, err := NewOAuthProxy(opts, func(email string) bool {

639642 return email == emailAddress

640643 })

644+ assert.NoError(t, err)

641645 proxy.addHeadersForProxying(rw, req, session)

642646 assert.Equal(t, expectedUserHeader, req.Header["Authorization"][0])

643647 assert.Equal(t, userName, req.Header["X-Forwarded-User"][0])

@@ -648,9 +652,10 @@ func TestBasicAuthWithEmail(t *testing.T) {

648652 rw := httptest.NewRecorder()

649653 req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase1", nil)

650654

651− proxy := NewOAuthProxy(opts, func(email string) bool {

655+ proxy, err := NewOAuthProxy(opts, func(email string) bool {

652656 return email == emailAddress

653657 })

658+ assert.NoError(t, err)

654659 proxy.addHeadersForProxying(rw, req, session)

655660 assert.Equal(t, expectedEmailHeader, req.Header["Authorization"][0])

656661 assert.Equal(t, emailAddress, req.Header["X-Forwarded-User"][0])

@@ -658,10 +663,11 @@ func TestBasicAuthWithEmail(t *testing.T) {

658663}

659664

660665func TestPassUserHeadersWithEmail(t *testing.T) {

661− opts := options.NewOptions()

666+ opts := baseTestOptions()

662667 opts.PassBasicAuth = false

663668 opts.PassUserHeaders = true

664669 opts.PreferEmailToUser = false

670+ opts.Cookie.Secret = base64CookieSecret

665671 validation.Validate(opts)

666672

667673 const emailAddress = "john.doe@example.com"

@@ -677,9 +683,10 @@ func TestPassUserHeadersWithEmail(t *testing.T) {

677683 {

678684 rw := httptest.NewRecorder()

679685 req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase0", nil)

680− proxy := NewOAuthProxy(opts, func(email string) bool {

686+ proxy, err := NewOAuthProxy(opts, func(email string) bool {

681687 return email == emailAddress

682688 })

689+ assert.NoError(t, err)

683690 proxy.addHeadersForProxying(rw, req, session)

684691 assert.Equal(t, userName, req.Header["X-Forwarded-User"][0])

685692 }

@@ -689,9 +696,10 @@ func TestPassUserHeadersWithEmail(t *testing.T) {

689696 rw := httptest.NewRecorder()

690697 req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase1", nil)

691698

692− proxy := NewOAuthProxy(opts, func(email string) bool {

699+ proxy, err := NewOAuthProxy(opts, func(email string) bool {

693700 return email == emailAddress

694701 })

702+ assert.NoError(t, err)

695703 proxy.addHeadersForProxying(rw, req, session)

696704 assert.Equal(t, emailAddress, req.Header["X-Forwarded-User"][0])

697705 }

@@ -727,7 +735,7 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) *PassAccessTokenTes

727735 w.Write([]byte(payload))

728736 }))

729737

730− t.opts = options.NewOptions()

738+ t.opts = baseTestOptions()

731739 t.opts.Upstreams = append(t.opts.Upstreams, t.providerServer.URL)

732740 if opts.ProxyUpstream != "" {

733741 t.opts.Upstreams = append(t.opts.Upstreams, opts.ProxyUpstream)

@@ -745,9 +753,13 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) *PassAccessTokenTes

745753 const emailAddress = "michael.bland@gsa.gov"

746754

747755 t.opts.SetProvider(NewTestProvider(providerURL, emailAddress))

748− t.proxy = NewOAuthProxy(t.opts, func(email string) bool {

756+ var err error

757+ t.proxy, err = NewOAuthProxy(t.opts, func(email string) bool {

749758 return email == emailAddress

750759 })

760+ if err != nil {

761+ panic(err)

762+ }

751763 return t

752764}

753765

@@ -886,16 +898,20 @@ const signInSkipProvider = `>Found<`

886898func NewSignInPageTest(skipProvider bool) *SignInPageTest {

887899 var sipTest SignInPageTest

888900

889− sipTest.opts = options.NewOptions()

901+ sipTest.opts = baseTestOptions()

890902 sipTest.opts.Cookie.Secret = rawCookieSecret

891903 sipTest.opts.ClientID = "lkdgj"

892904 sipTest.opts.ClientSecret = "sgiufgoi"

893905 sipTest.opts.SkipProviderButton = skipProvider

894906 validation.Validate(sipTest.opts)

895907

896− sipTest.proxy = NewOAuthProxy(sipTest.opts, func(email string) bool {

908+ var err error

909+ sipTest.proxy, err = NewOAuthProxy(sipTest.opts, func(email string) bool {

897910 return true

898911 })

912+ if err != nil {

913+ panic(err)

914+ }

899915 sipTest.signInRegexp = regexp.MustCompile(signInRedirectPattern)

900916 sipTest.signInProviderRegexp = regexp.MustCompile(signInSkipProvider)

901917

@@ -987,7 +1003,7 @@ type OptionsModifier func(*options.Options)

9871003func NewProcessCookieTest(opts ProcessCookieTestOpts, modifiers ...OptionsModifier) *ProcessCookieTest {

9881004 var pcTest ProcessCookieTest

9891005

990− pcTest.opts = options.NewOptions()

1006+ pcTest.opts = baseTestOptions()

9911007 for _, modifier := range modifiers {

9921008 modifier(pcTest.opts)

9931009 }

@@ -999,9 +1015,13 @@ func NewProcessCookieTest(opts ProcessCookieTestOpts, modifiers ...OptionsModifi

9991015 pcTest.opts.Cookie.Refresh = time.Hour

10001016 validation.Validate(pcTest.opts)

10011017

1002− pcTest.proxy = NewOAuthProxy(pcTest.opts, func(email string) bool {

1018+ var err error

1019+ pcTest.proxy, err = NewOAuthProxy(pcTest.opts, func(email string) bool {

10031020 return pcTest.validateUser

10041021 })

1022+ if err != nil {

1023+ panic(err)

1024+ }

10051025 pcTest.proxy.provider = &TestProvider{

10061026 ValidToken: opts.providerValidateCookieResponse,

10071027 }

@@ -1201,13 +1221,19 @@ func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) {

12011221func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) {

12021222 var pcTest ProcessCookieTest

12031223

1204− pcTest.opts = options.NewOptions()

1224+ pcTest.opts = baseTestOptions()

12051225 pcTest.opts.SetXAuthRequest = true

1206− validation.Validate(pcTest.opts)

1226+ err := validation.Validate(pcTest.opts)

1227+ if err != nil {

1228+ panic(err)

1229+ }

12071230

1208− pcTest.proxy = NewOAuthProxy(pcTest.opts, func(email string) bool {

1231+ pcTest.proxy, err = NewOAuthProxy(pcTest.opts, func(email string) bool {

12091232 return pcTest.validateUser

12101233 })

1234+ if err != nil {

1235+ panic(err)

1236+ }

12111237 pcTest.proxy.provider = &TestProvider{

12121238 ValidToken: true,

12131239 }

@@ -1232,14 +1258,18 @@ func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) {

12321258func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) {

12331259 var pcTest ProcessCookieTest

12341260

1235− pcTest.opts = options.NewOptions()

1261+ pcTest.opts = baseTestOptions()

12361262 pcTest.opts.SetXAuthRequest = true

12371263 pcTest.opts.SetBasicAuth = true

12381264 validation.Validate(pcTest.opts)

12391265

1240− pcTest.proxy = NewOAuthProxy(pcTest.opts, func(email string) bool {

1266+ var err error

1267+ pcTest.proxy, err = NewOAuthProxy(pcTest.opts, func(email string) bool {

12411268 return pcTest.validateUser

12421269 })

1270+ if err != nil {

1271+ panic(err)

1272+ }

12431273 pcTest.proxy.provider = &TestProvider{

12441274 ValidToken: true,

12451275 }

@@ -1266,14 +1296,18 @@ func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) {

12661296func TestAuthOnlyEndpointSetBasicAuthFalseRequestHeaders(t *testing.T) {

12671297 var pcTest ProcessCookieTest

12681298

1269− pcTest.opts = options.NewOptions()

1299+ pcTest.opts = baseTestOptions()

12701300 pcTest.opts.SetXAuthRequest = true

12711301 pcTest.opts.SetBasicAuth = false

12721302 validation.Validate(pcTest.opts)

12731303

1274− pcTest.proxy = NewOAuthProxy(pcTest.opts, func(email string) bool {

1304+ var err error

1305+ pcTest.proxy, err = NewOAuthProxy(pcTest.opts, func(email string) bool {

12751306 return pcTest.validateUser

12761307 })

1308+ if err != nil {

1309+ panic(err)

1310+ }

12771311 pcTest.proxy.provider = &TestProvider{

12781312 ValidToken: true,

12791313 }

@@ -1303,18 +1337,16 @@ func TestAuthSkippedForPreflightRequests(t *testing.T) {

13031337 }))

13041338 defer upstream.Close()

13051339

1306− opts := options.NewOptions()

1340+ opts := baseTestOptions()

13071341 opts.Upstreams = append(opts.Upstreams, upstream.URL)

1308− opts.ClientID = "aljsal"

1309− opts.ClientSecret = "jglkfsdgj"

1310− opts.Cookie.Secret = base64CookieSecret

13111342 opts.SkipAuthPreflight = true

13121343 validation.Validate(opts)

13131344

13141345 upstreamURL, _ := url.Parse(upstream.URL)

13151346 opts.SetProvider(NewTestProvider(upstreamURL, ""))

13161347

1317− proxy := NewOAuthProxy(opts, func(string) bool { return false })

1348+ proxy, err := NewOAuthProxy(opts, func(string) bool { return false })

1349+ assert.NoError(t, err)

13181350 rw := httptest.NewRecorder()

13191351 req, _ := http.NewRequest("OPTIONS", "/preflight-request", nil)

13201352 proxy.ServeHTTP(rw, req)

@@ -1353,7 +1385,7 @@ type SignatureTest struct {

13531385}

13541386

13551387func NewSignatureTest() *SignatureTest {

1356− opts := options.NewOptions()

1388+ opts := baseTestOptions()

13571389 opts.Cookie.Secret = rawCookieSecret

13581390 opts.ClientID = "client ID"

13591391 opts.ClientSecret = "client secret"

@@ -1409,7 +1441,10 @@ func (st *SignatureTest) MakeRequestWithExpectedKey(method, body, key string) {

14091441 if err != nil {

14101442 panic(err)

14111443 }

1412− proxy := NewOAuthProxy(st.opts, func(email string) bool { return true })

1444+ proxy, err := NewOAuthProxy(st.opts, func(email string) bool { return true })

1445+ if err != nil {

1446+ panic(err)

1447+ }

14131448

14141449 var bodyBuf io.ReadCloser

14151450 if body != "" {

@@ -1461,10 +1496,12 @@ func TestRequestSignaturePostRequest(t *testing.T) {

14611496}

14621497

14631498func TestGetRedirect(t *testing.T) {

1464− options := options.NewOptions()

1465− _ = validation.Validate(options)

1466− require.NotEmpty(t, options.ProxyPrefix)

1467− proxy := NewOAuthProxy(options, func(s string) bool { return false })

1499+ opts := baseTestOptions()

1500+ err := validation.Validate(opts)

1501+ assert.NoError(t, err)

1502+ require.NotEmpty(t, opts.ProxyPrefix)

1503+ proxy, err := NewOAuthProxy(opts, func(s string) bool { return false })

1504+ assert.NoError(t, err)

14681505

14691506 tests := []struct {

14701507 name string

@@ -1500,14 +1537,19 @@ type ajaxRequestTest struct {

15001537

15011538func newAjaxRequestTest() *ajaxRequestTest {

15021539 test := &ajaxRequestTest{}

1503− test.opts = options.NewOptions()

1540+ test.opts = baseTestOptions()

15041541 test.opts.Cookie.Secret = base64CookieSecret

15051542 test.opts.ClientID = "gkljfdl"

15061543 test.opts.ClientSecret = "sdflkjs"

15071544 validation.Validate(test.opts)

1508− test.proxy = NewOAuthProxy(test.opts, func(email string) bool {

1545+

1546+ var err error

1547+ test.proxy, err = NewOAuthProxy(test.opts, func(email string) bool {

15091548 return true

15101549 })

1550+ if err != nil {

1551+ panic(err)

1552+ }

15111553 return test

15121554}

15131555

@@ -1558,11 +1600,12 @@ func TestAjaxForbiddendRequest(t *testing.T) {

15581600}

15591601

15601602func TestClearSplitCookie(t *testing.T) {

1561− opts := options.NewOptions()

1603+ opts := baseTestOptions()

1604+ opts.Cookie.Secret = base64CookieSecret

15621605 opts.Cookie.Name = "oauth2"

15631606 opts.Cookie.Domains = []string{"abc"}

15641607 store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie)

1565− assert.Equal(t, err, nil)

1608+ assert.Equal(t, nil, err)

15661609 p := OAuthProxy{CookieName: opts.Cookie.Name, CookieDomains: opts.Cookie.Domains, sessionStore: store}

15671610 var rw = httptest.NewRecorder()

15681611 req := httptest.NewRequest("get", "/", nil)

@@ -1587,11 +1630,11 @@ func TestClearSplitCookie(t *testing.T) {

15871630}

15881631

15891632func TestClearSingleCookie(t *testing.T) {

1590− opts := options.NewOptions()

1633+ opts := baseTestOptions()

15911634 opts.Cookie.Name = "oauth2"

15921635 opts.Cookie.Domains = []string{"abc"}

15931636 store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie)

1594− assert.Equal(t, err, nil)

1637+ assert.Equal(t, nil, err)

15951638 p := OAuthProxy{CookieName: opts.Cookie.Name, CookieDomains: opts.Cookie.Domains, sessionStore: store}

15961639 var rw = httptest.NewRecorder()

15971640 req := httptest.NewRequest("get", "/", nil)

@@ -1768,13 +1811,14 @@ func Test_noCacheHeadersDoesNotExistsInResponseHeadersFromUpstream(t *testing.T)

17681811 }))

17691812 t.Cleanup(upstream.Close)

17701813

1771− opts := options.NewOptions()

1814+ opts := baseTestOptions()

17721815 opts.Upstreams = []string{upstream.URL}

17731816 opts.SkipAuthRegex = []string{".*"}

17741817 _ = validation.Validate(opts)

1775− proxy := NewOAuthProxy(opts, func(email string) bool {

1818+ proxy, err := NewOAuthProxy(opts, func(email string) bool {

17761819 return true

17771820 })

1821+ assert.NoError(t, err)

17781822

17791823 rec := httptest.NewRecorder()

17801824 req := httptest.NewRequest(http.MethodGet, "/upstream", nil)

@@ -1788,3 +1832,12 @@ func Test_noCacheHeadersDoesNotExistsInResponseHeadersFromUpstream(t *testing.T)

17881832 assert.Equal(t, "", rec.Header().Get(k))

17891833 }

17901834}

1835+

1836+func baseTestOptions() *options.Options {

1837+ opts := options.NewOptions()

1838+ opts.Cookie.Secret = rawCookieSecret

1839+ opts.ClientID = "cliend-id"

1840+ opts.ClientSecret = "client-secret"

1841+ opts.EmailDomains = []string{"*"}

1842+ return opts

1843+}

pkg/apis/options/options.go−4

@@ -8,7 +8,6 @@ import (

99 oidc "github.com/coreos/go-oidc"

1010 ipapi "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/ip"

11− sessionsapi "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"

1211 "github.com/oauth2-proxy/oauth2-proxy/providers"

1312 "github.com/spf13/pflag"

1413)

@@ -115,7 +114,6 @@ type Options struct {

115114 proxyURLs []*url.URL

116115 compiledRegex []*regexp.Regexp

117116 provider providers.Provider

118− sessionStore sessionsapi.SessionStore

119117 signatureData *SignatureData

120118 oidcVerifier *oidc.IDTokenVerifier

121119 jwtBearerVerifiers []*oidc.IDTokenVerifier

@@ -127,7 +125,6 @@ func (o *Options) GetRedirectURL() *url.URL { return o.re

127125func (o *Options) GetProxyURLs() []*url.URL { return o.proxyURLs }

128126func (o *Options) GetCompiledRegex() []*regexp.Regexp { return o.compiledRegex }

129127func (o *Options) GetProvider() providers.Provider { return o.provider }

130−func (o *Options) GetSessionStore() sessionsapi.SessionStore { return o.sessionStore }

131128func (o *Options) GetSignatureData() *SignatureData { return o.signatureData }

132129func (o *Options) GetOIDCVerifier() *oidc.IDTokenVerifier { return o.oidcVerifier }

133130func (o *Options) GetJWTBearerVerifiers() []*oidc.IDTokenVerifier { return o.jwtBearerVerifiers }

@@ -138,7 +135,6 @@ func (o *Options) SetRedirectURL(s *url.URL) { o.redirect

138135func (o *Options) SetProxyURLs(s []*url.URL) { o.proxyURLs = s }

139136func (o *Options) SetCompiledRegex(s []*regexp.Regexp) { o.compiledRegex = s }

140137func (o *Options) SetProvider(s providers.Provider) { o.provider = s }

141−func (o *Options) SetSessionStore(s sessionsapi.SessionStore) { o.sessionStore = s }

142138func (o *Options) SetSignatureData(s *SignatureData) { o.signatureData = s }

143139func (o *Options) SetOIDCVerifier(s *oidc.IDTokenVerifier) { o.oidcVerifier = s }

144140func (o *Options) SetJWTBearerVerifiers(s []*oidc.IDTokenVerifier) { o.jwtBearerVerifiers = s }

pkg/apis/options/sessions.go+2−5

@@ -1,12 +1,9 @@

11package options

3−import "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"

4−

53// SessionOptions contains configuration options for the SessionStore providers.

64type SessionOptions struct {

7− Type string `flag:"session-store-type" cfg:"session_store_type"`

8− Cipher encryption.Cipher `cfg:",internal"`

9− Redis RedisStoreOptions `cfg:",squash"`

5+ Type string `flag:"session-store-type" cfg:"session_store_type"`

6+ Redis RedisStoreOptions `cfg:",squash"`

107}

118

129// CookieSessionStoreType is used to indicate the CookieSessionStore should be

pkg/sessions/cookie/session_store.go+6−1

@@ -127,8 +127,13 @@ func (s *SessionStore) makeCookie(req *http.Request, name string, value string,

127127// NewCookieSessionStore initialises a new instance of the SessionStore from

128128// the configuration given

129129func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {

130+ cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(cookieOpts.Secret))

131+ if err != nil {

132+ return nil, fmt.Errorf("error initialising cipher: %v", err)

133+ }

134+

130135 return &SessionStore{

131− CookieCipher: opts.Cipher,

136+ CookieCipher: cipher,

132137 CookieOptions: cookieOpts,

133138 }, nil

134139}

pkg/sessions/redis/redis_store.go+6−1

@@ -40,6 +40,11 @@ type SessionStore struct {

4040// NewRedisSessionStore initialises a new instance of the SessionStore from

4141// the configuration given

4242func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {

43+ cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(cookieOpts.Secret))

44+ if err != nil {

45+ return nil, fmt.Errorf("error initialising cipher: %v", err)

46+ }

47+

4348 client, err := newRedisCmdable(opts.Redis)

4449 if err != nil {

4550 return nil, fmt.Errorf("error constructing redis client: %v", err)

@@ -47,7 +52,7 @@ func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.Cook

4752

4853 rs := &SessionStore{

4954 Client: client,

50− CookieCipher: opts.Cipher,

55+ CookieCipher: cipher,

5156 CookieOptions: cookieOpts,

5257 }

5358 return rs, nil

pkg/sessions/redis/redis_store_test.go+9

@@ -1,6 +1,7 @@

11package redis

33import (

4+ "crypto/rand"

45 "net/http"

56 "net/http/httptest"

67 "net/url"

@@ -15,6 +16,10 @@ import (

1516)

1617

1718func TestRedisStore(t *testing.T) {

19+ secret := make([]byte, 32)

20+ _, err := rand.Read(secret)

21+ assert.NoError(t, err)

22+

1823 t.Run("save session on redis standalone", func(t *testing.T) {

1924 redisServer, err := miniredis.Run()

2025 require.NoError(t, err)

@@ -25,6 +30,8 @@ func TestRedisStore(t *testing.T) {

2530 Host: redisServer.Addr(),

2631 }

2732 opts.Session.Redis.ConnectionURL = redisURL.String()

33+

34+ opts.Cookie.Secret = string(secret)

2835 redisStore, err := NewRedisSessionStore(&opts.Session, &opts.Cookie)

2936 require.NoError(t, err)

3037 err = redisStore.Save(

@@ -49,6 +56,8 @@ func TestRedisStore(t *testing.T) {

4956 opts.Session.Redis.SentinelConnectionURLs = []string{sentinelURL.String()}

5057 opts.Session.Redis.UseSentinel = true

5158 opts.Session.Redis.SentinelMasterName = sentinel.MasterInfo().Name

59+

60+ opts.Cookie.Secret = string(secret)

5261 redisStore, err := NewRedisSessionStore(&opts.Session, &opts.Cookie)

5362 require.NoError(t, err)

5463 err = redisStore.Save(

pkg/sessions/session_store_test.go+34−14

@@ -353,24 +353,11 @@ var _ = Describe("NewSessionStore", func() {

353353 SameSite: "strict",

354354 }

355355

356− var err error

357− ss, err = sessions.NewSessionStore(opts, cookieOpts)

358− Expect(err).ToNot(HaveOccurred())

359− })

360−

361− SessionStoreInterfaceTests(persistent)

362− })

363−

364− Context("with a cipher", func() {

365− BeforeEach(func() {

356+ // A secret is required but not defaulted

366357 secret := make([]byte, 32)

367358 _, err := rand.Read(secret)

368359 Expect(err).ToNot(HaveOccurred())

369360 cookieOpts.Secret = base64.URLEncoding.EncodeToString(secret)

370− cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(cookieOpts.Secret))

371− Expect(err).ToNot(HaveOccurred())

372− Expect(cipher).ToNot(BeNil())

373− opts.Cipher = cipher

374361

375362 ss, err = sessions.NewSessionStore(opts, cookieOpts)

376363 Expect(err).ToNot(HaveOccurred())

@@ -384,9 +371,16 @@ var _ = Describe("NewSessionStore", func() {

384371 ss = nil

385372 opts = &options.SessionOptions{}

386373

374+ // A secret is required to create a Cipher, validation ensures it is the correct

375+ // length before a session store is initialised.

376+ secret := make([]byte, 32)

377+ _, err := rand.Read(secret)

378+ Expect(err).ToNot(HaveOccurred())

379+

387380 // Set default options in CookieOptions

388381 cookieOpts = &options.CookieOptions{

389382 Name: "_oauth2_proxy",

383+ Secret: base64.URLEncoding.EncodeToString(secret),

390384 Path: "/",

391385 Expire: time.Duration(168) * time.Hour,

392386 Refresh: time.Duration(1) * time.Hour,

@@ -423,6 +417,19 @@ var _ = Describe("NewSessionStore", func() {

423417 Context("the cookie.SessionStore", func() {

424418 RunSessionTests(false)

425419 })

420+

421+ Context("with an invalid cookie secret", func() {

422+ BeforeEach(func() {

423+ cookieOpts.Secret = "invalid"

424+ })

425+

426+ It("returns an error", func() {

427+ ss, err := sessions.NewSessionStore(opts, cookieOpts)

428+ Expect(err).To(HaveOccurred())

429+ Expect(err.Error()).To(Equal("error initialising cipher: crypto/aes: invalid key size 7"))

430+ Expect(ss).To(BeNil())

431+ })

432+ })

426433 })

427434

428435 Context("with type 'redis'", func() {

@@ -447,6 +454,19 @@ var _ = Describe("NewSessionStore", func() {

447454 Context("the redis.SessionStore", func() {

448455 RunSessionTests(true)

449456 })

457+

458+ Context("with an invalid cookie secret", func() {

459+ BeforeEach(func() {

460+ cookieOpts.Secret = "invalid"

461+ })

462+

463+ It("returns an error", func() {

464+ ss, err := sessions.NewSessionStore(opts, cookieOpts)

465+ Expect(err).To(HaveOccurred())

466+ Expect(err.Error()).To(Equal("error initialising cipher: crypto/aes: invalid key size 7"))

467+ Expect(ss).To(BeNil())

468+ })

469+ })

450470 })

451471

452472 Context("with an invalid type", func() {

pkg/validation/options.go−17

@@ -21,7 +21,6 @@ import (

2121 "github.com/oauth2-proxy/oauth2-proxy/pkg/ip"

2222 "github.com/oauth2-proxy/oauth2-proxy/pkg/logger"

2323 "github.com/oauth2-proxy/oauth2-proxy/pkg/requests"

24− "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions"

2524 "github.com/oauth2-proxy/oauth2-proxy/providers"

2625)

2726

@@ -37,8 +36,6 @@ func Validate(o *options.Options) error {

3736 }

3837

3938 msgs := make([]string, 0)

40−

41− var cipher encryption.Cipher

4239 if o.Cookie.Secret == "" {

4340 msgs = append(msgs, "missing setting: cookie-secret")

4441 } else {

@@ -60,12 +57,6 @@ func Validate(o *options.Options) error {

6057 msgs = append(msgs,

6158 fmt.Sprintf("Cookie secret must be 16, 24, or 32 bytes to create an AES cipher. Got %d bytes.%s",

6259 len(encryption.SecretBytes(o.Cookie.Secret)), suffix))

63− } else {

64− var err error

65− cipher, err = encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(o.Cookie.Secret))

66− if err != nil {

67− msgs = append(msgs, fmt.Sprintf("cookie-secret error: %v", err))

68− }

6960 }

7061 }

7162

@@ -218,14 +209,6 @@ func Validate(o *options.Options) error {

218209 }

219210 msgs = parseProviderInfo(o, msgs)

220211

221− o.Session.Cipher = cipher

222− sessionStore, err := sessions.NewSessionStore(&o.Session, &o.Cookie)

223− if err != nil {

224− msgs = append(msgs, fmt.Sprintf("error initialising session storage: %v", err))

225− } else {

226− o.SetSessionStore(sessionStore)

227− }

228−

229212 if o.Cookie.Refresh >= o.Cookie.Expire {

230213 msgs = append(msgs, fmt.Sprintf(

231214 "cookie_refresh (%s) must be less than "+