dc8b162 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

feat(cookie): add feature support for cookie-secret-file (#3104)

* feat: add feature support for cookie-secret-file --------- Signed-off-by: Jan Larwig <jan@larwig.com> Co-Authored-By: Sandy Chen <Yuxuan.Chen@morganstanley.com> Co-authored-by: Jan Larwig <jan@larwig.com>

commitdc8b1623a26a2537a8d0119e087f2048234c9843 authorSandy Chen <sandy19890604@gmail.com> dateWed 23 Jul 2025, 01:59 (1 year ago) parent137e59d

10 files changed+226−36

compare with master

unifiedsplit

CHANGELOG.md+2−1 docs/docs/configuration/overview.md+1 pkg/apis/options/cookie.go+34−12 pkg/apis/options/cookie_test.go+70 pkg/cookies/csrf.go+24−8 pkg/sessions/cookie/session_store.go+18−8 pkg/sessions/persistence/ticket.go+5−1 pkg/validation/cookie.go+23−4 pkg/validation/cookie_test.go+48−1 pkg/validation/options_test.go+1−1

CHANGELOG.md+2−1

@@ -12,7 +12,8 @@

1212- [#2605](https://github.com/oauth2-proxy/oauth2-proxy/pull/2605) fix: show login page on broken cookie (@Primexz)

1313- [#2743](https://github.com/oauth2-proxy/oauth2-proxy/pull/2743) feat: allow use more possible google admin-sdk api scopes (@BobDu)

1414- [#2359](https://github.com/oauth2-proxy/oauth2-proxy/pull/2359) feat: add SourceHut (sr.ht) provider(@bitfehler)

15−-[#2524](https://github.com/oauth2-proxy/oauth2-proxy/pull/2524) fix: regex substitution for $ signs in upstream path handling before running envsubst (@dashkan / @tuunit)

15+- [#2524](https://github.com/oauth2-proxy/oauth2-proxy/pull/2524) fix: regex substitution for $ signs in upstream path handling before running envsubst (@dashkan / @tuunit)

16+- [#3104](https://github.com/oauth2-proxy/oauth2-proxy/pull/3104) feat(cookie): add feature support for cookie-secret-file (@sandy2008)

1617

1718# V7.10.0

1819

docs/docs/configuration/overview.md+1

@@ -128,6 +128,7 @@ Provider specific options can be found on their respective subpages.

132133

133134[^1]: The following providers support `--cookie-refresh`: ADFS, Azure, GitLab, Google, Keycloak and all other Identity Providers which support the full [OIDC specification](https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokens)

pkg/apis/options/cookie.go+34−12

@@ -1,8 +1,11 @@

11package options

33import (

4+ "errors"

5+ "os"

46 "time"

8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"

69 "github.com/spf13/pflag"

710)

811

@@ -10,6 +13,7 @@ import (

1013type Cookie struct {

1114 Name string `flag:"cookie-name" cfg:"cookie_name"`

1215 Secret string `flag:"cookie-secret" cfg:"cookie_secret"`

16+ SecretFile string `flag:"cookie-secret-file" cfg:"cookie_secret_file"`

1317 Domains []string `flag:"cookie-domain" cfg:"cookie_domains"`

1418 Path string `flag:"cookie-path" cfg:"cookie_path"`

1519 Expire time.Duration `flag:"cookie-expire" cfg:"cookie_expire"`

@@ -18,8 +22,8 @@ type Cookie struct {

1822 HTTPOnly bool `flag:"cookie-httponly" cfg:"cookie_httponly"`

1923 SameSite string `flag:"cookie-samesite" cfg:"cookie_samesite"`

2024 CSRFPerRequest bool `flag:"cookie-csrf-per-request" cfg:"cookie_csrf_per_request"`

21− CSRFExpire time.Duration `flag:"cookie-csrf-expire" cfg:"cookie_csrf_expire"`

2225 CSRFPerRequestLimit int `flag:"cookie-csrf-per-request-limit" cfg:"cookie_csrf_per_request_limit"`

26+ CSRFExpire time.Duration `flag:"cookie-csrf-expire" cfg:"cookie_csrf_expire"`

2327}

2428

2529func cookieFlagSet() *pflag.FlagSet {

@@ -27,6 +31,7 @@ func cookieFlagSet() *pflag.FlagSet {

2731

2832 flagSet.String("cookie-name", "_oauth2_proxy", "the name of the cookie that the oauth_proxy creates")

2933 flagSet.String("cookie-secret", "", "the seed string for secure cookies (optionally base64 encoded)")

34+ flagSet.String("cookie-secret-file", "", "For defining a separate cookie secret file to read the encryption key from")

3035 flagSet.StringSlice("cookie-domain", []string{}, "Optional cookie domains to force cookies to (ie: `.yourcompany.com`). The longest domain matching the request's host will be used (or the shortest cookie domain if there is no match).")

3136 flagSet.String("cookie-path", "/", "an optional cookie path to force cookies to (ie: /poc/)*")

3237 flagSet.Duration("cookie-expire", time.Duration(168)*time.Hour, "expire timeframe for cookie")

@@ -43,16 +48,33 @@ func cookieFlagSet() *pflag.FlagSet {

4348// cookieDefaults creates a Cookie populating each field with its default value

4449func cookieDefaults() Cookie {

4550 return Cookie{

46− Name: "_oauth2_proxy",

47− Secret: "",

48− Domains: nil,

49− Path: "/",

50− Expire: time.Duration(168) * time.Hour,

51− Refresh: time.Duration(0),

52− Secure: true,

53− HTTPOnly: true,

54− SameSite: "",

55− CSRFPerRequest: false,

56− CSRFExpire: time.Duration(15) * time.Minute,

51+ Name: "_oauth2_proxy",

52+ Secret: "",

53+ SecretFile: "",

54+ Domains: nil,

55+ Path: "/",

56+ Expire: time.Duration(168) * time.Hour,

57+ Refresh: time.Duration(0),

58+ Secure: true,

59+ HTTPOnly: true,

60+ SameSite: "",

61+ CSRFPerRequest: false,

62+ CSRFPerRequestLimit: 0,

63+ CSRFExpire: time.Duration(15) * time.Minute,

64+ }

65+}

66+

67+// GetSecret returns the cookie secret, reading from file if SecretFile is set

68+func (c *Cookie) GetSecret() (secret string, err error) {

69+ if c.Secret != "" || c.SecretFile == "" {

70+ return c.Secret, nil

71+ }

72+

73+ fileSecret, err := os.ReadFile(c.SecretFile)

74+ if err != nil {

75+ logger.Errorf("error reading cookie secret file %s: %s", c.SecretFile, err)

76+ return "", errors.New("could not read cookie secret file")

5777 }

78+

79+ return string(fileSecret), nil

5880}

pkg/apis/options/cookie_test.gonew file+70

@@ -0,0 +1,70 @@

1+package options

3+import (

4+ "os"

5+ "testing"

7+ "github.com/stretchr/testify/assert"

8+)

10+func TestCookieGetSecret(t *testing.T) {

11+ t.Run("returns secret when Secret is set", func(t *testing.T) {

12+ c := &Cookie{

13+ Secret: "my-secret",

14+ SecretFile: "",

15+ }

16+ secret, err := c.GetSecret()

17+ assert.NoError(t, err)

18+ assert.Equal(t, "my-secret", secret)

19+ })

20+

21+ t.Run("returns secret when both Secret and SecretFile are set", func(t *testing.T) {

22+ c := &Cookie{

23+ Secret: "my-secret",

24+ SecretFile: "/some/file",

25+ }

26+ secret, err := c.GetSecret()

27+ assert.NoError(t, err)

28+ assert.Equal(t, "my-secret", secret)

29+ })

30+

31+ t.Run("reads from file when only SecretFile is set", func(t *testing.T) {

32+ // Create a temporary file

33+ tmpfile, err := os.CreateTemp("", "cookie-secret-test")

34+ assert.NoError(t, err)

35+ defer os.Remove(tmpfile.Name())

36+

37+ _, err = tmpfile.Write([]byte("file-secret"))

38+ assert.NoError(t, err)

39+ tmpfile.Close()

40+

41+ c := &Cookie{

42+ Secret: "",

43+ SecretFile: tmpfile.Name(),

44+ }

45+ secret, err := c.GetSecret()

46+ assert.NoError(t, err)

47+ assert.Equal(t, "file-secret", secret)

48+ })

49+

50+ t.Run("returns error when file does not exist", func(t *testing.T) {

51+ c := &Cookie{

52+ Secret: "",

53+ SecretFile: "/nonexistent/file",

54+ }

55+ secret, err := c.GetSecret()

56+ assert.Error(t, err)

57+ assert.Equal(t, "", secret)

58+ assert.Contains(t, err.Error(), "could not read cookie secret file")

59+ })

60+

61+ t.Run("returns empty when both Secret and SecretFile are empty", func(t *testing.T) {

62+ c := &Cookie{

63+ Secret: "",

64+ SecretFile: "",

65+ }

66+ secret, err := c.GetSecret()

67+ assert.NoError(t, err)

68+ assert.Equal(t, "", secret)

69+ })

70+}

pkg/cookies/csrf.go+24−8

@@ -219,13 +219,22 @@ func (c *csrf) encodeCookie() (string, error) {

219219 return "", err

220220 }

221221

222− return encryption.SignedValue(c.cookieOpts.Secret, c.cookieName(), encrypted, c.time.Now())

222+ secret, err := c.cookieOpts.GetSecret()

223+ if err != nil {

224+ return "", fmt.Errorf("error getting cookie secret: %v", err)

225+ }

226+ return encryption.SignedValue(secret, c.cookieName(), encrypted, c.time.Now())

223227}

224228

225229// decodeCSRFCookie validates the signature then decrypts and decodes a CSRF

226230// cookie into a CSRF struct

227231func decodeCSRFCookie(cookie *http.Cookie, opts *options.Cookie) (*csrf, error) {

228− val, t, ok := encryption.Validate(cookie, opts.Secret, opts.Expire)

232+ secret, err := opts.GetSecret()

233+ if err != nil {

234+ return nil, fmt.Errorf("error getting cookie secret: %v", err)

235+ }

236+

237+ val, t, ok := encryption.Validate(cookie, secret, opts.Expire)

229238 if !ok {

230239 return nil, errors.New("CSRF cookie failed validation")

231240 }

@@ -235,15 +244,18 @@ func decodeCSRFCookie(cookie *http.Cookie, opts *options.Cookie) (*csrf, error)

235244 return nil, err

236245 }

237246

238− // Valid cookie, Unmarshal the CSRF

247+ return unmarshalCSRF(decrypted, opts, t)

248+}

249+

250+// unmarshalCSRF unmarshals decrypted data into a CSRF struct

251+func unmarshalCSRF(decrypted []byte, opts *options.Cookie, csrfTime time.Time) (*csrf, error) {

239252 clock := clock.Clock{}

240− clock.Set(t)

253+ clock.Set(csrfTime)

254+

241255 csrf := &csrf{cookieOpts: opts, time: clock}

242− err = msgpack.Unmarshal(decrypted, csrf)

243− if err != nil {

256+ if err := msgpack.Unmarshal(decrypted, csrf); err != nil {

244257 return nil, fmt.Errorf("error unmarshalling data to CSRF: %v", err)

245258 }

246−

247259 return csrf, nil

248260}

249261

@@ -290,5 +302,9 @@ func decrypt(data []byte, opts *options.Cookie) ([]byte, error) {

290302}

291303

292304func makeCipher(opts *options.Cookie) (encryption.Cipher, error) {

293− return encryption.NewCFBCipher(encryption.SecretBytes(opts.Secret))

305+ secret, err := opts.GetSecret()

306+ if err != nil {

307+ return nil, fmt.Errorf("error getting cookie secret: %v", err)

308+ }

309+ return encryption.NewCFBCipher(encryption.SecretBytes(secret))

294310}

pkg/sessions/cookie/session_store.go+18−8

@@ -54,16 +54,18 @@ func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) {

5454 // always http.ErrNoCookie

5555 return nil, err

5656 }

57− val, _, ok := encryption.Validate(c, s.Cookie.Secret, s.Cookie.Expire)

57+

58+ secret, err := s.Cookie.GetSecret()

59+ if err != nil {

60+ return nil, fmt.Errorf("error getting cookie secret: %v", err)

61+ }

62+

63+ val, _, ok := encryption.Validate(c, secret, s.Cookie.Expire)

5864 if !ok {

5965 return nil, errors.New("cookie signature not valid")

6066 }

6167

62− session, err := sessions.DecodeSessionState(val, s.CookieCipher, true)

63− if err != nil {

64− return nil, err

65− }

66− return session, nil

68+ return sessions.DecodeSessionState(val, s.CookieCipher, true)

6769}

6870

6971// Clear clears any saved session information by writing a cookie to

@@ -121,7 +123,11 @@ func (s *SessionStore) makeSessionCookie(req *http.Request, value []byte, now ti

121123 strValue := string(value)

122124 if strValue != "" {

123125 var err error

124− strValue, err = encryption.SignedValue(s.Cookie.Secret, s.Cookie.Name, value, now)

126+ secret, err := s.Cookie.GetSecret()

127+ if err != nil {

128+ return nil, fmt.Errorf("error getting cookie secret: %v", err)

129+ }

130+ strValue, err = encryption.SignedValue(secret, s.Cookie.Name, value, now)

125131 if err != nil {

126132 return nil, err

127133 }

@@ -146,7 +152,11 @@ func (s *SessionStore) makeCookie(req *http.Request, name string, value string,

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

147153// the configuration given

148154func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessions.SessionStore, error) {

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

155+ secret, err := cookieOpts.GetSecret()

156+ if err != nil {

157+ return nil, fmt.Errorf("error getting cookie secret: %v", err)

158+ }

159+ cipher, err := encryption.NewCFBCipher(encryption.SecretBytes(secret))

150160 if err != nil {

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

152162 }

pkg/sessions/persistence/ticket.go+5−1

@@ -146,7 +146,11 @@ func decodeTicketFromRequest(req *http.Request, cookieOpts *options.Cookie) (*ti

146146 }

147147

148148 // An existing cookie exists, try to retrieve the ticket

149− val, _, ok := encryption.Validate(requestCookie, cookieOpts.Secret, cookieOpts.Expire)

149+ secret, err := cookieOpts.GetSecret()

150+ if err != nil {

151+ return nil, fmt.Errorf("error getting cookie secret: %v", err)

152+ }

153+ val, _, ok := encryption.Validate(requestCookie, secret, cookieOpts.Expire)

150154 if !ok {

151155 return nil, fmt.Errorf("session ticket cookie failed validation: %v", err)

152156 }

pkg/validation/cookie.go+23−4

@@ -3,6 +3,7 @@ package validation

33import (

44 "fmt"

55 "net/http"

6+ "os"

67 "sort"

78 "time"

@@ -11,7 +12,7 @@ import (

1112)

1213

1314func validateCookie(o options.Cookie) []string {

14− msgs := validateCookieSecret(o.Secret)

15+ msgs := validateCookieSecret(o.Secret, o.SecretFile)

1516

1617 if o.Expire != time.Duration(0) && o.Refresh >= o.Expire {

1718 msgs = append(msgs, fmt.Sprintf(

@@ -49,9 +50,27 @@ func validateCookieName(name string) []string {

4950 return msgs

5051}

5152

52−func validateCookieSecret(secret string) []string {

53− if secret == "" {

54− return []string{"missing setting: cookie-secret"}

53+func validateCookieSecret(secret string, secretFile string) []string {

54+ if secret == "" && secretFile == "" {

55+ return []string{"missing setting: cookie-secret or cookie-secret-file"}

56+ }

57+ if secret == "" && secretFile != "" {

58+ fileData, err := os.ReadFile(secretFile)

59+ if err != nil {

60+ return []string{"could not read cookie secret file: " + secretFile}

61+ }

62+ // Validate the file content as a secret

63+ secretBytes := encryption.SecretBytes(string(fileData))

64+ switch len(secretBytes) {

65+ case 16, 24, 32:

66+ // Valid secret size found

67+ return []string{}

68+ }

69+ // Invalid secret size found, return a message

70+ return []string{fmt.Sprintf(

71+ "cookie_secret from file must be 16, 24, or 32 bytes to create an AES cipher, but is %d bytes",

72+ len(secretBytes)),

73+ }

5574 }

5675

5776 secretBytes := encryption.SecretBytes(secret)

pkg/validation/cookie_test.go+48−1

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

11package validation

33import (

4+ "os"

45 "strings"

56 "testing"

67 "time"

@@ -29,9 +30,23 @@ func TestValidateCookie(t *testing.T) {

2930 "a.cba.localhost",

3031 }

3132

33+ // Create a temporary file for the valid secret file test

34+ tmpfile, err := os.CreateTemp("", "cookie-secret-test")

35+ if err != nil {

36+ t.Fatalf("Failed to create temporary file: %v", err)

37+ }

38+ defer os.Remove(tmpfile.Name())

39+

40+ // Write a valid 32-byte secret to the file

41+ _, err = tmpfile.Write([]byte(validSecret))

42+ if err != nil {

43+ t.Fatalf("Failed to write to temporary file: %v", err)

44+ }

45+ tmpfile.Close()

46+

3247 invalidNameMsg := "invalid cookie name: \"_oauth2;proxy\""

3348 longNameMsg := "cookie name should be under 256 characters: cookie name is 260 characters"

34− missingSecretMsg := "missing setting: cookie-secret"

49+ missingSecretMsg := "missing setting: cookie-secret or cookie-secret-file"

3550 invalidSecretMsg := "cookie_secret must be 16, 24, or 32 bytes to create an AES cipher, but is 6 bytes"

3651 invalidBase64SecretMsg := "cookie_secret must be 16, 24, or 32 bytes to create an AES cipher, but is 10 bytes"

3752 refreshLongerThanExpireMsg := "cookie_refresh (\"1h0m0s\") must be less than cookie_expire (\"15m0s\")"

@@ -271,6 +286,38 @@ func TestValidateCookie(t *testing.T) {

271286 },

272287 errStrings: []string{},

273288 },

289+ {

290+ name: "with valid secret file",

291+ cookie: options.Cookie{

292+ Name: validName,

293+ Secret: "",

294+ SecretFile: tmpfile.Name(),

295+ Domains: domains,

296+ Path: "",

297+ Expire: 24 * time.Hour,

298+ Refresh: 0,

299+ Secure: true,

300+ HTTPOnly: true,

301+ SameSite: "",

302+ },

303+ errStrings: []string{},

304+ },

305+ {

306+ name: "with nonexistent secret file",

307+ cookie: options.Cookie{

308+ Name: validName,

309+ Secret: "",

310+ SecretFile: "/nonexistent/file.txt",

311+ Domains: domains,

312+ Path: "",

313+ Expire: 24 * time.Hour,

314+ Refresh: 0,

315+ Secure: true,

316+ HTTPOnly: true,

317+ SameSite: "",

318+ },

319+ errStrings: []string{"could not read cookie secret file: /nonexistent/file.txt"},

320+ },

274321 }

275322

276323 for _, tc := range testCases {

pkg/validation/options_test.go+1−1

@@ -48,7 +48,7 @@ func TestNewOptions(t *testing.T) {

4848 assert.NotEqual(t, nil, err)

4949

5050 expected := errorMsg([]string{

51− "missing setting: cookie-secret",

51+ "missing setting: cookie-secret or cookie-secret-file",

5252 "provider has empty id: ids are required for all providers",

5353 "provider missing setting: client-id",

5454 "missing setting: client-secret or client-secret-file"})