feat(manage): opt-in OIDC admin auth + Organization CRUD endpoints#371
feat(manage): opt-in OIDC admin auth + Organization CRUD endpoints#371patrickleet wants to merge 2 commits into
Conversation
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.
📝 WalkthroughWalkthroughThis PR adds optional OIDC JWT Bearer token authentication for admin ChangesAdmin Organization Management
Sequence Diagram(s)sequenceDiagram
participant Request as HTTP Request
participant validateAdminRequest as validateAdminRequest()
participant validateAdminJwtRequest as validateAdminJwtRequest()
participant validateManageRequest as validateManageRequest()
participant JWKS as JWKS Discovery
participant TokenVerifier as jwt.verify()
participant Result as Scoped Client
Request->>validateAdminRequest: headers
alt Bearer token + OIDC enabled
validateAdminRequest->>validateAdminJwtRequest: Bearer token
validateAdminJwtRequest->>JWKS: fetch/cache keys
validateAdminJwtRequest->>TokenVerifier: verify issuer, audience, role, org claim
TokenVerifier->>Result: synthesized root client (org scoped)
else Fallback
validateAdminRequest->>validateManageRequest: openpanel credentials
validateManageRequest->>Result: existing client
end
classDiagram
class zCreateOrganization {
name: string (required)
timezone: string | null (optional)
}
class zUpdateOrganization {
name: string | null (optional)
timezone: string | null (optional)
}
class OrganizationHandlers {
listOrganizations() Organization[]
getOrganization(id) Organization
createOrganization(body) Organization
updateOrganization(id, body) Organization
deleteOrganization(id) void
}
zCreateOrganization <|-- OrganizationHandlers: input
zUpdateOrganization <|-- OrganizationHandlers: input
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 277-295: The handlers are incorrectly allowing cross-org access by
overwriting or ignoring request.params.id; update getOrganization to enforce
that request.params.id must equal request.client!.organizationId (do not replace
params.id with the client org id) and return a 403/404 when they differ, and
apply the same strict equality guard to updateOrganization and
deleteOrganization (ensure both functions check request.params.id ===
request.client!.organizationId before performing DB reads/updates/deletes and
bail out if not equal). Use the unique symbols getOrganization,
updateOrganization, deleteOrganization, request.params.id, and
request.client!.organizationId to locate and add the guard logic so callers
cannot act on other organizations.
In `@apps/api/src/utils/auth.ts`:
- Around line 319-327: When ADMIN_OIDC_ISSUER is set, require
ADMIN_OIDC_AUDIENCE to be present so jwtVerify will enforce aud; in
loadAdminOidcConfig check process.env.ADMIN_OIDC_AUDIENCE and if it's missing
either throw a clear error or return undefined and log an error instead of
returning a config with undefined audience; update the symmetric loader
mentioned in the comment (the other OIDC config loader around lines 439-441,
e.g., loadUserOidcConfig or the equivalent function) the same way so any enabled
issuer always mandates a non-empty audience.
- Around line 350-353: The discovery URL is being constructed with new
URL('/.well-known/openid-configuration', config.issuer) which drops any path
segment on config.issuer; update the logic that creates discoveryUrl so it
appends '/.well-known/openid-configuration' to the issuer value itself
(preserving any path component on config.issuer) — e.g., normalize config.issuer
to ensure it ends (or does not double) with a slash and then join the issuer +
'/.well-known/openid-configuration' to produce discoveryUrl; modify the code
that sets discoveryUrl (the variable named discoveryUrl that uses config.issuer)
to use this concatenation approach so path-based issuers like
https://example.com/tenant-1 resolve to
https://example.com/tenant-1/.well-known/openid-configuration.
🪄 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: 1aa889fd-fbad-47f7-99bd-c0fe8ba9a6eb
⛔ 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
| export async function getOrganization( | ||
| request: FastifyRequest<{ Params: { id: string } }>, | ||
| reply: FastifyReply | ||
| ) { | ||
| const org = await db.organization.findFirst({ | ||
| where: { | ||
| id: request.params.id, | ||
| // Same-org scoping. JWT-auth callers can only `get` the org | ||
| // their claim is bound to; cross-org reads require additional | ||
| // claim plumbing we haven't designed yet. | ||
| ...(request.client!.organizationId | ||
| ? { id: request.client!.organizationId } | ||
| : {}), | ||
| }, | ||
| }); | ||
| if (!org) { | ||
| throw new HttpError('Organization not found', { status: 404 }); | ||
| } | ||
| reply.send({ data: org }); |
There was a problem hiding this comment.
Gate organization reads and mutations on request.params.id === request.client!.organizationId.
getOrganization currently overwrites request.params.id with request.client!.organizationId, so /organizations/other-id can return the caller's own org instead of a 404. updateOrganization and deleteOrganization skip that check entirely, which lets any authenticated manage caller update or delete another organization by ID.
🛡️ Suggested guard
export async function getOrganization(
request: FastifyRequest<{ Params: { id: string } }>,
reply: FastifyReply
) {
+ if (request.params.id !== request.client!.organizationId) {
+ throw new HttpError('Organization not found', { status: 404 });
+ }
const org = await db.organization.findFirst({
- where: {
- id: request.params.id,
- ...(request.client!.organizationId
- ? { id: request.client!.organizationId }
- : {}),
- },
+ where: { id: request.params.id },
}); export async function updateOrganization(...) {
+ if (request.params.id !== request.client!.organizationId) {
+ throw new HttpError('Organization not found', { status: 404 });
+ }
const existing = await db.organization.findFirst({
where: { id: request.params.id },
}); export async function deleteOrganization(...) {
+ if (request.params.id !== request.client!.organizationId) {
+ throw new HttpError('Organization not found', { status: 404 });
+ }
const existing = await db.organization.findFirst({
where: { id: request.params.id },
});Also applies to: 320-360
🤖 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 277 - 295, The
handlers are incorrectly allowing cross-org access by overwriting or ignoring
request.params.id; update getOrganization to enforce that request.params.id must
equal request.client!.organizationId (do not replace params.id with the client
org id) and return a 403/404 when they differ, and apply the same strict
equality guard to updateOrganization and deleteOrganization (ensure both
functions check request.params.id === request.client!.organizationId before
performing DB reads/updates/deletes and bail out if not equal). Use the unique
symbols getOrganization, updateOrganization, deleteOrganization,
request.params.id, and request.client!.organizationId to locate and add the
guard logic so callers cannot act on other organizations.
| function loadAdminOidcConfig(): AdminOidcConfig | undefined { | ||
| const issuer = process.env.ADMIN_OIDC_ISSUER; | ||
| if (!issuer) { | ||
| return undefined; | ||
| } | ||
| return { | ||
| issuer: issuer.replace(/\/$/, ''), | ||
| audience: process.env.ADMIN_OIDC_AUDIENCE, | ||
| requiredRole: process.env.ADMIN_OIDC_REQUIRED_ROLE ?? DEFAULT_REQUIRED_ROLE, |
There was a problem hiding this comment.
Require ADMIN_OIDC_AUDIENCE whenever JWT admin auth is enabled.
ADMIN_OIDC_ISSUER alone currently enables bearer auth, but audience stays optional so jwtVerify skips aud enforcement entirely. That accepts any token from the issuer with the required role, even if it was minted for a different client/application.
🔐 Suggested guard
function loadAdminOidcConfig(): AdminOidcConfig | undefined {
const issuer = process.env.ADMIN_OIDC_ISSUER;
if (!issuer) {
return undefined;
}
+ const audience = process.env.ADMIN_OIDC_AUDIENCE;
+ if (!audience) {
+ throw new Error(
+ 'Admin OIDC: ADMIN_OIDC_AUDIENCE must be set when ADMIN_OIDC_ISSUER is configured',
+ );
+ }
return {
issuer: issuer.replace(/\/$/, ''),
- audience: process.env.ADMIN_OIDC_AUDIENCE,
+ audience,
requiredRole: process.env.ADMIN_OIDC_REQUIRED_ROLE ?? DEFAULT_REQUIRED_ROLE,Also applies to: 439-441
🤖 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 319 - 327, When ADMIN_OIDC_ISSUER is
set, require ADMIN_OIDC_AUDIENCE to be present so jwtVerify will enforce aud; in
loadAdminOidcConfig check process.env.ADMIN_OIDC_AUDIENCE and if it's missing
either throw a clear error or return undefined and log an error instead of
returning a config with undefined audience; update the symmetric loader
mentioned in the comment (the other OIDC config loader around lines 439-441,
e.g., loadUserOidcConfig or the equivalent function) the same way so any enabled
issuer always mandates a non-empty audience.
| const discoveryUrl = new URL( | ||
| '/.well-known/openid-configuration', | ||
| config.issuer, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/api/src/utils/auth.ts | sed -n '340,360p'Repository: Openpanel-dev/openpanel
Length of output: 781
🌐 Web query:
URL constructor JavaScript path handling base parameter behavior
💡 Result:
The JavaScript URL constructor resolves a relative URL against a base URL using standard URL resolution rules, which are not a simple string concatenation [1][2]. When you provide a base URL, the constructor treats the path component of that base as a directory structure [1][3]. Key behaviors include: 1. Path Resolution: The resolution process considers the current directory of the base URL, which includes all path segments up to the last forward slash (/) [3][2]. Any path segments after the last slash in the base URL are discarded [3][2]. 2. Relative Path Handling: - If the relative URL starts with a forward slash (/), it is treated as root-relative and resolves relative to the base origin (replacing the entire path of the base) [1][2]. - If the relative URL does not start with a slash, it is appended to the base URL's directory path [1][2]. - Relative references like../ or./ are resolved relative to the base URL's directory [1][2]. To ensure a path is appended correctly to a base URL, the base URL should end with a trailing slash, and the relative path should not start with a slash [4]. Example: // Base path "v1" is treated as a file, so it is stripped new URL("/items", "https://example.com/v1"); // Result: https://example.com/items // Adding a trailing slash preserves the "v1" directory new URL("items", "https://example.com/v1/"); // Result: https://example.com/v1/items If the first argument is an absolute URL, the base argument is ignored [3][2]. The constructor will throw a TypeError if the resulting URL is invalid [2][5].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/URL_API/Resolving_relative_references
- 2: https://developer.mozilla.org/en-US/docs/Web/API/URL/URL
- 3: https://developer.mozilla.org/en-US/docs/Web/API/URL/parse_static
- 4: https://stackoverflow.com/questions/76406748/constructing-url-with-base-url-strips-the-path-in-base-url
- 5: https://github.com/mdn/content/blob/main/files/en-us/web/api/url/url/index.md
🌐 Web query:
OIDC Connect Discovery path-based issuer specification
💡 Result:
In OpenID Connect (OIDC) Discovery 1.0, the Issuer Identifier is a case-sensitive URL using the https scheme that may include scheme, host, port, and path components, but must not include query or fragment components [1][2][3]. When an OpenID Provider (OP) supports discovery, it must make its configuration document available at a specific location derived from the Issuer Identifier [1]. The specification defines a precise concatenation rule to handle cases where the Issuer Identifier contains a path component [1][4]: 1. If the Issuer Identifier contains a path component, any terminating forward slash (/) must be removed [1][5]. 2. The string /.well-known/openid-configuration is then appended to the resulting URL [1][4]. For example, if the Issuer Identifier is https://example.com/tenant-1, the Relying Party (RP) must request the configuration document from https://example.com/tenant-1/.well-known/openid-configuration [6][4]. This path-based approach is particularly important for multi-tenant environments, where a single host may support multiple distinct issuers, each identified by a unique path [3]. The Issuer Identifier returned by discovery must exactly match the iss claim in any ID Tokens issued by that provider, and it must also match the base URL used to retrieve the configuration document [4][3]. Failure to adhere to these exact matching rules can lead to security vulnerabilities, such as one provider impersonating another [4].
Citations:
- 1: https://openid.net/specs/openid-connect-discovery-1_0.html
- 2: https://openid.net/specs/openid-connect-core-1_0.html
- 3: https://stackoverflow.com/questions/61686204/what-is-the-proper-issuer-of-an-oidc-id-token
- 4: https://dev.to/kanywst/openid-connect-discovery-10-deep-dive-ops-self-introduction-and-dynamic-configuration-retrieval-2ja9
- 5: https://shoepapp.com/?_=%2Fspecs%2Fopenid-connect-discovery-1_0.html%23ePsMxVOlbgzWxs9KzhwK93X7
- 6: https://openid.net/specs/openid-connect-discovery-1_0-23.html
Build the discovery URL from the issuer value instead of the origin root.
Using new URL('/.well-known/openid-configuration', config.issuer) treats the first argument as a root-relative path and discards any path component from the issuer. The OIDC Discovery specification requires path-based issuers (e.g., https://example.com/tenant-1) to have the well-known endpoint appended to the issuer value itself, resulting in https://example.com/tenant-1/.well-known/openid-configuration. The current code would incorrectly resolve to https://example.com/.well-known/openid-configuration for such providers.
🧭 Suggested fix
- const discoveryUrl = new URL(
- '/.well-known/openid-configuration',
- config.issuer,
- );
+ const discoveryUrl = new URL(
+ `${config.issuer.replace(/\/$/, '')}/.well-known/openid-configuration`,
+ );📝 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.
| const discoveryUrl = new URL( | |
| '/.well-known/openid-configuration', | |
| config.issuer, | |
| ); | |
| const discoveryUrl = new URL( | |
| `${config.issuer.replace(/\/$/, '')}/.well-known/openid-configuration`, | |
| ); |
🤖 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 is
being constructed with new URL('/.well-known/openid-configuration',
config.issuer) which drops any path segment on config.issuer; update the logic
that creates discoveryUrl so it appends '/.well-known/openid-configuration' to
the issuer value itself (preserving any path component on config.issuer) — e.g.,
normalize config.issuer to ensure it ends (or does not double) with a slash and
then join the issuer + '/.well-known/openid-configuration' to produce
discoveryUrl; modify the code that sets discoveryUrl (the variable named
discoveryUrl that uses config.issuer) to use this concatenation approach so
path-based issuers like https://example.com/tenant-1 resolve to
https://example.com/tenant-1/.well-known/openid-configuration.
|
Closing this from our side for now — we've decided to take a different approach for our platform (chart-side bootstrap of a root Client at install time, mirroring how we already handle Zitadel's iam-admin) which sidesteps the need for any auth-model change in OpenPanel. The branch lives on at https://github.com/hops-ops/openpanel-app/tree/pr/admin-jwt-auth in case anyone else wants to pick it up or use it as a reference for adding opt-in OIDC admin auth. Not asking for upstream review while we're still uncertain about the design — thanks for taking the time to even glance at it, and apologies for the noise. #370 (the user-facing OIDC sign-in provider) remains open separately — that one we are confident in and would still love your review on whenever you have a moment. |
1 participant
Adds two related capabilities to the existing `/manage` REST surface:
An opt-in OIDC Bearer-token auth path. When configured with an OIDC issuer, the api pod accepts `Authorization: Bearer ` headers as an alternative to the existing `openpanel-client-id` / `openpanel-client-secret` Client-pair auth. The token is validated against the issuer's JWKS (discovered from `/.well-known/openid-configuration`) and must carry a configured audience + role claim.
`/manage/organizations` REST endpoints for GET/POST/PATCH/DELETE. Mirrors the existing `/manage/projects` shape. Useful for both Client-pair callers (managing their own Org) and OIDC callers (provisioning Organizations programmatically).
Why
The existing `/manage` API requires a root-typed Client, which is only mintable through the dashboard UI after onboarding. That makes it impossible to bootstrap an OpenPanel install from a fully programmatic flow (Terraform, Crossplane, GitOps pipelines, etc.).
If a self-hoster has already configured an OIDC IdP for dashboard sign-in (Zitadel / Keycloak / Authentik / Okta), the same IdP can issue ServiceUser tokens that `/manage` validates — no second auth primitive inside OpenPanel, no Personal Access Token table to maintain, the IdP's existing token rotation / revocation / audit handles the operational story.
For installs without an OIDC IdP, nothing changes: the env vars are unset and the JWT auth path is disabled.
Configuration
Off by default. Enabled by setting these env vars on the api pod:
```
ADMIN_OIDC_ISSUER # e.g. https://auth.example.com
ADMIN_OIDC_AUDIENCE # JWT aud claim required (e.g. openpanel-admin)
ADMIN_OIDC_REQUIRED_ROLE # default: openpanel:admin
ADMIN_OIDC_ORG_CLAIM # claim that holds the bound Org ID
# default: urn:zitadel:iam:user:resourceowner:id
```
The role check looks at three common claim shapes so different IdPs work without per-IdP code:
Implementation
`apps/api/src/utils/auth.ts` adds:
`apps/api/src/routes/manage.router.ts` switches the preHandler from `validateManageRequest` to `validateAdminRequest` (backwards-compatible) and registers the new `/manage/organizations` routes.
`apps/api/src/controllers/manage.controller.ts` adds `listOrganizations`, `getOrganization`, `createOrganization`, `updateOrganization`, `deleteOrganization` matching the project controller pattern.
Backwards compatibility
Purely additive:
How to test
Local docker-compose setup with any OIDC IdP:
```
ADMIN_OIDC_ISSUER=http://
ADMIN_OIDC_AUDIENCE=openpanel-admin
```
I verified end-to-end against a self-hosted Zitadel install: ServiceUser provisioned via Zitadel's Crossplane provider, JWT obtained via client_credentials grant, `POST /manage/organizations` + subsequent `POST /manage/projects` + `POST /manage/clients` all succeed and appear in the OpenPanel UI under the freshly-created Org.
Companion to #370
This builds on top of #370 (the user-facing OIDC sign-in provider) but is independent — the two PRs touch different files and can land in either order.
Not included
Summary by CodeRabbit
Dependencies
joselibrary for secure JWT token verification and processingNew Features