PKCE and Adding private key JWT support for OIDC #22732

panman90 · 2025-09-09T09:18:09Z

Description

Adding support for private key(client assertion) and PKCE for OIDC

Testing & Reproduction steps

Tested with Keyclock and Auth0 with private key JWT instead of client secret

Links

https://auth0.com/docs/authenticate/enterprise-connections/private-key-jwt-client-auth

PR Checklist

updated test coverage
external facing docs updated
appropriate backport labels added
not a security concern

PCI review checklist

I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.

Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

suresh-hashicorp

UI changes are good.

sreeram77 · 2025-09-11T06:10:57Z

.changelog/22732.txt

@@ -0,0 +1,3 @@
+```release-note:feature
+oidc: Added support for client authentication using JWT assertion and PKCE. default PKCE is disable


Suggested change

oidc: Added support for client authentication using JWT assertion and PKCE. default PKCE is disable

auth: add client authentication using JWT assertion and PKCE. default PKCE is disabled.

updated message, mentioning OIDC as it only cover that auth flow

internal/go-sso/oidcauth/auth.go

sreeram77 · 2025-09-11T10:35:08Z

internal/go-sso/oidcauth/jwt.go

 }
+
+func (a *Authenticator) verifyOIDCToken(ctx context.Context, rawToken string) (map[string]interface{}, error) {
+	allClaims := make(map[string]interface{})


Please replace interface{} with any

sreeram77 · 2025-09-11T10:35:36Z

internal/go-sso/oidcauth/oidc.go

 	"time"

-	"github.com/coreos/go-oidc/v3/oidc"
+	// HashiCorp CAP (Cloud Authentication Primitives) library for OIDC flows


This comment is unnecessary

internal/go-sso/oidcauth/oidc.go

…ests" This reverts commit 3842c91.

dduzgun-security

Thanks a lot for working on this.
Here are a few security points which could be reviewed:

Can we have the PKCE check enabled explicitly by default?
Can we add the kid header to identify which key was used to sign the JWT?
If the kid not set, can we include the same x5t header logic as Nomad (ref)? By defaulting the checksum to x5t#S256
Can we default to Elliptic Curve instead of RS256? It's more performant. We can also include RS256 in the list of allowed algs
Can we add some unit tests for the alg portion please? There are also various tests we can include which were captured in Nomads implementation (ref)

panman90 · 2025-09-14T11:17:38Z

@dduzgun-security - Thanks for your feedback

Will make necessary change to enable PKCE by default.
Regarding kid and EC algorithm, we have kept it out of scope for this release.

dduzgun-security · 2025-09-15T19:04:49Z

@mansi991999 thanks for the reply and looking at it. I forgot to mention on the initial comment but could we also include some unit tests for jwt.go and auth.go?

I've added a comment in the internal RFC to understand more why the kid was moved to out of scope.

sreeram77 · 2025-09-16T05:51:16Z

internal/go-sso/oidcauth/auth.go

+			return nil, fmt.Errorf("error creating provider config: %v", err)
+		}
+
+		provider, error := capOidc.NewProvider(providerConfig)


Please don't use 'error' for an error since this is a built in keyword. You can reuse err.

sreeram77 · 2025-09-16T05:52:00Z

internal/go-sso/oidcauth/jwt.go

 	return nil, errors.New("data does not contain any valid RSA, ECDSA, or ED25519 public keys")
 }
+
+func (a *Authenticator) verifyOIDCToken(ctx context.Context, rawToken string) (map[string]interface{}, error) {


Please replace interface{} with any.

sreeram77 · 2025-09-16T05:53:14Z

internal/go-sso/oidcauth/oidc.go


-	"github.com/coreos/go-oidc/v3/oidc"
+	"github.com/hashicorp/cap/oidc"
+	cass "github.com/hashicorp/cap/oidc/clientassertion"


Why is this alias required?

to make references more concise

sreeram77 · 2025-09-16T05:53:39Z

internal/go-sso/oidcauth/oidc.go

+	authURL, err := provider.AuthURL(ctx, request)
 	if err != nil {
-		return "", fmt.Errorf("error generating OAuth state: %v", err)
+		return "", fmt.Errorf("Error while generating AuthURL %q", err)


error string should start with a lower case

panman90 · 2025-09-16T08:44:05Z

@dduzgun-security
Added comment on RFC.
Regarding JWT, tests are already there. Added for Auth

* Adding private key JWT support for OIDC

Adding private key JWT support for OIDC

084acad

github-actions bot added theme/api Relating to the HTTP API interface theme/ui Anything related to the UI pr/dependencies PR specifically updates dependencies of project labels Sep 9, 2025

panman90 added 2 commits September 9, 2025 14:49

Adding label for UI component

17f9243

Merge branch 'main' into mansi/jwt_int

4a44ce6

vercel bot deployed to Preview – consul September 9, 2025 09:31 View deployment

panman90 added 4 commits September 10, 2025 10:29

Adding change log

8e79db5

dep upgrade

849e854

removing logs

f121ade

removing log

0ba0338

panman90 marked this pull request as ready for review September 10, 2025 05:41

panman90 requested a review from a team as a code owner September 10, 2025 05:41

removing dep

c3dfe2b

panman90 added the type/enhancement Proposed improvement or new feature label Sep 10, 2025

lint fix

ea7598b

panman90 self-assigned this Sep 10, 2025

panman90 changed the title ~~Adding private key JWT support for OIDC~~ PKCE and Adding private key JWT support for OIDC Sep 10, 2025

updated changelog

6b58663

panman90 added the pr/no-backport label Sep 10, 2025

suresh-hashicorp previously approved these changes Sep 10, 2025

View reviewed changes

sreeram77 requested changes Sep 11, 2025

View reviewed changes

Review comments

d70abf2

panman90 dismissed suresh-hashicorp’s stale review via d70abf2 September 12, 2025 08:30

panman90 added 3 commits September 12, 2025 14:08

review comments

b012aca

to fix exhaust resources and context cancellation errors in tests

3842c91

Revert "to fix exhaust resources and context cancellation errors in t…

efd69f8

…ests" This reverts commit 3842c91.

dduzgun-security requested changes Sep 12, 2025

View reviewed changes

enabling pkce by default

94d7031

mrgupta7 previously approved these changes Sep 16, 2025

View reviewed changes

sreeram77 requested changes Sep 16, 2025

View reviewed changes

Adding tests for auth

55d9fe9

panman90 dismissed mrgupta7’s stale review via 55d9fe9 September 16, 2025 07:05

panman90 added 2 commits September 16, 2025 13:00

review comments

a8efff8

code gen fix

e4df623

panman90 requested a review from sreeram77 September 16, 2025 11:23

removing unnecessary comments

4fdc5ab

dduzgun-security approved these changes Sep 16, 2025

View reviewed changes

Merge branch 'main' into mansi/jwt_int

4a35dee

sreeram77 approved these changes Sep 17, 2025

View reviewed changes

panman90 enabled auto-merge (squash) September 17, 2025 09:34

vercel bot deployed to Preview – consul September 17, 2025 09:35 View deployment

panman90 merged commit 5c4ec6c into main Sep 17, 2025
282 of 290 checks passed

sanikachavan5 pushed a commit that referenced this pull request Sep 23, 2025

PKCE and Adding private key JWT support for OIDC (#22732)

fe6149f

* Adding private key JWT support for OIDC

		@@ -0,0 +1,3 @@
		```release-note:feature
		oidc: Added support for client authentication using JWT assertion and PKCE. default PKCE is disable

	oidc: Added support for client authentication using JWT assertion and PKCE. default PKCE is disable
	auth: add client authentication using JWT assertion and PKCE. default PKCE is disabled.

PKCE and Adding private key JWT support for OIDC #22732

PKCE and Adding private key JWT support for OIDC #22732

Uh oh!

Conversation

panman90 commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing & Reproduction steps

Links

PR Checklist

PCI review checklist

Uh oh!

suresh-hashicorp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dduzgun-security left a comment

Choose a reason for hiding this comment

Uh oh!

panman90 commented Sep 14, 2025

Uh oh!

dduzgun-security commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

panman90 commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

panman90 commented Sep 9, 2025 •

edited

Loading

dduzgun-security commented Sep 15, 2025 •

edited

Loading

panman90 commented Sep 16, 2025 •

edited

Loading