Remove v5 JSON session support

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ## Important Notes
 
+- [#905](https://github.com/oauth2-proxy/oauth2-proxy/pull/905) Existing sessions from v6.0.0 or earlier are no longer valid. They will trigger a reauthentication.
 - [#826](https://github.com/oauth2-proxy/oauth2-proxy/pull/826) `skip-auth-strip-headers` now applies to all requests, not just those where authentication would be skipped.
 - [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) `--skip-auth-route` is (almost) backwards compatible with `--skip-auth-regex`
   - We are marking `--skip-auth-regex` as DEPRECATED and will remove it in the next major version.
@@ -33,6 +34,7 @@
 
 ## Changes since v6.1.1
 
+- [#905](https://github.com/oauth2-proxy/oauth2-proxy/pull/905) Remove v5 legacy sessions support (@NickMeves)
 - [#904](https://github.com/oauth2-proxy/oauth2-proxy/pull/904) Set `skip-auth-strip-headers` to `true` by default (@NickMeves)
 - [#826](https://github.com/oauth2-proxy/oauth2-proxy/pull/826) Integrate new header injectors into project (@JoelSpeed)
 - [#898](https://github.com/oauth2-proxy/oauth2-proxy/pull/898) Migrate documentation to Docusaurus (@JoelSpeed)

pkg/apis/sessions/legacy_v5_tester.go

@@ -1,87 +0,0 @@
-package sessions
-
-import (
-	"fmt"
-	"testing"
-	"time"
-
-	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption"
-	"github.com/stretchr/testify/assert"
-)
-
-const LegacyV5TestSecret = "0123456789abcdefghijklmnopqrstuv"
-
-// LegacyV5TestCase provides V5 JSON based test cases for legacy fallback code
-type LegacyV5TestCase struct {
-	Input  string
-	Error  bool
-	Output *SessionState
-}
-
-// CreateLegacyV5TestCases makes various V5 JSON sessions as test cases
-//
-// Used for `apis/sessions/session_state_test.go` & `sessions/redis/redis_store_test.go`
-//
-// TODO: Remove when this is deprecated (likely V7)
-func CreateLegacyV5TestCases(t *testing.T) (map[string]LegacyV5TestCase, encryption.Cipher, encryption.Cipher) {
-	created := time.Now()
-	createdJSON, err := created.MarshalJSON()
-	assert.NoError(t, err)
-	createdString := string(createdJSON)
-	e := time.Now().Add(time.Duration(1) * time.Hour)
-	eJSON, err := e.MarshalJSON()
-	assert.NoError(t, err)
-	eString := string(eJSON)
-
-	cfbCipher, err := encryption.NewCFBCipher([]byte(LegacyV5TestSecret))
-	assert.NoError(t, err)
-	legacyCipher := encryption.NewBase64Cipher(cfbCipher)
-
-	testCases := map[string]LegacyV5TestCase{
-		"User & email unencrypted": {
-			Input: `{"Email":"user@domain.com","User":"just-user"}`,
-			Error: true,
-		},
-		"Only email unencrypted": {
-			Input: `{"Email":"user@domain.com"}`,
-			Error: true,
-		},
-		"Just user unencrypted": {
-			Input: `{"User":"just-user"}`,
-			Error: true,
-		},
-		"User and Email unencrypted while rest is encrypted": {
-			Input: fmt.Sprintf(`{"Email":"user@domain.com","User":"just-user","AccessToken":"I6s+ml+/MldBMgHIiC35BTKTh57skGX24w==","IDToken":"xojNdyyjB1HgYWh6XMtXY/Ph5eCVxa1cNsklJw==","RefreshToken":"qEX0x6RmASxo4dhlBG6YuRs9Syn/e9sHu/+K","CreatedAt":%s,"ExpiresOn":%s}`, createdString, eString),
-			Error: true,
-		},
-		"Full session with cipher": {
-			Input: fmt.Sprintf(`{"Email":"FsKKYrTWZWrxSOAqA/fTNAUZS5QWCqOBjuAbBlbVOw==","User":"rT6JP3dxQhxUhkWrrd7yt6c1mDVyQCVVxw==","AccessToken":"I6s+ml+/MldBMgHIiC35BTKTh57skGX24w==","IDToken":"xojNdyyjB1HgYWh6XMtXY/Ph5eCVxa1cNsklJw==","RefreshToken":"qEX0x6RmASxo4dhlBG6YuRs9Syn/e9sHu/+K","CreatedAt":%s,"ExpiresOn":%s}`, createdString, eString),
-			Output: &SessionState{
-				Email:        "user@domain.com",
-				User:         "just-user",
-				AccessToken:  "token1234",
-				IDToken:      "rawtoken1234",
-				CreatedAt:    &created,
-				ExpiresOn:    &e,
-				RefreshToken: "refresh4321",
-			},
-		},
-		"Minimal session encrypted with cipher": {
-			Input: `{"Email":"EGTllJcOFC16b7LBYzLekaHAC5SMMSPdyUrg8hd25g==","User":"rT6JP3dxQhxUhkWrrd7yt6c1mDVyQCVVxw=="}`,
-			Output: &SessionState{
-				Email: "user@domain.com",
-				User:  "just-user",
-			},
-		},
-		"Unencrypted User, Email and AccessToken": {
-			Input: `{"Email":"user@domain.com","User":"just-user","AccessToken":"X"}`,
-			Error: true,
-		},
-		"Unencrypted User, Email and IDToken": {
-			Input: `{"Email":"user@domain.com","User":"just-user","IDToken":"XXXX"}`,
-			Error: true,
-		},
-	}
-
-	return testCases, cfbCipher, legacyCipher
-}

pkg/apis/sessions/session_state.go

@@ -2,7 +2,6 @@ package sessions
 
 import (
 	"bytes"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -18,15 +17,17 @@ import (
 
 // SessionState is used to store information about the currently authenticated user session
 type SessionState struct {
-	AccessToken       string     `json:",omitempty" msgpack:"at,omitempty"`
+	CreatedAt *time.Time `msgpack:"ca,omitempty"`
-	IDToken           string     `json:",omitempty" msgpack:"it,omitempty"`
+	ExpiresOn *time.Time `msgpack:"eo,omitempty"`
-	CreatedAt         *time.Time `json:",omitempty" msgpack:"ca,omitempty"`
+
-	ExpiresOn         *time.Time `json:",omitempty" msgpack:"eo,omitempty"`
+	AccessToken  string `msgpack:"at,omitempty"`
-	RefreshToken      string     `json:",omitempty" msgpack:"rt,omitempty"`
+	IDToken      string `msgpack:"it,omitempty"`
-	Email             string     `json:",omitempty" msgpack:"e,omitempty"`
+	RefreshToken string `msgpack:"rt,omitempty"`
-	User              string     `json:",omitempty" msgpack:"u,omitempty"`
+
-	Groups            []string   `json:",omitempty" msgpack:"g,omitempty"`
+	Email             string   `msgpack:"e,omitempty"`
-	PreferredUsername string     `json:",omitempty" msgpack:"pu,omitempty"`
+	User              string   `msgpack:"u,omitempty"`
+	Groups            []string `msgpack:"g,omitempty"`
+	PreferredUsername string   `msgpack:"pu,omitempty"`
 }
 
 // IsExpired checks whether the session has expired
@@ -146,52 +147,6 @@ func DecodeSessionState(data []byte, c encryption.Cipher, compressed bool) (*Ses
 	return &ss, nil
 }
 
-// LegacyV5DecodeSessionState decodes a legacy JSON session cookie string into a SessionState
-func LegacyV5DecodeSessionState(v string, c encryption.Cipher) (*SessionState, error) {
-	var ss SessionState
-	err := json.Unmarshal([]byte(v), &ss)
-	if err != nil {
-		return nil, fmt.Errorf("error unmarshalling session: %w", err)
-	}
-
-	for _, s := range []*string{
-		&ss.User,
-		&ss.Email,
-		&ss.PreferredUsername,
-		&ss.AccessToken,
-		&ss.IDToken,
-		&ss.RefreshToken,
-	} {
-		err := into(s, c.Decrypt)
-		if err != nil {
-			return nil, err
-		}
-	}
-	err = ss.validate()
-	if err != nil {
-		return nil, err
-	}
-
-	return &ss, nil
-}
-
-// codecFunc is a function that takes a []byte and encodes/decodes it
-type codecFunc func([]byte) ([]byte, error)
-
-func into(s *string, f codecFunc) error {
-	// Do not encrypt/decrypt nil or empty strings
-	if s == nil || *s == "" {
-		return nil
-	}
-
-	d, err := f([]byte(*s))
-	if err != nil {
-		return err
-	}
-	*s = string(d)
-	return nil
-}
-
 // lz4Compress compresses with LZ4
 //
 // The Compress:Decompress ratio is 1:Many. LZ4 gives fastest decompress speeds

pkg/apis/sessions/session_state_test.go

@@ -4,7 +4,6 @@ import (
 	"crypto/rand"
 	"fmt"
 	"io"
-	mathrand "math/rand"
 	"testing"
 	"time"
 
@@ -257,78 +256,6 @@ func TestEncodeAndDecodeSessionState(t *testing.T) {
 	}
 }
 
-// TestLegacyV5DecodeSessionState confirms V5 JSON sessions decode
-//
-// TODO: Remove when this is deprecated (likely V7)
-func TestLegacyV5DecodeSessionState(t *testing.T) {
-	testCases, cipher, legacyCipher := CreateLegacyV5TestCases(t)
-
-	for testName, tc := range testCases {
-		t.Run(testName, func(t *testing.T) {
-			// Legacy sessions fail in DecodeSessionState which results in
-			// the fallback to LegacyV5DecodeSessionState
-			_, err := DecodeSessionState([]byte(tc.Input), cipher, false)
-			assert.Error(t, err)
-			_, err = DecodeSessionState([]byte(tc.Input), cipher, true)
-			assert.Error(t, err)
-
-			ss, err := LegacyV5DecodeSessionState(tc.Input, legacyCipher)
-			if tc.Error {
-				assert.Error(t, err)
-				assert.Nil(t, ss)
-				return
-			}
-			assert.NoError(t, err)
-			compareSessionStates(t, tc.Output, ss)
-		})
-	}
-}
-
-// Test_into tests the into helper function used in LegacyV5DecodeSessionState
-//
-// TODO: Remove when this is deprecated (likely V7)
-func Test_into(t *testing.T) {
-	const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
-
-	// Test all 3 valid AES sizes
-	for _, secretSize := range []int{16, 24, 32} {
-		t.Run(fmt.Sprintf("%d", secretSize), func(t *testing.T) {
-			secret := make([]byte, secretSize)
-			_, err := io.ReadFull(rand.Reader, secret)
-			assert.Equal(t, nil, err)
-
-			cfb, err := encryption.NewCFBCipher(secret)
-			assert.NoError(t, err)
-			c := encryption.NewBase64Cipher(cfb)
-
-			// Check no errors with empty or nil strings
-			empty := ""
-			assert.Equal(t, nil, into(&empty, c.Encrypt))
-			assert.Equal(t, nil, into(&empty, c.Decrypt))
-			assert.Equal(t, nil, into(nil, c.Encrypt))
-			assert.Equal(t, nil, into(nil, c.Decrypt))
-
-			// Test various sizes tokens might be
-			for _, dataSize := range []int{10, 100, 1000, 5000, 10000} {
-				t.Run(fmt.Sprintf("%d", dataSize), func(t *testing.T) {
-					b := make([]byte, dataSize)
-					for i := range b {
-						b[i] = charset[mathrand.Intn(len(charset))]
-					}
-					data := string(b)
-					originalData := data
-
-					assert.Equal(t, nil, into(&data, c.Encrypt))
-					assert.NotEqual(t, originalData, data)
-
-					assert.Equal(t, nil, into(&data, c.Decrypt))
-					assert.Equal(t, originalData, data)
-				})
-			}
-		})
-	}
-}
-
 func compareSessionStates(t *testing.T, expected *SessionState, actual *SessionState) {
 	if expected.CreatedAt != nil {
 		assert.NotNil(t, actual.CreatedAt)

pkg/sessions/cookie/session_store.go

@@ -60,7 +60,7 @@ func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) {
 		return nil, errors.New("cookie signature not valid")
 	}
 
-	session, err := sessionFromCookie(val, s.CookieCipher)
+	session, err := sessions.DecodeSessionState(val, s.CookieCipher, true)
 	if err != nil {
 		return nil, err
 	}
@@ -98,20 +98,6 @@ func (s *SessionStore) cookieForSession(ss *sessions.SessionState) ([]byte, erro
 	return ss.EncodeSessionState(s.CookieCipher, true)
 }
 
-// sessionFromCookie deserializes a session from a cookie value
-func sessionFromCookie(v []byte, c encryption.Cipher) (s *sessions.SessionState, err error) {
-	ss, err := sessions.DecodeSessionState(v, c, true)
-	// If anything fails (Decrypt, LZ4, MessagePack), try legacy JSON decode
-	// LZ4 will likely fail for wrong header after AES-CFB spits out garbage
-	// data from trying to decrypt JSON it things is ciphertext
-	if err != nil {
-		// Legacy used Base64 + AES CFB
-		legacyCipher := encryption.NewBase64Cipher(c)
-		return sessions.LegacyV5DecodeSessionState(string(v), legacyCipher)
-	}
-	return ss, nil
-}
-
 // setSessionCookie adds the user's session cookie to the response
 func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Request, val []byte, created time.Time) error {
 	cookies, err := s.makeSessionCookie(req, val, created)

pkg/sessions/persistence/ticket.go

@@ -2,7 +2,6 @@ package persistence
 
 import (
 	"crypto/aes"
-	"crypto/cipher"
 	"crypto/rand"
 	"encoding/base64"
 	"encoding/hex"
@@ -123,8 +122,6 @@ func (t *ticket) saveSession(s *sessions.SessionState, saver saveFunc) error {
 // loadSession loads a session from the disk store via the passed loadFunc
 // using the ticket.id as the key. It then decodes the SessionState using
 // ticket.secret to make the AES-GCM cipher.
-//
-// TODO (@NickMeves): Remove legacyV5LoadSession support in V7
 func (t *ticket) loadSession(loader loadFunc) (*sessions.SessionState, error) {
 	ciphertext, err := loader(t.id)
 	if err != nil {
@@ -134,11 +131,8 @@ func (t *ticket) loadSession(loader loadFunc) (*sessions.SessionState, error) {
 	if err != nil {
 		return nil, err
 	}
-	ss, err := sessions.DecodeSessionState(ciphertext, c, false)
+
-	if err != nil {
+	return sessions.DecodeSessionState(ciphertext, c, false)
-		return t.legacyV5LoadSession(ciphertext)
-	}
-	return ss, nil
 }
 
 // clearSession uses the passed clearFunc to delete a session stored with a
@@ -203,28 +197,3 @@ func (t *ticket) makeCipher() (encryption.Cipher, error) {
 	}
 	return c, nil
 }
-
-// legacyV5LoadSession loads a Redis session created in V5 with historical logic
-//
-// TODO (@NickMeves): Remove in V7
-func (t *ticket) legacyV5LoadSession(resultBytes []byte) (*sessions.SessionState, error) {
-	block, err := aes.NewCipher(t.secret)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create a legacy AES-CFB cipher from the ticket secret: %v", err)
-	}
-
-	stream := cipher.NewCFBDecrypter(block, t.secret)
-	stream.XORKeyStream(resultBytes, resultBytes)
-
-	cfbCipher, err := encryption.NewCFBCipher(encryption.SecretBytes(t.options.Secret))
-	if err != nil {
-		return nil, err
-	}
-	legacyCipher := encryption.NewBase64Cipher(cfbCipher)
-
-	session, err := sessions.LegacyV5DecodeSessionState(string(resultBytes), legacyCipher)
-	if err != nil {
-		return nil, err
-	}
-	return session, nil
-}

pkg/sessions/persistence/ticket_test.go

@@ -1,14 +1,9 @@
 package persistence
 
 import (
-	"crypto/aes"
-	"crypto/cipher"
-	"crypto/rand"
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"io"
-	"testing"
 	"time"
 
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
@@ -153,64 +148,3 @@ var _ = Describe("Session Ticket Tests", func() {
 		})
 	})
 })
-
-// TestLegacyV5DecodeSession tests the fallback to LegacyV5DecodeSession
-// when a V5 encoded session is in Redis
-//
-// TODO (@NickMeves): Remove when this is deprecated (likely V7)
-func Test_legacyV5LoadSession(t *testing.T) {
-	testCases, _, _ := sessions.CreateLegacyV5TestCases(t)
-
-	for testName, tc := range testCases {
-		t.Run(testName, func(t *testing.T) {
-			g := NewWithT(t)
-
-			secret := make([]byte, aes.BlockSize)
-			_, err := io.ReadFull(rand.Reader, secret)
-			g.Expect(err).ToNot(HaveOccurred())
-			tckt := &ticket{
-				secret: secret,
-				options: &options.Cookie{
-					Secret: base64.RawURLEncoding.EncodeToString([]byte(sessions.LegacyV5TestSecret)),
-				},
-			}
-
-			encrypted, err := legacyStoreValue(tc.Input, tckt.secret)
-			g.Expect(err).ToNot(HaveOccurred())
-
-			ss, err := tckt.legacyV5LoadSession(encrypted)
-			if tc.Error {
-				g.Expect(err).To(HaveOccurred())
-				g.Expect(ss).To(BeNil())
-				return
-			}
-			g.Expect(err).ToNot(HaveOccurred())
-
-			// Compare sessions without *time.Time fields
-			exp := *tc.Output
-			exp.CreatedAt = nil
-			exp.ExpiresOn = nil
-			act := *ss
-			act.CreatedAt = nil
-			act.ExpiresOn = nil
-			g.Expect(exp).To(Equal(act))
-		})
-	}
-}
-
-// legacyStoreValue implements the legacy V5 Redis persistence AES-CFB value encryption
-//
-// TODO (@NickMeves): Remove when this is deprecated (likely V7)
-func legacyStoreValue(value string, ticketSecret []byte) ([]byte, error) {
-	ciphertext := make([]byte, len(value))
-	block, err := aes.NewCipher(ticketSecret)
-	if err != nil {
-		return nil, fmt.Errorf("error initiating cipher block: %v", err)
-	}
-
-	// Use secret as the Initialization Vector too, because each entry has it's own key
-	stream := cipher.NewCFBEncrypter(block, ticketSecret)
-	stream.XORKeyStream(ciphertext, []byte(value))
-
-	return ciphertext, nil
-}