Skip to content

Commit 47cbb3c

Browse files
aarmamaarmam
committed
feat: extend latest and session related consents
1 parent d37b323 commit 47cbb3c

File tree

6 files changed

+313
-6
lines changed

6 files changed

+313
-6
lines changed

‎consent/handler.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,14 @@ func (h *Handler) acceptOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ
675675
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
676676
return
677677
} else if hr.Skip {
678+
if p.Remember && p.RememberFor > 0 { // TODO: Consider removing 'p.RememberFor > 0' to update consent validity in both ways (limited (RememberFor > 0) -> indefinitely (RememberFor = 0) and vice versa)
679+
var ctx = r.Context()
680+
err = h.r.ConsentManager().ExtendConsentRequest(r.Context(), h.r.Config().GetScopeStrategy(ctx), hr, p.RememberFor)
681+
if err != nil {
682+
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
683+
return
684+
}
685+
}
678686
p.Remember = false
679687
}
680688

‎consent/handler_test.go‎

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,10 @@ import (
3030
"testing"
3131
"time"
3232

33-
"github.com/ory/x/pointerx"
34-
3533
"github.com/ory/hydra/consent"
3634
"github.com/ory/hydra/x"
3735
"github.com/ory/x/contextx"
36+
"github.com/ory/x/pointerx"
3837
"github.com/ory/x/sqlxx"
3938

4039
"github.com/ory/hydra/internal"
@@ -231,6 +230,98 @@ func TestGetConsentRequest(t *testing.T) {
231230
}
232231
}
233232

233+
func TestExtendConsentRequest(t *testing.T) {
234+
t.Run("case=extend consent expiry time", func(t *testing.T) {
235+
conf := internal.NewConfigurationWithDefaults()
236+
reg := internal.NewRegistryMemory(t, conf, &contextx.Default{})
237+
h := NewHandler(reg, conf)
238+
r := x.NewRouterAdmin(conf.AdminURL)
239+
h.SetRoutes(r)
240+
ts := httptest.NewServer(r)
241+
defer ts.Close()
242+
243+
c := &http.Client{}
244+
cl := &client.Client{LegacyClientID: "client-1"}
245+
require.NoError(t, reg.ClientManager().CreateClient(context.Background(), cl))
246+
247+
var initialRememberFor time.Duration = 300
248+
var remainingValidTime time.Duration = 100
249+
250+
require.NoError(t, reg.ConsentManager().CreateLoginSession(context.Background(), &LoginSession{
251+
ID: makeID("fk-login-session", "1", "1"),
252+
Subject: "subject-1",
253+
}))
254+
requestedTimeInPast := time.Now().UTC().Add(-(initialRememberFor - remainingValidTime) * time.Second)
255+
require.NoError(t, reg.ConsentManager().CreateLoginRequest(context.Background(), &LoginRequest{
256+
ID: makeID("challenge", "1", "1"),
257+
SessionID: sqlxx.NullString(makeID("fk-login-session", "1", "1")),
258+
Client: cl,
259+
Subject: "subject-1",
260+
RequestedAt: requestedTimeInPast,
261+
}))
262+
require.NoError(t, reg.ConsentManager().CreateConsentRequest(context.Background(), &OAuth2ConsentRequest{
263+
ID: makeID("challenge", "1", "1"),
264+
Subject: "subject-1",
265+
Client: cl,
266+
LoginSessionID: sqlxx.NullString(makeID("fk-login-session", "1", "1")),
267+
LoginChallenge: sqlxx.NullString(makeID("challenge", "1", "1")),
268+
Verifier: makeID("verifier", "1", "1"),
269+
CSRF: "csrf1",
270+
Skip: false,
271+
ACR: "1",
272+
}))
273+
_, err := reg.ConsentManager().HandleConsentRequest(context.Background(), &AcceptOAuth2ConsentRequest{
274+
ID: makeID("challenge", "1", "1"),
275+
Remember: true,
276+
RememberFor: int(initialRememberFor),
277+
WasHandled: true,
278+
HandledAt: sqlxx.NullTime(time.Now().UTC()),
279+
})
280+
require.NoError(t, err)
281+
282+
require.NoError(t, reg.ConsentManager().CreateLoginRequest(context.Background(), &LoginRequest{
283+
ID: makeID("challenge", "1", "2"),
284+
SessionID: sqlxx.NullString(makeID("fk-login-session", "1", "1")),
285+
Verifier: makeID("verifier", "1", "1"),
286+
Client: cl,
287+
RequestedAt: time.Now().UTC(),
288+
Subject: "subject-1",
289+
}))
290+
require.NoError(t, reg.ConsentManager().CreateConsentRequest(context.Background(), &OAuth2ConsentRequest{
291+
ID: makeID("challenge", "1", "2"),
292+
Subject: "subject-1",
293+
Client: cl,
294+
LoginSessionID: sqlxx.NullString(makeID("fk-login-session", "1", "1")),
295+
LoginChallenge: sqlxx.NullString(makeID("challenge", "1", "2")),
296+
Verifier: makeID("verifier", "1", "2"),
297+
CSRF: "csrf2",
298+
Skip: true,
299+
}))
300+
301+
var b bytes.Buffer
302+
var extendRememberFor time.Duration = 300
303+
require.NoError(t, json.NewEncoder(&b).Encode(&AcceptOAuth2ConsentRequest{
304+
Remember: true,
305+
RememberFor: int(extendRememberFor),
306+
}))
307+
308+
req, err := http.NewRequest(http.MethodPut, ts.URL+"/admin"+ConsentPath+"/accept?challenge=challenge-1-2", &b)
309+
require.NoError(t, err)
310+
resp, err := c.Do(req)
311+
require.NoError(t, err)
312+
require.EqualValues(t, 200, resp.StatusCode)
313+
314+
crs, err := reg.ConsentManager().FindSubjectsGrantedConsentRequests(context.Background(), "subject-1", 100, 0)
315+
require.NoError(t, err)
316+
require.NotNil(t, crs)
317+
require.EqualValues(t, 1, len(crs))
318+
expectedRememberFor := int(initialRememberFor + extendRememberFor - remainingValidTime)
319+
cr := crs[0]
320+
require.EqualValues(t, "challenge-1-1", cr.ID)
321+
require.InDelta(t, expectedRememberFor, cr.RememberFor, 1)
322+
})
323+
}
324+
234325
func TestGetLoginRequestWithDuplicateAccept(t *testing.T) {
235326
t.Run("Test get login request with duplicate accept", func(t *testing.T) {
236327
challenge := "challenge"

‎consent/manager.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
"context"
2525
"time"
2626

27+
"github.com/ory/fosite"
28+
2729
"github.com/gofrs/uuid"
2830

2931
"github.com/ory/hydra/client"
@@ -44,6 +46,7 @@ type Manager interface {
4446
CreateConsentRequest(ctx context.Context, req *OAuth2ConsentRequest) error
4547
GetConsentRequest(ctx context.Context, challenge string) (*OAuth2ConsentRequest, error)
4648
HandleConsentRequest(ctx context.Context, r *AcceptOAuth2ConsentRequest) (*OAuth2ConsentRequest, error)
49+
ExtendConsentRequest(ctx context.Context, scopeStrategy fosite.ScopeStrategy, req *OAuth2ConsentRequest, extendBy int) error
4750
RevokeSubjectConsentSession(ctx context.Context, user string) error
4851
RevokeSubjectClientConsentSession(ctx context.Context, user, client string) error
4952

‎consent/manager_test_helpers.go‎

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ func TestHelperNID(t1ClientManager client.Manager, t1ValidNID Manager, t2Invalid
309309
}
310310
}
311311

312-
func ManagerTests(m Manager, clientManager client.Manager, fositeManager x.FositeStorer, network string, parallel bool) func(t *testing.T) {
312+
func ManagerTests(m Manager, clientManager client.Manager, fositeManager x.FositeStorer, scopeStrategy fosite.ScopeStrategy, network string, parallel bool) func(t *testing.T) {
313313
lr := make(map[string]*LoginRequest)
314314

315315
return func(t *testing.T) {
@@ -548,6 +548,129 @@ func ManagerTests(m Manager, clientManager client.Manager, fositeManager x.Fosit
548548
}
549549
})
550550

551+
t.Run("case=extend consent request", func(t *testing.T) {
552+
cl := &client.Client{LegacyClientID: "client-1"}
553+
_ = clientManager.CreateClient(context.Background(), cl)
554+
consentFlow := func(subject, sessionId, challenge string, rememberFor time.Duration, requestedAt time.Time, requestedScope string, skip bool) *OAuth2ConsentRequest {
555+
require.NoError(t, m.CreateLoginRequest(context.Background(), &LoginRequest{
556+
ID: makeID("challenge", network, challenge),
557+
SessionID: sqlxx.NullString(makeID("fk-login-session", network, sessionId)),
558+
Client: cl,
559+
Subject: subject,
560+
Verifier: uuid.New().String(),
561+
RequestedAt: requestedAt,
562+
RequestedScope: []string{requestedScope},
563+
}))
564+
565+
require.NoError(t, m.CreateConsentRequest(context.Background(), &OAuth2ConsentRequest{
566+
ID: makeID("challenge", network, challenge),
567+
Client: cl,
568+
Subject: subject,
569+
LoginSessionID: sqlxx.NullString(makeID("fk-login-session", network, sessionId)),
570+
LoginChallenge: sqlxx.NullString(makeID("challenge", network, challenge)),
571+
Skip: skip,
572+
Verifier: uuid.New().String(),
573+
CSRF: "csrf1",
574+
}))
575+
cr, err := m.HandleConsentRequest(context.Background(), &AcceptOAuth2ConsentRequest{
576+
ID: makeID("challenge", network, challenge),
577+
Remember: true,
578+
RememberFor: int(rememberFor),
579+
WasHandled: true,
580+
HandledAt: sqlxx.NullTime(time.Now().UTC()),
581+
GrantedScope: []string{"scope-a"},
582+
})
583+
require.NoError(t, err)
584+
return cr
585+
}
586+
587+
t.Run("case=extend session related and latest consent expiry times", func(t *testing.T) {
588+
var rememberForSession1 time.Duration = 300
589+
var remainingValidTimeSession1 time.Duration = 100
590+
var rememberForSession2 time.Duration = 300
591+
var remainingValidTimeSession2 time.Duration = 150
592+
var extendRememberFor time.Duration = 1000
593+
requestedAt1 := time.Now().UTC().Round(time.Second).Add(-(rememberForSession1 - remainingValidTimeSession1) * time.Second)
594+
requestedAt2 := time.Now().UTC().Round(time.Second).Add(-(rememberForSession2 - remainingValidTimeSession2) * time.Second)
595+
requestedAt3 := time.Now().UTC()
596+
require.NoError(t, m.CreateLoginSession(context.Background(), &LoginSession{
597+
ID: makeID("fk-login-session", network, "ec1"),
598+
Subject: "subject-1",
599+
}))
600+
require.NoError(t, m.CreateLoginSession(context.Background(), &LoginSession{
601+
ID: makeID("fk-login-session", network, "ec2"),
602+
Subject: "subject-1",
603+
}))
604+
consentFlow("subject-1", "ec1", "c1", rememberForSession1, requestedAt1, "scope-a", false)
605+
consentFlow("subject-1", "ec2", "c2", rememberForSession2, requestedAt2, "scope-a", false)
606+
cr := consentFlow("subject-1", "ec1", "c3", extendRememberFor, requestedAt3, "scope-a", true)
607+
608+
require.NoError(t, m.ExtendConsentRequest(context.Background(), scopeStrategy, cr, int(extendRememberFor)))
609+
610+
crs, err := m.FindSubjectsGrantedConsentRequests(context.Background(), "subject-1", 100, 0)
611+
require.NoError(t, err)
612+
require.EqualValues(t, 2, len(crs))
613+
crSession := crs[1]
614+
require.EqualValues(t, makeID("challenge", network, "c1"), crSession.ID)
615+
expectedExtendedRememberFor1 := int(rememberForSession1 + extendRememberFor - remainingValidTimeSession1)
616+
require.InDelta(t, expectedExtendedRememberFor1, crSession.RememberFor, 1)
617+
crLatest := crs[0]
618+
require.EqualValues(t, makeID("challenge", network, "c2"), crLatest.ID)
619+
expectedExtendedRememberFor2 := int(rememberForSession2 + extendRememberFor - remainingValidTimeSession2)
620+
require.InDelta(t, expectedExtendedRememberFor2, crLatest.RememberFor, 1)
621+
})
622+
623+
t.Run("case=no previous consent found", func(t *testing.T) {
624+
require.NoError(t, m.CreateLoginSession(context.Background(), &LoginSession{
625+
ID: makeID("fk-login-session", network, "ec3"),
626+
Subject: "subject-1",
627+
}))
628+
cr := consentFlow("subject-1", "ec3", "c4", 300, time.Now().UTC(), "scope-a", true)
629+
630+
require.ErrorIs(t, m.ExtendConsentRequest(context.Background(), scopeStrategy, cr, 1000), ErrNoPreviousConsentFound)
631+
})
632+
633+
t.Run("case=invalid requested scope", func(t *testing.T) {
634+
var rememberForSession1 time.Duration = 300
635+
var remainingValidTimeSession1 time.Duration = 100
636+
requestedAt1 := time.Now().UTC().Round(time.Second).Add(-(rememberForSession1 - remainingValidTimeSession1) * time.Second)
637+
requestedAt2 := time.Now().UTC()
638+
require.NoError(t, m.CreateLoginSession(context.Background(), &LoginSession{
639+
ID: makeID("fk-login-session", network, "ec4"),
640+
Subject: "subject-2",
641+
}))
642+
consentFlow("subject-2", "ec4", "c5", 300, requestedAt1, "scope-a", false)
643+
cr := consentFlow("subject-2", "ec4", "c6", 300, requestedAt2, "scope-b", true)
644+
645+
require.NoError(t, m.ExtendConsentRequest(context.Background(), scopeStrategy, cr, 1000))
646+
647+
crs, err := m.FindSubjectsGrantedConsentRequests(context.Background(), "subject-2", 10, 0)
648+
require.NoError(t, err)
649+
require.EqualValues(t, 1, len(crs))
650+
cr1 := crs[0]
651+
require.EqualValues(t, makeID("challenge", network, "c5"), cr1.ID)
652+
require.EqualValues(t, 300, cr1.RememberFor)
653+
})
654+
655+
t.Run("case=initial consent request expired", func(t *testing.T) {
656+
var rememberForSession1 time.Duration = 300
657+
var remainingValidTimeSession1 time.Duration = 0
658+
requestedAtExpired := time.Now().UTC().Round(time.Second).Add(-(rememberForSession1 - remainingValidTimeSession1) * time.Second)
659+
require.NoError(t, m.CreateLoginSession(context.Background(), &LoginSession{
660+
ID: makeID("fk-login-session", network, "ec5"),
661+
Subject: "subject-3",
662+
}))
663+
consentFlow("subject-3", "ec5", "c7", 300, requestedAtExpired, "scope-a", false)
664+
time.Sleep(time.Second)
665+
cr := consentFlow("subject-3", "ec5", "c8", 300, time.Now().UTC(), "scope-a", true)
666+
667+
require.NoError(t, m.ExtendConsentRequest(context.Background(), scopeStrategy, cr, 1000))
668+
669+
_, err := m.FindSubjectsGrantedConsentRequests(context.Background(), "subject-3", 100, 0)
670+
require.Error(t, err, ErrNoPreviousConsentFound)
671+
})
672+
})
673+
551674
t.Run("case=revoke-auth-request", func(t *testing.T) {
552675
require.NoError(t, m.CreateLoginSession(context.Background(), &LoginSession{
553676
ID: makeID("rev-session", network, "-1"),

‎persistence/sql/persister_consent.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,88 @@ func (p *Persister) HandleConsentRequest(ctx context.Context, r *consent.AcceptO
280280
return p.GetConsentRequest(ctx, r.ID)
281281
}
282282

283+
func (p *Persister) ExtendConsentRequest(ctx context.Context, scopeStrategy fosite.ScopeStrategy, cr *consent.OAuth2ConsentRequest, extendBy int) error {
284+
return p.transaction(ctx, func(ctx context.Context, c *pop.Connection) error {
285+
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.ExtendConsentRequest")
286+
defer span.End()
287+
288+
sessionFlow := &flow.Flow{}
289+
if err := c.
290+
Where(
291+
strings.TrimSpace(fmt.Sprintf(`
292+
(state = %d OR state = %d) AND
293+
subject = ? AND
294+
client_id = ? AND
295+
login_session_id = ? AND
296+
consent_skip=FALSE AND
297+
consent_error='{}' AND
298+
consent_remember=TRUE AND
299+
nid = ?`, flow.FlowStateConsentUsed, flow.FlowStateConsentUnused,
300+
)),
301+
cr.Subject, cr.ClientID, cr.LoginSessionID.String(), p.NetworkID(ctx)).
302+
Order("requested_at DESC").
303+
Limit(1).
304+
First(sessionFlow); err != nil {
305+
if errors.Is(err, sql.ErrNoRows) {
306+
return errorsx.WithStack(consent.ErrNoPreviousConsentFound)
307+
}
308+
return sqlcon.HandleError(err)
309+
}
310+
311+
latestFlow := &flow.Flow{}
312+
if err := c.
313+
Where(
314+
strings.TrimSpace(fmt.Sprintf(`
315+
(state = %d OR state = %d) AND
316+
subject = ? AND
317+
client_id = ? AND
318+
consent_skip=FALSE AND
319+
consent_error='{}' AND
320+
consent_remember=TRUE AND
321+
nid = ?`, flow.FlowStateConsentUsed, flow.FlowStateConsentUnused,
322+
)),
323+
cr.Subject, cr.ClientID, p.NetworkID(ctx)).
324+
Order("requested_at DESC").
325+
Limit(1).
326+
First(latestFlow); err != nil {
327+
return sqlcon.HandleError(err)
328+
}
329+
330+
if err := p.extendHandledConsentRequest(ctx, cr, scopeStrategy, sessionFlow, extendBy); err != nil {
331+
return err
332+
}
333+
334+
if latestFlow.ID != sessionFlow.ID {
335+
if err := p.extendHandledConsentRequest(ctx, cr, scopeStrategy, latestFlow, extendBy); err != nil {
336+
return err
337+
}
338+
}
339+
return nil
340+
})
341+
}
342+
343+
func (p *Persister) extendHandledConsentRequest(ctx context.Context, cr *consent.OAuth2ConsentRequest, scopeStrategy fosite.ScopeStrategy, f *flow.Flow, extendBy int) error {
344+
for _, scope := range cr.RequestedScope {
345+
if !scopeStrategy(f.GrantedScope, scope) {
346+
return nil
347+
}
348+
}
349+
hcr := f.GetHandledConsentRequest()
350+
if isConsentRequestExpired := hcr.RememberFor > 0 && hcr.RequestedAt.Add(time.Duration(hcr.RememberFor)*time.Second).Before(time.Now().UTC()); isConsentRequestExpired {
351+
return nil
352+
}
353+
remainingTime := hcr.RequestedAt.Unix() + int64(hcr.RememberFor) - time.Now().Unix()
354+
extendedRememberFor := hcr.RememberFor + extendBy - int(remainingTime)
355+
f.ConsentRememberFor = &extendedRememberFor
356+
357+
_, err := p.UpdateWithNetwork(ctx, f)
358+
if err != nil {
359+
return sqlcon.HandleError(err)
360+
} else {
361+
return nil
362+
}
363+
}
364+
283365
func (p *Persister) VerifyAndInvalidateConsentRequest(ctx context.Context, verifier string) (*consent.AcceptOAuth2ConsentRequest, error) {
284366
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.VerifyAndInvalidateConsentRequest")
285367
defer span.End()

‎persistence/sql/persister_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ func testRegistry(t *testing.T, ctx context.Context, k string, t1 driver.Registr
4848
if k == "memory" || k == "mysql" || k == "cockroach" { // TODO enable parallel tests for cockroach once we configure the cockroach integration test server to support retry
4949
parallel = false
5050
}
51-
52-
t.Run("package=consent/manager="+k, consent.ManagerTests(t1.ConsentManager(), t1.ClientManager(), t1.OAuth2Storage(), "t1", parallel))
53-
t.Run("package=consent/manager="+k, consent.ManagerTests(t2.ConsentManager(), t2.ClientManager(), t2.OAuth2Storage(), "t2", parallel))
51+
scopeStrategy := t1.Config().GetScopeStrategy(ctx)
52+
t.Run("package=consent/manager="+k, consent.ManagerTests(t1.ConsentManager(), t1.ClientManager(), t1.OAuth2Storage(), scopeStrategy, "t1", parallel))
53+
t.Run("package=consent/manager="+k, consent.ManagerTests(t2.ConsentManager(), t2.ClientManager(), t2.OAuth2Storage(), scopeStrategy, "t2", parallel))
5454

5555
t.Run("parallel-boundary", func(t *testing.T) {
5656
t.Run("package=consent/janitor="+k, testhelpers.JanitorTests(t1.Config(), t1.ConsentManager(), t1.ClientManager(), t1.OAuth2Storage(), "t1", parallel))

0 commit comments

Comments
 (0)