From abf1c86ab9d3204b0bbeb1628574430c6da166d0 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Mon, 25 Apr 2022 10:31:05 +1000 Subject: [PATCH] fix(oidc): subject generated for anonymous users (#3238) Fix and issue that would prevent a correct ID Token from being generated for users who start off anonymous. This also avoids generating one in the first place for anonymous users. --- .../handlers/handler_oauth_introspection.go | 2 +- internal/handlers/handler_oauth_revocation.go | 2 +- .../handlers/handler_oidc_authorization.go | 19 +- .../handler_oidc_authorization_consent.go | 114 ++++++-- internal/handlers/handler_oidc_token.go | 4 +- internal/mocks/storage.go | 14 + internal/model/oidc.go | 4 +- internal/model/types.go | 28 ++ internal/oidc/types_test.go | 2 +- internal/storage/const.go | 2 +- .../V0005.ConsentSubjectNULL.mysql.down.sql | 3 + .../V0005.ConsentSubjectNULL.mysql.up.sql | 4 + ...V0005.ConsentSubjectNULL.postgres.down.sql | 1 + .../V0005.ConsentSubjectNULL.postgres.up.sql | 3 + .../V0005.ConsentSubjectNULL.sqlite.down.sql | 1 + .../V0005.ConsentSubjectNULL.sqlite.up.sql | 251 ++++++++++++++++++ internal/storage/provider.go | 1 + internal/storage/sql_provider.go | 18 +- .../storage/sql_provider_backend_postgres.go | 1 + internal/storage/sql_provider_queries.go | 5 + 20 files changed, 435 insertions(+), 44 deletions(-) create mode 100644 internal/storage/migrations/V0005.ConsentSubjectNULL.mysql.down.sql create mode 100644 internal/storage/migrations/V0005.ConsentSubjectNULL.mysql.up.sql create mode 100644 internal/storage/migrations/V0005.ConsentSubjectNULL.postgres.down.sql create mode 100644 internal/storage/migrations/V0005.ConsentSubjectNULL.postgres.up.sql create mode 100644 internal/storage/migrations/V0005.ConsentSubjectNULL.sqlite.down.sql create mode 100644 internal/storage/migrations/V0005.ConsentSubjectNULL.sqlite.up.sql diff --git a/internal/handlers/handler_oauth_introspection.go b/internal/handlers/handler_oauth_introspection.go index 990e8c5c2..f53e5a195 100644 --- a/internal/handlers/handler_oauth_introspection.go +++ b/internal/handlers/handler_oauth_introspection.go @@ -23,7 +23,7 @@ func OAuthIntrospectionPOST(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter if responder, err = ctx.Providers.OpenIDConnect.Fosite.NewIntrospectionRequest(ctx, req, oidcSession); err != nil { rfc := fosite.ErrorToRFC6749Error(err) - ctx.Logger.Errorf("Introspection Request failed with error: %s", rfc.GetDescription()) + ctx.Logger.Errorf("Introspection Request failed with error: %s", rfc.WithExposeDebug(true).GetDescription()) ctx.Providers.OpenIDConnect.Fosite.WriteIntrospectionError(rw, err) diff --git a/internal/handlers/handler_oauth_revocation.go b/internal/handlers/handler_oauth_revocation.go index 6f86596b1..95af94b9e 100644 --- a/internal/handlers/handler_oauth_revocation.go +++ b/internal/handlers/handler_oauth_revocation.go @@ -17,7 +17,7 @@ func OAuthRevocationPOST(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r if err = ctx.Providers.OpenIDConnect.Fosite.NewRevocationRequest(ctx, req); err != nil { rfc := fosite.ErrorToRFC6749Error(err) - ctx.Logger.Errorf("Revocation Request failed with error: %s", rfc.GetDescription()) + ctx.Logger.Errorf("Revocation Request failed with error: %s", rfc.WithExposeDebug(true).GetDescription()) } ctx.Providers.OpenIDConnect.Fosite.WriteRevocationResponse(rw, err) diff --git a/internal/handlers/handler_oidc_authorization.go b/internal/handlers/handler_oidc_authorization.go index 8344550b2..a6743c042 100644 --- a/internal/handlers/handler_oidc_authorization.go +++ b/internal/handlers/handler_oidc_authorization.go @@ -5,7 +5,6 @@ import ( "net/http" "time" - "github.com/google/uuid" "github.com/ory/fosite" "github.com/authelia/authelia/v4/internal/middlewares" @@ -29,7 +28,7 @@ func OpenIDConnectAuthorizationGET(ctx *middlewares.AutheliaCtx, rw http.Respons if requester, err = ctx.Providers.OpenIDConnect.Fosite.NewAuthorizeRequest(ctx, r); err != nil { rfc := fosite.ErrorToRFC6749Error(err) - ctx.Logger.Errorf("Authorization Request failed with error: %s", rfc.GetDescription()) + ctx.Logger.Errorf("Authorization Request failed with error: %s", rfc.WithExposeDebug(true).GetDescription()) ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, requester, err) @@ -62,14 +61,18 @@ func OpenIDConnectAuthorizationGET(ctx *middlewares.AutheliaCtx, rw http.Respons userSession := ctx.GetSession() - var subject uuid.UUID + var subject model.NullUUID - if subject, err = ctx.Providers.OpenIDConnect.Store.GetSubject(ctx, client.GetSectorIdentifier(), userSession.Username); err != nil { - ctx.Logger.Errorf("Authorization Request with id '%s' on client with id '%s' could not be processed: error occurred retrieving subject for user '%s': %+v", requester.GetID(), client.GetID(), userSession.Username, err) + if userSession.Username != "" { + if subject.UUID, err = ctx.Providers.OpenIDConnect.Store.GetSubject(ctx, client.GetSectorIdentifier(), userSession.Username); err != nil { + ctx.Logger.Errorf("Authorization Request with id '%s' on client with id '%s' could not be processed: error occurred retrieving subject for user '%s': %+v", requester.GetID(), client.GetID(), userSession.Username, err) - ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, requester, fosite.ErrServerError.WithHint("Could not retrieve the subject.")) + ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, requester, fosite.ErrServerError.WithHint("Could not retrieve the subject.")) - return + return + } + + subject.Valid = true } var ( @@ -104,7 +107,7 @@ func OpenIDConnectAuthorizationGET(ctx *middlewares.AutheliaCtx, rw http.Respons if responder, err = ctx.Providers.OpenIDConnect.Fosite.NewAuthorizeResponse(ctx, requester, oidcSession); err != nil { rfc := fosite.ErrorToRFC6749Error(err) - ctx.Logger.Errorf("Authorization Response for Request with id '%s' on client with id '%s' could not be created: %s", requester.GetID(), clientID, rfc.GetDescription()) + ctx.Logger.Errorf("Authorization Response for Request with id '%s' on client with id '%s' could not be created: %s", requester.GetID(), clientID, rfc.WithExposeDebug(true).GetDescription()) ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, requester, err) diff --git a/internal/handlers/handler_oidc_authorization_consent.go b/internal/handlers/handler_oidc_authorization_consent.go index 7c94f8eac..0230d4ec9 100644 --- a/internal/handlers/handler_oidc_authorization_consent.go +++ b/internal/handlers/handler_oidc_authorization_consent.go @@ -17,12 +17,18 @@ import ( ) func handleOIDCAuthorizationConsent(ctx *middlewares.AutheliaCtx, rootURI string, client *oidc.Client, - userSession session.UserSession, subject uuid.UUID, + userSession session.UserSession, subject model.NullUUID, rw http.ResponseWriter, r *http.Request, requester fosite.AuthorizeRequester) (consent *model.OAuth2ConsentSession, handled bool) { if userSession.ConsentChallengeID != nil { + ctx.Logger.Debugf("Authorization Request with id '%s' on client with id '%s' proceeding to lookup consent by challenge id '%s'", requester.GetID(), client.GetID(), userSession.ConsentChallengeID) + return handleOIDCAuthorizationConsentWithChallengeID(ctx, rootURI, client, userSession, rw, r, requester) } + if !subject.Valid { + return handleOIDCAuthorizationConsentGenerate(ctx, rootURI, client, userSession, subject, rw, r, requester) + } + return handleOIDCAuthorizationConsentOrGenerate(ctx, rootURI, client, userSession, subject, rw, r, requester) } @@ -47,6 +53,26 @@ func handleOIDCAuthorizationConsentWithChallengeID(ctx *middlewares.AutheliaCtx, return nil, true } + if !consent.Subject.Valid { + if consent.Subject.UUID, err = ctx.Providers.OpenIDConnect.Store.GetSubject(ctx, client.GetSectorIdentifier(), userSession.Username); err != nil { + ctx.Logger.Errorf("Authorization Request with id '%s' on client with id '%s' could not be processed: error occurred retrieving subject for user '%s': %+v", requester.GetID(), client.GetID(), userSession.Username, err) + + ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, requester, fosite.ErrServerError.WithHint("Could not retrieve the subject.")) + + return nil, true + } + + consent.Subject.Valid = true + + if err = ctx.Providers.StorageProvider.SaveOAuth2ConsentSessionSubject(ctx, *consent); err != nil { + ctx.Logger.Errorf("Authorization Request with id '%s' on client with id '%s' could not be processed: error occurred updating consent session subject for user '%s': %+v", requester.GetID(), client.GetID(), userSession.Username, err) + + ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, requester, fosite.ErrServerError.WithHint("Could not update the consent session subject.")) + + return nil, true + } + } + if consent.Responded() { userSession.ConsentChallengeID = nil @@ -85,40 +111,38 @@ func handleOIDCAuthorizationConsentWithChallengeID(ctx *middlewares.AutheliaCtx, } func handleOIDCAuthorizationConsentOrGenerate(ctx *middlewares.AutheliaCtx, rootURI string, client *oidc.Client, - userSession session.UserSession, subject uuid.UUID, + userSession session.UserSession, subject model.NullUUID, rw http.ResponseWriter, r *http.Request, requester fosite.AuthorizeRequester) (consent *model.OAuth2ConsentSession, handled bool) { var ( - rows *storage.ConsentSessionRows - scopes, audience []string - err error + err error ) - if rows, err = ctx.Providers.StorageProvider.LoadOAuth2ConsentSessionsPreConfigured(ctx, client.GetID(), subject); err != nil { + scopes, audience := getExpectedScopesAndAudience(requester) + + if consent, err = getOIDCPreconfiguredConsent(ctx, client.GetID(), subject.UUID, scopes, audience); err != nil { ctx.Logger.Errorf("Authorization Request with id '%s' on client with id '%s' had error looking up pre-configured consent sessions: %+v", requester.GetID(), requester.GetClient().GetID(), err) + + ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, requester, fosite.ErrServerError.WithHint("Could not lookup the consent session.")) + + return nil, true } - defer rows.Close() + if consent != nil { + ctx.Logger.Debugf("Authorization Request with id '%s' on client with id '%s' successfully looked up pre-configured consent with challenge id '%s'", requester.GetID(), client.GetID(), consent.ChallengeID) - for rows.Next() { - if consent, err = rows.Get(); err != nil { - ctx.Logger.Errorf("Authorization Request with id '%s' on client with id '%s' had error looking up pre-configured consent sessions: %+v", requester.GetID(), requester.GetClient().GetID(), err) - - ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, requester, fosite.ErrServerError.WithHint("Could not lookup pre-configured consent sessions.")) - - return nil, true - } - - scopes, audience = getExpectedScopesAndAudience(requester) - - if consent.HasExactGrants(scopes, audience) && consent.CanGrant() { - break - } - } - - if consent != nil && consent.HasExactGrants(scopes, audience) && consent.CanGrant() { return consent, false } + ctx.Logger.Debugf("Authorization Request with id '%s' on client with id '%s' proceeding to generate a new consent due to unsuccessful lookup of pre-configured consent", requester.GetID(), client.GetID()) + + return handleOIDCAuthorizationConsentGenerate(ctx, rootURI, client, userSession, subject, rw, r, requester) +} + +func handleOIDCAuthorizationConsentGenerate(ctx *middlewares.AutheliaCtx, rootURI string, client *oidc.Client, + userSession session.UserSession, subject model.NullUUID, + rw http.ResponseWriter, r *http.Request, requester fosite.AuthorizeRequester) (consent *model.OAuth2ConsentSession, handled bool) { + var err error + if consent, err = model.NewOAuth2ConsentSession(subject, requester); err != nil { ctx.Logger.Errorf("Authorization Request with id '%s' on client with id '%s' could not be processed: error occurred generating consent: %+v", requester.GetID(), requester.GetClient().GetID(), err) @@ -166,3 +190,45 @@ func getExpectedScopesAndAudience(requester fosite.Requester) (scopes, audience return requester.GetRequestedScopes(), audience } + +func getOIDCPreconfiguredConsent(ctx *middlewares.AutheliaCtx, clientID string, subject uuid.UUID, scopes, audience []string) (consent *model.OAuth2ConsentSession, err error) { + var ( + rows *storage.ConsentSessionRows + ) + + ctx.Logger.Debugf("Consent Session is being checked for pre-configuration with signature of client id '%s' and subject '%s'", clientID, subject) + + if rows, err = ctx.Providers.StorageProvider.LoadOAuth2ConsentSessionsPreConfigured(ctx, clientID, subject); err != nil { + ctx.Logger.Debugf("Consent Session checked for pre-configuration with signature of client id '%s' and subject '%s' failed with error during load: %+v", clientID, subject, err) + + return nil, err + } + + defer func() { + if err := rows.Close(); err != nil { + ctx.Logger.Errorf("Consent Session checked for pre-configuration with signature of client id '%s' and subject '%s' failed to close rows with error: %+v", clientID, subject, err) + } + }() + + for rows.Next() { + if consent, err = rows.Get(); err != nil { + ctx.Logger.Debugf("Consent Session checked for pre-configuration with signature of client id '%s' and subject '%s' failed with error during iteration: %+v", clientID, subject, err) + + return nil, err + } + + if consent.HasExactGrants(scopes, audience) && consent.CanGrant() { + break + } + } + + if consent != nil && consent.HasExactGrants(scopes, audience) && consent.CanGrant() { + ctx.Logger.Debugf("Consent Session checked for pre-configuration with signature of client id '%s' and subject '%s' found a result with challenge id '%s'", clientID, subject, consent.ChallengeID) + + return consent, nil + } + + ctx.Logger.Debugf("Consent Session checked for pre-configuration with signature of client id '%s' and subject '%s' did not find any results", clientID, subject) + + return nil, nil +} diff --git a/internal/handlers/handler_oidc_token.go b/internal/handlers/handler_oidc_token.go index c46afa358..bda2205ae 100644 --- a/internal/handlers/handler_oidc_token.go +++ b/internal/handlers/handler_oidc_token.go @@ -24,7 +24,7 @@ func OpenIDConnectTokenPOST(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter if requester, err = ctx.Providers.OpenIDConnect.Fosite.NewAccessRequest(ctx, req, oidcSession); err != nil { rfc := fosite.ErrorToRFC6749Error(err) - ctx.Logger.Errorf("Access Request failed with error: %s", rfc.GetDescription()) + ctx.Logger.Errorf("Access Request failed with error: %s", rfc.WithExposeDebug(true).GetDescription()) ctx.Providers.OpenIDConnect.Fosite.WriteAccessError(rw, requester, err) @@ -47,7 +47,7 @@ func OpenIDConnectTokenPOST(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter if responder, err = ctx.Providers.OpenIDConnect.Fosite.NewAccessResponse(ctx, requester); err != nil { rfc := fosite.ErrorToRFC6749Error(err) - ctx.Logger.Errorf("Access Response for Request with id '%s' failed to be created with error: %s", requester.GetID(), rfc.GetDescription()) + ctx.Logger.Errorf("Access Response for Request with id '%s' failed to be created with error: %s", requester.GetID(), rfc.WithExposeDebug(true).GetDescription()) ctx.Providers.OpenIDConnect.Fosite.WriteAccessError(rw, requester, err) diff --git a/internal/mocks/storage.go b/internal/mocks/storage.go index 9371fbe2d..2f6160d76 100644 --- a/internal/mocks/storage.go +++ b/internal/mocks/storage.go @@ -518,6 +518,20 @@ func (mr *MockStorageMockRecorder) SaveOAuth2ConsentSessionResponse(arg0, arg1, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveOAuth2ConsentSessionResponse", reflect.TypeOf((*MockStorage)(nil).SaveOAuth2ConsentSessionResponse), arg0, arg1, arg2) } +// SaveOAuth2ConsentSessionSubject mocks base method. +func (m *MockStorage) SaveOAuth2ConsentSessionSubject(arg0 context.Context, arg1 model.OAuth2ConsentSession) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SaveOAuth2ConsentSessionSubject", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// SaveOAuth2ConsentSessionSubject indicates an expected call of SaveOAuth2ConsentSessionSubject. +func (mr *MockStorageMockRecorder) SaveOAuth2ConsentSessionSubject(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveOAuth2ConsentSessionSubject", reflect.TypeOf((*MockStorage)(nil).SaveOAuth2ConsentSessionSubject), arg0, arg1) +} + // SaveOAuth2Session mocks base method. func (m *MockStorage) SaveOAuth2Session(arg0 context.Context, arg1 storage.OAuth2SessionType, arg2 model.OAuth2Session) error { m.ctrl.T.Helper() diff --git a/internal/model/oidc.go b/internal/model/oidc.go index 2c1325561..d20b4cc8e 100644 --- a/internal/model/oidc.go +++ b/internal/model/oidc.go @@ -17,7 +17,7 @@ import ( ) // NewOAuth2ConsentSession creates a new OAuth2ConsentSession. -func NewOAuth2ConsentSession(subject uuid.UUID, r fosite.Requester) (consent *OAuth2ConsentSession, err error) { +func NewOAuth2ConsentSession(subject NullUUID, r fosite.Requester) (consent *OAuth2ConsentSession, err error) { consent = &OAuth2ConsentSession{ ClientID: r.GetClient().GetID(), Subject: subject, @@ -86,7 +86,7 @@ type OAuth2ConsentSession struct { ID int `db:"id"` ChallengeID uuid.UUID `db:"challenge_id"` ClientID string `db:"client_id"` - Subject uuid.UUID `db:"subject"` + Subject NullUUID `db:"subject"` Authorized bool `db:"authorized"` Granted bool `db:"granted"` diff --git a/internal/model/types.go b/internal/model/types.go index 674e6aa43..b326e76af 100644 --- a/internal/model/types.go +++ b/internal/model/types.go @@ -7,9 +7,37 @@ import ( "fmt" "net" + "github.com/google/uuid" + "github.com/authelia/authelia/v4/internal/utils" ) +// NullUUID is a nullable uuid.UUID. +type NullUUID struct { + uuid.UUID + Valid bool +} + +// Value is the NullUUID implementation of the databases/sql driver.Valuer. +func (u NullUUID) Value() (value driver.Value, err error) { + if !u.Valid { + return nil, nil + } + + return u.UUID.Value() +} + +// Scan is the NullUUID implementation of the sql.Scanner. +func (u *NullUUID) Scan(src interface{}) (err error) { + if src == nil { + u.UUID, u.Valid = uuid.UUID{}, false + + return nil + } + + return u.UUID.Scan(src) +} + // NewIP easily constructs a new IP. func NewIP(value net.IP) (ip IP) { return IP{IP: value} diff --git a/internal/oidc/types_test.go b/internal/oidc/types_test.go index 7a96558ac..9702bae21 100644 --- a/internal/oidc/types_test.go +++ b/internal/oidc/types_test.go @@ -56,7 +56,7 @@ func TestNewSessionWithAuthorizeRequest(t *testing.T) { consent := &model.OAuth2ConsentSession{ ChallengeID: uuid.New(), RequestedAt: requested, - Subject: subject, + Subject: model.NullUUID{UUID: subject, Valid: true}, } session := NewSessionWithAuthorizeRequest(issuer, "primary", "john", amr, extra, authAt, consent, request) diff --git a/internal/storage/const.go b/internal/storage/const.go index cf188b10e..4f451d8e3 100644 --- a/internal/storage/const.go +++ b/internal/storage/const.go @@ -78,7 +78,7 @@ const ( const ( // This is the latest schema version for the purpose of tests. - testLatestVersion = 4 + testLatestVersion = 5 ) const ( diff --git a/internal/storage/migrations/V0005.ConsentSubjectNULL.mysql.down.sql b/internal/storage/migrations/V0005.ConsentSubjectNULL.mysql.down.sql new file mode 100644 index 000000000..95aab5ef0 --- /dev/null +++ b/internal/storage/migrations/V0005.ConsentSubjectNULL.mysql.down.sql @@ -0,0 +1,3 @@ +ALTER TABLE oauth2_consent_session +DROP FOREIGN KEY oauth2_consent_session_subject_fkey, + ADD CONSTRAINT oauth2_consent_subject_fkey FOREIGN KEY (subject) REFERENCES user_opaque_identifier(identifier) ON UPDATE RESTRICT ON DELETE RESTRICT; \ No newline at end of file diff --git a/internal/storage/migrations/V0005.ConsentSubjectNULL.mysql.up.sql b/internal/storage/migrations/V0005.ConsentSubjectNULL.mysql.up.sql new file mode 100644 index 000000000..eac4de80f --- /dev/null +++ b/internal/storage/migrations/V0005.ConsentSubjectNULL.mysql.up.sql @@ -0,0 +1,4 @@ +ALTER TABLE oauth2_consent_session MODIFY subject CHAR(36) NULL DEFAULT NULL; +ALTER TABLE oauth2_consent_session + DROP FOREIGN KEY oauth2_consent_subject_fkey, + ADD CONSTRAINT oauth2_consent_session_subject_fkey FOREIGN KEY (subject) REFERENCES user_opaque_identifier(identifier) ON UPDATE RESTRICT ON DELETE RESTRICT; diff --git a/internal/storage/migrations/V0005.ConsentSubjectNULL.postgres.down.sql b/internal/storage/migrations/V0005.ConsentSubjectNULL.postgres.down.sql new file mode 100644 index 000000000..7fd6d685d --- /dev/null +++ b/internal/storage/migrations/V0005.ConsentSubjectNULL.postgres.down.sql @@ -0,0 +1 @@ +ALTER TABLE oauth2_consent_session RENAME CONSTRAINT oauth2_consent_session_subject_fkey TO oauth2_consent_subject_fkey; \ No newline at end of file diff --git a/internal/storage/migrations/V0005.ConsentSubjectNULL.postgres.up.sql b/internal/storage/migrations/V0005.ConsentSubjectNULL.postgres.up.sql new file mode 100644 index 000000000..0645384c3 --- /dev/null +++ b/internal/storage/migrations/V0005.ConsentSubjectNULL.postgres.up.sql @@ -0,0 +1,3 @@ +ALTER TABLE oauth2_consent_session ALTER COLUMN subject DROP NOT NULL; +ALTER TABLE oauth2_consent_session ALTER COLUMN subject SET DEFAULT NULL; +ALTER TABLE oauth2_consent_session RENAME CONSTRAINT oauth2_consent_subject_fkey TO oauth2_consent_session_subject_fkey; diff --git a/internal/storage/migrations/V0005.ConsentSubjectNULL.sqlite.down.sql b/internal/storage/migrations/V0005.ConsentSubjectNULL.sqlite.down.sql new file mode 100644 index 000000000..e0ac49d1e --- /dev/null +++ b/internal/storage/migrations/V0005.ConsentSubjectNULL.sqlite.down.sql @@ -0,0 +1 @@ +SELECT 1; diff --git a/internal/storage/migrations/V0005.ConsentSubjectNULL.sqlite.up.sql b/internal/storage/migrations/V0005.ConsentSubjectNULL.sqlite.up.sql new file mode 100644 index 000000000..3526a0c2f --- /dev/null +++ b/internal/storage/migrations/V0005.ConsentSubjectNULL.sqlite.up.sql @@ -0,0 +1,251 @@ +PRAGMA foreign_keys=off; + +BEGIN TRANSACTION; + +ALTER TABLE oauth2_consent_session RENAME TO _bkp_UP_V0005_oauth2_consent_session; + +CREATE TABLE IF NOT EXISTS oauth2_consent_session ( + id INTEGER, + challenge_id CHAR(36) NOT NULL, + client_id VARCHAR(255) NOT NULL, + subject CHAR(36) NULL DEFAULT NULL, + authorized BOOLEAN NOT NULL DEFAULT FALSE, + granted BOOLEAN NOT NULL DEFAULT FALSE, + requested_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + responded_at TIMESTAMP NULL DEFAULT NULL, + expires_at TIMESTAMP NULL DEFAULT NULL, + form_data TEXT NOT NULL, + requested_scopes TEXT NOT NULL, + granted_scopes TEXT NOT NULL, + requested_audience TEXT NULL DEFAULT '', + granted_audience TEXT NULL DEFAULT '', + PRIMARY KEY (id), + CONSTRAINT oauth2_consent_session_subject_fkey + FOREIGN KEY(subject) + REFERENCES user_opaque_identifier(identifier) ON UPDATE RESTRICT ON DELETE RESTRICT +); + +INSERT INTO oauth2_consent_session (id, challenge_id, client_id, subject, authorized, granted, requested_at, responded_at, expires_at, form_data, requested_scopes, granted_scopes, requested_audience, granted_audience) +SELECT id, challenge_id, client_id, subject, authorized, granted, requested_at, responded_at, expires_at, form_data, requested_scopes, granted_scopes, requested_audience, granted_audience +FROM _bkp_UP_V0005_oauth2_consent_session +ORDER BY id; + +DROP INDEX oauth2_consent_session_challenge_id_key; + +CREATE UNIQUE INDEX oauth2_consent_session_challenge_id_key ON oauth2_consent_session (challenge_id); + +DROP TABLE _bkp_UP_V0005_oauth2_consent_session; + +ALTER TABLE oauth2_authorization_code_session RENAME TO _bkp_UP_V0005_oauth2_authorization_code_session; + +CREATE TABLE IF NOT EXISTS oauth2_authorization_code_session ( + id INTEGER, + challenge_id CHAR(36) NOT NULL, + request_id VARCHAR(40) NOT NULL, + client_id VARCHAR(255) NOT NULL, + signature VARCHAR(255) NOT NULL, + subject CHAR(36) NOT NULL, + requested_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + requested_scopes TEXT NOT NULL, + granted_scopes TEXT NOT NULL, + requested_audience TEXT NULL DEFAULT '', + granted_audience TEXT NULL DEFAULT '', + active BOOLEAN NOT NULL DEFAULT FALSE, + revoked BOOLEAN NOT NULL DEFAULT FALSE, + form_data TEXT NOT NULL, + session_data BLOB NOT NULL, + PRIMARY KEY (id), + CONSTRAINT oauth2_authorization_code_session_challenge_id_fkey + FOREIGN KEY(challenge_id) + REFERENCES oauth2_consent_session(challenge_id) ON UPDATE CASCADE ON DELETE CASCADE, + CONSTRAINT oauth2_authorization_code_session_subject_fkey + FOREIGN KEY(subject) + REFERENCES user_opaque_identifier(identifier) ON UPDATE RESTRICT ON DELETE RESTRICT +); + +INSERT INTO oauth2_authorization_code_session (id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data) +SELECT id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data +FROM _bkp_UP_V0005_oauth2_authorization_code_session +ORDER BY id; + +DROP INDEX oauth2_authorization_code_session_request_id_idx; +DROP INDEX oauth2_authorization_code_session_client_id_idx; +DROP INDEX oauth2_authorization_code_session_client_id_subject_idx; + +CREATE INDEX oauth2_authorization_code_session_request_id_idx ON oauth2_authorization_code_session (request_id); +CREATE INDEX oauth2_authorization_code_session_client_id_idx ON oauth2_authorization_code_session (client_id); +CREATE INDEX oauth2_authorization_code_session_client_id_subject_idx ON oauth2_authorization_code_session (client_id, subject); + +DROP TABLE _bkp_UP_V0005_oauth2_authorization_code_session; + +ALTER TABLE oauth2_access_token_session RENAME TO _bkp_UP_V0005_oauth2_access_token_session; + +CREATE TABLE IF NOT EXISTS oauth2_access_token_session ( + id INTEGER, + challenge_id CHAR(36) NOT NULL, + request_id VARCHAR(40) NOT NULL, + client_id VARCHAR(255) NOT NULL, + signature VARCHAR(255) NOT NULL, + subject CHAR(36) NOT NULL, + requested_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + requested_scopes TEXT NOT NULL, + granted_scopes TEXT NOT NULL, + requested_audience TEXT NULL DEFAULT '', + granted_audience TEXT NULL DEFAULT '', + active BOOLEAN NOT NULL DEFAULT FALSE, + revoked BOOLEAN NOT NULL DEFAULT FALSE, + form_data TEXT NOT NULL, + session_data BLOB NOT NULL, + PRIMARY KEY (id), + CONSTRAINT oauth2_access_token_session_challenge_id_fkey + FOREIGN KEY(challenge_id) + REFERENCES oauth2_consent_session(challenge_id) ON UPDATE CASCADE ON DELETE CASCADE, + CONSTRAINT oauth2_access_token_session_subject_fkey + FOREIGN KEY(subject) + REFERENCES user_opaque_identifier(identifier) ON UPDATE RESTRICT ON DELETE RESTRICT +); + +INSERT INTO oauth2_access_token_session (id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data) +SELECT id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data +FROM _bkp_UP_V0005_oauth2_access_token_session +ORDER BY id; + +DROP INDEX oauth2_access_token_session_request_id_idx; +DROP INDEX oauth2_access_token_session_client_id_idx; +DROP INDEX oauth2_access_token_session_client_id_subject_idx; + +CREATE INDEX oauth2_access_token_session_request_id_idx ON oauth2_access_token_session (request_id); +CREATE INDEX oauth2_access_token_session_client_id_idx ON oauth2_access_token_session (client_id); +CREATE INDEX oauth2_access_token_session_client_id_subject_idx ON oauth2_access_token_session (client_id, subject); + +DROP TABLE _bkp_UP_V0005_oauth2_access_token_session; + +ALTER TABLE oauth2_refresh_token_session RENAME TO _bkp_UP_V0005_oauth2_refresh_token_session; + +CREATE TABLE IF NOT EXISTS oauth2_refresh_token_session ( + id INTEGER, + challenge_id CHAR(36) NOT NULL, + request_id VARCHAR(40) NOT NULL, + client_id VARCHAR(255) NOT NULL, + signature VARCHAR(255) NOT NULL, + subject CHAR(36) NOT NULL, + requested_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + requested_scopes TEXT NOT NULL, + granted_scopes TEXT NOT NULL, + requested_audience TEXT NULL DEFAULT '', + granted_audience TEXT NULL DEFAULT '', + active BOOLEAN NOT NULL DEFAULT FALSE, + revoked BOOLEAN NOT NULL DEFAULT FALSE, + form_data TEXT NOT NULL, + session_data BLOB NOT NULL, + PRIMARY KEY (id), + CONSTRAINT oauth2_refresh_token_session_challenge_id_fkey + FOREIGN KEY(challenge_id) + REFERENCES oauth2_consent_session(challenge_id) ON UPDATE CASCADE ON DELETE CASCADE, + CONSTRAINT oauth2_refresh_token_session_subject_fkey + FOREIGN KEY(subject) + REFERENCES user_opaque_identifier(identifier) ON UPDATE RESTRICT ON DELETE RESTRICT +); + +INSERT INTO oauth2_refresh_token_session (id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data) +SELECT id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data +FROM _bkp_UP_V0005_oauth2_refresh_token_session +ORDER BY id; + +DROP INDEX oauth2_refresh_token_session_request_id_idx; +DROP INDEX oauth2_refresh_token_session_client_id_idx; +DROP INDEX oauth2_refresh_token_session_client_id_subject_idx; + +CREATE INDEX oauth2_refresh_token_session_request_id_idx ON oauth2_refresh_token_session (request_id); +CREATE INDEX oauth2_refresh_token_session_client_id_idx ON oauth2_refresh_token_session (client_id); +CREATE INDEX oauth2_refresh_token_session_client_id_subject_idx ON oauth2_refresh_token_session (client_id, subject); + +DROP TABLE _bkp_UP_V0005_oauth2_refresh_token_session; + +ALTER TABLE oauth2_pkce_request_session RENAME TO _bkp_UP_V0005_oauth2_pkce_request_session; + +CREATE TABLE IF NOT EXISTS oauth2_pkce_request_session ( + id INTEGER, + challenge_id CHAR(36) NOT NULL, + request_id VARCHAR(40) NOT NULL, + client_id VARCHAR(255) NOT NULL, + signature VARCHAR(255) NOT NULL, + subject CHAR(36) NOT NULL, + requested_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + requested_scopes TEXT NOT NULL, + granted_scopes TEXT NOT NULL, + requested_audience TEXT NULL DEFAULT '', + granted_audience TEXT NULL DEFAULT '', + active BOOLEAN NOT NULL DEFAULT FALSE, + revoked BOOLEAN NOT NULL DEFAULT FALSE, + form_data TEXT NOT NULL, + session_data BLOB NOT NULL, + PRIMARY KEY (id), + CONSTRAINT oauth2_pkce_request_session_challenge_id_fkey + FOREIGN KEY(challenge_id) + REFERENCES oauth2_consent_session(challenge_id) ON UPDATE CASCADE ON DELETE CASCADE, + CONSTRAINT oauth2_pkce_request_session_subject_fkey + FOREIGN KEY(subject) + REFERENCES user_opaque_identifier(identifier) ON UPDATE RESTRICT ON DELETE RESTRICT +); + +INSERT INTO oauth2_pkce_request_session (id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data) +SELECT id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data +FROM _bkp_UP_V0005_oauth2_pkce_request_session +ORDER BY id; + +DROP INDEX oauth2_pkce_request_session_request_id_idx; +DROP INDEX oauth2_pkce_request_session_client_id_idx; +DROP INDEX oauth2_pkce_request_session_client_id_subject_idx; + +CREATE INDEX oauth2_pkce_request_session_request_id_idx ON oauth2_pkce_request_session (request_id); +CREATE INDEX oauth2_pkce_request_session_client_id_idx ON oauth2_pkce_request_session (client_id); +CREATE INDEX oauth2_pkce_request_session_client_id_subject_idx ON oauth2_pkce_request_session (client_id, subject); + +DROP TABLE _bkp_UP_V0005_oauth2_pkce_request_session; + +ALTER TABLE oauth2_openid_connect_session RENAME TO _bkp_UP_V0005_oauth2_openid_connect_session; + +CREATE TABLE IF NOT EXISTS oauth2_openid_connect_session ( + id INTEGER, + challenge_id CHAR(36) NOT NULL, + request_id VARCHAR(40) NOT NULL, + client_id VARCHAR(255) NOT NULL, + signature VARCHAR(255) NOT NULL, + subject CHAR(36) NOT NULL, + requested_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + requested_scopes TEXT NOT NULL, + granted_scopes TEXT NOT NULL, + requested_audience TEXT NULL DEFAULT '', + granted_audience TEXT NULL DEFAULT '', + active BOOLEAN NOT NULL DEFAULT FALSE, + revoked BOOLEAN NOT NULL DEFAULT FALSE, + form_data TEXT NOT NULL, + session_data BLOB NOT NULL, + PRIMARY KEY (id), + CONSTRAINT oauth2_openid_connect_session_challenge_id_fkey + FOREIGN KEY(challenge_id) + REFERENCES oauth2_consent_session(challenge_id) ON UPDATE CASCADE ON DELETE CASCADE, + CONSTRAINT oauth2_openid_connect_session_subject_fkey + FOREIGN KEY(subject) + REFERENCES user_opaque_identifier(identifier) ON UPDATE RESTRICT ON DELETE RESTRICT +); + +INSERT INTO oauth2_openid_connect_session (id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data) +SELECT id, challenge_id, request_id, client_id, signature, subject, requested_at, requested_scopes, granted_scopes, requested_audience, granted_audience, active, revoked, form_data, session_data +FROM _bkp_UP_V0005_oauth2_openid_connect_session +ORDER BY id; + +DROP INDEX oauth2_openid_connect_session_request_id_idx; +DROP INDEX oauth2_openid_connect_session_client_id_idx; +DROP INDEX oauth2_openid_connect_session_client_id_subject_idx; + +CREATE INDEX oauth2_openid_connect_session_request_id_idx ON oauth2_openid_connect_session (request_id); +CREATE INDEX oauth2_openid_connect_session_client_id_idx ON oauth2_openid_connect_session (client_id); +CREATE INDEX oauth2_openid_connect_session_client_id_subject_idx ON oauth2_openid_connect_session (client_id, subject); + +DROP TABLE _bkp_UP_V0005_oauth2_openid_connect_session; + +COMMIT; + +PRAGMA foreign_keys=on; diff --git a/internal/storage/provider.go b/internal/storage/provider.go index 1d2a8698e..7696966a5 100644 --- a/internal/storage/provider.go +++ b/internal/storage/provider.go @@ -47,6 +47,7 @@ type Provider interface { LoadPreferredDuoDevice(ctx context.Context, username string) (device *model.DuoDevice, err error) SaveOAuth2ConsentSession(ctx context.Context, consent model.OAuth2ConsentSession) (err error) + SaveOAuth2ConsentSessionSubject(ctx context.Context, consent model.OAuth2ConsentSession) (err error) SaveOAuth2ConsentSessionResponse(ctx context.Context, consent model.OAuth2ConsentSession, rejection bool) (err error) SaveOAuth2ConsentSessionGranted(ctx context.Context, id int) (err error) LoadOAuth2ConsentSessionByChallengeID(ctx context.Context, challengeID uuid.UUID) (consent *model.OAuth2ConsentSession, err error) diff --git a/internal/storage/sql_provider.go b/internal/storage/sql_provider.go index 6872022d8..7e19e674c 100644 --- a/internal/storage/sql_provider.go +++ b/internal/storage/sql_provider.go @@ -105,6 +105,7 @@ func NewSQLProvider(config *schema.Configuration, name, driverName, dataSourceNa sqlDeactivateOAuth2OpenIDConnectSessionByRequestID: fmt.Sprintf(queryFmtDeactivateOAuth2SessionByRequestID, tableOAuth2OpenIDConnectSession), sqlInsertOAuth2ConsentSession: fmt.Sprintf(queryFmtInsertOAuth2ConsentSession, tableOAuth2ConsentSession), + sqlUpdateOAuth2ConsentSessionSubject: fmt.Sprintf(queryFmtUpdateOAuth2ConsentSessionSubject, tableOAuth2ConsentSession), sqlUpdateOAuth2ConsentSessionResponse: fmt.Sprintf(queryFmtUpdateOAuth2ConsentSessionResponse, tableOAuth2ConsentSession), sqlUpdateOAuth2ConsentSessionGranted: fmt.Sprintf(queryFmtUpdateOAuth2ConsentSessionGranted, tableOAuth2ConsentSession), sqlSelectOAuth2ConsentSessionByChallengeID: fmt.Sprintf(queryFmtSelectOAuth2ConsentSessionByChallengeID, tableOAuth2ConsentSession), @@ -235,6 +236,7 @@ type SQLProvider struct { // Table: oauth2_consent_session. sqlInsertOAuth2ConsentSession string + sqlUpdateOAuth2ConsentSessionSubject string sqlUpdateOAuth2ConsentSessionResponse string sqlUpdateOAuth2ConsentSessionGranted string sqlSelectOAuth2ConsentSessionByChallengeID string @@ -390,7 +392,7 @@ func (p *SQLProvider) LoadUserOpaqueIdentifierBySignature(ctx context.Context, s return opaqueID, nil } -// SaveOAuth2ConsentSession inserts an OAuth2.0 consent. +// SaveOAuth2ConsentSession inserts an OAuth2.0 consent session. func (p *SQLProvider) SaveOAuth2ConsentSession(ctx context.Context, consent model.OAuth2ConsentSession) (err error) { if _, err = p.db.ExecContext(ctx, p.sqlInsertOAuth2ConsentSession, consent.ChallengeID, consent.ClientID, consent.Subject, consent.Authorized, consent.Granted, @@ -402,10 +404,18 @@ func (p *SQLProvider) SaveOAuth2ConsentSession(ctx context.Context, consent mode return nil } -// SaveOAuth2ConsentSessionResponse updates an OAuth2.0 consent with the consent response. +// SaveOAuth2ConsentSessionSubject updates an OAuth2.0 consent session with the subject. +func (p *SQLProvider) SaveOAuth2ConsentSessionSubject(ctx context.Context, consent model.OAuth2ConsentSession) (err error) { + if _, err = p.db.ExecContext(ctx, p.sqlUpdateOAuth2ConsentSessionSubject, consent.Subject, consent.ID); err != nil { + return fmt.Errorf("error updating oauth2 consent session subject with id '%d' and challenge id '%s' for subject '%s': %w", consent.ID, consent.ChallengeID, consent.Subject, err) + } + + return nil +} + +// SaveOAuth2ConsentSessionResponse updates an OAuth2.0 consent session with the response. func (p *SQLProvider) SaveOAuth2ConsentSessionResponse(ctx context.Context, consent model.OAuth2ConsentSession, authorized bool) (err error) { - _, err = p.db.ExecContext(ctx, p.sqlUpdateOAuth2ConsentSessionResponse, authorized, consent.ExpiresAt, consent.GrantedScopes, consent.GrantedAudience, consent.ID) - if err != nil { + if _, err = p.db.ExecContext(ctx, p.sqlUpdateOAuth2ConsentSessionResponse, authorized, consent.ExpiresAt, consent.GrantedScopes, consent.GrantedAudience, consent.ID); err != nil { return fmt.Errorf("error updating oauth2 consent session (authorized '%t') with id '%d' and challenge id '%s' for subject '%s': %w", authorized, consent.ID, consent.ChallengeID, consent.Subject, err) } diff --git a/internal/storage/sql_provider_backend_postgres.go b/internal/storage/sql_provider_backend_postgres.go index e1fdf6302..3b98e64e4 100644 --- a/internal/storage/sql_provider_backend_postgres.go +++ b/internal/storage/sql_provider_backend_postgres.go @@ -75,6 +75,7 @@ func NewPostgreSQLProvider(config *schema.Configuration) (provider *PostgreSQLPr provider.sqlSelectEncryptionValue = provider.db.Rebind(provider.sqlSelectEncryptionValue) provider.sqlInsertOAuth2ConsentSession = provider.db.Rebind(provider.sqlInsertOAuth2ConsentSession) + provider.sqlUpdateOAuth2ConsentSessionSubject = provider.db.Rebind(provider.sqlUpdateOAuth2ConsentSessionSubject) provider.sqlUpdateOAuth2ConsentSessionResponse = provider.db.Rebind(provider.sqlUpdateOAuth2ConsentSessionResponse) provider.sqlUpdateOAuth2ConsentSessionGranted = provider.db.Rebind(provider.sqlUpdateOAuth2ConsentSessionGranted) provider.sqlSelectOAuth2ConsentSessionByChallengeID = provider.db.Rebind(provider.sqlSelectOAuth2ConsentSessionByChallengeID) diff --git a/internal/storage/sql_provider_queries.go b/internal/storage/sql_provider_queries.go index 3fbbfeb0a..ed72c5b68 100644 --- a/internal/storage/sql_provider_queries.go +++ b/internal/storage/sql_provider_queries.go @@ -240,6 +240,11 @@ const ( form_data, requested_scopes, granted_scopes, requested_audience, granted_audience) VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);` + queryFmtUpdateOAuth2ConsentSessionSubject = ` + UPDATE %s + SET subject = ? + WHERE id = ?;` + queryFmtUpdateOAuth2ConsentSessionResponse = ` UPDATE %s SET authorized = ?, responded_at = CURRENT_TIMESTAMP, expires_at = ?, granted_scopes = ?, granted_audience = ?