Skip to content

Commit b80a01c

Browse files
ycdzjycdzj
authored
Merge pull request #213 from import-ai/refactor/permissions
refactor(permissions): add owner check
2 parents 5d649f0 + ec126ff commit b80a01c

File tree

9 files changed

+96
-68
lines changed

9 files changed

+96
-68
lines changed

‎src/groups/groups.controller.ts‎

Lines changed: 10 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6,34 +6,23 @@ import {
66
Param,
77
Patch,
88
Post,
9-
Req,
10-
HttpStatus,
119
} from '@nestjs/common';
12-
import { AppException } from 'omniboxd/common/exceptions/app.exception';
13-
import { I18n, I18nContext } from 'nestjs-i18n';
1410
import { GroupsService } from './groups.service';
1511
import { CreateGroupDto } from './dto/create-group.dto';
1612
import { plainToInstance } from 'class-transformer';
1713
import { GroupDto } from './dto/group.dto';
1814
import { UpdateGroupDto } from './dto/update-group.dto';
1915
import { GroupUserDto } from './dto/group-user.dto';
2016
import { AddGroupUserDto } from './dto/add-group-user.dto';
21-
import { NamespacesService } from 'omniboxd/namespaces/namespaces.service';
17+
import { NamespaceOwner } from 'omniboxd/namespaces/decorators/namespace-owner.decorator';
2218

2319
@Controller('api/v1/namespaces/:namespaceId/groups')
2420
export class GroupsController {
25-
constructor(
26-
private readonly groupsService: GroupsService,
27-
private readonly namespacesService: NamespacesService,
28-
) {}
21+
constructor(private readonly groupsService: GroupsService) {}
2922

23+
@NamespaceOwner()
3024
@Get()
31-
async list(@Req() req, @Param('namespaceId') namespaceId: string, @I18n() i18n: I18nContext) {
32-
if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
33-
const message = i18n.t('namespace.errors.userNotOwner');
34-
throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
35-
}
36-
25+
async list(@Param('namespaceId') namespaceId: string) {
3726
const groups = await this.groupsService.listGroups(namespaceId);
3827
const invitations =
3928
await this.groupsService.listGroupInvitations(namespaceId);
@@ -50,36 +39,26 @@ export class GroupsController {
5039
});
5140
}
5241

42+
@NamespaceOwner()
5343
@Post()
5444
async create(
55-
@Req() req,
5645
@Param('namespaceId') namespaceId: string,
5746
@Body() createGroupDto: CreateGroupDto,
58-
@I18n() i18n: I18nContext,
5947
) {
60-
if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
61-
const message = i18n.t('namespace.errors.userNotOwner');
62-
throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
63-
}
6448
const group = await this.groupsService.createGroup(
6549
namespaceId,
6650
createGroupDto,
6751
);
6852
return plainToInstance(GroupDto, group);
6953
}
7054

55+
@NamespaceOwner()
7156
@Patch(':groupId')
7257
async update(
73-
@Req() req,
7458
@Param('namespaceId') namespaceId: string,
7559
@Param('groupId') groupId: string,
7660
@Body() updateGroupDto: UpdateGroupDto,
77-
@I18n() i18n: I18nContext,
7861
) {
79-
if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
80-
const message = i18n.t('namespace.errors.userNotOwner');
81-
throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
82-
}
8362
const group = await this.groupsService.updateGroup(
8463
namespaceId,
8564
groupId,
@@ -88,49 +67,34 @@ export class GroupsController {
8867
return plainToInstance(GroupDto, group);
8968
}
9069

70+
@NamespaceOwner()
9171
@Delete(':groupId')
9272
async delete(
93-
@Req() req,
9473
@Param('namespaceId') namespaceId: string,
9574
@Param('groupId') groupId: string,
96-
@I18n() i18n: I18nContext,
9775
) {
98-
if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
99-
const message = i18n.t('namespace.errors.userNotOwner');
100-
throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
101-
}
10276
await this.groupsService.deleteGroup(namespaceId, groupId);
10377
}
10478

79+
@NamespaceOwner()
10580
@Get(':groupId/users')
10681
async listGroupUsers(
107-
@Req() req,
10882
@Param('namespaceId') namespaceId: string,
10983
@Param('groupId') groupId: string,
110-
@I18n() i18n: I18nContext,
11184
) {
112-
if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
113-
const message = i18n.t('namespace.errors.userNotOwner');
114-
throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
115-
}
11685
const users = await this.groupsService.listGroupUsers(namespaceId, groupId);
11786
return plainToInstance(GroupUserDto, users, {
11887
excludeExtraneousValues: true,
11988
});
12089
}
12190

91+
@NamespaceOwner()
12292
@Post(':groupId/users')
12393
async addGroupUser(
124-
@Req() req,
12594
@Param('namespaceId') namespaceId: string,
12695
@Param('groupId') groupId: string,
12796
@Body() addGroupUserDto: AddGroupUserDto,
128-
@I18n() i18n: I18nContext,
12997
) {
130-
if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
131-
const message = i18n.t('namespace.errors.userNotOwner');
132-
throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
133-
}
13498
const actions: Array<Promise<any>> = [];
13599
addGroupUserDto.userIds.forEach((userId) => {
136100
if (userId) {
@@ -142,18 +106,13 @@ export class GroupsController {
142106
await Promise.all(actions);
143107
}
144108

109+
@NamespaceOwner()
145110
@Delete(':groupId/users/:userId')
146111
async deleteGroupUser(
147-
@Req() req,
148112
@Param('namespaceId') namespaceId: string,
149113
@Param('groupId') groupId: string,
150114
@Param('userId') userId: string,
151-
@I18n() i18n: I18nContext,
152115
) {
153-
if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
154-
const message = i18n.t('namespace.errors.userNotOwner');
155-
throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
156-
}
157116
await this.groupsService.deleteGroupUser(namespaceId, groupId, userId);
158117
}
159118
}

‎src/invitations/invitations.controller.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ import {
1010
} from '@nestjs/common';
1111
import { InvitationsService } from './invitations.service';
1212
import { CreateInvitationReqDto } from './dto/create-invitation-req.dto';
13+
import { NamespaceOwner } from 'omniboxd/namespaces/decorators/namespace-owner.decorator';
1314

1415
@Controller('api/v1/namespaces/:namespaceId')
1516
export class InvitationsController {
1617
constructor(private readonly invitationsService: InvitationsService) {}
1718

19+
@NamespaceOwner()
1820
@Get('invitations')
1921
async listInvitations(
2022
@Param('namespaceId') namespaceId: string,
@@ -23,6 +25,7 @@ export class InvitationsController {
2325
return await this.invitationsService.listInvitations(namespaceId, type);
2426
}
2527

28+
@NamespaceOwner()
2629
@Post('invitations')
2730
async createInvitation(
2831
@Param('namespaceId') namespaceId: string,
@@ -31,6 +34,7 @@ export class InvitationsController {
3134
return await this.invitationsService.createInvitation(namespaceId, req);
3235
}
3336

37+
@NamespaceOwner()
3438
@Delete('invitations/:invitationId')
3539
async deleteInvitation(
3640
@Param('namespaceId') namespaceId: string,

‎src/invitations/invitations.module.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,17 @@ import { TypeOrmModule } from '@nestjs/typeorm';
55
import { Invitation } from './entities/invitation.entity';
66
import { AuthModule } from 'omniboxd/auth/auth.module';
77
import { GroupsModule } from 'omniboxd/groups/groups.module';
8+
import { NamespacesModule } from 'omniboxd/namespaces/namespaces.module';
89

910
@Module({
1011
exports: [],
1112
controllers: [InvitationsController],
1213
providers: [InvitationsService],
13-
imports: [TypeOrmModule.forFeature([Invitation]), AuthModule, GroupsModule],
14+
imports: [
15+
TypeOrmModule.forFeature([Invitation]),
16+
AuthModule,
17+
GroupsModule,
18+
NamespacesModule,
19+
],
1420
})
1521
export class InvitationsModule {}

‎src/namespaces/decorators/namespace-owner.decorator.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { applyDecorators, UseInterceptors } from '@nestjs/common';
2+
import { NamespaceOwnerInterceptor } from '../interceptors/namespace-owner.interceptor';
3+
4+
export const NamespaceOwner = () =>
5+
applyDecorators(UseInterceptors(NamespaceOwnerInterceptor));

‎src/namespaces/interceptors/namespace-owner.interceptor.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import {
2+
CallHandler,
3+
ExecutionContext,
4+
HttpStatus,
5+
Injectable,
6+
NestInterceptor,
7+
} from '@nestjs/common';
8+
import { AppException } from 'omniboxd/common/exceptions/app.exception';
9+
import { Observable } from 'rxjs';
10+
import { NamespacesService } from '../namespaces.service';
11+
import { I18nService } from 'nestjs-i18n';
12+
13+
@Injectable()
14+
export class NamespaceOwnerInterceptor implements NestInterceptor {
15+
constructor(
16+
private readonly namespacesService: NamespacesService,
17+
private readonly i18n: I18nService,
18+
) {}
19+
20+
async intercept(
21+
context: ExecutionContext,
22+
next: CallHandler,
23+
): Promise<Observable<any>> {
24+
const request = context.switchToHttp().getRequest();
25+
const namespaceId = request.params?.namespaceId;
26+
const userId = request.user?.id;
27+
28+
if (!namespaceId || !userId) {
29+
const message = this.i18n.t('namespace.errors.userNotOwner');
30+
throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
31+
}
32+
33+
const isOwner = await this.namespacesService.userIsOwner(
34+
namespaceId,
35+
userId,
36+
);
37+
38+
if (!isOwner) {
39+
const message = this.i18n.t('namespace.errors.userNotOwner');
40+
throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
41+
}
42+
43+
return next.handle();
44+
}
45+
}

‎src/namespaces/namespaces.controller.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import { UserId } from 'omniboxd/decorators/user-id.decorator';
1414
import { CreateNamespaceDto } from './dto/create-namespace.dto';
1515
import { UpdateNamespaceDto } from './dto/update-namespace.dto';
16+
import { NamespaceOwner } from './decorators/namespace-owner.decorator';
1617

1718
@Controller('api/v1/namespaces')
1819
export class NamespacesController {
@@ -28,11 +29,13 @@ export class NamespacesController {
2829
return await this.namespacesService.getNamespace(namespaceId);
2930
}
3031

32+
@NamespaceOwner()
3133
@Get(':namespaceId/members')
32-
async listMembers(@Req() req, @Param('namespaceId') namespaceId: string) {
34+
async listMembers(@Param('namespaceId') namespaceId: string) {
3335
return await this.namespacesService.listMembers(namespaceId);
3436
}
3537

38+
@NamespaceOwner()
3639
@Get(':namespaceId/members/:userId')
3740
async getMemberByUserId(
3841
@Param('namespaceId') namespaceId: string,
@@ -41,6 +44,7 @@ export class NamespacesController {
4144
return await this.namespacesService.getMemberByUserId(namespaceId, userId);
4245
}
4346

47+
@NamespaceOwner()
4448
@Patch(':namespaceId/members/:userId')
4549
async UpdateMemberRole(
4650
@Param('namespaceId') namespaceId: string,
@@ -54,6 +58,7 @@ export class NamespacesController {
5458
);
5559
}
5660

61+
@NamespaceOwner()
5762
@Delete(':namespaceId/members/:userId')
5863
async deleteMember(
5964
@Param('namespaceId') namespaceId: string,
@@ -78,6 +83,7 @@ export class NamespacesController {
7883
);
7984
}
8085

86+
@NamespaceOwner()
8187
@Patch(':namespaceId')
8288
async update(
8389
@Param('namespaceId') namespaceId: string,
@@ -86,6 +92,7 @@ export class NamespacesController {
8692
return await this.namespacesService.update(namespaceId, updateDto);
8793
}
8894

95+
@NamespaceOwner()
8996
@Delete(':namespaceId')
9097
async delete(@Param('namespaceId') namespaceId: string) {
9198
return await this.namespacesService.delete(namespaceId);

‎src/namespaces/namespaces.e2e-spec.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -213,10 +213,10 @@ describe('NamespacesController (e2e)', () => {
213213
.send({ role: NamespaceRole.MEMBER })
214214
.expect(HttpStatus.OK);
215215

216-
// Clean up
216+
// Members are not allowed to delete the namespace.
217217
await client
218218
.delete(`/api/v1/namespaces/${tempNamespaceId}`)
219-
.expect(HttpStatus.OK);
219+
.expect(HttpStatus.FORBIDDEN);
220220
});
221221

222222
it('should fail for non-existent member', async () => {
@@ -264,12 +264,9 @@ describe('NamespacesController (e2e)', () => {
264264
.expect(HttpStatus.OK);
265265

266266
// Verify member was removed by checking if they can still access the namespace
267-
// This might still work due to business logic allowing owner access
268-
269-
// Clean up
270267
await client
271268
.delete(`/api/v1/namespaces/${tempNamespaceId}`)
272-
.expect(HttpStatus.OK);
269+
.expect(HttpStatus.FORBIDDEN);
273270
});
274271
});
275272

@@ -356,7 +353,7 @@ describe('NamespacesController (e2e)', () => {
356353
await client
357354
.patch('/api/v1/namespaces/nonexistent')
358355
.send(updateNamespaceDto)
359-
.expect(HttpStatus.NOT_FOUND);
356+
.expect(HttpStatus.FORBIDDEN);
360357
});
361358

362359
it('should validate update fields', async () => {
@@ -375,7 +372,7 @@ describe('NamespacesController (e2e)', () => {
375372
it('should succeed even if already deleted (soft delete behavior)', async () => {
376373
await client
377374
.delete('/api/v1/namespaces/nonexistent')
378-
.expect(HttpStatus.OK);
375+
.expect(HttpStatus.FORBIDDEN);
379376
});
380377

381378
it('should soft delete namespace', async () => {
@@ -455,11 +452,11 @@ describe('NamespacesController (e2e)', () => {
455452

456453
await client
457454
.get('/api/v1/namespaces/invalid-uuid/members')
458-
.expect(HttpStatus.NOT_FOUND);
455+
.expect(HttpStatus.FORBIDDEN);
459456

460457
await client
461458
.get('/api/v1/namespaces/invalid-uuid/members/also-invalid')
462-
.expect(HttpStatus.INTERNAL_SERVER_ERROR);
459+
.expect(HttpStatus.FORBIDDEN);
463460
});
464461

465462
it('should handle concurrent namespace operations', async () => {

‎src/namespaces/namespaces.module.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,16 @@ import { UserModule } from 'omniboxd/user/user.module';
44
import { Resource } from 'omniboxd/resources/entities/resource.entity';
55
import { NamespacesService } from 'omniboxd/namespaces/namespaces.service';
66
import { NamespacesController } from 'omniboxd/namespaces/namespaces.controller';
7-
import { Namespace } from './entities/namespace.entity';
87
import { NamespaceMember } from './entities/namespace-member.entity';
98
import { NamespaceResourcesModule } from 'omniboxd/namespace-resources/namespace-resources.module';
109
import { PermissionsModule } from 'omniboxd/permissions/permissions.module';
1110
import { ResourcesModule } from 'omniboxd/resources/resources.module';
11+
import { Namespace } from './entities/namespace.entity';
12+
import { NamespaceOwnerInterceptor } from './interceptors/namespace-owner.interceptor';
1213

1314
@Module({
1415
exports: [NamespacesService],
15-
providers: [NamespacesService],
16+
providers: [NamespacesService, NamespaceOwnerInterceptor],
1617
controllers: [NamespacesController],
1718
imports: [
1819
UserModule,

0 commit comments

Comments
 (0)