feature: migration of openid-client from v4 to v6

[aks96]

Description

This PR upgrades express-openid-connect to use openid-client v6.8.1 while maintaining zero breaking changes for end users. The migration includes comprehensive Node.js 18+ compatibility with Web API polyfills and enhanced CI reliability.

• openid-client: 4.9.1 → 6.8.1
• nock: 11.9.1 → 14.0.10
• undici: 5.29.0 (Web API polyfills)
• Node.js engines: >=14 → >=18.0.0

Changes

• Complete client rewrite - changed to adapt to v6 functional API
• Custom TokenSet implementation - Compatibility layer for removed v6 class
• Web API polyfills - Added fetch/crypto polyfills for Node.js 18 compatibility
• ESM import handling - Dynamic imports with race condition protection
• Enhanced CI reliability - Fixed GitHub Actions issues with dependency management

Backward Compatibility

• Zero breaking changes for library users
• All existing APIs work identically
• Same configuration options and behavior

New Feature & Enhancement

• Enhanced client authentication - Full support for all OAuth 2.1 methods
• PAR support - Pushed Authorization Requests implementation
• Better localhost development - HTTP bypass for local testing
• Improved error handling - v4-compatible error messages
• Performance improvements - Optimized client caching and timeouts

[gyaneshgouraw-okta]

@claude

[github-actions]

Claude finished @gyaneshgouraw-okta's task —— View job

---

Code Review Complete

Tasks

• Read PR diff and understand changes
• Read key modified files for deeper context
• Perform code quality review
• Perform security review
• Perform performance review
• Provide inline feedback for specific issues
• Provide overall assessment

---

Overall Assessment

❌ This PR has critical security issues that must be addressed before merging, particularly around global state management and authentication validation.

Critical Issues:

• Race condition in global fetch handling (lib/client.js:136-137) - Multiple concurrent requests will interfere with each other, potentially causing one client's configuration to be used for another's authentication flow
• Insufficient state and nonce validation (lib/client.js:447-449, 467-469) - Does not properly enforce CSRF protection required by OpenID Connect specification
• JWT parsing without validation (lib/tokenSet.js:16-19) - Decodes JWT claims without signature verification, creating potential security vulnerabilities

Security Concerns:

• Global namespace pollution with crypto/fetch polyfills could conflict with other libraries
• Automatic localhost HTTP bypass without explicit opt-in configuration flag
• Silent error swallowing masks security-relevant events (tampering, replay attacks)
• Test code in production paths increases attack surface

Code Quality Issues:

• Significant code duplication in JWT assertion creation
• Incomplete error handling loses valuable debugging context
• Magic numbers and hardcoded test values in production code
• Missing validation for token expiration values

Recommendations:

1. Fix the critical global state race condition by using per-request fetch configuration instead of mutating globals
2. Strengthen OpenID Connect validation to meet specification requirements for state and nonce
3. Add explicit validation and error logging for security-relevant failures
4. Refactor duplicated authentication code into shared helper functions
5. Remove or isolate test-specific code from production code paths

While the migration from v4 to v6 is architecturally sound and the backward compatibility approach is well-designed, the implementation has security and reliability issues that must be resolved before production use.

---

Job Run | Branch: SDK-6785

[nandan-bhat]

I am not sure if this is correct.
tokenIssuer is fetched from the logoutToken and then used to verify the same logoutToken.

This seems like a security issue because JWT.LogoutToken.verify would always pass in this case as long as the token is signed by a trusted key.

[aks96]

Right issuer should be coming from the oepnid config that will be trusted source of token.

[nandan-bhat]

This default value clockTolerance is no longer used.

Earlier we had this https://github.com/auth0/express-openid-connect/pull/735/files#diff-ca407d959d33ce92a7f7efee354a5ea5e6e61fcd797d32c04205e429a074ed54L115. Now it's removed. And we no longer use this default value.

Does that mean now clockTolerance falls back to 0 ? If the answer is Yes, it could be a breaking change a small clock difference between the app and the AS can result in validation failure.

[aks96]

Thanks for catching this, now i have added the clocktolerence with JWT verification which used to so if our server responds late it can wait till the expiry time.
What do you think of this approach or should we fully remove the clockTolerence but it might impact to authenticaition.

[nandan-bhat]

Do we need client_id here ?

parBody.append('client_id', config.clientID);

[aks96]

Right client_id shouldnt need to send it in body

[nandan-bhat]

Are we mutating the global fetch here ?
What will happen when multiple get() calls are made at the same time ?

[aks96]

Ohk will be changing this to direct reference to global fetch. Multiple calls could have created issues by interfering with each other due to same global variable.

[nandan-bhat]

Should we customFetch here ?
We aren't passing Auth0-Client telemetry and httpAgent data here.

[aks96]

Yes customfetchFn should be used, fetch was by passing it. telemetry data added

[nandan-bhat]

I don't know what is this. What are we doing here ?

[aks96]

I had added to check the telemetry data but now instead of this using mock server for testing.

[nandan-bhat]

What will happen if AS returns only expires_in (No expires_at). Then newTokenSet.expires_at would be undefined ?

Can you confirm if that's a possibility ? If that happens you'll end up setting a session without expiry. Also TokenSet.expired() here would return false.

[aks96]

yea it return only expires_in, have added logic to convert into expires_at when token is created.