Drop support for pre v3.1 cookies (#535)

Co-authored-by: Henry Jenkins <henry@henryjenkins.name>

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)

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{

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 {