feat(api): admin JWT auth + /manage Organization endpoints + platform-admin scope#1
Open
patrickleet wants to merge 40 commits into
Open
feat(api): admin JWT auth + /manage Organization endpoints + platform-admin scope#1patrickleet wants to merge 40 commits into
patrickleet wants to merge 40 commits into
Conversation
…Openpanel-dev#365) * Add support for base url, project and org for open ai and base url, token and version for anthropic * Update dependencies * update: use named import for zod
…#352) Closes Openpanel-dev#177. Browser tabs previously read e.g. "Dashboard | OpenPanel.dev" regardless of which project was open, making it hard to tell tabs apart when multiple projects are pinned in the same window. The existing `createProjectTitle()` helper already accepts an optional projectName argument, but no caller passed it. Threading project name through every route's loader/head would touch ~30 files; instead, this adds a single `useProjectDocumentTitle` hook mounted in the project layout that patches `document.title` on the client whenever it changes. The hook observes mutations on `<head>` so it re-applies the project name after navigation (when TanStack's head() re-renders the title) and when the project query first resolves. SSR title is unchanged — this is purely a browser tab UX improvement. Result: "Dashboard | MyProject | OpenPanel.dev"
* feat: added smtp support to email * refactor: added SMTP_PORT check and timeout configurations to createSmtpTransport * doc: added description for SMTP * chore: updating .env.example * doc: added warning for SMTP failure * Remove SMTP options from .env.template Removed SMTP configuration options from the .env.template file. --------- Co-authored-by: Carl-Gerhard Lindesvärd <1987198+lindesvard@users.noreply.github.com>
|
Important Review skippedToo many files! This PR contains 223 files, which is 73 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (223)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughPR adds OIDC JWT bearer-token admin authentication and platform-scoped organization management. New auth flow verifies JWTs against remote JWKS, extracts role/org claims, and dispatches between JWT and legacy client-secret validation. Organization CRUD endpoints enforce platform-admin access, reject reserved org slugs, and prevent sentinel deletion. Manage router switches to new auth dispatcher and registers org endpoints. ChangesAdmin Organization Management with OIDC JWT Authentication
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/controllers/manage.controller.ts`:
- Around line 441-450: Before calling db.organization.delete, invalidate cached
entries for all descendant projects and clients so deleted resources can't be
served from cache: after the existence check for the org (the existing variable)
but before db.organization.delete, query the org's projects and clients (e.g.
select their ids) and call the cache invalidation helpers getProjectByIdCached
and getClientByIdCached for each id (or their batch/invalidateAll variant if
available); ensure invalidation errors are handled or logged but do not abort
the deletion flow.
In `@apps/api/src/utils/auth.ts`:
- Around line 354-360: The discovery fetch currently uses fetch(discoveryUrl)
without a timeout which can hang; update the call in auth.ts to pass an
AbortSignal with AbortSignal.timeout(MS) (e.g., const signal =
AbortSignal.timeout(ADMIN_OIDC_DISCOVERY_TIMEOUT_MS) or inline
fetch(discoveryUrl, { signal:
AbortSignal.timeout(ADMIN_OIDC_DISCOVERY_TIMEOUT_MS) })) so the request is
bounded, and ensure you handle the abort/error path consistently where
discoveryRes and discovery are used (keep the existing error throw for non-ok
responses and add handling for an abort/timeout error).
- Around line 350-353: The discovery URL construction uses a root-relative path
("/.well-known/openid-configuration") which discards any path component of
config.issuer; update the code that builds discoveryUrl so it uses
".well-known/openid-configuration" (no leading slash) and ensure config.issuer
has a trailing slash when passed to new URL (e.g., append '/' if missing) so
issuers like "https://example.com/issuer1" resolve to
"https://example.com/issuer1/.well-known/openid-configuration"; locate the
discoveryUrl creation where config.issuer is used and adjust accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ae6ab6b-db40-4db7-b250-2d8887112ffb
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/api/package.jsonapps/api/src/controllers/manage.controller.tsapps/api/src/routes/manage.router.tsapps/api/src/utils/auth.ts
Comment on lines
+441
to
+450
| const existing = await db.organization.findFirst({ | ||
| where: { id: request.params.id }, | ||
| }); | ||
| if (!existing) { | ||
| throw new HttpError('Organization not found', { status: 404 }); | ||
| } | ||
|
|
||
| await db.organization.delete({ | ||
| where: { id: request.params.id }, | ||
| }); |
There was a problem hiding this comment.
Invalidate descendant client/project caches before cascading org deletion.
db.organization.delete() removes the org's projects and clients, but this handler never clears getProjectByIdCached or getClientByIdCached. That leaves deleted credentials and project records potentially usable until cache expiry.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/controllers/manage.controller.ts` around lines 441 - 450, Before
calling db.organization.delete, invalidate cached entries for all descendant
projects and clients so deleted resources can't be served from cache: after the
existence check for the org (the existing variable) but before
db.organization.delete, query the org's projects and clients (e.g. select their
ids) and call the cache invalidation helpers getProjectByIdCached and
getClientByIdCached for each id (or their batch/invalidateAll variant if
available); ensure invalidation errors are handled or logged but do not abort
the deletion flow.
Comment on lines
+350
to
+353
| const discoveryUrl = new URL( | ||
| '/.well-known/openid-configuration', | ||
| config.issuer, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For an OpenID Connect issuer URL that includes a path segment (for example a realm or tenant path), what is the correct discovery URL format for /.well-known/openid-configuration?
💡 Result:
For an OpenID Connect (OIDC) issuer URL that includes a path segment (such as a realm or tenant path), the correct discovery URL is formed by appending /.well-known/openid-configuration to the issuer URL, ensuring that any trailing slash in the issuer URL is removed first [1][2]. According to the OpenID Connect Discovery 1.0 specification, if the issuer value contains a path component, the Relying Party (RP) must remove any terminating slash from the issuer URL before appending the /.well-known/openid-configuration string [1][3]. For example, if the issuer URL is https://example.com/issuer1, the discovery URL is https://example.com/issuer1/.well-known/openid-configuration [1][2]. If the issuer URL were provided as https://example.com/issuer1/, the trailing slash would be removed to result in the same discovery URL [1].
Citations:
- 1: https://openid.net/specs/openid-connect-discovery-1_0.html
- 2: https://openid.net/specs/openid-connect-discovery-1_0-errata2.html
- 3: https://shoepapp.com/?_=%2Fspecs%2Fopenid-connect-discovery-1_0.html%23ePsMxVOlbgzWxs9KzhwK93X7
🏁 Script executed:
cd apps/api/src/utils && sed -n '340,365p' auth.tsRepository: hops-ops/openpanel-app
Length of output: 828
🏁 Script executed:
node <<'EOF'
// Test URL constructor behavior with leading slash
const issuerWithPath = 'https://example.com/issuer1';
const issuerWithoutPath = 'https://example.com';
// Current code pattern (with leading slash)
const url1 = new URL('/.well-known/openid-configuration', issuerWithPath);
console.log('With leading slash and path-based issuer:');
console.log(' Input issuer:', issuerWithPath);
console.log(' Result:', url1.toString());
// Correct pattern (without leading slash)
const url2 = new URL('.well-known/openid-configuration', issuerWithPath);
console.log('\nWithout leading slash and path-based issuer:');
console.log(' Input issuer:', issuerWithPath);
console.log(' Result:', url2.toString());
// What about trailing slashes in issuer?
const issuerWithTrailingSlash = 'https://example.com/issuer1/';
const url3 = new URL('.well-known/openid-configuration', issuerWithTrailingSlash);
console.log('\nWithout leading slash and issuer with trailing slash:');
console.log(' Input issuer:', issuerWithTrailingSlash);
console.log(' Result:', url3.toString());
const url4 = new URL('/.well-known/openid-configuration', issuerWithTrailingSlash);
console.log('\nWith leading slash and issuer with trailing slash:');
console.log(' Input issuer:', issuerWithTrailingSlash);
console.log(' Result:', url4.toString());
EOFRepository: hops-ops/openpanel-app
Length of output: 699
Remove the leading slash from the discovery URL path to preserve issuer paths.
Line 350 uses '/.well-known/openid-configuration' which treats the path as root-relative, discarding any path component in config.issuer. According to OIDC Discovery 1.0 specification, for an issuer like https://example.com/issuer1, the discovery URL must be https://example.com/issuer1/.well-known/openid-configuration, not https://example.com/.well-known/openid-configuration. This breaks authentication for path-scoped issuers (realms, tenants, etc.).
Fix approach
Change:
const discoveryUrl = new URL(
'/.well-known/openid-configuration',
config.issuer,
);
To (without leading slash and ensuring issuer has trailing slash):
const discoveryUrl = new URL(
'.well-known/openid-configuration',
config.issuer.endsWith('/') ? config.issuer : config.issuer + '/',
);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/utils/auth.ts` around lines 350 - 353, The discovery URL
construction uses a root-relative path ("/.well-known/openid-configuration")
which discards any path component of config.issuer; update the code that builds
discoveryUrl so it uses ".well-known/openid-configuration" (no leading slash)
and ensure config.issuer has a trailing slash when passed to new URL (e.g.,
append '/' if missing) so issuers like "https://example.com/issuer1" resolve to
"https://example.com/issuer1/.well-known/openid-configuration"; locate the
discoveryUrl creation where config.issuer is used and adjust accordingly.
Comment on lines
+354
to
+360
| const discoveryRes = await fetch(discoveryUrl); | ||
| if (!discoveryRes.ok) { | ||
| throw new Error( | ||
| `Admin OIDC: discovery fetch failed (${discoveryRes.status}) for ${discoveryUrl}`, | ||
| ); | ||
| } | ||
| const discovery = (await discoveryRes.json()) as { jwks_uri?: string }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In current Node.js, does fetch()have a default request timeout, and isAbortSignal.timeout() the recommended way to bound it?
💡 Result:
In Node.js, the native fetch function does not have a built-in, configurable request timeout [1][2]. By default, it will wait indefinitely for a response unless the underlying network connection times out or is otherwise interrupted [2]. Yes, AbortSignal.timeout is the recommended, standard way to bound the duration of a fetch request [3][4][5]. This approach is part of the Web Fetch API standard, making it the cross-platform, future-proof method for handling timeouts [6][7]. To use it, pass the signal to the fetch options object [3][5]: try { const response = await fetch('https://example.com', { signal: AbortSignal.timeout(5000) // Aborts after 5 seconds }); const data = await response.json; } catch (err) { if (err.name === 'TimeoutError') { console.error('The request timed out'); } else { console.error('Fetch error:', err); } } When a timeout occurs, the signal aborts the request and throws a TimeoutError [5]. Note that while some older third-party libraries (like node-fetch) previously offered a proprietary timeout option, this is non-standard and discouraged in favor of AbortSignal [6][7].
Citations:
- 1: fetch fails with internal error after 5 minutes nodejs/node#46375
- 2: https://stackoverflow.com/questions/46946380/fetch-api-request-timeout
- 3: https://medium.com/deno-the-complete-reference/timeout-fetch-request-in-node-js-4231f33a9b95
- 4: https://medium.com/%40Brahmbhatnilay/introducing-abortsignal-timeout-enhancing-fetch-functionality-68f01adabaea
- 5: https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout_static
- 6: https://stackoverflow.com/questions/54204342/node-fetch-why-is-signal-recommended-over-timeout
- 7: Should we deprecate timeout in favor of AbortController? node-fetch/node-fetch#523
Add a timeout to the discovery fetch to prevent indefinite hangs on admin requests.
Node.js's native fetch() has no default timeout and will wait indefinitely if the issuer is slow or unreachable, blocking admin requests in the /manage auth preHandler. AbortSignal.timeout() is the standard, recommended way to bound fetch duration:
- const discoveryRes = await fetch(discoveryUrl);
+ const discoveryRes = await fetch(discoveryUrl, {
+ signal: AbortSignal.timeout(5_000),
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const discoveryRes = await fetch(discoveryUrl); | |
| if (!discoveryRes.ok) { | |
| throw new Error( | |
| `Admin OIDC: discovery fetch failed (${discoveryRes.status}) for ${discoveryUrl}`, | |
| ); | |
| } | |
| const discovery = (await discoveryRes.json()) as { jwks_uri?: string }; | |
| const discoveryRes = await fetch(discoveryUrl, { | |
| signal: AbortSignal.timeout(5_000), | |
| }); | |
| if (!discoveryRes.ok) { | |
| throw new Error( | |
| `Admin OIDC: discovery fetch failed (${discoveryRes.status}) for ${discoveryUrl}`, | |
| ); | |
| } | |
| const discovery = (await discoveryRes.json()) as { jwks_uri?: string }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/utils/auth.ts` around lines 354 - 360, The discovery fetch
currently uses fetch(discoveryUrl) without a timeout which can hang; update the
call in auth.ts to pass an AbortSignal with AbortSignal.timeout(MS) (e.g., const
signal = AbortSignal.timeout(ADMIN_OIDC_DISCOVERY_TIMEOUT_MS) or inline
fetch(discoveryUrl, { signal:
AbortSignal.timeout(ADMIN_OIDC_DISCOVERY_TIMEOUT_MS) })) so the request is
bounded, and ensure you handle the abort/error path consistently where
discoveryRes and discovery are used (keep the existing error throw for non-ok
responses and add handling for an abort/timeout error).
This reverts commit c470b8b.
* init * update * performance changes * bump bklit * update bklit * bklit update
Fork-only commit; do NOT cherry-pick upstream.
Single env var change so the existing multi-arch build + manifest
merge pipeline publishes to `ghcr.io/hops-ops/{api,worker,dashboard}`
instead of `ghcr.io/openpanel-dev/{api,worker,dashboard}`. Everything
else — Dockerfiles, build matrix, GHA cache, tag scheme — is
unchanged.
Followup commits will add OIDC + SMTP support and a semver tag step.
Adds a generic OIDC sign-in path that works with any compliant
Identity Provider (Zitadel, Keycloak, Authentik, Okta, etc.) by
configuring endpoint URLs directly rather than per-provider
library support.
Provider is enabled when these env vars are all set:
OIDC_CLIENT_ID
OIDC_CLIENT_SECRET
OIDC_REDIRECT_URI — <dashboard>/api/oauth/oidc/callback
OIDC_AUTHORIZATION_ENDPOINT
OIDC_TOKEN_ENDPOINT
Optional:
OIDC_DISPLAY_NAME — label shown on sign-in button
(defaults to "Single Sign-On")
Implementation mirrors the existing Google flow:
- PKCE (S256) for the auth code exchange
- User identity from the ID token (sub, email, given_name,
family_name; falls back to `name` claim if given_name absent)
- email_verified honoured when present; defaults to verified when
the IdP doesn't emit the claim (operators choose trusted IdPs)
Files:
- packages/auth/src/oauth.ts — new `oidc` Arctic.OAuth2Client +
endpoint exports + `isOidcEnabled()` helper.
- packages/trpc/src/routers/auth.ts — zProvider enum gains 'oidc';
signInOAuth branch creates the authorization URL.
- apps/api/src/controllers/oauth-callback.controller.tsx — new
oidcCallback + fetchOidcUser; validateOAuthCallback generalised
to handle any PKCE provider via cookie naming convention
(<provider>_code_verifier).
- apps/api/src/routes/oauth-callback.router.ts — registers
GET /oidc/callback.
Dashboard sign-in button rendering still needs a small UI change
(separate commit) so end-users see an OIDC option. tRPC route is
fully wired regardless.
Adds a SignInOidc component that renders on the /login and
/onboarding pages whenever the server's getServerEnvs reports
oidc.enabled=true (i.e. when the OIDC_CLIENT_ID/SECRET/REDIRECT_URI/
AUTHORIZATION_ENDPOINT/TOKEN_ENDPOINT env vars are all set).
- packages/auth flag isOidcEnabled already gates the tRPC route
and the callback; this commit just surfaces the same check to
the client via the existing getServerEnvs server function.
- Button label uses OIDC_DISPLAY_NAME, defaulting to "Single Sign-On".
- Falls back to null render when OIDC is not configured so existing
installs (Google/GitHub only) see no UI change.
- last-auth-provider cookie path extended so an OIDC user lands
back on the OIDC button after sign-out.
Pair: /api/config now returns oidc.{enabled,displayName} so the
debug endpoint reflects what the UI is seeing.
The dashboard's getServerEnvs previously gated `oidc.enabled` on all five OIDC_* env vars being set. That forced operators to either pass OIDC_CLIENT_SECRET / endpoint URLs to the dashboard pod (where they aren't needed — the OAuth flow runs in apps/api) or accept that the SignInOidc button never renders. Tightened to check only OIDC_CLIENT_ID. The api pod still validates the full env when handling /api/oauth/oidc/callback (oidcCallback in apps/api/src/controllers/oauth-callback.controller.tsx) so an incomplete config on the api side still errors loudly at sign-in time — just not in a way that hides the button.
Adds a JWT-bearer admin auth path for /manage routes, gated behind
ADMIN_OIDC_ISSUER configuration. When unset, /manage continues to
accept the existing openpanel-client-id / openpanel-client-secret
Client-pair auth unchanged.
When configured, /manage accepts `Authorization: Bearer <jwt>` from
any token signed by the configured OIDC issuer (Zitadel, Keycloak,
Authentik, etc.) that:
- validates against the issuer's JWKS (discovered via
/.well-known/openid-configuration)
- matches the configured audience (ADMIN_OIDC_AUDIENCE)
- carries the required role claim (ADMIN_OIDC_REQUIRED_ROLE,
defaults to 'openpanel:admin'), tolerantly looking at
`roles[]`, `scope`, and Zitadel's nested
`urn:zitadel:iam:org:project:roles` shape
- carries an organization claim (ADMIN_OIDC_ORG_CLAIM, defaults
to the Zitadel resourceowner-id claim)
The JWT-validated request synthesizes a Client-shaped record with
`type: root`, `secret: null`, `id: jwt:<sub>`, and the
organizationId from the claim — so existing controllers in
manage.controller.ts work without branching on auth source.
Implementation:
- jose@^6 added to apps/api for JWKS-based JWT verification
- validateAdminJwtRequest + validateAdminRequest wrapper in
apps/api/src/utils/auth.ts
- manage.router.ts switches its preHandler from
validateManageRequest to validateAdminRequest
Backwards-compatible. Off by default. No DB schema changes. No new
auth tier inside OpenPanel — identity / orgs / roles stay in the
configured IdP.
Refs the openpanel-admin-jwt-auth spec.
Adds GET / POST / PATCH / DELETE /manage/organizations[/:id] routes mirroring the existing /manage/projects shape. Use cases: - Platform-admin OIDC callers (the new JWT auth path) provisioning new Organizations as part of tenant onboarding workflows - Root-Client callers reading/updating/deleting their own Organization v1 deliberately scopes list/get to the caller's bound organization. Cross-org listing for a true instance-admin claim would require a richer claim model than v1 ships; the endpoints just trust the auth context's organizationId. Member and Invite admin endpoints are deferred — the JWT-auth-bootstrap-then-OIDC-sign-in flow makes them less load-bearing than they'd be for a session-cookie admin UX. Adding them later is additive. Refs the openpanel-admin-jwt-auth spec.
…e Create The original `getOrganization` built its Prisma `where` with an object spread that always overrode `id` with `request.client.organizationId`, so GET `/manage/organizations/<any-id>` silently returned the caller's own Organization rather than 404'ing. The same gap let UPDATE and DELETE target the caller's org even when the URL param pointed at a different ID. Crossplane / Terraform reconciles surfaced this loudly: an `openpanel_organization` MR with `forProvider.name = smoketest-org` caused a successful POST (new Org row in DB) followed by a READ that returned the caller's anchored Org. TF treated that as drift, called UPDATE on the wrong row, then DELETE on tear-down — wiping the bootstrap Org and cascading to its root Client. Fixes: - `getOrganization`: explicit `params.id === client.organizationId` check; 404 otherwise. No more spread override. - `updateOrganization` / `deleteOrganization`: same scoping check before mutating. - `createOrganization`: 403 for now. Both client-pair root auth and the synthesized OIDC-JWT admin client are anchored to one organizationId; a successful Create would produce an Org the same caller can never READ back. Until a real platform-admin scope exists (multi-org JWT claim, or a reserved `organizationId = "platform"`), Org provisioning is operator-driven via the chart bootstrap Job. TenantStack will route through that platform scope once it lands. Also fixes a pre-existing typecheck break in `synthesizeAdminClient` (IServiceClientWithProject schema changed under the JWT auth PR — drop `cors`, add `ignoreCorsAndSecret`). Local verification on pat-local pending image rebuild.
Detects platform-admin callers by sentinel `client.organizationId === 'platform-admin'` (the row + root Client the openpanel-chart bootstrap Job seeds at install time). Bypasses same-org filtering on: LIST — returns every Organization in the install GET — fetches by URL id without org-match UPDATE — updates by URL id without org-match DELETE — deletes by URL id without org-match CREATE — allowed (was 403 for everyone in the previous commit) Tenant-scoped callers still get the strict same-org behavior. Guardrails: - `createOrganization` refuses to mint a new Org whose slugified id collides with the `platform-admin` sentinel. The bootstrap Job INSERTs that row directly via SQL and isn't subject to the gate. - `deleteOrganization` refuses to delete the `platform-admin` Org itself (would cascade-delete the platform-admin root Client and lock the operator out of /manage until the chart bootstrap re-runs). Unblocks TenantStack: a Crossplane Organization MR using the platform-admin client's credentials can now provision a fresh tenant Org, after which the per-tenant chart install's own bootstrap Job mints that tenant's first root Client scoped to the new Org. Project / Client / Reference handlers stay tenant-scoped — the per-tenant credentials manage those, not the platform-admin.
…rm-admin already appends a random -NNNN suffix on id collision, so naming a tenant 'Platform Admin' silently produced `platform-admin-1459` (or similar) rather than 409'ing. That's a confusing UX even though the suffixed id doesn't grant god-mode (the isPlatformAdmin sentinel check requires an exact match). Reject the request whenever `slug(name) === 'platform-admin'`, before calling getId. Tenants who name themselves 'Platform Admin' now get the explicit 409 they meant to see.
A JWT that carries the configured `platformAdminRole` (default
`openpanel:platform-admin`) is synthesized with the sentinel
`organizationId='platform-admin'`. The manage controllers'
`isPlatformAdmin` check then bypasses tenant scoping — same code path
as the chart-managed bootstrap root Client, just with rotation +
audit-trail provided by the IdP instead of a static shared secret.
Existing behavior preserved for tenant-scoped admin JWTs: tokens
without the platform-admin role get the org claim treatment as
before.
This lets Crossplane and TF callers reach the upjet provider via
the `oidc {}` provider config block — Zitadel mints a fresh token
per reconcile via client_credentials, the token's role grant
authorizes platform-admin scope, the server elevates accordingly.
Env:
ADMIN_OIDC_PLATFORM_ADMIN_ROLE defaults openpanel:platform-admin
Set this to the same value as ADMIN_OIDC_REQUIRED_ROLE if every admin
JWT should imply platform-admin. Leave as default to keep a separation
between tenant-admins and platform-admins.
Zitadel's `client_credentials` grant for MachineUsers does NOT include project role claims in the access JWT — those are only retrievable via the introspection endpoint. Role-based elevation works fine for Keycloak / Auth0 / generic IdPs that DO assert roles in the access token, but our primary IdP is Zitadel. Add `ADMIN_OIDC_PLATFORM_ADMIN_AUDIENCE`: when the token's `aud` claim contains the configured value, elevate to the `platform-admin` sentinel scope. Operators restrict MachineUser membership of that Zitadel Project to platform-admins; the audience-bound JWT then is sufficient signal on its own. Either path (`platformAdminRole` OR `platformAdminAudience`) is independently sufficient. Both elevate to the sentinel; both bypass the `requiredRole` check (platform-admin ≥ admin). Neither configured falls back to the existing role-based admin / tenant-scoped flow. `aud` is parsed correctly as either a string or string array per RFC 7519 §4.1.3.
patrickleet
force-pushed
the
feat/admin-jwt-auth
branch
from
276ae72 to
4b30e33
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds admin-only
/managecontrollers to the OpenPanel API server with two auth modes and a platform-admin tenancy scope. The goal is to make the entire OpenPanel install programmatically manageable — from inside Terraform, OpenTofu, and Crossplane — without ever clicking through the dashboard.This is a fork-only PR for review; not intended to be merged to upstream
Openpanel-dev/openpanelas-is (some shape changes would be needed first).Commits in this branch (oldest → newest)
51d2dbd66639661e88dc0ef147199f25248cd688What's in
/manageadmin endpointsFull CRUD over
Organization,Project,Client,Reference, plus list endpoints for Dashboards and Reports. Routes live under/manage/*(rewritten from/api/manage/*at the ingress) and share a singlevalidateAdminRequestpreHandler.Two auth modes for
/manageopenpanel-client-id/openpanel-client-secretheaders; requirestype=rootClient in the DB.Authorization: Bearer <jwt>against a configured issuer. Validated via JWKS discovery +jose. Required role configurable (ADMIN_OIDC_REQUIRED_ROLE, defaultopenpanel:admin). Synthesizes a request-scopedIServiceClientWithProjectso the controllers don't need to branch on auth source.Both modes produce a Client-shaped record with a single
organizationId. The OIDC mode reads the org from a configurable claim (ADMIN_OIDC_ORG_CLAIM, defaults to Zitadel'surn:zitadel:iam:user:resourceowner:id).Same-org scoping (with the
getOrganizationbug fixed)The earlier shape of
getOrganizationbuilt its Prismawherewith an object spread that always overrode the URL-paramidwithclient.organizationId. GET/manage/organizations/<any-id>silently returned the caller's own Org rather than 404'ing, andUPDATE/DELETEhad no scoping at all. Real-world consequence: a Crossplane Organization MR aimed at a brand-new Org tag silently renamed-then-deleted the platform-admin Org during reconcile.Fix: explicit
params.id === client.organizationIdchecks (with platform-admin bypass — see below), and a 409 onDELETE /manage/organizations/platform-adminso the sentinel can't be cascade-deleted out from under the operator.Platform-admin scope
Detects platform-admin callers by sentinel:
client.organizationId === 'platform-admin'. The openpanel-chart's bootstrap Job seeds that Organization row + root Client at install time.Platform-admin bypasses tenant scoping on Org handlers:
idmatches caller's orgiddirectlyidmatches caller's orgididmatches caller's orgid(except the sentinel itself → 409)Project / Client / Reference handlers stay strictly tenant-scoped. The flow for multi-tenant provisioning becomes: platform-admin creates the tenant Org via Crossplane → per-tenant chart install's bootstrap Job mints the tenant's first root Client scoped to the new Org → tenant credentials manage their own Projects / Clients / References.
Guardrails:
createOrganizationrefuses to mint an Org whoseslug(name) === 'platform-admin'. Without this the slug helper would silently produceplatform-admin-1459on the collision path, and the operator would never see the explicit 409 they probably meant to hit.deleteOrganizationrefuses to delete theplatform-adminOrg itself.Verification on pat-local
End-to-end via Crossplane:
platform-adminroot Client; chart pushes credentials to AWS Secrets Manager.provider-upjet-openpanel(generated from the OpenPanel Terraform provider via upjet) consumes those credentials via aProviderConfig.OrganizationMR (smoketest-attempt-3) reconciles toSynced=True, Ready=True, atProvider.id=smoketest-attempt-3— the new Org is real, NOT a silent rename of platform-admin.Direct curl against
/manage(7-step suite, 6/7 pass on 4719; 7/7 on 248c image once it lands):Known boundaries / follow-ups
organizationIdplumbing on Project/Client/Reference handlers + the matching TF provider schema additions + an upjet regen. Deliberately out of scope for this PR — the per-tenant chart install handles it cleanly today.synthesizeAdminClient(IServiceClientWithProjectschema picked upignoreCorsAndSecret, droppedcors); fixed in the same commit as the same-org bug fix because it was blockingpnpm typecheck.'platform-admin'), which is a convention from this fork's chart, not an upstream notion. For upstream we'd want a proper platform-admin role/claim in the data model.Test plan
pnpm --filter @openpanel/api typecheckis cleanopenpanel-bootstrap-rootSecret all survive MR deletefeat-admin-jwt-auth-248c🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores