fix(backend): resolve security advisories for IDOR and onboarding bypass (#5897)

Improve error handling in the onboarding status check

---

Co-authored-by: James George <25279263+jamesgeorge007@users.noreply.github.com>

packages/hoppscotch-backend/src/errors.ts

@@ -783,6 +783,20 @@ export const INFRA_CONFIG_SERVICE_NOT_CONFIGURED =
 export const INFRA_CONFIG_OPERATION_NOT_ALLOWED =
   'infra_config/operation_not_allowed';
 
+/**
+ * Error message for when the onboarding status fetch fails
+ * (InfraConfigService)
+ */
+export const INFRA_CONFIG_FETCH_FAILED =
+  'infra_config/fetch_failed' as const;
+
+/**
+ * Onboarding has already been completed and cannot be re-run
+ * (OnboardingController)
+ */
+export const ONBOARDING_CANNOT_BE_RERUN =
+  'onboarding/cannot_be_rerun' as const;
+
 /**
  * Error message for when the database table does not exist
  * (InfraConfigService)

packages/hoppscotch-backend/src/infra-config/infra-config.service.ts

@@ -7,6 +7,7 @@ import { InfraConfigEnum } from 'src/types/InfraConfig';
 import {
   AUTH_PROVIDER_NOT_SPECIFIED,
   DATABASE_TABLE_NOT_EXIST,
+  INFRA_CONFIG_FETCH_FAILED,
   INFRA_CONFIG_INVALID_INPUT,
   INFRA_CONFIG_NOT_FOUND,
   INFRA_CONFIG_RESET_FAILED,
@@ -512,13 +513,17 @@ export class InfraConfigService implements OnModuleInit, OnModuleDestroy {
    * @returns GetOnboardingStatusResponse
    */
   async getOnboardingStatus() {
-    const configMap = await this.getInfraConfigsMap();
-    const usersCount = await this.userService.getUsersCount();
+    try {
+      const configMap = await this.getInfraConfigsMap();
+      const usersCount = await this.userService.getUsersCount();
 
-    return E.right({
-      onboardingCompleted: configMap.ONBOARDING_COMPLETED === 'true',
-      canReRunOnboarding: usersCount === 0,
-    } as GetOnboardingStatusResponse);
+      return E.right({
+        onboardingCompleted: configMap.ONBOARDING_COMPLETED === 'true',
+        canReRunOnboarding: usersCount === 0,
+      } as GetOnboardingStatusResponse);
+    } catch {
+      return E.left(INFRA_CONFIG_FETCH_FAILED);
+    }
   }
 
   /**

packages/hoppscotch-backend/src/infra-config/onboarding.controller.ts

@@ -11,6 +11,7 @@ import { InfraConfigService } from './infra-config.service';
 import { RESTError } from 'src/types/RESTError';
 import { throwHTTPErr } from 'src/utils';
 import * as E from 'fp-ts/Either';
+import { ONBOARDING_CANNOT_BE_RERUN } from 'src/errors';
 import {
   GetOnboardingConfigResponse,
   GetOnboardingStatusResponse,
@@ -60,6 +61,24 @@ export class OnboardingController {
     type: SaveOnboardingConfigResponse,
   })
   async updateOnboardingConfig(@Body() dto: SaveOnboardingConfigRequest) {
+    const onboardingStatus =
+      await this.infraConfigService.getOnboardingStatus();
+
+    if (E.isLeft(onboardingStatus))
+      throwHTTPErr(<RESTError>{
+        message: onboardingStatus.left,
+        statusCode: HttpStatus.UNPROCESSABLE_ENTITY,
+      });
+
+    if (
+      onboardingStatus.right.onboardingCompleted &&
+      !onboardingStatus.right.canReRunOnboarding
+    )
+      throwHTTPErr(<RESTError>{
+        message: ONBOARDING_CANNOT_BE_RERUN,
+        statusCode: HttpStatus.BAD_REQUEST,
+      });
+
     const updateConfigResult =
       await this.infraConfigService.updateOnboardingConfig(dto);
 

packages/hoppscotch-backend/src/user-environment/user-environments.resolver.ts

@@ -80,6 +80,7 @@ export class UserEnvironmentsResolver {
   })
   @UseGuards(GqlAuthGuard)
   async updateUserEnvironment(
+    @GqlUser() user: User,
     @Args({
       name: 'id',
       description: 'ID of the user environment',
@@ -103,6 +104,7 @@ export class UserEnvironmentsResolver {
         id,
         name,
         variables,
+        user
       );
     if (E.isLeft(userEnvironment)) throwErr(userEnvironment.left);
     return userEnvironment.right;

packages/hoppscotch-backend/src/user-environment/user-environments.service.spec.ts

@@ -9,10 +9,24 @@ import {
   USER_ENVIRONMENT_INVALID_ENVIRONMENT_NAME,
 } from '../errors';
 import { PubSubService } from '../pubsub/pubsub.service';
+import { User } from '../user/user.model';
 
 const mockPrisma = mockDeep<PrismaService>();
 const mockPubSub = mockDeep<PubSubService>();
 
+const mockUser: User = {
+  uid: 'abc123',
+  displayName: 'Test User',
+  email: 'support@example.com',
+  photoURL: 'https://example.com/profile.jpg',
+  isAdmin: false,
+  lastLoggedOn: new Date(),
+  lastActiveOn: new Date(),
+  createdOn: new Date(),
+  currentRESTSession: JSON.stringify({}),
+  currentGQLSession: JSON.stringify({}),
+};
+
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-ignore
 const userEnvironmentsService = new UserEnvironmentsService(
@@ -301,6 +315,7 @@ describe('UserEnvironmentsService', () => {
           'abc123',
           'test',
           '[{}]',
+          mockUser,
         ),
       ).toEqualRight(result);
     });
@@ -327,6 +342,7 @@ describe('UserEnvironmentsService', () => {
           'abc123',
           null,
           '[{}]',
+          mockUser,
         ),
       ).toEqualRight(result);
     });
@@ -341,6 +357,7 @@ describe('UserEnvironmentsService', () => {
           'abc123',
           'test',
           '[{}]',
+          mockUser,
         ),
       ).toEqualLeft(USER_ENVIRONMENT_ENV_DOES_NOT_EXISTS);
     });
@@ -366,6 +383,7 @@ describe('UserEnvironmentsService', () => {
         'abc123',
         'test',
         '[{}]',
+        mockUser,
       );
 
       return expect(mockPubSub.publish).toHaveBeenCalledWith(
@@ -395,6 +413,7 @@ describe('UserEnvironmentsService', () => {
         'abc123',
         null,
         '[{}]',
+        mockUser,
       );
 
       return expect(mockPubSub.publish).toHaveBeenCalledWith(

packages/hoppscotch-backend/src/user-environment/user-environments.service.ts

@@ -14,6 +14,7 @@ import {
   USER_ENVIRONMENT_INVALID_ENVIRONMENT_NAME,
 } from '../errors';
 import { stringToJson } from '../utils';
+import { User } from '../user/user.model';
 
 @Injectable()
 export class UserEnvironmentsService {
@@ -128,14 +129,20 @@ export class UserEnvironmentsService {
    * @param id environment id
    * @param name environments name
    * @param variables environment variables
+   * @param user User object for authorization
    * @returns an Either of `UserEnvironment` or error
    */
-  async updateUserEnvironment(id: string, name: string, variables: string) {
+  async updateUserEnvironment(
+    id: string,
+    name: string,
+    variables: string,
+    user: User,
+  ) {
     const envVariables = stringToJson(variables);
     if (E.isLeft(envVariables)) return E.left(envVariables.left);
     try {
       const updatedEnvironment = await this.prisma.userEnvironment.update({
-        where: { id: id },
+        where: { id: id, userUid: user.uid },
         data: {
           name: name,
           variables: envVariables.right,
@@ -179,6 +186,7 @@ export class UserEnvironmentsService {
       const deletedEnvironment = await this.prisma.userEnvironment.delete({
         where: {
           id: id,
+          userUid: uid,
         },
       });
 
@@ -238,7 +246,7 @@ export class UserEnvironmentsService {
     if (env.id === id) {
       try {
         const updatedEnvironment = await this.prisma.userEnvironment.update({
-          where: { id: id },
+          where: { id: id, userUid: uid },
           data: {
             variables: [],
           },