Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: allow skipping the consent flow #3451

Merged
merged 1 commit into from
Mar 2, 2023
Merged

feat: allow skipping the consent flow #3451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...dler-common-case=create_clients-case=0-description=basic_dynamic_client_registration.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions ...s/TestHandler-common-case=create_clients-case=1-description=basic_admin_registration.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion ...ase=create_clients-case=2-description=metadata_fails_for_dynamic_client_registration.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
}
4 changes: 4 additions & 0 deletions ...reate_clients-case=6-description=setting_skip_consent_fails_for_dynamic_registration.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"error": "invalid_request",
"error_description": "'skip_consent' cannot be set for dynamic client registration"
}
35 changes: 35 additions & 0 deletions ...reate_clients-case=7-description=setting_skip_consent_suceeds_for_admin_registration.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
0 ...on=basic_dynamic_client_registration.json → ...on=basic_dynamic_client_registration.json
View file
Open in desktop
File renamed without changes.
1 change: 1 addition & 0 deletions ...case=7-description=empty_ID_succeeds.json → ...case=9-description=empty_ID_succeeds.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions client/.snapshots/TestHandler-common-case=fetching_existing_client-endpoint=admin.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions client/.snapshots/TestHandler-common-case=fetching_existing_client-endpoint=selfservice.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions client/.snapshots/TestHandler-common-case=update_the_lifespans_of_an_OAuth2_client.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions client/.snapshots/TestHandler-common-case=updating_existing_client-endpoint=admin.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions ...estHandler-common-case=updating_existing_client-endpoint=dynamic_client_registration.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion ...TestHandler-common-case=updating_existing_client_fails_with_metadata_on_self_service.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions client/.snapshots/TestHandler-create_client_registration_tokens-case=0-dynamic=true.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions client/.snapshots/TestHandler-create_client_registration_tokens-case=1-dynamic=false.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions client/.snapshots/TestHandler-create_client_registration_tokens-case=2-dynamic=false.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems to conflict with the comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea IDK, my IDE (Goland) does this automatically. I think this is not needed anyways (because the package is named jose already), but we can remove the comment. OK?


"github.com/ory/fosite"
"github.com/ory/hydra/v2/x"
Expand Down Expand Up @@ -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
}

Expand Down
6 changes: 6 additions & 0 deletions client/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
19 changes: 19 additions & 0 deletions client/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
hperl marked this conversation as resolved.
Show resolved Hide resolved
},
{
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{
Expand Down
25 changes: 13 additions & 12 deletions client/sdk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}

Expand Down
5 changes: 4 additions & 1 deletion client/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions cmd/.snapshots/TestCreateClient-case=creates_successfully.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": "",
Expand Down
1 change: 1 addition & 0 deletions cmd/.snapshots/TestCreateClient-case=supports_encryption.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": "",
Expand Down
1 change: 1 addition & 0 deletions cmd/.snapshots/TestCreateClient-case=supports_setting_flags.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": "",
Expand Down
1 change: 1 addition & 0 deletions cmd/.snapshots/TestGetClient-case=gets_client.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"owner": "",
"policy_uri": "",
"scope": "",
"skip_consent": false,
"subject_type": "",
"token_endpoint_auth_method": "client_secret_post",
"tos_uri": ""
Expand Down
2 changes: 2 additions & 0 deletions cmd/.snapshots/TestGetClient-case=gets_multiple_clients.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"redirect_uris": [],
"response_types": [],
"scope": "",
"skip_consent": false,
"subject_type": "",
"token_endpoint_auth_method": "client_secret_post",
"tos_uri": ""
Expand All @@ -35,6 +36,7 @@
"redirect_uris": [],
"response_types": [],
"scope": "",
"skip_consent": false,
"subject_type": "",
"token_endpoint_auth_method": "client_secret_post",
"tos_uri": ""
Expand Down
2 changes: 2 additions & 0 deletions cmd/.snapshots/TestImportClient-case=imports_clients_from_single_file.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"owner": "",
"policy_uri": "",
"scope": "foo",
"skip_consent": false,
"subject_type": "public",
"token_endpoint_auth_method": "client_secret_basic",
"tos_uri": "",
Expand All @@ -28,6 +29,7 @@
"owner": "",
"policy_uri": "",
"scope": "bar",
"skip_consent": false,
"subject_type": "public",
"token_endpoint_auth_method": "client_secret_basic",
"tos_uri": "",
Expand Down
2 changes: 2 additions & 0 deletions cmd/.snapshots/TestImportClient-case=performs_appropriate_error_reporting.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"owner": "",
"policy_uri": "",
"scope": "foo",
"skip_consent": false,
"subject_type": "public",
"token_endpoint_auth_method": "client_secret_basic",
"tos_uri": "",
Expand All @@ -28,6 +29,7 @@
"owner": "",
"policy_uri": "",
"scope": "bar",
"skip_consent": false,
"subject_type": "public",
"token_endpoint_auth_method": "client_secret_basic",
"tos_uri": "",
Expand Down
1 change: 1 addition & 0 deletions cmd/.snapshots/TestUpdateClient-case=creates_successfully.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": "",
Expand Down
1 change: 1 addition & 0 deletions cmd/.snapshots/TestUpdateClient-case=supports_encryption.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": "",
Expand Down
24 changes: 23 additions & 1 deletion cypress/integration/oauth2/authorize_code.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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
})
})
})
Loading