Skip to content

Commit

Permalink
feat(oidc): add pkce support (authelia#2924)
Browse files Browse the repository at this point in the history
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 authelia#2921
  • Loading branch information
FineWolf authored Mar 2, 2022
1 parent 8dcb8c4 commit 6ef6d04
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 12 deletions.
4 changes: 4 additions & 0 deletions config.template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
# -
Expand Down
34 changes: 34 additions & 0 deletions docs/configuration/identity-providers/oidc.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions internal/configuration/config.template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
# -
Expand Down
4 changes: 4 additions & 0 deletions internal/configuration/schema/identity_providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand Down Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions internal/configuration/validator/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 " +
Expand Down
8 changes: 8 additions & 0 deletions internal/configuration/validator/identity_providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions internal/configuration/validator/identity_providers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
5 changes: 5 additions & 0 deletions internal/handlers/handler_oidc_wellknown.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func oidcWellKnown(ctx *middlewares.AutheliaCtx) {
oidc.ClaimPreferredUsername,
oidc.ClaimDisplayName,
},
CodeChallengeMethodsSupported: []string{"S256"},

RequestURIParameterSupported: false,
BackChannelLogoutSupported: false,
Expand All @@ -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 {
Expand Down
17 changes: 10 additions & 7 deletions internal/oidc/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@ func NewOpenIDConnectProvider(configuration *schema.OpenIDConnectConfiguration)
provider.Store = NewOpenIDConnectStore(configuration)

composeConfiguration := &compose.Config{
AccessTokenLifespan: configuration.AccessTokenLifespan,
AuthorizeCodeLifespan: configuration.AuthorizeCodeLifespan,
IDTokenLifespan: configuration.IDTokenLifespan,
RefreshTokenLifespan: configuration.RefreshTokenLifespan,
SendDebugMessagesToClients: configuration.EnableClientDebugMessages,
MinParameterEntropy: configuration.MinimumParameterEntropy,
AccessTokenLifespan: configuration.AccessTokenLifespan,
AuthorizeCodeLifespan: configuration.AuthorizeCodeLifespan,
IDTokenLifespan: configuration.IDTokenLifespan,
RefreshTokenLifespan: configuration.RefreshTokenLifespan,
SendDebugMessagesToClients: configuration.EnableClientDebugMessages,
MinParameterEntropy: configuration.MinimumParameterEntropy,
EnforcePKCE: configuration.EnforcePKCE == "always",
EnforcePKCEForPublicClients: configuration.EnforcePKCE != "never",
EnablePKCEPlainChallengeMethod: configuration.EnablePKCEPlainChallenge,
}

keyManager, err := NewKeyManagerWithConfiguration(configuration)
Expand Down Expand Up @@ -82,7 +85,7 @@ func NewOpenIDConnectProvider(configuration *schema.OpenIDConnectConfiguration)
compose.OAuth2TokenIntrospectionFactory,
compose.OAuth2TokenRevocationFactory,

// compose.OAuth2PKCEFactory,.
compose.OAuth2PKCEFactory,
)

provider.herodot = herodot.NewJSONWriter(nil)
Expand Down
11 changes: 6 additions & 5 deletions internal/oidc/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,12 @@ type WellKnownConfiguration struct {
Algorithms []string `json:"id_token_signing_alg_values_supported"`
UserinfoAlgorithms []string `json:"userinfo_signing_alg_values_supported"`

SubjectTypesSupported []string `json:"subject_types_supported"`
ResponseTypesSupported []string `json:"response_types_supported"`
ResponseModesSupported []string `json:"response_modes_supported"`
ScopesSupported []string `json:"scopes_supported"`
ClaimsSupported []string `json:"claims_supported"`
SubjectTypesSupported []string `json:"subject_types_supported"`
ResponseTypesSupported []string `json:"response_types_supported"`
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"`
Expand Down

0 comments on commit 6ef6d04

Please sign in to comment.