-
Notifications
You must be signed in to change notification settings - Fork 4.5k
PKCE and Adding private key JWT support for OIDC #22732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
084acad
Adding private key JWT support for OIDC
panman90 17f9243
Adding label for UI component
panman90 4a44ce6
Merge branch 'main' into mansi/jwt_int
panman90 8e79db5
Adding change log
panman90 849e854
dep upgrade
panman90 f121ade
removing logs
panman90 0ba0338
removing log
panman90 c3dfe2b
removing dep
panman90 ea7598b
lint fix
panman90 6b58663
updated changelog
panman90 d70abf2
Review comments
panman90 b012aca
review comments
panman90 3842c91
to fix exhaust resources and context cancellation errors in tests
panman90 efd69f8
Revert "to fix exhaust resources and context cancellation errors in …
panman90 94d7031
enabling pkce by default
panman90 55d9fe9
Adding tests for auth
panman90 a8efff8
review comments
panman90 e4df623
code gen fix
panman90 4fdc5ab
removing unnecessary comments
panman90 4a35dee
Merge branch 'main' into mansi/jwt_int
panman90 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ```release-note:feature | ||
| oidc: add client authentication using JWT assertion and PKCE. default PKCE is enabled. | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
|
|
||
| package oidcauth | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/hashicorp/consul/internal/go-sso/oidcauth/oidcauthtest" | ||
| "github.com/hashicorp/go-hclog" | ||
| "github.com/patrickmn/go-cache" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func mockConfig(typ string, t *testing.T) *Config { | ||
| t.Helper() | ||
|
|
||
| srv := oidcauthtest.Start(t) | ||
| srv.SetClientCreds("abc", "def") | ||
| cfg := &Config{ | ||
| Type: typ, | ||
| } | ||
| if typ == TypeJWT { | ||
| cfg.JWKSURL = srv.Addr() + "/certs" | ||
| cfg.JWKSCACert = srv.CACert() | ||
| } | ||
| if typ == TypeOIDC { | ||
| cfg.OIDCDiscoveryURL = srv.Addr() | ||
| cfg.OIDCClientID = "abc" | ||
| cfg.OIDCClientSecret = "def" | ||
| cfg.AllowedRedirectURIs = []string{"https://redirect"} | ||
| cfg.OIDCDiscoveryCACert = srv.CACert() | ||
| } | ||
| return cfg | ||
| } | ||
|
|
||
| const testPublicKeyPEM = `-----BEGIN PUBLIC KEY----- | ||
| MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEEVs/o5+uQbTjL3chynL4wXgUg2R9 | ||
| q9UU8I5mEovUf86QZ7kOBIjJwqnzD1omageEHWwHdBO6B+dFabmdT9POxg== | ||
| -----END PUBLIC KEY-----` | ||
|
|
||
| func TestAuthenticator_JWTGroup(t *testing.T) { | ||
| t.Run("JWTType static keys", func(t *testing.T) { | ||
| cfg := mockConfig(TypeJWT, t) | ||
| cfg.JWTValidationPubKeys = []string{testPublicKeyPEM} | ||
| cfg.JWKSURL = "" | ||
| cfg.JWKSCACert = "" | ||
| logger := hclog.NewNullLogger() | ||
| auth, err := New(cfg, logger) | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, auth) | ||
| assert.Equal(t, cfg, auth.config) | ||
| assert.NotEmpty(t, auth.parsedJWTPubKeys) | ||
| }) | ||
|
|
||
| t.Run("JWTType JWKS", func(t *testing.T) { | ||
| cfg := mockConfig(TypeJWT, t) | ||
| logger := hclog.NewNullLogger() | ||
| auth, err := New(cfg, logger) | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, auth) | ||
| assert.Equal(t, cfg, auth.config) | ||
| }) | ||
|
|
||
| t.Run("JWTType failure", func(t *testing.T) { | ||
| cfg := mockConfig(TypeJWT, t) | ||
| cfg.OIDCClientID = "abc" | ||
| logger := hclog.NewNullLogger() | ||
| _, err := New(cfg, logger) | ||
| assert.Error(t, err) | ||
| requireErrorContains(t, err, "'OIDCClientID' must not be set for type") | ||
| }) | ||
|
|
||
| t.Run("Stop", func(t *testing.T) { | ||
| cfg := mockConfig(TypeJWT, t) | ||
| logger := hclog.NewNullLogger() | ||
| auth, err := New(cfg, logger) | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, auth.backgroundCtxCancel) | ||
| auth.Stop() | ||
| assert.Nil(t, auth.backgroundCtxCancel) | ||
| }) | ||
|
|
||
| t.Run("BackgroundContextCancel", func(t *testing.T) { | ||
| cfg := mockConfig(TypeJWT, t) | ||
| logger := hclog.NewNullLogger() | ||
| auth, err := New(cfg, logger) | ||
| assert.NoError(t, err) | ||
| done := make(chan struct{}) | ||
| go func() { | ||
| <-auth.backgroundCtx.Done() | ||
| close(done) | ||
| }() | ||
| auth.Stop() | ||
| select { | ||
| case <-done: | ||
| case <-time.After(time.Second): | ||
| t.Fatal("backgroundCtx was not cancelled") | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| func TestAuthenticator_OIDCGroup(t *testing.T) { | ||
| t.Run("OIDCType", func(t *testing.T) { | ||
| cfg := mockConfig(TypeOIDC, t) | ||
| logger := hclog.NewNullLogger() | ||
| auth, err := New(cfg, logger) | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, auth.capProvider) | ||
| assert.NotNil(t, auth.oidcStates) | ||
| }) | ||
|
|
||
| t.Run("OIDCDiscovery", func(t *testing.T) { | ||
| srv := oidcauthtest.Start(t) | ||
| srv.SetClientCreds("abc", "def") | ||
| cfg := mockConfig(TypeJWT, t) | ||
| cfg.JWKSURL = "" | ||
| cfg.JWKSCACert = "" | ||
| cfg.OIDCDiscoveryURL = srv.Addr() | ||
| cfg.OIDCDiscoveryCACert = srv.CACert() | ||
|
|
||
| logger := hclog.NewNullLogger() | ||
| auth, err := New(cfg, logger) | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, auth) | ||
| assert.NotNil(t, auth.provider) | ||
| assert.NotNil(t, auth.httpClient) | ||
| }) | ||
|
|
||
| t.Run("OIDCStatesCache", func(t *testing.T) { | ||
| cfg := mockConfig(TypeOIDC, t) | ||
| logger := hclog.NewNullLogger() | ||
| auth, err := New(cfg, logger) | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, auth.oidcStates) | ||
| auth.oidcStates.Set("state", "value", cache.DefaultExpiration) | ||
| val, found := auth.oidcStates.Get("state") | ||
| assert.True(t, found) | ||
| assert.Equal(t, "value", val) | ||
| }) | ||
| } | ||
|
|
||
| func TestAuthenticator_OIDCFlow_Failure(t *testing.T) { | ||
| t.Run("InvalidCACert", func(t *testing.T) { | ||
| cfg := mockConfig(TypeOIDC, t) | ||
| cfg.OIDCDiscoveryCACert = "invalid cert data" | ||
|
|
||
| logger := hclog.NewNullLogger() | ||
| _, err := New(cfg, logger) | ||
|
|
||
| assert.Error(t, err) | ||
| requireErrorContains(t, err, "could not parse CA PEM value successfully") | ||
| }) | ||
|
|
||
| t.Run("ProviderConfig_error", func(t *testing.T) { | ||
| cfg := mockConfig(TypeOIDC, t) | ||
| cfg.OIDCDiscoveryURL = "::invalid-url::" | ||
|
|
||
| logger := hclog.NewNullLogger() | ||
| _, err := New(cfg, logger) | ||
|
|
||
| assert.Error(t, err) | ||
| requireErrorContains(t, err, "error checking OIDCDiscoveryURL") | ||
| }) | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.