Feature/iot 1433 uptimize access control (#285)

* Wip

* More work

* Last fixes

* Package update

* went through endpoints to verify no extra data should be shown

* npm i

* addd minimal on org

* get one with security and removed minimal from org controller

---------

Co-authored-by: Frederik Christ Vestergaard <[email protected]>
Co-authored-by: August Andersen <[email protected]>

Author: 3 people

src/controllers/admin-controller/chirpstack/chirpstack-gateway.controller.ts

@@ -126,7 +126,7 @@ export class ChirpstackGatewayController {
 
   @Put("updateGatewayOrganization/:id")
   @ApiProduces("application/json")
-  @ApiOperation({ summary: "Create a new Chirpstack Gateway" })
+  @ApiOperation({ summary: "Update gateway organization" })
   @ApiBadRequestResponse()
   @GatewayAdmin()
   async changeOrganization(

src/controllers/admin-controller/data-target-log.controller.ts

@@ -2,10 +2,13 @@ import { ComposeAuthGuard } from "@auth/compose-auth.guard";
 import { Read } from "@auth/roles.decorator";
 import { RolesGuard } from "@auth/roles.guard";
 import { ApiAuth } from "@auth/swagger-auth-decorator";
+import { AuthenticatedRequest } from "@dto/internal/authenticated-request";
 import { DatatargetLog } from "@entities/datatarget-log.entity";
-import { Controller, Get, Param, ParseIntPipe, UseGuards } from "@nestjs/common";
+import { ApplicationAccessScope, checkIfUserHasAccessToApplication } from "@helpers/security-helper";
+import { Controller, Get, Param, ParseIntPipe, Req, UseGuards } from "@nestjs/common";
 import { ApiForbiddenResponse, ApiTags, ApiUnauthorizedResponse } from "@nestjs/swagger";
 import { InjectRepository } from "@nestjs/typeorm";
+import { DataTargetService } from "@services/data-targets/data-target.service";
 import { Repository } from "typeorm";
 
 @ApiTags("Data Target Logs")
@@ -18,11 +21,18 @@ import { Repository } from "typeorm";
 export class DatatargetLogController {
   constructor(
     @InjectRepository(DatatargetLog)
-    private datatargetLogRepository: Repository<DatatargetLog>
+    private datatargetLogRepository: Repository<DatatargetLog>,
+    private dataTargetService: DataTargetService
   ) {}
 
   @Get(":datatargetId")
-  async getDatatargetLogs(@Param("datatargetId", new ParseIntPipe()) datatargetId: number): Promise<DatatargetLog[]> {
+  async getDatatargetLogs(
+    @Req() req: AuthenticatedRequest,
+    @Param("datatargetId", new ParseIntPipe()) datatargetId: number
+  ): Promise<DatatargetLog[]> {
+    const dataTarget = await this.dataTargetService.findOne(datatargetId);
+    checkIfUserHasAccessToApplication(req, dataTarget.application.id, ApplicationAccessScope.Read);
+
     return await this.datatargetLogRepository.find({
       where: {
         datatarget: { id: datatargetId },

src/controllers/admin-controller/data-target.controller.ts

@@ -75,12 +75,18 @@ export class DataTargetController {
   @Get(":id")
   @ApiOperation({ summary: "Find DataTarget by id" })
   async findOne(@Req() req: AuthenticatedRequest, @Param("id", new ParseIntPipe()) id: number): Promise<DataTarget> {
+    let dataTarget;
+    try {
+      dataTarget = await this.dataTargetService.findOneWithHasRecentError(id);
+    } catch (err) {
+      throw new NotFoundException(ErrorCodes.IdDoesNotExists);
+    }
+
     try {
-      const dataTarget = await this.dataTargetService.findOneWithHasRecentError(id);
       checkIfUserHasAccessToApplication(req, dataTarget.application.id, ApplicationAccessScope.Read);
       return dataTarget;
     } catch (err) {
-      throw new NotFoundException(ErrorCodes.IdDoesNotExists);
+      throw err;
     }
   }
 
@@ -197,8 +203,13 @@ export class DataTargetController {
 
   @Post("testDataTarget")
   @ApiOperation({ summary: "Send a ping or test data packet to a data target" })
-  async testDataTarget(@Body() testDto: TestDataTargetDto): Promise<TestDataTargetResultDto> {
+  async testDataTarget(
+    @Req() req: AuthenticatedRequest,
+    @Body() testDto: TestDataTargetDto
+  ): Promise<TestDataTargetResultDto> {
+    const dataTarget = await this.dataTargetService.findOne(testDto.dataTargetId);
+    checkIfUserHasAccessToApplication(req, dataTarget.application.id, ApplicationAccessScope.Read);
     // Send package
-    return await this.dataTargetService.testDataTarget(testDto);
+    return await this.dataTargetService.testDataTarget(testDto, dataTarget);
   }
 }

src/controllers/admin-controller/iot-device-payload-decoder-data-target-connection.controller.ts

@@ -56,38 +56,6 @@ export class IoTDevicePayloadDecoderDataTargetConnectionController {
     private iotDeviceService: IoTDeviceService
   ) {}
 
-  @Get()
-  @ApiProduces("application/json")
-  @ApiOperation({
-    summary: "Find all connections between IoT-Devices, PayloadDecoders and DataTargets (paginated)",
-  })
-  @ApiResponse({
-    status: 200,
-    description: "Success",
-    type: ListAllApplicationsResponseDto,
-  })
-  async findAll(
-    @Req() req: AuthenticatedRequest,
-    @Query() query?: ListAllEntitiesDto
-  ): Promise<ListAllConnectionsResponseDto> {
-    if (req.user.permissions.isGlobalAdmin) {
-      return await this.service.findAndCountWithPagination(query);
-    } else {
-      const allowed = req.user.permissions.getAllApplicationsWithAtLeastRead();
-      return await this.service.findAndCountWithPagination(query, allowed);
-    }
-  }
-
-  @Get(":id")
-  @ApiNotFoundResponse({
-    description: "If the id of the entity doesn't exist",
-  })
-  async findOne(
-    @Req() req: AuthenticatedRequest,
-    @Param("id", new ParseIntPipe()) id: number
-  ): Promise<IoTDevicePayloadDecoderDataTargetConnection> {
-    return await this.service.findOne(id);
-  }
 
   @Get("byIoTDevice/:id")
   @ApiOperation({
@@ -104,24 +72,6 @@ export class IoTDevicePayloadDecoderDataTargetConnectionController {
     }
   }
 
-  @Get("byPayloadDecoder/:id")
-  @ApiOperation({
-    summary: "Find all connections by PayloadDecoder id",
-  })
-  async findByPayloadDecoderId(
-    @Req() req: AuthenticatedRequest,
-    @Param("id", new ParseIntPipe()) id: number
-  ): Promise<ListAllConnectionsResponseDto> {
-    if (req.user.permissions.isGlobalAdmin) {
-      return await this.service.findAllByPayloadDecoderId(id);
-    } else {
-      return await this.service.findAllByPayloadDecoderId(
-        id,
-        req.user.permissions.getAllOrganizationsWithAtLeastApplicationRead()
-      );
-    }
-  }
-
   @Get("byDataTarget/:id")
   @ApiOperation({
     summary: "Find all connections by DataTarget id",
@@ -196,7 +146,7 @@ export class IoTDevicePayloadDecoderDataTargetConnectionController {
   }
 
   private async checkIfUpdateIsAllowed(updateDto: UpdateConnectionDto, req: AuthenticatedRequest, id: number) {
-    const newIotDevice = await this.iotDeviceService.findOne(updateDto.iotDeviceIds[0]);
+    const newIotDevice = await this.iotDeviceService.findOneWithApplicationAndMetadata(updateDto.iotDeviceIds[0]);
     checkIfUserHasAccessToApplication(req, newIotDevice.application.id, ApplicationAccessScope.Write);
     const oldConnection = await this.service.findOne(id);
     await this.checkUserHasWriteAccessToAllIotDevices(updateDto.iotDeviceIds, req);

src/controllers/admin-controller/iot-device-payload-decoder.controller.ts

@@ -31,7 +31,29 @@ export class IoTDevicePayloadDecoderController {
     @Query() query: PayloadDecoderIoDeviceMinimalQuery
   ): Promise<ListAllIoTDevicesMinimalResponseDto> {
     try {
-      return await this.iotDeviceService.findAllByPayloadDecoder(req, payloadDecoderId, +query.limit, +query.offset);
+      const iotDevices = await this.iotDeviceService.findAllByPayloadDecoder(
+        req,
+        payloadDecoderId,
+        +query.limit,
+        +query.offset
+      );
+
+      if (req.user.permissions.isGlobalAdmin) {
+        return iotDevices;
+      }
+
+      const allowedAppIds = req.user.permissions.getAllApplicationsWithAtLeastRead();
+
+      const filteredIotDevices = iotDevices.data.filter(device =>
+        allowedAppIds.find(appId => appId === device.applicationId)
+      );
+
+      const response: ListAllIoTDevicesMinimalResponseDto = {
+        data: filteredIotDevices,
+        count: filteredIotDevices.length,
+      };
+
+      return response;
     } catch (err) {
       throw new NotFoundException(ErrorCodes.IdDoesNotExists);
     }

src/controllers/admin-controller/iot-device.controller.ts

@@ -433,6 +433,7 @@ export class IoTDeviceController {
       return new StreamableFile(csvFile);
     } catch (err) {
       this.logger.error(err);
+      throw err;
     }
   }
 }

src/controllers/admin-controller/test-payload-decoder.controller.ts

@@ -1,5 +1,6 @@
+import { AuthenticatedRequest } from "@dto/internal/authenticated-request";
 import { TestPayloadDecoderDto } from "@dto/test-payload-decoder.dto";
-import { BadRequestException, Body, Controller, Post } from "@nestjs/common";
+import { BadRequestException, Body, Controller, Post, Req } from "@nestjs/common";
 import { ApiOperation, ApiTags } from "@nestjs/swagger";
 import { PayloadDecoderExecutorService } from "@services/data-management/payload-decoder-executor.service";
 

src/controllers/user-management/new-kombit-creation.controller.ts

@@ -19,7 +19,6 @@ import {
   Param,
   ParseIntPipe,
   Put,
-  Query,
   Req,
   UseGuards,
 } from "@nestjs/common";
@@ -35,6 +34,7 @@ import { OrganizationService } from "@services/user-management/organization.serv
 import { PermissionService } from "@services/user-management/permission.service";
 import { UserService } from "@services/user-management/user.service";
 import { ApiAuth } from "@auth/swagger-auth-decorator";
+import { checkIfUserHasAccessToUser } from "@helpers/security-helper";
 
 @UseGuards(JwtAuthGuard)
 @ApiAuth()
@@ -133,19 +133,28 @@ export class NewKombitCreationController {
 
   @Get(":id")
   @ApiOperation({ summary: "Get one user" })
-  async find(
-    @Param("id", new ParseIntPipe()) id: number,
-    @Query("extendedInfo") extendedInfo?: boolean
-  ): Promise<UserResponseDto> {
-    const getExtendedInfo = extendedInfo != null ? extendedInfo : false;
+  async find(@Req() req: AuthenticatedRequest, @Param("id", new ParseIntPipe()) id: number): Promise<UserResponseDto> {
+    let dbUser;
+
+    try {
+      dbUser = await this.userService.findOne(id);
+    } catch (err) {
+      throw new NotFoundException(ErrorCodes.IdDoesNotExists);
+    }
+
     try {
+      checkIfUserHasAccessToUser(req, dbUser);
+
+      dbUser.permissions.forEach(perm => {
+        delete perm.organization;
+      });
+
       // Don't leak the passwordHash
-      // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      const { passwordHash, ...user } = await this.userService.findOne(id, getExtendedInfo);
+      const { passwordHash: _, ...user } = dbUser;
 
       return user;
     } catch (err) {
-      throw new NotFoundException(ErrorCodes.IdDoesNotExists);
+      throw err;
     }
   }
 }

src/controllers/user-management/organization.controller.ts

@@ -89,15 +89,6 @@ export class OrganizationController {
     }
   }
 
-  @Get("minimal")
-  @ApiOperation({
-    summary: "Get list of the minimal representation of organizations, i.e. id and name.",
-  })
-  @Read()
-  async findAllMinimal(): Promise<ListAllMinimalOrganizationsResponseDto> {
-    return await this.organizationService.findAllMinimal();
-  }
-
   @Get()
   @ApiOperation({ summary: "Get list of all Organizations" })
   @UserAdmin()

src/controllers/user-management/user.controller.ts

@@ -28,6 +28,7 @@ import { UserResponseDto } from "@dto/user-response.dto";
 import { ErrorCodes } from "@entities/enum/error-codes.enum";
 import {
   checkIfUserHasAccessToOrganization,
+  checkIfUserHasAccessToUser,
   checkIfUserIsGlobalAdmin,
   OrganizationAccessScope,
 } from "@helpers/security-helper";
@@ -54,12 +55,6 @@ export class UserController {
 
   constructor(private userService: UserService, private organizationService: OrganizationService) {}
 
-  @Get("minimal")
-  @ApiOperation({ summary: "Get all id,names of users" })
-  async findAllMinimal(): Promise<ListAllUsersMinimalResponseDto> {
-    return await this.userService.findAllMinimal();
-  }
-
   @Post()
   @ApiOperation({ summary: "Create a new User" })
   async create(@Req() req: AuthenticatedRequest, @Body() createUserDto: CreateUserDto): Promise<UserResponseDto> {
@@ -189,18 +184,28 @@ export class UserController {
 
   @Get(":id")
   @ApiOperation({ summary: "Get one user" })
-  async find(
-    @Param("id", new ParseIntPipe()) id: number,
-    @Query("extendedInfo") extendedInfo?: boolean
-  ): Promise<UserResponseDto> {
-    const getExtendedInfo = extendedInfo != null ? extendedInfo : false;
+  async find(@Req() req: AuthenticatedRequest, @Param("id", new ParseIntPipe()) id: number): Promise<UserResponseDto> {
+    let dbUser;
+
+    try {
+      dbUser = await this.userService.findOne(id);
+    } catch (err) {
+      throw new NotFoundException(ErrorCodes.IdDoesNotExists);
+    }
+
     try {
+      checkIfUserHasAccessToUser(req, dbUser);
+
+      dbUser.permissions.forEach(perm => {
+        delete perm.organization;
+      });
+
       // Don't leak the passwordHash
-      const { passwordHash: _, ...user } = await this.userService.findOne(id, getExtendedInfo);
+      const { passwordHash: _, ...user } = dbUser;
 
       return user;
     } catch (err) {
-      throw new NotFoundException(ErrorCodes.IdDoesNotExists);
+      throw err;
     }
   }
 
@@ -213,13 +218,13 @@ export class UserController {
     @Param("organizationId", new ParseIntPipe()) organizationId: number,
     @Query() query?: ListAllEntitiesDto
   ): Promise<ListAllUsersResponseDto> {
-    try {
-      // Check if user has access to organization
-      if (!req.user.permissions.hasUserAdminOnOrganization(organizationId)) {
-        throw new ForbiddenException("User does not have org admin permissions for this organization");
-      }
+    // Check if user has access to organization
+    if (!req.user.permissions.hasUserAdminOnOrganization(organizationId)) {
+      throw new ForbiddenException("User does not have org admin permissions for this organization");
+    }
 
-      // Get user objects
+    // Get user objects
+    try {
       return await this.userService.getUsersOnOrganization(organizationId, query);
     } catch (err) {
       throw new NotFoundException(ErrorCodes.IdDoesNotExists);