Skip to content

Commit ba13222

Browse files
Merge pull request #990 from Max-Health-Inc/develop
πŸ§ͺ Auto-PR: Merge `develop` β†’ `test`
2 parents 1ca374c + 33b59dc commit ba13222

19 files changed

Lines changed: 388 additions & 30 deletions

File tree

β€Žbackend/package.jsonβ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "proxy-smart-backend",
33
"displayName": "Proxy Smart Backend",
4-
"version": "0.3.7-beta.202608091534.25d7f9a8e",
4+
"version": "0.3.8-alpha.202608120751.92961df39",
55
"type": "module",
66
"scripts": {
77
"test": "bun test --isolate",

β€Žbackend/src/lib/admin-error-handler.tsβ€Ž

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,15 @@ export function handleAdminError(error: unknown, set: Context['set']) {
6565
/**
6666
* Sanitize error details before sending in HTTP response.
6767
* Removes stack traces and internal file paths to prevent information disclosure.
68+
*
69+
* Prefers Keycloak's own response body over `error.message`. The admin client throws
70+
* with the message set to just the error CODE, so a failed provider create surfaced as
71+
* the bare word `unknown_error` while the body carried the actual reason. Reporting the
72+
* code alone is indistinguishable from reporting nothing.
6873
*/
6974
function sanitizeErrorForResponse(error: unknown): string {
75+
const fromKeycloak = keycloakErrorDetail(error)
76+
if (fromKeycloak) return fromKeycloak
7077
if (error instanceof Error) {
7178
return error.message
7279
}
@@ -76,4 +83,31 @@ function sanitizeErrorForResponse(error: unknown): string {
7683
return 'An unexpected error occurred'
7784
}
7885

86+
/**
87+
* The most specific description Keycloak returned, across the field names its various
88+
* endpoints use (`error_description`, `errorMessage`, `error`) and the two places the
89+
* admin client stashes the parsed body.
90+
*/
91+
function keycloakErrorDetail(error: unknown): string | null {
92+
if (!error || typeof error !== 'object') return null
93+
const err = error as Record<string, unknown>
94+
const response = err.response as Record<string, unknown> | undefined
95+
const bodies = [err.responseData, response?.data, response?.body].filter(
96+
(b): b is Record<string, unknown> => !!b && typeof b === 'object',
97+
)
98+
99+
const parts: string[] = []
100+
for (const body of bodies) {
101+
for (const field of ['error_description', 'errorMessage', 'error']) {
102+
const value = body[field]
103+
if (typeof value === 'string' && value.trim() && !parts.includes(value)) parts.push(value)
104+
}
105+
}
106+
if (parts.length === 0) return null
107+
108+
// A message plus the code reads better than either alone, and the code is what
109+
// Keycloak's own docs are searchable by.
110+
return parts.join(': ')
111+
}
112+
79113

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// SPDX-FileCopyrightText: Max Health Inc.
2+
// SPDX-License-Identifier: AGPL-3.0-or-later OR LicenseRef-Commercial
3+
4+
/**
5+
* Config keys Keycloak requires before it will accept an identity provider.
6+
*
7+
* WHY THIS EXISTS. Keycloak does not validate an incomplete provider into a useful
8+
* message β€” `POST identity-provider/instances` for an `oidc` provider with no
9+
* `clientId`/`authorizationUrl` answers 500 `{"error":"unknown_error"}`. That reached
10+
* callers verbatim, so the only signal anyone got was the word "unknown_error": no
11+
* indication that a field was missing, which field, or that the request was the
12+
* problem at all. Checking here turns it into a 400 that names the keys.
13+
*
14+
* These are the keys the provider CANNOT work without, not every key it accepts.
15+
* Anything else a provider supports still passes through untouched.
16+
*/
17+
18+
/** Keycloak's own generic OIDC/OAuth2 brokers. */
19+
const OIDC_REQUIRED = ['clientId', 'clientSecret', 'authorizationUrl', 'tokenUrl'] as const
20+
21+
/**
22+
* Social brokers ship their endpoints in the provider factory, so a caller only
23+
* supplies credentials. Listed explicitly rather than matched by a fallback, so an
24+
* unknown providerId is left to Keycloak instead of being silently under-checked.
25+
*/
26+
const SOCIAL_PROVIDERS = [
27+
'google', 'github', 'facebook', 'microsoft', 'gitlab', 'bitbucket',
28+
'linkedin', 'linkedin-openid-connect', 'twitter', 'instagram',
29+
'paypal', 'stackoverflow', 'openshift-v3', 'openshift-v4',
30+
] as const
31+
32+
const REQUIRED_CONFIG: Record<string, readonly string[]> = {
33+
oidc: OIDC_REQUIRED,
34+
'keycloak-oidc': OIDC_REQUIRED,
35+
oauth2: OIDC_REQUIRED,
36+
saml: ['singleSignOnServiceUrl'],
37+
...Object.fromEntries(SOCIAL_PROVIDERS.map((id) => [id, ['clientId', 'clientSecret']])),
38+
}
39+
40+
/** The keys `providerId` needs, or an empty list when the type is not known here. */
41+
export function requiredConfigFor(providerId: string): readonly string[] {
42+
return REQUIRED_CONFIG[providerId] ?? []
43+
}
44+
45+
/**
46+
* Which required keys are absent or blank.
47+
*
48+
* Blank counts as absent: Keycloak treats `clientId: ""` exactly as it treats a
49+
* missing one, and a form that submits empty inputs is the common way to get here.
50+
*/
51+
export function missingConfigKeys(
52+
providerId: string,
53+
config: Record<string, unknown> | undefined,
54+
): string[] {
55+
return requiredConfigFor(providerId).filter((key) => {
56+
const value = config?.[key]
57+
return value === undefined || value === null || (typeof value === 'string' && value.trim() === '')
58+
})
59+
}

β€Žbackend/src/routes/admin/identity-providers.tsβ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
import { handleAdminError } from '@/lib/admin-error-handler'
2222
import { extractBearerToken } from '@/lib/admin-utils'
2323
import { ensureIdpAttributeMappers } from '@/lib/idp-mappers'
24+
import { missingConfigKeys } from '@/lib/idp-required-config'
2425
import { logger } from '@/lib/logger'
2526
import type IdentityProviderRepresentation from '@keycloak/keycloak-admin-client/lib/defs/identityProviderRepresentation.js'
2627

@@ -137,6 +138,17 @@ export const identityProvidersRoutes = new Elysia({ prefix: '/idps' })
137138
return { error: 'Alias and providerId are required' }
138139
}
139140

141+
// Keycloak answers 500 `unknown_error` for an incomplete provider, naming nothing.
142+
// Refuse first, and say which keys are missing.
143+
const missing = missingConfigKeys(payload.providerId, payload.config as Record<string, unknown>)
144+
if (missing.length > 0) {
145+
set.status = 400
146+
return {
147+
error: `Missing required config for providerId "${payload.providerId}": ${missing.join(', ')}. ` +
148+
`Supply them under "config".`,
149+
}
150+
}
151+
140152
await admin.identityProviders.create({
141153
...payload,
142154
config: payload.config ?? {}

β€Žbackend/src/schemas/admin/identity-providers.tsβ€Ž

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,66 @@ import { t, type Static } from 'elysia'
1111
/**
1212
* Identity Provider configuration schema for OIDC/SAML providers
1313
*/
14+
/**
15+
* The Keycloak identity-provider `config` map.
16+
*
17+
* TWO DEFECTS THIS FIXES, both of which made an OIDC provider impossible to create
18+
* through any caller β€” the admin UI, the generated client, and the MCP tool alike.
19+
*
20+
* 1. `clientId` and `authorizationUrl` were absent. Two of the four keys Keycloak
21+
* requires for `oidc` could not be named at all, so every create reached Keycloak
22+
* without them and came back 500 `unknown_error`.
23+
* 2. The object was closed, and Elysia STRIPS undeclared body properties rather than
24+
* rejecting them. So a caller that sent `clientId` anyway had it removed silently,
25+
* before the handler ran β€” no error, no warning, nothing in a log.
26+
*
27+
* Hence `additionalProperties`: the named keys below document what a provider commonly
28+
* needs, and anything else Keycloak supports still travels. The old `additionalConfig`
29+
* sub-object was not that escape hatch β€” Keycloak has no `additionalConfig` key, so
30+
* anything nested under it was passed to Keycloak and ignored, which reads as an escape
31+
* hatch while being a dead end.
32+
*
33+
* Values are strings because Keycloak stores this map as strings; the few booleans it
34+
* accepts are declared explicitly below.
35+
*/
1436
export const IdentityProviderConfig = t.Object({
15-
// Common fields
16-
displayName: t.Optional(t.String({ description: 'Display name for UI' })),
17-
enabled: t.Optional(t.Boolean({ description: 'Whether the provider is enabled', default: true })),
18-
19-
// OIDC/OAuth2 specific fields
20-
clientSecret: t.Optional(t.String({ description: 'OAuth2 client secret' })),
21-
tokenUrl: t.Optional(t.String({ description: 'Token endpoint URL' })),
37+
// ── OIDC / OAuth2 ──
38+
clientId: t.Optional(t.String({ description: 'OAuth2 client id issued by the provider. REQUIRED for oidc, oauth2, keycloak-oidc and the social providers.' })),
39+
clientSecret: t.Optional(t.String({ description: 'OAuth2 client secret. REQUIRED for oidc, oauth2, keycloak-oidc and the social providers.' })),
40+
authorizationUrl: t.Optional(t.String({ description: "The provider's authorization endpoint URL. REQUIRED for oidc, oauth2 and keycloak-oidc." })),
41+
tokenUrl: t.Optional(t.String({ description: "The provider's token endpoint URL. REQUIRED for oidc, oauth2 and keycloak-oidc." })),
2242
userInfoUrl: t.Optional(t.String({ description: 'UserInfo endpoint URL' })),
23-
issuer: t.Optional(t.String({ description: 'OIDC issuer URL' })),
24-
defaultScopes: t.Optional(t.String({ description: 'Default OAuth2 scopes' })),
43+
issuer: t.Optional(t.String({ description: 'OIDC issuer URL, validated against the id token iss claim' })),
44+
defaultScopes: t.Optional(t.String({ description: 'Space-separated OAuth2 scopes to request (e.g. "openid profile email")' })),
2545
logoutUrl: t.Optional(t.String({ description: 'Logout endpoint URL' })),
26-
27-
// SAML specific fields
46+
clientAuthMethod: t.Optional(t.String({ description: 'Client authentication method (client_secret_post, client_secret_basic, client_secret_jwt, private_key_jwt)' })),
47+
validateSignature: t.Optional(t.Boolean({ description: 'Validate signatures on tokens/assertions from this provider' })),
48+
useJwksUrl: t.Optional(t.Boolean({ description: 'Fetch the signing keys from jwksUrl rather than using a static certificate' })),
49+
jwksUrl: t.Optional(t.String({ description: "The provider's JWKS URL, used when useJwksUrl is true" })),
50+
pkceEnabled: t.Optional(t.Boolean({ description: 'Send a PKCE challenge on the authorization request' })),
51+
pkceMethod: t.Optional(t.String({ description: 'PKCE code challenge method (S256 or plain)' })),
52+
syncMode: t.Optional(t.String({ description: 'How brokered user data is synced on later logins: IMPORT, LEGACY or FORCE' })),
53+
54+
// ── SAML ──
2855
entityId: t.Optional(t.String({ description: 'SAML entity ID' })),
29-
singleSignOnServiceUrl: t.Optional(t.String({ description: 'SAML SSO URL' })),
56+
singleSignOnServiceUrl: t.Optional(t.String({ description: 'SAML SSO URL. REQUIRED for saml.' })),
3057
singleLogoutServiceUrl: t.Optional(t.String({ description: 'SAML SLO URL' })),
3158
metadataDescriptorUrl: t.Optional(t.String({ description: 'SAML metadata URL' })),
3259
signatureAlgorithm: t.Optional(t.String({ description: 'SAML signature algorithm' })),
3360
nameIdPolicyFormat: t.Optional(t.String({ description: 'SAML NameID format' })),
3461
signingCertificate: t.Optional(t.String({ description: 'SAML signing certificate' })),
35-
validateSignature: t.Optional(t.Boolean({ description: 'Validate SAML signatures' })),
3662
wantAuthnRequestsSigned: t.Optional(t.Boolean({ description: 'Require signed AuthN requests' })),
37-
38-
// Allow additional configuration
39-
additionalConfig: t.Optional(t.Record(t.String(), t.Any()))
40-
}, { title: 'IdentityProviderConfig' })
63+
64+
// ── Kept for the admin UI, which writes it alongside the real keys ──
65+
displayName: t.Optional(t.String({ description: 'Display name for UI' })),
66+
}, {
67+
title: 'IdentityProviderConfig',
68+
// Any other key this provider type supports, passed through to Keycloak verbatim.
69+
// Booleans and numbers are accepted as well as strings: Keycloak stores the map as
70+
// strings and coerces, and restricting to strings would recreate the dead end this
71+
// replaces for keys whose natural JSON form is not a string.
72+
additionalProperties: t.Union([t.String(), t.Boolean(), t.Number()]),
73+
})
4174

4275
export const IdentityProvider = t.Object({
4376
alias: t.Optional(t.String({ description: 'Provider alias (unique identifier)' })),

0 commit comments

Comments
Β (0)