feat(oidc): add pkce support (#2924)

Implements Proof Key for Code Exchange for OpenID Connect Authorization Code Flow. By default this is enabled for the public client type and requires the S256 challenge method.

Closes #2921
pull/2937/head^2
Andrew Moore 2022-03-01 23:44:05 -05:00 committed by GitHub
parent 8dcb8c4e29
commit 6ef6d0499a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 96 additions and 12 deletions

View File

@ -684,6 +684,10 @@ notifier:
## security reasons.
# minimum_parameter_entropy: 8
## SECURITY NOTICE: It's not recommended changing this option, and highly discouraged to have it set to 'never'
## for security reasons.
# enforce_pkce: public_clients_only
## Clients is a list of known clients and their configuration.
# clients:
# -

View File

@ -34,6 +34,7 @@ identity_providers:
id_token_lifespan: 1h
refresh_token_lifespan: 90m
enable_client_debug_messages: false
enforce_pkce: public_clients_only
clients:
- id: myapp
description: My Application
@ -184,6 +185,39 @@ This controls the minimum length of the `nonce` and `state` parameters.
make certain scenarios less secure. It is highly encouraged that if your OpenID Connect RP does not send these parameters
or sends parameters with a lower length than the default that they implement a change rather than changing this value.
### enforce_pkce
<div markdown="1">
type: string
{: .label .label-config .label-purple }
default: public_clients_only
{: .label .label-config .label-blue }
required: no
{: .label .label-config .label-green }
</div>
[Proof Key for Code Exchange](https://datatracker.ietf.org/doc/html/rfc7636) enforcement policy: if specified, must be either `never`, `public_clients_only` or `always`.
If set to `public_clients_only` (default), PKCE will be required for public clients using the Authorization Code flow.
When set to `always`, PKCE will be required for all clients using the Authorization Code flow.
***Security Notice:*** Changing this value to `never` is generally discouraged, reducing it from the default can theoretically
make certain client-side applications (mobile applications, SPA) vulnerable to CSRF and authorization code interception attacks.
### enable_pkce_plain_challenge
<div markdown="1">
type: boolean
{: .label .label-config .label-purple }
default: false
{: .label .label-config .label-blue }
required: no
{: .label .label-config .label-green }
</div>
Allows PKCE `plain` challenges when set to `true`.
***Security Notice:*** Changing this value is generally discouraged. Applications should use the `S256` PKCE challenge method instead.
### clients
A list of clients to configure. The options for each client are described below.

View File

@ -684,6 +684,10 @@ notifier:
## security reasons.
# minimum_parameter_entropy: 8
## SECURITY NOTICE: It's not recommended changing this option, and highly discouraged to have it set to 'never'
## for security reasons.
# enforce_pkce: public_clients_only
## Clients is a list of known clients and their configuration.
# clients:
# -

View File

@ -21,6 +21,9 @@ type OpenIDConnectConfiguration struct {
EnableClientDebugMessages bool `koanf:"enable_client_debug_messages"`
MinimumParameterEntropy int `koanf:"minimum_parameter_entropy"`
EnforcePKCE string `koanf:"enforce_pkce"`
EnablePKCEPlainChallenge bool `koanf:"enable_pkce_plain_challenge"`
Clients []OpenIDConnectClientConfiguration `koanf:"clients"`
}
@ -49,6 +52,7 @@ var DefaultOpenIDConnectConfiguration = OpenIDConnectConfiguration{
AuthorizeCodeLifespan: time.Minute,
IDTokenLifespan: time.Hour,
RefreshTokenLifespan: time.Minute * 90,
EnforcePKCE: "public_clients_only",
}
// DefaultOpenIDConnectClientConfiguration contains defaults for OIDC Clients.

View File

@ -121,6 +121,9 @@ const (
"more clients configured"
errFmtOIDCNoPrivateKey = "identity_providers: oidc: option 'issuer_private_key' is required"
errFmtOIDCEnforcePKCEInvalidValue = "identity_providers: oidc: option 'enforce_pkce' must be 'never', " +
"'public_clients_only' or 'always', but it is configured as '%s'"
errFmtOIDCClientsDuplicateID = "identity_providers: oidc: one or more clients have the same id but all client" +
"id's must be unique"
errFmtOIDCClientsWithEmptyID = "identity_providers: oidc: one or more clients have been configured with " +

View File

@ -41,6 +41,14 @@ func validateOIDC(config *schema.OpenIDConnectConfiguration, validator *schema.S
validator.PushWarning(fmt.Errorf(errFmtOIDCServerInsecureParameterEntropy, config.MinimumParameterEntropy))
}
if config.EnforcePKCE == "" {
config.EnforcePKCE = schema.DefaultOpenIDConnectConfiguration.EnforcePKCE
}
if config.EnforcePKCE != "never" && config.EnforcePKCE != "public_clients_only" && config.EnforcePKCE != "always" {
validator.Push(fmt.Errorf(errFmtOIDCEnforcePKCEInvalidValue, config.EnforcePKCE))
}
validateOIDCClients(config, validator)
if len(config.Clients) == 0 {

View File

@ -29,6 +29,24 @@ func TestShouldRaiseErrorWhenInvalidOIDCServerConfiguration(t *testing.T) {
assert.EqualError(t, validator.Errors()[1], errFmtOIDCNoClientsConfigured)
}
func TestShouldRaiseErrorWhenOIDCPKCEEnforceValueInvalid(t *testing.T) {
validator := schema.NewStructValidator()
config := &schema.IdentityProvidersConfiguration{
OIDC: &schema.OpenIDConnectConfiguration{
HMACSecret: "rLABDrx87et5KvRHVUgTm3pezWWd8LMN",
IssuerPrivateKey: "key-material",
EnforcePKCE: "invalid",
},
}
ValidateIdentityProviders(config, validator)
require.Len(t, validator.Errors(), 2)
assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: option 'enforce_pkce' must be 'never', 'public_clients_only' or 'always', but it is configured as 'invalid'")
assert.EqualError(t, validator.Errors()[1], errFmtOIDCNoClientsConfigured)
}
func TestShouldRaiseErrorWhenOIDCServerIssuerPrivateKeyPathInvalid(t *testing.T) {
validator := schema.NewStructValidator()
config := &schema.IdentityProvidersConfiguration{

View File

@ -74,6 +74,7 @@ func oidcWellKnown(ctx *middlewares.AutheliaCtx) {
oidc.ClaimPreferredUsername,
oidc.ClaimDisplayName,
},
CodeChallengeMethodsSupported: []string{"S256"},
RequestURIParameterSupported: false,
BackChannelLogoutSupported: false,
@ -82,6 +83,10 @@ func oidcWellKnown(ctx *middlewares.AutheliaCtx) {
FrontChannelLogoutSessionSupported: false,
}
if ctx.Configuration.IdentityProviders.OIDC.EnablePKCEPlainChallenge {
wellKnown.CodeChallengeMethodsSupported = append(wellKnown.CodeChallengeMethodsSupported, "plain")
}
ctx.SetContentType("application/json")
if err := json.NewEncoder(ctx).Encode(wellKnown); err != nil {

View File

@ -29,6 +29,9 @@ func NewOpenIDConnectProvider(configuration *schema.OpenIDConnectConfiguration)
RefreshTokenLifespan: configuration.RefreshTokenLifespan,
SendDebugMessagesToClients: configuration.EnableClientDebugMessages,
MinParameterEntropy: configuration.MinimumParameterEntropy,
EnforcePKCE: configuration.EnforcePKCE == "always",
EnforcePKCEForPublicClients: configuration.EnforcePKCE != "never",
EnablePKCEPlainChallengeMethod: configuration.EnablePKCEPlainChallenge,
}
keyManager, err := NewKeyManagerWithConfiguration(configuration)
@ -82,7 +85,7 @@ func NewOpenIDConnectProvider(configuration *schema.OpenIDConnectConfiguration)
compose.OAuth2TokenIntrospectionFactory,
compose.OAuth2TokenRevocationFactory,
// compose.OAuth2PKCEFactory,.
compose.OAuth2PKCEFactory,
)
provider.herodot = herodot.NewJSONWriter(nil)

View File

@ -91,6 +91,7 @@ type WellKnownConfiguration struct {
ResponseModesSupported []string `json:"response_modes_supported"`
ScopesSupported []string `json:"scopes_supported"`
ClaimsSupported []string `json:"claims_supported"`
CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"`
RequestURIParameterSupported bool `json:"request_uri_parameter_supported"`
BackChannelLogoutSupported bool `json:"backchannel_logout_supported"`