Skip to content

Commit

Permalink
feat: return HTTP 410 and initial auth url for consent app to redirec…
Browse files Browse the repository at this point in the history
…t the user agent to when an expired challenge is supplied
  • Loading branch information
terev committed Jul 21, 2024
1 parent 9ba07a7 commit 4441c17
Show file tree
Hide file tree
Showing 5 changed files with 429 additions and 10 deletions.
42 changes: 42 additions & 0 deletions consent/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,13 @@ func (h *Handler) getOAuth2LoginRequest(w http.ResponseWriter, r *http.Request,

request, err := h.r.ConsentManager().GetLoginRequest(r.Context(), challenge)
if err != nil {
var challengeExpired *ErrChallengeExpired
if errors.As(err, &challengeExpired) {
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
RedirectTo: challengeExpired.RedirectURL,
})
return
}
h.r.Writer().WriteError(w, r, err)
return
}
Expand Down Expand Up @@ -447,6 +454,13 @@ func (h *Handler) acceptOAuth2LoginRequest(w http.ResponseWriter, r *http.Reques
handledLoginRequest.ID = challenge
loginRequest, err := h.r.ConsentManager().GetLoginRequest(ctx, challenge)
if err != nil {
var challengeExpired *ErrChallengeExpired
if errors.As(err, &challengeExpired) {
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
RedirectTo: challengeExpired.RedirectURL,
})
return
}
h.r.Writer().WriteError(w, r, err)
return
} else if loginRequest.Subject != "" && handledLoginRequest.Subject != loginRequest.Subject {
Expand Down Expand Up @@ -571,6 +585,13 @@ func (h *Handler) rejectOAuth2LoginRequest(w http.ResponseWriter, r *http.Reques
p.SetDefaults(flow.LoginRequestDeniedErrorName)
ar, err := h.r.ConsentManager().GetLoginRequest(ctx, challenge)
if err != nil {
var challengeExpired *ErrChallengeExpired
if errors.As(err, &challengeExpired) {
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
RedirectTo: challengeExpired.RedirectURL,
})
return
}
h.r.Writer().WriteError(w, r, err)
return
}
Expand Down Expand Up @@ -661,6 +682,13 @@ func (h *Handler) getOAuth2ConsentRequest(w http.ResponseWriter, r *http.Request

request, err := h.r.ConsentManager().GetConsentRequest(r.Context(), challenge)
if err != nil {
var challengeExpired *ErrChallengeExpired
if errors.As(err, &challengeExpired) {
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
RedirectTo: challengeExpired.RedirectURL,
})
return
}
h.r.Writer().WriteError(w, r, err)
return
}
Expand Down Expand Up @@ -753,6 +781,13 @@ func (h *Handler) acceptOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ

cr, err := h.r.ConsentManager().GetConsentRequest(ctx, challenge)
if err != nil {
var challengeExpired *ErrChallengeExpired
if errors.As(err, &challengeExpired) {
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
RedirectTo: challengeExpired.RedirectURL,
})
return
}
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
return
}
Expand Down Expand Up @@ -864,6 +899,13 @@ func (h *Handler) rejectOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ
p.SetDefaults(flow.ConsentRequestDeniedErrorName)
hr, err := h.r.ConsentManager().GetConsentRequest(ctx, challenge)
if err != nil {
var challengeExpired *ErrChallengeExpired
if errors.As(err, &challengeExpired) {
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
RedirectTo: challengeExpired.RedirectURL,
})
return
}
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
return
}
Expand Down
30 changes: 22 additions & 8 deletions consent/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"testing"
"time"

"github.com/ory/hydra/v2/driver/config"

"github.com/stretchr/testify/require"

hydra "github.com/ory/hydra-client-go/v2"
Expand Down Expand Up @@ -85,11 +87,13 @@ func TestGetLoginRequest(t *testing.T) {
for k, tc := range []struct {
exists bool
handled bool
expired bool
status int
}{
{false, false, http.StatusNotFound},
{true, false, http.StatusOK},
{true, true, http.StatusGone},
{false, false, false, http.StatusNotFound},
{true, false, false, http.StatusOK},
{true, true, false, http.StatusGone},
{true, false, true, http.StatusGone},
} {
t.Run(fmt.Sprintf("exists=%v/handled=%v", tc.exists, tc.handled), func(t *testing.T) {
ctx := context.Background()
Expand All @@ -109,6 +113,10 @@ func TestGetLoginRequest(t *testing.T) {
RequestURL: requestURL,
RequestedAt: time.Now(),
})
if tc.expired {
require.NoError(t, conf.Set(ctx, config.KeyConsentRequestMaxAge, time.Millisecond))
time.Sleep(time.Millisecond * 5)
}
require.NoError(t, err)
challenge, err = f.ToLoginChallenge(ctx, reg)
require.NoError(t, err)
Expand All @@ -132,7 +140,7 @@ func TestGetLoginRequest(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, tc.status, resp.StatusCode)

if tc.handled {
if tc.handled || tc.expired {
var result flow.OAuth2RedirectTo
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
require.Equal(t, requestURL, result.RedirectTo)
Expand All @@ -151,11 +159,13 @@ func TestGetConsentRequest(t *testing.T) {
for k, tc := range []struct {
exists bool
handled bool
expired bool
status int
}{
{false, false, http.StatusNotFound},
{true, false, http.StatusOK},
{true, true, http.StatusGone},
{false, false, false, http.StatusNotFound},
{true, false, false, http.StatusOK},
{true, true, false, http.StatusGone},
{true, false, true, http.StatusGone},
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
ctx := context.Background()
Expand Down Expand Up @@ -192,6 +202,10 @@ func TestGetConsentRequest(t *testing.T) {
CSRF: challenge,
LoginChallenge: sqlxx.NullString(lr.ID),
}))
if tc.expired {
require.NoError(t, conf.Set(ctx, config.KeyConsentRequestMaxAge, time.Millisecond))
time.Sleep(time.Millisecond * 5)
}

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

if tc.handled {
if tc.handled || tc.expired {
var result flow.OAuth2RedirectTo
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
require.Equal(t, requestURL, result.RedirectTo)
Expand Down
8 changes: 8 additions & 0 deletions consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ var ErrNoPreviousConsentFound = stderrs.New("no previous OAuth 2.0 Consent could
var ErrNoAuthenticationSessionFound = stderrs.New("no previous login session was found")
var ErrHintDoesNotMatchAuthentication = stderrs.New("subject from hint does not match subject from session")

type ErrChallengeExpired struct {
RedirectURL string
}

func (e *ErrChallengeExpired) Error() string {
return "challenge has expired"
}

func (s *DefaultStrategy) matchesValueFromSession(ctx context.Context, c fosite.Client, hintSubject string, sessionSubject string) error {
obfuscatedUserID, err := s.ObfuscateSubjectIdentifier(ctx, c, sessionSubject, "")
if err != nil {
Expand Down
Loading

0 comments on commit 4441c17

Please sign in to comment.