From 15ef449effda16aa47b79ea7467c4df99235221e Mon Sep 17 00:00:00 2001 From: Dennis Pattmann Date: Mon, 13 Jan 2020 14:51:59 +0100 Subject: [PATCH] consent: add field for handled at to consent request type and database References: https://github.com/ory/hydra/issues/1684 Co-authored-by: Marco Hutzsch --- Makefile | 2 +- consent/handler.go | 2 ++ consent/handler_test.go | 2 +- consent/manager_test_helpers.go | 4 +++ consent/migrations/sql/cockroach/14.sql | 7 ++++ consent/migrations/sql/shared/14.sql | 7 ++++ consent/migrations/sql/tests/14_test.sql | 46 ++++++++++++++++++++++++ consent/sql_helper.go | 6 ++++ consent/types.go | 9 +++++ cypress/integration/oauth2/consent.js | 25 ++++++++++++- 10 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 consent/migrations/sql/cockroach/14.sql create mode 100644 consent/migrations/sql/shared/14.sql create mode 100644 consent/migrations/sql/tests/14_test.sql diff --git a/Makefile b/Makefile index 00ba8480dda..9c5b787c918 100644 --- a/Makefile +++ b/Makefile @@ -113,4 +113,4 @@ install: .PHONY: init init: GO111MODULE=on go get . - GO111MODULE=on go install github.com/ory/go-acc \ No newline at end of file + GO111MODULE=on go install github.com/ory/go-acc diff --git a/consent/handler.go b/consent/handler.go index c09cbedf644..4b6c0024601 100644 --- a/consent/handler.go +++ b/consent/handler.go @@ -553,6 +553,7 @@ func (h *Handler) AcceptConsentRequest(w http.ResponseWriter, r *http.Request, p p.Challenge = challenge p.RequestedAt = cr.RequestedAt + p.HandledAt = time.Now().UTC() hr, err := h.r.ConsentManager().HandleConsentRequest(r.Context(), challenge, &p) if err != nil { @@ -633,6 +634,7 @@ func (h *Handler) RejectConsentRequest(w http.ResponseWriter, r *http.Request, p Error: &p, Challenge: challenge, RequestedAt: hr.RequestedAt, + HandledAt: time.Now().UTC(), }) if err != nil { h.r.Writer().WriteError(w, r, errors.WithStack(err)) diff --git a/consent/handler_test.go b/consent/handler_test.go index d841325de2a..18d8f404eae 100644 --- a/consent/handler_test.go +++ b/consent/handler_test.go @@ -204,4 +204,4 @@ func TestGetConsentRequest(t *testing.T) { require.EqualValues(t, tc.status, resp.StatusCode) }) } -} +} \ No newline at end of file diff --git a/consent/manager_test_helpers.go b/consent/manager_test_helpers.go index b0bd3482822..c1c30a9a736 100644 --- a/consent/manager_test_helpers.go +++ b/consent/manager_test_helpers.go @@ -86,6 +86,7 @@ func MockConsentRequest(key string, remember bool, rememberFor int, hasError boo GrantedScope: []string{"scopea" + key, "scopeb" + key}, GrantedAudience: []string{"auda" + key, "audb" + key}, Error: err, + HandledAt: time.Now().UTC(), //WasUsed: true, } @@ -330,6 +331,7 @@ func ManagerTests(m Manager, clientManager client.Manager, fositeManager x.Fosit got1, err = m.HandleConsentRequest(context.TODO(), "challenge"+tc.key, h) require.NoError(t, err) + require.Equal(t, time.Now().UTC().Round(time.Minute), h.HandledAt.Round(time.Minute)) compareConsentRequest(t, c, got1) _, err = m.HandleConsentRequest(context.TODO(), "challenge"+tc.key, h) @@ -502,6 +504,7 @@ func ManagerTests(m Manager, clientManager client.Manager, fositeManager x.Fosit subject string challenges []string clients []string + handledAt time.Time }{ { subject: "subjectrv1", @@ -530,6 +533,7 @@ func ManagerTests(m Manager, clientManager client.Manager, fositeManager x.Fosit for _, consent := range consents { assert.Contains(t, tc.challenges, consent.Challenge) assert.Contains(t, tc.clients, consent.ConsentRequest.Client.ClientID) + assert.NotNil(t, consent.HandledAt) } } diff --git a/consent/migrations/sql/cockroach/14.sql b/consent/migrations/sql/cockroach/14.sql new file mode 100644 index 00000000000..59d4049b24e --- /dev/null +++ b/consent/migrations/sql/cockroach/14.sql @@ -0,0 +1,7 @@ +-- +migrate Up +ALTER TABLE hydra_oauth2_consent_request ADD handled_at timestamp NULL; +ALTER TABLE hydra_oauth2_consent_request_handled ADD handled_at timestamp NULL; + +-- +migrate Down +ALTER TABLE hydra_oauth2_consent_request DROP COLUMN handled_at; +ALTER TABLE hydra_oauth2_consent_request_handled DROP COLUMN handled_at; diff --git a/consent/migrations/sql/shared/14.sql b/consent/migrations/sql/shared/14.sql new file mode 100644 index 00000000000..59d4049b24e --- /dev/null +++ b/consent/migrations/sql/shared/14.sql @@ -0,0 +1,7 @@ +-- +migrate Up +ALTER TABLE hydra_oauth2_consent_request ADD handled_at timestamp NULL; +ALTER TABLE hydra_oauth2_consent_request_handled ADD handled_at timestamp NULL; + +-- +migrate Down +ALTER TABLE hydra_oauth2_consent_request DROP COLUMN handled_at; +ALTER TABLE hydra_oauth2_consent_request_handled DROP COLUMN handled_at; diff --git a/consent/migrations/sql/tests/14_test.sql b/consent/migrations/sql/tests/14_test.sql new file mode 100644 index 00000000000..6ba65e2157e --- /dev/null +++ b/consent/migrations/sql/tests/14_test.sql @@ -0,0 +1,46 @@ +-- +migrate Up +INSERT INTO hydra_client (id, allowed_cors_origins, client_name, client_secret, redirect_uris, grant_types, response_types, scope, owner, policy_uri, tos_uri, client_uri, logo_uri, contacts, client_secret_expires_at, sector_identifier_uri, jwks, jwks_uri, token_endpoint_auth_method, request_uris, request_object_signing_alg, userinfo_signed_response_alg, subject_type, audience, frontchannel_logout_uri, frontchannel_logout_session_required, post_logout_redirect_uris, backchannel_logout_uri, backchannel_logout_session_required, metadata) +VALUES + ('14-client', 'http://localhost|http://google', 'some-client', 'abcdef', 'http://localhost|http://google', 'authorize_code|implicit', 'token|id_token', 'foo|bar', 'aeneas', 'http://policy', 'http://tos', 'http://client', 'http://logo', 'aeneas|foo', 0, 'http://sector', '{"keys": []}', 'http://jwks', 'none', 'http://uri1|http://uri2', 'rs256', 'rs526', 'public', 'https://www.ory.sh/api', 'http://fc-logout/', true, 'http://redir1/|http://redir2/', 'http://bc-logout/', true, '{"foo":"bar"}'); + +INSERT INTO + hydra_oauth2_authentication_session (id, authenticated_at, subject, remember) +VALUES + ('14-login-session-id', NOW(), '14-sub', true); + +INSERT INTO + hydra_oauth2_authentication_request (challenge, verifier, client_id, subject, request_url, skip, requested_scope, csrf, authenticated_at, requested_at, oidc_context, login_session_id, requested_at_audience) +VALUES + ('14-challenge', '14-verifier', '14-client', '14-subject', '14-redirect', false, '14-scope', '14-csrf', NOW(), NOW(), '{}', '14-login-session-id', '14-aud'); + +INSERT INTO + hydra_oauth2_consent_request (challenge, verifier, client_id, subject, request_url, skip, requested_scope, csrf, authenticated_at, requested_at, oidc_context, forced_subject_identifier, login_session_id, login_challenge, requested_at_audience, acr, context, handled_at) +VALUES + ('14-challenge', '14-verifier', '14-client', '14-subject', '14-redirect', false, '14-scope', '14-csrf', NOW(), NOW(), '{}', '14-forced-sub', '14-login-session-id', '14-challenge', '14-aud', '14-acr', '{"foo":"bar"}', NOW()); + +INSERT INTO + hydra_oauth2_consent_request_handled (challenge, granted_scope, remember, remember_for, error, requested_at, session_access_token, session_id_token, authenticated_at, was_used, granted_at_audience, handled_at) +VALUES + ('14-challenge', '14-scope', true, 3600, '{}', NOW(), '{}', '{}', NOW(), false, '14-aud', NOW()); + +INSERT INTO + hydra_oauth2_authentication_request_handled (challenge, subject, remember, remember_for, error, acr, requested_at, authenticated_at, was_used, forced_subject_identifier, context) +VALUES + ('14-challenge', '14-sub', true, 3600, '{}', '1', NOW(), NOW(), false, '14-forced-sub', '{"foo":"bar"}'); + +INSERT INTO + hydra_oauth2_obfuscated_authentication_session (subject, client_id, subject_obfuscated) +VALUES + ('14-sub', '14-client', '14-obfuscated'); + +INSERT INTO + hydra_oauth2_logout_request (challenge, verifier, subject, sid, client_id, request_url, redir_url, was_used, accepted, rejected, rp_initiated) +VALUES + ('14-challenge', '14-verifier', '14-subject', '14-session-id', '14-client', 'https://request-url/', 'https://redirect-url', false, false, false, false); + +INSERT INTO + hydra_oauth2_logout_request (challenge, verifier, subject, sid, client_id, request_url, redir_url, was_used, accepted, rejected, rp_initiated) +VALUES + ('14-1-challenge', '14-1-verifier', '14-1-subject', '14-1-session-id', NULL, 'https://request-url/', 'https://redirect-url', false, false, false, false); + +-- +migrate Down diff --git a/consent/sql_helper.go b/consent/sql_helper.go index e3a84e885c3..3656148bb18 100644 --- a/consent/sql_helper.go +++ b/consent/sql_helper.go @@ -98,6 +98,7 @@ var sqlParamsConsentRequestHandled = []string{ "session_access_token", "session_id_token", "was_used", + "handled_at", } var sqlParamsConsentRequestHandledUpdate = func() []string { p := make([]string, len(sqlParamsConsentRequestHandled)) @@ -196,6 +197,7 @@ type sqlAuthenticationRequest struct { LoginSessionID sql.NullString `db:"login_session_id"` Context string `db:"context"` WasHandled bool `db:"was_handled"` + HandledAt *time.Time `db:"handled_at"` } type sqlConsentRequest struct { @@ -354,6 +356,7 @@ func (s *sqlConsentRequest) toConsentRequest(client *client.Client) (*ConsentReq LoginChallenge: s.LoginChallenge.String, Context: context, ACR: s.ACR, + HandledAt: fromMySQLDateHack(s.HandledAt), }, nil } @@ -369,6 +372,7 @@ type sqlHandledConsentRequest struct { RequestedAt time.Time `db:"requested_at"` WasUsed bool `db:"was_used"` AuthenticatedAt *time.Time `db:"authenticated_at"` + HandledAt *time.Time `db:"handled_at"` } func newSQLHandledConsentRequest(c *HandledConsentRequest) (*sqlHandledConsentRequest, error) { @@ -414,6 +418,7 @@ func newSQLHandledConsentRequest(c *HandledConsentRequest) (*sqlHandledConsentRe RequestedAt: c.RequestedAt, WasUsed: c.WasUsed, AuthenticatedAt: toMySQLDateHack(c.AuthenticatedAt), + HandledAt: toMySQLDateHack(c.HandledAt), }, nil } @@ -451,6 +456,7 @@ func (s *sqlHandledConsentRequest) toHandledConsentRequest(r *ConsentRequest) (* Error: e, ConsentRequest: r, AuthenticatedAt: fromMySQLDateHack(s.AuthenticatedAt), + HandledAt: fromMySQLDateHack(s.HandledAt), }, nil } diff --git a/consent/types.go b/consent/types.go index 1b9c4460037..7e735d1a03b 100644 --- a/consent/types.go +++ b/consent/types.go @@ -97,6 +97,9 @@ type HandledConsentRequest struct { // authorization will be remembered indefinitely. RememberFor int `json:"remember_for"` + // HandledAt contains the timestamp the consent request was handled. + HandledAt time.Time `json:"handled_at,omitempty"` + ConsentRequest *ConsentRequest `json:"-"` Error *RequestDeniedError `json:"-"` Challenge string `json:"-"` @@ -125,6 +128,9 @@ type PreviousConsentSession struct { // authorization will be remembered indefinitely. RememberFor int `json:"remember_for"` + // HandledAt contains the timestamp the consent request was handled. + HandledAt time.Time `json:"handled_at,omitempty"` + ConsentRequest *ConsentRequest `json:"consent_request"` Error *RequestDeniedError `json:"-"` Challenge string `json:"-"` @@ -367,6 +373,9 @@ type ConsentRequest struct { // Context contains arbitrary information set by the login endpoint or is empty if not set. Context map[string]interface{} `json:"context,omitempty"` + // HandledAt contains the timestamp the consent request was handled. + HandledAt time.Time `json:"handled_at,omitempty"` + // ForceSubjectIdentifier is the value from authentication (if set). ForceSubjectIdentifier string `json:"-"` SubjectIdentifier string `json:"-"` diff --git a/cypress/integration/oauth2/consent.js b/cypress/integration/oauth2/consent.js index 59d5b544a59..e74c6b48a72 100644 --- a/cypress/integration/oauth2/consent.js +++ b/cypress/integration/oauth2/consent.js @@ -30,12 +30,35 @@ describe('OAuth 2.0 End-User Authorization', () => { cy.request( Cypress.env('admin_url') + - '/oauth2/auth/sessions/consent?subject=foo@bar.com' + '/oauth2/auth/sessions/consent?subject=foo@bar.com' ) .its('body') .then(body => { expect(body.length).to.be.greaterThan(0); expect(hasConsent(client, body)).to.be.true; + body.forEach((consent) => { + cy.request( + 'PUT', + Cypress.env('admin_url') + + '/oauth2/auth/requests/consent/accept?consent_challenge=' + consent.consent_request.challenge, + { + "grant_scope": ["offline_access"], + "remember": true, + "remember_for": 0 + } + ); + }) + }); + + cy.request( + Cypress.env('admin_url') + + '/oauth2/auth/sessions/consent?subject=foo@bar.com' + ) + .its('body') + .then(body => { + body.forEach ((consent) => { + expect(consent.handled_at.match(/^[2-9]\d{3}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+Z$/)).not.to.be.empty + }) }); cy.request(