Skip to content

Commit fe6149f

Browse files
panman90sanikachavan5
authored andcommitted
PKCE and Adding private key JWT support for OIDC (#22732)
* Adding private key JWT support for OIDC
1 parent ada5f88 commit fe6149f

File tree

14 files changed

+713
-114
lines changed

14 files changed

+713
-114
lines changed

.changelog/22732.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:feature
2+
oidc: add client authentication using JWT assertion and PKCE. default PKCE is enabled.
3+
```

api/acl.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -483,12 +483,14 @@ type OIDCAuthMethodConfig struct {
483483
OIDCDiscoveryURL string `json:",omitempty"`
484484
OIDCDiscoveryCACert string `json:",omitempty"`
485485
// just for type=oidc
486-
OIDCClientID string `json:",omitempty"`
487-
OIDCClientSecret string `json:",omitempty"`
488-
OIDCScopes []string `json:",omitempty"`
489-
OIDCACRValues []string `json:",omitempty"`
490-
AllowedRedirectURIs []string `json:",omitempty"`
491-
VerboseOIDCLogging bool `json:",omitempty"`
486+
OIDCClientID string `json:",omitempty"`
487+
OIDCClientSecret string `json:",omitempty"`
488+
OIDCClientAssertion *OIDCClientAssertion `json:",omitempty"`
489+
OIDCClientUsePKCE *bool `json:",omitempty"`
490+
OIDCScopes []string `json:",omitempty"`
491+
OIDCACRValues []string `json:",omitempty"`
492+
AllowedRedirectURIs []string `json:",omitempty"`
493+
VerboseOIDCLogging bool `json:",omitempty"`
492494
// just for type=jwt
493495
JWKSURL string `json:",omitempty"`
494496
JWKSCACert string `json:",omitempty"`
@@ -513,6 +515,8 @@ func (c *OIDCAuthMethodConfig) RenderToConfig() map[string]interface{} {
513515
// just for type=oidc
514516
"OIDCClientID": c.OIDCClientID,
515517
"OIDCClientSecret": c.OIDCClientSecret,
518+
"OIDCClientAssertion": c.OIDCClientAssertion,
519+
"OIDCClientUsePKCE": c.OIDCClientUsePKCE,
516520
"OIDCScopes": c.OIDCScopes,
517521
"OIDCACRValues": c.OIDCACRValues,
518522
"AllowedRedirectURIs": c.AllowedRedirectURIs,
@@ -528,6 +532,16 @@ func (c *OIDCAuthMethodConfig) RenderToConfig() map[string]interface{} {
528532
}
529533
}
530534

535+
type OIDCClientAssertion struct {
536+
Audience []string
537+
PrivateKey *OIDCClientAssertionKey
538+
KeyAlgorithm string
539+
}
540+
541+
type OIDCClientAssertionKey struct {
542+
PemKey string
543+
}
544+
531545
type ACLLoginParams struct {
532546
AuthMethod string
533547
BearerToken string

go.mod

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ require (
2222
github.com/armon/go-metrics v0.4.1
2323
github.com/armon/go-radix v1.0.0
2424
github.com/aws/aws-sdk-go v1.55.7
25-
github.com/coreos/go-oidc/v3 v3.9.0
25+
github.com/coreos/go-oidc/v3 v3.11.0
2626
github.com/deckarep/golang-set/v2 v2.3.1
2727
github.com/docker/go-connections v0.4.0
2828
github.com/envoyproxy/go-control-plane v0.13.4
@@ -37,12 +37,14 @@ require (
3737
github.com/go-openapi/runtime v0.26.2
3838
github.com/go-openapi/strfmt v0.23.0
3939
github.com/go-viper/mapstructure/v2 v2.4.0
40+
github.com/golang-jwt/jwt/v5 v5.2.2
4041
github.com/google/go-cmp v0.7.0
4142
github.com/google/gofuzz v1.2.0
4243
github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1
4344
github.com/google/tcpproxy v0.0.0-20180808230851-dfa16c61dad2
4445
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
4546
github.com/hashi-derek/grpc-proxy v0.0.0-20231207191910-191266484d75
47+
github.com/hashicorp/cap v0.10.0
4648
github.com/hashicorp/consul-awsauth v0.0.0-20250825122907-9e35fe9ded3a
4749
github.com/hashicorp/consul-net-rpc v0.0.0-20221205195236-156cfab66a69
4850
github.com/hashicorp/consul/api v1.31.2
@@ -191,6 +193,7 @@ require (
191193
github.com/emicklei/go-restful/v3 v3.10.1 // indirect
192194
github.com/envoyproxy/protoc-gen-validate v1.2.1 // indirect
193195
github.com/felixge/httpsnoop v1.0.4 // indirect
196+
github.com/go-jose/go-jose/v4 v4.1.1 // indirect
194197
github.com/go-logr/logr v1.4.3 // indirect
195198
github.com/go-logr/stdr v1.2.2 // indirect
196199
github.com/go-ole/go-ole v1.2.6 // indirect

go.sum

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,8 @@ github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc
197197
github.com/coreos/etcd v3.3.27+incompatible h1:QIudLb9KeBsE5zyYxd1mjzRSkzLg9Wf9QlRwFgd6oTA=
198198
github.com/coreos/etcd v3.3.27+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
199199
github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=
200-
github.com/coreos/go-oidc/v3 v3.9.0 h1:0J/ogVOd4y8P0f0xUh8l9t07xRP/d8tccvjHl2dcsSo=
201-
github.com/coreos/go-oidc/v3 v3.9.0/go.mod h1:rTKz2PYwftcrtoCzV5g5kvfJoWcm0Mk8AF8y1iAQro4=
200+
github.com/coreos/go-oidc/v3 v3.11.0 h1:Ia3MxdwpSw702YW0xgfmP1GVCMA9aEFWu12XUZ3/OtI=
201+
github.com/coreos/go-oidc/v3 v3.11.0/go.mod h1:gE3LgjOgFoHi9a4ce4/tJczr0Ai2/BoDhf0r5lltWI0=
202202
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
203203
github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
204204
github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf h1:iW4rZ826su+pqaw19uhpSCzhj44qo35pNgKFGqzDKkU=
@@ -279,6 +279,8 @@ github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2
279279
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
280280
github.com/go-jose/go-jose/v3 v3.0.4 h1:Wp5HA7bLQcKnf6YYao/4kpRpVMp/yf6+pJKV8WFSaNY=
281281
github.com/go-jose/go-jose/v3 v3.0.4/go.mod h1:5b+7YgP7ZICgJDBdfjZaIt+H/9L9T/YQrVfLAMboGkQ=
282+
github.com/go-jose/go-jose/v4 v4.1.1 h1:JYhSgy4mXXzAdF3nUx3ygx347LRXJRrpgyU3adRmkAI=
283+
github.com/go-jose/go-jose/v4 v4.1.1/go.mod h1:BdsZGqgdO3b6tTc6LSE56wcDbMMLuPsw5d4ZD5f94kA=
282284
github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
283285
github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
284286
github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY=
@@ -331,6 +333,8 @@ github.com/golang-jwt/jwt/v4 v4.0.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzw
331333
github.com/golang-jwt/jwt/v4 v4.2.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg=
332334
github.com/golang-jwt/jwt/v4 v4.5.2 h1:YtQM7lnr8iZ+j5q71MGKkNw9Mn7AjHM68uc9g5fXeUI=
333335
github.com/golang-jwt/jwt/v4 v4.5.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
336+
github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8=
337+
github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
334338
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
335339
github.com/golang/glog v1.2.5 h1:DrW6hGnjIhtvhOIiAKT6Psh/Kd/ldepEa81DKeiRJ5I=
336340
github.com/golang/glog v1.2.5/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w=
@@ -445,6 +449,8 @@ github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0 h1:YBftPWNWd4WwGqtY2yeZL2ef8rH
445449
github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0/go.mod h1:YN5jB8ie0yfIUg6VvR9Kz84aCaG7AsGZnLjhHbUqwPg=
446450
github.com/hashi-derek/grpc-proxy v0.0.0-20231207191910-191266484d75 h1:V5Uqf7VoWMd6UhNf/5EMA8LMPUm95GYvk2YF5SzT24o=
447451
github.com/hashi-derek/grpc-proxy v0.0.0-20231207191910-191266484d75/go.mod h1:5eEnHfK72jOkp4gC1dI/Q/E9MFNOM/ewE/vql5ijV3g=
452+
github.com/hashicorp/cap v0.10.0 h1:OJM3JQTwVO1DigRIPNTxM387oqXlokKhttZHotU0b1s=
453+
github.com/hashicorp/cap v0.10.0/go.mod h1:HKbv27kfps+wONFNyNTHpAQmU/DCjjDuB5HF6mFsqPQ=
448454
github.com/hashicorp/consul-awsauth v0.0.0-20250825122907-9e35fe9ded3a h1:Qd0N8lIr1QP/d7FYxseYjRLUtJp2+2R8k+mjiC2rmiY=
449455
github.com/hashicorp/consul-awsauth v0.0.0-20250825122907-9e35fe9ded3a/go.mod h1:++exZ1sI8JLIv4QvzGvTjZdf1eZARoZcaNEjNT9SZYA=
450456
github.com/hashicorp/consul-net-rpc v0.0.0-20221205195236-156cfab66a69 h1:wzWurXrxfSyG1PHskIZlfuXlTSCj1Tsyatp9DtaasuY=

internal/go-sso/oidcauth/auth.go

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"sync"
1818

1919
"github.com/coreos/go-oidc/v3/oidc"
20+
capOidc "github.com/hashicorp/cap/oidc"
2021
"github.com/hashicorp/go-hclog"
2122
"github.com/patrickmn/go-cache"
2223
)
@@ -41,8 +42,14 @@ type Authenticator struct {
4142

4243
// parsedJWTPubKeys is the parsed form of config.JWTValidationPubKeys
4344
parsedJWTPubKeys []interface{}
44-
provider *oidc.Provider
45-
keySet oidc.KeySet
45+
46+
// provider is the coreos/go-oidc provider used for JWT validation
47+
provider *oidc.Provider
48+
49+
// capProvider is the HashiCorp CAP library provider used for OIDC flows
50+
// with support for private key JWT client authentication
51+
capProvider *capOidc.Provider
52+
keySet oidc.KeySet
4653

4754
// httpClient should be configured with all relevant root CA certs and be
4855
// reused for all OIDC or JWKS operations. This will be nil for the static
@@ -86,13 +93,43 @@ func New(c *Config, logger hclog.Logger) (*Authenticator, error) {
8693
}
8794
a.backgroundCtx, a.backgroundCtxCancel = context.WithCancel(context.Background())
8895

96+
var err error
8997
if c.Type == TypeOIDC {
9098
a.oidcStates = cache.New(oidcStateTimeout, oidcStateCleanupInterval)
9199
}
92100

93-
var err error
94101
switch c.authType() {
95-
case authOIDCDiscovery, authOIDCFlow:
102+
case authOIDCFlow:
103+
var supported []capOidc.Alg
104+
if len(a.config.JWTSupportedAlgs) == 0 {
105+
// Default to RS256 if nothing is specified.
106+
supported = []capOidc.Alg{capOidc.RS256}
107+
} else {
108+
for _, alg := range a.config.JWTSupportedAlgs {
109+
supported = append(supported, capOidc.Alg(alg))
110+
}
111+
}
112+
// Use CAP's OIDC provider to leverage its built-in support for
113+
// both standard client secret and JWT assertion authentication methods
114+
providerConfig, err := capOidc.NewConfig(
115+
a.config.OIDCDiscoveryURL,
116+
a.config.OIDCClientID,
117+
capOidc.ClientSecret(a.config.OIDCClientSecret),
118+
supported,
119+
a.config.AllowedRedirectURIs,
120+
capOidc.WithAudiences(a.config.BoundAudiences...),
121+
capOidc.WithProviderCA(a.config.OIDCDiscoveryCACert),
122+
)
123+
if err != nil {
124+
return nil, fmt.Errorf("error creating provider config: %v", err)
125+
}
126+
127+
provider, err := capOidc.NewProvider(providerConfig)
128+
if err != nil {
129+
return nil, fmt.Errorf("error creating provider: %v", err)
130+
}
131+
a.capProvider = provider
132+
case authOIDCDiscovery:
96133
a.httpClient, err = createHTTPClient(a.config.OIDCDiscoveryCACert)
97134
if err != nil {
98135
return nil, fmt.Errorf("error parsing OIDCDiscoveryCACert: %v", err)
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: BUSL-1.1
3+
4+
package oidcauth
5+
6+
import (
7+
"testing"
8+
"time"
9+
10+
"github.com/hashicorp/consul/internal/go-sso/oidcauth/oidcauthtest"
11+
"github.com/hashicorp/go-hclog"
12+
"github.com/patrickmn/go-cache"
13+
"github.com/stretchr/testify/assert"
14+
)
15+
16+
func mockConfig(typ string, t *testing.T) *Config {
17+
t.Helper()
18+
19+
srv := oidcauthtest.Start(t)
20+
srv.SetClientCreds("abc", "def")
21+
cfg := &Config{
22+
Type: typ,
23+
}
24+
if typ == TypeJWT {
25+
cfg.JWKSURL = srv.Addr() + "/certs"
26+
cfg.JWKSCACert = srv.CACert()
27+
}
28+
if typ == TypeOIDC {
29+
cfg.OIDCDiscoveryURL = srv.Addr()
30+
cfg.OIDCClientID = "abc"
31+
cfg.OIDCClientSecret = "def"
32+
cfg.AllowedRedirectURIs = []string{"https://redirect"}
33+
cfg.OIDCDiscoveryCACert = srv.CACert()
34+
}
35+
return cfg
36+
}
37+
38+
const testPublicKeyPEM = `-----BEGIN PUBLIC KEY-----
39+
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEEVs/o5+uQbTjL3chynL4wXgUg2R9
40+
q9UU8I5mEovUf86QZ7kOBIjJwqnzD1omageEHWwHdBO6B+dFabmdT9POxg==
41+
-----END PUBLIC KEY-----`
42+
43+
func TestAuthenticator_JWTGroup(t *testing.T) {
44+
t.Run("JWTType static keys", func(t *testing.T) {
45+
cfg := mockConfig(TypeJWT, t)
46+
cfg.JWTValidationPubKeys = []string{testPublicKeyPEM}
47+
cfg.JWKSURL = ""
48+
cfg.JWKSCACert = ""
49+
logger := hclog.NewNullLogger()
50+
auth, err := New(cfg, logger)
51+
assert.NoError(t, err)
52+
assert.NotNil(t, auth)
53+
assert.Equal(t, cfg, auth.config)
54+
assert.NotEmpty(t, auth.parsedJWTPubKeys)
55+
})
56+
57+
t.Run("JWTType JWKS", func(t *testing.T) {
58+
cfg := mockConfig(TypeJWT, t)
59+
logger := hclog.NewNullLogger()
60+
auth, err := New(cfg, logger)
61+
assert.NoError(t, err)
62+
assert.NotNil(t, auth)
63+
assert.Equal(t, cfg, auth.config)
64+
})
65+
66+
t.Run("JWTType failure", func(t *testing.T) {
67+
cfg := mockConfig(TypeJWT, t)
68+
cfg.OIDCClientID = "abc"
69+
logger := hclog.NewNullLogger()
70+
_, err := New(cfg, logger)
71+
assert.Error(t, err)
72+
requireErrorContains(t, err, "'OIDCClientID' must not be set for type")
73+
})
74+
75+
t.Run("Stop", func(t *testing.T) {
76+
cfg := mockConfig(TypeJWT, t)
77+
logger := hclog.NewNullLogger()
78+
auth, err := New(cfg, logger)
79+
assert.NoError(t, err)
80+
assert.NotNil(t, auth.backgroundCtxCancel)
81+
auth.Stop()
82+
assert.Nil(t, auth.backgroundCtxCancel)
83+
})
84+
85+
t.Run("BackgroundContextCancel", func(t *testing.T) {
86+
cfg := mockConfig(TypeJWT, t)
87+
logger := hclog.NewNullLogger()
88+
auth, err := New(cfg, logger)
89+
assert.NoError(t, err)
90+
done := make(chan struct{})
91+
go func() {
92+
<-auth.backgroundCtx.Done()
93+
close(done)
94+
}()
95+
auth.Stop()
96+
select {
97+
case <-done:
98+
case <-time.After(time.Second):
99+
t.Fatal("backgroundCtx was not cancelled")
100+
}
101+
})
102+
}
103+
104+
func TestAuthenticator_OIDCGroup(t *testing.T) {
105+
t.Run("OIDCType", func(t *testing.T) {
106+
cfg := mockConfig(TypeOIDC, t)
107+
logger := hclog.NewNullLogger()
108+
auth, err := New(cfg, logger)
109+
assert.NoError(t, err)
110+
assert.NotNil(t, auth.capProvider)
111+
assert.NotNil(t, auth.oidcStates)
112+
})
113+
114+
t.Run("OIDCDiscovery", func(t *testing.T) {
115+
srv := oidcauthtest.Start(t)
116+
srv.SetClientCreds("abc", "def")
117+
cfg := mockConfig(TypeJWT, t)
118+
cfg.JWKSURL = ""
119+
cfg.JWKSCACert = ""
120+
cfg.OIDCDiscoveryURL = srv.Addr()
121+
cfg.OIDCDiscoveryCACert = srv.CACert()
122+
123+
logger := hclog.NewNullLogger()
124+
auth, err := New(cfg, logger)
125+
assert.NoError(t, err)
126+
assert.NotNil(t, auth)
127+
assert.NotNil(t, auth.provider)
128+
assert.NotNil(t, auth.httpClient)
129+
})
130+
131+
t.Run("OIDCStatesCache", func(t *testing.T) {
132+
cfg := mockConfig(TypeOIDC, t)
133+
logger := hclog.NewNullLogger()
134+
auth, err := New(cfg, logger)
135+
assert.NoError(t, err)
136+
assert.NotNil(t, auth.oidcStates)
137+
auth.oidcStates.Set("state", "value", cache.DefaultExpiration)
138+
val, found := auth.oidcStates.Get("state")
139+
assert.True(t, found)
140+
assert.Equal(t, "value", val)
141+
})
142+
}
143+
144+
func TestAuthenticator_OIDCFlow_Failure(t *testing.T) {
145+
t.Run("InvalidCACert", func(t *testing.T) {
146+
cfg := mockConfig(TypeOIDC, t)
147+
cfg.OIDCDiscoveryCACert = "invalid cert data"
148+
149+
logger := hclog.NewNullLogger()
150+
_, err := New(cfg, logger)
151+
152+
assert.Error(t, err)
153+
requireErrorContains(t, err, "could not parse CA PEM value successfully")
154+
})
155+
156+
t.Run("ProviderConfig_error", func(t *testing.T) {
157+
cfg := mockConfig(TypeOIDC, t)
158+
cfg.OIDCDiscoveryURL = "::invalid-url::"
159+
160+
logger := hclog.NewNullLogger()
161+
_, err := New(cfg, logger)
162+
163+
assert.Error(t, err)
164+
requireErrorContains(t, err, "error checking OIDCDiscoveryURL")
165+
})
166+
}

0 commit comments

Comments
 (0)