refactor(permissions): add owner check

src/groups/groups.controller.ts

@@ -6,34 +6,23 @@ import {
   Param,
   Patch,
   Post,
-  Req,
-  HttpStatus,
 } from '@nestjs/common';
-import { AppException } from 'omniboxd/common/exceptions/app.exception';
-import { I18n, I18nContext } from 'nestjs-i18n';
 import { GroupsService } from './groups.service';
 import { CreateGroupDto } from './dto/create-group.dto';
 import { plainToInstance } from 'class-transformer';
 import { GroupDto } from './dto/group.dto';
 import { UpdateGroupDto } from './dto/update-group.dto';
 import { GroupUserDto } from './dto/group-user.dto';
 import { AddGroupUserDto } from './dto/add-group-user.dto';
-import { NamespacesService } from 'omniboxd/namespaces/namespaces.service';
+import { NamespaceOwner } from 'omniboxd/namespaces/decorators/namespace-owner.decorator';
 
 @Controller('api/v1/namespaces/:namespaceId/groups')
 export class GroupsController {
-  constructor(
-    private readonly groupsService: GroupsService,
-    private readonly namespacesService: NamespacesService,
-  ) {}
+  constructor(private readonly groupsService: GroupsService) {}
 
+  @NamespaceOwner()
   @Get()
-  async list(@Req() req, @Param('namespaceId') namespaceId: string, @I18n() i18n: I18nContext) {
-    if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
-      const message = i18n.t('namespace.errors.userNotOwner');
-      throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
-    }
-
+  async list(@Param('namespaceId') namespaceId: string) {
     const groups = await this.groupsService.listGroups(namespaceId);
     const invitations =
       await this.groupsService.listGroupInvitations(namespaceId);
@@ -50,36 +39,26 @@ export class GroupsController {
     });
   }
 
+  @NamespaceOwner()
   @Post()
   async create(
-    @Req() req,
     @Param('namespaceId') namespaceId: string,
     @Body() createGroupDto: CreateGroupDto,
-    @I18n() i18n: I18nContext,
   ) {
-    if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
-      const message = i18n.t('namespace.errors.userNotOwner');
-      throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
-    }
     const group = await this.groupsService.createGroup(
       namespaceId,
       createGroupDto,
     );
     return plainToInstance(GroupDto, group);
   }
 
+  @NamespaceOwner()
   @Patch(':groupId')
   async update(
-    @Req() req,
     @Param('namespaceId') namespaceId: string,
     @Param('groupId') groupId: string,
     @Body() updateGroupDto: UpdateGroupDto,
-    @I18n() i18n: I18nContext,
   ) {
-    if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
-      const message = i18n.t('namespace.errors.userNotOwner');
-      throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
-    }
     const group = await this.groupsService.updateGroup(
       namespaceId,
       groupId,
@@ -88,49 +67,34 @@ export class GroupsController {
     return plainToInstance(GroupDto, group);
   }
 
+  @NamespaceOwner()
   @Delete(':groupId')
   async delete(
-    @Req() req,
     @Param('namespaceId') namespaceId: string,
     @Param('groupId') groupId: string,
-    @I18n() i18n: I18nContext,
   ) {
-    if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
-      const message = i18n.t('namespace.errors.userNotOwner');
-      throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
-    }
     await this.groupsService.deleteGroup(namespaceId, groupId);
   }
 
+  @NamespaceOwner()
   @Get(':groupId/users')
   async listGroupUsers(
-    @Req() req,
     @Param('namespaceId') namespaceId: string,
     @Param('groupId') groupId: string,
-    @I18n() i18n: I18nContext,
   ) {
-    if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
-      const message = i18n.t('namespace.errors.userNotOwner');
-      throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
-    }
     const users = await this.groupsService.listGroupUsers(namespaceId, groupId);
     return plainToInstance(GroupUserDto, users, {
       excludeExtraneousValues: true,
     });
   }
 
+  @NamespaceOwner()
   @Post(':groupId/users')
   async addGroupUser(
-    @Req() req,
     @Param('namespaceId') namespaceId: string,
     @Param('groupId') groupId: string,
     @Body() addGroupUserDto: AddGroupUserDto,
-    @I18n() i18n: I18nContext,
   ) {
-    if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
-      const message = i18n.t('namespace.errors.userNotOwner');
-      throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
-    }
     const actions: Array<Promise<any>> = [];
     addGroupUserDto.userIds.forEach((userId) => {
       if (userId) {
@@ -142,18 +106,13 @@ export class GroupsController {
     await Promise.all(actions);
   }
 
+  @NamespaceOwner()
   @Delete(':groupId/users/:userId')
   async deleteGroupUser(
-    @Req() req,
     @Param('namespaceId') namespaceId: string,
     @Param('groupId') groupId: string,
     @Param('userId') userId: string,
-    @I18n() i18n: I18nContext,
   ) {
-    if (!(await this.namespacesService.userIsOwner(namespaceId, req.user.id))) {
-      const message = i18n.t('namespace.errors.userNotOwner');
-      throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
-    }
     await this.groupsService.deleteGroupUser(namespaceId, groupId, userId);
   }
 }

src/invitations/invitations.controller.ts

@@ -10,11 +10,13 @@ import {
 } from '@nestjs/common';
 import { InvitationsService } from './invitations.service';
 import { CreateInvitationReqDto } from './dto/create-invitation-req.dto';
+import { NamespaceOwner } from 'omniboxd/namespaces/decorators/namespace-owner.decorator';
 
 @Controller('api/v1/namespaces/:namespaceId')
 export class InvitationsController {
   constructor(private readonly invitationsService: InvitationsService) {}
 
+  @NamespaceOwner()
   @Get('invitations')
   async listInvitations(
     @Param('namespaceId') namespaceId: string,
@@ -23,6 +25,7 @@ export class InvitationsController {
     return await this.invitationsService.listInvitations(namespaceId, type);
   }
 
+  @NamespaceOwner()
   @Post('invitations')
   async createInvitation(
     @Param('namespaceId') namespaceId: string,
@@ -31,6 +34,7 @@ export class InvitationsController {
     return await this.invitationsService.createInvitation(namespaceId, req);
   }
 
+  @NamespaceOwner()
   @Delete('invitations/:invitationId')
   async deleteInvitation(
     @Param('namespaceId') namespaceId: string,

src/invitations/invitations.module.ts

@@ -5,11 +5,17 @@ import { TypeOrmModule } from '@nestjs/typeorm';
 import { Invitation } from './entities/invitation.entity';
 import { AuthModule } from 'omniboxd/auth/auth.module';
 import { GroupsModule } from 'omniboxd/groups/groups.module';
+import { NamespacesModule } from 'omniboxd/namespaces/namespaces.module';
 
 @Module({
   exports: [],
   controllers: [InvitationsController],
   providers: [InvitationsService],
-  imports: [TypeOrmModule.forFeature([Invitation]), AuthModule, GroupsModule],
+  imports: [
+    TypeOrmModule.forFeature([Invitation]),
+    AuthModule,
+    GroupsModule,
+    NamespacesModule,
+  ],
 })
 export class InvitationsModule {}

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

@@ -0,0 +1,5 @@
+import { applyDecorators, UseInterceptors } from '@nestjs/common';
+import { NamespaceOwnerInterceptor } from '../interceptors/namespace-owner.interceptor';
+
+export const NamespaceOwner = () =>
+  applyDecorators(UseInterceptors(NamespaceOwnerInterceptor));

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

@@ -0,0 +1,45 @@
+import {
+  CallHandler,
+  ExecutionContext,
+  HttpStatus,
+  Injectable,
+  NestInterceptor,
+} from '@nestjs/common';
+import { AppException } from 'omniboxd/common/exceptions/app.exception';
+import { Observable } from 'rxjs';
+import { NamespacesService } from '../namespaces.service';
+import { I18nService } from 'nestjs-i18n';
+
+@Injectable()
+export class NamespaceOwnerInterceptor implements NestInterceptor {
+  constructor(
+    private readonly namespacesService: NamespacesService,
+    private readonly i18n: I18nService,
+  ) {}
+
+  async intercept(
+    context: ExecutionContext,
+    next: CallHandler,
+  ): Promise<Observable<any>> {
+    const request = context.switchToHttp().getRequest();
+    const namespaceId = request.params?.namespaceId;
+    const userId = request.user?.id;
+
+    if (!namespaceId || !userId) {
+      const message = this.i18n.t('namespace.errors.userNotOwner');
+      throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
+    }
+
+    const isOwner = await this.namespacesService.userIsOwner(
+      namespaceId,
+      userId,
+    );
+
+    if (!isOwner) {
+      const message = this.i18n.t('namespace.errors.userNotOwner');
+      throw new AppException(message, 'USER_NOT_OWNER', HttpStatus.FORBIDDEN);
+    }
+
+    return next.handle();
+  }
+}

src/namespaces/namespaces.controller.ts

@@ -13,6 +13,7 @@ import {
 import { UserId } from 'omniboxd/decorators/user-id.decorator';
 import { CreateNamespaceDto } from './dto/create-namespace.dto';
 import { UpdateNamespaceDto } from './dto/update-namespace.dto';
+import { NamespaceOwner } from './decorators/namespace-owner.decorator';
 
 @Controller('api/v1/namespaces')
 export class NamespacesController {
@@ -28,11 +29,13 @@ export class NamespacesController {
     return await this.namespacesService.getNamespace(namespaceId);
   }
 
+  @NamespaceOwner()
   @Get(':namespaceId/members')
-  async listMembers(@Req() req, @Param('namespaceId') namespaceId: string) {
+  async listMembers(@Param('namespaceId') namespaceId: string) {
     return await this.namespacesService.listMembers(namespaceId);
   }
 
+  @NamespaceOwner()
   @Get(':namespaceId/members/:userId')
   async getMemberByUserId(
     @Param('namespaceId') namespaceId: string,
@@ -41,6 +44,7 @@ export class NamespacesController {
     return await this.namespacesService.getMemberByUserId(namespaceId, userId);
   }
 
+  @NamespaceOwner()
   @Patch(':namespaceId/members/:userId')
   async UpdateMemberRole(
     @Param('namespaceId') namespaceId: string,
@@ -54,6 +58,7 @@ export class NamespacesController {
     );
   }
 
+  @NamespaceOwner()
   @Delete(':namespaceId/members/:userId')
   async deleteMember(
     @Param('namespaceId') namespaceId: string,
@@ -78,6 +83,7 @@ export class NamespacesController {
     );
   }
 
+  @NamespaceOwner()
   @Patch(':namespaceId')
   async update(
     @Param('namespaceId') namespaceId: string,
@@ -86,6 +92,7 @@ export class NamespacesController {
     return await this.namespacesService.update(namespaceId, updateDto);
   }
 
+  @NamespaceOwner()
   @Delete(':namespaceId')
   async delete(@Param('namespaceId') namespaceId: string) {
     return await this.namespacesService.delete(namespaceId);

src/namespaces/namespaces.e2e-spec.ts

@@ -213,10 +213,10 @@ describe('NamespacesController (e2e)', () => {
         .send({ role: NamespaceRole.MEMBER })
         .expect(HttpStatus.OK);
 
-      // Clean up
+      // Members are not allowed to delete the namespace.
       await client
         .delete(`/api/v1/namespaces/${tempNamespaceId}`)
-        .expect(HttpStatus.OK);
+        .expect(HttpStatus.FORBIDDEN);
     });
 
     it('should fail for non-existent member', async () => {
@@ -264,12 +264,9 @@ describe('NamespacesController (e2e)', () => {
         .expect(HttpStatus.OK);
 
       // Verify member was removed by checking if they can still access the namespace
-      // This might still work due to business logic allowing owner access
-
-      // Clean up
       await client
         .delete(`/api/v1/namespaces/${tempNamespaceId}`)
-        .expect(HttpStatus.OK);
+        .expect(HttpStatus.FORBIDDEN);
     });
   });
 
@@ -356,7 +353,7 @@ describe('NamespacesController (e2e)', () => {
       await client
         .patch('/api/v1/namespaces/nonexistent')
         .send(updateNamespaceDto)
-        .expect(HttpStatus.NOT_FOUND);
+        .expect(HttpStatus.FORBIDDEN);
     });
 
     it('should validate update fields', async () => {
@@ -375,7 +372,7 @@ describe('NamespacesController (e2e)', () => {
     it('should succeed even if already deleted (soft delete behavior)', async () => {
       await client
         .delete('/api/v1/namespaces/nonexistent')
-        .expect(HttpStatus.OK);
+        .expect(HttpStatus.FORBIDDEN);
     });
 
     it('should soft delete namespace', async () => {
@@ -455,11 +452,11 @@ describe('NamespacesController (e2e)', () => {
 
       await client
         .get('/api/v1/namespaces/invalid-uuid/members')
-        .expect(HttpStatus.NOT_FOUND);
+        .expect(HttpStatus.FORBIDDEN);
 
       await client
         .get('/api/v1/namespaces/invalid-uuid/members/also-invalid')
-        .expect(HttpStatus.INTERNAL_SERVER_ERROR);
+        .expect(HttpStatus.FORBIDDEN);
     });
 
     it('should handle concurrent namespace operations', async () => {

src/namespaces/namespaces.module.ts

@@ -4,15 +4,16 @@ import { UserModule } from 'omniboxd/user/user.module';
 import { Resource } from 'omniboxd/resources/entities/resource.entity';
 import { NamespacesService } from 'omniboxd/namespaces/namespaces.service';
 import { NamespacesController } from 'omniboxd/namespaces/namespaces.controller';
-import { Namespace } from './entities/namespace.entity';
 import { NamespaceMember } from './entities/namespace-member.entity';
 import { NamespaceResourcesModule } from 'omniboxd/namespace-resources/namespace-resources.module';
 import { PermissionsModule } from 'omniboxd/permissions/permissions.module';
 import { ResourcesModule } from 'omniboxd/resources/resources.module';
+import { Namespace } from './entities/namespace.entity';
+import { NamespaceOwnerInterceptor } from './interceptors/namespace-owner.interceptor';
 
 @Module({
   exports: [NamespacesService],
-  providers: [NamespacesService],
+  providers: [NamespacesService, NamespaceOwnerInterceptor],
   controllers: [NamespacesController],
   imports: [
     UserModule,