feat: allow skipping consent for trusted clients

This adds a new boolean parameter `skip_consent` to the admin APIs of the OAuth clients. This parameter will be forwarded to the consent app as `client.skip_consent`. It is up to the consent app to act on this parameter, but the canonical implementation accepts the consent on the user's behalf, similar to when `skip` is set.
ory · Mar 1, 2023 · b383582 · b383582
1 parent 9a5afd2
commit b383582
Show file tree

Hide file tree

Showing 58 changed files with 452 additions and 37 deletions.
diff --git a/...dler-common-case=create_clients-case=0-description=basic_dynamic_client_registration.json b/...dler-common-case=create_clients-case=0-description=basic_dynamic_client_registration.json
@@ -20,6 +20,7 @@
   "token_endpoint_auth_method": "client_secret_basic",
   "userinfo_signed_response_alg": "none",
   "metadata": {},
+  "skip_consent": false,
   "authorization_code_grant_access_token_lifespan": null,
   "authorization_code_grant_id_token_lifespan": null,
   "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/...s/TestHandler-common-case=create_clients-case=1-description=basic_admin_registration.json b/...s/TestHandler-common-case=create_clients-case=1-description=basic_admin_registration.json
@@ -23,6 +23,7 @@
   "metadata": {
     "foo": "bar"
   },
+  "skip_consent": false,
   "authorization_code_grant_access_token_lifespan": null,
   "authorization_code_grant_id_token_lifespan": null,
   "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/...ase=create_clients-case=2-description=metadata_fails_for_dynamic_client_registration.json b/...ase=create_clients-case=2-description=metadata_fails_for_dynamic_client_registration.json
@@ -1,4 +1,4 @@
 {
   "error": "invalid_client_metadata",
-  "error_description": "The value of one of the Client Metadata fields is invalid and the server has rejected this request. Note that an Authorization Server MAY choose to substitute a valid value for any requested parameter of a Client's Metadata. metadata cannot be set for dynamic client registration'"
+  "error_description": "The value of one of the Client Metadata fields is invalid and the server has rejected this request. Note that an Authorization Server MAY choose to substitute a valid value for any requested parameter of a Client's Metadata. 'metadata' cannot be set for dynamic client registration"
 }
diff --git a/...reate_clients-case=6-description=setting_skip_consent_fails_for_dynamic_registration.json b/...reate_clients-case=6-description=setting_skip_consent_fails_for_dynamic_registration.json
@@ -0,0 +1,4 @@
+{
+  "error": "invalid_request",
+  "error_description": "'skip_consent' cannot be set for dynamic client registration"
+}
diff --git a/...reate_clients-case=7-description=setting_skip_consent_suceeds_for_admin_registration.json b/...reate_clients-case=7-description=setting_skip_consent_suceeds_for_admin_registration.json
@@ -0,0 +1,35 @@
+{
+  "client_name": "",
+  "client_secret": "2SKZkBf2P5g4toAXXnCrr~_sDM",
+  "redirect_uris": [
+    "http://localhost:3000/cb"
+  ],
+  "grant_types": null,
+  "response_types": null,
+  "scope": "offline_access offline openid",
+  "audience": [],
+  "owner": "",
+  "policy_uri": "",
+  "allowed_cors_origins": [],
+  "tos_uri": "",
+  "client_uri": "",
+  "logo_uri": "",
+  "contacts": null,
+  "client_secret_expires_at": 0,
+  "subject_type": "public",
+  "jwks": {},
+  "token_endpoint_auth_method": "client_secret_basic",
+  "userinfo_signed_response_alg": "none",
+  "metadata": {},
+  "skip_consent": true,
+  "authorization_code_grant_access_token_lifespan": null,
+  "authorization_code_grant_id_token_lifespan": null,
+  "authorization_code_grant_refresh_token_lifespan": null,
+  "client_credentials_grant_access_token_lifespan": null,
+  "implicit_grant_access_token_lifespan": null,
+  "implicit_grant_id_token_lifespan": null,
+  "jwt_bearer_grant_access_token_lifespan": null,
+  "refresh_token_grant_id_token_lifespan": null,
+  "refresh_token_grant_access_token_lifespan": null,
+  "refresh_token_grant_refresh_token_lifespan": null
+}
diff --git a/...on=basic_dynamic_client_registration.json → ...on=basic_dynamic_client_registration.json b/...on=basic_dynamic_client_registration.json → ...on=basic_dynamic_client_registration.json
diff --git a/...case=7-description=empty_ID_succeeds.json → ...case=9-description=empty_ID_succeeds.json b/...case=7-description=empty_ID_succeeds.json → ...case=9-description=empty_ID_succeeds.json
@@ -21,6 +21,7 @@
   "token_endpoint_auth_method": "client_secret_basic",
   "userinfo_signed_response_alg": "none",
   "metadata": {},
+  "skip_consent": false,
   "authorization_code_grant_access_token_lifespan": null,
   "authorization_code_grant_id_token_lifespan": null,
   "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/client/.snapshots/TestHandler-common-case=fetching_existing_client-endpoint=admin.json b/client/.snapshots/TestHandler-common-case=fetching_existing_client-endpoint=admin.json
@@ -21,6 +21,7 @@
     "token_endpoint_auth_method": "client_secret_basic",
     "userinfo_signed_response_alg": "none",
     "metadata": {},
+    "skip_consent": false,
     "authorization_code_grant_access_token_lifespan": null,
     "authorization_code_grant_id_token_lifespan": null,
     "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/client/.snapshots/TestHandler-common-case=fetching_existing_client-endpoint=selfservice.json b/client/.snapshots/TestHandler-common-case=fetching_existing_client-endpoint=selfservice.json
@@ -20,6 +20,7 @@
     "jwks": {},
     "token_endpoint_auth_method": "client_secret_basic",
     "userinfo_signed_response_alg": "none",
+    "skip_consent": false,
     "authorization_code_grant_access_token_lifespan": null,
     "authorization_code_grant_id_token_lifespan": null,
     "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/client/.snapshots/TestHandler-common-case=update_the_lifespans_of_an_OAuth2_client.json b/client/.snapshots/TestHandler-common-case=update_the_lifespans_of_an_OAuth2_client.json
@@ -21,6 +21,7 @@
     "token_endpoint_auth_method": "client_secret_basic",
     "userinfo_signed_response_alg": "none",
     "metadata": {},
+    "skip_consent": false,
     "authorization_code_grant_access_token_lifespan": "31h0m0s",
     "authorization_code_grant_id_token_lifespan": "32h0m0s",
     "authorization_code_grant_refresh_token_lifespan": "33h0m0s",

diff --git a/client/.snapshots/TestHandler-common-case=updating_existing_client-endpoint=admin.json b/client/.snapshots/TestHandler-common-case=updating_existing_client-endpoint=admin.json
@@ -23,6 +23,7 @@
     "token_endpoint_auth_method": "client_secret_basic",
     "userinfo_signed_response_alg": "none",
     "metadata": {},
+    "skip_consent": false,
     "authorization_code_grant_access_token_lifespan": null,
     "authorization_code_grant_id_token_lifespan": null,
     "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/...estHandler-common-case=updating_existing_client-endpoint=dynamic_client_registration.json b/...estHandler-common-case=updating_existing_client-endpoint=dynamic_client_registration.json
@@ -22,6 +22,7 @@
     "token_endpoint_auth_method": "client_secret_basic",
     "userinfo_signed_response_alg": "none",
     "metadata": {},
+    "skip_consent": false,
     "authorization_code_grant_access_token_lifespan": null,
     "authorization_code_grant_id_token_lifespan": null,
     "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/...TestHandler-common-case=updating_existing_client_fails_with_metadata_on_self_service.json b/...TestHandler-common-case=updating_existing_client_fails_with_metadata_on_self_service.json
@@ -1,7 +1,7 @@
 {
   "body": {
     "error": "invalid_client_metadata",
-    "error_description": "The value of one of the Client Metadata fields is invalid and the server has rejected this request. Note that an Authorization Server MAY choose to substitute a valid value for any requested parameter of a Client's Metadata. metadata cannot be set for dynamic client registration'"
+    "error_description": "The value of one of the Client Metadata fields is invalid and the server has rejected this request. Note that an Authorization Server MAY choose to substitute a valid value for any requested parameter of a Client's Metadata. 'metadata' cannot be set for dynamic client registration"
   },
   "status": 400
 }
diff --git a/client/.snapshots/TestHandler-create_client_registration_tokens-case=0-dynamic=true.json b/client/.snapshots/TestHandler-create_client_registration_tokens-case=0-dynamic=true.json
@@ -16,6 +16,7 @@
   "subject_type": "",
   "jwks": {},
   "metadata": {},
+  "skip_consent": false,
   "authorization_code_grant_access_token_lifespan": null,
   "authorization_code_grant_id_token_lifespan": null,
   "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/client/.snapshots/TestHandler-create_client_registration_tokens-case=1-dynamic=false.json b/client/.snapshots/TestHandler-create_client_registration_tokens-case=1-dynamic=false.json
@@ -16,6 +16,7 @@
   "subject_type": "",
   "jwks": {},
   "metadata": {},
+  "skip_consent": false,
   "authorization_code_grant_access_token_lifespan": null,
   "authorization_code_grant_id_token_lifespan": null,
   "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/client/.snapshots/TestHandler-create_client_registration_tokens-case=2-dynamic=false.json b/client/.snapshots/TestHandler-create_client_registration_tokens-case=2-dynamic=false.json
@@ -17,6 +17,7 @@
   "subject_type": "",
   "jwks": {},
   "metadata": {},
+  "skip_consent": false,
   "authorization_code_grant_access_token_lifespan": null,
   "authorization_code_grant_id_token_lifespan": null,
   "authorization_code_grant_refresh_token_lifespan": null,

diff --git a/client/client.go b/client/client.go
@@ -12,7 +12,7 @@ import (
 	"github.com/gobuffalo/pop/v6"
 	"github.com/gofrs/uuid"
 
-	jose "gopkg.in/square/go-jose.v2" // Naming the dependency jose is important for go-swagger to work, see https://github.com/go-swagger/go-swagger/issues/1587
+	"gopkg.in/square/go-jose.v2" // Naming the dependency jose is important for go-swagger to work, see https://github.com/go-swagger/go-swagger/issues/1587
 
 	"github.com/ory/fosite"
 	"github.com/ory/hydra/v2/x"
@@ -291,6 +291,10 @@ type Client struct {
 	// RegistrationClientURI is the URL used to update, get, or delete the OAuth2 Client.
 	RegistrationClientURI string `json:"registration_client_uri,omitempty" db:"-"`
 
+	// SkipConsent skips the consent screen for this client. This field can only
+	// be set from the admin API.
+	SkipConsent bool `json:"skip_consent" db:"skip_consent" faker:"-"`
+
 	Lifespans
 }
 

diff --git a/client/error.go b/client/error.go
@@ -20,3 +20,9 @@ var ErrInvalidRedirectURI = &fosite.RFC6749Error{
 	ErrorField:       "invalid_redirect_uri",
 	CodeField:        http.StatusBadRequest,
 }
+
+var ErrInvalidRequest = &fosite.RFC6749Error{
+	DescriptionField: "The request is missing a required parameter, includes an unsupported parameter value (other than grant type), repeats a parameter, includes multiple credentials, utilizes more than one mechanism for authenticating the client, or is otherwise malformed.",
+	ErrorField:       "invalid_request",
+	CodeField:        http.StatusBadRequest,
+}
diff --git a/client/handler_test.go b/client/handler_test.go
@@ -328,6 +328,25 @@ func TestHandler(t *testing.T) {
 					path:       client.ClientsHandlerPath,
 					statusCode: http.StatusBadRequest,
 				},
+				{
+					d: "setting skip_consent fails for dynamic registration",
+					payload: &client.Client{
+						RedirectURIs: []string{"http://localhost:3000/cb"},
+						SkipConsent:  true,
+					},
+					path:       client.DynClientsHandlerPath,
+					statusCode: http.StatusBadRequest,
+				},
+				{
+					d: "setting skip_consent suceeds for admin registration",
+					payload: &client.Client{
+						RedirectURIs: []string{"http://localhost:3000/cb"},
+						SkipConsent:  true,
+						Secret:       "2SKZkBf2P5g4toAXXnCrr~_sDM",
+					},
+					path:       client.ClientsHandlerPath,
+					statusCode: http.StatusCreated,
+				},
 				{
 					d: "basic dynamic client registration",
 					payload: &client.Client{

diff --git a/client/sdk_test.go b/client/sdk_test.go
@@ -38,27 +38,28 @@ import (
 
 func createTestClient(prefix string) hydra.OAuth2Client {
 	return hydra.OAuth2Client{
-		ClientName:                pointerx.String(prefix + "name"),
-		ClientSecret:              pointerx.String(prefix + "secret"),
-		ClientUri:                 pointerx.String(prefix + "uri"),
+		ClientName:                pointerx.Ptr(prefix + "name"),
+		ClientSecret:              pointerx.Ptr(prefix + "secret"),
+		ClientUri:                 pointerx.Ptr(prefix + "uri"),
 		Contacts:                  []string{prefix + "peter", prefix + "pan"},
 		GrantTypes:                []string{prefix + "client_credentials", prefix + "authorize_code"},
-		LogoUri:                   pointerx.String(prefix + "logo"),
-		Owner:                     pointerx.String(prefix + "an-owner"),
-		PolicyUri:                 pointerx.String(prefix + "policy-uri"),
-		Scope:                     pointerx.String(prefix + "foo bar baz"),
-		TosUri:                    pointerx.String(prefix + "tos-uri"),
+		LogoUri:                   pointerx.Ptr(prefix + "logo"),
+		Owner:                     pointerx.Ptr(prefix + "an-owner"),
+		PolicyUri:                 pointerx.Ptr(prefix + "policy-uri"),
+		Scope:                     pointerx.Ptr(prefix + "foo bar baz"),
+		TosUri:                    pointerx.Ptr(prefix + "tos-uri"),
 		ResponseTypes:             []string{prefix + "id_token", prefix + "code"},
 		RedirectUris:              []string{"https://" + prefix + "redirect-url", "https://" + prefix + "redirect-uri"},
-		ClientSecretExpiresAt:     pointerx.Int64(0),
-		TokenEndpointAuthMethod:   pointerx.String("client_secret_basic"),
-		UserinfoSignedResponseAlg: pointerx.String("none"),
-		SubjectType:               pointerx.String("public"),
+		ClientSecretExpiresAt:     pointerx.Ptr[int64](0),
+		TokenEndpointAuthMethod:   pointerx.Ptr("client_secret_basic"),
+		UserinfoSignedResponseAlg: pointerx.Ptr("none"),
+		SubjectType:               pointerx.Ptr("public"),
 		Metadata:                  map[string]interface{}{"foo": "bar"},
 		// because these values are not nullable in the SQL schema, we have to set them not nil
 		AllowedCorsOrigins: []string{},
 		Audience:           []string{},
 		Jwks:               map[string]interface{}{},
+		SkipConsent:        pointerx.Ptr(false),
 	}
 }
 

diff --git a/client/validator.go b/client/validator.go
@@ -179,9 +179,12 @@ func (v *Validator) Validate(ctx context.Context, c *Client) error {
 func (v *Validator) ValidateDynamicRegistration(ctx context.Context, c *Client) error {
 	if c.Metadata != nil {
 		return errorsx.WithStack(ErrInvalidClientMetadata.
-			WithHint(`metadata cannot be set for dynamic client registration'`),
+			WithHint(`"metadata" cannot be set for dynamic client registration`),
 		)
 	}
+	if c.SkipConsent {
+		return errorsx.WithStack(ErrInvalidRequest.WithDescription(`"skip_consent" cannot be set for dynamic client registration`))
+	}
 
 	return v.Validate(ctx, c)
 }

diff --git a/cmd/.snapshots/TestCreateClient-case=creates_successfully.json b/cmd/.snapshots/TestCreateClient-case=creates_successfully.json
@@ -15,6 +15,7 @@
     "code"
   ],
   "scope": "offline_access offline openid",
+  "skip_consent": false,
   "subject_type": "public",
   "token_endpoint_auth_method": "client_secret_basic",
   "tos_uri": "",

diff --git a/cmd/.snapshots/TestCreateClient-case=supports_encryption.json b/cmd/.snapshots/TestCreateClient-case=supports_encryption.json
@@ -21,6 +21,7 @@
     "code"
   ],
   "scope": "offline_access offline openid",
+  "skip_consent": false,
   "subject_type": "public",
   "token_endpoint_auth_method": "client_secret_basic",
   "tos_uri": "",

diff --git a/cmd/.snapshots/TestCreateClient-case=supports_setting_flags.json b/cmd/.snapshots/TestCreateClient-case=supports_setting_flags.json
@@ -21,6 +21,7 @@
     "code"
   ],
   "scope": "offline_access offline openid",
+  "skip_consent": false,
   "subject_type": "public",
   "token_endpoint_auth_method": "client_secret_basic",
   "tos_uri": "",

diff --git a/cmd/.snapshots/TestGetClient-case=gets_client.json b/cmd/.snapshots/TestGetClient-case=gets_client.json
@@ -8,6 +8,7 @@
   "owner": "",
   "policy_uri": "",
   "scope": "",
+  "skip_consent": false,
   "subject_type": "",
   "token_endpoint_auth_method": "client_secret_post",
   "tos_uri": ""

diff --git a/cmd/.snapshots/TestGetClient-case=gets_multiple_clients.json b/cmd/.snapshots/TestGetClient-case=gets_multiple_clients.json
@@ -15,6 +15,7 @@
     "redirect_uris": [],
     "response_types": [],
     "scope": "",
+    "skip_consent": false,
     "subject_type": "",
     "token_endpoint_auth_method": "client_secret_post",
     "tos_uri": ""
@@ -35,6 +36,7 @@
     "redirect_uris": [],
     "response_types": [],
     "scope": "",
+    "skip_consent": false,
     "subject_type": "",
     "token_endpoint_auth_method": "client_secret_post",
     "tos_uri": ""

diff --git a/cmd/.snapshots/TestImportClient-case=imports_clients_from_single_file.json b/cmd/.snapshots/TestImportClient-case=imports_clients_from_single_file.json
@@ -11,6 +11,7 @@
     "owner": "",
     "policy_uri": "",
     "scope": "foo",
+    "skip_consent": false,
     "subject_type": "public",
     "token_endpoint_auth_method": "client_secret_basic",
     "tos_uri": "",
@@ -28,6 +29,7 @@
     "owner": "",
     "policy_uri": "",
     "scope": "bar",
+    "skip_consent": false,
     "subject_type": "public",
     "token_endpoint_auth_method": "client_secret_basic",
     "tos_uri": "",

diff --git a/cmd/.snapshots/TestImportClient-case=performs_appropriate_error_reporting.json b/cmd/.snapshots/TestImportClient-case=performs_appropriate_error_reporting.json
@@ -11,6 +11,7 @@
     "owner": "",
     "policy_uri": "",
     "scope": "foo",
+    "skip_consent": false,
     "subject_type": "public",
     "token_endpoint_auth_method": "client_secret_basic",
     "tos_uri": "",
@@ -28,6 +29,7 @@
     "owner": "",
     "policy_uri": "",
     "scope": "bar",
+    "skip_consent": false,
     "subject_type": "public",
     "token_endpoint_auth_method": "client_secret_basic",
     "tos_uri": "",

diff --git a/cmd/.snapshots/TestUpdateClient-case=creates_successfully.json b/cmd/.snapshots/TestUpdateClient-case=creates_successfully.json
@@ -15,6 +15,7 @@
     "code"
   ],
   "scope": "offline_access offline openid",
+  "skip_consent": false,
   "subject_type": "public",
   "token_endpoint_auth_method": "client_secret_basic",
   "tos_uri": "",

diff --git a/cmd/.snapshots/TestUpdateClient-case=supports_encryption.json b/cmd/.snapshots/TestUpdateClient-case=supports_encryption.json
@@ -15,6 +15,7 @@
     "code"
   ],
   "scope": "offline_access offline openid",
+  "skip_consent": false,
   "subject_type": "public",
   "token_endpoint_auth_method": "client_secret_basic",
   "tos_uri": "",

diff --git a/cypress/integration/oauth2/authorize_code.js b/cypress/integration/oauth2/authorize_code.js
@@ -4,13 +4,14 @@
 import { prng } from "../../helpers"
 
 describe("The OAuth 2.0 Authorization Code Grant", function () {
-  const nc = () => ({
+  const nc = (extradata) => ({
     client_secret: prng(),
     scope: "offline_access openid",
     subject_type: "public",
     token_endpoint_auth_method: "client_secret_basic",
     redirect_uris: [`${Cypress.env("client_url")}/oauth2/callback`],
     grant_types: ["authorization_code", "refresh_token"],
+    ...extradata,
   })
 
   it("should return an Access, Refresh, and ID Token when scope offline_access and openid are granted", function () {
@@ -90,4 +91,25 @@ describe("The OAuth 2.0 Authorization Code Grant", function () {
         expect(refresh_token).to.be.undefined
       })
   })
+
+  it("should skip consent if the client is confgured thus", function () {
+    const client = nc({ skip_consent: true })
+    cy.authCodeFlow(client, {
+      consent: { scope: ["offline_access", "openid"], skip: true },
+    })
+
+    cy.get("body")
+      .invoke("text")
+      .then((content) => {
+        const {
+          result,
+          token: { access_token, id_token, refresh_token },
+        } = JSON.parse(content)
+
+        expect(result).to.equal("success")
+        expect(access_token).to.not.be.empty
+        expect(id_token).to.not.be.empty
+        expect(refresh_token).to.not.be.empty
+      })
+  })
 })