Skip to content

Commit

Permalink
fix: incorrect consent removal on authentication revokation
Browse files Browse the repository at this point in the history
This patch resolves a regression where, in a certain condition, an accepted consent could be incorrectly deleted when the related authentication session was removed.
  • Loading branch information
aeneasr committed Nov 10, 2022
1 parent cec489f commit ccf2388
Show file tree
Hide file tree
Showing 12 changed files with 71 additions and 6 deletions.
51 changes: 51 additions & 0 deletions consent/manager_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,57 @@ func ManagerTests(m Manager, clientManager client.Manager, fositeManager x.Fosit
}
})
})

t.Run("case=foreign key regression", func(t *testing.T) {
cl := &client.Client{LegacyClientID: uuid.New().String()}
require.NoError(t, clientManager.CreateClient(context.Background(), cl))

subject := uuid.New().String()
s := LoginSession{
ID: uuid.New().String(),
AuthenticatedAt: sqlxx.NullTime(time.Now().Round(time.Minute).Add(-time.Minute).UTC()),
Subject: subject,
}

err := m.CreateLoginSession(context.Background(), &s)
require.NoError(t, err)

lr := &LoginRequest{
ID: uuid.New().String(),
Subject: uuid.New().String(),
Verifier: uuid.New().String(),
Client: cl,
AuthenticatedAt: sqlxx.NullTime(time.Now()),
RequestedAt: time.Now(),
SessionID: sqlxx.NullString(s.ID),
}

require.NoError(t, m.CreateLoginRequest(context.Background(), lr))
expected := &OAuth2ConsentRequest{
ID: uuid.New().String(),
Skip: true,
Subject: subject,
OpenIDConnectContext: nil,
Client: cl,
ClientID: cl.LegacyClientID,
RequestURL: "",
LoginChallenge: sqlxx.NullString(lr.ID),
LoginSessionID: sqlxx.NullString(s.ID),
Verifier: uuid.New().String(),
CSRF: uuid.New().String(),
}
require.NoError(t, m.CreateConsentRequest(context.Background(), expected))

result, err := m.GetConsentRequest(context.Background(), expected.ID)
require.NoError(t, err)
assert.EqualValues(t, expected.ID, result.ID)

require.NoError(t, m.DeleteLoginSession(context.Background(), s.ID))

result, err = m.GetConsentRequest(context.Background(), expected.ID)
require.NoError(t, err)
assert.EqualValues(t, expected.ID, result.ID)
})
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ CREATE TABLE "_hydra_oauth2_flow_tmp" (
client_id VARCHAR(255) NOT NULL,
requested_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
oidc_context TEXT NOT NULL,
login_session_id VARCHAR(40) NULL REFERENCES hydra_oauth2_authentication_session (id) ON DELETE CASCADE DEFAULT '',
login_session_id VARCHAR(40) NULL REFERENCES hydra_oauth2_authentication_session (id) ON DELETE SET NULL,
requested_at_audience TEXT NULL DEFAULT '',
login_initialized_at TIMESTAMP NULL,

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE ONLY hydra_oauth2_flow ALTER COLUMN login_session_id SET DEFAULT '';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE hydra_oauth2_flow DROP CONSTRAINT hydra_oauth2_flow_login_session_id_fk;
ALTER TABLE hydra_oauth2_flow ADD CONSTRAINT hydra_oauth2_flow_login_session_id_fk FOREIGN KEY (login_session_id) REFERENCES public.hydra_oauth2_authentication_session(id) ON DELETE SET NULL;
ALTER TABLE hydra_oauth2_flow ALTER COLUMN login_session_id DROP DEFAULT;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE ONLY hydra_oauth2_flow ALTER COLUMN login_session_id SET DEFAULT '';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE hydra_oauth2_flow DROP CONSTRAINT hydra_oauth2_flow_login_session_id_fk;
ALTER TABLE hydra_oauth2_flow ADD CONSTRAINT hydra_oauth2_flow_login_session_id_fk FOREIGN KEY (login_session_id) REFERENCES hydra_oauth2_authentication_session(id) ON DELETE SET NULL;
ALTER TABLE hydra_oauth2_flow ALTER COLUMN login_session_id DROP DEFAULT;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE ONLY hydra_oauth2_flow ALTER COLUMN login_session_id SET DEFAULT '';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE hydra_oauth2_flow DROP CONSTRAINT hydra_oauth2_flow_login_session_id_fk;
ALTER TABLE hydra_oauth2_flow ADD CONSTRAINT hydra_oauth2_flow_login_session_id_fk FOREIGN KEY (login_session_id) REFERENCES public.hydra_oauth2_authentication_session(id) ON DELETE SET NULL;
ALTER TABLE hydra_oauth2_flow ALTER COLUMN login_session_id DROP DEFAULT;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--
8 changes: 4 additions & 4 deletions persistence/sql/persister_consent.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,13 @@ func (p *Persister) CreateLoginRequest(ctx context.Context, req *consent.LoginRe
return sqlcon.HandleError(p.CreateWithNetwork(ctx, f))
}

func (p *Persister) GetFlow(ctx context.Context, login_challenge string) (*flow.Flow, error) {
func (p *Persister) GetFlow(ctx context.Context, loginChallenge string) (*flow.Flow, error) {
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.GetFlow")
defer span.End()

var f flow.Flow
return &f, p.transaction(ctx, func(ctx context.Context, c *pop.Connection) error {
if err := p.QueryWithNetwork(ctx).Where("login_challenge = ?", login_challenge).First(&f); err != nil {
if err := p.QueryWithNetwork(ctx).Where("login_challenge = ?", loginChallenge).First(&f); err != nil {
if errors.Is(err, sql.ErrNoRows) {
return errorsx.WithStack(x.ErrNotFound)
}
Expand All @@ -242,14 +242,14 @@ func (p *Persister) GetFlow(ctx context.Context, login_challenge string) (*flow.
})
}

func (p *Persister) GetLoginRequest(ctx context.Context, login_challenge string) (*consent.LoginRequest, error) {
func (p *Persister) GetLoginRequest(ctx context.Context, loginChallenge string) (*consent.LoginRequest, error) {
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.GetLoginRequest")
defer span.End()

var lr *consent.LoginRequest
return lr, p.transaction(ctx, func(ctx context.Context, c *pop.Connection) error {
var f flow.Flow
if err := p.QueryWithNetwork(ctx).Where("login_challenge = ?", login_challenge).First(&f); err != nil {
if err := p.QueryWithNetwork(ctx).Where("login_challenge = ?", loginChallenge).First(&f); err != nil {
if errors.Is(err, sql.ErrNoRows) {
return errorsx.WithStack(x.ErrNotFound)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ CREATE TABLE "_hydra_oauth2_flow_tmp" (
client_id VARCHAR(255) NOT NULL,
requested_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
oidc_context TEXT NOT NULL,
login_session_id VARCHAR(40) NULL REFERENCES hydra_oauth2_authentication_session (id) ON DELETE CASCADE DEFAULT '',
login_session_id VARCHAR(40) NULL REFERENCES hydra_oauth2_authentication_session (id) ON DELETE SET NULL,
requested_at_audience TEXT NULL DEFAULT '',
login_initialized_at TIMESTAMP NULL,

Expand Down

0 comments on commit ccf2388

Please sign in to comment.