From 58545a1710cd9720289ac9529be0fe36aca7efc7 Mon Sep 17 00:00:00 2001 From: Livio Amstutz Date: Fri, 29 May 2020 09:40:34 +0200 Subject: [PATCH 1/4] fix: handle code separately (#30) --- example/internal/mock/storage.go | 13 ++++++++++- pkg/op/authrequest.go | 13 ++++++++++- pkg/op/mock/signer.mock.go | 6 +++--- pkg/op/mock/storage.mock.go | 37 ++++++++++++++++++++++++++++---- pkg/op/storage.go | 2 ++ pkg/op/token.go | 5 +++++ pkg/op/tokenrequest.go | 17 ++------------- 7 files changed, 69 insertions(+), 24 deletions(-) diff --git a/example/internal/mock/storage.go b/example/internal/mock/storage.go index 96f9b45..5fb823b 100644 --- a/example/internal/mock/storage.go +++ b/example/internal/mock/storage.go @@ -110,6 +110,7 @@ func (a *AuthRequest) Done() bool { var ( a = &AuthRequest{} t bool + c string ) func (s *AuthStorage) Health(ctx context.Context) error { @@ -127,9 +128,19 @@ func (s *AuthStorage) CreateAuthRequest(_ context.Context, authReq *oidc.AuthReq t = false return a, nil } -func (s *AuthStorage) AuthRequestByCode(context.Context, string) (op.AuthRequest, error) { +func (s *AuthStorage) AuthRequestByCode(_ context.Context, code string) (op.AuthRequest, error) { + if code != c { + return nil, errors.New("invalid code") + } return a, nil } +func (s *AuthStorage) SaveAuthCode(_ context.Context, id, code string) error { + if a.ID != id { + return errors.New("not found") + } + c = code + return nil +} func (s *AuthStorage) DeleteAuthRequest(context.Context, string) error { t = true return nil diff --git a/pkg/op/authrequest.go b/pkg/op/authrequest.go index e01de51..c25f60d 100644 --- a/pkg/op/authrequest.go +++ b/pkg/op/authrequest.go @@ -173,7 +173,7 @@ func AuthResponse(authReq AuthRequest, authorizer Authorizer, w http.ResponseWri } func AuthResponseCode(w http.ResponseWriter, r *http.Request, authReq AuthRequest, authorizer Authorizer) { - code, err := BuildAuthRequestCode(authReq, authorizer.Crypto()) + code, err := CreateAuthRequestCode(r.Context(), authReq, authorizer.Storage(), authorizer.Crypto()) if err != nil { AuthRequestError(w, r, authReq, err, authorizer.Encoder()) return @@ -201,6 +201,17 @@ func AuthResponseToken(w http.ResponseWriter, r *http.Request, authReq AuthReque http.Redirect(w, r, callback, http.StatusFound) } +func CreateAuthRequestCode(ctx context.Context, authReq AuthRequest, storage Storage, crypto Crypto) (string, error) { + code, err := BuildAuthRequestCode(authReq, crypto) + if err != nil { + return "", err + } + if err := storage.SaveAuthCode(ctx, authReq.GetID(), code); err != nil { + return "", err + } + return code, nil +} + func BuildAuthRequestCode(authReq AuthRequest, crypto Crypto) (string, error) { return crypto.Encrypt(authReq.GetID()) } diff --git a/pkg/op/mock/signer.mock.go b/pkg/op/mock/signer.mock.go index c780752..a7d909c 100644 --- a/pkg/op/mock/signer.mock.go +++ b/pkg/op/mock/signer.mock.go @@ -8,7 +8,7 @@ import ( context "context" oidc "github.com/caos/oidc/pkg/oidc" gomock "github.com/golang/mock/gomock" - go_jose_v2 "gopkg.in/square/go-jose.v2" + jose "gopkg.in/square/go-jose.v2" reflect "reflect" ) @@ -80,10 +80,10 @@ func (mr *MockSignerMockRecorder) SignIDToken(arg0 interface{}) *gomock.Call { } // SignatureAlgorithm mocks base method -func (m *MockSigner) SignatureAlgorithm() go_jose_v2.SignatureAlgorithm { +func (m *MockSigner) SignatureAlgorithm() jose.SignatureAlgorithm { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SignatureAlgorithm") - ret0, _ := ret[0].(go_jose_v2.SignatureAlgorithm) + ret0, _ := ret[0].(jose.SignatureAlgorithm) return ret0 } diff --git a/pkg/op/mock/storage.mock.go b/pkg/op/mock/storage.mock.go index 405865c..ac8ba27 100644 --- a/pkg/op/mock/storage.mock.go +++ b/pkg/op/mock/storage.mock.go @@ -9,7 +9,7 @@ import ( oidc "github.com/caos/oidc/pkg/oidc" op "github.com/caos/oidc/pkg/op" gomock "github.com/golang/mock/gomock" - go_jose_v2 "gopkg.in/square/go-jose.v2" + jose "gopkg.in/square/go-jose.v2" reflect "reflect" time "time" ) @@ -37,6 +37,21 @@ func (m *MockStorage) EXPECT() *MockStorageMockRecorder { return m.recorder } +// AuthRequestByCode mocks base method +func (m *MockStorage) AuthRequestByCode(arg0 context.Context, arg1 string) (op.AuthRequest, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthRequestByCode", arg0, arg1) + ret0, _ := ret[0].(op.AuthRequest) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AuthRequestByCode indicates an expected call of AuthRequestByCode +func (mr *MockStorageMockRecorder) AuthRequestByCode(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthRequestByCode", reflect.TypeOf((*MockStorage)(nil).AuthRequestByCode), arg0, arg1) +} + // AuthRequestByID mocks base method func (m *MockStorage) AuthRequestByID(arg0 context.Context, arg1 string) (op.AuthRequest, error) { m.ctrl.T.Helper() @@ -127,10 +142,10 @@ func (mr *MockStorageMockRecorder) GetClientByClientID(arg0, arg1 interface{}) * } // GetKeySet mocks base method -func (m *MockStorage) GetKeySet(arg0 context.Context) (*go_jose_v2.JSONWebKeySet, error) { +func (m *MockStorage) GetKeySet(arg0 context.Context) (*jose.JSONWebKeySet, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetKeySet", arg0) - ret0, _ := ret[0].(*go_jose_v2.JSONWebKeySet) + ret0, _ := ret[0].(*jose.JSONWebKeySet) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -142,7 +157,7 @@ func (mr *MockStorageMockRecorder) GetKeySet(arg0 interface{}) *gomock.Call { } // GetSigningKey mocks base method -func (m *MockStorage) GetSigningKey(arg0 context.Context, arg1 chan<- go_jose_v2.SigningKey, arg2 chan<- error, arg3 <-chan time.Time) { +func (m *MockStorage) GetSigningKey(arg0 context.Context, arg1 chan<- jose.SigningKey, arg2 chan<- error, arg3 <-chan time.Time) { m.ctrl.T.Helper() m.ctrl.Call(m, "GetSigningKey", arg0, arg1, arg2, arg3) } @@ -197,6 +212,20 @@ func (mr *MockStorageMockRecorder) Health(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Health", reflect.TypeOf((*MockStorage)(nil).Health), arg0) } +// SaveAuthCode mocks base method +func (m *MockStorage) SaveAuthCode(arg0 context.Context, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SaveAuthCode", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// SaveAuthCode indicates an expected call of SaveAuthCode +func (mr *MockStorageMockRecorder) SaveAuthCode(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveAuthCode", reflect.TypeOf((*MockStorage)(nil).SaveAuthCode), arg0, arg1, arg2) +} + // SaveNewKeyPair mocks base method func (m *MockStorage) SaveNewKeyPair(arg0 context.Context) error { m.ctrl.T.Helper() diff --git a/pkg/op/storage.go b/pkg/op/storage.go index 4655b88..e3ef5ff 100644 --- a/pkg/op/storage.go +++ b/pkg/op/storage.go @@ -12,6 +12,8 @@ import ( type AuthStorage interface { CreateAuthRequest(context.Context, *oidc.AuthRequest, string) (AuthRequest, error) AuthRequestByID(context.Context, string) (AuthRequest, error) + AuthRequestByCode(context.Context, string) (AuthRequest, error) + SaveAuthCode(context.Context, string, string) error DeleteAuthRequest(context.Context, string) error CreateToken(context.Context, AuthRequest) (string, time.Time, error) diff --git a/pkg/op/token.go b/pkg/op/token.go index 06e9f9c..9d37788 100644 --- a/pkg/op/token.go +++ b/pkg/op/token.go @@ -29,6 +29,11 @@ func CreateTokenResponse(ctx context.Context, authReq AuthRequest, client Client return nil, err } + err = creator.Storage().DeleteAuthRequest(ctx, authReq.GetID()) + if err != nil { + return nil, err + } + exp := uint64(validity.Seconds()) return &oidc.AccessTokenResponse{ AccessToken: accessToken, diff --git a/pkg/op/tokenrequest.go b/pkg/op/tokenrequest.go index cce3564..5ef4b22 100644 --- a/pkg/op/tokenrequest.go +++ b/pkg/op/tokenrequest.go @@ -34,11 +34,6 @@ func CodeExchange(w http.ResponseWriter, r *http.Request, exchanger Exchanger) { RequestError(w, r, err) return } - err = exchanger.Storage().DeleteAuthRequest(r.Context(), authReq.GetID()) - if err != nil { - RequestError(w, r, err) - return - } resp, err := CreateTokenResponse(r.Context(), authReq, client, exchanger, true, tokenReq.Code) if err != nil { RequestError(w, r, err) @@ -96,7 +91,7 @@ func AuthorizeClient(ctx context.Context, tokenReq *oidc.AccessTokenRequest, exc if err != nil { return nil, nil, err } - authReq, err := AuthRequestByCode(ctx, tokenReq.Code, exchanger.Crypto(), exchanger.Storage()) + authReq, err := exchanger.Storage().AuthRequestByCode(ctx, tokenReq.Code) if err != nil { return nil, nil, ErrInvalidRequest("invalid code") } @@ -111,7 +106,7 @@ func AuthorizeCodeChallenge(ctx context.Context, tokenReq *oidc.AccessTokenReque if tokenReq.CodeVerifier == "" { return nil, ErrInvalidRequest("code_challenge required") } - authReq, err := AuthRequestByCode(ctx, tokenReq.Code, exchanger.Crypto(), exchanger.Storage()) + authReq, err := exchanger.Storage().AuthRequestByCode(ctx, tokenReq.Code) if err != nil { return nil, ErrInvalidRequest("invalid code") } @@ -121,14 +116,6 @@ func AuthorizeCodeChallenge(ctx context.Context, tokenReq *oidc.AccessTokenReque return authReq, nil } -func AuthRequestByCode(ctx context.Context, code string, crypto Crypto, storage AuthStorage) (AuthRequest, error) { - id, err := crypto.Decrypt(code) - if err != nil { - return nil, err - } - return storage.AuthRequestByID(ctx, id) -} - func TokenExchange(w http.ResponseWriter, r *http.Request, exchanger Exchanger) { tokenRequest, err := ParseTokenExchangeRequest(w, r) if err != nil { From 2e6c6fa8e94be561c30b863e9ee577f349619c6c Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 3 Jun 2020 13:49:32 +0200 Subject: [PATCH 2/4] chore(dependabot): config (#32) Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- .dependabot/config.yml | 8 -------- .github/dependabot.yml | 11 +++++++++++ 2 files changed, 11 insertions(+), 8 deletions(-) delete mode 100644 .dependabot/config.yml create mode 100644 .github/dependabot.yml diff --git a/.dependabot/config.yml b/.dependabot/config.yml deleted file mode 100644 index bbc39b2..0000000 --- a/.dependabot/config.yml +++ /dev/null @@ -1,8 +0,0 @@ -version: 1 -update_configs: - - package_manager: "go:modules" - directory: "/" - update_schedule: "daily" - commit_message: - prefix: "chore" - include_scope: true \ No newline at end of file diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..3d24fe9 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,11 @@ +version: 2 +updates: +- package-ecosystem: gomod + directory: "/" + schedule: + interval: daily + time: '04:00' + open-pull-requests-limit: 10 + commit-message: + prefix: chore + include: scope From 8ed8f4918cd0957b25f1814744182ada9ce16a1a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 Jul 2020 12:45:55 +0200 Subject: [PATCH 3/4] chore(deps): bump github.com/stretchr/testify from 1.5.1 to 1.6.1 (#33) Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.5.1 to 1.6.1. - [Release notes](https://github.com/stretchr/testify/releases) - [Commits](https://github.com/stretchr/testify/compare/v1.5.1...v1.6.1) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 4242267..4424352 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/gorilla/securecookie v1.1.1 github.com/kr/pretty v0.1.0 // indirect github.com/sirupsen/logrus v1.6.0 - github.com/stretchr/testify v1.5.1 + github.com/stretchr/testify v1.6.1 golang.org/x/crypto v0.0.0-20191128160524-b544559bb6d1 // indirect golang.org/x/net v0.0.0-20191126235420-ef20fe5d7933 golang.org/x/oauth2 v0.0.0-20191122200657-5d9234df094c diff --git a/go.sum b/go.sum index e2cef80..62653ed 100644 --- a/go.sum +++ b/go.sum @@ -48,6 +48,8 @@ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191128160524-b544559bb6d1 h1:anGSYQpPhQwXlwsu5wmfq0nWkCNaMEMUwAv13Y92hd8= @@ -92,6 +94,8 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.7 h1:VUgggvou5XRW9mHwD/yXxIYSMtY0zoKQf/v226p2nyo= gopkg.in/yaml.v2 v2.2.7/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= rsc.io/quote/v3 v3.1.0 h1:9JKUTTIUgS6kzR9mK1YuGKv6Nl+DijDNIc0ghT58FaY= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0 h1:7uVkIFmeBqHfdjD+gZwtXXI+RODJ2Wc4O7MPEh/QiW4= From 21dfd6c22e462e1cbfb1ef0fa3fb61eafb90408c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 Jul 2020 12:46:16 +0200 Subject: [PATCH 4/4] chore(deps): bump golang.org/x/text from 0.3.2 to 0.3.3 (#34) Bumps [golang.org/x/text](https://github.com/golang/text) from 0.3.2 to 0.3.3. - [Release notes](https://github.com/golang/text/releases) - [Commits](https://github.com/golang/text/compare/v0.3.2...v0.3.3) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 4424352..24a1b57 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( golang.org/x/crypto v0.0.0-20191128160524-b544559bb6d1 // indirect golang.org/x/net v0.0.0-20191126235420-ef20fe5d7933 golang.org/x/oauth2 v0.0.0-20191122200657-5d9234df094c - golang.org/x/text v0.3.2 + golang.org/x/text v0.3.3 google.golang.org/appengine v1.6.5 // indirect gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect gopkg.in/square/go-jose.v2 v2.5.1 diff --git a/go.sum b/go.sum index 62653ed..5af7d7e 100644 --- a/go.sum +++ b/go.sum @@ -76,6 +76,8 @@ golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fq golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= +golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=