Skip to content

Commit

Permalink
chore(backend,nextjs,types): Prevent system permissions usage in serv…
Browse files Browse the repository at this point in the history
…er-side (#4816)

Co-authored-by: panteliselef <panteliselef@outlook.com>
  • Loading branch information
LauraBeatris and panteliselef authored Jan 8, 2025
1 parent 4fb345f commit 44cab60
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 14 deletions.
9 changes: 9 additions & 0 deletions .changeset/fair-bobcats-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@clerk/backend': patch
'@clerk/types': patch
'@clerk/nextjs': patch
---

Add type-level validation to prevent server-side usage of system permissions

System permissions (e.g., `org:sys_domains:manage`) are intentionally excluded from session claims to maintain reasonable JWT sizes. For more information, refer to our docs: https://clerk.com/docs/organizations/roles-permissions#system-permissions
6 changes: 3 additions & 3 deletions packages/backend/src/tokens/authObjects.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createCheckAuthorization } from '@clerk/shared/authorization';
import type {
ActClaim,
CheckAuthorizationWithCustomPermissions,
CheckAuthorizationFromSessionClaims,
JwtPayload,
OrganizationCustomPermissionKey,
OrganizationCustomRoleKey,
Expand Down Expand Up @@ -42,7 +42,7 @@ export type SignedInAuthObject = {
*/
factorVerificationAge: [number, number] | null;
getToken: ServerGetToken;
has: CheckAuthorizationWithCustomPermissions;
has: CheckAuthorizationFromSessionClaims;
debug: AuthObjectDebug;
};

Expand All @@ -65,7 +65,7 @@ export type SignedOutAuthObject = {
*/
factorVerificationAge: null;
getToken: ServerGetToken;
has: CheckAuthorizationWithCustomPermissions;
has: CheckAuthorizationFromSessionClaims;
debug: AuthObjectDebug;
};

Expand Down
3 changes: 1 addition & 2 deletions packages/nextjs/src/app-router/server/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const auth: AuthFn = async () => {
return Object.assign(authObject, { redirectToSignIn });
};

auth.protect = async (...args) => {
auth.protect = async (...args: any[]) => {
require('server-only');

const request = await buildRequestLike();
Expand All @@ -66,6 +66,5 @@ auth.protect = async (...args) => {
redirect,
});

// @ts-expect-error TS flattens all possible combinations of the for AuthProtect signatures in a union.
return protect(...args);
};
11 changes: 11 additions & 0 deletions packages/nextjs/src/server/__tests__/clerkMiddleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ describe('ClerkMiddleware type tests', () => {
clerkMiddlewareMock();
});

it('prevents usage of system permissions with auth.has()', () => {
clerkMiddlewareMock(async (auth, _event, _request) => {
// @ts-expect-error - system permissions are not allowed
(await auth()).has({ permission: 'org:sys_foo' });
// @ts-expect-error - system permissions are not allowed
await auth.protect(has => has({ permission: 'org:sys_foo' }));
// @ts-expect-error - system permissions are not allowed
await auth.protect({ permission: 'org:sys_foo' });
});
});

describe('Multi domain', () => {
const defaultProps = { publishableKey: '', secretKey: '' };

Expand Down
10 changes: 8 additions & 2 deletions packages/nextjs/src/server/protect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ import type { AuthObject } from '@clerk/backend';
import type { RedirectFun, SignedInAuthObject } from '@clerk/backend/internal';
import { constants } from '@clerk/backend/internal';
import type {
CheckAuthorizationFromSessionClaims,
CheckAuthorizationParamsFromSessionClaims,
CheckAuthorizationParamsWithCustomPermissions,
CheckAuthorizationWithCustomPermissions,
OrganizationCustomPermissionKey,
} from '@clerk/types';

import { constants as nextConstants } from '../constants';
Expand All @@ -15,10 +18,13 @@ type AuthProtectOptions = { unauthorizedUrl?: string; unauthenticatedUrl?: strin
* Throws a Nextjs notFound error if user is not authenticated or authorized.
*/
export interface AuthProtect {
(params?: CheckAuthorizationParamsWithCustomPermissions, options?: AuthProtectOptions): Promise<SignedInAuthObject>;
<P extends OrganizationCustomPermissionKey>(
params?: CheckAuthorizationParamsFromSessionClaims<P>,
options?: AuthProtectOptions,
): Promise<SignedInAuthObject>;

(
params?: (has: CheckAuthorizationWithCustomPermissions) => boolean,
params?: (has: CheckAuthorizationFromSessionClaims) => boolean,
options?: AuthProtectOptions,
): Promise<SignedInAuthObject>;

Expand Down
8 changes: 8 additions & 0 deletions packages/react/src/hooks/__tests__/useAuth.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -194,4 +194,12 @@ describe('useDerivedAuth', () => {
expectTypeOf(result).toBeBoolean();
expect(mockHas).toHaveBeenCalledWith({ permission: 'test' });
});

it('allows to pass system permissions', () => {
const {
result: { current },
} = renderHook(() => useDerivedAuth({ sessionId: null, userId: null }));

current.has?.({ permission: 'org:sys_foo' });
});
});
2 changes: 1 addition & 1 deletion packages/types/src/jwtv2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export interface JwtPayload extends CustomJwtSessionClaims {
org_role?: OrganizationCustomRoleKey;

/**
* Active organization role
* Active organization permissions
*/
org_permissions?: OrganizationCustomPermissionKey[];

Expand Down
13 changes: 7 additions & 6 deletions packages/types/src/organizationMembership.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,14 @@ export type OrganizationCustomRoleKey = ClerkAuthorization extends Placeholder
: Base['role']
: Base['role'];

export type OrganizationSystemPermissionPrefix = 'org:sys_';
export type OrganizationSystemPermissionKey =
| 'org:sys_domains:manage'
| 'org:sys_profile:manage'
| 'org:sys_profile:delete'
| 'org:sys_memberships:read'
| 'org:sys_memberships:manage'
| 'org:sys_domains:read';
| `${OrganizationSystemPermissionPrefix}domains:manage`
| `${OrganizationSystemPermissionPrefix}profile:manage`
| `${OrganizationSystemPermissionPrefix}profile:delete`
| `${OrganizationSystemPermissionPrefix}memberships:read`
| `${OrganizationSystemPermissionPrefix}memberships:manage`
| `${OrganizationSystemPermissionPrefix}domains:read`;

/**
* OrganizationPermissionKey is a combination of system and custom permissions.
Expand Down
26 changes: 26 additions & 0 deletions packages/types/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
OrganizationCustomPermissionKey,
OrganizationCustomRoleKey,
OrganizationPermissionKey,
OrganizationSystemPermissionPrefix,
} from './organizationMembership';
import type { ClerkResource } from './resource';
import type {
Expand All @@ -24,6 +25,10 @@ import type { SessionJSONSnapshot } from './snapshots';
import type { TokenResource } from './token';
import type { UserResource } from './user';

type DisallowSystemPermissions<P extends string> = P extends `${OrganizationSystemPermissionPrefix}${string}`
? 'System permissions are not included in session claims and cannot be used on the server-side'
: P;

export type CheckAuthorizationFn<Params> = (isAuthorizedParams: Params) => boolean;

export type CheckAuthorizationWithCustomPermissions =
Expand Down Expand Up @@ -62,6 +67,27 @@ type CheckAuthorizationParams = WithReverification<
}
>;

/**
* Type guard for server-side authorization checks using session claims.
* System permissions are not allowed since they are not included
* in session claims and cannot be verified on the server side.
*/
export type CheckAuthorizationFromSessionClaims = <P extends OrganizationCustomPermissionKey>(
isAuthorizedParams: CheckAuthorizationParamsFromSessionClaims<P>,
) => boolean;

export type CheckAuthorizationParamsFromSessionClaims<P extends OrganizationCustomPermissionKey> = WithReverification<
| {
role: OrganizationCustomRoleKey;
permission?: never;
}
| {
role?: never;
permission: DisallowSystemPermissions<P>;
}
| { role?: never; permission?: never }
>;

export interface SessionResource extends ClerkResource {
id: string;
status: SessionStatus;
Expand Down

0 comments on commit 44cab60

Please sign in to comment.