Skip to content

Commit

Permalink
Merge 2ec7a94 into f8d6542
Browse files Browse the repository at this point in the history
  • Loading branch information
hperl authored Mar 2, 2023
2 parents f8d6542 + 2ec7a94 commit 602b423
Show file tree
Hide file tree
Showing 61 changed files with 2,459 additions and 1,574 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
.bin/
.idea/
.vscode/
node_modules/
*.iml
*.exe
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ quicktest:
quicktest-hsm:
docker build --progress=plain -f .docker/Dockerfile-hsm --target test-hsm .

.PHONY: refresh
refresh:
UPDATE_SNAPSHOTS=true go test -failfast -short -tags sqlite,json1 ./...

authors: # updates the AUTHORS file
curl https://raw.githubusercontent.com/ory/ci/master/authors/authors.sh | env PRODUCT="Ory Hydra" bash

Expand Down
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
@@ -0,0 +1,34 @@
{
"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": {},
"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
@@ -0,0 +1,4 @@
{
"error": "The request was malformed or contained invalid parameters",
"error_description": "It is not allowed to choose your own access token strategy."
}
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,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,34 @@
{
"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": {},
"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
@@ -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
}
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."
}
24 changes: 23 additions & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ import (
"strings"
"time"

"github.com/ory/hydra/v2/driver/config"
"github.com/ory/x/stringsx"

"github.com/gobuffalo/pop/v6"
"github.com/gofrs/uuid"

"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"

"github.com/ory/fosite"
"github.com/ory/hydra/v2/x"
Expand Down Expand Up @@ -291,6 +292,13 @@ type Client struct {
// RegistrationClientURI is the URL used to update, get, or delete the OAuth2 Client.
RegistrationClientURI string `json:"registration_client_uri,omitempty" db:"-"`

// OAuth 2.0 Access Token Strategy
//
// AccessTokenStrategy is the strategy used to generate access tokens.
// Valid options are `jwt` and `opaque`. `jwt` is a bad idea, see https://www.ory.sh/docs/hydra/advanced#json-web-tokens
// Setting the stragegy here overrides the global setting in `strategies.access_token`.
AccessTokenStrategy string `json:"access_token_strategy,omitempty" db:"access_token_strategy" faker:"-"`

// 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:"-"`
Expand Down Expand Up @@ -536,3 +544,17 @@ func (c *Client) GetEffectiveLifespan(gt fosite.GrantType, tt fosite.TokenType,
}
return *cl
}

func (c *Client) GetAccessTokenStrategy() config.AccessTokenStrategyType {
// We ignore the error here, because the empty string will default to
// the global access token strategy.
s, _ := config.ToAccessTokenStrategyType(c.AccessTokenStrategy)
return s
}

func AccessTokenStrategySource(client fosite.Client) config.AccessTokenStrategySource {
if source, ok := client.(config.AccessTokenStrategySource); ok {
return source
}
return nil
}
17 changes: 13 additions & 4 deletions client/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,22 +109,22 @@ func TestHandler(t *testing.T) {

t.Run("valid auth", func(t *testing.T) {
actual, err := h.ValidDynamicAuth(&http.Request{Header: http.Header{"Authorization": {"Bearer " + expected.RegistrationAccessToken}}}, httprouter.Params{
httprouter.Param{Key: "id", Value: expected.GetID()},
{Key: "id", Value: expected.GetID()},
})
require.NoError(t, err, "authentication with registration access token works")
assert.EqualValues(t, expected.GetID(), actual.GetID())
})

t.Run("missing auth", func(t *testing.T) {
_, err := h.ValidDynamicAuth(&http.Request{}, httprouter.Params{
httprouter.Param{Key: "id", Value: expected.GetID()},
{Key: "id", Value: expected.GetID()},
})
require.Error(t, err, "authentication without registration access token fails")
})

t.Run("incorrect auth", func(t *testing.T) {
_, err := h.ValidDynamicAuth(&http.Request{Header: http.Header{"Authorization": {"Bearer invalid"}}}, httprouter.Params{
httprouter.Param{Key: "id", Value: expected.GetID()},
{Key: "id", Value: expected.GetID()},
})
require.Error(t, err, "authentication with invalid registration access token fails")
})
Expand Down Expand Up @@ -328,6 +328,15 @@ func TestHandler(t *testing.T) {
path: client.ClientsHandlerPath,
statusCode: http.StatusBadRequest,
},
{
d: "setting access token strategy fails",
payload: &client.Client{
RedirectURIs: []string{"http://localhost:3000/cb"},
AccessTokenStrategy: "jwt",
},
path: client.DynClientsHandlerPath,
statusCode: http.StatusBadRequest,
},
{
d: "setting skip_consent fails for dynamic registration",
payload: &client.Client{
Expand Down Expand Up @@ -383,7 +392,7 @@ func TestHandler(t *testing.T) {
assert.NotEmpty(t, gjson.Get(body, key).String(), "%s in %s", key, body)
}
}
snapshotx.SnapshotTExcept(t, json.RawMessage(body), exclude)
snapshotx.SnapshotT(t, json.RawMessage(body), snapshotx.ExceptPaths(exclude...))
})
}
})
Expand Down
22 changes: 18 additions & 4 deletions client/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net/url"
"strings"

"github.com/ory/herodot"
"github.com/ory/hydra/v2/driver/config"
"github.com/ory/hydra/v2/x"
"github.com/ory/x/ipx"
Expand Down Expand Up @@ -140,14 +141,14 @@ func (v *Validator) Validate(ctx context.Context, c *Client) error {
}

if c.SubjectType != "" {
if !stringslice.Has(v.r.Config().SubjectTypesSupported(ctx), c.SubjectType) {
return errorsx.WithStack(ErrInvalidClientMetadata.WithHintf("Subject type %s is not supported by server, only %v are allowed.", c.SubjectType, v.r.Config().SubjectTypesSupported(ctx)))
if !stringslice.Has(v.r.Config().SubjectTypesSupported(ctx, c), c.SubjectType) {
return errorsx.WithStack(ErrInvalidClientMetadata.WithHintf("Subject type %s is not supported by server, only %v are allowed.", c.SubjectType, v.r.Config().SubjectTypesSupported(ctx, c)))
}
} else {
if stringslice.Has(v.r.Config().SubjectTypesSupported(ctx), "public") {
if stringslice.Has(v.r.Config().SubjectTypesSupported(ctx, c), "public") {
c.SubjectType = "public"
} else {
c.SubjectType = v.r.Config().SubjectTypesSupported(ctx)[0]
c.SubjectType = v.r.Config().SubjectTypesSupported(ctx, c)[0]
}
}

Expand All @@ -173,6 +174,16 @@ func (v *Validator) Validate(ctx context.Context, c *Client) error {
}
}

if c.AccessTokenStrategy != "" {
s, err := config.ToAccessTokenStrategyType(c.AccessTokenStrategy)
if err != nil {
return errorsx.WithStack(ErrInvalidClientMetadata.
WithHintf("invalid access token strategy: %v", err))
}
// Canonicalize, just in case.
c.AccessTokenStrategy = string(s)
}

return nil
}

Expand All @@ -182,6 +193,9 @@ func (v *Validator) ValidateDynamicRegistration(ctx context.Context, c *Client)
WithHint(`"metadata" cannot be set for dynamic client registration`),
)
}
if c.AccessTokenStrategy != "" {
return errorsx.WithStack(herodot.ErrBadRequest.WithReasonf("It is not allowed to choose your own access token strategy."))
}
if c.SkipConsent {
return errorsx.WithStack(ErrInvalidRequest.WithDescription(`"skip_consent" cannot be set for dynamic client registration`))
}
Expand Down
Loading

0 comments on commit 602b423

Please sign in to comment.