Skip to content

Commit 4441c17

Browse files
terevterev
committed
feat: return HTTP 410 and initial auth url for consent app to redirect the user agent to when an expired challenge is supplied
1 parent 9ba07a7 commit 4441c17

File tree

5 files changed

+429
-10
lines changed

5 files changed

+429
-10
lines changed

‎consent/handler.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,13 @@ func (h *Handler) getOAuth2LoginRequest(w http.ResponseWriter, r *http.Request,
354354

355355
request, err := h.r.ConsentManager().GetLoginRequest(r.Context(), challenge)
356356
if err != nil {
357+
var challengeExpired *ErrChallengeExpired
358+
if errors.As(err, &challengeExpired) {
359+
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
360+
RedirectTo: challengeExpired.RedirectURL,
361+
})
362+
return
363+
}
357364
h.r.Writer().WriteError(w, r, err)
358365
return
359366
}
@@ -447,6 +454,13 @@ func (h *Handler) acceptOAuth2LoginRequest(w http.ResponseWriter, r *http.Reques
447454
handledLoginRequest.ID = challenge
448455
loginRequest, err := h.r.ConsentManager().GetLoginRequest(ctx, challenge)
449456
if err != nil {
457+
var challengeExpired *ErrChallengeExpired
458+
if errors.As(err, &challengeExpired) {
459+
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
460+
RedirectTo: challengeExpired.RedirectURL,
461+
})
462+
return
463+
}
450464
h.r.Writer().WriteError(w, r, err)
451465
return
452466
} else if loginRequest.Subject != "" && handledLoginRequest.Subject != loginRequest.Subject {
@@ -571,6 +585,13 @@ func (h *Handler) rejectOAuth2LoginRequest(w http.ResponseWriter, r *http.Reques
571585
p.SetDefaults(flow.LoginRequestDeniedErrorName)
572586
ar, err := h.r.ConsentManager().GetLoginRequest(ctx, challenge)
573587
if err != nil {
588+
var challengeExpired *ErrChallengeExpired
589+
if errors.As(err, &challengeExpired) {
590+
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
591+
RedirectTo: challengeExpired.RedirectURL,
592+
})
593+
return
594+
}
574595
h.r.Writer().WriteError(w, r, err)
575596
return
576597
}
@@ -661,6 +682,13 @@ func (h *Handler) getOAuth2ConsentRequest(w http.ResponseWriter, r *http.Request
661682

662683
request, err := h.r.ConsentManager().GetConsentRequest(r.Context(), challenge)
663684
if err != nil {
685+
var challengeExpired *ErrChallengeExpired
686+
if errors.As(err, &challengeExpired) {
687+
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
688+
RedirectTo: challengeExpired.RedirectURL,
689+
})
690+
return
691+
}
664692
h.r.Writer().WriteError(w, r, err)
665693
return
666694
}
@@ -753,6 +781,13 @@ func (h *Handler) acceptOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ
753781

754782
cr, err := h.r.ConsentManager().GetConsentRequest(ctx, challenge)
755783
if err != nil {
784+
var challengeExpired *ErrChallengeExpired
785+
if errors.As(err, &challengeExpired) {
786+
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
787+
RedirectTo: challengeExpired.RedirectURL,
788+
})
789+
return
790+
}
756791
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
757792
return
758793
}
@@ -864,6 +899,13 @@ func (h *Handler) rejectOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ
864899
p.SetDefaults(flow.ConsentRequestDeniedErrorName)
865900
hr, err := h.r.ConsentManager().GetConsentRequest(ctx, challenge)
866901
if err != nil {
902+
var challengeExpired *ErrChallengeExpired
903+
if errors.As(err, &challengeExpired) {
904+
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
905+
RedirectTo: challengeExpired.RedirectURL,
906+
})
907+
return
908+
}
867909
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
868910
return
869911
}

‎consent/handler_test.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import (
1313
"testing"
1414
"time"
1515

16+
"github.com/ory/hydra/v2/driver/config"
17+
1618
"github.com/stretchr/testify/require"
1719

1820
hydra "github.com/ory/hydra-client-go/v2"
@@ -85,11 +87,13 @@ func TestGetLoginRequest(t *testing.T) {
8587
for k, tc := range []struct {
8688
exists bool
8789
handled bool
90+
expired bool
8891
status int
8992
}{
90-
{false, false, http.StatusNotFound},
91-
{true, false, http.StatusOK},
92-
{true, true, http.StatusGone},
93+
{false, false, false, http.StatusNotFound},
94+
{true, false, false, http.StatusOK},
95+
{true, true, false, http.StatusGone},
96+
{true, false, true, http.StatusGone},
9397
} {
9498
t.Run(fmt.Sprintf("exists=%v/handled=%v", tc.exists, tc.handled), func(t *testing.T) {
9599
ctx := context.Background()
@@ -109,6 +113,10 @@ func TestGetLoginRequest(t *testing.T) {
109113
RequestURL: requestURL,
110114
RequestedAt: time.Now(),
111115
})
116+
if tc.expired {
117+
require.NoError(t, conf.Set(ctx, config.KeyConsentRequestMaxAge, time.Millisecond))
118+
time.Sleep(time.Millisecond * 5)
119+
}
112120
require.NoError(t, err)
113121
challenge, err = f.ToLoginChallenge(ctx, reg)
114122
require.NoError(t, err)
@@ -132,7 +140,7 @@ func TestGetLoginRequest(t *testing.T) {
132140
require.NoError(t, err)
133141
require.EqualValues(t, tc.status, resp.StatusCode)
134142

135-
if tc.handled {
143+
if tc.handled || tc.expired {
136144
var result flow.OAuth2RedirectTo
137145
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
138146
require.Equal(t, requestURL, result.RedirectTo)
@@ -151,11 +159,13 @@ func TestGetConsentRequest(t *testing.T) {
151159
for k, tc := range []struct {
152160
exists bool
153161
handled bool
162+
expired bool
154163
status int
155164
}{
156-
{false, false, http.StatusNotFound},
157-
{true, false, http.StatusOK},
158-
{true, true, http.StatusGone},
165+
{false, false, false, http.StatusNotFound},
166+
{true, false, false, http.StatusOK},
167+
{true, true, false, http.StatusGone},
168+
{true, false, true, http.StatusGone},
159169
} {
160170
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
161171
ctx := context.Background()
@@ -192,6 +202,10 @@ func TestGetConsentRequest(t *testing.T) {
192202
CSRF: challenge,
193203
LoginChallenge: sqlxx.NullString(lr.ID),
194204
}))
205+
if tc.expired {
206+
require.NoError(t, conf.Set(ctx, config.KeyConsentRequestMaxAge, time.Millisecond))
207+
time.Sleep(time.Millisecond * 5)
208+
}
195209

196210
if tc.handled {
197211
_, err := reg.ConsentManager().HandleConsentRequest(ctx, f, &flow.AcceptOAuth2ConsentRequest{
@@ -217,7 +231,7 @@ func TestGetConsentRequest(t *testing.T) {
217231
require.NoError(t, err)
218232
require.EqualValues(t, tc.status, resp.StatusCode)
219233

220-
if tc.handled {
234+
if tc.handled || tc.expired {
221235
var result flow.OAuth2RedirectTo
222236
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
223237
require.Equal(t, requestURL, result.RedirectTo)

‎consent/strategy_default.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ var ErrNoPreviousConsentFound = stderrs.New("no previous OAuth 2.0 Consent could
6363
var ErrNoAuthenticationSessionFound = stderrs.New("no previous login session was found")
6464
var ErrHintDoesNotMatchAuthentication = stderrs.New("subject from hint does not match subject from session")
6565

66+
type ErrChallengeExpired struct {
67+
RedirectURL string
68+
}
69+
70+
func (e *ErrChallengeExpired) Error() string {
71+
return "challenge has expired"
72+
}
73+
6674
func (s *DefaultStrategy) matchesValueFromSession(ctx context.Context, c fosite.Client, hintSubject string, sessionSubject string) error {
6775
obfuscatedUserID, err := s.ObfuscateSubjectIdentifier(ctx, c, sessionSubject, "")
6876
if err != nil {

0 commit comments

Comments
 (0)