feat(studio): role-aware access feedback in scoped token creation - #48858
Merged
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
Contributor
📝 WalkthroughWalkthroughChangesScoped token access flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant NewScopedTokenForm
participant useTokenAccessEvaluation
participant PermissionsAccordion
participant NewScopedTokenFormReview
NewScopedTokenForm->>useTokenAccessEvaluation: organization and project selections
useTokenAccessEvaluation-->>NewScopedTokenForm: TokenAccessEvaluation
NewScopedTokenForm->>PermissionsAccordion: access evaluation
NewScopedTokenForm->>NewScopedTokenFormReview: access evaluation
PermissionsAccordion-->>NewScopedTokenForm: permission access indicators
NewScopedTokenFormReview-->>NewScopedTokenForm: capability and role warnings
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/NewScopedTokenFormReview.tsx (1)
45-53: 📐 Maintainability & Code Quality | 🔵 TrivialDerive these local values during render.
groupFailingResources()andcomputeOverallRisk()feed this component directly. No supplied consumer requires stable references. Remove theseuseMemocalls unless profiling shows a material cost.As per coding guidelines, “Do not use
useMemooruseCallbackby default; use them only for measured expense or referential stability required by a memoized child.”[low_effort_and-low_reward]
🤖 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/studio/components/interfaces/Account/AccessTokens/Scoped/Form/NewScopedTokenFormReview.tsx` around lines 45 - 53, Remove the useMemo wrappers around exceedingByResource and risk in NewScopedTokenFormReview, deriving both values directly during render by calling groupFailingResources and computeOverallRisk. Preserve their existing inputs and resulting behavior, and remove any now-unused useMemo import.Source: Coding guidelines
apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.test.tsx (1)
18-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the module type cast with a typed
importOriginalcall for string paths.Vitest can infer the module types from a dynamic import like
vi.mock(import('common'), ...). With the current string path, the returned value is untyped, so use either the dynamic import or a typed helper instead ofas typeof import('common').🤖 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/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.test.tsx` around lines 18 - 20, Update the vi.mock call around useIsLoggedIn to use a typed dynamic import path or Vitest’s typed helper for importOriginal, and remove the explicit as typeof import('common') cast. Preserve the existing behavior of spreading the actual common module and overriding useIsLoggedIn to return true.Source: Coding guidelines
🤖 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/studio/components/interfaces/Account/AccessTokens/Scoped/Form/NewScopedTokenFormReview.tsx`:
- Around line 55-82: Update the resourceSummary logic in
NewScopedTokenFormReview to iterate over values.projectRefs and
values.organizationSlugs rather than filtered query results, preserving every
selected resource in the review. Resolve each resource from projects or
organizations when available, fall back to its ref or slug as the label when
missing, and set isInaccessible for unresolved entries so ResourceSummaryItem
can indicate them.
In
`@apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.test.tsx`:
- Around line 45-68: Update mockPermissionsApi and both organization-selection
tests in ResourceAccessStep.test.tsx to expose an awaitable signal for the
/platform/profile/permissions MSW request. Await that signal after configuring
the mock and before opening the organization selector or asserting option state,
ensuring assertions run only after permissions have loaded.
In
`@apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.tsx`:
- Around line 94-106: Update the form synchronization around
projectScopedOrgSlugs so that when permissions resolve, any restricted
organization slugs are removed from the existing organizationSlugs value and the
field is revalidated. Preserve unrestricted selections, avoid changing values
while permissions are still undefined, and add coverage for selecting an
organization before the permissions response resolves.
---
Nitpick comments:
In
`@apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/NewScopedTokenFormReview.tsx`:
- Around line 45-53: Remove the useMemo wrappers around exceedingByResource and
risk in NewScopedTokenFormReview, deriving both values directly during render by
calling groupFailingResources and computeOverallRisk. Preserve their existing
inputs and resulting behavior, and remove any now-unused useMemo import.
In
`@apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.test.tsx`:
- Around line 18-20: Update the vi.mock call around useIsLoggedIn to use a typed
dynamic import path or Vitest’s typed helper for importOriginal, and remove the
explicit as typeof import('common') cast. Preserve the existing behavior of
spreading the actual common module and overriding useIsLoggedIn to return true.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b6f4e4ac-b8c3-499f-a27a-443fa5d50023
📒 Files selected for processing (6)
apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/NewScopedTokenForm.tsxapps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/NewScopedTokenFormReview.tsxapps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/PermissionRow.tsxapps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/PermissionsAccordion.tsxapps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.test.tsxapps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.tsx
Comment on lines
55
to
82
| const resourceSummary = useMemo(() => { | ||
| if (values.resourceAccess === 'project') { | ||
| const selectedProjects = projects.filter((p) => values.projectRefs.includes(p.ref)) | ||
| return { | ||
| title: 'Project', | ||
| items: selectedProjects.length > 0 ? selectedProjects.map((p) => p.name) : ['-'], | ||
| title: 'Projects', | ||
| items: | ||
| selectedProjects.length > 0 | ||
| ? selectedProjects.map((p) => ({ key: p.ref, label: p.name, sublabel: p.ref })) | ||
| : [{ key: 'none', label: '-', sublabel: undefined }], | ||
| } | ||
| } | ||
| if (values.resourceAccess === 'organization') { | ||
| const selectedOrganizations = organizations.filter((o) => | ||
| values.organizationSlugs.includes(o.slug) | ||
| ) | ||
| return { | ||
| title: 'Organization', | ||
| items: selectedOrganizations.length > 0 ? selectedOrganizations.map((o) => o.name) : ['-'], | ||
| title: 'Organizations', | ||
| items: | ||
| selectedOrganizations.length > 0 | ||
| ? selectedOrganizations.map((o) => ({ key: o.slug, label: o.name, sublabel: o.slug })) | ||
| : [{ key: 'none', label: '-', sublabel: undefined }], | ||
| } | ||
| } | ||
| return { title: 'Account', items: ['Account-level access'] } | ||
| return { | ||
| title: 'Account', | ||
| items: [{ key: 'account', label: 'Account-level access', sublabel: undefined }], | ||
| } | ||
| }, [values, projects, organizations]) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep each selected resource in the review summary.
filter() drops selected projects or organizations that are no longer returned by the resource query. The fallback then renders -, but NewScopedTokenForm.tsx line 219 still submits the original formValues. The review can therefore omit resources that the token creation request targets.
Build summary items from values.projectRefs or values.organizationSlugs. Use the ID as a fallback label when lookup fails. Pass isInaccessible to ResourceSummaryItem for missing resources.
Also applies to: 112-115
🤖 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/studio/components/interfaces/Account/AccessTokens/Scoped/Form/NewScopedTokenFormReview.tsx`
around lines 55 - 82, Update the resourceSummary logic in
NewScopedTokenFormReview to iterate over values.projectRefs and
values.organizationSlugs rather than filtered query results, preserving every
selected resource in the review. Resolve each resource from projects or
organizations when available, fall back to its ref or slug as the label when
missing, and set isInaccessible for unresolved entries so ResourceSummaryItem
can indicate them.
Comment on lines
+45
to
+68
| test('disables organizations where the user only has project-level access', async () => { | ||
| mockPermissionsApi(readonlyRows(MOCK_ORG.slug, [MOCK_PROJECT.ref])) | ||
| await openOrganizationSelector() | ||
|
|
||
| const option = await screen.findByRole('option', { name: new RegExp(MOCK_ORG.name) }) | ||
| expect(option).toHaveAttribute('aria-disabled', 'true') | ||
| expect( | ||
| await screen.findByText( | ||
| 'Your access is limited to specific projects. Create a project-scoped token instead.' | ||
| ) | ||
| ).toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('keeps organizations selectable for members with org-wide access', async () => { | ||
| mockPermissionsApi(readonlyRows(MOCK_ORG.slug)) | ||
| await openOrganizationSelector() | ||
|
|
||
| const option = await screen.findByRole('option', { name: new RegExp(MOCK_ORG.name) }) | ||
| expect(option).not.toHaveAttribute('aria-disabled', 'true') | ||
| expect( | ||
| screen.queryByText( | ||
| 'Your access is limited to specific projects. Create a project-scoped token instead.' | ||
| ) | ||
| ).toBeNull() |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline apps/studio/components/interfaces/Account/AccessTokens/Scoped/AccessToken.fixtures.ts --items all
rg -n -C 5 '\bmockPermissionsApi\b|MSW|http\.(get|post)|delay\(' \
apps/studio/components/interfaces/Account/AccessTokens/Scoped/AccessToken.fixtures.tsRepository: supabase/supabase
Length of output: 545
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Candidate files:"
fd -a 'ResourceAccessStep\.tsx|ResourceAccessStep\.test\.tsx|AccessToken\.fixtures\.ts|AccessTokens' apps/studio 2>/dev/null | sed 's#^\./##' | head -80
echo
echo "Locate mockPermissionsApi:"
rg -n -C 4 '\bmockPermissionsApi\b' apps/studio/components/interfaces apps/studio 2>/dev/null | head -200
echo
echo "Locate ResourceAccessStep.tsx:"
fd 'ResourceAccessStep\.tsx$' apps/studio 2>/dev/nullRepository: supabase/supabase
Length of output: 21620
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "ResourceAccessStep.tsx relevant sections:"
wc -l apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.tsx
sed -n '1,180p' apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.tsx
echo
echo "ResourceAccessStep.test.tsx:"
wc -l apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.test.tsx
sed -n '1,120p' apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.test.tsx
echo
echo "AccessToken.fixtures.ts mock implementations:"
wc -l apps/studio/components/interfaces/Account/AccessTokens/AccessToken.fixtures.ts
sed -n '1,180p' apps/studio/components/interfaces/Account/AccessTokens/AccessToken.fixtures.ts
echo
echo "Find addAPIMock definition:"
rg -n -C 8 'export .*addAPIMock|function addAPIMock|const addAPIMock' apps/studio packages 2>/dev/null | head -200Repository: supabase/supabase
Length of output: 14844
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "ResourceAccessStep organization/project selector sections:"
sed -n '180,305p' apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.tsx
echo
echo "useOrgAndProjectData implementation:"
rg -n -C 15 'useOrgAndProjectData|export function useOrgAndProjectData|export const useOrgAndProjectData' apps/studio/components/interfaces/Account apps/studio -g '*.[tj]s' -g '*.[tj]sx' | head -220
echo
echo "addAPIMock definition in full file:"
wc -l apps/studio/tests/lib/msw.ts
sed -n '1,180p' apps/studio/tests/lib/msw.tsRepository: supabase/supabase
Length of output: 35485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "permissions-query implementation:"
rg -n -C 15 'export .*usePermissionsQuery|function usePermissionsQuery|const usePermissionsQuery|usePermissionsQuery' apps/studio -g '*.[tj]s' -g '*.[tj]sx' | head -240
echo
echo "permissions directory:"
fd . apps/studio/data/permissions -t f -x sh echo '--- $1 ---; cat -n "$1"' sh {}
echo
echo "Query/React Query version hints:"
rg -n -C 3 '"`@tanstack/react-query`"|react-query|useQuery' package.json apps/studio/package.json pnpm-lock.yaml package-lock.json yarn.lock 2>/dev/null | head -160
echo
echo "React Query import usages for permissions:"
rg -n "from '(`@tanstack/react-query`|react-query')" apps/studio 2>/dev/null | head -60Repository: supabase/supabase
Length of output: 18648
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "permissions-query.ts full relevant file:"
cat -n apps/studio/data/permissions/permissions-query.ts
echo
echo "permissions-query test/fixture references:"
rg -n -C 4 'mockPermissionsQuery|invalidatePermissionsQuery|/platform/profile/permissions|permissionKeys|data permissions' apps/studio 2>/dev/null | head -240
echo
echo "MSW test wait helpers:"
fd . apps/studio/tests/lib apps/studio/tests -t f | xargs rg -n "waitFor|screen.findBy|addAPIMock|waitForRequest|request/wait" 2>/dev/null | head -200
echo
echo "permissions-query imports:"
rg -n "import .*permissions-query|usePermissionsQuery" apps/studio/components/interfaces/Account/AccessTokens/Scoped apps/studio/components/interfaces/Account/AccessTokens/hooks 2>/dev/null | head -120
echo
echo "Scoped token sheet/form outline/data flow:"
fd -a 'NewScopedTokenSheet\.tsx|NewScopedTokenForm\.utils\.ts|NewScopedTokenFormReview\.tsx' apps/studio/components/interfaces/Account/AccessTokens/Scoped -t f -x sh 'echo "--- $1 ---; sed -n 1,220p $1"' sh {}Repository: supabase/supabase
Length of output: 48703
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "NewScopedTokenSheet.tsx:"
cat -n apps/studio/components/interfaces/Account/AccessTokens/Scoped/NewScopedTokenSheet.tsx | sed -n '1,240p'
echo
echo "useTokenAccessEvaluation.ts implementation:"
cat -n apps/studio/components/interfaces/Account/AccessTokens/hooks/useTokenAccessEvaluation.ts
echo
echo "NewScopedTokenReview permissions usage:"
cat -n apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/NewScopedTokenFormReview.tsx | sed -n '130,220p'
echo
echo "Permission rows / project refs tests:"
cat -n apps/studio/components/interfaces/Account/AccessTokens/AccessToken.roles.test.ts | sed -n '70,115p'Repository: supabase/supabase
Length of output: 16148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "NewScopedTokenForm.tsx:"
cat -n apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/NewScopedTokenForm.tsx | sed -n '1,240p'
echo
echo "ResourceAccessStep component relevant section with organization selector and permissions:"
sed -n '60,240p' apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.tsx | cat -nRepository: supabase/supabase
Length of output: 18123
Wait for /platform/profile/permissions before asserting organization-selection state.
Both tests add the permission mock and then render the dialog in the same tick, but ResourceAccessStep sets opt-out org-disabling to disabled while permissions are undefined, so unchecked options can pass before mockPermissionsApi() completes. Expose an awaitable MSW request signal from the mock and wait for it before opening the org selector and checking enabled/disabled state.
🤖 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/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.test.tsx`
around lines 45 - 68, Update mockPermissionsApi and both organization-selection
tests in ResourceAccessStep.test.tsx to expose an awaitable signal for the
/platform/profile/permissions MSW request. Await that signal after configuring
the mock and before opening the organization selector or asserting option state,
ensuring assertions run only after permissions have loaded.
Comment on lines
+94
to
+106
| // Users invited to specific projects (rather than the whole org) can't select that org for an | ||
| // org-wide token. Skipped while permissions are still loading so nothing gets disabled by | ||
| // mistake. The project list itself needs no permission filter — /platform/projects is already | ||
| // scoped server-side to what the user can access. | ||
| const { data: permissions } = usePermissionsQuery() | ||
| const projectScopedOrgSlugs = useMemo(() => { | ||
| if (permissions === undefined) return new Set<string>() | ||
| return new Set( | ||
| organizations | ||
| .map((org) => org.slug) | ||
| .filter((slug) => getIsProjectScopedOnly(permissions, slug)) | ||
| ) | ||
| }, [permissions, organizations]) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove restricted selections after permissions load.
Lines 98-105 leave every organization selectable while the permission query is pending. A user can select a restricted organization during that interval. Lines 276-295 only disable the option after resolution. They do not remove the existing organizationSlugs value.
Synchronize the form value when projectScopedOrgSlugs changes. Clear restricted organization selections and validate the field again. Add coverage for selecting an organization before the permissions response resolves.
🤖 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/studio/components/interfaces/Account/AccessTokens/Scoped/Form/ResourceAccessStep.tsx`
around lines 94 - 106, Update the form synchronization around
projectScopedOrgSlugs so that when permissions resolve, any restricted
organization slugs are removed from the existing organizationSlugs value and the
field is revalidated. Preserve unrestricted selections, avoid changing values
while permissions are still undefined, and add coverage for selecting an
organization before the permissions response resolves.
Contributor
🎭 Playwright Test Results (tanstack)
Details
Skipped testsFeatures › auth-users.spec.ts › should show web3 users as enabled when the matching web3 provider is enabled |
awaseem
approved these changes
Aug 7, 2026
Contributor
Braintrust eval report
|
hunleyd
added a commit
that referenced
this pull request
Aug 8, 2026
…for-pitr-archiving-stale * origin/master: (342 commits) feat(studio): role-aware access feedback in scoped token creation (#48858) Initialize notebook editor page (#48842) test(docs): scan changed pages for WCAG 2.1 A/AA in warn mode (#48727) docs(auth): clarify audit logs storage options and configuration (#48852) feat(studio): warn that scoped tokens don't support the MCP server (#48849) feat(studio): lifecycle and role-aware scoped token view sheet (#48848) feat(www): add Open Authorization Integration Addendum (#48804) feat(studio): add legacy token mode to scoped pat creation flow (#48844) Add blog post: Supabase is now a connector on Perplexity Computer (#48776) feat(studio): enforce expiry scoped pat (#48811) fix(studio): model scoped pat permissions as OR-of-AND alternatives - smaller version (#48809) fix(ui): opaque default button hover fills (#48837) fix(ui): restore normal alert text wrapping (#48840) feat: notebook content schema (#48813) fix(ui): remove extra spacing before custom reports section (#48796) feat(marketing): add hint text below Go page form labels (#48824) Initialize notebooks store (#48801) Use white wordmark for QA.tech primary logo (#48827) fix titleless Admonition alignment (#48784) fix inconsistent product menu dividers (#48787) ...
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
2 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.
I have read the CONTRIBUTING.md file.
YES
What kind of change does this PR introduce?
Remaining bits of #48714
Summary by CodeRabbit
New Features
Documentation
Bug Fixes