You've already forked oauth2-proxy
							
							
				mirror of
				https://github.com/oauth2-proxy/oauth2-proxy.git
				synced 2025-10-30 23:47:52 +02:00 
			
		
		
		
	Merge pull request #905 from grnhse/deprecate-legacy-sessions
Remove v5 JSON session support
This commit is contained in:
		| @@ -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) | ||||
|   | ||||
| @@ -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 | ||||
| } | ||||
| @@ -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"` | ||||
| 	IDToken           string     `json:",omitempty" msgpack:"it,omitempty"` | ||||
| 	CreatedAt         *time.Time `json:",omitempty" msgpack:"ca,omitempty"` | ||||
| 	ExpiresOn         *time.Time `json:",omitempty" msgpack:"eo,omitempty"` | ||||
| 	RefreshToken      string     `json:",omitempty" msgpack:"rt,omitempty"` | ||||
| 	Email             string     `json:",omitempty" msgpack:"e,omitempty"` | ||||
| 	User              string     `json:",omitempty" msgpack:"u,omitempty"` | ||||
| 	Groups            []string   `json:",omitempty" msgpack:"g,omitempty"` | ||||
| 	PreferredUsername string     `json:",omitempty" msgpack:"pu,omitempty"` | ||||
| 	CreatedAt *time.Time `msgpack:"ca,omitempty"` | ||||
| 	ExpiresOn *time.Time `msgpack:"eo,omitempty"` | ||||
|  | ||||
| 	AccessToken  string `msgpack:"at,omitempty"` | ||||
| 	IDToken      string `msgpack:"it,omitempty"` | ||||
| 	RefreshToken string `msgpack:"rt,omitempty"` | ||||
|  | ||||
| 	Email             string   `msgpack:"e,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 | ||||
|   | ||||
| @@ -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) | ||||
|   | ||||
| @@ -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) | ||||
|   | ||||
| @@ -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 t.legacyV5LoadSession(ciphertext) | ||||
| 	} | ||||
| 	return ss, nil | ||||
|  | ||||
| 	return sessions.DecodeSessionState(ciphertext, c, false) | ||||
| } | ||||
|  | ||||
| // 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 | ||||
| } | ||||
|   | ||||
| @@ -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 | ||||
| } | ||||
|   | ||||
		Reference in New Issue
	
	Block a user