Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revised PR: Restrict user data access in User query (replaces #2623) #2646

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions src/resolvers/Query/user.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,62 @@
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)
const user: InterfaceUser = (await User.findById(
args.id,
).lean()) as InterfaceUser;
nitintwt marked this conversation as resolved.
Show resolved Hide resolved

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,
Expand Down
190 changes: 190 additions & 0 deletions tests/resolvers/Query/userAccess.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
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 } from "vitest";
import type mongoose from "mongoose";
import { Types } from "mongoose";
import { FundraisingCampaignPledge } from "../../../src/models/FundraisingCampaignPledge";
import { deleteUserFromCache } from "../../../src/services/UserCache/deleteUserFromCache";

let testUser: TestUserType;
let anotherTestUser: TestUserType;
let adminUser: TestUserType;
let superAdminUser: TestUserType;
let MONGOOSE_INSTANCE: typeof mongoose;

beforeAll(async () => {
MONGOOSE_INSTANCE = await connect();
await deleteUserFromCache(testUser?.id);
const pledges = await FundraisingCampaignPledge.find({
_id: new Types.ObjectId(),
}).lean();
console.log(pledges);
try {
testUser = await createTestUser();
if (!testUser?.id) throw new Error("Failed to create test user");
anotherTestUser = await createTestUser();
if (!anotherTestUser?.id)
throw new Error("Failed to create another test user");
adminUser = await createTestUser();
if (!adminUser?.id) throw new Error("Failed to create admin user");
superAdminUser = await createTestUser();
if (!superAdminUser?.id)
throw new Error("Failed to create super admin user");

const org = await Organization.create({
creatorId: adminUser?.id,
members: [anotherTestUser?.id],
admins: [adminUser?.id],
name: "Test Organization",
description: "A test organization for user query testing",
});
if (!org) throw new Error("Failed to create organization");

const profile = await AppUserProfile.create({
userId: superAdminUser?.id,
isSuperAdmin: true,
});
if (!profile) throw new Error("Failed to create super admin profile");
} catch (error) {
console.error("Failed to set up test data:", error);
throw error;
}
});

afterAll(async () => {
await Promise.all([
User.deleteMany({
_id: {
$in: [
testUser?.id,
anotherTestUser?.id,
adminUser?.id,
superAdminUser?.id,
],
},
}),
Organization.deleteMany({}),
AppUserProfile.deleteMany({ userId: superAdminUser?.id }),
]);
await disconnect(MONGOOSE_INSTANCE);
});

describe("user Query", () => {
// Test case 1: Invalid user ID scenario
it("throws error if user doesn't exist", async () => {
expect.assertions(2);
const args = {
id: new Types.ObjectId().toString(),
};

const context = {
userId: testUser?.id,
};

try {
await userResolver?.({}, args, context);
} catch (error) {
if (!(error instanceof Error)) {
throw new Error("Unexpected error type");
}
expect(error).toBeInstanceOf(Error);
expect(error.message).toEqual(USER_NOT_FOUND_ERROR.DESC);

Check failure on line 95 in tests/resolvers/Query/userAccess.spec.ts

View workflow job for this annotation

GitHub Actions / Testing Application (22.x)

tests/resolvers/Query/userAccess.spec.ts > user Query > throws error if user doesn't exist

AssertionError: expected 'Access denied. Only the user themselv…' to deeply equal 'User not found' Expected: "User not found" Received: "Access denied. Only the user themselves, organization admins, or super admins can view this profile." ❯ tests/resolvers/Query/userAccess.spec.ts:95:29
}
});

// Test case 2: Unauthorized access scenario
it("throws unauthorized error when trying to access another user's data", async () => {
expect.assertions(1);
const args = {
id: anotherTestUser?.id,
};

const context = {
userId: testUser?.id,
};

try {
await userResolver?.({}, args, context);
} catch (error: unknown) {
expect((error as Error).message).toEqual(
"Access denied. Only the user themselves, organization admins, or super admins can view this profile.",
);
}
});

// Test case 3: Admin access scenario
it("allows an admin to access another user's data within the same organization", async () => {
expect.assertions(2);
const args = {
id: anotherTestUser?.id,
};

const context = {
userId: adminUser?.id,
apiRootURL: BASE_URL,
};

const org = await Organization.findOne({ admins: adminUser?._id });
expect(org?.members).toContain(anotherTestUser?._id);

Check failure on line 132 in tests/resolvers/Query/userAccess.spec.ts

View workflow job for this annotation

GitHub Actions / Testing Application (22.x)

tests/resolvers/Query/userAccess.spec.ts > user Query > allows an admin to access another user's data within the same organization

AssertionError: expected [ …(1) ] to include new ObjectId("672fa75ab3c30d0c68d8e51d") ❯ tests/resolvers/Query/userAccess.spec.ts:132:26

const result = await userResolver?.({}, args, context);

const user = await User.findById(anotherTestUser?._id).lean();

expect(result?.user).toEqual({
...user,
organizationsBlockedBy: [],
image: user?.image ? `${BASE_URL}${user.image}` : null,
});
});

// Test case 4: SuperAdmin access scenario
it("allows a super admin to access any user's data", async () => {
expect.assertions(1);
const args = {
id: anotherTestUser?.id,
};

const context = {
userId: superAdminUser?.id,
apiRootURL: BASE_URL,
};

const result = await userResolver?.({}, args, context);

const user = await User.findById(anotherTestUser?._id).lean();

expect(result?.user).toEqual({
...user,
organizationsBlockedBy: [],
image: user?.image ? `${BASE_URL}${user.image}` : null,
});
});

// Test case 5: Successful access to own profile
it("successfully returns user data when accessing own profile", async () => {
expect.assertions(1);
const args = {
id: testUser?.id,
};

const context = {
userId: testUser?.id,
apiRootURL: BASE_URL,
};

const result = await userResolver?.({}, args, context);

const user = await User.findById(testUser?._id).lean();

expect(result?.user).toEqual({
...user,
organizationsBlockedBy: [],
image: user?.image ? `${BASE_URL}${user.image}` : null,
});
});
});
Loading