Merge branch 'master' into alnr/fast-revoke-login

Makefile

@@ -93,7 +93,7 @@ quicktest-hsm:
 
 .PHONY: test-refresh
 test-refresh:
-	UPDATE_SNAPSHOTS=true go test -failfast -short -tags sqlite,sqlite_omit_load_extension ./...
+	UPDATE_SNAPSHOTS=true go test -short -tags sqlite,sqlite_omit_load_extension ./...
 	DOCKER_CONTENT_TRUST=1 docker build --progress=plain -f .docker/Dockerfile-test-hsm  --target test-refresh-hsm -t oryd/hydra:${IMAGE_TAG} --target test-refresh-hsm .
 
 authors:  # updates the AUTHORS file

consent/handler.go

@@ -88,12 +88,12 @@ type revokeOAuth2ConsentSessions struct {
 	// in: query
 	Client string `json:"client"`
 
-	// Consent Challenge ID
+	// Consent Request ID
 	//
 	// If set, revoke all token chains derived from this particular consent request ID.
 	//
 	// in: query
-	ConsentChallengeID string `json:"consent_challenge_id"`
+	ConsentRequestID string `json:"consent_request_id"`
 
 	// Revoke All Consent Sessions
 	//
@@ -122,44 +122,37 @@ type revokeOAuth2ConsentSessions struct {
 //	  204: emptyResponse
 //	  default: errorOAuth2
 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 == "" && 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
-	}
+	var (
+		subject          = r.URL.Query().Get("subject")
+		client           = r.URL.Query().Get("client")
+		consentRequestID = r.URL.Query().Get("consent_request_id")
+		allClients       = r.URL.Query().Get("all") == "true"
+	)
 
 	switch {
-	case client != "":
+	case consentRequestID != "" && subject == "" && client == "":
+		if err := h.r.ConsentManager().RevokeConsentSessionByID(r.Context(), consentRequestID); err != nil && !errors.Is(err, x.ErrNotFound) {
+			h.r.Writer().WriteError(w, r, err)
+			return
+		}
+		events.Trace(r.Context(), events.ConsentRevoked, events.WithConsentRequestID(consentRequestID))
+
+	case consentRequestID == "" && subject != "" && client != "" && !allClients:
 		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
 		}
 		events.Trace(r.Context(), events.ConsentRevoked, events.WithSubject(subject), events.WithClientID(client))
-	case allClients:
+
+	case consentRequestID == "" && subject != "" && client == "" && allClients:
 		if err := h.r.ConsentManager().RevokeSubjectConsentSession(r.Context(), subject); err != nil && !errors.Is(err, x.ErrNotFound) {
 			h.r.Writer().WriteError(w, r, err)
 			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.`)))
+		h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint("Invalid combination of query parameters.")))
 		return
 	}
 
@@ -782,7 +775,7 @@ func (h *Handler) acceptOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ
 		return
 	}
 
-	p.ID = challenge
+	p.ConsentRequestID = cr.ConsentRequestID
 	p.RequestedAt = cr.RequestedAt
 	p.HandledAt = sqlxx.NullTime(time.Now().UTC())
 
@@ -899,12 +892,13 @@ func (h *Handler) rejectOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ
 		h.r.Writer().WriteError(w, r, err)
 		return
 	}
+	cr := f.GetConsentRequest(challenge)
 
 	request, err := h.r.ConsentManager().HandleConsentRequest(ctx, f, &flow.AcceptOAuth2ConsentRequest{
-		Error:       &p,
-		ID:          challenge,
-		RequestedAt: hr.RequestedAt,
-		HandledAt:   sqlxx.NullTime(time.Now().UTC()),
+		Error:            &p,
+		ConsentRequestID: cr.ConsentRequestID,
+		RequestedAt:      hr.RequestedAt,
+		HandledAt:        sqlxx.NullTime(time.Now().UTC()),
 	})
 	if err != nil {
 		h.r.Writer().WriteError(w, r, errorsx.WithStack(err))

consent/handler_test.go

@@ -187,18 +187,18 @@ func TestGetConsentRequest(t *testing.T) {
 				challenge, err = f.ToConsentChallenge(ctx, reg)
 				require.NoError(t, err)
 				require.NoError(t, reg.ConsentManager().CreateConsentRequest(ctx, f, &flow.OAuth2ConsentRequest{
-					Client:         cl,
-					ID:             challenge,
-					Verifier:       challenge,
-					CSRF:           challenge,
-					LoginChallenge: sqlxx.NullString(lr.ID),
+					Client:           cl,
+					ConsentRequestID: challenge,
+					Verifier:         challenge,
+					CSRF:             challenge,
+					LoginChallenge:   sqlxx.NullString(lr.ID),
 				}))
 
 				if tc.handled {
 					_, err := reg.ConsentManager().HandleConsentRequest(ctx, f, &flow.AcceptOAuth2ConsentRequest{
-						ID:         challenge,
-						WasHandled: true,
-						HandledAt:  sqlxx.NullTime(time.Now()),
+						ConsentRequestID: challenge,
+						WasHandled:       true,
+						HandledAt:        sqlxx.NullTime(time.Now()),
 					})
 					require.NoError(t, err)
 					challenge, err = f.ToConsentChallenge(ctx, reg)

consent/helper_test.go

@@ -39,52 +39,52 @@ func TestSanitizeClient(t *testing.T) {
 
 func TestMatchScopes(t *testing.T) {
 	for k, tc := range []struct {
-		granted         []flow.AcceptOAuth2ConsentRequest
-		requested       []string
-		expectChallenge string
+		granted    []flow.AcceptOAuth2ConsentRequest
+		requested  []string
+		expectedID string
 	}{
 		{
-			granted:         []flow.AcceptOAuth2ConsentRequest{{ID: "1", GrantedScope: []string{"foo", "bar"}}},
-			requested:       []string{"foo", "bar"},
-			expectChallenge: "1",
+			granted:    []flow.AcceptOAuth2ConsentRequest{{ConsentRequestID: "1", GrantedScope: []string{"foo", "bar"}}},
+			requested:  []string{"foo", "bar"},
+			expectedID: "1",
 		},
 		{
-			granted:         []flow.AcceptOAuth2ConsentRequest{{ID: "1", GrantedScope: []string{"foo", "bar"}}},
-			requested:       []string{"foo", "bar", "baz"},
-			expectChallenge: "",
+			granted:    []flow.AcceptOAuth2ConsentRequest{{ConsentRequestID: "1", GrantedScope: []string{"foo", "bar"}}},
+			requested:  []string{"foo", "bar", "baz"},
+			expectedID: "",
 		},
 		{
 			granted: []flow.AcceptOAuth2ConsentRequest{
-				{ID: "1", GrantedScope: []string{"foo", "bar"}},
-				{ID: "2", GrantedScope: []string{"foo", "bar"}},
+				{ConsentRequestID: "1", GrantedScope: []string{"foo", "bar"}},
+				{ConsentRequestID: "2", GrantedScope: []string{"foo", "bar"}},
 			},
-			requested:       []string{"foo", "bar"},
-			expectChallenge: "1",
+			requested:  []string{"foo", "bar"},
+			expectedID: "1",
 		},
 		{
 			granted: []flow.AcceptOAuth2ConsentRequest{
-				{ID: "1", GrantedScope: []string{"foo", "bar"}},
-				{ID: "2", GrantedScope: []string{"foo", "bar", "baz"}},
+				{ConsentRequestID: "1", GrantedScope: []string{"foo", "bar"}},
+				{ConsentRequestID: "2", GrantedScope: []string{"foo", "bar", "baz"}},
 			},
-			requested:       []string{"foo", "bar", "baz"},
-			expectChallenge: "2",
+			requested:  []string{"foo", "bar", "baz"},
+			expectedID: "2",
 		},
 		{
 			granted: []flow.AcceptOAuth2ConsentRequest{
-				{ID: "1", GrantedScope: []string{"foo", "bar"}},
-				{ID: "2", GrantedScope: []string{"foo", "bar", "baz"}},
+				{ConsentRequestID: "1", GrantedScope: []string{"foo", "bar"}},
+				{ConsentRequestID: "2", GrantedScope: []string{"foo", "bar", "baz"}},
 			},
-			requested:       []string{"zab"},
-			expectChallenge: "",
+			requested:  []string{"zab"},
+			expectedID: "",
 		},
 	} {
 		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
 			got := matchScopes(fosite.ExactScopeStrategy, tc.granted, tc.requested)
-			if tc.expectChallenge == "" {
+			if tc.expectedID == "" {
 				assert.Nil(t, got)
 				return
 			}
-			assert.Equal(t, tc.expectChallenge, got.ID)
+			assert.Equal(t, tc.expectedID, got.ConsentRequestID)
 		})
 	}
 }

consent/janitor_consent_test_helper.go

@@ -32,7 +32,7 @@ func NewHandledLoginRequest(challenge string, hasError bool, requestedAt time.Ti
 	}
 }
 
-func NewHandledConsentRequest(challenge string, hasError bool, requestedAt time.Time, authenticatedAt sqlxx.NullTime) *flow.AcceptOAuth2ConsentRequest {
+func NewHandledConsentRequest(consentRequestID string, hasError bool, requestedAt time.Time, authenticatedAt sqlxx.NullTime) *flow.AcceptOAuth2ConsentRequest {
 	var deniedErr *flow.RequestDeniedError
 	if hasError {
 		deniedErr = &flow.RequestDeniedError{
@@ -46,11 +46,11 @@ func NewHandledConsentRequest(challenge string, hasError bool, requestedAt time.
 	}
 
 	return &flow.AcceptOAuth2ConsentRequest{
-		ID:              challenge,
-		HandledAt:       sqlxx.NullTime(time.Now().Round(time.Second)),
-		Error:           deniedErr,
-		RequestedAt:     requestedAt,
-		AuthenticatedAt: authenticatedAt,
-		WasHandled:      true,
+		ConsentRequestID: consentRequestID,
+		HandledAt:        sqlxx.NullTime(time.Now().Round(time.Second)),
+		Error:            deniedErr,
+		RequestedAt:      requestedAt,
+		AuthenticatedAt:  authenticatedAt,
+		WasHandled:       true,
 	}
 }

consent/manager.go

@@ -30,7 +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
+		RevokeConsentSessionByID(ctx context.Context, consentRequestID string) error
 
 		VerifyAndInvalidateConsentRequest(ctx context.Context, verifier string) (*flow.AcceptOAuth2ConsentRequest, error)
 		FindGrantedAndRememberedConsentRequests(ctx context.Context, client, user string) ([]flow.AcceptOAuth2ConsentRequest, error)

consent/strategy_default.go

@@ -585,13 +585,13 @@ func (s *DefaultStrategy) forwardConsentRequest(
 
 	// Set up csrf/challenge/verifier values
 	verifier := strings.Replace(uuid.New(), "-", "", -1)
-	consentChallengeID := strings.Replace(uuid.New(), "-", "", -1)
+	consentRequestID := strings.Replace(uuid.New(), "-", "", -1)
 	csrf := strings.Replace(uuid.New(), "-", "", -1)
 
 	cl := sanitizeClientFromRequest(ar)
 
 	consentRequest := &flow.OAuth2ConsentRequest{
-		ID:                     consentChallengeID,
+		ConsentRequestID:       consentRequestID,
 		ACR:                    as.ACR,
 		AMR:                    as.AMR,
 		Verifier:               verifier,

consent/strategy_default_test.go

@@ -47,7 +47,7 @@ func checkAndAcceptConsentHandler(t *testing.T, apiClient *hydra.APIClient, cb f
 		payload := cb(t, res, err)
 
 		v, _, err := apiClient.OAuth2API.AcceptOAuth2ConsentRequest(context.Background()).
-			ConsentChallenge(r.URL.Query().Get("consent_challenge")).
+			ConsentChallenge(res.Challenge).
 			AcceptOAuth2ConsentRequest(payload).
 			Execute()
 		require.NoError(t, err)

consent/strategy_oauth_test.go

@@ -207,11 +207,12 @@ func TestStrategyLoginConsentNext(t *testing.T) {
 
 		testhelpers.NewLoginConsentUI(t, reg.Config(),
 			func(w http.ResponseWriter, r *http.Request) {
+				loginChallenge = r.URL.Query().Get("login_challenge")
 				res, _, err := adminClient.OAuth2API.GetOAuth2LoginRequest(ctx).
-					LoginChallenge(r.URL.Query().Get("login_challenge")).
+					LoginChallenge(loginChallenge).
 					Execute()
 				require.NoError(t, err)
-				loginChallenge = res.Challenge
+				require.Equal(t, loginChallenge, res.Challenge)
 
 				v, _, err := adminClient.OAuth2API.AcceptOAuth2LoginRequest(ctx).
 					LoginChallenge(loginChallenge).
@@ -223,10 +224,11 @@ func TestStrategyLoginConsentNext(t *testing.T) {
 			},
 			func(w http.ResponseWriter, r *http.Request) {
 				consentChallenge = r.URL.Query().Get("consent_challenge")
-				_, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(ctx).
+				res, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(ctx).
 					ConsentChallenge(consentChallenge).
 					Execute()
 				require.NoError(t, err)
+				require.Equal(t, consentChallenge, res.Challenge)
 
 				v, _, err := adminClient.OAuth2API.AcceptOAuth2ConsentRequest(ctx).
 					ConsentChallenge(consentChallenge).