Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions api/src/main/controllers/UserController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class UserController {
return res.status(401).send({ error: 'Authentication required' });
}

const user = await this.userService.findByUsername(req.user.username);
const user = await this.userService.findByUsername(req.user.username, req.user);
res.json({ username: user.username, role: user.role });
} catch (err: any) {
res.status(500).send({ error: err.message });
Expand Down Expand Up @@ -89,7 +89,7 @@ class UserController {
.send({ error: 'INVALID DATA: Offset must be a non-negative number' });
}

const users = await this.userService.getUsers(q, searchLimit, searchOffset);
const users = await this.userService.getUsers(req.user, q, searchLimit, searchOffset);
const total = await this.userService.countUsers(q);
return res.json({
data: users,
Expand All @@ -108,7 +108,7 @@ class UserController {

async getByUsername(req: any, res: any) {
try {
const user = await this.userService.findByUsername(req.params.username);
const user = await this.userService.findByUsername(req.params.username, req.user);
res.json(user);
} catch (err: any) {
res.status(404).send({ error: err.message });
Expand Down
4 changes: 2 additions & 2 deletions api/src/main/repositories/mongoose/OrganizationRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class OrganizationRepository extends RepositoryBase {
const organization = await OrganizationMongoose.findOne({ _id: organizationId })
.populate({
path: 'ownerDetails',
select: '-password',
select: '-password -apiKey -role',
})
.exec();

Expand Down Expand Up @@ -65,7 +65,7 @@ class OrganizationRepository extends RepositoryBase {
})
.populate({
path: 'ownerDetails',
select: '-password',
select: '-password -apiKey -role',
})
.exec();

Expand Down
15 changes: 8 additions & 7 deletions api/src/main/repositories/mongoose/UserRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class UserRepository extends RepositoryBase {

async findAll(query?: any): Promise<LeanUser[]> {
try {
const users = await UserMongoose.find(query || {}, { password: 0 }).exec();
const users = await UserMongoose.find(query || {}, { password: 0, apiKey: 0 }).exec();
return users.map(user => user.toObject({ getters: true, virtuals: true, versionKey: false }) as unknown as LeanUser);
} catch (err) {
return [];
Expand All @@ -18,7 +18,7 @@ class UserRepository extends RepositoryBase {

async find(username: string, limit: number = 10, offset: number = 0): Promise<LeanUser[]> {
try {
const users = await UserMongoose.find({ username: { $regex: username, $options: 'i' } }, { password: 0 }).skip(offset).limit(limit).exec();
const users = await UserMongoose.find({ username: { $regex: username, $options: 'i' } }).select({ password: 0 }).skip(offset).limit(limit).exec();
return users.map(user => user.toObject({ getters: true, virtuals: true, versionKey: false }) as unknown as LeanUser);
} catch (err) {
return [];
Expand All @@ -35,20 +35,21 @@ class UserRepository extends RepositoryBase {

async findByUsername(username: string): Promise<LeanUser | null> {
try {
const user = (await this.find(username))[0];
const user = await UserMongoose.findOne({ username })
.select({ password: 0 })
.exec();

if (!user) return null;


return user as unknown as LeanUser;
return user.toObject({ getters: true, virtuals: true, versionKey: false }) as unknown as LeanUser;
} catch (err) {
return null;
}
}

async findByRole(role: UserRole): Promise<LeanUser[]> {
try {
const users = await UserMongoose.find({ role: role }, { password: 0 }).exec();
const users = await UserMongoose.find({ role: role }, { password: 0, apiKey: 0 }).exec();
return users.map(user => user.toObject({ getters: true, virtuals: true, versionKey: false }) as unknown as LeanUser);
} catch (err) {
return [];
Expand Down Expand Up @@ -95,7 +96,7 @@ class UserRepository extends RepositoryBase {
async update(username: string, userData: any) {
const updatedUser = await UserMongoose.findOneAndUpdate({ username: username }, userData, {
new: true,
projection: { password: 0 },
projection: { password: 0, apiKey: 0 },
})

if (!updatedUser) {
Expand Down
34 changes: 27 additions & 7 deletions api/src/main/services/UserService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,44 @@ class UserService {
private userRepository: UserRepository;
private organizationService: OrganizationService;

private sanitizeUserForRequester(user: LeanUser, requester?: LeanUser, targetUsername?: string): LeanUser {
if (!requester || requester.role === 'ADMIN') {
return user;
}

const effectiveTargetUsername = targetUsername || user.username;
const isOwnData = effectiveTargetUsername === requester.username;

if (isOwnData) {
return user;
}

const { role, apiKey, ...safeUser } = user as any;
return safeUser as LeanUser;
}

constructor() {
this.userRepository = container.resolve('userRepository');
this.organizationService = container.resolve('organizationService');
}

async getUsers(query: string = '', limit: number = 10, offset: number = 0): Promise<LeanUser[]> {
return await this.userRepository.find(query.trim(), limit, offset);
async getUsers(reqUser: LeanUser, query: string = '', limit: number = 10, offset: number = 0): Promise<LeanUser[]> {

const users = await this.userRepository.find(query.trim(), limit, offset);

return users.map(user => this.sanitizeUserForRequester(user, reqUser, user.username));
}

async countUsers(query: string = '') {
return this.userRepository.count(query.trim());
}

async findByUsername(username: string) {
async findByUsername(username: string, reqUser: LeanUser): Promise<LeanUser> {
const user = await this.userRepository.findByUsername(username);
if (!user) {
throw new Error('INVALID DATA: User not found');
}
return user;
return this.sanitizeUserForRequester(user, reqUser, username);
}

async findByApiKey(apiKey: string) {
Expand Down Expand Up @@ -61,7 +80,7 @@ class UserService {
createdUser
);

return createdUser;
return this.sanitizeUserForRequester(createdUser, creatorData, createdUser.username);
}

async update(username: string, userData: any, creatorData: LeanUser) {
Expand Down Expand Up @@ -106,7 +125,7 @@ class UserService {
await this.organizationService.updateUsername(username, userData.username);
}

return updatedUser;
return this.sanitizeUserForRequester(updatedUser, creatorData, username);
}

async regenerateApiKey(username: string, reqUser: LeanUser): Promise<string> {
Expand Down Expand Up @@ -150,7 +169,8 @@ class UserService {
}
}

return this.userRepository.changeRole(username, role);
const updatedUser = await this.userRepository.changeRole(username, role);
return this.sanitizeUserForRequester(updatedUser, creatorData, username);
}

async authenticate(username: string, password: string): Promise<LeanUser> {
Expand Down
152 changes: 152 additions & 0 deletions api/src/test/user.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ describe('User API routes', function () {
}
};

const expectNoSensitiveUserFields = (user: any) => {
expect(user.role).toBeUndefined();
expect(user.apiKey).toBeUndefined();
};

const expectNoSensitiveFieldsForOtherUsers = (users: any[], requesterUsername: string) => {
users
.filter(user => user?.username && user.username !== requesterUsername)
.forEach(user => expectNoSensitiveUserFields(user));
};

beforeAll(async function () {
app = await getApp();
adminUser = await createTestUser('ADMIN');
Expand Down Expand Up @@ -732,6 +743,18 @@ describe('User API routes', function () {
});

it('returns 403 when trying to demote the last admin', async function () {
const allUsersResponse = await request(app)
.get(`${baseUrl}/users?limit=50`)
.set('x-api-key', adminApiKey);

const allAdmins = allUsersResponse.body.data.filter((u: any) => u.role === 'ADMIN');

for (const admin of allAdmins) {
if (admin.username !== adminUser.username) {
await deleteTestUser(admin.username);
}
}

const response = await request(app)
.put(`${baseUrl}/users/${adminUser.username}/role`)
.set('x-api-key', adminApiKey)
Expand Down Expand Up @@ -980,4 +1003,133 @@ describe('User API routes', function () {
expect(response.body.error).toContain('API Key');
});
});

describe('USER privacy on user endpoints', function () {
it('GET /users with search never exposes role or apiKey for other users', async function () {
const requester = await createTestUser('USER', `privacy_viewer_${Date.now()}`);
const otherUser1 = await createTestUser('USER', `privacy_target_1_${Date.now()}`);
const otherUser2 = await createTestUser('USER', `privacy_target_2_${Date.now()}`);

trackUserForCleanup(requester);
trackUserForCleanup(otherUser1);
trackUserForCleanup(otherUser2);

const response = await request(app)
.get(`${baseUrl}/users?q=privacy_target_`)
.set('x-api-key', requester.apiKey);

expect(response.status).toBe(200);
expect(Array.isArray(response.body.data)).toBeTruthy();
expect(response.body.data.length).toBeGreaterThanOrEqual(2);
expectNoSensitiveFieldsForOtherUsers(response.body.data, requester.username);
});

it('GET /users/:username never exposes role or apiKey when USER requests another user', async function () {
const requester = await createTestUser('USER', `privacy_reader_${Date.now()}`);
const targetUser = await createTestUser('USER', `privacy_profile_target_${Date.now()}`);

trackUserForCleanup(requester);
trackUserForCleanup(targetUser);

const response = await request(app)
.get(`${baseUrl}/users/${targetUser.username}`)
.set('x-api-key', requester.apiKey);

expect(response.status).toBe(200);
expect(response.body.username).toBe(targetUser.username);
expectNoSensitiveUserFields(response.body);
});

it('POST /users never exposes role or apiKey when USER creates another user', async function () {
const creator = await createTestUser('USER', `privacy_creator_${Date.now()}`);
trackUserForCleanup(creator);

const userData = {
username: `privacy_created_${Date.now()}`,
password: 'password123',
role: USER_ROLES[USER_ROLES.length - 1],
};

const response = await request(app)
.post(`${baseUrl}/users`)
.set('x-api-key', creator.apiKey)
.send(userData);

expect(response.status).toBe(201);
expect(response.body.username).toBe(userData.username);
expectNoSensitiveUserFields(response.body);
trackUserForCleanup(response.body);
});

it('PUT /users/:username never exposes role or apiKey when USER updates another user', async function () {
const requester = await createTestUser('USER', `privacy_updater_${Date.now()}`);
const targetUser = await createTestUser('USER', `privacy_update_target_${Date.now()}`);
const updatedUsername = `upd_${Date.now()}_${Math.floor(Math.random() * 10000)}`;

trackUserForCleanup(requester);
trackUserForCleanup(targetUser);

const response = await request(app)
.put(`${baseUrl}/users/${targetUser.username}`)
.set('x-api-key', requester.apiKey)
.send({ username: updatedUsername });

expect([200, 403, 404, 422]).toContain(response.status);
expect(response.body.role).toBeUndefined();
expect(response.body.apiKey).toBeUndefined();

if (response.status === 200) {
expect(response.body.username).toBe(updatedUsername);
trackUserForCleanup(response.body);
}
});

it('PUT /users/:username/api-key forbidden response does not expose sensitive user fields', async function () {
const requester = await createTestUser('USER', `privacy_apikey_req_${Date.now()}`);
const targetUser = await createTestUser('USER', `privacy_apikey_target_${Date.now()}`);

trackUserForCleanup(requester);
trackUserForCleanup(targetUser);

const response = await request(app)
.put(`${baseUrl}/users/${targetUser.username}/api-key`)
.set('x-api-key', requester.apiKey);

expect(response.status).toBe(403);
expect(response.body.role).toBeUndefined();
expect(response.body.apiKey).toBeUndefined();
});

it('PUT /users/:username/role forbidden response does not expose sensitive user fields', async function () {
const requester = await createTestUser('USER', `privacy_role_req_${Date.now()}`);
const targetUser = await createTestUser('USER', `privacy_role_target_${Date.now()}`);

trackUserForCleanup(requester);
trackUserForCleanup(targetUser);

const response = await request(app)
.put(`${baseUrl}/users/${targetUser.username}/role`)
.set('x-api-key', requester.apiKey)
.send({ role: USER_ROLES[USER_ROLES.length - 1] });

expect(response.status).toBe(403);
expect(response.body.role).toBeUndefined();
expect(response.body.apiKey).toBeUndefined();
});

it('DELETE /users/:username returns no body for USER deleting another non-admin user', async function () {
const requester = await createTestUser('USER', `privacy_delete_req_${Date.now()}`);
const targetUser = await createTestUser('USER', `privacy_delete_target_${Date.now()}`);

trackUserForCleanup(requester);
trackUserForCleanup(targetUser);

const response = await request(app)
.delete(`${baseUrl}/users/${targetUser.username}`)
.set('x-api-key', requester.apiKey);

expect(response.status).toBe(204);
expect(response.body).toEqual({});
});
});
});
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ services:
networks:
- space-network
healthcheck:
test: ["CMD", "curl", "-f", "http://localhost:5403/${BASE_URL_PATH:-}/api/v1/healthcheck"]
test: ["CMD", "curl", "-f", "http://localhost:5403${BASE_PATH:-}/api/v1/healthcheck"]
interval: 5s
timeout: 5s
retries: 3
Expand Down
Loading