From 1df97588a8fe84dbda4112d33285c289b03ad7f0 Mon Sep 17 00:00:00 2001 From: Nikos Date: Tue, 9 Apr 2024 16:05:00 +0300 Subject: [PATCH 1/2] fix: validate user code has not expired --- consent/handler.go | 7 +++++- consent/handler_test.go | 33 +++++++++++++++++++++++++++++ persistence/sql/persister_oauth2.go | 3 +++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/consent/handler.go b/consent/handler.go index b1d1f576df9..af95ffccf28 100644 --- a/consent/handler.go +++ b/consent/handler.go @@ -1113,11 +1113,16 @@ func (h *Handler) acceptUserCodeRequest(w http.ResponseWriter, r *http.Request, h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithHint(`'user_code' signature could not be computed`))) return } - userCodeRequest, err := h.r.OAuth2Storage().GetUserCodeSession(r.Context(), userCodeSignature, &fosite.DefaultSession{}) + userCodeRequest, err := h.r.OAuth2Storage().GetUserCodeSession(r.Context(), userCodeSignature, nil) if err != nil { h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrNotFound.WithWrap(err).WithHint(`'user_code' session not found`))) return } + err = h.r.RFC8628HMACStrategy().ValidateUserCode(ctx, userCodeRequest, reqBody.UserCode) + if err != nil { + h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrTokenExpired.WithWrap(err).WithHint(`'user_code' has expired`))) + return + } err = h.r.OAuth2Storage().UpdateAndInvalidateUserCodeSession(r.Context(), userCodeSignature, f.ID) if err != nil { diff --git a/consent/handler_test.go b/consent/handler_test.go index 35416de58ed..5195e62715e 100644 --- a/consent/handler_test.go +++ b/consent/handler_test.go @@ -16,6 +16,8 @@ import ( "github.com/stretchr/testify/require" "github.com/ory/fosite" + "github.com/ory/fosite/handler/openid" + "github.com/ory/fosite/token/jwt" hydra "github.com/ory/hydra-client-go/v2" "github.com/ory/hydra/v2/client" . "github.com/ory/hydra/v2/consent" @@ -529,6 +531,37 @@ func TestAcceptCodeDeviceRequestFailure(t *testing.T) { require.EqualValues(t, http.StatusNotFound, resp.StatusCode) }, }, + { + desc: "expired user_code", + getBody: func() ([]byte, error) { + deviceRequest := fosite.NewDeviceRequest() + deviceRequest.Client = cl + userCode, sig, err := reg.RFC8628HMACStrategy().GenerateUserCode(ctx) + require.NoError(t, err) + deviceRequest.SetSession( + &oauth2.Session{ + DefaultSession: &openid.DefaultSession{ + Headers: &jwt.Headers{}, + }, + BrowserFlowCompleted: false, + }, + ) + exp := time.Now().UTC() + deviceRequest.Session.SetExpiresAt(fosite.UserCode, exp) + err = reg.OAuth2Storage().CreateUserCodeSession(ctx, sig, deviceRequest) + require.NoError(t, err) + return json.Marshal(&hydra.AcceptDeviceUserCodeRequest{UserCode: &userCode}) + }, + getURL: func() string { + return ts.URL + "/admin" + DevicePath + "/accept?challenge=" + challenge + }, + validateResponse: func(resp *http.Response) { + require.EqualValues(t, http.StatusUnauthorized, resp.StatusCode) + result := &fosite.RFC6749Error{} + require.NoError(t, json.NewDecoder(resp.Body).Decode(&result)) + require.EqualValues(t, result.ErrorField, fosite.ErrTokenExpired.ErrorField) + }, + }, { desc: "extra fields", getBody: func() ([]byte, error) { diff --git a/persistence/sql/persister_oauth2.go b/persistence/sql/persister_oauth2.go index 4d11fffdd6a..e8422845211 100644 --- a/persistence/sql/persister_oauth2.go +++ b/persistence/sql/persister_oauth2.go @@ -613,6 +613,9 @@ func (p *Persister) CreateUserCodeSession(ctx context.Context, signature string, func (p *Persister) GetUserCodeSession(ctx context.Context, signature string, session fosite.Session) (_ fosite.Requester, err error) { ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.GetUserCodeSession") defer otelx.End(span, &err) + if session == nil { + session = oauth2.NewSession("") + } return p.findSessionBySignature(ctx, signature, session, sqlTableUserCode) } From 372c4aa1994bce2bc2711f4922b6b033c0008c5f Mon Sep 17 00:00:00 2001 From: Nikos Date: Wed, 10 Apr 2024 14:12:29 +0300 Subject: [PATCH 2/2] fixup! 17957f5e7327fb6f724e7e31fead9a7c161e87fe --- consent/handler.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/consent/handler.go b/consent/handler.go index af95ffccf28..d17d6542680 100644 --- a/consent/handler.go +++ b/consent/handler.go @@ -1091,6 +1091,11 @@ func (h *Handler) acceptUserCodeRequest(w http.ResponseWriter, r *http.Request, return } + if reqBody.UserCode == "" { + h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint("Field 'user_code' must not be empty."))) + return + } + cr, err := h.r.ConsentManager().GetDeviceUserAuthRequest(ctx, challenge) if err != nil { h.r.Writer().WriteError(w, r, errorsx.WithStack(err)) @@ -1103,11 +1108,6 @@ func (h *Handler) acceptUserCodeRequest(w http.ResponseWriter, r *http.Request, return } - if reqBody.UserCode == "" { - h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint("Field 'user_code' must not be empty."))) - return - } - userCodeSignature, err := h.r.RFC8628HMACStrategy().UserCodeSignature(r.Context(), reqBody.UserCode) if err != nil { h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithHint(`'user_code' signature could not be computed`)))