diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c1a4b8bed1f..f63598a24e1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -37,6 +37,9 @@ jobs: outputs: sdk-cache-key: ${{ steps.sdk-generate.outputs.sdk-cache-key }} steps: + - uses: actions/setup-go@v4 + with: + go-version: "1.23" - uses: ory/ci/sdk/generate@master with: token: ${{ secrets.ORY_BOT_PAT }} diff --git a/consent/handler.go b/consent/handler.go index f2605b405e9..d25b359c9ba 100644 --- a/consent/handler.go +++ b/consent/handler.go @@ -4,7 +4,6 @@ package consent import ( - "context" "encoding/json" "net/http" "net/url" @@ -90,6 +89,13 @@ type revokeOAuth2ConsentSessions struct { // in: query Client string `json:"client"` + // Consent Challenge ID + // + // If set, revoke all token chains derived from this particular consent request ID. + // + // in: query + ConsentChallengeID string `json:"consent_challenge_id"` + // Revoke All Consent Sessions // // If set to `true` deletes all consent sessions by the Subject that have been granted. @@ -119,14 +125,23 @@ type revokeOAuth2ConsentSessions struct { func (h *Handler) revokeOAuth2ConsentSessions(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { subject := r.URL.Query().Get("subject") client := r.URL.Query().Get("client") + consentChallengeID := r.URL.Query().Get("consent_challenge_id") allClients := r.URL.Query().Get("all") == "true" - if subject == "" { - h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint(`Query parameter 'subject' is not defined but should have been.`))) + if subject == "" && consentChallengeID == "" { + h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint(`Query parameter 'subject' or 'consent_challenge_id' are required.`))) + return + } + if consentChallengeID != "" && subject != "" { + h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint(`Query parameter 'subject' and 'consent_challenge_id' cannot be set at the same time.`))) + return + } + if consentChallengeID != "" && client != "" { + h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint(`Query parameter 'client' and 'consent_challenge_id' cannot be set at the same time.`))) return } switch { - case len(client) > 0: + case client != "": if err := h.r.ConsentManager().RevokeSubjectClientConsentSession(r.Context(), subject, client); err != nil && !errors.Is(err, x.ErrNotFound) { h.r.Writer().WriteError(w, r, err) return @@ -138,6 +153,12 @@ func (h *Handler) revokeOAuth2ConsentSessions(w http.ResponseWriter, r *http.Req return } events.Trace(r.Context(), events.ConsentRevoked, events.WithSubject(subject)) + case consentChallengeID != "": + if err := h.r.ConsentManager().RevokeConsentSessionByID(r.Context(), consentChallengeID); err != nil && !errors.Is(err, x.ErrNotFound) { + h.r.Writer().WriteError(w, r, err) + return + } + return default: h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint(`Query parameter both 'client' and 'all' is not defined but one of them should have been.`))) return @@ -479,7 +500,7 @@ func (h *Handler) acceptOAuth2LoginRequest(w http.ResponseWriter, r *http.Reques } handledLoginRequest.RequestedAt = loginRequest.RequestedAt - f, err := h.decodeFlowWithClient(ctx, challenge, flowctx.AsLoginChallenge) + f, err := flowctx.Decode[flow.Flow](ctx, h.r.FlowCipher(), challenge, flowctx.AsLoginChallenge) if err != nil { h.r.Writer().WriteError(w, r, err) return @@ -579,11 +600,12 @@ func (h *Handler) rejectOAuth2LoginRequest(w http.ResponseWriter, r *http.Reques return } - f, err := h.decodeFlowWithClient(ctx, challenge, flowctx.AsLoginChallenge) + f, err := flowctx.Decode[flow.Flow](ctx, h.r.FlowCipher(), challenge, flowctx.AsLoginChallenge) if err != nil { h.r.Writer().WriteError(w, r, err) return } + request, err := h.r.ConsentManager().HandleLoginRequest(ctx, f, challenge, &flow.HandledLoginRequest{ Error: &p, ID: challenge, @@ -765,11 +787,12 @@ func (h *Handler) acceptOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ p.RequestedAt = cr.RequestedAt p.HandledAt = sqlxx.NullTime(time.Now().UTC()) - f, err := h.decodeFlowWithClient(ctx, challenge, flowctx.AsConsentChallenge) + f, err := flowctx.Decode[flow.Flow](ctx, h.r.FlowCipher(), challenge, flowctx.AsConsentChallenge) if err != nil { h.r.Writer().WriteError(w, r, err) return } + hr, err := h.r.ConsentManager().HandleConsentRequest(ctx, f, &p) if err != nil { h.r.Writer().WriteError(w, r, errorsx.WithStack(err)) @@ -872,7 +895,7 @@ func (h *Handler) rejectOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ return } - f, err := h.decodeFlowWithClient(ctx, challenge, flowctx.AsConsentChallenge) + f, err := flowctx.Decode[flow.Flow](ctx, h.r.FlowCipher(), challenge, flowctx.AsConsentChallenge) if err != nil { h.r.Writer().WriteError(w, r, err) return @@ -1048,12 +1071,3 @@ func (h *Handler) getOAuth2LogoutRequest(w http.ResponseWriter, r *http.Request, h.r.Writer().Write(w, r, request) } - -func (h *Handler) decodeFlowWithClient(ctx context.Context, challenge string, opts ...flowctx.CodecOption) (*flow.Flow, error) { - f, err := flowctx.Decode[flow.Flow](ctx, h.r.FlowCipher(), challenge, opts...) - if err != nil { - return nil, err - } - - return f, nil -} diff --git a/consent/handler_test.go b/consent/handler_test.go index 45ba2b7733a..4ee7ddfddff 100644 --- a/consent/handler_test.go +++ b/consent/handler_test.go @@ -225,7 +225,6 @@ func TestGetConsentRequest(t *testing.T) { } else if tc.exists { var result flow.OAuth2ConsentRequest require.NoError(t, json.NewDecoder(resp.Body).Decode(&result)) - require.Equal(t, challenge, result.ID) require.Equal(t, requestURL, result.RequestURL) require.NotNil(t, result.Client) } diff --git a/consent/manager.go b/consent/manager.go index fe4b018352e..3bd9152e07a 100644 --- a/consent/manager.go +++ b/consent/manager.go @@ -30,6 +30,7 @@ type ( HandleConsentRequest(ctx context.Context, f *flow.Flow, r *flow.AcceptOAuth2ConsentRequest) (*flow.OAuth2ConsentRequest, error) RevokeSubjectConsentSession(ctx context.Context, user string) error RevokeSubjectClientConsentSession(ctx context.Context, user, client string) error + RevokeConsentSessionByID(ctx context.Context, consentChallengeID string) error VerifyAndInvalidateConsentRequest(ctx context.Context, verifier string) (*flow.AcceptOAuth2ConsentRequest, error) FindGrantedAndRememberedConsentRequests(ctx context.Context, client, user string) ([]flow.AcceptOAuth2ConsentRequest, error) diff --git a/consent/sdk_test.go b/consent/sdk_test.go deleted file mode 100644 index 0f30d16e7c8..00000000000 --- a/consent/sdk_test.go +++ /dev/null @@ -1,263 +0,0 @@ -// Copyright © 2022 Ory Corp -// SPDX-License-Identifier: Apache-2.0 - -package consent_test - -import ( - "context" - "fmt" - "net/http" - "net/http/httptest" - "testing" - "time" - - "github.com/ory/hydra/v2/internal/testhelpers" - - "github.com/ory/hydra/v2/consent/test" - - hydra "github.com/ory/hydra-client-go/v2" - . "github.com/ory/hydra/v2/flow" - - "github.com/ory/x/httprouterx" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - . "github.com/ory/hydra/v2/consent" - "github.com/ory/hydra/v2/driver/config" - "github.com/ory/hydra/v2/x" - "github.com/ory/x/contextx" -) - -func makeID(base string, network string, key string) string { - return fmt.Sprintf("%s-%s-%s", base, network, key) -} - -func TestSDK(t *testing.T) { - ctx := context.Background() - network := "t1" - conf := testhelpers.NewConfigurationWithDefaults() - conf.MustSet(ctx, config.KeyIssuerURL, "https://www.ory.sh") - conf.MustSet(ctx, config.KeyAccessTokenLifespan, time.Minute) - reg := testhelpers.NewRegistryMemory(t, conf, &contextx.Default{}) - - consentChallenge := func(f *Flow) string { return x.Must(f.ToConsentChallenge(ctx, reg)) } - consentVerifier := func(f *Flow) string { return x.Must(f.ToConsentVerifier(ctx, reg)) } - loginChallenge := func(f *Flow) string { return x.Must(f.ToLoginChallenge(ctx, reg)) } - - router := x.NewRouterPublic() - h := NewHandler(reg, conf) - - h.SetRoutes(httprouterx.NewRouterAdminWithPrefixAndRouter(router.Router, "/admin", conf.AdminURL)) - ts := httptest.NewServer(router) - - sdk := hydra.NewAPIClient(hydra.NewConfiguration()) - sdk.GetConfig().Servers = hydra.ServerConfigurations{{URL: ts.URL}} - - m := reg.ConsentManager() - - require.NoError(t, m.CreateLoginSession(context.Background(), &LoginSession{ - ID: "session1", - Subject: "subject1", - })) - - ar1, _, _ := test.MockAuthRequest("1", false, network) - ar2, _, _ := test.MockAuthRequest("2", false, network) - require.NoError(t, m.CreateLoginSession(context.Background(), &LoginSession{ - ID: ar1.SessionID.String(), - Subject: ar1.Subject, - })) - require.NoError(t, m.CreateLoginSession(context.Background(), &LoginSession{ - ID: ar2.SessionID.String(), - Subject: ar2.Subject, - })) - _, err := m.CreateLoginRequest(context.Background(), ar1) - require.NoError(t, err) - _, err = m.CreateLoginRequest(context.Background(), ar2) - require.NoError(t, err) - - cr1, hcr1, _ := test.MockConsentRequest("1", false, 0, false, false, false, "fk-login-challenge", network) - cr2, hcr2, _ := test.MockConsentRequest("2", false, 0, false, false, false, "fk-login-challenge", network) - cr3, hcr3, _ := test.MockConsentRequest("3", true, 3600, false, false, false, "fk-login-challenge", network) - cr4, hcr4, _ := test.MockConsentRequest("4", true, 3600, false, false, false, "fk-login-challenge", network) - require.NoError(t, reg.ClientManager().CreateClient(context.Background(), cr1.Client)) - require.NoError(t, reg.ClientManager().CreateClient(context.Background(), cr2.Client)) - require.NoError(t, reg.ClientManager().CreateClient(context.Background(), cr3.Client)) - require.NoError(t, reg.ClientManager().CreateClient(context.Background(), cr4.Client)) - - cr1Flow, err := m.CreateLoginRequest(context.Background(), &LoginRequest{ - ID: cr1.LoginChallenge.String(), - Subject: cr1.Subject, - Client: cr1.Client, - Verifier: cr1.ID, - RequestedAt: time.Now(), - }) - require.NoError(t, err) - cr1Flow.LoginSkip = ar1.Skip - - cr2Flow, err := m.CreateLoginRequest(context.Background(), &LoginRequest{ - ID: cr2.LoginChallenge.String(), - Subject: cr2.Subject, - Client: cr2.Client, - Verifier: cr2.ID, - RequestedAt: time.Now(), - }) - require.NoError(t, err) - cr2Flow.LoginSkip = ar2.Skip - - loginSession3 := &LoginSession{ID: cr3.LoginSessionID.String()} - require.NoError(t, m.CreateLoginSession(context.Background(), loginSession3)) - require.NoError(t, m.ConfirmLoginSession(context.Background(), loginSession3)) - cr3Flow, err := m.CreateLoginRequest(context.Background(), &LoginRequest{ - ID: cr3.LoginChallenge.String(), - Subject: cr3.Subject, - Client: cr3.Client, - Verifier: cr3.ID, - RequestedAt: hcr3.RequestedAt, - SessionID: cr3.LoginSessionID, - }) - require.NoError(t, err) - - loginSession4 := &LoginSession{ID: cr4.LoginSessionID.String()} - require.NoError(t, m.CreateLoginSession(context.Background(), loginSession4)) - require.NoError(t, m.ConfirmLoginSession(context.Background(), loginSession4)) - cr4Flow, err := m.CreateLoginRequest(context.Background(), &LoginRequest{ - ID: cr4.LoginChallenge.String(), - Client: cr4.Client, - Verifier: cr4.ID, - SessionID: cr4.LoginSessionID, - }) - require.NoError(t, err) - - require.NoError(t, m.CreateConsentRequest(context.Background(), cr1Flow, cr1)) - require.NoError(t, m.CreateConsentRequest(context.Background(), cr2Flow, cr2)) - require.NoError(t, m.CreateConsentRequest(context.Background(), cr3Flow, cr3)) - require.NoError(t, m.CreateConsentRequest(context.Background(), cr4Flow, cr4)) - _, err = m.HandleConsentRequest(context.Background(), cr1Flow, hcr1) - require.NoError(t, err) - _, err = m.HandleConsentRequest(context.Background(), cr2Flow, hcr2) - require.NoError(t, err) - _, err = m.HandleConsentRequest(context.Background(), cr3Flow, hcr3) - require.NoError(t, err) - _, err = m.HandleConsentRequest(context.Background(), cr4Flow, hcr4) - require.NoError(t, err) - - _, err = m.VerifyAndInvalidateConsentRequest(context.Background(), consentVerifier(cr3Flow)) - require.NoError(t, err) - _, err = m.VerifyAndInvalidateConsentRequest(context.Background(), consentVerifier(cr4Flow)) - require.NoError(t, err) - - lur1 := test.MockLogoutRequest("testsdk-1", true, network) - require.NoError(t, reg.ClientManager().CreateClient(context.Background(), lur1.Client)) - require.NoError(t, m.CreateLogoutRequest(context.Background(), lur1)) - - lur2 := test.MockLogoutRequest("testsdk-2", false, network) - require.NoError(t, m.CreateLogoutRequest(context.Background(), lur2)) - - cr1.ID = consentChallenge(cr1Flow) - crGot := execute[hydra.OAuth2ConsentRequest](t, sdk.OAuth2API.GetOAuth2ConsentRequest(ctx).ConsentChallenge(cr1.ID)) - compareSDKConsentRequest(t, cr1, *crGot) - - cr2.ID = consentChallenge(cr2Flow) - crGot = execute[hydra.OAuth2ConsentRequest](t, sdk.OAuth2API.GetOAuth2ConsentRequest(ctx).ConsentChallenge(cr2.ID)) - compareSDKConsentRequest(t, cr2, *crGot) - - ar1.ID = loginChallenge(cr1Flow) - arGot := execute[hydra.OAuth2LoginRequest](t, sdk.OAuth2API.GetOAuth2LoginRequest(ctx).LoginChallenge(ar1.ID)) - compareSDKLoginRequest(t, ar1, *arGot) - - ar2.ID = loginChallenge(cr2Flow) - arGot = execute[hydra.OAuth2LoginRequest](t, sdk.OAuth2API.GetOAuth2LoginRequest(ctx).LoginChallenge(ar2.ID)) - require.NoError(t, err) - compareSDKLoginRequest(t, ar2, *arGot) - - _, err = sdk.OAuth2API.RevokeOAuth2LoginSessions(ctx).Subject("subject1").Execute() - require.NoError(t, err) - - _, err = sdk.OAuth2API.RevokeOAuth2ConsentSessions(ctx).Subject("subject1").Execute() - require.Error(t, err) - - _, err = sdk.OAuth2API.RevokeOAuth2ConsentSessions(ctx).Subject(cr4.Subject).Client(cr4.Client.GetID()).Execute() - require.NoError(t, err) - - _, err = sdk.OAuth2API.RevokeOAuth2ConsentSessions(ctx).Subject("subject1").All(true).Execute() - require.NoError(t, err) - - _, _, err = sdk.OAuth2API.GetOAuth2ConsentRequest(ctx).ConsentChallenge(makeID("challenge", network, "1")).Execute() - require.Error(t, err) - - cr2.ID = consentChallenge(cr2Flow) - crGot, _, err = sdk.OAuth2API.GetOAuth2ConsentRequest(ctx).ConsentChallenge(cr2.ID).Execute() - require.NoError(t, err) - compareSDKConsentRequest(t, cr2, *crGot) - - _, err = sdk.OAuth2API.RevokeOAuth2ConsentSessions(ctx).Subject("subject2").Client("fk-client-2").Execute() - require.NoError(t, err) - - _, _, err = sdk.OAuth2API.GetOAuth2ConsentRequest(ctx).ConsentChallenge(makeID("challenge", network, "2")).Execute() - require.Error(t, err) - - csGot, _, err := sdk.OAuth2API.ListOAuth2ConsentSessions(ctx).Subject("subject3").Execute() - require.NoError(t, err) - assert.Equal(t, 1, len(csGot)) - - csGot, _, err = sdk.OAuth2API.ListOAuth2ConsentSessions(ctx).Subject("subject2").Execute() - require.NoError(t, err) - assert.Equal(t, 0, len(csGot)) - - csGot, _, err = sdk.OAuth2API.ListOAuth2ConsentSessions(ctx).Subject("subject3").LoginSessionId("fk-login-session-t1-3").Execute() - require.NoError(t, err) - assert.Equal(t, 1, len(csGot)) - - csGot, _, err = sdk.OAuth2API.ListOAuth2ConsentSessions(ctx).Subject("subject3").LoginSessionId("fk-login-session-t1-X").Execute() - require.NoError(t, err) - assert.Equal(t, 0, len(csGot)) - - luGot, _, err := sdk.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(makeID("challenge", network, "testsdk-1")).Execute() - require.NoError(t, err) - compareSDKLogoutRequest(t, lur1, luGot) - - luaGot, _, err := sdk.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(makeID("challenge", network, "testsdk-1")).Execute() - require.NoError(t, err) - assert.EqualValues(t, "https://www.ory.sh/oauth2/sessions/logout?logout_verifier="+makeID("verifier", network, "testsdk-1"), luaGot.RedirectTo) - - _, err = sdk.OAuth2API.RejectOAuth2LogoutRequest(ctx).LogoutChallenge(lur2.ID).Execute() - require.NoError(t, err) - - _, _, err = sdk.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(lur2.ID).Execute() - require.Error(t, err) -} - -func compareSDKLoginRequest(t *testing.T, expected *LoginRequest, got hydra.OAuth2LoginRequest) { - assert.EqualValues(t, expected.ID, got.Challenge) - assert.EqualValues(t, expected.Subject, got.Subject) - assert.EqualValues(t, expected.Skip, got.Skip) - assert.EqualValues(t, expected.Client.GetID(), *got.Client.ClientId) -} - -func compareSDKConsentRequest(t *testing.T, expected *OAuth2ConsentRequest, got hydra.OAuth2ConsentRequest) { - assert.EqualValues(t, expected.ID, got.Challenge) - assert.EqualValues(t, expected.Subject, *got.Subject) - assert.EqualValues(t, expected.Skip, *got.Skip) - assert.EqualValues(t, expected.Client.GetID(), *got.Client.ClientId) -} - -func compareSDKLogoutRequest(t *testing.T, expected *LogoutRequest, got *hydra.OAuth2LogoutRequest) { - assert.EqualValues(t, expected.Subject, *got.Subject) - assert.EqualValues(t, expected.SessionID, *got.Sid) - assert.EqualValues(t, expected.SessionID, *got.Sid) - assert.EqualValues(t, expected.RequestURL, *got.RequestUrl) - assert.EqualValues(t, expected.RPInitiated, *got.RpInitiated) -} - -type executer[T any] interface { - Execute() (*T, *http.Response, error) -} - -func execute[T any](t *testing.T, e executer[T]) *T { - got, res, err := e.Execute() - require.NoError(t, err) - require.NoError(t, res.Body.Close()) - - return got -} diff --git a/consent/strategy_oauth_test.go b/consent/strategy_oauth_test.go index a2e39d5b6ec..e7d660f9fec 100644 --- a/consent/strategy_oauth_test.go +++ b/consent/strategy_oauth_test.go @@ -222,11 +222,11 @@ func TestStrategyLoginConsentNext(t *testing.T) { http.Redirect(w, r, v.RedirectTo, http.StatusFound) }, func(w http.ResponseWriter, r *http.Request) { - res, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(ctx). - ConsentChallenge(r.URL.Query().Get("consent_challenge")). + consentChallenge = r.URL.Query().Get("consent_challenge") + _, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(ctx). + ConsentChallenge(consentChallenge). Execute() require.NoError(t, err) - consentChallenge = res.Challenge v, _, err := adminClient.OAuth2API.AcceptOAuth2ConsentRequest(ctx). ConsentChallenge(consentChallenge). diff --git a/flow/consent_types.go b/flow/consent_types.go index 399938a8020..936dbf2b8e5 100644 --- a/flow/consent_types.go +++ b/flow/consent_types.go @@ -479,8 +479,7 @@ func (n *OAuth2ConsentRequestOpenIDConnectContext) Value() (driver.Value, error) // // swagger:model oAuth2LogoutRequest type LogoutRequest struct { - // Challenge is the identifier ("logout challenge") of the logout authentication request. It is used to - // identify the session. + // Challenge is the identifier of the logout authentication request. ID string `json:"challenge" db:"challenge"` NID uuid.UUID `json:"-" db:"nid"` @@ -546,8 +545,7 @@ type LogoutResult struct { // // swagger:model oAuth2LoginRequest type LoginRequest struct { - // ID is the identifier ("login challenge") of the login request. It is used to - // identify the session. + // ID is the identifier of the login request. // // required: true ID string `json:"challenge"` @@ -629,8 +627,7 @@ func (r *LoginRequest) MarshalJSON() ([]byte, error) { // // swagger:model oAuth2ConsentRequest type OAuth2ConsentRequest struct { - // ID is the identifier ("authorization challenge") of the consent authorization request. It is used to - // identify the session. + // ID is the identifier of the consent authorization request. // // required: true ID string `json:"challenge"` diff --git a/flow/flow.go b/flow/flow.go index 0502bc544ba..dc5afe65959 100644 --- a/flow/flow.go +++ b/flow/flow.go @@ -69,22 +69,16 @@ const ( FlowStateConsentError = int16(129) ) -// Flow is an abstraction used in the persistence layer to unify LoginRequest, -// HandledLoginRequest, ConsentRequest, and AcceptOAuth2ConsentRequest. -// -// TODO: Deprecate the structs that are made obsolete by the Flow concept. -// Context: Before Flow was introduced, the API and the database used the same -// structs, LoginRequest and HandledLoginRequest. These two tables and structs -// were merged into a new concept, Flow, in order to optimize the persistence -// layer. We currently limit the use of Flow to the persistence layer and keep -// using the original structs in the API in order to minimize the impact of the -// database refactoring on the API. +// Flow is an abstraction used in the persistence layer. +// It has outlived its usefulness but is difficult to remove. type Flow struct { - // ID is the identifier ("login challenge") of the login request. It is used to - // identify the session. + // ID is the identifier of the login request. // - // required: true - ID string `db:"login_challenge" json:"i"` + // The struct field is named ID for compatibility with gobuffalo/pop. + // + // The database column should be named `login_challenge_id`, but is not for + // historical reasons. + ID string `db:"login_challenge" json:"i"` // PK in database NID uuid.UUID `db:"nid" json:"n"` // RequestedScope contains the OAuth 2.0 Scope requested by the OAuth 2.0 Client. @@ -202,10 +196,7 @@ type Flow struct { LoginError *RequestDeniedError `db:"login_error" json:"le,omitempty"` LoginAuthenticatedAt sqlxx.NullTime `db:"login_authenticated_at" json:"la,omitempty"` - // ConsentChallengeID is the identifier ("authorization challenge") of the consent authorization request. It is used to - // identify the session. - // - // required: true + // ConsentChallengeID is the identifier of the consent request. ConsentChallengeID sqlxx.NullString `db:"consent_challenge_id" json:"cc,omitempty"` // ConsentSkip, if true, implies that the client has requested the same scopes from the same user previously. diff --git a/internal/httpclient/api/openapi.yaml b/internal/httpclient/api/openapi.yaml index ae5ad4dd6f3..4b8cf239fa9 100644 --- a/internal/httpclient/api/openapi.yaml +++ b/internal/httpclient/api/openapi.yaml @@ -1047,6 +1047,17 @@ paths: schema: type: string style: form + - description: |- + Consent Challenge ID + + If set, revoke all token chains derived from this particular consent request ID. + explode: true + in: query + name: consent_challenge_id + required: false + schema: + type: string + style: form - description: |- Revoke All Consent Sessions @@ -3127,9 +3138,7 @@ components: \ JSON for SQL storage." type: array challenge: - description: |- - ID is the identifier ("authorization challenge") of the consent authorization request. It is used to - identify the session. + description: ID is the identifier of the consent authorization request. type: string client: $ref: '#/components/schemas/oAuth2Client' @@ -3501,9 +3510,7 @@ components: - requested_scope properties: challenge: - description: |- - ID is the identifier ("login challenge") of the login request. It is used to - identify the session. + description: ID is the identifier of the login request. type: string client: $ref: '#/components/schemas/oAuth2Client' @@ -3631,9 +3638,7 @@ components: sid: sid properties: challenge: - description: |- - Challenge is the identifier ("logout challenge") of the logout authentication request. It is used to - identify the session. + description: Challenge is the identifier of the logout authentication request. type: string client: $ref: '#/components/schemas/oAuth2Client' diff --git a/internal/httpclient/api_o_auth2.go b/internal/httpclient/api_o_auth2.go index 0538dfc3b7a..4c6120c7307 100644 --- a/internal/httpclient/api_o_auth2.go +++ b/internal/httpclient/api_o_auth2.go @@ -2881,11 +2881,12 @@ func (a *OAuth2APIService) RejectOAuth2LogoutRequestExecute(r ApiRejectOAuth2Log } type ApiRevokeOAuth2ConsentSessionsRequest struct { - ctx context.Context - ApiService *OAuth2APIService - subject *string - client *string - all *bool + ctx context.Context + ApiService *OAuth2APIService + subject *string + client *string + consentChallengeId *string + all *bool } // OAuth 2.0 Consent Subject The subject whose consent sessions should be deleted. @@ -2900,6 +2901,12 @@ func (r ApiRevokeOAuth2ConsentSessionsRequest) Client(client string) ApiRevokeOA return r } +// Consent Challenge ID If set, revoke all token chains derived from this particular consent request ID. +func (r ApiRevokeOAuth2ConsentSessionsRequest) ConsentChallengeId(consentChallengeId string) ApiRevokeOAuth2ConsentSessionsRequest { + r.consentChallengeId = &consentChallengeId + return r +} + // Revoke All Consent Sessions If set to `true` deletes all consent sessions by the Subject that have been granted. func (r ApiRevokeOAuth2ConsentSessionsRequest) All(all bool) ApiRevokeOAuth2ConsentSessionsRequest { r.all = &all @@ -2952,6 +2959,9 @@ func (a *OAuth2APIService) RevokeOAuth2ConsentSessionsExecute(r ApiRevokeOAuth2C if r.client != nil { parameterAddToHeaderOrQuery(localVarQueryParams, "client", r.client, "") } + if r.consentChallengeId != nil { + parameterAddToHeaderOrQuery(localVarQueryParams, "consent_challenge_id", r.consentChallengeId, "") + } if r.all != nil { parameterAddToHeaderOrQuery(localVarQueryParams, "all", r.all, "") } diff --git a/internal/httpclient/docs/OAuth2API.md b/internal/httpclient/docs/OAuth2API.md index 6f8c0aac527..1362ff3ccf2 100644 --- a/internal/httpclient/docs/OAuth2API.md +++ b/internal/httpclient/docs/OAuth2API.md @@ -1532,7 +1532,7 @@ No authorization required ## RevokeOAuth2ConsentSessions -> RevokeOAuth2ConsentSessions(ctx).Subject(subject).Client(client).All(all).Execute() +> RevokeOAuth2ConsentSessions(ctx).Subject(subject).Client(client).ConsentChallengeId(consentChallengeId).All(all).Execute() Revoke OAuth 2.0 Consent Sessions of a Subject @@ -1553,11 +1553,12 @@ import ( func main() { subject := "subject_example" // string | OAuth 2.0 Consent Subject The subject whose consent sessions should be deleted. client := "client_example" // string | OAuth 2.0 Client ID If set, deletes only those consent sessions that have been granted to the specified OAuth 2.0 Client ID. (optional) + consentChallengeId := "consentChallengeId_example" // string | Consent Challenge ID If set, revoke all token chains derived from this particular consent request ID. (optional) all := true // bool | Revoke All Consent Sessions If set to `true` deletes all consent sessions by the Subject that have been granted. (optional) configuration := openapiclient.NewConfiguration() apiClient := openapiclient.NewAPIClient(configuration) - r, err := apiClient.OAuth2API.RevokeOAuth2ConsentSessions(context.Background()).Subject(subject).Client(client).All(all).Execute() + r, err := apiClient.OAuth2API.RevokeOAuth2ConsentSessions(context.Background()).Subject(subject).Client(client).ConsentChallengeId(consentChallengeId).All(all).Execute() if err != nil { fmt.Fprintf(os.Stderr, "Error when calling `OAuth2API.RevokeOAuth2ConsentSessions``: %v\n", err) fmt.Fprintf(os.Stderr, "Full HTTP response: %v\n", r) @@ -1578,6 +1579,7 @@ Name | Type | Description | Notes ------------- | ------------- | ------------- | ------------- **subject** | **string** | OAuth 2.0 Consent Subject The subject whose consent sessions should be deleted. | **client** | **string** | OAuth 2.0 Client ID If set, deletes only those consent sessions that have been granted to the specified OAuth 2.0 Client ID. | + **consentChallengeId** | **string** | Consent Challenge ID If set, revoke all token chains derived from this particular consent request ID. | **all** | **bool** | Revoke All Consent Sessions If set to `true` deletes all consent sessions by the Subject that have been granted. | ### Return type diff --git a/internal/httpclient/docs/OAuth2ConsentRequest.md b/internal/httpclient/docs/OAuth2ConsentRequest.md index f01dc3f79f9..ae2be51002d 100644 --- a/internal/httpclient/docs/OAuth2ConsentRequest.md +++ b/internal/httpclient/docs/OAuth2ConsentRequest.md @@ -6,7 +6,7 @@ Name | Type | Description | Notes ------------ | ------------- | ------------- | ------------- **Acr** | Pointer to **string** | ACR represents the Authentication AuthorizationContext Class Reference value for this authentication session. You can use it to express that, for example, a user authenticated using two factor authentication. | [optional] **Amr** | Pointer to **[]string** | | [optional] -**Challenge** | **string** | ID is the identifier (\"authorization challenge\") of the consent authorization request. It is used to identify the session. | +**Challenge** | **string** | ID is the identifier of the consent authorization request. | **Client** | Pointer to [**OAuth2Client**](OAuth2Client.md) | | [optional] **Context** | Pointer to **interface{}** | | [optional] **LoginChallenge** | Pointer to **string** | LoginChallenge is the login challenge this consent challenge belongs to. It can be used to associate a login and consent request in the login & consent app. | [optional] diff --git a/internal/httpclient/docs/OAuth2LoginRequest.md b/internal/httpclient/docs/OAuth2LoginRequest.md index e70218c297d..34037195301 100644 --- a/internal/httpclient/docs/OAuth2LoginRequest.md +++ b/internal/httpclient/docs/OAuth2LoginRequest.md @@ -4,7 +4,7 @@ Name | Type | Description | Notes ------------ | ------------- | ------------- | ------------- -**Challenge** | **string** | ID is the identifier (\"login challenge\") of the login request. It is used to identify the session. | +**Challenge** | **string** | ID is the identifier of the login request. | **Client** | [**OAuth2Client**](OAuth2Client.md) | | **OidcContext** | Pointer to [**OAuth2ConsentRequestOpenIDConnectContext**](OAuth2ConsentRequestOpenIDConnectContext.md) | | [optional] **RequestUrl** | **string** | RequestURL is the original OAuth 2.0 Authorization URL requested by the OAuth 2.0 client. It is the URL which initiates the OAuth 2.0 Authorization Code or OAuth 2.0 Implicit flow. This URL is typically not needed, but might come in handy if you want to deal with additional request parameters. | diff --git a/internal/httpclient/docs/OAuth2LogoutRequest.md b/internal/httpclient/docs/OAuth2LogoutRequest.md index 65a48ecb29d..81da891ef54 100644 --- a/internal/httpclient/docs/OAuth2LogoutRequest.md +++ b/internal/httpclient/docs/OAuth2LogoutRequest.md @@ -4,7 +4,7 @@ Name | Type | Description | Notes ------------ | ------------- | ------------- | ------------- -**Challenge** | Pointer to **string** | Challenge is the identifier (\"logout challenge\") of the logout authentication request. It is used to identify the session. | [optional] +**Challenge** | Pointer to **string** | Challenge is the identifier of the logout authentication request. | [optional] **Client** | Pointer to [**OAuth2Client**](OAuth2Client.md) | | [optional] **ExpiresAt** | Pointer to **time.Time** | | [optional] **RequestUrl** | Pointer to **string** | RequestURL is the original Logout URL requested. | [optional] diff --git a/internal/httpclient/model_o_auth2_consent_request.go b/internal/httpclient/model_o_auth2_consent_request.go index 78be5c543fa..ef753515dbb 100644 --- a/internal/httpclient/model_o_auth2_consent_request.go +++ b/internal/httpclient/model_o_auth2_consent_request.go @@ -25,7 +25,7 @@ type OAuth2ConsentRequest struct { // ACR represents the Authentication AuthorizationContext Class Reference value for this authentication session. You can use it to express that, for example, a user authenticated using two factor authentication. Acr *string `json:"acr,omitempty"` Amr []string `json:"amr,omitempty"` - // ID is the identifier (\"authorization challenge\") of the consent authorization request. It is used to identify the session. + // ID is the identifier of the consent authorization request. Challenge string `json:"challenge"` Client *OAuth2Client `json:"client,omitempty"` Context interface{} `json:"context,omitempty"` diff --git a/internal/httpclient/model_o_auth2_login_request.go b/internal/httpclient/model_o_auth2_login_request.go index 2ddac3113c8..9ab2ad3e606 100644 --- a/internal/httpclient/model_o_auth2_login_request.go +++ b/internal/httpclient/model_o_auth2_login_request.go @@ -22,7 +22,7 @@ var _ MappedNullable = &OAuth2LoginRequest{} // OAuth2LoginRequest struct for OAuth2LoginRequest type OAuth2LoginRequest struct { - // ID is the identifier (\"login challenge\") of the login request. It is used to identify the session. + // ID is the identifier of the login request. Challenge string `json:"challenge"` Client OAuth2Client `json:"client"` OidcContext *OAuth2ConsentRequestOpenIDConnectContext `json:"oidc_context,omitempty"` diff --git a/internal/httpclient/model_o_auth2_logout_request.go b/internal/httpclient/model_o_auth2_logout_request.go index 6ff85eb4062..8a792ab43ec 100644 --- a/internal/httpclient/model_o_auth2_logout_request.go +++ b/internal/httpclient/model_o_auth2_logout_request.go @@ -21,7 +21,7 @@ var _ MappedNullable = &OAuth2LogoutRequest{} // OAuth2LogoutRequest struct for OAuth2LogoutRequest type OAuth2LogoutRequest struct { - // Challenge is the identifier (\"logout challenge\") of the logout authentication request. It is used to identify the session. + // Challenge is the identifier of the logout authentication request. Challenge *string `json:"challenge,omitempty"` Client *OAuth2Client `json:"client,omitempty"` ExpiresAt *time.Time `json:"expires_at,omitempty"` diff --git a/oauth2/oauth2_auth_code_test.go b/oauth2/oauth2_auth_code_test.go index c74e5356570..40fa2fb0a82 100644 --- a/oauth2/oauth2_auth_code_test.go +++ b/oauth2/oauth2_auth_code_test.go @@ -129,7 +129,6 @@ func acceptConsentHandler(t *testing.T, c *client.Client, adminClient *hydra.API assert.EqualValues(t, c.RedirectURIs, rr.Client.RedirectUris) assert.EqualValues(t, subject, pointerx.Deref(rr.Subject)) assert.EqualValues(t, []string{"hydra", "offline", "openid"}, rr.RequestedScope) - assert.EqualValues(t, r.URL.Query().Get("consent_challenge"), rr.Challenge) assert.Contains(t, *rr.RequestUrl, reg.Config().OAuth2AuthURL(r.Context()).String()) if checkRequestPayload != nil { checkRequestPayload(rr) @@ -1192,6 +1191,38 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) { t.Run("strategy=jwt", run("jwt")) }) + t.Run("case=can revoke token chains with ID obtained from consent requests", func(t *testing.T) { + c, conf := newOAuth2Client(t, reg, testhelpers.NewCallbackURL(t, "callback", testhelpers.HTTPServerNotImplementedHandler)) + + var consentRequestID string + testhelpers.NewLoginConsentUI(t, reg.Config(), + acceptLoginHandler(t, c, adminClient, reg, subject, nil), + acceptConsentHandler(t, c, adminClient, reg, subject, func(ocr *hydra.OAuth2ConsentRequest) { + consentRequestID = ocr.Challenge + }), + ) + + code, _ := getAuthorizeCode(t, conf, nil) + require.NotEmpty(t, code) + + require.NotZero(t, consentRequestID) + t.Logf("Consent Request ID: %s", consentRequestID) + + token, err := conf.Exchange(context.Background(), code) + require.NoError(t, err) + + introspectAccessToken(t, conf, token, subject) + + _, err = adminClient.OAuth2API. + RevokeOAuth2ConsentSessions(context.Background()). + ConsentChallengeId(consentRequestID). + Execute() + require.NoError(t, err) + + at := testhelpers.IntrospectToken(t, conf, token.AccessToken, adminTS) + assert.False(t, at.Get("active").Bool(), "%s", at) + }) + t.Run("case=graceful token rotation", func(t *testing.T) { reg.Config().MustSet(ctx, config.KeyRefreshTokenRotationGracePeriod, "2s") reg.Config().Delete(ctx, config.KeyTokenHook) diff --git a/persistence/sql/persister_consent.go b/persistence/sql/persister_consent.go index ad3c25b3d72..168e099afe6 100644 --- a/persistence/sql/persister_consent.go +++ b/persistence/sql/persister_consent.go @@ -44,6 +44,14 @@ func (p *Persister) RevokeSubjectClientConsentSession(ctx context.Context, user, return p.Transaction(ctx, p.revokeConsentSession("consent_challenge_id IS NOT NULL AND subject = ? AND client_id = ?", user, client)) } +func (p *Persister) RevokeConsentSessionByID(ctx context.Context, consentChallengeID string) (err error) { + ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.RevokeConsentSessionByID", + trace.WithAttributes(attribute.String("consent_challenge_id", consentChallengeID))) + defer otelx.End(span, &err) + + return p.Transaction(ctx, p.revokeConsentSession("consent_challenge_id = ?", consentChallengeID)) +} + func (p *Persister) revokeConsentSession(whereStmt string, whereArgs ...interface{}) func(context.Context, *pop.Connection) error { return func(ctx context.Context, c *pop.Connection) error { fs := make([]*flow.Flow, 0) @@ -214,9 +222,6 @@ func (p *Persister) GetConsentRequest(ctx context.Context, challenge string) (_ return nil, err } - // We need to overwrite the ID with the encoded flow (challenge) so that the client is not confused. - f.ConsentChallengeID = sqlxx.NullString(challenge) - return f.GetConsentRequest(), nil } @@ -306,10 +311,6 @@ func (p *Persister) VerifyAndInvalidateConsentRequest(ctx context.Context, verif return nil, errorsx.WithStack(fosite.ErrInvalidRequest.WithDebug(err.Error())) } - // We set the consent challenge ID to a new UUID that we can use as a foreign key in the database - // without encoding the whole flow. - f.ConsentChallengeID = sqlxx.NullString(uuid.Must(uuid.NewV4()).String()) - if err = p.Connection(ctx).Create(f); err != nil { return nil, sqlcon.HandleError(err) } diff --git a/persistence/sql/persister_oauth2.go b/persistence/sql/persister_oauth2.go index 7595d3b58c4..851367e3411 100644 --- a/persistence/sql/persister_oauth2.go +++ b/persistence/sql/persister_oauth2.go @@ -681,7 +681,7 @@ func (p *Persister) strictRefreshRotation(ctx context.Context, requestID string) return nil } -func (p *Persister) gracefulRefreshRotation(ctx context.Context, requestID string, refreshSignature string, period time.Duration) (err error) { +func (p *Persister) gracefulRefreshRotation(ctx context.Context, requestID string, refreshSignature string) (err error) { ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.gracefulRefreshRotation", trace.WithAttributes( attribute.String("request_id", requestID), @@ -747,9 +747,8 @@ func (p *Persister) RotateRefreshToken(ctx context.Context, requestID string, re defer otelx.End(span, &err) // If we end up here, we have a valid refresh token and can proceed with the rotation. - gracePeriod := p.r.Config().RefreshTokenRotationGracePeriod(ctx) - if gracePeriod > 0 { - return handleRetryError(p.gracefulRefreshRotation(ctx, requestID, refreshTokenSignature, gracePeriod)) + if p.r.Config().RefreshTokenRotationGracePeriod(ctx) > 0 { + return handleRetryError(p.gracefulRefreshRotation(ctx, requestID, refreshTokenSignature)) } return handleRetryError(p.strictRefreshRotation(ctx, requestID)) diff --git a/spec/api.json b/spec/api.json index f1f96c7c1ba..a5d68c93b7b 100644 --- a/spec/api.json +++ b/spec/api.json @@ -839,7 +839,7 @@ "$ref": "#/components/schemas/StringSliceJSONFormat" }, "challenge": { - "description": "ID is the identifier (\"authorization challenge\") of the consent authorization request. It is used to\nidentify the session.", + "description": "ID is the identifier of the consent authorization request.", "type": "string" }, "client": { @@ -986,7 +986,7 @@ "oAuth2LoginRequest": { "properties": { "challenge": { - "description": "ID is the identifier (\"login challenge\") of the login request. It is used to\nidentify the session.", + "description": "ID is the identifier of the login request.", "type": "string" }, "client": { @@ -1031,7 +1031,7 @@ "oAuth2LogoutRequest": { "properties": { "challenge": { - "description": "Challenge is the identifier (\"logout challenge\") of the logout authentication request. It is used to\nidentify the session.", + "description": "Challenge is the identifier of the logout authentication request.", "type": "string" }, "client": { @@ -2908,6 +2908,14 @@ "type": "string" } }, + { + "description": "Consent Challenge ID\n\nIf set, revoke all token chains derived from this particular consent request ID.", + "in": "query", + "name": "consent_challenge_id", + "schema": { + "type": "string" + } + }, { "description": "Revoke All Consent Sessions\n\nIf set to `true` deletes all consent sessions by the Subject that have been granted.", "in": "query", diff --git a/spec/swagger.json b/spec/swagger.json index 4128c56448e..0e344153128 100755 --- a/spec/swagger.json +++ b/spec/swagger.json @@ -1258,6 +1258,12 @@ "name": "client", "in": "query" }, + { + "type": "string", + "description": "Consent Challenge ID\n\nIf set, revoke all token chains derived from this particular consent request ID.", + "name": "consent_challenge_id", + "in": "query" + }, { "type": "boolean", "description": "Revoke All Consent Sessions\n\nIf set to `true` deletes all consent sessions by the Subject that have been granted.", @@ -2862,7 +2868,7 @@ "$ref": "#/definitions/StringSliceJSONFormat" }, "challenge": { - "description": "ID is the identifier (\"authorization challenge\") of the consent authorization request. It is used to\nidentify the session.", + "description": "ID is the identifier of the consent authorization request.", "type": "string" }, "client": { @@ -2988,7 +2994,7 @@ ], "properties": { "challenge": { - "description": "ID is the identifier (\"login challenge\") of the login request. It is used to\nidentify the session.", + "description": "ID is the identifier of the login request.", "type": "string" }, "client": { @@ -3026,7 +3032,7 @@ "title": "Contains information about an ongoing logout request.", "properties": { "challenge": { - "description": "Challenge is the identifier (\"logout challenge\") of the logout authentication request. It is used to\nidentify the session.", + "description": "Challenge is the identifier of the logout authentication request.", "type": "string" }, "client": {