fix(ui): hide OrganizationProfile Security tab when lacking manage enterprise connections permission#8971
Conversation
🦋 Changeset detectedLatest commit: 6b479e1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughOrganizationProfile now hides the Security tab for members without ChangesSecurity Tab Permission Gating
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsx (1)
540-557: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a direct-navigation test for the new security route guard.
This assertion only validates navbar/tab visibility. Please add a case that initializes routing to
organization-securitywithoutorg:sys_entconns:manageand verifies the security page content does not render, so the new route-level<Protect>behavior is covered too.As per coding guidelines, "
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality" and "Include tests for all new features."🤖 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 `@packages/ui/src/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsx` around lines 540 - 557, The current test only checks that the Security tab is hidden, but it does not cover the new route guard behavior. Add a direct-navigation test in OrganizationProfile.test.tsx that starts the router on the organization-security route without the org:sys_entconns:manage permission, then assert the security page content is not rendered; use the existing OrganizationProfile render setup and the route/protection behavior introduced around OrganizationProfile and Protect to locate the right test path.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.
Outside diff comments:
In
`@packages/ui/src/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsx`:
- Around line 540-557: The current test only checks that the Security tab is
hidden, but it does not cover the new route guard behavior. Add a
direct-navigation test in OrganizationProfile.test.tsx that starts the router on
the organization-security route without the org:sys_entconns:manage permission,
then assert the security page content is not rendered; use the existing
OrganizationProfile render setup and the route/protection behavior introduced
around OrganizationProfile and Protect to locate the right test path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4f69c249-aa9e-4dfa-a31b-dffc15bee231
📒 Files selected for processing (5)
.changeset/hide-security-tab-without-permission.mdpackages/ui/src/components/OrganizationProfile/OrganizationProfileNavbar.tsxpackages/ui/src/components/OrganizationProfile/OrganizationProfileRoutes.tsxpackages/ui/src/components/OrganizationProfile/OrganizationSecurityPage.tsxpackages/ui/src/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsx
iagodahlem
force-pushed
the
iago/orgs-1676-hide-security-tab
branch
from
2472ddf to
0109697
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.changeset/olive-pans-sin.md:
- Around line 1-6: Remove the `@clerk/clerk-js` package from the changeset file.
The PR changes are scoped exclusively to `@clerk/ui` components and do not
affect `@clerk/clerk-js`, so only `@clerk/ui` should be listed with a patch
version bump in the changeset. Delete the line containing `'`@clerk/clerk-js`':
patch` and keep only the `@clerk/ui` entry.
🪄 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: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 6f402220-e8b7-4390-bb60-aa60a21fd6fd
📒 Files selected for processing (1)
.changeset/olive-pans-sin.md
iagodahlem
force-pushed
the
iago/orgs-1676-hide-security-tab
branch
from
714ea02 to
6b479e1
Compare
2 participants
Summary
ConfigureSSOis embedded in<OrganizationProfile />→ Security tab, which shows when self-serve SSO is enabled at the org level. A member without theorg:sys_entconns:managepermission previously landed on the Security tab and hit a missing-permission state that was built as a wizard step, so it rendered wizard chrome (header/footer) that doesn't fit someone who never entered a flow.Since SSO is currently the only section in Security, this hides the Security tab entirely when the current member lacks
org:sys_entconns:manage— matching how the Members, Billing, and API keys tabs are permission-gated.Changes
OrganizationProfileNavbar.tsx— filter out the Security nav item viauseProtect(has => has({ permission: 'org:sys_entconns:manage' })), mirroring the existing Members/Billing filters.OrganizationProfileRoutes.tsx— wrap the Security route in<Protect>so a direct navigation is gated too (matches the Billing/API keys routes).OrganizationSecurityPage.tsx— drop the now-redundant page-levelConfigureSSOProtectwrapper; the route guard covers it.ConfigureSSOProtectstays in place for the standalone<ConfigureSSO />component.Follow-up
When the Security page gains non-SSO settings, the permission-error state should be redesigned to scope to the SSO section only (so the tab can stay visible for other settings). Tracked separately.
Resolves ORGS-1676.
Summary by CodeRabbit
org:sys_entconns:manageis granted.