Skip to content

Commit

Permalink
feat: allow skipping consent for trusted clients
Browse files Browse the repository at this point in the history
This adds a new boolean parameter `skip_consent` to the admin APIs of
the OAuth clients. When set, the consent flow will instruct the
consent app to skip user 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.
  • Loading branch information
hperl committed Feb 28, 2023
1 parent 9a5afd2 commit 87e9ad1
Show file tree
Hide file tree
Showing 42 changed files with 159 additions and 16 deletions.
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
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
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"
}
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"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"error": "The request was malformed or contained invalid parameters",
"error_description": "It is not allowed to choose your own OAuth2 Client secret."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"client_name": "",
"client_secret": "averylongsecret",
"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": false,
"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
}
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
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
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
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
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
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
}
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
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
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
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

"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
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,
}
9 changes: 9 additions & 0 deletions client/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,15 @@ func TestHandler(t *testing.T) {
path: client.ClientsHandlerPath,
statusCode: http.StatusBadRequest,
},
{
d: "setting skip_consent fails",
payload: &client.Client{
RedirectURIs: []string{"http://localhost:3000/cb"},
SkipConsent: true,
},
path: client.DynClientsHandlerPath,
statusCode: http.StatusBadRequest,
},
{
d: "basic dynamic client registration",
payload: &client.Client{
Expand Down
5 changes: 4 additions & 1 deletion client/validator.go
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
4 changes: 4 additions & 0 deletions consent/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,10 @@ func (h *Handler) getOAuth2ConsentRequest(w http.ResponseWriter, r *http.Request
request.RequestedAudience = []string{}
}

if request.Client.SkipConsent {
request.Skip = true
}

request.Client = sanitizeClient(request.Client)
h.r.Writer().Write(w, r, request)
}
Expand Down
66 changes: 55 additions & 11 deletions cypress/integration/oauth2/authorize_code.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
// Copyright © 2022 Ory Corp
// SPDX-License-Identifier: Apache-2.0

import { prng } from "../../helpers"
import {
prng,
} from "../../helpers"

describe("The OAuth 2.0 Authorization Code Grant", function () {
const nc = () => ({
describe("The OAuth 2.0 Authorization Code Grant", function() {
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 () {
it("should return an Access, Refresh, and ID Token when scope offline_access and openid are granted", function() {
const client = nc()
cy.authCodeFlow(client, {
consent: { scope: ["offline_access", "openid"] },
Expand All @@ -24,7 +27,11 @@ describe("The OAuth 2.0 Authorization Code Grant", function () {
.then((content) => {
const {
result,
token: { access_token, id_token, refresh_token },
token: {
access_token,
id_token,
refresh_token,
},
} = JSON.parse(content)

expect(result).to.equal("success")
Expand All @@ -34,7 +41,7 @@ describe("The OAuth 2.0 Authorization Code Grant", function () {
})
})

it("should return an Access and Refresh Token when scope offline_access is granted", function () {
it("should return an Access and Refresh Token when scope offline_access is granted", function() {
const client = nc()
cy.authCodeFlow(client, { consent: { scope: ["offline_access"] } })

Expand All @@ -43,7 +50,11 @@ describe("The OAuth 2.0 Authorization Code Grant", function () {
.then((content) => {
const {
result,
token: { access_token, id_token, refresh_token },
token: {
access_token,
id_token,
refresh_token,
},
} = JSON.parse(content)

expect(result).to.equal("success")
Expand All @@ -53,7 +64,7 @@ describe("The OAuth 2.0 Authorization Code Grant", function () {
})
})

it("should return an Access and ID Token when scope offline_access is granted", function () {
it("should return an Access and ID Token when scope offline_access is granted", function() {
const client = nc()
cy.authCodeFlow(client, { consent: { scope: ["openid"] } })

Expand All @@ -62,7 +73,11 @@ describe("The OAuth 2.0 Authorization Code Grant", function () {
.then((content) => {
const {
result,
token: { access_token, id_token, refresh_token },
token: {
access_token,
id_token,
refresh_token,
},
} = JSON.parse(content)

expect(result).to.equal("success")
Expand All @@ -72,7 +87,7 @@ describe("The OAuth 2.0 Authorization Code Grant", function () {
})
})

it("should return an Access Token when no scope is granted", function () {
it("should return an Access Token when no scope is granted", function() {
const client = nc()
cy.authCodeFlow(client, { consent: { scope: [] } })

Expand All @@ -81,7 +96,11 @@ describe("The OAuth 2.0 Authorization Code Grant", function () {
.then((content) => {
const {
result,
token: { access_token, id_token, refresh_token },
token: {
access_token,
id_token,
refresh_token,
},
} = JSON.parse(content)

expect(result).to.equal("success")
Expand All @@ -90,4 +109,29 @@ 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
})
})
})
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"scripts": {
"lint": "standard --fix \"test/**/*.js\" \"cypress/**/*.js\"",
"openapi-generator-cli": "openapi-generator-cli",
"test": "cypress run",
"test": "cypress run --spec \"cypress/integration/oauth2/authorize_code.js\"",
"test:watch": "cypress open",
"wait-on": "wait-on"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
"Secret": "secret-0001",
"SecretExpiresAt": 0,
"SectorIdentifierURI": "",
"SkipConsent": false,
"SubjectType": "",
"TermsOfServiceURI": "http://tos/0001",
"TokenEndpointAuthMethod": "none",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
"Secret": "secret-0002",
"SecretExpiresAt": 0,
"SectorIdentifierURI": "",
"SkipConsent": false,
"SubjectType": "",
"TermsOfServiceURI": "http://tos/0002",
"TokenEndpointAuthMethod": "none",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
"Secret": "secret-0003",
"SecretExpiresAt": 0,
"SectorIdentifierURI": "",
"SkipConsent": false,
"SubjectType": "",
"TermsOfServiceURI": "http://tos/0003",
"TokenEndpointAuthMethod": "none",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
"Secret": "secret-0004",
"SecretExpiresAt": 0,
"SectorIdentifierURI": "http://sector_id/0004",
"SkipConsent": false,
"SubjectType": "",
"TermsOfServiceURI": "http://tos/0004",
"TokenEndpointAuthMethod": "none",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
"Secret": "secret-0005",
"SecretExpiresAt": 0,
"SectorIdentifierURI": "http://sector_id/0005",
"SkipConsent": false,
"SubjectType": "",
"TermsOfServiceURI": "http://tos/0005",
"TokenEndpointAuthMethod": "token_auth-0005",
Expand Down
Loading

0 comments on commit 87e9ad1

Please sign in to comment.