Skip to content

Commit

Permalink
fix(clerk-js): Hide Members page of OrgProfile if user doesn't have a…
Browse files Browse the repository at this point in the history
…ny member related permissions (#2138)

* fix(clerk-js): Hide Members page of OrgProfile if user doesn't have any member related permissions

* fix(clerk-js): Address pr comments
  • Loading branch information
panteliselef authored Nov 22, 2023
1 parent 5e41845 commit 9955938
Show file tree
Hide file tree
Showing 11 changed files with 173 additions and 34 deletions.
5 changes: 5 additions & 0 deletions .changeset/rich-actors-cross.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Hide members page of <OrganizationProfile/> if user doesn't have any membership related permissions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ export const OrganizationMembers = withCardStateProvider(() => {
const { organizationSettings } = useEnvironment();
const card = useCardState();
const { isAuthorizedUser: canManageMemberships } = useGate({ permission: 'org:sys_memberships:manage' });
const { isAuthorizedUser: canReadMemberships } = useGate({ permission: 'org:sys_memberships:read' });
const isDomainsEnabled = organizationSettings?.domains?.enabled;
const { membershipRequests } = useCoreOrganization({
membershipRequests: isDomainsEnabled || undefined,
});

// @ts-expect-error
// @ts-expect-error This property is not typed. It is used by our dashboard in order to render a billing widget.
const { __unstable_manageBillingUrl } = useOrganizationProfileContext();

if (canManageMemberships === null) {
Expand Down Expand Up @@ -55,7 +56,9 @@ export const OrganizationMembers = withCardStateProvider(() => {
</Header.Root>
<Tabs>
<TabsList>
<Tab localizationKey={localizationKeys('organizationProfile.membersPage.start.headerTitle__members')} />
{canReadMemberships && (
<Tab localizationKey={localizationKeys('organizationProfile.membersPage.start.headerTitle__members')} />
)}
{canManageMemberships && (
<Tab
localizationKey={localizationKeys('organizationProfile.membersPage.start.headerTitle__invitations')}
Expand All @@ -68,18 +71,20 @@ export const OrganizationMembers = withCardStateProvider(() => {
)}
</TabsList>
<TabPanels>
<TabPanel sx={{ width: '100%' }}>
<Flex
gap={4}
direction='col'
sx={{
width: '100%',
}}
>
{canManageMemberships && __unstable_manageBillingUrl && <MembershipWidget />}
<ActiveMembersList />
</Flex>
</TabPanel>
{canReadMemberships && (
<TabPanel sx={{ width: '100%' }}>
<Flex
gap={4}
direction='col'
sx={{
width: '100%',
}}
>
{canManageMemberships && __unstable_manageBillingUrl && <MembershipWidget />}
<ActiveMembersList />
</Flex>
</TabPanel>
)}
{canManageMemberships && (
<TabPanel sx={{ width: '100%' }}>
<OrganizationMembersTabInvitations />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React from 'react';

import { useGate } from '../../common';
import { ORGANIZATION_PROFILE_NAVBAR_ROUTE_ID } from '../../constants';
import { useCoreOrganization, useOrganizationProfileContext } from '../../contexts';
import { Breadcrumbs, NavBar, NavbarContextProvider, OrganizationPreview } from '../../elements';
import type { PropsOfComponent } from '../../styledSystem';
Expand All @@ -10,6 +12,17 @@ export const OrganizationProfileNavbar = (
const { organization } = useCoreOrganization();
const { pages } = useOrganizationProfileContext();

const { isAuthorizedUser: allowMembersRoute } = useGate({
some: [
{
permission: 'org:sys_memberships:read',
},
{
permission: 'org:sys_memberships:manage',
},
],
});

if (!organization) {
return null;
}
Expand All @@ -24,7 +37,11 @@ export const OrganizationProfileNavbar = (
sx={t => ({ margin: `0 0 ${t.space.$4} ${t.space.$2}` })}
/>
}
routes={pages.routes}
routes={pages.routes.filter(
r =>
r.id !== ORGANIZATION_PROFILE_NAVBAR_ROUTE_ID.MEMBERS ||
(r.id === ORGANIZATION_PROFILE_NAVBAR_ROUTE_ID.MEMBERS && allowMembersRoute),
)}
contentRef={props.contentRef}
/>
{props.children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,12 @@ export const OrganizationProfileRoutes = (props: PropsOfComponent<typeof Profile
</Gate>
</Route>
<Route index>
<OrganizationMembers />
<Gate
some={[{ permission: 'org:sys_memberships:read' }, { permission: 'org:sys_memberships:manage' }]}
redirectTo='./organization-settings'
>
<OrganizationMembers />
</Gate>
</Route>
</Switch>
</Route>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('OrganizationMembers', () => {
f.withOrganizations();
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [{ name: 'Org1', permissions: [] }],
organization_memberships: [{ name: 'Org1', permissions: ['org:sys_memberships:read'] }],
});
});

Expand All @@ -85,6 +85,26 @@ describe('OrganizationMembers', () => {
});
});

it('does not show members tab or navbar route if user is lacking permissions', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withOrganizations();
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [{ name: 'Org1', permissions: [] }],
});
});

fixtures.clerk.organization?.getRoles.mockRejectedValue(null);

const { queryByRole } = render(<OrganizationMembers />, { wrapper });

await waitFor(() => {
expect(queryByRole('tab', { name: 'Members' })).not.toBeInTheDocument();
expect(queryByRole('tab', { name: 'Invitations' })).not.toBeInTheDocument();
expect(queryByRole('tab', { name: 'Requests' })).not.toBeInTheDocument();
});
});

it('navigates to invite screen when user clicks on Invite button', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withOrganizations();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,24 @@ describe('OrganizationProfile', () => {
expect(getByText('Custom1')).toBeDefined();
expect(getByText('ExternalLink')).toBeDefined();
});

it('removes member nav item if user is lacking permissions', async () => {
const { wrapper } = await createFixtures(f => {
f.withOrganizations();
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [
{
name: 'Org1',
permissions: [],
},
],
});
});

const { queryByText } = render(<OrganizationProfile />, { wrapper });
expect(queryByText('Org1')).toBeInTheDocument();
expect(queryByText('Members')).not.toBeInTheDocument();
expect(queryByText('Settings')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ export const OrganizationMembers = withCardStateProvider(() => {
const { organizationSettings } = useEnvironment();
const card = useCardState();
const { isAuthorizedUser: canManageMemberships } = useGate({ permission: 'org:sys_memberships:manage' });
const { isAuthorizedUser: canReadMemberships } = useGate({ permission: 'org:sys_memberships:read' });
const isDomainsEnabled = organizationSettings?.domains?.enabled;
const { membershipRequests } = useCoreOrganization({
membershipRequests: isDomainsEnabled || undefined,
});

// @ts-expect-error
// @ts-expect-error This property is not typed. It is used by our dashboard in order to render a billing widget.
const { __unstable_manageBillingUrl } = useOrganizationProfileContext();

if (canManageMemberships === null) {
Expand Down Expand Up @@ -55,7 +56,9 @@ export const OrganizationMembers = withCardStateProvider(() => {
</Header.Root>
<Tabs>
<TabsList>
<Tab localizationKey={localizationKeys('organizationProfile.membersPage.start.headerTitle__members')} />
{canReadMemberships && (
<Tab localizationKey={localizationKeys('organizationProfile.membersPage.start.headerTitle__members')} />
)}
{canManageMemberships && (
<Tab
localizationKey={localizationKeys('organizationProfile.membersPage.start.headerTitle__invitations')}
Expand All @@ -68,18 +71,20 @@ export const OrganizationMembers = withCardStateProvider(() => {
)}
</TabsList>
<TabPanels>
<TabPanel sx={{ width: '100%' }}>
<Flex
gap={4}
direction='col'
sx={{
width: '100%',
}}
>
{canManageMemberships && __unstable_manageBillingUrl && <MembershipWidget />}
<ActiveMembersList />
</Flex>
</TabPanel>
{canReadMemberships && (
<TabPanel sx={{ width: '100%' }}>
<Flex
gap={4}
direction='col'
sx={{
width: '100%',
}}
>
{canManageMemberships && __unstable_manageBillingUrl && <MembershipWidget />}
<ActiveMembersList />
</Flex>
</TabPanel>
)}
{canManageMemberships && (
<TabPanel sx={{ width: '100%' }}>
<OrganizationMembersTabInvitations />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React from 'react';

import { useGate } from '../../../ui/common';
import { ORGANIZATION_PROFILE_NAVBAR_ROUTE_ID } from '../../../ui/constants';
import { useCoreOrganization, useOrganizationProfileContext } from '../../contexts';
import { Breadcrumbs, NavBar, NavbarContextProvider, OrganizationPreview } from '../../elements';
import type { PropsOfComponent } from '../../styledSystem';
Expand All @@ -10,6 +12,17 @@ export const OrganizationProfileNavbar = (
const { organization } = useCoreOrganization();
const { pages } = useOrganizationProfileContext();

const { isAuthorizedUser: allowMembersRoute } = useGate({
some: [
{
permission: 'org:sys_memberships:read',
},
{
permission: 'org:sys_memberships:manage',
},
],
});

if (!organization) {
return null;
}
Expand All @@ -24,7 +37,11 @@ export const OrganizationProfileNavbar = (
sx={t => ({ margin: `0 0 ${t.space.$4} ${t.space.$2}` })}
/>
}
routes={pages.routes}
routes={pages.routes.filter(
r =>
r.id !== ORGANIZATION_PROFILE_NAVBAR_ROUTE_ID.MEMBERS ||
(r.id === ORGANIZATION_PROFILE_NAVBAR_ROUTE_ID.MEMBERS && allowMembersRoute),
)}
contentRef={props.contentRef}
/>
{props.children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,12 @@ export const OrganizationProfileRoutes = (props: PropsOfComponent<typeof Profile
</Gate>
</Route>
<Route index>
<OrganizationMembers />
<Gate
some={[{ permission: 'org:sys_memberships:read' }, { permission: 'org:sys_memberships:manage' }]}
redirectTo={isSettingsPageRoot ? '../' : './organization-settings'}
>
<OrganizationMembers />
</Gate>
</Route>
</Switch>
</Route>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('OrganizationMembers', () => {
f.withOrganizations();
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [{ name: 'Org1', permissions: [] }],
organization_memberships: [{ name: 'Org1', permissions: ['org:sys_memberships:read'] }],
});
});

Expand All @@ -85,6 +85,26 @@ describe('OrganizationMembers', () => {
});
});

it('does not show members tab or navbar route if user is lacking permissions', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withOrganizations();
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [{ name: 'Org1', permissions: [] }],
});
});

fixtures.clerk.organization?.getRoles.mockRejectedValue(null);

const { queryByRole } = render(<OrganizationMembers />, { wrapper });

await waitFor(() => {
expect(queryByRole('tab', { name: 'Members' })).not.toBeInTheDocument();
expect(queryByRole('tab', { name: 'Invitations' })).not.toBeInTheDocument();
expect(queryByRole('tab', { name: 'Requests' })).not.toBeInTheDocument();
});
});

it('navigates to invite screen when user clicks on Invite button', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withOrganizations();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,24 @@ describe('OrganizationProfile', () => {
expect(getByText('Custom1')).toBeDefined();
expect(getByText('ExternalLink')).toBeDefined();
});

it('removes member nav item if user is lacking permissions', async () => {
const { wrapper } = await createFixtures(f => {
f.withOrganizations();
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [
{
name: 'Org1',
permissions: [],
},
],
});
});

const { queryByText } = render(<OrganizationProfile />, { wrapper });
expect(queryByText('Org1')).toBeInTheDocument();
expect(queryByText('Members')).not.toBeInTheDocument();
expect(queryByText('Settings')).toBeInTheDocument();
});
});

0 comments on commit 9955938

Please sign in to comment.