git.iko.soy/typeable/oauth2-proxy
on master help sign in
Use encoding/json for SessionState serialization (#63)

* Use encoding/json for SessionState serialization In order to make it easier to extend in future. * Store only email and user in cookie when cipher is unavailable This improves safety and robustness, and also preserves the existing behaviour. * Add TestEncodeSessionState/TestDecodeSessionState Use the test vectors with JSON encoding just introduced. * Support session state encoding in older versions * Add test cases for legacy session state strings * Add check for wrong expiration time in session state strings * Avoid exposing time.Time zero value when encoding session state string * Update CHANGELOG.md

commit2070fae47c6fb00c56ae4610e05913a96507e915 authorYAEGASHI Takeshi <yaegashi@users.noreply.github.com> dateWed 20 Mar 2019, 22:59 (8 years ago) parenta656435
3 files changed+308−85
compare with master
unifiedsplit
CHANGELOG.md+4 providers/session_state.go+122−65 providers/session_state_test.go+182−20
CHANGELOG.md+4
@@ -2,6 +2,10 @@
22
33## Changes since v3.1.0
44
5+- [#63](https://github.com/pusher/oauth2_proxy/pull/63) Use encoding/json for SessionState serialization (@yaegashi)
6+ - Use JSON to encode session state to be stored in browser cookies
7+ - Implement legacy decode function to support existing cookies generated by older versions
8+ - Add detailed table driven tests in session_state_test.go
59- [#55](https://github.com/pusher/oauth2_proxy/pull/55) Added login.gov provider (@timothy-spencer)
610- [#55](https://github.com/pusher/oauth2_proxy/pull/55) Added environment variables for all config options (@timothy-spencer)
711- [#70](https://github.com/pusher/oauth2_proxy/pull/70) Fix handling of splitted cookies (@einfachchr)
providers/session_state.go+122−65
@@ -1,6 +1,7 @@
11package providers
22
33import (
4+ "encoding/json"
45 "fmt"
56 "strconv"
67 "strings"
@@ -11,12 +12,18 @@ import (
1112
1213// SessionState is used to store information about the currently authenticated user session
1314type SessionState struct {
14− AccessToken string
15− IDToken string
16− ExpiresOn time.Time
17− RefreshToken string
18− Email string
19− User string
15+ AccessToken string `json:",omitempty"`
16+ IDToken string `json:",omitempty"`
17+ ExpiresOn time.Time `json:"-"`
18+ RefreshToken string `json:",omitempty"`
19+ Email string `json:",omitempty"`
20+ User string `json:",omitempty"`
21+}
22+
23+// SessionStateJSON is used to encode SessionState into JSON without exposing time.Time zero value
24+type SessionStateJSON struct {
25+ *SessionState
26+ ExpiresOn *time.Time `json:",omitempty"`
2027}
2128
2229// IsExpired checks whether the session has expired
@@ -29,7 +36,7 @@ func (s *SessionState) IsExpired() bool {
2936
3037// String constructs a summary of the session state
3138func (s *SessionState) String() string {
32− o := fmt.Sprintf("Session{%s", s.accountInfo())
39+ o := fmt.Sprintf("Session{email:%s user:%s", s.Email, s.User)
3340 if s.AccessToken != "" {
3441 o += " token:true"
3542 }
@@ -47,95 +54,145 @@ func (s *SessionState) String() string {
4754
4855// EncodeSessionState returns string representation of the current session
4956func (s *SessionState) EncodeSessionState(c *cookie.Cipher) (string, error) {
50− if c == nil || s.AccessToken == "" {
51− return s.accountInfo(), nil
52− }
53− return s.EncryptedString(c)
54−}
55−
56−func (s *SessionState) accountInfo() string {
57− return fmt.Sprintf("email:%s user:%s", s.Email, s.User)
58−}
59−
60−// EncryptedString encrypts the session state into a cookie string
61−func (s *SessionState) EncryptedString(c *cookie.Cipher) (string, error) {
62− var err error
57+ var ss SessionState
6358 if c == nil {
64− panic("error. missing cipher")
65− }
66− a := s.AccessToken
67− if a != "" {
68− if a, err = c.Encrypt(a); err != nil {
69− return "", err
59+ // Store only Email and User when cipher is unavailable
60+ ss.Email = s.Email
61+ ss.User = s.User
62+ } else {
63+ ss = *s
64+ var err error
65+ if ss.AccessToken != "" {
66+ ss.AccessToken, err = c.Encrypt(ss.AccessToken)
67+ if err != nil {
68+ return "", err
69+ }
7070 }
71− }
72− i := s.IDToken
73− if i != "" {
74− if i, err = c.Encrypt(i); err != nil {
75− return "", err
71+ if ss.IDToken != "" {
72+ ss.IDToken, err = c.Encrypt(ss.IDToken)
73+ if err != nil {
74+ return "", err
75+ }
7676 }
77− }
78− r := s.RefreshToken
79− if r != "" {
80− if r, err = c.Encrypt(r); err != nil {
81− return "", err
77+ if ss.RefreshToken != "" {
78+ ss.RefreshToken, err = c.Encrypt(ss.RefreshToken)
79+ if err != nil {
80+ return "", err
81+ }
8282 }
8383 }
84− return fmt.Sprintf("%s|%s|%s|%d|%s", s.accountInfo(), a, i, s.ExpiresOn.Unix(), r), nil
84+ // Embed SessionState and ExpiresOn pointer into SessionStateJSON
85+ ssj := &SessionStateJSON{SessionState: &ss}
86+ if !ss.ExpiresOn.IsZero() {
87+ ssj.ExpiresOn = &ss.ExpiresOn
88+ }
89+ b, err := json.Marshal(ssj)
90+ return string(b), err
8591}
8692
87−func decodeSessionStatePlain(v string) (s *SessionState, err error) {
93+// legacyDecodeSessionStatePlain decodes older plain session state string
94+func legacyDecodeSessionStatePlain(v string) (*SessionState, error) {
8895 chunks := strings.Split(v, " ")
8996 if len(chunks) != 2 {
90− return nil, fmt.Errorf("could not decode session state: expected 2 chunks got %d", len(chunks))
97+ return nil, fmt.Errorf("invalid session state (legacy: expected 2 chunks for user/email got %d)", len(chunks))
9198 }
9299
93− email := strings.TrimPrefix(chunks[0], "email:")
94100 user := strings.TrimPrefix(chunks[1], "user:")
95− if user == "" {
96− user = strings.Split(email, "@")[0]
97− }
101+ email := strings.TrimPrefix(chunks[0], "email:")
98102
99103 return &SessionState{User: user, Email: email}, nil
100104}
101105
102−// DecodeSessionState decodes the session cookie string into a SessionState
103−func DecodeSessionState(v string, c *cookie.Cipher) (s *SessionState, err error) {
106+// legacyDecodeSessionState attempts to decode the session state string
107+// generated by v3.1.0 or older
108+func legacyDecodeSessionState(v string, c *cookie.Cipher) (*SessionState, error) {
109+ chunks := strings.Split(v, "|")
110+
104111 if c == nil {
105− return decodeSessionStatePlain(v)
112+ if len(chunks) != 1 {
113+ return nil, fmt.Errorf("invalid session state (legacy: expected 1 chunk for plain got %d)", len(chunks))
114+ }
115+ return legacyDecodeSessionStatePlain(chunks[0])
106116 }
107117
108− chunks := strings.Split(v, "|")
109− if len(chunks) != 5 {
110− err = fmt.Errorf("invalid number of fields (got %d expected 5)", len(chunks))
111− return
118+ if len(chunks) != 4 && len(chunks) != 5 {
119+ return nil, fmt.Errorf("invalid session state (legacy: expected 4 or 5 chunks for full got %d)", len(chunks))
112120 }
113121
114− sessionState, err := decodeSessionStatePlain(chunks[0])
122+ i := 0
123+ ss, err := legacyDecodeSessionStatePlain(chunks[i])
115124 if err != nil {
116125 return nil, err
117126 }
118127
119− if chunks[1] != "" {
120− if sessionState.AccessToken, err = c.Decrypt(chunks[1]); err != nil {
121− return nil, err
122− }
128+ i++
129+ ss.AccessToken = chunks[i]
130+
131+ if len(chunks) == 5 {
132+ // SessionState with IDToken in v3.1.0
133+ i++
134+ ss.IDToken = chunks[i]
123135 }
124136
125− if chunks[2] != "" {
126− if sessionState.IDToken, err = c.Decrypt(chunks[2]); err != nil {
127− return nil, err
128− }
137+ i++
138+ ts, err := strconv.Atoi(chunks[i])
139+ if err != nil {
140+ return nil, fmt.Errorf("invalid session state (legacy: wrong expiration time: %s)", err)
129141 }
142+ ss.ExpiresOn = time.Unix(int64(ts), 0)
143+
144+ i++
145+ ss.RefreshToken = chunks[i]
130146
131− ts, _ := strconv.Atoi(chunks[3])
132− sessionState.ExpiresOn = time.Unix(int64(ts), 0)
147+ return ss, nil
148+}
133149
134− if chunks[4] != "" {
135− if sessionState.RefreshToken, err = c.Decrypt(chunks[4]); err != nil {
150+// DecodeSessionState decodes the session cookie string into a SessionState
151+func DecodeSessionState(v string, c *cookie.Cipher) (*SessionState, error) {
152+ var ssj SessionStateJSON
153+ var ss *SessionState
154+ err := json.Unmarshal([]byte(v), &ssj)
155+ if err == nil && ssj.SessionState != nil {
156+ // Extract SessionState and ExpiresOn value from SessionStateJSON
157+ ss = ssj.SessionState
158+ if ssj.ExpiresOn != nil {
159+ ss.ExpiresOn = *ssj.ExpiresOn
160+ }
161+ } else {
162+ // Try to decode a legacy string when json.Unmarshal failed
163+ ss, err = legacyDecodeSessionState(v, c)
164+ if err != nil {
136165 return nil, err
137166 }
138167 }
139−
140− return sessionState, nil
168+ if c == nil {
169+ // Load only Email and User when cipher is unavailable
170+ ss = &SessionState{
171+ Email: ss.Email,
172+ User: ss.User,
173+ }
174+ } else {
175+ if ss.AccessToken != "" {
176+ ss.AccessToken, err = c.Decrypt(ss.AccessToken)
177+ if err != nil {
178+ return nil, err
179+ }
180+ }
181+ if ss.IDToken != "" {
182+ ss.IDToken, err = c.Decrypt(ss.IDToken)
183+ if err != nil {
184+ return nil, err
185+ }
186+ }
187+ if ss.RefreshToken != "" {
188+ ss.RefreshToken, err = c.Decrypt(ss.RefreshToken)
189+ if err != nil {
190+ return nil, err
191+ }
192+ }
193+ }
194+ if ss.User == "" {
195+ ss.User = strings.Split(ss.Email, "@")[0]
196+ }
197+ return ss, nil
141198}
providers/session_state_test.go+182−20
@@ -2,7 +2,6 @@ package providers
22
33import (
44 "fmt"
5− "strings"
65 "testing"
76 "time"
87
@@ -27,7 +26,6 @@ func TestSessionStateSerialization(t *testing.T) {
2726 }
2827 encoded, err := s.EncodeSessionState(c)
2928 assert.Equal(t, nil, err)
30− assert.Equal(t, 4, strings.Count(encoded, "|"))
3129
3230 ss, err := DecodeSessionState(encoded, c)
3331 t.Logf("%#v", ss)
@@ -65,7 +63,6 @@ func TestSessionStateSerializationWithUser(t *testing.T) {
6563 }
6664 encoded, err := s.EncodeSessionState(c)
6765 assert.Equal(t, nil, err)
68− assert.Equal(t, 4, strings.Count(encoded, "|"))
6966
7067 ss, err := DecodeSessionState(encoded, c)
7168 t.Logf("%#v", ss)
@@ -96,8 +93,6 @@ func TestSessionStateSerializationNoCipher(t *testing.T) {
9693 }
9794 encoded, err := s.EncodeSessionState(nil)
9895 assert.Equal(t, nil, err)
99− expected := fmt.Sprintf("email:%s user:", s.Email)
100− assert.Equal(t, expected, encoded)
10196
10297 // only email should have been serialized
10398 ss, err := DecodeSessionState(encoded, nil)
@@ -118,8 +113,6 @@ func TestSessionStateSerializationNoCipherWithUser(t *testing.T) {
118113 }
119114 encoded, err := s.EncodeSessionState(nil)
120115 assert.Equal(t, nil, err)
121− expected := fmt.Sprintf("email:%s user:%s", s.Email, s.User)
122− assert.Equal(t, expected, encoded)
123116
124117 // only email should have been serialized
125118 ss, err := DecodeSessionState(encoded, nil)
@@ -130,19 +123,6 @@ func TestSessionStateSerializationNoCipherWithUser(t *testing.T) {
130123 assert.Equal(t, "", ss.RefreshToken)
131124}
132125
133−func TestSessionStateAccountInfo(t *testing.T) {
134− s := &SessionState{
135− Email: "user@domain.com",
136− User: "just-user",
137− }
138− expected := fmt.Sprintf("email:%v user:%v", s.Email, s.User)
139− assert.Equal(t, expected, s.accountInfo())
140−
141− s.Email = ""
142− expected = fmt.Sprintf("email:%v user:%v", s.Email, s.User)
143− assert.Equal(t, expected, s.accountInfo())
144−}
145−
146126func TestExpired(t *testing.T) {
147127 s := &SessionState{ExpiresOn: time.Now().Add(time.Duration(-1) * time.Minute)}
148128 assert.Equal(t, true, s.IsExpired())
@@ -153,3 +133,185 @@ func TestExpired(t *testing.T) {
153133 s = &SessionState{}
154134 assert.Equal(t, false, s.IsExpired())
155135}
136+
137+type testCase struct {
138+ SessionState
139+ Encoded string
140+ Cipher *cookie.Cipher
141+ Error bool
142+}
143+
144+// TestEncodeSessionState tests EncodeSessionState with the test vector
145+//
146+// Currently only tests without cipher here because we have no way to mock
147+// the random generator used in EncodeSessionState.
148+func TestEncodeSessionState(t *testing.T) {
149+ e := time.Now().Add(time.Duration(1) * time.Hour)
150+
151+ testCases := []testCase{
152+ {
153+ SessionState: SessionState{
154+ Email: "user@domain.com",
155+ User: "just-user",
156+ },
157+ Encoded: `{"Email":"user@domain.com","User":"just-user"}`,
158+ },
159+ {
160+ SessionState: SessionState{
161+ Email: "user@domain.com",
162+ User: "just-user",
163+ AccessToken: "token1234",
164+ IDToken: "rawtoken1234",
165+ ExpiresOn: e,
166+ RefreshToken: "refresh4321",
167+ },
168+ Encoded: `{"Email":"user@domain.com","User":"just-user"}`,
169+ },
170+ }
171+
172+ for i, tc := range testCases {
173+ encoded, err := tc.EncodeSessionState(tc.Cipher)
174+ t.Logf("i:%d Encoded:%#v SessionState:%#v Error:%#v", i, encoded, tc.SessionState, err)
175+ if tc.Error {
176+ assert.Error(t, err)
177+ assert.Empty(t, encoded)
178+ continue
179+ }
180+ assert.NoError(t, err)
181+ assert.JSONEq(t, tc.Encoded, encoded)
182+ }
183+}
184+
185+// TestDecodeSessionState tests DecodeSessionState with the test vector
186+func TestDecodeSessionState(t *testing.T) {
187+ e := time.Now().Add(time.Duration(1) * time.Hour)
188+ eJSON, _ := e.MarshalJSON()
189+ eString := string(eJSON)
190+ eUnix := e.Unix()
191+
192+ c, err := cookie.NewCipher([]byte(secret))
193+ assert.NoError(t, err)
194+
195+ testCases := []testCase{
196+ {
197+ SessionState: SessionState{
198+ Email: "user@domain.com",
199+ User: "just-user",
200+ },
201+ Encoded: `{"Email":"user@domain.com","User":"just-user"}`,
202+ },
203+ {
204+ SessionState: SessionState{
205+ Email: "user@domain.com",
206+ User: "user",
207+ },
208+ Encoded: `{"Email":"user@domain.com"}`,
209+ },
210+ {
211+ SessionState: SessionState{
212+ User: "just-user",
213+ },
214+ Encoded: `{"User":"just-user"}`,
215+ },
216+ {
217+ SessionState: SessionState{
218+ Email: "user@domain.com",
219+ User: "just-user",
220+ },
221+ Encoded: fmt.Sprintf(`{"Email":"user@domain.com","User":"just-user","AccessToken":"I6s+ml+/MldBMgHIiC35BTKTh57skGX24w==","IDToken":"xojNdyyjB1HgYWh6XMtXY/Ph5eCVxa1cNsklJw==","RefreshToken":"qEX0x6RmASxo4dhlBG6YuRs9Syn/e9sHu/+K","ExpiresOn":%s}`, eString),
222+ },
223+ {
224+ SessionState: SessionState{
225+ Email: "user@domain.com",
226+ User: "just-user",
227+ AccessToken: "token1234",
228+ IDToken: "rawtoken1234",
229+ ExpiresOn: e,
230+ RefreshToken: "refresh4321",
231+ },
232+ Encoded: fmt.Sprintf(`{"Email":"user@domain.com","User":"just-user","AccessToken":"I6s+ml+/MldBMgHIiC35BTKTh57skGX24w==","IDToken":"xojNdyyjB1HgYWh6XMtXY/Ph5eCVxa1cNsklJw==","RefreshToken":"qEX0x6RmASxo4dhlBG6YuRs9Syn/e9sHu/+K","ExpiresOn":%s}`, eString),
233+ Cipher: c,
234+ },
235+ {
236+ SessionState: SessionState{
237+ Email: "user@domain.com",
238+ User: "just-user",
239+ },
240+ Encoded: `{"Email":"user@domain.com","User":"just-user"}`,
241+ Cipher: c,
242+ },
243+ {
244+ Encoded: `{"Email":"user@domain.com","User":"just-user","AccessToken":"X"}`,
245+ Cipher: c,
246+ Error: true,
247+ },
248+ {
249+ Encoded: `{"Email":"user@domain.com","User":"just-user","IDToken":"XXXX"}`,
250+ Cipher: c,
251+ Error: true,
252+ },
253+ {
254+ SessionState: SessionState{
255+ User: "just-user",
256+ Email: "user@domain.com",
257+ },
258+ Encoded: "email:user@domain.com user:just-user",
259+ },
260+ {
261+ Encoded: "email:user@domain.com user:just-user||||",
262+ Error: true,
263+ },
264+ {
265+ Encoded: "email:user@domain.com user:just-user",
266+ Cipher: c,
267+ Error: true,
268+ },
269+ {
270+ Encoded: "email:user@domain.com user:just-user|||99999999999999999999|",
271+ Cipher: c,
272+ Error: true,
273+ },
274+ {
275+ SessionState: SessionState{
276+ Email: "user@domain.com",
277+ User: "just-user",
278+ AccessToken: "token1234",
279+ ExpiresOn: e,
280+ RefreshToken: "refresh4321",
281+ },
282+ Encoded: fmt.Sprintf("email:user@domain.com user:just-user|I6s+ml+/MldBMgHIiC35BTKTh57skGX24w==|%d|qEX0x6RmASxo4dhlBG6YuRs9Syn/e9sHu/+K", eUnix),
283+ Cipher: c,
284+ },
285+ {
286+ SessionState: SessionState{
287+ Email: "user@domain.com",
288+ User: "just-user",
289+ AccessToken: "token1234",
290+ IDToken: "rawtoken1234",
291+ ExpiresOn: e,
292+ RefreshToken: "refresh4321",
293+ },
294+ Encoded: fmt.Sprintf("email:user@domain.com user:just-user|I6s+ml+/MldBMgHIiC35BTKTh57skGX24w==|xojNdyyjB1HgYWh6XMtXY/Ph5eCVxa1cNsklJw==|%d|qEX0x6RmASxo4dhlBG6YuRs9Syn/e9sHu/+K", eUnix),
295+ Cipher: c,
296+ },
297+ }
298+
299+ for i, tc := range testCases {
300+ ss, err := DecodeSessionState(tc.Encoded, tc.Cipher)
301+ t.Logf("i:%d Encoded:%#v SessionState:%#v Error:%#v", i, tc.Encoded, ss, err)
302+ if tc.Error {
303+ assert.Error(t, err)
304+ assert.Nil(t, ss)
305+ continue
306+ }
307+ assert.NoError(t, err)
308+ if assert.NotNil(t, ss) {
309+ assert.Equal(t, tc.User, ss.User)
310+ assert.Equal(t, tc.Email, ss.Email)
311+ assert.Equal(t, tc.AccessToken, ss.AccessToken)
312+ assert.Equal(t, tc.RefreshToken, ss.RefreshToken)
313+ assert.Equal(t, tc.IDToken, ss.IDToken)
314+ assert.Equal(t, tc.ExpiresOn.Unix(), ss.ExpiresOn.Unix())
315+ }
316+ }
317+}