Restrict user data access in User query (#2797)

* Restrict user data access in User query Signed-off-by: NishantSinghhhhh <[email protected]> * fixes Signed-off-by: NishantSinghhhhh <[email protected]> * fixes Signed-off-by: NishantSinghhhhh <[email protected]> * fixes for tests Signed-off-by: NishantSinghhhhh <[email protected]> * Rename node-version to .node-version * Update users.ts * fixes for tests Signed-off-by: NishantSinghhhhh <[email protected]> * fixes for failing tests Signed-off-by: NishantSinghhhhh <[email protected]> * full test coverage Signed-off-by: NishantSinghhhhh <[email protected]> * full test coverage Signed-off-by: NishantSinghhhhh <[email protected]> * fixes Signed-off-by: NishantSinghhhhh <[email protected]> * fixes Signed-off-by: NishantSinghhhhh <[email protected]> * removing unnessary changes Signed-off-by: NishantSinghhhhh <[email protected]> * adding tests Signed-off-by: NishantSinghhhhh <[email protected]> * code-Rabbit's changes Signed-off-by: NishantSinghhhhh <[email protected]> * rabbit's changes Signed-off-by: NishantSinghhhhh <[email protected]> * adding tests Signed-off-by: NishantSinghhhhh <[email protected]> * fixes Signed-off-by: NishantSinghhhhh <[email protected]> * removed tests and added tests with full coverage Signed-off-by: NishantSinghhhhh <[email protected]> * linting users Signed-off-by: NishantSinghhhhh <[email protected]> --------- Signed-off-by: NishantSinghhhhh <[email protected]>
PalisadoesFoundation · Dec 31, 2024 · 3a1cd1e · 3a1cd1e
1 parent f998e32
commit 3a1cd1e
Show file tree

Hide file tree

Showing 3 changed files with 296 additions and 5 deletions.
diff --git a/src/libraries/errors/notFoundError.ts b/src/libraries/errors/notFoundError.ts
@@ -5,6 +5,9 @@ import { ApplicationError } from "./applicationError";
  * and is used to handle situations where a requested resource is not found.
  */
 export class NotFoundError extends ApplicationError {
+  code: string | null;
+  param: string | null;
+
   /**
    * Creates an instance of NotFoundError.
    * @param message - The error message. Defaults to "Not Found".
@@ -28,5 +31,8 @@ export class NotFoundError extends ApplicationError {
       },
     ];
     super(errorJson, 404, message);
+
+    this.code = code; // Ensure the code is directly accessible
+    this.param = param; // Ensure the param is directly accessible
   }
 }
diff --git a/src/resolvers/Query/user.ts b/src/resolvers/Query/user.ts
@@ -1,31 +1,64 @@
 import { USER_NOT_FOUND_ERROR } from "../../constants";
 import { errors } from "../../libraries";
 import type { InterfaceAppUserProfile, InterfaceUser } from "../../models";
-import { AppUserProfile, User } from "../../models";
+import { AppUserProfile, User, Organization } from "../../models";
 import type { QueryResolvers } from "../../types/generatedGraphQLTypes";
 /**
  * This query fetch the user from the database.
+ *
+ * This function ensure that users can only query their own data and not access details of other users , protecting sensitive data.
+ *
  * @param _parent-
  * @param args - An object that contains `id` for the user.
  * @param context-
  * @returns An object that contains user data. If the user is not found then it throws a `NotFoundError` error.
  */
+
 export const user: QueryResolvers["user"] = async (_parent, args, context) => {
+  // Check if the current user exists in the system
   const currentUserExists = !!(await User.exists({
     _id: context.userId,
   }));
 
-  if (currentUserExists === false) {
+  if (!currentUserExists) {
+    throw new errors.NotFoundError(
+      USER_NOT_FOUND_ERROR.DESC,
+      USER_NOT_FOUND_ERROR.CODE,
+      USER_NOT_FOUND_ERROR.PARAM,
+    );
+  }
+
+  const [userOrganization, superAdminProfile] = await Promise.all([
+    Organization.exists({
+      members: args.id,
+      admins: context.userId,
+    }),
+    AppUserProfile.exists({
+      userId: context.userId,
+      isSuperAdmin: true,
+    }),
+  ]);
+
+  if (!userOrganization && context.userId !== args.id && !superAdminProfile) {
+    throw new errors.UnauthorizedError(
+      "Access denied. Only the user themselves, organization admins, or super admins can view this profile.",
+    );
+  }
+
+  // Fetch the user data from the database based on the provided ID (args.id)
+  // Fetch the user data from the database based on the provided ID (args.id)
+  const user: InterfaceUser = (await User.findById(
+    args.id,
+  ).lean()) as InterfaceUser;
+
+  if (!user) {
     throw new errors.NotFoundError(
       USER_NOT_FOUND_ERROR.DESC,
       USER_NOT_FOUND_ERROR.CODE,
       USER_NOT_FOUND_ERROR.PARAM,
     );
   }
 
-  const user: InterfaceUser = (await User.findOne({
-    _id: args.id,
-  }).lean()) as InterfaceUser;
   const userAppProfile: InterfaceAppUserProfile = (await AppUserProfile.findOne(
     {
       userId: user._id,

diff --git a/tests/resolvers/User/userAccess.spec.ts b/tests/resolvers/User/userAccess.spec.ts
@@ -0,0 +1,252 @@
+import { USER_NOT_FOUND_ERROR, BASE_URL } from "../../../src/constants";
+import { user as userResolver } from "../../../src/resolvers/Query/user";
+import { User, Organization, AppUserProfile } from "../../../src/models";
+import { connect, disconnect } from "../../helpers/db";
+import type { TestUserType } from "../../helpers/userAndOrg";
+import { createTestUser } from "../../helpers/userAndOrg";
+import { beforeAll, afterAll, describe, it, expect, vi } from "vitest";
+import type mongoose from "mongoose";
+import { Types } from "mongoose";
+import { FundraisingCampaignPledge } from "../../../src/models/FundraisingCampaignPledge";
+import { errors } from "../../../src/libraries";
+
+// Mock FundraisingCampaignPledge populate
+vi.mock("../../../src/models/FundraisingCampaignPledge", () => ({
+  FundraisingCampaignPledge: {
+    schema: {
+      obj: {},
+      paths: {},
+      tree: {},
+      virtuals: {},
+      methods: {},
+      statics: {},
+    },
+  },
+}));
+
+type AdditionalUserFields = {
+  createdAt?: Date;
+  updatedAt?: Date;
+  isAdmin?: boolean;
+  isSuperAdmin?: boolean;
+  blocked?: boolean;
+  role?: string;
+  userType?: string;
+  appLanguageCode?: string;
+  pluginCreationAllowed?: boolean;
+  adminApproved?: boolean;
+  adminFor?: mongoose.Types.ObjectId[];
+  memberOf?: mongoose.Types.ObjectId[];
+  createdOrganizations?: mongoose.Types.ObjectId[];
+  joinedOrganizations?: mongoose.Types.ObjectId[];
+  registeredEvents?: mongoose.Types.ObjectId[];
+  eventAdmin?: mongoose.Types.ObjectId[];
+  createdEvents?: mongoose.Types.ObjectId[];
+  tokenVersion?: number;
+};
+
+type UserType = {
+  _id: mongoose.Types.ObjectId;
+  email: string;
+  firstName?: string;
+  lastName?: string;
+  image?: string | null;
+  organizationsBlockedBy: string[];
+} & AdditionalUserFields;
+
+type ResolverReturnType = {
+  user: UserType;
+};
+
+// Rename `ITestUsers` to `TestInterfaceUsers`
+interface TestInterfaceUsers {
+  testUser: NonNullable<TestUserType>;
+  anotherTestUser: NonNullable<TestUserType>;
+  adminUser: NonNullable<TestUserType>;
+  superAdminUser: NonNullable<TestUserType>;
+}
+
+let users: TestInterfaceUsers;
+let MONGOOSE_INSTANCE: typeof mongoose;
+
+beforeAll(async () => {
+  try {
+    MONGOOSE_INSTANCE = await connect();
+
+    // Register FundraisingCampaignPledge schema mock
+    if (!MONGOOSE_INSTANCE.models.FundraisingCampaignPledge) {
+      MONGOOSE_INSTANCE.model(
+        "FundraisingCampaignPledge",
+        FundraisingCampaignPledge.schema,
+      );
+    }
+
+    // Create users sequentially
+    const [testUser, anotherTestUser, adminUser, superAdminUser] =
+      await Promise.all([
+        createTestUser(),
+        createTestUser(),
+        createTestUser(),
+        createTestUser(),
+      ]);
+
+    // Verify all users were created
+    if (!testUser || !anotherTestUser || !adminUser || !superAdminUser) {
+      throw new Error("Failed to create test users");
+    }
+
+    users = { testUser, anotherTestUser, adminUser, superAdminUser };
+
+    // Create organization
+    const org = await Organization.create({
+      creatorId: users.adminUser._id,
+      members: [users.anotherTestUser._id],
+      admins: [users.adminUser._id],
+      name: "Test Organization",
+      description: "Test organization",
+    });
+
+    if (!org) {
+      throw new Error("Failed to create organization");
+    }
+
+    // Update super admin profile
+    const superAdminUpdate = await AppUserProfile.findOneAndUpdate(
+      { userId: users.superAdminUser._id },
+      { isSuperAdmin: true },
+      { new: true },
+    );
+
+    if (!superAdminUpdate) {
+      throw new Error("Failed to update super admin profile");
+    }
+  } catch (error) {
+    console.error("Setup failed:", error);
+    throw error;
+  }
+}, 30000);
+
+afterAll(async () => {
+  if (users) {
+    await Promise.all([
+      User.deleteMany({
+        _id: { $in: Object.values(users).map((user) => user._id) },
+      }),
+      Organization.deleteMany({}),
+      AppUserProfile.deleteMany({
+        userId: { $in: Object.values(users).map((user) => user._id) },
+      }),
+    ]);
+  }
+  await disconnect(MONGOOSE_INSTANCE);
+}, 30000);
+
+describe("user Query", () => {
+  it("throws error if user doesn't exist", async () => {
+    const args = { id: new Types.ObjectId().toString() };
+    const context = { userId: new Types.ObjectId().toString() };
+
+    await expect(userResolver?.({}, args, context)).rejects.toThrow(
+      USER_NOT_FOUND_ERROR.DESC,
+    );
+  });
+
+  it("throws unauthorized error when trying to access another user's data", async () => {
+    const args = { id: users.anotherTestUser._id.toString() };
+    const context = { userId: users.testUser._id.toString() };
+
+    await expect(userResolver?.({}, args, context)).rejects.toThrow(
+      "Access denied. Only the user themselves, organization admins, or super admins can view this profile.",
+    );
+  });
+
+  it("allows an admin to access another user's data within the same organization", async () => {
+    const args = { id: users.anotherTestUser._id.toString() };
+    const context = {
+      userId: users.adminUser._id.toString(),
+      apiRootURL: BASE_URL,
+    };
+
+    const result = (await userResolver?.(
+      {},
+      args,
+      context,
+    )) as ResolverReturnType;
+    const user = await User.findById(users.anotherTestUser._id).lean();
+
+    expect(result.user._id.toString()).toBe(
+      users.anotherTestUser._id.toString(),
+    );
+    expect(result.user).toEqual({
+      ...user,
+      organizationsBlockedBy: [],
+      image: user?.image ? `${BASE_URL}${user.image}` : null,
+    });
+  });
+
+  it("allows a super admin to access any user's data", async () => {
+    const args = { id: users.anotherTestUser._id.toString() };
+    const context = {
+      userId: users.superAdminUser._id.toString(),
+      apiRootURL: BASE_URL,
+    };
+
+    const result = (await userResolver?.(
+      {},
+      args,
+      context,
+    )) as ResolverReturnType;
+    const user = await User.findById(users.anotherTestUser._id).lean();
+
+    expect(result.user).toEqual({
+      ...user,
+      organizationsBlockedBy: [],
+      image: user?.image ? `${BASE_URL}${user.image}` : null,
+    });
+  });
+
+  it("successfully returns user data when accessing own profile", async () => {
+    const args = { id: users.testUser._id.toString() };
+    const context = {
+      userId: users.testUser._id.toString(),
+      apiRootURL: BASE_URL,
+    };
+
+    const result = (await userResolver?.(
+      {},
+      args,
+      context,
+    )) as ResolverReturnType;
+    const user = await User.findById(users.testUser._id).lean();
+
+    expect(result.user).toEqual({
+      ...user,
+      organizationsBlockedBy: [],
+      image: user?.image ? `${BASE_URL}${user.image}` : null,
+    });
+  });
+
+  it("should throw NotFoundError with correct error details when requested user does not exist but current user exists", async () => {
+    const args = { id: new Types.ObjectId().toString() };
+    const context = { userId: users.superAdminUser._id.toString() };
+
+    await expect(userResolver?.({}, args, context)).rejects.toThrowError(
+      new errors.NotFoundError(
+        USER_NOT_FOUND_ERROR.DESC,
+        USER_NOT_FOUND_ERROR.CODE,
+        USER_NOT_FOUND_ERROR.PARAM,
+      ),
+    );
+  });
+
+  it("should throw UnauthorizedError when trying to access another user's data", async () => {
+    const args = { id: users.anotherTestUser._id.toString() };
+    const context = { userId: users.testUser._id.toString() };
+
+    await expect(userResolver?.({}, args, context)).rejects.toThrowError(
+      new errors.UnauthorizedError(
+        "Access denied. Only the user themselves, organization admins, or super admins can view this profile.",
+      ),
+    );
+  });
+});