From de280824de5e9025d7e319e6e0c77f30a84c2b96 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Sun, 10 May 2020 10:09:53 +0100
Subject: [PATCH] Drop support for pre v3.1 cookies (#535)
Co-authored-by: Henry Jenkins <henry@henryjenkins.name>
---
CHANGELOG.md | 4 ++
pkg/apis/sessions/session_state.go | 90 +++++--------------------
pkg/apis/sessions/session_state_test.go | 45 -------------
3 files changed, 20 insertions(+), 119 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d52b9e3..bf13cd8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,9 +22,13 @@
- [#487](https://github.com/oauth2-proxy/oauth2-proxy/pull/487) Switch flags to StringSlice instead of StringArray
- Options that take multiple arguments now split strings on commas if present
- Eg `--foo=a,b,c,d` would result in the values `a`, `b`, `c` and `d` instead of a single `a,b,c,d` value as before
+- [#535](https://github.com/oauth2-proxy/oauth2-proxy/pull/535) Drop support for pre v3.1 cookies
+ - The encoding for session cookies was changed starting in v3.1.0, support for the previous encoding is now dropped
+ - If you are upgrading from a version earlier than this, please upgrade via a version between v3.1.0 and v5.1.1
## Changes since v5.1.1
+- [#535](https://github.com/oauth2-proxy/oauth2-proxy/pull/535) Drop support for pre v3.1 cookies (@JoelSpeed)
- [#533](https://github.com/oauth2-proxy/oauth2-proxy/pull/487) Set up code coverage within Travis for Code Climate (@JoelSpeed)
- [#514](https://github.com/oauth2-proxy/oauth2-proxy/pull/514) Add basic string functions to templates
- [#524](https://github.com/oauth2-proxy/oauth2-proxy/pull/524) Sign cookies with SHA256 (@NickMeves)
diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go
index d3855b1..f970052 100644
--- a/pkg/apis/sessions/session_state.go
+++ b/pkg/apis/sessions/session_state.go
@@ -2,9 +2,8 @@ package sessions
import (
"encoding/json"
+ "errors"
"fmt"
- "strconv"
- "strings"
"time"
"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
@@ -126,84 +125,27 @@ func (s *SessionState) EncodeSessionState(c *encryption.Cipher) (string, error)
return string(b), err
}
-// legacyDecodeSessionStatePlain decodes older plain session state string
-func legacyDecodeSessionStatePlain(v string) (*SessionState, error) {
- chunks := strings.Split(v, " ")
- if len(chunks) != 2 {
- return nil, fmt.Errorf("invalid session state (legacy: expected 2 chunks for user/email got %d)", len(chunks))
- }
-
- user := strings.TrimPrefix(chunks[1], "user:")
- email := strings.TrimPrefix(chunks[0], "email:")
-
- return &SessionState{User: user, Email: email}, nil
-}
-
-// legacyDecodeSessionState attempts to decode the session state string
-// generated by v3.1.0 or older
-func legacyDecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {
- chunks := strings.Split(v, "|")
-
- if c == nil {
- if len(chunks) != 1 {
- return nil, fmt.Errorf("invalid session state (legacy: expected 1 chunk for plain got %d)", len(chunks))
- }
- return legacyDecodeSessionStatePlain(chunks[0])
- }
-
- if len(chunks) != 4 && len(chunks) != 5 {
- return nil, fmt.Errorf("invalid session state (legacy: expected 4 or 5 chunks for full got %d)", len(chunks))
- }
-
- i := 0
- ss, err := legacyDecodeSessionStatePlain(chunks[i])
- if err != nil {
- return nil, err
- }
-
- i++
- ss.AccessToken = chunks[i]
-
- if len(chunks) == 5 {
- // SessionState with IDToken in v3.1.0
- i++
- ss.IDToken = chunks[i]
- }
-
- i++
- ts, err := strconv.Atoi(chunks[i])
- if err != nil {
- return nil, fmt.Errorf("invalid session state (legacy: wrong expiration time: %s)", err)
- }
- ss.ExpiresOn = time.Unix(int64(ts), 0)
-
- i++
- ss.RefreshToken = chunks[i]
-
- return ss, nil
-}
-
// DecodeSessionState decodes the session cookie string into a SessionState
func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {
var ssj SessionStateJSON
var ss *SessionState
err := json.Unmarshal([]byte(v), &ssj)
- if err == nil && ssj.SessionState != nil {
- // Extract SessionState and CreatedAt,ExpiresOn value from SessionStateJSON
- ss = ssj.SessionState
- if ssj.CreatedAt != nil {
- ss.CreatedAt = *ssj.CreatedAt
- }
- if ssj.ExpiresOn != nil {
- ss.ExpiresOn = *ssj.ExpiresOn
- }
- } else {
- // Try to decode a legacy string when json.Unmarshal failed
- ss, err = legacyDecodeSessionState(v, c)
- if err != nil {
- return nil, err
- }
+ if err != nil {
+ return nil, fmt.Errorf("error unmarshalling session: %w", err)
+ }
+ if ssj.SessionState == nil {
+ return nil, errors.New("expected session state to not be nil")
}
+
+ // Extract SessionState and CreatedAt,ExpiresOn value from SessionStateJSON
+ ss = ssj.SessionState
+ if ssj.CreatedAt != nil {
+ ss.CreatedAt = *ssj.CreatedAt
+ }
+ if ssj.ExpiresOn != nil {
+ ss.ExpiresOn = *ssj.ExpiresOn
+ }
+
if c == nil {
// Load only Email and User when cipher is unavailable
ss = &SessionState{
diff --git a/pkg/apis/sessions/session_state_test.go b/pkg/apis/sessions/session_state_test.go
index 6c9d9e3..dbfea6e 100644
--- a/pkg/apis/sessions/session_state_test.go
+++ b/pkg/apis/sessions/session_state_test.go
@@ -211,7 +211,6 @@ func TestDecodeSessionState(t *testing.T) {
e := time.Now().Add(time.Duration(1) * time.Hour)
eJSON, _ := e.MarshalJSON()
eString := string(eJSON)
- eUnix := e.Unix()
c, err := encryption.NewCipher([]byte(secret))
assert.NoError(t, err)
@@ -275,50 +274,6 @@ func TestDecodeSessionState(t *testing.T) {
Cipher: c,
Error: true,
},
- {
- SessionState: sessions.SessionState{
- User: "just-user",
- Email: "user@domain.com",
- },
- Encoded: "email:user@domain.com user:just-user",
- },
- {
- Encoded: "email:user@domain.com user:just-user||||",
- Error: true,
- },
- {
- Encoded: "email:user@domain.com user:just-user",
- Cipher: c,
- Error: true,
- },
- {
- Encoded: "email:user@domain.com user:just-user|||99999999999999999999|",
- Cipher: c,
- Error: true,
- },
- {
- SessionState: sessions.SessionState{
- Email: "user@domain.com",
- User: "just-user",
- AccessToken: "token1234",
- ExpiresOn: e,
- RefreshToken: "refresh4321",
- },
- Encoded: fmt.Sprintf("email:user@domain.com user:just-user|I6s+ml+/MldBMgHIiC35BTKTh57skGX24w==|%d|qEX0x6RmASxo4dhlBG6YuRs9Syn/e9sHu/+K", eUnix),
- Cipher: c,
- },
- {
- SessionState: sessions.SessionState{
- Email: "user@domain.com",
- User: "just-user",
- AccessToken: "token1234",
- IDToken: "rawtoken1234",
- ExpiresOn: e,
- RefreshToken: "refresh4321",
- },
- Encoded: fmt.Sprintf("email:user@domain.com user:just-user|I6s+ml+/MldBMgHIiC35BTKTh57skGX24w==|xojNdyyjB1HgYWh6XMtXY/Ph5eCVxa1cNsklJw==|%d|qEX0x6RmASxo4dhlBG6YuRs9Syn/e9sHu/+K", eUnix),
- Cipher: c,
- },
}
for i, tc := range testCases {
--
GitLab