Skip to content

Commit

Permalink
refactor: redefining the permit action structure (#825)
Browse files Browse the repository at this point in the history
  • Loading branch information
boris-w authored Aug 15, 2024
1 parent 13081fb commit 1ad330a
Show file tree
Hide file tree
Showing 62 changed files with 424 additions and 708 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Injectable, UnauthorizedException } from '@nestjs/common';
import type { AllActions } from '@teable/core';
import type { Action } from '@teable/core';
import { generateAccessTokenId, getRandomString } from '@teable/core';
import { PrismaService } from '@teable/db-main-prisma';
import type {
Expand Down Expand Up @@ -34,7 +34,7 @@ export class AccessTokenService {
return {
...accessTokenEntity,
description: description || undefined,
scopes: JSON.parse(scopes) as AllActions[],
scopes: JSON.parse(scopes) as Action[],
spaceIds: spaceIds ? (JSON.parse(spaceIds) as string[]) : undefined,
baseIds: baseIds ? (JSON.parse(baseIds) as string[]) : undefined,
createdTime: createdTime?.toISOString(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { SetMetadata } from '@nestjs/common';
import type { PermissionAction } from '@teable/core';
import type { Action } from '@teable/core';

export const PERMISSIONS_KEY = 'permissions';

// eslint-disable-next-line @typescript-eslint/naming-convention
export const Permissions = (...permissions: PermissionAction[]) =>
SetMetadata(PERMISSIONS_KEY, permissions);
export const Permissions = (...permissions: Action[]) => SetMetadata(PERMISSIONS_KEY, permissions);
15 changes: 6 additions & 9 deletions apps/nestjs-backend/src/features/auth/guard/permission.guard.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ExecutionContext } from '@nestjs/common';
import { ForbiddenException, Injectable } from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { type PermissionAction } from '@teable/core';
import { type Action } from '@teable/core';
import { ClsService } from 'nestjs-cls';
import type { IClsStore } from '../../../types/cls';
import { IS_DISABLED_PERMISSION } from '../decorators/disabled-permission.decorator';
Expand Down Expand Up @@ -48,10 +48,7 @@ export class PermissionGuard {
return true;
}

protected async resourcePermission(
resourceId: string | undefined,
permissions: PermissionAction[]
) {
protected async resourcePermission(resourceId: string | undefined, permissions: Action[]) {
if (!resourceId) {
throw new ForbiddenException('permission check ID does not exist');
}
Expand All @@ -66,10 +63,10 @@ export class PermissionGuard {
}

protected async permissionCheck(context: ExecutionContext) {
const permissions = this.reflector.getAllAndOverride<PermissionAction[] | undefined>(
PERMISSIONS_KEY,
[context.getHandler(), context.getClass()]
);
const permissions = this.reflector.getAllAndOverride<Action[] | undefined>(PERMISSIONS_KEY, [
context.getHandler(),
context.getClass(),
]);

const accessTokenId = this.cls.get('accessTokenId');
if (accessTokenId && !permissions?.length) {
Expand Down
16 changes: 7 additions & 9 deletions apps/nestjs-backend/src/features/auth/permission.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import { ForbiddenException, NotFoundException } from '@nestjs/common';
import type { TestingModule } from '@nestjs/testing';
import { Test } from '@nestjs/testing';
import type { AllActions } from '@teable/core';
import { RoleType, SpaceRole, getPermissions } from '@teable/core';
import type { Action } from '@teable/core';
import { Role, getPermissions } from '@teable/core';
import { PrismaService } from '@teable/db-main-prisma';
import { noop } from 'lodash';
import { ClsService } from 'nestjs-cls';
Expand Down Expand Up @@ -218,7 +218,7 @@ describe('PermissionService', () => {
it('should return scopes when resourceId is a valid spaceId and allowed', async () => {
const resourceId = 'spcxxxxxxx';
const accessTokenId = 'validAccessTokenId';
const scopes: AllActions[] = ['table|create', 'table|update'];
const scopes: Action[] = ['table|create', 'table|update'];
const spaceIds = ['spcxxxxxxx'];

vi.spyOn(service, 'getAccessToken').mockResolvedValueOnce({
Expand Down Expand Up @@ -288,7 +288,7 @@ describe('PermissionService', () => {
it('should return permissions for a user', async () => {
const resourceId = 'bsexxxxxx';
vi.spyOn(service, 'getPermissionsByResourceId').mockResolvedValue(
getPermissions(RoleType.Space, SpaceRole.Editor)
getPermissions(Role.Editor)
);
const result = await service.getPermissions(resourceId);
expect(result.includes('view|create')).toEqual(true);
Expand All @@ -298,7 +298,7 @@ describe('PermissionService', () => {
it('should return permissions for access token', async () => {
const resourceId = 'bsexxxxxx';
vi.spyOn(service, 'getPermissionsByResourceId').mockResolvedValue(
getPermissions(RoleType.Space, SpaceRole.Editor)
getPermissions(Role.Editor)
);
vi.spyOn(service, 'getPermissionsByAccessToken').mockResolvedValue([
'view|create',
Expand All @@ -313,17 +313,15 @@ describe('PermissionService', () => {

describe('validPermissions', () => {
it('should return true if user has all required permissions', async () => {
const permissions = getPermissions(RoleType.Space, SpaceRole.Creator);
const permissions = getPermissions(Role.Creator);
vi.spyOn(service, 'getPermissions').mockResolvedValue(permissions);
const resourceId = 'bsexxxxxx';
const result = await service.validPermissions(resourceId, ['base|create']);
expect(result).toEqual(permissions);
});

it('should throw an error if user does not have all required permissions', async () => {
vi.spyOn(service, 'getPermissions').mockResolvedValue(
getPermissions(RoleType.Space, SpaceRole.Editor)
);
vi.spyOn(service, 'getPermissions').mockResolvedValue(getPermissions(Role.Editor));
const resourceId = 'bsexxxxxx';
await expect(service.validPermissions(resourceId, ['space|create'])).rejects.toThrowError(
new ForbiddenException(`not allowed to operate space|create on ${resourceId}`)
Expand Down
19 changes: 7 additions & 12 deletions apps/nestjs-backend/src/features/auth/permission.service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { ForbiddenException, NotFoundException, Injectable } from '@nestjs/common';
import type { BaseRole, PermissionAction, SpaceRole } from '@teable/core';
import type { IBaseRole, Action, IRole } from '@teable/core';
import { IdPrefix, getPermissions } from '@teable/core';
import { PrismaService } from '@teable/db-main-prisma';
import { intersection } from 'lodash';
import { ClsService } from 'nestjs-cls';
import { RoleType } from '../../../../../packages/core/src/auth/types';
import type { IClsStore } from '../../types/cls';

@Injectable()
Expand All @@ -29,7 +28,7 @@ export class PermissionService {
if (!collaborator) {
throw new ForbiddenException(`can't find collaborator`);
}
return collaborator.roleName as SpaceRole;
return collaborator.roleName as IRole;
}

async getRoleByBaseId(baseId: string) {
Expand All @@ -47,7 +46,7 @@ export class PermissionService {
if (!collaborator) {
return null;
}
return collaborator.roleName as BaseRole;
return collaborator.roleName as IBaseRole;
}

async getOAuthAccessBy(userId: string) {
Expand Down Expand Up @@ -83,7 +82,7 @@ export class PermissionService {
where: { id: accessTokenId },
select: { scopes: true, spaceIds: true, baseIds: true, clientId: true, userId: true },
});
const scopes = JSON.parse(stringifyScopes) as PermissionAction[];
const scopes = JSON.parse(stringifyScopes) as Action[];
if (clientId) {
const { spaceIds: spaceIdsByOAuth, baseIds: baseIdsByOAuth } =
await this.getOAuthAccessBy(userId);
Expand Down Expand Up @@ -186,13 +185,13 @@ export class PermissionService {

private async getPermissionBySpaceId(spaceId: string) {
const role = await this.getRoleBySpaceId(spaceId);
return getPermissions(RoleType.Space, role);
return getPermissions(role);
}

private async getPermissionByBaseId(baseId: string) {
const role = await this.getRoleByBaseId(baseId);
if (role) {
return getPermissions(RoleType.Base, role);
return getPermissions(role);
}
return this.getPermissionBySpaceId((await this.getUpperIdByBaseId(baseId)).spaceId);
}
Expand Down Expand Up @@ -227,11 +226,7 @@ export class PermissionService {
return userPermissions;
}

async validPermissions(
resourceId: string,
permissions: PermissionAction[],
accessTokenId?: string
) {
async validPermissions(resourceId: string, permissions: Action[], accessTokenId?: string) {
const ownPermissions = await this.getPermissions(resourceId, accessTokenId);
if (permissions.every((permission) => ownPermissions.includes(permission))) {
return ownPermissions;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { TestingModule } from '@nestjs/testing';
import { Test } from '@nestjs/testing';
import { RoleType, SpaceRole, getPermissions } from '@teable/core';
import { Role, getPermissions } from '@teable/core';
import { PrismaService } from '@teable/db-main-prisma';
import { ClsService } from 'nestjs-cls';
import { mockDeep } from 'vitest-mock-extended';
Expand Down Expand Up @@ -41,21 +41,17 @@ describe('CollaboratorService', () => {
{
user: mockUser,
tx: {},
permissions: getPermissions(RoleType.Space, SpaceRole.Owner),
permissions: getPermissions(Role.Owner),
},
async () => {
await collaboratorService.createSpaceCollaborator(
mockUser.id,
mockSpace.id,
SpaceRole.Owner
);
await collaboratorService.createSpaceCollaborator(mockUser.id, mockSpace.id, Role.Owner);
}
);

expect(prismaService.collaborator.create).toBeCalledWith({
data: {
spaceId: mockSpace.id,
roleName: SpaceRole.Owner,
roleName: Role.Owner,
userId: mockUser.id,
createdBy: mockUser.id,
},
Expand All @@ -66,7 +62,7 @@ describe('CollaboratorService', () => {
prismaService.collaborator.count.mockResolvedValue(1);

await expect(
collaboratorService.createSpaceCollaborator(mockUser.id, mockSpace.id, SpaceRole.Owner)
collaboratorService.createSpaceCollaborator(mockUser.id, mockSpace.id, Role.Owner)
).rejects.toThrow('has already existed in space');
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BadRequestException, Injectable } from '@nestjs/common';
import type { SpaceRole } from '@teable/core';
import type { IRole } from '@teable/core';
import { PrismaService } from '@teable/db-main-prisma';
import {
UploadType,
Expand Down Expand Up @@ -30,7 +30,7 @@ export class CollaboratorService {
@InjectModel('CUSTOM_KNEX') private readonly knex: Knex
) {}

async createSpaceCollaborator(userId: string, spaceId: string, role: SpaceRole) {
async createSpaceCollaborator(userId: string, spaceId: string, role: IRole) {
const currentUserId = this.cls.get('user.id');
const exist = await this.prismaService
.txClient()
Expand Down Expand Up @@ -181,17 +181,17 @@ export class CollaboratorService {
spaceId: true,
},
});
const roleMap: Record<string, SpaceRole> = {};
const roleMap: Record<string, IRole> = {};
const baseIds = new Set<string>();
const spaceIds = new Set<string>();
collaborators.forEach(({ baseId, spaceId, roleName }) => {
if (baseId) {
baseIds.add(baseId);
roleMap[baseId] = roleName as SpaceRole;
roleMap[baseId] = roleName as IRole;
}
if (spaceId) {
spaceIds.add(spaceId);
roleMap[spaceId] = roleName as SpaceRole;
roleMap[spaceId] = roleName as IRole;
}
});
return {
Expand Down
Loading

0 comments on commit 1ad330a

Please sign in to comment.