From d1907d670f923bb7a2433b04c85b57077b747a59 Mon Sep 17 00:00:00 2001 From: sawadashota Date: Thu, 28 May 2020 17:45:11 +0900 Subject: [PATCH] feat: configure pkce enforcement for public clients (#1874) Signed-off-by: sawadashota --- .schema/config.schema.json | 7 +++++++ docs/docs/reference/configuration.md | 6 ++++++ driver/configuration/provider.go | 1 + driver/configuration/provider_viper.go | 5 +++++ driver/configuration/provider_viper_test.go | 1 + driver/registry_base.go | 1 + go.mod | 2 +- go.sum | 10 ++-------- internal/.hydra.yaml | 1 + internal/config/config.yaml | 2 ++ 10 files changed, 27 insertions(+), 9 deletions(-) diff --git a/.schema/config.schema.json b/.schema/config.schema.json index ffbdf180f3f..9e808f66f12 100644 --- a/.schema/config.schema.json +++ b/.schema/config.schema.json @@ -723,6 +723,13 @@ "examples": [ true ] + }, + "enforced_for_public_clients": { + "type": "boolean", + "description": "Sets whether PKCE should be enforced for public clients.", + "examples": [ + true + ] } } } diff --git a/docs/docs/reference/configuration.md b/docs/docs/reference/configuration.md index 572ef473c1c..98a374c284e 100644 --- a/docs/docs/reference/configuration.md +++ b/docs/docs/reference/configuration.md @@ -994,6 +994,12 @@ oauth2: # > set OAUTH2_PKCE_ENFORCED= # enforced: true + # Sets whether PKCE should be enforced for public clients. + # + # Examples: + # - true + enforced_for_public_clients: true + ## secrets ## # diff --git a/driver/configuration/provider.go b/driver/configuration/provider.go index 6170689ff91..1011224f9fc 100644 --- a/driver/configuration/provider.go +++ b/driver/configuration/provider.go @@ -63,6 +63,7 @@ type Provider interface { LoginURL() *url.URL LogoutURL() *url.URL PKCEEnforced() bool + EnforcePKCEForPublicClients() bool } func MustValidate(l logrus.FieldLogger, p Provider) { diff --git a/driver/configuration/provider_viper.go b/driver/configuration/provider_viper.go index 75d7ab6d0bf..d9bd49ce78a 100644 --- a/driver/configuration/provider_viper.go +++ b/driver/configuration/provider_viper.go @@ -71,6 +71,7 @@ const ( ViperKeyAccessTokenStrategy = "strategies.access_token" ViperKeySubjectIdentifierAlgorithmSalt = "oidc.subject_identifiers.pairwise.salt" ViperKeyPKCEEnforced = "oauth2.pkce.enforced" + ViperKeyPKCEEnforcedForPublicClients = "oauth2.pkce.enforced_for_public_clients" ViperKeyLogLevel = "log.level" ) @@ -441,3 +442,7 @@ func (v *ViperProvider) ShareOAuth2Debug() bool { func (v *ViperProvider) PKCEEnforced() bool { return viperx.GetBool(v.l, ViperKeyPKCEEnforced, false, "OAUTH2_PKCE_ENFORCED") } + +func (v *ViperProvider) EnforcePKCEForPublicClients() bool { + return viperx.GetBool(v.l, ViperKeyPKCEEnforcedForPublicClients, false, "OAUTH2_PKCE_ENFORCED_FOR_PUBLIC_CLIENTS") +} diff --git a/driver/configuration/provider_viper_test.go b/driver/configuration/provider_viper_test.go index 4b655baa2db..b225edb7b91 100644 --- a/driver/configuration/provider_viper_test.go +++ b/driver/configuration/provider_viper_test.go @@ -225,6 +225,7 @@ func TestViperProviderValidates(t *testing.T) { assert.Equal(t, true, c.ShareOAuth2Debug()) assert.Equal(t, 20, c.BCryptCost()) assert.Equal(t, true, c.PKCEEnforced()) + assert.Equal(t, true, c.EnforcePKCEForPublicClients()) // secrets assert.Equal(t, []byte{0x64, 0x40, 0x5f, 0xd4, 0x66, 0xc9, 0x8c, 0x88, 0xa7, 0xf2, 0xcb, 0x95, 0xcd, 0x95, 0xcb, 0xa3, 0x41, 0x49, 0x8b, 0x97, 0xba, 0x9e, 0x92, 0xee, 0x4c, 0xaf, 0xe0, 0x71, 0x23, 0x28, 0xeb, 0xfc}, c.GetSystemSecret()) diff --git a/driver/registry_base.go b/driver/registry_base.go index 5ae71710a3b..0dc1794723c 100644 --- a/driver/registry_base.go +++ b/driver/registry_base.go @@ -234,6 +234,7 @@ func (m *RegistryBase) oAuth2Config() *compose.Config { ScopeStrategy: m.ScopeStrategy(), SendDebugMessagesToClients: m.c.ShareOAuth2Debug(), EnforcePKCE: m.c.PKCEEnforced(), + EnforcePKCEForPublicClients: m.c.EnforcePKCEForPublicClients(), EnablePKCEPlainChallengeMethod: false, TokenURL: urlx.AppendPaths(m.c.PublicURL(), oauth2.TokenPath).String(), RedirectSecureChecker: x.IsRedirectURISecure(m.c), diff --git a/go.mod b/go.mod index 06664b1df99..c256e976ad8 100644 --- a/go.mod +++ b/go.mod @@ -31,7 +31,7 @@ require ( github.com/olekukonko/tablewriter v0.0.1 github.com/opentracing/opentracing-go v1.1.1-0.20190913142402-a7454ce5950e github.com/ory/analytics-go/v4 v4.0.1 - github.com/ory/fosite v0.31.2 + github.com/ory/fosite v0.31.3 github.com/ory/go-acc v0.2.1 github.com/ory/graceful v0.1.1 github.com/ory/herodot v0.7.0 diff --git a/go.sum b/go.sum index a8542a4c937..fbbe46d693f 100644 --- a/go.sum +++ b/go.sum @@ -823,8 +823,8 @@ github.com/ory/dockertest v3.3.5+incompatible/go.mod h1:1vX4m9wsvi00u5bseYwXaSnh github.com/ory/dockertest/v3 v3.5.4 h1:rYijlJuraj8D4OgC1DpYpCV8SGXrkviT3RVrjFy7OFc= github.com/ory/dockertest/v3 v3.5.4/go.mod h1:J8ZUbNB2FOhm1cFZW9xBpDsODqsSWcyYgtJYVPcnF70= github.com/ory/fosite v0.29.0/go.mod h1:0atSZmXO7CAcs6NPMI/Qtot8tmZYj04Nddoold4S2h0= -github.com/ory/fosite v0.31.2 h1:ZxKcIpQ5zJjIOBnp0M3vq2Tr/0+cTln1uXNRsOCqN08= -github.com/ory/fosite v0.31.2/go.mod h1:UeBhRgW6nAjTcd8S7kAo0IFsY/rTPyOXPq/t8N20Q8I= +github.com/ory/fosite v0.31.3 h1:5WjLwfs+yUALZjzKUKGN/M+ddBJ5Ol6NawxuCO2TuAg= +github.com/ory/fosite v0.31.3/go.mod h1:UeBhRgW6nAjTcd8S7kAo0IFsY/rTPyOXPq/t8N20Q8I= github.com/ory/go-acc v0.0.0-20181118080137-ddc355013f90/go.mod h1:sxnvPCxChFuSmTJGj8FdMupeq1BezCiEpDjTUXQ4hf4= github.com/ory/go-acc v0.2.1 h1:Pwcmwd/cSnwJsYN76+w3HU7oXeWFTkwj/KUj1qGDrVw= github.com/ory/go-acc v0.2.1/go.mod h1:0omgy2aa3nDBJ45VAKeLHH8ccPBudxLeic4xiDRtug0= @@ -839,12 +839,6 @@ github.com/ory/herodot v0.7.0 h1:DGPUyPDBZwQSaQzci4UW/edjG6OWixZTwXyfjBgEVgs= github.com/ory/herodot v0.7.0/go.mod h1:YXKOfAXYdQojDP5sD8m0ajowq3+QXNdtxA+QiUXBwn0= github.com/ory/jsonschema/v3 v3.0.1 h1:xzV7w2rt/Qn+jvh71joIXNKKOCqqNyTlaIxdxU0IQJc= github.com/ory/jsonschema/v3 v3.0.1/go.mod h1:jgLHekkFk0uiGdEWGleC+tOm6JSSP8cbf17PnBuGXlw= -github.com/ory/sdk/swagutil v0.0.0-20200430101046-db494fac5eb6 h1:oKOQcB9DZy7govO5k12s9Ofwm4d4yZiMLmeMZE6QWZY= -github.com/ory/sdk/swagutil v0.0.0-20200430101046-db494fac5eb6/go.mod h1:Ufg1eAyz+Zt3+oweSZVThG13ewewWCKwBmoNmK8Z0co= -github.com/ory/sdk/swagutil v0.0.0-20200518102822-125dad3c7cca h1:DjUzF+fp090HXh22UAxce9zvAjBXQUxwOfhSp/7P5ls= -github.com/ory/sdk/swagutil v0.0.0-20200518102822-125dad3c7cca/go.mod h1:71A/G1sB4wLBi2cvhOSC0RYb6KkiFKTaPMwoq/2gTqU= -github.com/ory/sdk/swagutil v0.0.0-20200523151358-56119ba2f929 h1:iVgpkGCW172uMWssek5ydbwniyq3hive4OzEfBhZ6/8= -github.com/ory/sdk/swagutil v0.0.0-20200523151358-56119ba2f929/go.mod h1:71A/G1sB4wLBi2cvhOSC0RYb6KkiFKTaPMwoq/2gTqU= github.com/ory/sdk/swagutil v0.0.0-20200523161155-efb49dc7bc74 h1:de+Kyuf480dHqkl2/P3aFpaljUi9JnFQUqs68l9vf6o= github.com/ory/sdk/swagutil v0.0.0-20200523161155-efb49dc7bc74/go.mod h1:71A/G1sB4wLBi2cvhOSC0RYb6KkiFKTaPMwoq/2gTqU= github.com/ory/viper v1.5.6/go.mod h1:TYmpFpKLxjQwvT4f0QPpkOn4sDXU1kDgAwJpgLYiQ28= diff --git a/internal/.hydra.yaml b/internal/.hydra.yaml index bab6758231c..e2b925cd7dd 100644 --- a/internal/.hydra.yaml +++ b/internal/.hydra.yaml @@ -104,6 +104,7 @@ oauth2: cost: 20 pkce: enforced: true + enforced_for_public_clients: true secrets: system: diff --git a/internal/config/config.yaml b/internal/config/config.yaml index 39d0ccf8382..fad51f1244c 100644 --- a/internal/config/config.yaml +++ b/internal/config/config.yaml @@ -376,6 +376,8 @@ oauth2: pkce: # Set this to true if you want PKCE to be enforced for all clients. enforced: false + # Set this to true if you want PKCE to be enforced for public clients. + enforced_for_public_clients: false session: # store encrypted data in database, default true encrypt_at_rest: true