git.iko.soy/typeable/oauth2-proxy
on master help sign in
Improvements to Session State code (#536)

* Drop SessionStateJSON wrapper * Use EncrpytInto/DecryptInto to reduce sessionstate Co-authored-by: Henry Jenkins <henry@henryjenkins.name>

commitf7b28cb1d362f54d13379136c37f0edc1303a1b0 authorJoel Speed <Joel.speed@hotmail.co.uk> dateSat 30 May 2020, 08:53 (6 years ago) parent6a88da7
17 files changed+185−164
compare with master
unifiedsplit
CHANGELOG.md+1 oauthproxy_test.go+23−15 pkg/apis/sessions/session_state.go+34−98 pkg/apis/sessions/session_state_test.go+23−16 pkg/encryption/cipher.go+27 pkg/encryption/cipher_test.go+22 pkg/sessions/cookie/session_store.go+4−3 pkg/sessions/redis/redis_store.go+4−3 pkg/sessions/session_store_test.go+11−7 providers/azure.go+4−2 providers/gitlab.go+4−3 providers/google.go+8−4 providers/logingov.go+5−2 providers/oidc.go+5−4 providers/oidc_test.go+4−4 providers/provider_default.go+3−2 providers/provider_default_test.go+3−1
CHANGELOG.md+1
@@ -51,6 +51,7 @@
5151
5252## Changes since v5.1.1
5353
54+- [#536](https://github.com/oauth2-proxy/oauth2-proxy/pull/536) Improvements to Session State code (@JoelSpeed)
5455- [#573](https://github.com/oauth2-proxy/oauth2-proxy/pull/573) Properly parse redis urls for cluster and sentinel connections (@amnay-mo)
5556- [#574](https://github.com/oauth2-proxy/oauth2-proxy/pull/574) render error page on 502 proxy status (@amnay-mo)
5657- [#559](https://github.com/oauth2-proxy/oauth2-proxy/pull/559) Rename cookie-domain config to cookie-domains (@JoelSpeed)
oauthproxy_test.go+23−15
@@ -484,8 +484,7 @@ func TestBasicAuthPassword(t *testing.T) {
484484 })
485485
486486 rw := httptest.NewRecorder()
487− req, _ := http.NewRequest("GET", "/oauth2/callback?code=callback_code&state=nonce:",
488− strings.NewReader(""))
487+ req, _ := http.NewRequest("GET", "/oauth2/callback?code=callback_code&state=nonce:", strings.NewReader(""))
489488 req.AddCookie(proxy.MakeCSRFCookie(req, "nonce", proxy.CookieExpire, time.Now()))
490489 proxy.ServeHTTP(rw, req)
491490 if rw.Code >= 400 {
@@ -541,11 +540,12 @@ func TestBasicAuthWithEmail(t *testing.T) {
541540 expectedEmailHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(emailAddress+":"+opts.BasicAuthPassword))
542541 expectedUserHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(userName+":"+opts.BasicAuthPassword))
543542
543+ created := time.Now()
544544 session := &sessions.SessionState{
545545 User: userName,
546546 Email: emailAddress,
547547 AccessToken: "oauth_token",
548− CreatedAt: time.Now(),
548+ CreatedAt: &created,
549549 }
550550 {
551551 rw := httptest.NewRecorder()
@@ -582,11 +582,12 @@ func TestPassUserHeadersWithEmail(t *testing.T) {
582582 const emailAddress = "john.doe@example.com"
583583 const userName = "9fcab5c9b889a557"
584584
585+ created := time.Now()
585586 session := &sessions.SessionState{
586587 User: userName,
587588 Email: emailAddress,
588589 AccessToken: "oauth_token",
589− CreatedAt: time.Now(),
590+ CreatedAt: &created,
590591 }
591592 {
592593 rw := httptest.NewRecorder()
@@ -959,7 +960,8 @@ func (p *ProcessCookieTest) LoadCookiedSession() (*sessions.SessionState, error)
959960func TestLoadCookiedSession(t *testing.T) {
960961 pcTest := NewProcessCookieTestWithDefaults()
961962
962− startSession := &sessions.SessionState{Email: "john.doe@example.com", AccessToken: "my_access_token", CreatedAt: time.Now()}
963+ created := time.Now()
964+ startSession := &sessions.SessionState{Email: "john.doe@example.com", AccessToken: "my_access_token", CreatedAt: &created}
963965 pcTest.SaveSession(startSession)
964966
965967 session, err := pcTest.LoadCookiedSession()
@@ -985,7 +987,7 @@ func TestProcessCookieRefreshNotSet(t *testing.T) {
985987 })
986988 reference := time.Now().Add(time.Duration(-2) * time.Hour)
987989
988− startSession := &sessions.SessionState{Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: reference}
990+ startSession := &sessions.SessionState{Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: &reference}
989991 pcTest.SaveSession(startSession)
990992
991993 session, err := pcTest.LoadCookiedSession()
@@ -1001,7 +1003,7 @@ func TestProcessCookieFailIfCookieExpired(t *testing.T) {
10011003 opts.Cookie.Expire = time.Duration(24) * time.Hour
10021004 })
10031005 reference := time.Now().Add(time.Duration(25) * time.Hour * -1)
1004− startSession := &sessions.SessionState{Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: reference}
1006+ startSession := &sessions.SessionState{Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: &reference}
10051007 pcTest.SaveSession(startSession)
10061008
10071009 session, err := pcTest.LoadCookiedSession()
@@ -1016,7 +1018,7 @@ func TestProcessCookieFailIfRefreshSetAndCookieExpired(t *testing.T) {
10161018 opts.Cookie.Expire = time.Duration(24) * time.Hour
10171019 })
10181020 reference := time.Now().Add(time.Duration(25) * time.Hour * -1)
1019− startSession := &sessions.SessionState{Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: reference}
1021+ startSession := &sessions.SessionState{Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: &reference}
10201022 pcTest.SaveSession(startSession)
10211023
10221024 pcTest.proxy.CookieRefresh = time.Hour
@@ -1062,8 +1064,9 @@ func NewAuthOnlyEndpointTest(modifiers ...OptionsModifier) *ProcessCookieTest {
10621064
10631065func TestAuthOnlyEndpointAccepted(t *testing.T) {
10641066 test := NewAuthOnlyEndpointTest()
1067+ created := time.Now()
10651068 startSession := &sessions.SessionState{
1066− Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: time.Now()}
1069+ Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: &created}
10671070 test.SaveSession(startSession)
10681071
10691072 test.proxy.ServeHTTP(test.rw, test.req)
@@ -1087,7 +1090,7 @@ func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {
10871090 })
10881091 reference := time.Now().Add(time.Duration(25) * time.Hour * -1)
10891092 startSession := &sessions.SessionState{
1090− Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: reference}
1093+ Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: &reference}
10911094 test.SaveSession(startSession)
10921095
10931096 test.proxy.ServeHTTP(test.rw, test.req)
@@ -1098,8 +1101,9 @@ func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {
10981101
10991102func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) {
11001103 test := NewAuthOnlyEndpointTest()
1104+ created := time.Now()
11011105 startSession := &sessions.SessionState{
1102− Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: time.Now()}
1106+ Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: &created}
11031107 test.SaveSession(startSession)
11041108 test.validateUser = false
11051109
@@ -1129,8 +1133,9 @@ func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) {
11291133 pcTest.req, _ = http.NewRequest("GET",
11301134 pcTest.opts.ProxyPrefix+"/auth", nil)
11311135
1136+ created := time.Now()
11321137 startSession := &sessions.SessionState{
1133− User: "oauth_user", Email: "oauth_user@example.com", AccessToken: "oauth_token", CreatedAt: time.Now()}
1138+ User: "oauth_user", Email: "oauth_user@example.com", AccessToken: "oauth_token", CreatedAt: &created}
11341139 pcTest.SaveSession(startSession)
11351140
11361141 pcTest.proxy.ServeHTTP(pcTest.rw, pcTest.req)
@@ -1160,8 +1165,9 @@ func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) {
11601165 pcTest.req, _ = http.NewRequest("GET",
11611166 pcTest.opts.ProxyPrefix+"/auth", nil)
11621167
1168+ created := time.Now()
11631169 startSession := &sessions.SessionState{
1164− User: "oauth_user", Email: "oauth_user@example.com", AccessToken: "oauth_token", CreatedAt: time.Now()}
1170+ User: "oauth_user", Email: "oauth_user@example.com", AccessToken: "oauth_token", CreatedAt: &created}
11651171 pcTest.SaveSession(startSession)
11661172
11671173 pcTest.proxy.ServeHTTP(pcTest.rw, pcTest.req)
@@ -1193,8 +1199,9 @@ func TestAuthOnlyEndpointSetBasicAuthFalseRequestHeaders(t *testing.T) {
11931199 pcTest.req, _ = http.NewRequest("GET",
11941200 pcTest.opts.ProxyPrefix+"/auth", nil)
11951201
1202+ created := time.Now()
11961203 startSession := &sessions.SessionState{
1197− User: "oauth_user", Email: "oauth_user@example.com", AccessToken: "oauth_token", CreatedAt: time.Now()}
1204+ User: "oauth_user", Email: "oauth_user@example.com", AccessToken: "oauth_token", CreatedAt: &created}
11981205 pcTest.SaveSession(startSession)
11991206
12001207 pcTest.proxy.ServeHTTP(pcTest.rw, pcTest.req)
@@ -1569,10 +1576,11 @@ func TestGetJwtSession(t *testing.T) {
15691576 }
15701577
15711578 // Bearer
1579+ expires := time.Unix(1912151821, 0)
15721580 session, _ := test.proxy.GetJwtSession(test.req)
15731581 assert.Equal(t, session.User, "john@example.com")
15741582 assert.Equal(t, session.Email, "john@example.com")
1575− assert.Equal(t, session.ExpiresOn, time.Unix(1912151821, 0))
1583+ assert.Equal(t, session.ExpiresOn, &expires)
15761584 assert.Equal(t, session.IDToken, goodJwt)
15771585
15781586 test.proxy.ServeHTTP(test.rw, test.req)
pkg/apis/sessions/session_state.go+34−98
@@ -2,7 +2,6 @@ package sessions
22
33import (
44 "encoding/json"
5− "errors"
65 "fmt"
76 "time"
87
@@ -11,26 +10,19 @@ import (
1110
1211// SessionState is used to store information about the currently authenticated user session
1312type SessionState struct {
14− AccessToken string `json:",omitempty"`
15− IDToken string `json:",omitempty"`
16− CreatedAt time.Time `json:"-"`
17− ExpiresOn time.Time `json:"-"`
18− RefreshToken string `json:",omitempty"`
19− Email string `json:",omitempty"`
20− User string `json:",omitempty"`
21− PreferredUsername string `json:",omitempty"`
22−}
23−
24−// SessionStateJSON is used to encode SessionState into JSON without exposing time.Time zero value
25−type SessionStateJSON struct {
26− *SessionState
27− CreatedAt *time.Time `json:",omitempty"`
28− ExpiresOn *time.Time `json:",omitempty"`
13+ AccessToken string `json:",omitempty"`
14+ IDToken string `json:",omitempty"`
15+ CreatedAt *time.Time `json:",omitempty"`
16+ ExpiresOn *time.Time `json:",omitempty"`
17+ RefreshToken string `json:",omitempty"`
18+ Email string `json:",omitempty"`
19+ User string `json:",omitempty"`
20+ PreferredUsername string `json:",omitempty"`
2921}
3022
3123// IsExpired checks whether the session has expired
3224func (s *SessionState) IsExpired() bool {
33− if !s.ExpiresOn.IsZero() && s.ExpiresOn.Before(time.Now()) {
25+ if s.ExpiresOn != nil && !s.ExpiresOn.IsZero() && s.ExpiresOn.Before(time.Now()) {
3426 return true
3527 }
3628 return false
@@ -38,8 +30,8 @@ func (s *SessionState) IsExpired() bool {
3830
3931// Age returns the age of a session
4032func (s *SessionState) Age() time.Duration {
41− if !s.CreatedAt.IsZero() {
42− return time.Now().Truncate(time.Second).Sub(s.CreatedAt)
33+ if s.CreatedAt != nil && !s.CreatedAt.IsZero() {
34+ return time.Now().Truncate(time.Second).Sub(*s.CreatedAt)
4335 }
4436 return 0
4537}
@@ -75,80 +67,36 @@ func (s *SessionState) EncodeSessionState(c *encryption.Cipher) (string, error)
7567 ss.PreferredUsername = s.PreferredUsername
7668 } else {
7769 ss = *s
78− var err error
79− if ss.Email != "" {
80− ss.Email, err = c.Encrypt(ss.Email)
81− if err != nil {
82− return "", err
83− }
84− }
85− if ss.User != "" {
86− ss.User, err = c.Encrypt(ss.User)
87− if err != nil {
88− return "", err
89− }
90− }
91− if ss.PreferredUsername != "" {
92− ss.PreferredUsername, err = c.Encrypt(ss.PreferredUsername)
93− if err != nil {
94− return "", err
95− }
96− }
97− if ss.AccessToken != "" {
98− ss.AccessToken, err = c.Encrypt(ss.AccessToken)
99− if err != nil {
100− return "", err
101− }
102− }
103− if ss.IDToken != "" {
104− ss.IDToken, err = c.Encrypt(ss.IDToken)
105− if err != nil {
106− return "", err
107− }
108− }
109− if ss.RefreshToken != "" {
110− ss.RefreshToken, err = c.Encrypt(ss.RefreshToken)
70+ for _, s := range []*string{
71+ &ss.Email,
72+ &ss.User,
73+ &ss.PreferredUsername,
74+ &ss.AccessToken,
75+ &ss.IDToken,
76+ &ss.RefreshToken,
77+ } {
78+ err := c.EncryptInto(s)
11179 if err != nil {
11280 return "", err
11381 }
11482 }
11583 }
116− // Embed SessionState and ExpiresOn pointer into SessionStateJSON
117− ssj := &SessionStateJSON{SessionState: &ss}
118− if !ss.CreatedAt.IsZero() {
119− ssj.CreatedAt = &ss.CreatedAt
120− }
121− if !ss.ExpiresOn.IsZero() {
122− ssj.ExpiresOn = &ss.ExpiresOn
123− }
124− b, err := json.Marshal(ssj)
84+
85+ b, err := json.Marshal(ss)
12586 return string(b), err
12687}
12788
12889// DecodeSessionState decodes the session cookie string into a SessionState
12990func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {
130− var ssj SessionStateJSON
131− var ss *SessionState
132− err := json.Unmarshal([]byte(v), &ssj)
91+ var ss SessionState
92+ err := json.Unmarshal([]byte(v), &ss)
13393 if err != nil {
13494 return nil, fmt.Errorf("error unmarshalling session: %w", err)
13595 }
136− if ssj.SessionState == nil {
137− return nil, errors.New("expected session state to not be nil")
138− }
139−
140− // Extract SessionState and CreatedAt,ExpiresOn value from SessionStateJSON
141− ss = ssj.SessionState
142− if ssj.CreatedAt != nil {
143− ss.CreatedAt = *ssj.CreatedAt
144− }
145− if ssj.ExpiresOn != nil {
146− ss.ExpiresOn = *ssj.ExpiresOn
147− }
14896
14997 if c == nil {
15098 // Load only Email and User when cipher is unavailable
151− ss = &SessionState{
99+ ss = SessionState{
152100 Email: ss.Email,
153101 User: ss.User,
154102 PreferredUsername: ss.PreferredUsername,
@@ -168,30 +116,18 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {
168116 ss.User = decryptedUser
169117 }
170118 }
171− if ss.PreferredUsername != "" {
172− ss.PreferredUsername, err = c.Decrypt(ss.PreferredUsername)
173− if err != nil {
174− return nil, err
175− }
176− }
177− if ss.AccessToken != "" {
178− ss.AccessToken, err = c.Decrypt(ss.AccessToken)
179− if err != nil {
180− return nil, err
181− }
182− }
183− if ss.IDToken != "" {
184− ss.IDToken, err = c.Decrypt(ss.IDToken)
185− if err != nil {
186− return nil, err
187− }
188− }
189− if ss.RefreshToken != "" {
190− ss.RefreshToken, err = c.Decrypt(ss.RefreshToken)
119+
120+ for _, s := range []*string{
121+ &ss.PreferredUsername,
122+ &ss.AccessToken,
123+ &ss.IDToken,
124+ &ss.RefreshToken,
125+ } {
126+ err := c.DecryptInto(s)
191127 if err != nil {
192128 return nil, err
193129 }
194130 }
195131 }
196− return ss, nil
132+ return &ss, nil
197133}
pkg/apis/sessions/session_state_test.go+23−16
@@ -13,6 +13,10 @@ import (
1313const secret = "0123456789abcdefghijklmnopqrstuv"
1414const altSecret = "0000000000abcdefghijklmnopqrstuv"
1515
16+func timePtr(t time.Time) *time.Time {
17+ return &t
18+}
19+
1620func TestSessionStateSerialization(t *testing.T) {
1721 c, err := encryption.NewCipher([]byte(secret))
1822 assert.Equal(t, nil, err)
@@ -23,8 +27,8 @@ func TestSessionStateSerialization(t *testing.T) {
2327 PreferredUsername: "user",
2428 AccessToken: "token1234",
2529 IDToken: "rawtoken1234",
26− CreatedAt: time.Now(),
27− ExpiresOn: time.Now().Add(time.Duration(1) * time.Hour),
30+ CreatedAt: timePtr(time.Now()),
31+ ExpiresOn: timePtr(time.Now().Add(time.Duration(1) * time.Hour)),
2832 RefreshToken: "refresh4321",
2933 }
3034 encoded, err := s.EncodeSessionState(c)
@@ -66,8 +70,8 @@ func TestSessionStateSerializationWithUser(t *testing.T) {
6670 PreferredUsername: "ju",
6771 Email: "user@domain.com",
6872 AccessToken: "token1234",
69− CreatedAt: time.Now(),
70− ExpiresOn: time.Now().Add(time.Duration(1) * time.Hour),
73+ CreatedAt: timePtr(time.Now()),
74+ ExpiresOn: timePtr(time.Now().Add(time.Duration(1) * time.Hour)),
7175 RefreshToken: "refresh4321",
7276 }
7377 encoded, err := s.EncodeSessionState(c)
@@ -102,8 +106,8 @@ func TestSessionStateSerializationNoCipher(t *testing.T) {
102106 Email: "user@domain.com",
103107 PreferredUsername: "user",
104108 AccessToken: "token1234",
105− CreatedAt: time.Now(),
106− ExpiresOn: time.Now().Add(time.Duration(1) * time.Hour),
109+ CreatedAt: timePtr(time.Now()),
110+ ExpiresOn: timePtr(time.Now().Add(time.Duration(1) * time.Hour)),
107111 RefreshToken: "refresh4321",
108112 }
109113 encoded, err := s.EncodeSessionState(nil)
@@ -125,8 +129,8 @@ func TestSessionStateSerializationNoCipherWithUser(t *testing.T) {
125129 Email: "user@domain.com",
126130 PreferredUsername: "user",
127131 AccessToken: "token1234",
128− CreatedAt: time.Now(),
129− ExpiresOn: time.Now().Add(time.Duration(1) * time.Hour),
132+ CreatedAt: timePtr(time.Now()),
133+ ExpiresOn: timePtr(time.Now().Add(time.Duration(1) * time.Hour)),
130134 RefreshToken: "refresh4321",
131135 }
132136 encoded, err := s.EncodeSessionState(nil)
@@ -143,10 +147,10 @@ func TestSessionStateSerializationNoCipherWithUser(t *testing.T) {
143147}
144148
145149func TestExpired(t *testing.T) {
146− s := &sessions.SessionState{ExpiresOn: time.Now().Add(time.Duration(-1) * time.Minute)}
150+ s := &sessions.SessionState{ExpiresOn: timePtr(time.Now().Add(time.Duration(-1) * time.Minute))}
147151 assert.Equal(t, true, s.IsExpired())
148152
149− s = &sessions.SessionState{ExpiresOn: time.Now().Add(time.Duration(1) * time.Minute)}
153+ s = &sessions.SessionState{ExpiresOn: timePtr(time.Now().Add(time.Duration(1) * time.Minute))}
150154 assert.Equal(t, false, s.IsExpired())
151155
152156 s = &sessions.SessionState{}
@@ -182,8 +186,8 @@ func TestEncodeSessionState(t *testing.T) {
182186 User: "just-user",
183187 AccessToken: "token1234",
184188 IDToken: "rawtoken1234",
185− CreatedAt: c,
186− ExpiresOn: e,
189+ CreatedAt: &c,
190+ ExpiresOn: &e,
187191 RefreshToken: "refresh4321",
188192 },
189193 Encoded: `{"Email":"user@domain.com","User":"just-user"}`,
@@ -249,8 +253,8 @@ func TestDecodeSessionState(t *testing.T) {
249253 User: "just-user",
250254 AccessToken: "token1234",
251255 IDToken: "rawtoken1234",
252− CreatedAt: created,
253− ExpiresOn: e,
256+ CreatedAt: &created,
257+ ExpiresOn: &e,
254258 RefreshToken: "refresh4321",
255259 },
256260 Encoded: fmt.Sprintf(`{"Email":"FsKKYrTWZWrxSOAqA/fTNAUZS5QWCqOBjuAbBlbVOw==","User":"rT6JP3dxQhxUhkWrrd7yt6c1mDVyQCVVxw==","AccessToken":"I6s+ml+/MldBMgHIiC35BTKTh57skGX24w==","IDToken":"xojNdyyjB1HgYWh6XMtXY/Ph5eCVxa1cNsklJw==","RefreshToken":"qEX0x6RmASxo4dhlBG6YuRs9Syn/e9sHu/+K","CreatedAt":%s,"ExpiresOn":%s}`, createdString, eString),
@@ -291,7 +295,10 @@ func TestDecodeSessionState(t *testing.T) {
291295 assert.Equal(t, tc.AccessToken, ss.AccessToken)
292296 assert.Equal(t, tc.RefreshToken, ss.RefreshToken)
293297 assert.Equal(t, tc.IDToken, ss.IDToken)
294− assert.Equal(t, tc.ExpiresOn.Unix(), ss.ExpiresOn.Unix())
298+ if tc.ExpiresOn != nil {
299+ assert.NotEqual(t, nil, ss.ExpiresOn)
300+ assert.Equal(t, tc.ExpiresOn.Unix(), ss.ExpiresOn.Unix())
301+ }
295302 }
296303 }
297304}
@@ -303,6 +310,6 @@ func TestSessionStateAge(t *testing.T) {
303310 assert.Equal(t, time.Duration(0), ss.Age())
304311
305312 // Set CreatedAt to 1 hour ago
306− ss.CreatedAt = time.Now().Add(-1 * time.Hour)
313+ ss.CreatedAt = timePtr(time.Now().Add(-1 * time.Hour))
307314 assert.Equal(t, time.Hour, ss.Age().Round(time.Minute))
308315}
pkg/encryption/cipher.go+27
@@ -156,3 +156,30 @@ func (c *Cipher) Decrypt(s string) (string, error) {
156156
157157 return string(encrypted), nil
158158}
159+
160+// EncryptInto encrypts the value and stores it back in the string pointer
161+func (c *Cipher) EncryptInto(s *string) error {
162+ return into(c.Encrypt, s)
163+}
164+
165+// DecryptInto decrypts the value and stores it back in the string pointer
166+func (c *Cipher) DecryptInto(s *string) error {
167+ return into(c.Decrypt, s)
168+}
169+
170+// codecFunc is a function that takes a string and encodes/decodes it
171+type codecFunc func(string) (string, error)
172+
173+func into(f codecFunc, s *string) error {
174+ // Do not encrypt/decrypt nil or empty strings
175+ if s == nil || *s == "" {
176+ return nil
177+ }
178+
179+ d, err := f(*s)
180+ if err != nil {
181+ return err
182+ }
183+ *s = d
184+ return nil
185+}
pkg/encryption/cipher_test.go+22
@@ -133,3 +133,25 @@ func TestEncodeAndDecodeAccessTokenB64(t *testing.T) {
133133 assert.NotEqual(t, token, encoded)
134134 assert.Equal(t, token, decoded)
135135}
136+
137+func TestEncodeIntoAndDecodeIntoAccessToken(t *testing.T) {
138+ const secret = "0123456789abcdefghijklmnopqrstuv"
139+ c, err := NewCipher([]byte(secret))
140+ assert.Equal(t, nil, err)
141+
142+ token := "my access token"
143+ originalToken := token
144+
145+ assert.Equal(t, nil, c.EncryptInto(&token))
146+ assert.NotEqual(t, originalToken, token)
147+
148+ assert.Equal(t, nil, c.DecryptInto(&token))
149+ assert.Equal(t, originalToken, token)
150+
151+ // Check no errors with empty or nil strings
152+ empty := ""
153+ assert.Equal(t, nil, c.EncryptInto(&empty))
154+ assert.Equal(t, nil, c.DecryptInto(&empty))
155+ assert.Equal(t, nil, c.EncryptInto(nil))
156+ assert.Equal(t, nil, c.DecryptInto(nil))
157+}
pkg/sessions/cookie/session_store.go+4−3
@@ -34,14 +34,15 @@ type SessionStore struct {
3434// Save takes a sessions.SessionState and stores the information from it
3535// within Cookies set on the HTTP response writer
3636func (s *SessionStore) Save(rw http.ResponseWriter, req *http.Request, ss *sessions.SessionState) error {
37− if ss.CreatedAt.IsZero() {
38− ss.CreatedAt = time.Now()
37+ if ss.CreatedAt == nil || ss.CreatedAt.IsZero() {
38+ now := time.Now()
39+ ss.CreatedAt = &now
3940 }
4041 value, err := cookieForSession(ss, s.CookieCipher)
4142 if err != nil {
4243 return err
4344 }
44− s.setSessionCookie(rw, req, value, ss.CreatedAt)
45+ s.setSessionCookie(rw, req, value, *ss.CreatedAt)
4546 return nil
4647}
4748
pkg/sessions/redis/redis_store.go+4−3
@@ -133,8 +133,9 @@ func parseRedisURLs(urls []string) ([]string, error) {
133133// Save takes a sessions.SessionState and stores the information from it
134134// to redies, and adds a new ticket cookie on the HTTP response writer
135135func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *sessions.SessionState) error {
136− if s.CreatedAt.IsZero() {
137− s.CreatedAt = time.Now()
136+ if s.CreatedAt == nil || s.CreatedAt.IsZero() {
137+ now := time.Now()
138+ s.CreatedAt = &now
138139 }
139140
140141 // Old sessions that we are refreshing would have a request cookie
@@ -154,7 +155,7 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se
154155 req,
155156 ticketString,
156157 store.CookieOptions.Expire,
157− s.CreatedAt,
158+ *s.CreatedAt,
158159 )
159160
160161 http.SetCookie(rw, ticketCookie)
pkg/sessions/session_store_test.go+11−7
@@ -15,6 +15,7 @@ import (
1515 sessionsapi "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
1616 cookiesapi "github.com/oauth2-proxy/oauth2-proxy/pkg/cookies"
1717 "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
18+ "github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
1819 "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions"
1920 sessionscookie "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/cookie"
2021 "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/redis"
@@ -23,6 +24,8 @@ import (
2324)
2425
2526func TestSessionStore(t *testing.T) {
27+ logger.SetOutput(GinkgoWriter)
28+
2629 RegisterFailHandler(Fail)
2730 RunSpecs(t, "SessionStore")
2831}
@@ -253,16 +256,16 @@ var _ = Describe("NewSessionStore", func() {
253256
254257 // Can't compare time.Time using Equal() so remove ExpiresOn from sessions
255258 l := *loadedSession
256− l.CreatedAt = time.Time{}
257− l.ExpiresOn = time.Time{}
259+ l.CreatedAt = nil
260+ l.ExpiresOn = nil
258261 s := *session
259− s.CreatedAt = time.Time{}
260− s.ExpiresOn = time.Time{}
262+ s.CreatedAt = nil
263+ s.ExpiresOn = nil
261264 Expect(l).To(Equal(s))
262265
263266 // Compare time.Time separately
264− Expect(loadedSession.CreatedAt.Equal(session.CreatedAt)).To(BeTrue())
265− Expect(loadedSession.ExpiresOn.Equal(session.ExpiresOn)).To(BeTrue())
267+ Expect(loadedSession.CreatedAt.Equal(*session.CreatedAt)).To(BeTrue())
268+ Expect(loadedSession.ExpiresOn.Equal(*session.ExpiresOn)).To(BeTrue())
266269 }
267270 })
268271 }
@@ -392,10 +395,11 @@ var _ = Describe("NewSessionStore", func() {
392395 SameSite: "",
393396 }
394397
398+ expires := time.Now().Add(1 * time.Hour)
395399 session = &sessionsapi.SessionState{
396400 AccessToken: "AccessToken",
397401 IDToken: "IDToken",
398− ExpiresOn: time.Now().Add(1 * time.Hour),
402+ ExpiresOn: &expires,
399403 RefreshToken: "RefreshToken",
400404 Email: "john.doe@example.com",
401405 User: "john.doe",
providers/azure.go+4−2
@@ -126,11 +126,13 @@ func (p *AzureProvider) Redeem(ctx context.Context, redirectURL, code string) (s
126126 return
127127 }
128128
129+ created := time.Now()
130+ expires := time.Unix(jsonResponse.ExpiresOn, 0)
129131 s = &sessions.SessionState{
130132 AccessToken: jsonResponse.AccessToken,
131133 IDToken: jsonResponse.IDToken,
132− CreatedAt: time.Now(),
133− ExpiresOn: time.Unix(jsonResponse.ExpiresOn, 0),
134+ CreatedAt: &created,
135+ ExpiresOn: &expires,
134136 RefreshToken: jsonResponse.RefreshToken,
135137 }
136138 return
providers/gitlab.go+4−3
@@ -67,7 +67,7 @@ func (p *GitLabProvider) Redeem(ctx context.Context, redirectURL, code string) (
6767// RefreshSessionIfNeeded checks if the session has expired and uses the
6868// RefreshToken to fetch a new ID token if required
6969func (p *GitLabProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) {
70− if s == nil || s.ExpiresOn.After(time.Now()) || s.RefreshToken == "" {
70+ if s == nil || (s.ExpiresOn != nil && s.ExpiresOn.After(time.Now())) || s.RefreshToken == "" {
7171 return false, nil
7272 }
7373
@@ -209,12 +209,13 @@ func (p *GitLabProvider) createSessionState(ctx context.Context, token *oauth2.T
209209 return nil, fmt.Errorf("could not verify id_token: %v", err)
210210 }
211211
212+ created := time.Now()
212213 return &sessions.SessionState{
213214 AccessToken: token.AccessToken,
214215 IDToken: rawIDToken,
215216 RefreshToken: token.RefreshToken,
216− CreatedAt: time.Now(),
217− ExpiresOn: idToken.Expiry,
217+ CreatedAt: &created,
218+ ExpiresOn: &idToken.Expiry,
218219 }, nil
219220}
220221
providers/google.go+8−4
@@ -153,11 +153,14 @@ func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code string) (
153153 if err != nil {
154154 return
155155 }
156+
157+ created := time.Now()
158+ expires := time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second)
156159 s = &sessions.SessionState{
157160 AccessToken: jsonResponse.AccessToken,
158161 IDToken: jsonResponse.IDToken,
159− CreatedAt: time.Now(),
160− ExpiresOn: time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second),
162+ CreatedAt: &created,
163+ ExpiresOn: &expires,
161164 RefreshToken: jsonResponse.RefreshToken,
162165 Email: c.Email,
163166 User: c.Subject,
@@ -245,7 +248,7 @@ func (p *GoogleProvider) ValidateGroup(email string) bool {
245248// RefreshSessionIfNeeded checks if the session has expired and uses the
246249// RefreshToken to fetch a new ID token if required
247250func (p *GoogleProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) {
248− if s == nil || s.ExpiresOn.After(time.Now()) || s.RefreshToken == "" {
251+ if s == nil || (s.ExpiresOn != nil && s.ExpiresOn.After(time.Now())) || s.RefreshToken == "" {
249252 return false, nil
250253 }
251254
@@ -260,9 +263,10 @@ func (p *GoogleProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions
260263 }
261264
262265 origExpiration := s.ExpiresOn
266+ expires := time.Now().Add(duration).Truncate(time.Second)
263267 s.AccessToken = newToken
264268 s.IDToken = newIDToken
265− s.ExpiresOn = time.Now().Add(duration).Truncate(time.Second)
269+ s.ExpiresOn = &expires
266270 logger.Printf("refreshed access token %s (expired on %s)", s, origExpiration)
267271 return true, nil
268272}
providers/logingov.go+5−2
@@ -250,12 +250,15 @@ func (p *LoginGovProvider) Redeem(ctx context.Context, redirectURL, code string)
250250 return
251251 }
252252
253+ created := time.Now()
254+ expires := time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second)
255+
253256 // Store the data that we found in the session state
254257 s = &sessions.SessionState{
255258 AccessToken: jsonResponse.AccessToken,
256259 IDToken: jsonResponse.IDToken,
257− CreatedAt: time.Now(),
258− ExpiresOn: time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second),
260+ CreatedAt: &created,
261+ ExpiresOn: &expires,
259262 Email: email,
260263 }
261264 return
providers/oidc.go+5−4
@@ -72,7 +72,7 @@ func (p *OIDCProvider) Redeem(ctx context.Context, redirectURL, code string) (s
7272// RefreshSessionIfNeeded checks if the session has expired and uses the
7373// RefreshToken to fetch a new Access Token (and optional ID token) if required
7474func (p *OIDCProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) {
75− if s == nil || s.ExpiresOn.After(time.Now()) || s.RefreshToken == "" {
75+ if s == nil || (s.ExpiresOn != nil && s.ExpiresOn.After(time.Now())) || s.RefreshToken == "" {
7676 return false, nil
7777 }
7878
@@ -163,10 +163,11 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok
163163 }
164164 }
165165
166+ created := time.Now()
166167 newSession.AccessToken = token.AccessToken
167168 newSession.RefreshToken = token.RefreshToken
168− newSession.CreatedAt = time.Now()
169− newSession.ExpiresOn = token.Expiry
169+ newSession.CreatedAt = &created
170+ newSession.ExpiresOn = &token.Expiry
170171 return newSession, nil
171172}
172173
@@ -179,7 +180,7 @@ func (p *OIDCProvider) CreateSessionStateFromBearerToken(ctx context.Context, ra
179180 newSession.AccessToken = rawIDToken
180181 newSession.IDToken = rawIDToken
181182 newSession.RefreshToken = ""
182− newSession.ExpiresOn = idToken.Expiry
183+ newSession.ExpiresOn = &idToken.Expiry
183184
184185 return newSession, nil
185186}
providers/oidc_test.go+4−4
@@ -204,8 +204,8 @@ func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) {
204204 existingSession := &sessions.SessionState{
205205 AccessToken: "changeit",
206206 IDToken: idToken,
207− CreatedAt: time.Time{},
208− ExpiresOn: time.Time{},
207+ CreatedAt: nil,
208+ ExpiresOn: nil,
209209 RefreshToken: refreshToken,
210210 Email: "janedoe@example.com",
211211 User: "11223344",
@@ -238,8 +238,8 @@ func TestOIDCProviderRefreshSessionIfNeededWithIdToken(t *testing.T) {
238238 existingSession := &sessions.SessionState{
239239 AccessToken: "changeit",
240240 IDToken: "changeit",
241− CreatedAt: time.Time{},
242− ExpiresOn: time.Time{},
241+ CreatedAt: nil,
242+ ExpiresOn: nil,
243243 RefreshToken: refreshToken,
244244 Email: "changeit",
245245 User: "changeit",
providers/provider_default.go+3−2
@@ -81,7 +81,8 @@ func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (s
8181 return
8282 }
8383 if a := v.Get("access_token"); a != "" {
84− s = &sessions.SessionState{AccessToken: a, CreatedAt: time.Now()}
84+ created := time.Now()
85+ s = &sessions.SessionState{AccessToken: a, CreatedAt: &created}
8586 } else {
8687 err = fmt.Errorf("no access token found %s", body)
8788 }
@@ -168,7 +169,7 @@ func (p *ProviderData) CreateSessionStateFromBearerToken(ctx context.Context, ra
168169 newSession.AccessToken = rawIDToken
169170 newSession.IDToken = rawIDToken
170171 newSession.RefreshToken = ""
171− newSession.ExpiresOn = idToken.Expiry
172+ newSession.ExpiresOn = &idToken.Expiry
172173
173174 return newSession, nil
174175}
providers/provider_default_test.go+3−1
@@ -11,8 +11,10 @@ import (
1111
1212func TestRefresh(t *testing.T) {
1313 p := &ProviderData{}
14+
15+ expires := time.Now().Add(time.Duration(-11) * time.Minute)
1416 refreshed, err := p.RefreshSessionIfNeeded(context.Background(), &sessions.SessionState{
15− ExpiresOn: time.Now().Add(time.Duration(-11) * time.Minute),
17+ ExpiresOn: &expires,
1618 })
1719 assert.Equal(t, false, refreshed)
1820 assert.Equal(t, nil, err)