Skip to content

Commit

Permalink
feat: client-side PKCE
Browse files Browse the repository at this point in the history
This change introduces a new configuration for OIDC providers: pkce with values auto (default), never, force.

When auto is specified or the field is omitted, Kratos will perform autodiscovery and perform PKCE when the server advertises support for it. This requires the issuer_url to be set for the provider.

never completely disables PKCE support. This is only theoretically useful: when a provider advertises PKCE support but doesn't actually implement it.

force always sends a PKCE challenge in the initial redirect URL, regardless of what the provider advertises. This setting is useful when the provider offers PKCE but doesn't advertise it in his ./well-known/openid-configuration.

Important: When setting pkce: force, you must whitelist a different return URL for your OAuth2 client in the provider's configuration. Instead of <base-url>/self-service/methods/oidc/callback/<provider>, you must use <base-url>/self-service/methods/oidc/callback (note missing last path segment). This is to enable the use of the same OAuth client ID+secret when configuring several Kratos OIDC providers, without having to whitelist individual redirect_uris for each Kratos provider config.
  • Loading branch information
alnr committed Sep 10, 2024
1 parent 2a2d707 commit 6c40e7f
Show file tree
Hide file tree
Showing 43 changed files with 3,639 additions and 142 deletions.
21 changes: 18 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,28 @@ docs/swagger:
curl https://raw.githubusercontent.com/ory/meta/master/install.sh | bash -s -- -b .bin ory v0.2.2
touch -a -m .bin/ory

.bin/buf: Makefile
curl -sSL \
"https://github.com/bufbuild/buf/releases/download/v1.39.0/buf-$(shell uname -s)-$(shell uname -m).tar.gz" | \
tar -xvzf - -C ".bin/" --strip-components=2 buf/bin/buf buf/bin/protoc-gen-buf-breaking buf/bin/protoc-gen-buf-lint
touch -a -m .bin/buf

.PHONY: lint
lint: .bin/golangci-lint
golangci-lint run -v --timeout 10m ./...
.bin/golangci-lint run -v --timeout 10m ./...
.bin/buf lint

.PHONY: mocks
mocks: .bin/mockgen
mockgen -mock_names Manager=MockLoginExecutorDependencies -package internal -destination internal/hook_login_executor_dependencies.go github.com/ory/kratos/selfservice loginExecutorDependencies

.PHONY: proto
proto: gen/oidc/v1/state.pb.go

gen/oidc/v1/state.pb.go: proto/oidc/v1/state.proto buf.yaml buf.gen.yaml .bin/buf .bin/goimports
.bin/buf generate
.bin/goimports -w gen/

.PHONY: install
install:
go install -tags sqlite .
Expand Down Expand Up @@ -162,11 +176,12 @@ authors: # updates the AUTHORS file

# Formats the code
.PHONY: format
format: .bin/goimports .bin/ory node_modules
.bin/ory dev headers copyright --exclude=internal/httpclient --exclude=internal/client-go --exclude test/e2e/proxy/node_modules --exclude test/e2e/node_modules --exclude node_modules
format: .bin/goimports .bin/ory node_modules .bin/buf
.bin/ory dev headers copyright --exclude=gen --exclude=internal/httpclient --exclude=internal/client-go --exclude test/e2e/proxy/node_modules --exclude test/e2e/node_modules --exclude node_modules
goimports -w -local github.com/ory .
npm exec -- prettier --write 'test/e2e/**/*{.ts,.js}'
npm exec -- prettier --write '.github'
.bin/buf format --write

# Build local docker image
.PHONY: docker
Expand Down
12 changes: 12 additions & 0 deletions buf.gen.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
version: v2
managed:
enabled: true
override:
- file_option: go_package_prefix
value: github.com/ory/kratos
plugins:
- remote: buf.build/protocolbuffers/go
out: gen
opt: paths=source_relative
inputs:
- directory: proto
9 changes: 9 additions & 0 deletions buf.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
version: v2
modules:
- path: proto
lint:
use:
- DEFAULT
breaking:
use:
- FILE
6 changes: 6 additions & 0 deletions cipher/chacha20.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"crypto/rand"
"encoding/hex"
"io"
"math"

"github.com/pkg/errors"
"golang.org/x/crypto/chacha20poly1305"
Expand Down Expand Up @@ -43,6 +44,11 @@ func (c *XChaCha20Poly1305) Encrypt(ctx context.Context, message []byte) (string
return "", herodot.ErrInternalServerError.WithWrap(err).WithReason("Unable to generate key")
}

// Make sure the size calculation does not overflow.
if len(message) > math.MaxInt-aead.NonceSize()-aead.Overhead() {
return "", errors.WithStack(herodot.ErrInternalServerError.WithReason("plaintext too large"))
}

nonce := make([]byte, aead.NonceSize(), aead.NonceSize()+len(message)+aead.Overhead())
_, err = io.ReadFull(rand.Reader, nonce)
if err != nil {
Expand Down
9 changes: 5 additions & 4 deletions cmd/identities/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package identities_test

import (
"context"
"encoding/hex"
"encoding/json"
"testing"

Expand Down Expand Up @@ -63,10 +62,12 @@ func TestGetCmd(t *testing.T) {
return out
}
transform := func(token string) string {
if !encrypt {
return token
if encrypt {
s, err := reg.Cipher(context.Background()).Encrypt(context.Background(), []byte(token))
require.NoError(t, err)
return s
}
return hex.EncodeToString([]byte(token))
return token
}
return identity.Credentials{
Type: identity.CredentialsTypeOIDC,
Expand Down
2 changes: 1 addition & 1 deletion cmd/identities/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"github.com/ory/kratos/internal/testhelpers"
)

func setup(t *testing.T, newCmd func() *cobra.Command) (driver.Registry, *cmdx.CommandExecuter) {
func setup(t *testing.T, newCmd func() *cobra.Command) (*driver.RegistryDefault, *cmdx.CommandExecuter) {
conf, reg := internal.NewFastRegistryWithMocks(t)
_, admin := testhelpers.NewKratosServerWithCSRF(t, reg)
testhelpers.SetDefaultIdentitySchema(conf, "file://./stubs/identity.schema.json")
Expand Down
183 changes: 183 additions & 0 deletions gen/oidc/v1/state.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ require (
golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240528184218-531527333157 // indirect
google.golang.org/protobuf v1.34.1 // indirect
google.golang.org/protobuf v1.34.2
gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1315,8 +1315,8 @@ google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlba
google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg=
google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg=
google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw=
gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc=
gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc h1:2gGKlE2+asNV9m7xrywl36YYNnBG5ZQ0r/BOOxqPpmk=
Expand Down
13 changes: 9 additions & 4 deletions internal/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
confighelpers "github.com/ory/kratos/driver/config/testhelpers"

"github.com/ory/x/contextx"
"github.com/ory/x/randx"

"github.com/sirupsen/logrus"

Expand Down Expand Up @@ -86,10 +87,14 @@ func NewFastRegistryWithMocks(t *testing.T, opts ...configx.OptionModifier) (*co
// NewRegistryDefaultWithDSN returns a more standard registry without mocks. Good for e2e and advanced integration testing!
func NewRegistryDefaultWithDSN(t testing.TB, dsn string, opts ...configx.OptionModifier) (*config.Config, *driver.RegistryDefault) {
ctx := context.Background()
c := NewConfigurationWithDefaults(t, append(opts, configx.WithValues(map[string]interface{}{
config.ViperKeyDSN: stringsx.Coalesce(dsn, dbal.NewSQLiteTestDatabase(t)+"&lock=false&max_conns=1"),
"dev": true,
}))...)
c := NewConfigurationWithDefaults(t, append([]configx.OptionModifier{configx.WithValues(map[string]interface{}{
config.ViperKeyDSN: stringsx.Coalesce(dsn, dbal.NewSQLiteTestDatabase(t)+"&lock=false&max_conns=1"),
"dev": true,
config.ViperKeySecretsCipher: []string{randx.MustString(32, randx.AlphaNum)},
config.ViperKeySecretsCookie: []string{randx.MustString(32, randx.AlphaNum)},
config.ViperKeySecretsDefault: []string{randx.MustString(32, randx.AlphaNum)},
config.ViperKeyCipherAlgorithm: "xchacha20-poly1305",
})}, opts...)...)
reg, err := driver.NewRegistryFromDSN(ctx, c, logrusx.New("", "", logrusx.ForceLevel(logrus.ErrorLevel)))
require.NoError(t, err)
pool := jsonnetsecure.NewProcessPool(runtime.GOMAXPROCS(0))
Expand Down
10 changes: 10 additions & 0 deletions proto/oidc/v1/state.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
syntax = "proto3";

package oidc.v1;

message State {
bytes flow_id = 1;
bytes session_token_exchange_code_sha512 = 2;
string provider_id = 3;
string pkce_verifier = 4;
}
Loading

0 comments on commit 6c40e7f

Please sign in to comment.