Replies: 2 comments 2 replies
-
This is the way to go. Not fragile, it is part of the public API. I will not be adding a different way of supporting "hashed" client secrets because it would not be immediately obvious that doing so prohibits the use of client_secret_jwt and symmetric encryption which is derived off of the client secret value. |
Beta Was this translation helpful? Give feedback.
-
|
We need this too, especially for zero-downtime client secret rotation. Our use case is allowing multiple active secrets for the same client during a grace period, so both the old and new secret authenticate until the old one can be retired. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Description
Use Case
Multi-tenant OIDC providers that manage their own client registry (not using Dynamic Client Registration) need to:
Current Behavior
compareClientSecret(actual)inlib/models/client.jsdoes a direct string comparison:This means the adapter must return the plaintext
client_secretin its metadata for authentication to work. There's no hook to customize how secrets are compared.Problem
This forces implementers into one of these suboptimal patterns:
client_secret. This works forclient_secret_basicandclient_secret_postbut breaksclient_secret_jwt(where the secret is used as an HMAC key, not compared directly)None of these support multi-secret rotation cleanly.
Proposed Solution
Add a
compareClientSecretconfiguration callback, defaulting to the current behavior:Implementation
The change is minimal (~10 lines). In
lib/models/client.js:This is fully backwards-compatible because
compareClientSecretis already called withawaitinlib/shared/client_auth.js:In
lib/helpers/defaults.js, add with documentation:Why This Matters
Alternatives Considered
provider.Client.prototype.compareClientSecret = ...— fragile, the Client class is created inside a closure and not easily accessible after constructionclient_secret_basic/client_secret_postbut notclient_secret_jwtclient_secretas array: Would support multi-secret but is a bigger API change and doesn't solve the hashing problemImpact
Context
We're building Porta (soon to be public oss), a multi-tenant OIDC provider on top of node-oidc-provider. We manage our own client registry in PostgreSQL with Argon2id-hashed secrets and need a clean way to verify secrets without storing them in plaintext.
Currently we use a SHA-256 pre-hashing middleware workaround, which works for basic/post auth methods but is architecturally inelegant and doesn't support
client_secret_jwt.Beta Was this translation helpful? Give feedback.
All reactions