feat: add Kanidm OIDC support with PKCE and ID token algo config#682
feat: add Kanidm OIDC support with PKCE and ID token algo config#682mikimandoki wants to merge 2 commits into
Conversation
| type: 'oauth', | ||
| wellKnown: env.OIDC_WELL_KNOWN_URL, | ||
| authorization: { params: { scope: 'openid email profile' } }, | ||
| checks: env.OIDC_PKCE_ENABLED ? ['pkce'] : [], |
There was a problem hiding this comment.
As far as I understand OIDC, the encryption configs should be provided from the well known url endpoint. I am against opening up the env var config to the whole breadth of OAuth2 options as this would just continue increasing and be a bad incentive for AI agents.
I am not familiar with Kandim, but please double check it's not an issue on their end or with your configuration, as setting these options by hand should not be required.
There was a problem hiding this comment.
Thanks for the feedback. I'm no OIDC expert myself. Here's what I could find out:
Kanidm correctly advertises ES256 and its PKCE requirements in the .well-known files. These are the defaults Kanidm ships with. Disabling them is possible but the commands are along the lines of "enable legacy crypto" etc., highly suggesting that users should only mess with this if necessary.
wget -qO- https://kanidm:8443/oauth2/openid/splitpro/.well-known/openid-configuration 2>/dev/null
"id_token_signing_alg_values_supported":["ES256"],
"code_challenge_methods_supported":["S256"]The issue appears to lie in openid-client NextAuth v4 ships.
openid-client defaults to RS256 and intentionally ignores id_token_signing_alg_values_supported from discovery. The expectation per spec is that the OP signs tokens with whatever alg the client was registered with. Since SplitPro passes credentials directly to openid-client with no additional configuration, and Kanidm defaults to ES256, the mismatch has to be resolved on the client side via id_token_signed_response_alg. The maintainer has expressed this as well: panva/openid-client#115 (comment)
As far as I can tell adding in the checks and client params is the intended way to deal with this, however, I have come across a different implementation as well:
danny-avila/LibreChat#5348 reads the discovered value and passes that on to the client. Happy to give this approach shot if it is more desirable.
| refresh_expires_in: _refresh_expires_in, | ||
| ...standardAccountData | ||
| } = account as AdapterAccount & Record<string, unknown>; | ||
| const knownAccountFields = new Set<string>(Object.values(Prisma.AccountScalarFieldEnum)); |
| OIDC_WELL_KNOWN_URL="https://keycloak.example.com/realms/My_Realm/.well-known/openid-configuration" | ||
| ``` | ||
|
|
||
| ### OIDC (Kanidm) |
There was a problem hiding this comment.
This document should be concise and does not have space for the myriad of OAuth providers out there. Respective values should be available in the provider's docs, please remove this.
There was a problem hiding this comment.
Fair enough - changes made.
Description
While I was attempting to set up Kanidm to work with SplitPro I had hit a few roadblocks:
Kanidm includes an
issued_token_typefield in its token response which isn't in the Prisma schema, causing a DB crash before the account could be created. Rather than adding another provider-specific case tolinkAccount, this PR replaces the entire chain with a generic sanitisation pass usingPrisma.AccountScalarFieldEnumas the source of truth. Any provider with non-standard fields now works without further changes.Kanidm also enforces PKCE and ES256 by default, both of which were previously not supported by SplitPro. These can now be configured by optional env vars. I also added an example kanidm env config.
To test these changes, I ran the updated code on my NAS in a Docker container and finally managed to authenticate with Kanidm. Disabling PKCE or defaulting to RS256 both fail on the default Kanidm config.
Checklist
CONTRIBUTING.mdin its entirety