Skip to content

Commit fdca960

Browse files
zepatrikory-bot
zepatrik
authored and
ory-bot
committed
revert: simplify consent store
GitOrigin-RevId: 5cb638da28b47533c6a82661a36e9add4ca4be97
1 parent 2dd6b94 commit fdca960

File tree

10 files changed

+131
-133
lines changed

10 files changed

+131
-133
lines changed

‎consent/test/manager_test_helpers.go‎

Lines changed: 25 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import (
2727
"github.com/ory/x/uuidx"
2828
)
2929

30-
func MockConsentFlow(remember bool, rememberFor int, skip bool) *flow.Flow {
30+
func mockConsentRequest(remember bool, rememberFor int, skip bool) *flow.Flow {
3131
return &flow.Flow{
3232
ID: uuidx.NewV4().String(),
3333
Client: &client.Client{ID: uuidx.NewV4().String()},
@@ -219,8 +219,8 @@ func ConsentManagerTests(t *testing.T, deps Deps, m consent.Manager, loginManage
219219
{"6", true, 120, true, false},
220220
} {
221221
t.Run("key="+tc.key, func(t *testing.T) {
222-
f := MockConsentFlow(tc.remember, tc.rememberFor, tc.skip)
223-
require.NoError(t, clientManager.CreateClient(t.Context(), f.Client))
222+
f := mockConsentRequest(tc.remember, tc.rememberFor, tc.skip)
223+
_ = clientManager.CreateClient(t.Context(), f.Client) // Ignore errors that are caused by duplication
224224
f.NID = deps.Networker().NetworkID(t.Context())
225225

226226
require.NoError(t, m.CreateConsentSession(t.Context(), f))
@@ -234,7 +234,7 @@ func ConsentManagerTests(t *testing.T, deps Deps, m consent.Manager, loginManage
234234
// unfortunately the interface does not allow us to set the absolute time, so we have to wait
235235
time.Sleep(2 * time.Second)
236236
}
237-
actual, err := m.FindGrantedAndRememberedConsentRequest(t.Context(), f.Client.ID, f.Subject)
237+
actual, err := m.FindGrantedAndRememberedConsentRequest(t.Context(), f.ClientID, f.Subject)
238238
if !tc.expectRemembered {
239239
assert.Nil(t, actual)
240240
assert.ErrorIs(t, err, consent.ErrNoPreviousConsentFound)
@@ -260,45 +260,36 @@ func ConsentManagerTests(t *testing.T, deps Deps, m consent.Manager, loginManage
260260

261261
t.Run("case=revoke consent request", func(t *testing.T) {
262262
type tc struct {
263-
f *flow.Flow
264-
at, rt string
265-
revoke func(*testing.T, tc)
263+
at, rt, subject, client string
264+
revoke func(*testing.T)
266265
}
267-
revokeFuncs := []func(*testing.T, tc){
268-
func(t *testing.T, c tc) {
269-
require.NoError(t, m.RevokeSubjectConsentSession(t.Context(), c.f.Subject))
270-
},
271-
func(t *testing.T, c tc) {
272-
require.NoError(t, m.RevokeSubjectClientConsentSession(t.Context(), c.f.Subject, c.f.Client.ID))
273-
},
274-
func(t *testing.T, c tc) {
275-
require.NoError(t, m.RevokeConsentSessionByID(t.Context(), c.f.ConsentRequestID.String()))
276-
},
277-
}
278-
tcs := make([]tc, 2*len(revokeFuncs))
266+
tcs := make([]tc, 2)
279267
for i := range tcs {
280-
f := MockConsentFlow(i < len(revokeFuncs), 0, true)
268+
f := mockConsentRequest(true, 0, false)
281269
f.NID = deps.Networker().NetworkID(t.Context())
282270

283271
tcs[i] = tc{
284-
f: f,
285-
at: uuidx.NewV4().String(),
286-
rt: uuidx.NewV4().String(),
287-
revoke: revokeFuncs[i%len(revokeFuncs)],
272+
subject: f.Subject,
273+
client: f.Client.ID,
274+
at: uuidx.NewV4().String(),
275+
rt: uuidx.NewV4().String(),
288276
}
289277

290278
require.NoError(t, clientManager.CreateClient(t.Context(), f.Client))
291279
require.NoError(t, m.CreateConsentSession(t.Context(), f))
292-
sess := &oauth2.Session{DefaultSession: openid.NewDefaultSession()}
293-
sess.Subject = f.Subject
294-
sess.ConsentChallenge = f.ConsentRequestID.String()
295280
require.NoError(t, fositeManager.CreateAccessTokenSession(t.Context(), tcs[i].at,
296-
&fosite.Request{Client: f.Client, ID: f.ConsentRequestID.String(), RequestedAt: time.Now(), Session: sess}),
297-
)
281+
&fosite.Request{Client: f.Client, ID: f.ConsentRequestID.String(), RequestedAt: time.Now(), Session: &oauth2.Session{DefaultSession: openid.NewDefaultSession()}},
282+
))
298283
require.NoError(t, fositeManager.CreateRefreshTokenSession(t.Context(), tcs[i].rt, tcs[i].at,
299-
&fosite.Request{Client: f.Client, ID: f.ConsentRequestID.String(), RequestedAt: time.Now(), Session: sess},
284+
&fosite.Request{Client: f.Client, ID: f.ConsentRequestID.String(), RequestedAt: time.Now(), Session: &oauth2.Session{DefaultSession: openid.NewDefaultSession()}},
300285
))
301286
}
287+
tcs[0].revoke = func(t *testing.T) {
288+
require.NoError(t, m.RevokeSubjectConsentSession(t.Context(), tcs[0].subject))
289+
}
290+
tcs[1].revoke = func(t *testing.T) {
291+
require.NoError(t, m.RevokeSubjectClientConsentSession(t.Context(), tcs[1].subject, tcs[1].client))
292+
}
302293

303294
for i, tc := range tcs {
304295
t.Run(fmt.Sprintf("run=%d", i), func(t *testing.T) {
@@ -307,7 +298,7 @@ func ConsentManagerTests(t *testing.T, deps Deps, m consent.Manager, loginManage
307298
_, err = fositeManager.GetRefreshTokenSession(t.Context(), tc.rt, nil)
308299
require.NoError(t, err)
309300

310-
tc.revoke(t, tc)
301+
tc.revoke(t)
311302

312303
r, err := fositeManager.GetAccessTokenSession(t.Context(), tc.at, nil)
313304
assert.ErrorIsf(t, err, fosite.ErrNotFound, "%+v", r)
@@ -325,7 +316,7 @@ func ConsentManagerTests(t *testing.T, deps Deps, m consent.Manager, loginManage
325316
t.Run("case=list consents", func(t *testing.T) {
326317
flows := make([]*flow.Flow, 2)
327318
for i := range flows {
328-
f := MockConsentFlow(true, 0, false)
319+
f := mockConsentRequest(true, 0, false)
329320
f.NID = deps.Networker().NetworkID(t.Context())
330321
f.SessionID = sqlxx.NullString(uuidx.NewV4().String())
331322
flows[i] = f
@@ -354,8 +345,8 @@ func ConsentManagerTests(t *testing.T, deps Deps, m consent.Manager, loginManage
354345
}
355346

356347
t.Run("random subject", func(t *testing.T) {
357-
res, _, err := m.FindSubjectsSessionGrantedConsentRequests(t.Context(), uuidx.NewV4().String(), flows[0].SessionID.String())
358-
assert.ErrorIsf(t, err, consent.ErrNoPreviousConsentFound, "%+v", res)
348+
_, _, err := m.FindSubjectsSessionGrantedConsentRequests(t.Context(), uuidx.NewV4().String(), flows[0].SessionID.String())
349+
assert.ErrorIs(t, err, consent.ErrNoPreviousConsentFound)
359350
})
360351
})
361352

@@ -435,7 +426,6 @@ func ConsentManagerTests(t *testing.T, deps Deps, m consent.Manager, loginManage
435426
SessionID: sqlxx.NullString(ls.ID),
436427
ConsentRequestID: sqlxx.NullString(uuid.Must(uuid.NewV4()).String()),
437428
GrantedScope: sqlxx.StringSliceJSONFormat{"scopea", "scopeb"},
438-
ConsentRemember: true,
439429
ConsentRememberFor: pointerx.Ptr(0),
440430
}
441431

‎driver/di.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package driver
55

66
import (
7-
"github.com/ory/hydra/v2/consent"
87
"github.com/ory/hydra/v2/fosite"
98
"github.com/ory/hydra/v2/fosite/handler/oauth2"
109
"github.com/ory/hydra/v2/jwk"
@@ -59,10 +58,3 @@ func RegistryWithAuthorizeCodeStorage(s func(r *RegistrySQL) oauth2.AuthorizeCod
5958
return nil
6059
}
6160
}
62-
63-
func RegistryWithConsentManager(cm func(r *RegistrySQL) (consent.Manager, error)) RegistryModifier {
64-
return func(r *RegistrySQL) (err error) {
65-
r.consentManager, err = cm(r)
66-
return err
67-
}
68-
}

‎driver/registry_sql.go‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,7 @@ type RegistrySQL struct {
9292
migrator *sql.MigrationManager
9393
dbOptsModifier []func(details *pop.ConnectionDetails)
9494

95-
keyManager jwk.Manager
96-
consentManager consent.Manager
97-
95+
keyManager jwk.Manager
9896
initialPing func(ctx context.Context, l *logrusx.Logger, p *sql.BasePersister) error
9997
middlewares []negroni.Handler
10098
}
@@ -265,12 +263,7 @@ func (m *RegistrySQL) PingContext(ctx context.Context) error { return m.basePers
265263

266264
func (m *RegistrySQL) BasePersister() *sql.BasePersister { return m.basePersister }
267265
func (m *RegistrySQL) ClientManager() client.Manager { return m.Persister() }
268-
func (m *RegistrySQL) ConsentManager() consent.Manager {
269-
if m.consentManager != nil {
270-
return m.consentManager
271-
}
272-
return &sql.ConsentPersister{BasePersister: m.basePersister}
273-
}
266+
func (m *RegistrySQL) ConsentManager() consent.Manager { return m.Persister() }
274267
func (m *RegistrySQL) ObfuscatedSubjectManager() consent.ObfuscatedSubjectManager {
275268
return m.Persister()
276269
}
@@ -387,14 +380,8 @@ func (m *RegistrySQL) HealthHandler() *healthx.Handler {
387380
}
388381

389382
if status.HasPending() {
390-
var notApplied []string
391-
for _, s := range status {
392-
if s.State != "Applied" {
393-
notApplied = append(notApplied, s.Version)
394-
}
395-
}
396-
err := errors.Errorf("migrations have not yet been fully applied: %+v", notApplied)
397-
m.Logger().WithField("not_applied", fmt.Sprintf("%+v", notApplied)).WithError(err).Warn("Instance is not yet ready because migrations have not yet been fully applied.")
383+
err := errors.Errorf("migrations have not yet been fully applied: %+v", status)
384+
m.Logger().WithField("status", fmt.Sprintf("%+v", status)).WithError(err).Warn("Instance is not yet ready because migrations have not yet been fully applied.")
398385
return err
399386
}
400387
return nil

‎flow/encoding_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func TestEncoding(t *testing.T) {
7474
State: 1,
7575
LoginRemember: true,
7676
LoginRememberFor: 3600,
77-
Context: sqlxx.NullJSONRawMessage(`{"context-key1": "val1"}`),
77+
Context: sqlxx.JSONRawMessage(`{"context-key1": "val1"}`),
7878
GrantedScope: []string{"scope1", "scope2"},
7979
GrantedAudience: []string{"https://api.example.org/v1", "https://api.example.org/v2"},
8080
ConsentRemember: true,

‎flow/flow.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ type Flow struct {
228228
// Context is an optional object which can hold arbitrary data. The data will be made available when fetching the
229229
// consent request under the "context" field. This is useful in scenarios where login and consent endpoints share
230230
// data.
231-
Context sqlxx.NullJSONRawMessage `db:"context" json:"ct"`
231+
Context sqlxx.JSONRawMessage `db:"context" json:"ct"`
232232

233233
LoginError *RequestDeniedError `db:"-" json:"le,omitempty"`
234234
LoginAuthenticatedAt sqlxx.NullTime `db:"-" json:"la,omitempty"`
@@ -317,7 +317,7 @@ func (f *Flow) HandleLoginRequest(h *HandledLoginRequest) error {
317317
f.State = FlowStateLoginUnused
318318

319319
if f.Context != nil {
320-
f.Context = sqlxx.NullJSONRawMessage(h.Context)
320+
f.Context = h.Context
321321
}
322322

323323
f.Subject = h.Subject
@@ -398,7 +398,7 @@ func (f *Flow) HandleConsentRequest(r *AcceptOAuth2ConsentRequest) error {
398398
f.ConsentHandledAt = sqlxx.NullTime(time.Now().UTC())
399399
f.ConsentError = nil
400400
if r.Context != nil {
401-
f.Context = sqlxx.NullJSONRawMessage(r.Context)
401+
f.Context = r.Context
402402
}
403403

404404
if r.Session != nil {
@@ -455,7 +455,7 @@ func (f *Flow) GetConsentRequest(challenge string) *OAuth2ConsentRequest {
455455
LoginSessionID: f.SessionID,
456456
ACR: f.ACR,
457457
AMR: f.AMR,
458-
Context: sqlxx.JSONRawMessage(f.Context),
458+
Context: f.Context,
459459
}
460460
// set some defaults for the API
461461
if cs.RequestedAudience == nil {
@@ -475,6 +475,9 @@ func (f *Flow) BeforeSave(_ *pop.Connection) error {
475475
if f.Client != nil {
476476
f.ClientID = f.Client.GetID()
477477
}
478+
if f.State == FlowStateLoginUnused && string(f.Context) == "" {
479+
f.Context = sqlxx.JSONRawMessage("{}")
480+
}
478481
return nil
479482
}
480483

@@ -553,7 +556,7 @@ func (f Flow) ToListConsentSessionResponse() *OAuth2ConsentSession {
553556
Session: &AcceptOAuth2ConsentRequestSession{AccessToken: f.SessionAccessToken, IDToken: f.SessionIDToken},
554557
Remember: f.ConsentRemember,
555558
HandledAt: f.ConsentHandledAt,
556-
Context: sqlxx.JSONRawMessage(f.Context),
559+
Context: f.Context,
557560
ConsentRequest: f.GetConsentRequest( /* No longer available and no longer needed: challenge = */ ""),
558561
}
559562
s.ConsentRequest.Client.Secret = "" // do not leak client secret in response

‎flow/flow_encoding_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func createTestFlow(nid uuid.UUID, state flow.State) *flow.Flow {
6565
ACR: "http://acrvalues.example.org",
6666
AMR: []string{"pwd"},
6767
ForceSubjectIdentifier: "forced-subject",
68-
Context: sqlxx.NullJSONRawMessage(`{"foo":"bar"}`),
68+
Context: sqlxx.JSONRawMessage(`{"foo":"bar"}`),
6969
LoginAuthenticatedAt: sqlxx.NullTime(time.Date(2025, 10, 9, 12, 52, 0, 0, time.UTC)),
7070
DeviceChallengeID: "device-challenge",
7171
DeviceCodeRequestID: "device-code-request",

‎flow/flow_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func (f *Flow) setConsentRequest(r OAuth2ConsentRequest) {
4040
f.SessionID = r.LoginSessionID
4141
f.ACR = r.ACR
4242
f.AMR = r.AMR
43-
f.Context = sqlxx.NullJSONRawMessage(r.Context)
43+
f.Context = r.Context
4444
}
4545

4646
func TestFlow_HandleDeviceUserAuthRequest(t *testing.T) {
@@ -110,7 +110,7 @@ func TestFlow_UpdateFlowWithHandledLoginRequest(t *testing.T) {
110110
assert.Equal(t, r.ACR, f.ACR)
111111
assert.Equal(t, r.AMR, f.AMR)
112112
assert.Equal(t, r.IdentityProviderSessionID, f.IdentityProviderSessionID.String())
113-
assert.EqualValues(t, r.Context, f.Context)
113+
assert.Equal(t, r.Context, f.Context)
114114
},
115115
)
116116
}

‎persistence/definitions.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
type (
1818
Persister interface {
19+
consent.Manager
1920
consent.ObfuscatedSubjectManager
2021
consent.LoginManager
2122
consent.LogoutManager

0 commit comments

Comments
 (0)