Skip to content

Commit da4ce51

Browse files
yash-pouranikcoderabbitai[bot]CodeRabbit
authored
Team Collaboration, Security Fixes, and Quota Isolation (#311)
* feat: team collaboration, auth response fixes, and member limit enforcement * fix: apply CodeRabbit auto-fixes Fixed 21 file(s) based on 19 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * fix: resolve failing dashboard and react sdk tests * chore: add migration script to clean up duplicate pending invitations * fix: restore missing transitive dependencies in lockfile * chore: remove unnecessary migration script for invitations * fix: restore original working package-lock.json before CodeRabbit corruption --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
1 parent 064f3a5 commit da4ce51

53 files changed

Lines changed: 1779 additions & 536 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/dashboard-api/src/__tests__/auth.controller.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ describe('auth.controller', () => {
242242

243243
expect(next).toHaveBeenCalledWith(expect.any(AppError));
244244
expect(next.mock.calls[0][0].statusCode).toBe(400);
245-
expect(next.mock.calls[0][0].message).toBe('User not found');
245+
expect(next.mock.calls[0][0].message).toBe('Invalid email or password');
246246
});
247247

248248
test('returns 400 on invalid password', async () => {
@@ -256,7 +256,7 @@ describe('auth.controller', () => {
256256

257257
expect(next).toHaveBeenCalledWith(expect.any(AppError));
258258
expect(next.mock.calls[0][0].statusCode).toBe(400);
259-
expect(next.mock.calls[0][0].message).toBe('Invalid password');
259+
expect(next.mock.calls[0][0].message).toBe('Invalid email or password');
260260
});
261261

262262
test('returns 400 on Zod validation error (missing password)', async () => {

apps/dashboard-api/src/__tests__/loadProjectForAdmin.test.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ jest.mock('@urbackend/common', () => ({
1111
AppError,
1212
Project: {
1313
findOne: jest.fn()
14-
}
14+
},
15+
getProjectAccessQuery: jest.fn((userId) => ({ $or: [{ owner: userId }, { "members.user": userId }] }))
1516
}));
1617

1718
const { Project } = require('@urbackend/common');
@@ -48,7 +49,7 @@ describe('loadProjectForAdmin Middleware', () => {
4849

4950
await loadProjectForAdmin(req, res, next);
5051

51-
expect(Project.findOne).toHaveBeenCalledWith({ _id: 'proj123', owner: 'user123' });
52+
expect(Project.findOne).toHaveBeenCalledWith({ _id: 'proj123', $or: [{ owner: 'user123' }, { "members.user": 'user123' }] });
5253
expect(next).toHaveBeenCalledTimes(1);
5354
expect(next).toHaveBeenCalledWith(expect.any(AppError));
5455
const error = next.mock.calls[0][0];
@@ -63,7 +64,7 @@ describe('loadProjectForAdmin Middleware', () => {
6364

6465
await loadProjectForAdmin(req, res, next);
6566

66-
expect(Project.findOne).toHaveBeenCalledWith({ _id: 'proj123', owner: 'user123' });
67+
expect(Project.findOne).toHaveBeenCalledWith({ _id: 'proj123', $or: [{ owner: 'user123' }, { "members.user": 'user123' }] });
6768
expect(req.project).toEqual(mockProject);
6869
expect(next).toHaveBeenCalledTimes(1);
6970
expect(next).toHaveBeenCalledWith();

apps/dashboard-api/src/__tests__/project.controller.softDelete.test.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ jest.mock('@urbackend/common', () => ({
2424
this.statusCode = statusCode;
2525
this.isOperational = true;
2626
}
27-
}
27+
},
28+
getProjectAccessQuery: jest.fn((userId) => ({ $or: [{ owner: userId }, { "members.user": userId }] }))
2829
}));
2930

3031
const { deleteRow, recoverRow } = require('../controllers/project.controller');
@@ -142,7 +143,13 @@ describe('Soft Delete in dashboard project.controller', () => {
142143

143144
await recoverRow(req, res, next);
144145

145-
expect(mockFindOne).toHaveBeenCalledWith({ _id: 'proj_1', owner: 'user_1' });
146+
expect(mockFindOne).toHaveBeenCalledWith({
147+
_id: 'proj_1',
148+
$or: [
149+
{ owner: 'user_1' },
150+
{ 'members.user': 'user_1' }
151+
]
152+
});
146153
expect(mockFindOneAndUpdate).toHaveBeenCalledWith(
147154
expect.objectContaining({
148155
_id: '507f1f77bcf86cd799439011',

apps/dashboard-api/src/__tests__/routes.projects.storage.test.js

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,21 @@ jest.mock('../middlewares/planEnforcement', () => ({
1515
checkByodGate: jest.fn((_req, _res, next) => next()),
1616
checkWebhookGate: jest.fn((_req, _res, next) => next()),
1717
checkMailTemplatesGate: jest.fn((_req, _res, next) => next()),
18+
checkMemberLimit: jest.fn((_req, _res, next) => next()),
1819
}));
1920

2021
jest.mock('@urbackend/common', () => ({
2122
verifyEmail: jest.fn((_req, _res, next) => next()),
2223
checkAuthEnabled: jest.fn((_req, _res, next) => next()),
23-
loadProjectForAdmin: jest.fn((_req, _res, next) => next()),
2424
}));
2525

26+
const mockAuthZ = jest.fn((req, res, next) => next());
27+
jest.mock('../middlewares/authorizeProject', () => {
28+
const m = jest.fn(() => mockAuthZ);
29+
m.middleware = mockAuthZ;
30+
return m;
31+
});
32+
2633
jest.mock('../controllers/userAuth.controller', () => ({
2734
createAdminUser: jest.fn((_req, res) => res.json({ ok: true })),
2835
resetPassword: jest.fn((_req, res) => res.json({ ok: true })),
@@ -76,6 +83,10 @@ jest.mock('../controllers/project.controller', () => {
7683
manageContacts: jest.fn(ok),
7784
deleteContact: jest.fn(ok),
7885
sendMarketingBroadcast: jest.fn(ok),
86+
getMembers: jest.fn(ok),
87+
inviteMember: jest.fn(ok),
88+
updateMemberRole: jest.fn(ok),
89+
removeMember: jest.fn(ok),
7990
};
8091
});
8192

@@ -84,7 +95,8 @@ const request = require('supertest');
8495
const projectsRouter = require('../routes/projects');
8596
const projectController = require('../controllers/project.controller');
8697
const authMiddleware = require('../middlewares/authMiddleware');
87-
const { verifyEmail, loadProjectForAdmin } = require('@urbackend/common');
98+
const { verifyEmail } = require('@urbackend/common');
99+
const authorizeProject = require('../middlewares/authorizeProject');
88100

89101
let app;
90102

@@ -112,7 +124,7 @@ describe('projects storage presigned routes', () => {
112124
expect(res.status).toBe(200);
113125
expect(authMiddleware).toHaveBeenCalled();
114126
expect(verifyEmail).toHaveBeenCalled();
115-
expect(loadProjectForAdmin).toHaveBeenCalled();
127+
expect(authorizeProject.middleware).toHaveBeenCalled();
116128
expect(projectController.requestUpload).toHaveBeenCalledTimes(1);
117129
});
118130

@@ -124,7 +136,7 @@ describe('projects storage presigned routes', () => {
124136
expect(res.status).toBe(200);
125137
expect(authMiddleware).toHaveBeenCalled();
126138
expect(verifyEmail).toHaveBeenCalled();
127-
expect(loadProjectForAdmin).toHaveBeenCalled();
139+
expect(authorizeProject.middleware).toHaveBeenCalled();
128140
expect(projectController.confirmUpload).toHaveBeenCalledTimes(1);
129141
});
130142
});

apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ jest.mock('@urbackend/common', () => {
5151
return normalized;
5252
}),
5353
AppError,
54+
getProjectAccessQuery: jest.fn((userId) => ({ owner: userId })),
5455
__mockStorageFrom: mockStorageFrom,
5556
};
5657
});

apps/dashboard-api/src/__tests__/webhook.controller.test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ jest.mock('@urbackend/common', () => {
5151
});
5252
}
5353
},
54+
getProjectAccessQuery: jest.fn((userId) => ({ owner: userId })),
5455
};
5556
});
5657

apps/dashboard-api/src/app.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ const billingRoute = require('./routes/billing');
106106
const eventsRoute = require('./routes/events');
107107
const adminMetricsRoute = require('./routes/admin.metrics');
108108
const aiRoute = require('./routes/ai.routes');
109+
const invitationsRoute = require('./routes/invitations');
109110

110111
app.use('/api/auth', authRoute);
111112
app.use('/api/projects', dashboardLimiter, projectRoute);
@@ -116,6 +117,7 @@ app.use('/api/analytics', dashboardLimiter, analyticsRoute);
116117
app.use('/api/billing', billingRoute);
117118
app.use('/api/events', dashboardLimiter, eventsRoute);
118119
app.use('/api/admin/metrics', dashboardLimiter, adminMetricsRoute);
120+
app.use('/api/invitations', dashboardLimiter, invitationsRoute);
119121

120122

121123

apps/dashboard-api/src/controllers/ai.controller.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
const { Project } = require('@urbackend/common/src/models');
22
const { forwardToPythonService } = require('../utils/internalPythonClient');
3-
const { AppError, ApiResponse } = require('@urbackend/common');
3+
const { AppError, ApiResponse, getProjectAccessQuery } = require('@urbackend/common');
44

55
/**
66
* Controller to handle AI Query Builder requests.
@@ -35,7 +35,7 @@ const queryBuilder = async (req, res, next) => {
3535

3636
// 1. Fetch the project and specifically the requested collection schema
3737
const project = await Project.findOne(
38-
{ _id: projectId, owner: req.user._id, "collections.name": safeCollectionName },
38+
{ ...getProjectAccessQuery(req.user._id), _id: projectId, "collections.name": safeCollectionName },
3939
{ "collections.$": 1 }
4040
);
4141

apps/dashboard-api/src/controllers/analytics.controller.js

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const { Project, Log, Developer, Webhook, getConnection, resolveEffectivePlan, getPlanLimits, PlatformEvent, DeveloperActivity, AppError, ApiResponse } = require("@urbackend/common");
1+
const { Project, Log, Developer, Webhook, getConnection, resolveEffectivePlan, getPlanLimits, PlatformEvent, DeveloperActivity, AppError, ApiResponse, getProjectAccessQuery } = require("@urbackend/common");
22
const mongoose = require("mongoose");
33

44
/**
@@ -12,12 +12,7 @@ module.exports.getGlobalStats = async (req, res, next) => {
1212
const [stats, dev] = await Promise.all([
1313
Project.aggregate([
1414
{
15-
$match: {
16-
$or: [
17-
{ owner: user_id },
18-
{ owner: userId }
19-
]
20-
}
15+
$match: { owner: userId },
2116
},
2217
{
2318
$group: {
@@ -90,7 +85,7 @@ module.exports.getGlobalStats = async (req, res, next) => {
9085
module.exports.getRecentActivity = async (req, res, next) => {
9186
try {
9287
const userId = req.user._id;
93-
const projectIds = await Project.find({ owner: userId }).distinct("_id");
88+
const projectIds = await Project.find(getProjectAccessQuery(userId)).distinct("_id");
9489

9590
const logs = await Log.find({ projectId: { $in: projectIds } })
9691
.sort({ timestamp: -1 })
@@ -271,7 +266,7 @@ module.exports.getNorthStar = async (req, res, next) => {
271266
const sevenDaysAgo = new Date();
272267
sevenDaysAgo.setUTCDate(sevenDaysAgo.getUTCDate() - 7);
273268

274-
// Projects owned by this developer
269+
// Projects owned by this developer (North Star should be owner-only)
275270
const allProjects = await Project.find({ owner: developerId }).select('_id name').lean();
276271
const projectIds = allProjects.map((p) => p._id);
277272
const totalProjects = projectIds.length;

apps/dashboard-api/src/controllers/auth.controller.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,10 +320,10 @@ module.exports.login = async (req, res, next) => {
320320
const { email, password } = loginSchema.parse(req.body);
321321

322322
const dev = await Developer.findOne({ email: email.toLowerCase().trim() }).select('+password');
323-
if (!dev) return next(new AppError(400, "User not found"));
323+
if (!dev) return next(new AppError(400, "Invalid email or password"));
324324

325325
const validPass = await bcrypt.compare(password, dev.password);
326-
if (!validPass) return next(new AppError(400, "Invalid password"));
326+
if (!validPass) return next(new AppError(400, "Invalid email or password"));
327327

328328
await sendTokenResponse(dev, 200, res);
329329
} catch (err) {

0 commit comments

Comments
 (0)