Skip to content

feat(portal, permissions): add portal operator URNs - #7377

Open
mcstepp wants to merge 15 commits into
mainfrom
eng-3149-finish-portal-urn-permissions-before-the-root-key-migration
Open

feat(portal, permissions): add portal operator URNs #7377
mcstepp wants to merge 15 commits into
mainfrom
eng-3149-finish-portal-urn-permissions-before-the-root-key-migration

Conversation

@mcstepp

@mcstepp mcstepp commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes ENG-3149

Portal routes only understood legacy dotted tuples, so when root keys move to URNs all five of them break. This adds portal to the URN catalog and makes it actually work.

  • portals.project_id, new and non-nullable. A portal's canonical name is projects/{project_id}/portals/{portal_id}, so it needs a project. Always the project of the app or keyspace it maps to.
  • All five routes accept either vocabulary, URN or legacy tuple, until callers migrate.
  • Session minting is now root-key only. Dashboard tokens used to be refused by accident, because the two permission vocabularies never met. Now that stage 1 evaluates URNs, the admin role's workspace-wide grant covers the session URN, so the credential check is the only thing left. A dashboard admin who could mint would get a URL that authenticates as any end user, which is the impersonation dashboard feature we have deferred.

Decisions

Project-wide grants now reach portals, including session minting, because Covers strips a trailing double wildcard and prefix-matches. Nothing had to name portals for that. No root key carries a URN today, so this only affects grants the migration creates.

Cross-project re-points now 412. A portal's URN names its project, so changing it would move the portal under a different set of grants while keeping its id. Not an edge case: keyspaces land in the default project and apps take a named one, so app to keyspace re-points nearly always cross.

WorkOS Dashboard developer and viewer roles gained portal permissions. Before this, only admin could manage portals, via a hardcoded *# string. Added projects/*/portals/* read/write/delete to developer and read to viewer. Can easily change this if we want to refine dashboard permissions a bit more.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

All automated. The suites that matter:

mise exec -- rask ./svc/api/routes/v2_portal_create_session  # the credential restriction
mise exec -- rask ./pkg/rbac                                 # the legacy to URN translation
mise exec -- rask ./pkg/auth/workos                          # roles manage portals, don't mint
mise exec -- rask ./svc/api/internal/portal                  # project resolution

Tests that encode a decision rather than just coverage:

  • TestCreateSessionRefusesDashboardTokens and TestDashboardTokensStillManagePortals
  • TestProjectWideGrantsDeliberatelyReachPortals
  • TestCreateSessionCeilingInheritsRerollURNWeakness, a weakness inherited from the reroll route, which maps its create and encryption arms to the same canonical key-write leaf. Mirroring it is intentional: a stricter ceiling would refuse mints the operator endpoint allows.
  • 412_test.go, the cross-project refusal

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Internal Workflow Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Ran mise run fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues
  • If I made a visual change: Filled out the "Screenshots / recordings" section

Appreciated

  • Updated the Unkey Docs if changes were necessary

@linear-code

linear-code Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

ENG-3149

@mintlify

mintlify Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
engineering 🟢 Ready View Preview Sep 11, 2026, 4:37 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

@vercel

vercel Bot commented Sep 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
dashboard Ready Ready Preview Sep 11, 2026 4:40pm UTC
design Ready Ready Preview Sep 11, 2026 4:40pm UTC

Request Review

Comment thread web/internal/db/src/schema/logdrains.ts
Comment thread pkg/db/queries/portal_update.sql Outdated
// from.
func seedAppWithKeyspaces(t *testing.T, h *testutil.Harness, workspaceID, slugBase string, keyspaceIDs []string) seededApp {
// appMapping addresses a portal at an app rather than a keyspace.
func appMapping(appID string) portal.Mapping {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this util doesn't save LOCs and just adds abstractions, should we remove it?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's to save width, not lines. i copied it from keyspaceMapping in the other portal tests, but that seeds an actual api and this one is just a struct literal with a name on it. if that's the part that's weird to you, I'll inline it instead

Comment thread svc/api/routes/v2_portal_create_session/handler.go Outdated
Comment on lines +455 to +460
// This is the only control standing between a dashboard token and minting: the
// WorkOS admin role grants unkey:v1:{workspace}:**#*, which covers the portal
// session URN stage 1 evaluates, so without this any workspace admin could mint
// a session authenticating as an arbitrary end user. Naming every enum member
// keeps a future credential type failing the exhaustive linter until someone
// decides whether it may mint.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't a workspace admin also create a root key with the required permissions and just mint sessions?

what does this protect against?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, an admin can make a root key with the permission and mint with it. this isn't trying to stop that.

what it stops is the browser credential being a mint credential. a dashboard JWT is just sitting in the tab, so without this an XSS on the dashboard mints impersonation URLs for any end user immediately and leaves nothing behind. with it you have to create a root key first, which is deliberate and audit logged.

we're being very explicit about only a root key can be used to mint a session because minting from the dashboard is what we're calling "impersonation."

// CanonicalScopeQueries returns the canonical form of the requirement
// [ScopeQueries] expresses in legacy tuples, for one scope on one keyspace.
//
// The ok flag is load-bearing: rbac.And over zero children evaluates to valid,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ok flag is load-bearing

:panic-intensifies:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok but jokes aside, I have no idea what this entire comment is trying to tell me

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it a question about the comment itself, or the intent of the comment?

because what it means is that rbac.And with no children returns true. it looks through the children for a failure, doesn't find one, passes (pkg/rbac/rbac.go:88).

So if I drop the ok flag and always add the URN check, a scope that has no URN form adds an empty And to the Or. that passes, so the whole check for that keyspace passes, and the caller needs no permission at all.

"load bearing" is just saying "don't delete this, it looks pointless and isn't."

Comment thread svc/api/routes/v2_portal_create_session/handler.go Outdated
Comment thread svc/api/routes/v2_portal_create_session/handler.go Outdated
Comment thread svc/api/routes/v2_portal_create_session/handler.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants