diff --git a/CHANGELOG.md b/CHANGELOG.md index 46ec8c5a..b5e0bd08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,13 @@ ## Changes since v7.1.3 +- [#1063](https://github.com/oauth2-proxy/oauth2-proxy/pull/1063) Add Redis lock feature to lock persistent sessions (@Bibob7) - [#1108](https://github.com/oauth2-proxy/oauth2-proxy/pull/1108) Add alternative ways to generate cookie secrets to docs (@JoelSpeed) - [#1142](https://github.com/oauth2-proxy/oauth2-proxy/pull/1142) Add pagewriter to upstream proxy (@JoelSpeed) - [#1181](https://github.com/oauth2-proxy/oauth2-proxy/pull/1181) Fix incorrect `cfg` name in show-debug-on-error flag (@iTaybb) - [#1207](https://github.com/oauth2-proxy/oauth2-proxy/pull/1207) Fix URI fragment handling on sign-in page, regression introduced in 7.1.0 (@tarvip) + # V7.1.3 ## Release Highlights diff --git a/go.mod b/go.mod index b4ced24b..174484df 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/benbjohnson/clock v1.1.1-0.20210213131748-c97fc7b6bee0 github.com/bitly/go-simplejson v0.5.0 github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 // indirect + github.com/bsm/redislock v0.7.0 github.com/coreos/go-oidc v2.2.1+incompatible github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/frankban/quicktest v1.10.0 // indirect diff --git a/go.sum b/go.sum index d1f2d89a..bc2f7038 100644 --- a/go.sum +++ b/go.sum @@ -2,10 +2,12 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMT cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.38.0 h1:ROfEUZz+Gh5pa62DJWXSaonyu3StP6EA6lPEXPI6mCo= cloud.google.com/go v0.38.0/go.mod h1:990N+gfupTy94rShfmMCWGDn0LpTmnzTp2qbd1dvSRU= +dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= github.com/Bose/minisentinel v0.0.0-20200130220412-917c5a9223bb h1:ZVN4Iat3runWOFLaBCDVU5a9X/XikSRBosye++6gojw= github.com/Bose/minisentinel v0.0.0-20200130220412-917c5a9223bb/go.mod h1:WsAABbY4HQBgd3mGuG4KMNTbHJCPvx9IVBHzysbknss= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/FZambia/sentinel v1.0.0 h1:KJ0ryjKTZk5WMp0dXvSdNqp3lFaW1fNFuEYfrkLOYIc= github.com/FZambia/sentinel v1.0.0/go.mod h1:ytL1Am/RLlAoAXG6Kj5LNuw/TRRQrv2rt2FT26vP5gI= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= @@ -49,6 +51,8 @@ github.com/bitly/go-simplejson v0.5.0 h1:6IH+V8/tVMab511d5bn4M7EwGXZf9Hj6i2xSwkN github.com/bitly/go-simplejson v0.5.0/go.mod h1:cXHtHw4XUPsvGaxgjIAn8PhEWG9NfngEKAMDJEczWVA= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 h1:DDGfHa7BWjL4YnC6+E63dPcxHo2sUxDIu8g3QgEJdRY= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= +github.com/bsm/redislock v0.7.0 h1:RL7aZJhCKkuBjQbnSTKCeedTRifBWxd/ffP+GZ599Mo= +github.com/bsm/redislock v0.7.0/go.mod h1:3Kgu+cXw0JrkZ5pmY/JbcFpixGZ5M9v9G2PGWYqku+k= github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ= github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -107,6 +111,7 @@ github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2H github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 h1:Mn26/9ZMNWSw9C9ERFA1PUxfmGpolnw2v0bKOREu5ew= github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32/go.mod h1:GIjDIg/heH5DOkXY3YJ/wNhfHsQHoXGjl8G8amsYQ1I= +github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.10.0/go.mod h1:xUsJbQ/Fp4kEt7AFgCuvyX4a71u8h9jB8tj/ORgOZ7o= @@ -120,6 +125,7 @@ github.com/go-openapi/jsonpointer v0.0.0-20160704185906-46af16f9f7b1/go.mod h1:+ github.com/go-openapi/jsonreference v0.0.0-20160704190145-13c6e3589ad9/go.mod h1:W3Z9FmVs9qj+KR4zFKmDPGiLdk1D9Rlm7cyMvf57TTg= github.com/go-openapi/spec v0.0.0-20160808142527-6aced65f8501/go.mod h1:J8+jY1nAiCcj+friV/PDoE1/3eeccG9LYBs0tYvLOWc= github.com/go-openapi/swag v0.0.0-20160704191624-1d0bd113de87/go.mod h1:DXUve3Dpr1UfpPtxFw+EFuQ41HhCWZfha5jSVRG7C7I= +github.com/go-redis/redis/v8 v8.1.0/go.mod h1:isLoQT/NFSP7V67lyvM9GmdvLdyZ7pEhsXvvyQtnQTo= github.com/go-redis/redis/v8 v8.2.3 h1:eNesND+DWt/sjQOtPFxAbQkTIXaXX00qNLxjVWkZ70k= github.com/go-redis/redis/v8 v8.2.3/go.mod h1:ysgGY09J/QeDYbu3HikWEIPCwaeOkuNoTgKayTEaEOw= github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= @@ -325,6 +331,7 @@ github.com/pierrec/lz4 v2.5.2+incompatible h1:WCjObylUIOlKy/+7Abdn34TLIkXiA4UWUM github.com/pierrec/lz4 v2.5.2+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/profile v1.2.1/go.mod h1:hJw3o1OdXxsrSjjVksARp5W95eeEaEfptyVZyv6JUPA= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -454,16 +461,23 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 h1:psW17arqaxU48Z5kZ0CQnkZWQJsqcURM6tKiBApRjXI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= +golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= +golang.org/x/exp v0.0.0-20200908183739-ae8ad444f925/go.mod h1:1phAWC201xIgDyaFpmDeZkgf70Q4Pd/CNqfRtVPtxNw= +golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= +golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/lint v0.0.0-20190409202823-959b441ac422/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= +golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028/go.mod h1:E/iHnbuqvinMTCcRqshq8CkpyQDoeVncDDYHnLhea+o= golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc= golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= golang.org/x/mod v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449 h1:xUIPaMhvROX9dhPvRCenIJtU78+lbEenGbgqB5hfHCQ= +golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -512,6 +526,7 @@ golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20181122145206-62eef0e2fa9b/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190204203706-41f3e6584952/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -519,6 +534,7 @@ golang.org/x/sys v0.0.0-20190507160741-ecd444e8653b/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190726091711-fc99dfbffb4e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191220142924-d4481acd189f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -555,6 +571,7 @@ golang.org/x/tools v0.0.0-20191029041327-9cc4af7d6b2c/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20191029190741-b9c20aec41a5/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200103221440-774c71fcf114/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200505023115-26f46d2f7ef8 h1:BMFHd4OFnFtWX46Xj4DN6vvT1btiBxyq+s0orYBqcQY= golang.org/x/tools v0.0.0-20200505023115-26f46d2f7ef8/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/pkg/apis/sessions/interfaces.go b/pkg/apis/sessions/interfaces.go index 34d945f3..6569f2f3 100644 --- a/pkg/apis/sessions/interfaces.go +++ b/pkg/apis/sessions/interfaces.go @@ -1,7 +1,10 @@ package sessions import ( + "context" + "errors" "net/http" + "time" ) // SessionStore is an interface to storing user sessions in the proxy @@ -10,3 +13,24 @@ type SessionStore interface { Load(req *http.Request) (*SessionState, error) Clear(rw http.ResponseWriter, req *http.Request) error } + +var ErrLockNotObtained = errors.New("lock: not obtained") +var ErrNotLocked = errors.New("tried to release not existing lock") + +// Lock is an interface for controlling session locks +type Lock interface { + // Obtain obtains the lock on the distributed + // lock resource if no lock exists yet. + // Otherwise it will return ErrLockNotObtained + Obtain(ctx context.Context, expiration time.Duration) error + // Peek returns true if the lock currently exists + // Otherwise it returns false. + Peek(ctx context.Context) (bool, error) + // Refresh refreshes the expiration time of the lock, + // if is still applied. + // Otherwise it will return ErrNotLocked + Refresh(ctx context.Context, expiration time.Duration) error + // Release removes the existing lock, + // Otherwise it will return ErrNotLocked + Release(ctx context.Context) error +} diff --git a/pkg/apis/sessions/lock.go b/pkg/apis/sessions/lock.go new file mode 100644 index 00000000..dd080770 --- /dev/null +++ b/pkg/apis/sessions/lock.go @@ -0,0 +1,24 @@ +package sessions + +import ( + "context" + "time" +) + +type NoOpLock struct{} + +func (l *NoOpLock) Obtain(ctx context.Context, expiration time.Duration) error { + return nil +} + +func (l *NoOpLock) Peek(ctx context.Context) (bool, error) { + return false, nil +} + +func (l *NoOpLock) Refresh(ctx context.Context, expiration time.Duration) error { + return nil +} + +func (l *NoOpLock) Release(ctx context.Context) error { + return nil +} diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go index c3441fe3..e1ee4a6c 100644 --- a/pkg/apis/sessions/session_state.go +++ b/pkg/apis/sessions/session_state.go @@ -2,6 +2,7 @@ package sessions import ( "bytes" + "context" "errors" "fmt" "io" @@ -30,6 +31,36 @@ type SessionState struct { User string `msgpack:"u,omitempty"` Groups []string `msgpack:"g,omitempty"` PreferredUsername string `msgpack:"pu,omitempty"` + + Lock Lock `msgpack:"-"` +} + +func (s *SessionState) ObtainLock(ctx context.Context, expiration time.Duration) error { + if s.Lock == nil { + s.Lock = &NoOpLock{} + } + return s.Lock.Obtain(ctx, expiration) +} + +func (s *SessionState) RefreshLock(ctx context.Context, expiration time.Duration) error { + if s.Lock == nil { + s.Lock = &NoOpLock{} + } + return s.Lock.Refresh(ctx, expiration) +} + +func (s *SessionState) ReleaseLock(ctx context.Context) error { + if s.Lock == nil { + s.Lock = &NoOpLock{} + } + return s.Lock.Release(ctx) +} + +func (s *SessionState) PeekLock(ctx context.Context) (bool, error) { + if s.Lock == nil { + s.Lock = &NoOpLock{} + } + return s.Lock.Peek(ctx) } // IsExpired checks whether the session has expired diff --git a/pkg/middleware/stored_session_test.go b/pkg/middleware/stored_session_test.go index 0ab6c994..3d8dd087 100644 --- a/pkg/middleware/stored_session_test.go +++ b/pkg/middleware/stored_session_test.go @@ -101,7 +101,7 @@ var _ = Describe("Stored Session Suite", func() { Session: in.existingSession, } - // Set up the request with the request headesr and a request scope + // Set up the request with the request header and a request scope req := httptest.NewRequest("", "/", nil) req.Header = in.requestHeaders req = middlewareapi.AddRequestScope(req, scope) diff --git a/pkg/sessions/persistence/interfaces.go b/pkg/sessions/persistence/interfaces.go index cd983b4c..b1f50da5 100644 --- a/pkg/sessions/persistence/interfaces.go +++ b/pkg/sessions/persistence/interfaces.go @@ -3,6 +3,8 @@ package persistence import ( "context" "time" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" ) // Store is used for persistent session stores (IE not Cookie) @@ -12,4 +14,5 @@ type Store interface { Save(context.Context, string, []byte, time.Duration) error Load(context.Context, string) ([]byte, error) Clear(context.Context, string) error + Lock(key string) sessions.Lock } diff --git a/pkg/sessions/persistence/manager.go b/pkg/sessions/persistence/manager.go index fc621a81..49225171 100644 --- a/pkg/sessions/persistence/manager.go +++ b/pkg/sessions/persistence/manager.go @@ -60,9 +60,12 @@ func (m *Manager) Load(req *http.Request) (*sessions.SessionState, error) { return nil, err } - return tckt.loadSession(func(key string) ([]byte, error) { - return m.Store.Load(req.Context(), key) - }) + return tckt.loadSession( + func(key string) ([]byte, error) { + return m.Store.Load(req.Context(), key) + }, + m.Store.Lock, + ) } // Clear clears any saved session information for a given ticket cookie. diff --git a/pkg/sessions/persistence/ticket.go b/pkg/sessions/persistence/ticket.go index 020f35e9..70bb58a0 100644 --- a/pkg/sessions/persistence/ticket.go +++ b/pkg/sessions/persistence/ticket.go @@ -30,6 +30,10 @@ type loadFunc func(string) ([]byte, error) // a string key for the target of the deletion. type clearFunc func(string) error +// initLockFunc returns a lock object for a persistent store using a +// string key +type initLockFunc func(string) sessions.Lock + // ticket is a structure representing the ticket used in server based // session storage. It provides a unique per session decryption secret giving // more security than the shared CookieSecret. @@ -122,7 +126,8 @@ 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. -func (t *ticket) loadSession(loader loadFunc) (*sessions.SessionState, error) { +// finally it appends a lock implementation +func (t *ticket) loadSession(loader loadFunc, initLock initLockFunc) (*sessions.SessionState, error) { ciphertext, err := loader(t.id) if err != nil { return nil, fmt.Errorf("failed to load the session state with the ticket: %v", err) @@ -132,7 +137,13 @@ func (t *ticket) loadSession(loader loadFunc) (*sessions.SessionState, error) { return nil, err } - return sessions.DecodeSessionState(ciphertext, c, false) + sessionState, err := sessions.DecodeSessionState(ciphertext, c, false) + if err != nil { + return nil, err + } + lock := initLock(t.id) + sessionState.Lock = lock + return sessionState, nil } // clearSession uses the passed clearFunc to delete a session stored with a diff --git a/pkg/sessions/persistence/ticket_test.go b/pkg/sessions/persistence/ticket_test.go index 001a306f..6b868b90 100644 --- a/pkg/sessions/persistence/ticket_test.go +++ b/pkg/sessions/persistence/ticket_test.go @@ -103,10 +103,17 @@ var _ = Describe("Session Ticket Tests", func() { c, err := t.makeCipher() Expect(err).ToNot(HaveOccurred()) - ss := &sessions.SessionState{User: "foobar"} - loadedSession, err := t.loadSession(func(k string) ([]byte, error) { - return ss.EncodeSessionState(c, false) - }) + ss := &sessions.SessionState{ + User: "foobar", + Lock: &sessions.NoOpLock{}, + } + loadedSession, err := t.loadSession( + func(k string) ([]byte, error) { + return ss.EncodeSessionState(c, false) + }, + func(k string) sessions.Lock { + return &sessions.NoOpLock{} + }) Expect(err).ToNot(HaveOccurred()) Expect(loadedSession).To(Equal(ss)) }) @@ -115,9 +122,13 @@ var _ = Describe("Session Ticket Tests", func() { t, err := newTicket(&options.Cookie{Name: "dummy"}) Expect(err).ToNot(HaveOccurred()) - data, err := t.loadSession(func(k string) ([]byte, error) { - return nil, errors.New("load error") - }) + data, err := t.loadSession( + func(k string) ([]byte, error) { + return nil, errors.New("load error") + }, + func(k string) sessions.Lock { + return &sessions.NoOpLock{} + }) Expect(data).To(BeNil()) Expect(err).To(MatchError(errors.New("failed to load the session state with the ticket: load error"))) }) diff --git a/pkg/sessions/redis/client.go b/pkg/sessions/redis/client.go index 3d312b34..d17aebbe 100644 --- a/pkg/sessions/redis/client.go +++ b/pkg/sessions/redis/client.go @@ -5,11 +5,13 @@ import ( "time" "github.com/go-redis/redis/v8" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" ) // Client is wrapper interface for redis.Client and redis.ClusterClient. type Client interface { Get(ctx context.Context, key string) ([]byte, error) + Lock(key string) sessions.Lock Set(ctx context.Context, key string, value []byte, expiration time.Duration) error Del(ctx context.Context, key string) error } @@ -21,7 +23,9 @@ type client struct { } func newClient(c *redis.Client) Client { - return &client{Client: c} + return &client{ + Client: c, + } } func (c *client) Get(ctx context.Context, key string) ([]byte, error) { @@ -36,6 +40,10 @@ func (c *client) Del(ctx context.Context, key string) error { return c.Client.Del(ctx, key).Err() } +func (c *client) Lock(key string) sessions.Lock { + return NewLock(c.Client, key) +} + var _ Client = (*clusterClient)(nil) type clusterClient struct { @@ -43,7 +51,9 @@ type clusterClient struct { } func newClusterClient(c *redis.ClusterClient) Client { - return &clusterClient{ClusterClient: c} + return &clusterClient{ + ClusterClient: c, + } } func (c *clusterClient) Get(ctx context.Context, key string) ([]byte, error) { @@ -57,3 +67,7 @@ func (c *clusterClient) Set(ctx context.Context, key string, value []byte, expir func (c *clusterClient) Del(ctx context.Context, key string) error { return c.ClusterClient.Del(ctx, key).Err() } + +func (c *clusterClient) Lock(key string) sessions.Lock { + return NewLock(c.ClusterClient, key) +} diff --git a/pkg/sessions/redis/lock.go b/pkg/sessions/redis/lock.go new file mode 100644 index 00000000..91da0e97 --- /dev/null +++ b/pkg/sessions/redis/lock.go @@ -0,0 +1,84 @@ +package redis + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/bsm/redislock" + "github.com/go-redis/redis/v8" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" +) + +const LockSuffix = "lock" + +type Lock struct { + client redis.Cmdable + locker *redislock.Client + lock *redislock.Lock + key string +} + +// NewLock instantiate a new lock instance. This will not yet apply a lock on Redis side. +// For that you have to call Obtain(ctx context.Context, expiration time.Duration) +func NewLock(client redis.Cmdable, key string) sessions.Lock { + return &Lock{ + client: client, + locker: redislock.New(client), + key: key, + } +} + +// Obtain obtains a distributed lock on Redis for the configured key. +func (l *Lock) Obtain(ctx context.Context, expiration time.Duration) error { + lock, err := l.locker.Obtain(ctx, l.lockKey(), expiration, nil) + if errors.Is(err, redislock.ErrNotObtained) { + return sessions.ErrLockNotObtained + } + if err != nil { + return err + } + l.lock = lock + return nil +} + +// Refresh refreshes an already existing lock. +func (l *Lock) Refresh(ctx context.Context, expiration time.Duration) error { + if l.lock == nil { + return sessions.ErrNotLocked + } + err := l.lock.Refresh(ctx, expiration, nil) + if errors.Is(err, redislock.ErrNotObtained) { + return sessions.ErrNotLocked + } + return err +} + +// Peek returns true, if the lock is still applied. +func (l *Lock) Peek(ctx context.Context) (bool, error) { + v, err := l.client.Exists(ctx, l.lockKey()).Result() + if err != nil { + return false, err + } + if v == 0 { + return false, nil + } + return true, nil +} + +// Release releases the lock on Redis side. +func (l *Lock) Release(ctx context.Context) error { + if l.lock == nil { + return sessions.ErrNotLocked + } + err := l.lock.Release(ctx) + if errors.Is(err, redislock.ErrLockNotHeld) { + return sessions.ErrNotLocked + } + return err +} + +func (l *Lock) lockKey() string { + return fmt.Sprintf("%s.%s", l.key, LockSuffix) +} diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index 5de5ce5a..5beee8cb 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/pkg/sessions/redis/redis_store.go @@ -35,7 +35,7 @@ func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.Cook } // Save takes a sessions.SessionState and stores the information from it -// to redies, and adds a new persistence cookie on the HTTP response writer +// to redis, and adds a new persistence cookie on the HTTP response writer func (store *SessionStore) Save(ctx context.Context, key string, value []byte, exp time.Duration) error { err := store.Client.Set(ctx, key, value, exp) if err != nil { @@ -64,6 +64,11 @@ func (store *SessionStore) Clear(ctx context.Context, key string) error { return nil } +// Lock creates a lock object for sessions.SessionState +func (store *SessionStore) Lock(key string) sessions.Lock { + return store.Client.Lock(key) +} + // NewRedisClient makes a redis.Client (either standalone, sentinel aware, or // redis cluster) func NewRedisClient(opts options.RedisStoreOptions) (Client, error) { @@ -151,7 +156,7 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) { } // parseRedisURLs parses a list of redis urls and returns a list -// of addresses in the form of host:port that can be used to connnect to Redis +// of addresses in the form of host:port that can be used to connect to Redis func parseRedisURLs(urls []string) ([]string, error) { addrs := []string{} for _, u := range urls { diff --git a/pkg/sessions/tests/mock_lock.go b/pkg/sessions/tests/mock_lock.go new file mode 100644 index 00000000..dbf3d638 --- /dev/null +++ b/pkg/sessions/tests/mock_lock.go @@ -0,0 +1,48 @@ +package tests + +import ( + "context" + "time" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" +) + +type MockLock struct { + expiration time.Duration + elapsed time.Duration +} + +func (l *MockLock) Obtain(ctx context.Context, expiration time.Duration) error { + l.expiration = expiration + return nil +} + +func (l *MockLock) Peek(ctx context.Context) (bool, error) { + if l.elapsed < l.expiration { + return true, nil + } + return false, nil +} + +func (l *MockLock) Refresh(ctx context.Context, expiration time.Duration) error { + if l.expiration <= l.elapsed { + return sessions.ErrNotLocked + } + l.expiration = expiration + l.elapsed = time.Duration(0) + return nil +} + +func (l *MockLock) Release(ctx context.Context) error { + if l.expiration <= l.elapsed { + return sessions.ErrNotLocked + } + l.expiration = time.Duration(0) + l.elapsed = time.Duration(0) + return nil +} + +// FastForward simulates the flow of time to test expirations +func (l *MockLock) FastForward(duration time.Duration) { + l.elapsed += duration +} diff --git a/pkg/sessions/tests/mock_store.go b/pkg/sessions/tests/mock_store.go index 1ddca76e..0ea66145 100644 --- a/pkg/sessions/tests/mock_store.go +++ b/pkg/sessions/tests/mock_store.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "time" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" ) // entry is a MockStore cache entry with an expiration @@ -15,15 +17,17 @@ type entry struct { // MockStore is a generic in-memory implementation of persistence.Store // for mocking in tests type MockStore struct { - cache map[string]entry - elapsed time.Duration + cache map[string]entry + lockCache map[string]*MockLock + elapsed time.Duration } // NewMockStore creates a MockStore func NewMockStore() *MockStore { return &MockStore{ - cache: map[string]entry{}, - elapsed: 0 * time.Second, + cache: map[string]entry{}, + lockCache: map[string]*MockLock{}, + elapsed: 0 * time.Second, } } @@ -52,7 +56,19 @@ func (s *MockStore) Clear(_ context.Context, key string) error { return nil } +func (s *MockStore) Lock(key string) sessions.Lock { + if s.lockCache[key] != nil { + return s.lockCache[key] + } + lock := &MockLock{} + s.lockCache[key] = lock + return lock +} + // FastForward simulates the flow of time to test expirations func (s *MockStore) FastForward(duration time.Duration) { + for _, mockLock := range s.lockCache { + mockLock.FastForward(duration) + } s.elapsed += duration } diff --git a/pkg/sessions/tests/session_store_tests.go b/pkg/sessions/tests/session_store_tests.go index df90c7f3..416969a3 100644 --- a/pkg/sessions/tests/session_store_tests.go +++ b/pkg/sessions/tests/session_store_tests.go @@ -286,6 +286,78 @@ func PersistentSessionStoreInterfaceTests(in *testInput) { }) }) }) + + Context("when lock is applied", func() { + var loadedSession *sessionsapi.SessionState + BeforeEach(func() { + resp := httptest.NewRecorder() + err := in.ss().Save(resp, in.request, in.session) + Expect(err).ToNot(HaveOccurred()) + + for _, cookie := range resp.Result().Cookies() { + in.request.AddCookie(cookie) + } + + loadedSession, err = in.ss().Load(in.request) + Expect(err).ToNot(HaveOccurred()) + err = loadedSession.ObtainLock(in.request.Context(), 2*time.Minute) + Expect(err).ToNot(HaveOccurred()) + isLocked, err := loadedSession.PeekLock(in.request.Context()) + Expect(err).ToNot(HaveOccurred()) + Expect(isLocked).To(BeTrue()) + }) + + Context("before lock expired", func() { + BeforeEach(func() { + Expect(in.persistentFastForward(time.Minute)).To(Succeed()) + }) + + It("peek returns true on loaded session lock", func() { + l := *loadedSession + isLocked, err := l.PeekLock(in.request.Context()) + + Expect(err).NotTo(HaveOccurred()) + Expect(isLocked).To(BeTrue()) + }) + + It("lock can be released", func() { + l := *loadedSession + + err := l.ReleaseLock(in.request.Context()) + Expect(err).NotTo(HaveOccurred()) + + isLocked, err := l.PeekLock(in.request.Context()) + Expect(err).NotTo(HaveOccurred()) + Expect(isLocked).To(BeFalse()) + }) + + It("lock is refreshed", func() { + l := *loadedSession + err := l.RefreshLock(in.request.Context(), 3*time.Minute) + Expect(err).NotTo(HaveOccurred()) + + Expect(in.persistentFastForward(2 * time.Minute)).To(Succeed()) + + isLocked, err := l.PeekLock(in.request.Context()) + Expect(err).NotTo(HaveOccurred()) + Expect(isLocked).To(BeTrue()) + }) + }) + + Context("after lock expired", func() { + BeforeEach(func() { + Expect(in.persistentFastForward(3 * time.Minute)).To(Succeed()) + }) + + It("peek returns false on loaded session lock", func() { + l := *loadedSession + isLocked, err := l.PeekLock(in.request.Context()) + + Expect(err).NotTo(HaveOccurred()) + Expect(isLocked).To(BeFalse()) + }) + }) + }) } func SessionStoreInterfaceTests(in *testInput) { @@ -411,9 +483,11 @@ func LoadSessionTests(in *testInput) { l := *loadedSession l.CreatedAt = nil l.ExpiresOn = nil + l.Lock = &sessionsapi.NoOpLock{} s := *in.session s.CreatedAt = nil s.ExpiresOn = nil + s.Lock = &sessionsapi.NoOpLock{} Expect(l).To(Equal(s)) // Compare time.Time separately