From 5cbdddfb98dd82e8e9d833ff5d49cc670cbec41f Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 12 Jun 2026 21:41:29 +0100 Subject: [PATCH] fix: return access_denied to device token poll when consent is denied When a user denies the consent prompt during the OAuth 2.0 Device Authorization Grant (RFC 8628) flow, the device-code token poll kept returning authorization_pending until the device code expired, instead of access_denied. fosite already returns access_denied when the device session is in the UserCodeRejected state, but Hydra never transitioned a session into that state: on consent denial, performOAuth2DeviceVerificationFlow wrote the error to the browser and returned before reaching SetUserCodeState. verifyConsent now returns the flow alongside the error on the consent-denied path (the authorization code caller discards the flow on error, so this is safe), and performOAuth2DeviceVerificationFlow marks the associated device-code session as UserCodeRejected so the polling client receives access_denied on its next poll. Closes #4110 Signed-off-by: Pete Bacon Darwin --- consent/strategy_default.go | 8 +++- oauth2/handler.go | 29 +++++++++++++ oauth2/oauth2_device_code_test.go | 72 +++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/consent/strategy_default.go b/consent/strategy_default.go index 60764397d9..c9772a079d 100644 --- a/consent/strategy_default.go +++ b/consent/strategy_default.go @@ -636,7 +636,13 @@ func (s *defaultStrategy) verifyConsent(ctx context.Context, _ http.ResponseWrit if f.ConsentError.IsError() { f.ConsentError.SetDefaults(flow.ConsentRequestDeniedErrorName) - return nil, errors.WithStack(f.ConsentError.ToRFCError()) + // Return the flow alongside the error so device-flow callers can read + // DeviceCodeRequestID and mark the device-code session as rejected. This + // lets the polling client receive access_denied (RFC 8628 section 3.5) + // instead of waiting for the device code to expire. The authorization + // code flow caller (HandleOAuth2AuthorizationRequest) discards the flow + // on error, so returning it here is safe for that path. + return f, errors.WithStack(f.ConsentError.ToRFCError()) } if err := s.r.ConsentManager().CreateConsentSession(ctx, f); errors.Is(err, sqlcon.ErrUniqueViolation()) { diff --git a/oauth2/handler.go b/oauth2/handler.go index b17ba34478..cdb9b09bc2 100644 --- a/oauth2/handler.go +++ b/oauth2/handler.go @@ -765,6 +765,17 @@ func (h *Handler) performOAuth2DeviceVerificationFlow(w http.ResponseWriter, r * if errors.Is(err, consent.ErrUserRedirected) { return } else if err != nil { + // If the user denied the consent request, mark the associated + // device-code session as rejected so the polling client receives + // access_denied (RFC 8628 section 3.5) on its next poll instead of + // continuing to receive authorization_pending until the device code + // expires. f is non-nil here only when consent was denied (see + // DefaultStrategy.verifyConsent); other errors return a nil flow. + if f != nil && f.DeviceCodeRequestID.String() != "" && f.ConsentError.IsError() { + if rejectErr := h.markDeviceUserCodeRejected(ctx, f.DeviceCodeRequestID.String()); rejectErr != nil { + x.LogError(r, rejectErr, h.r.Logger()) + } + } x.LogError(r, err, h.r.Logger()) h.r.Writer().WriteError(w, r, err) return @@ -815,6 +826,24 @@ func (h *Handler) performOAuth2DeviceVerificationFlow(w http.ResponseWriter, r * http.Redirect(w, r, redirectURL, http.StatusFound) } +// markDeviceUserCodeRejected transitions the device-code session identified by +// deviceCodeRequestID into the UserCodeRejected state. It is called when the +// user denies the device authorization request during the verification flow so +// that the device's token poll returns access_denied (RFC 8628 section 3.5) +// rather than continuing to return authorization_pending until the device code +// expires. This mirrors the UserCodeAccepted transition performed on the +// accept path above. +func (h *Handler) markDeviceUserCodeRejected(ctx context.Context, deviceCodeRequestID string) error { + req, signature, err := h.r.OAuth2Storage().GetDeviceCodeSessionByRequestID(ctx, deviceCodeRequestID, &Session{}) + if err != nil { + return err + } + + req.SetUserCodeState(fosite.UserCodeRejected) + + return h.r.OAuth2Storage().UpdateDeviceCodeSessionBySignature(ctx, signature, req) +} + // OAuth2 Device Flow // // # Ory's OAuth 2.0 Device Authorization API diff --git a/oauth2/oauth2_device_code_test.go b/oauth2/oauth2_device_code_test.go index e3b9e7313e..54bb1f35c8 100644 --- a/oauth2/oauth2_device_code_test.go +++ b/oauth2/oauth2_device_code_test.go @@ -5,7 +5,9 @@ package oauth2_test import ( "context" + "io" "net/http" + "net/url" "strings" "testing" "time" @@ -451,6 +453,56 @@ func TestDeviceCodeWithDefaultStrategy(t *testing.T) { assert.Empty(t, token.Extra("id_token")) assert.Empty(t, token.RefreshToken) }) + t.Run("case=polling client receives access_denied when consent is denied", func(t *testing.T) { + c, conf := newDeviceClient(t, reg) + conf.Scopes = []string{"hydra"} + + rejectConsentHandler := func(w http.ResponseWriter, r *http.Request) { + vr, _, err := adminClient.OAuth2API.RejectOAuth2ConsentRequest(context.Background()). + ConsentChallenge(r.URL.Query().Get("consent_challenge")). + RejectOAuth2Request(hydra.RejectOAuth2Request{ + Error: new(fosite.ErrAccessDenied.ErrorField), + ErrorDescription: new("user denied the device authorization request"), + StatusCode: new(int64(fosite.ErrAccessDenied.CodeField)), + }). + Execute() + require.NoError(t, err) + require.NotEmpty(t, vr.RedirectTo) + http.Redirect(w, r, vr.RedirectTo, http.StatusFound) + } + + testhelpers.NewDeviceLoginConsentUI(t, reg.Config(), + acceptDeviceHandler(t, c), + acceptLoginHandler(t, c, subject, conf.Scopes, nil), + rejectConsentHandler, + ) + + resp, err := getDeviceCode(t, conf, nil) + require.NoError(t, err) + require.NotEmpty(t, resp.DeviceCode) + require.NotEmpty(t, resp.UserCode) + + // Drive the browser through device accept -> login -> consent denial. + // The user agent ends at an error page rather than the device-done URL. + hc := testhelpers.NewEmptyJarClient(t) + browserResp, err := hc.Get(resp.VerificationURIComplete) + require.NoError(t, err) + require.NoError(t, browserResp.Body.Close()) + + // The polling client must now receive access_denied on its next poll, + // instead of continuing to receive authorization_pending until the + // device code expires. + tokenResp := pollDeviceToken(t, conf, resp.DeviceCode) + body, err := io.ReadAll(tokenResp.Body) + require.NoError(t, err) + require.NoError(t, tokenResp.Body.Close()) + + // The key assertion: the polling client receives the access_denied error + // code (RFC 8628 section 3.5) rather than authorization_pending. fosite + // serialises ErrAccessDenied with HTTP 403. + assert.Equal(t, http.StatusForbidden, tokenResp.StatusCode, "body: %s", body) + assert.Equal(t, "access_denied", gjson.GetBytes(body, "error").String(), "body: %s", body) + }) t.Run("case=perform device flow with ID token", func(t *testing.T) { c, conf := newDeviceClient(t, reg) conf.Scopes = []string{"openid", "hydra"} @@ -873,3 +925,23 @@ func newDeviceClient( Scopes: strings.Split(c.Scope, " "), } } + +// pollDeviceToken performs a single device-code token poll (RFC 8628 section 3.4) +// against the token endpoint and returns the raw response. It deliberately does +// not loop on authorization_pending, so callers can assert on the exact response +// to a single poll (e.g. access_denied after the user denied consent) without +// risking a hang until the device code expires. +func pollDeviceToken(t *testing.T, conf *oauth2.Config, deviceCode string) *http.Response { + form := url.Values{} + form.Set("grant_type", string(fosite.GrantTypeDeviceCode)) + form.Set("device_code", deviceCode) + form.Set("client_id", conf.ClientID) + + req, err := http.NewRequest(http.MethodPost, conf.Endpoint.TokenURL, strings.NewReader(form.Encode())) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + return resp +}