Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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: 5 additions & 1 deletion modules/audit/middlewares/audit.middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import AuditService from '../services/audit.service.js';
import logger from '../../../lib/services/logger.js';
import config from '../../../config/index.js';

/**
* Default route prefixes to skip when auto-capturing audit events.
Expand Down Expand Up @@ -124,7 +125,10 @@ const createAuditMiddleware = (options = {}) => {

AuditService.log({
action,
req,
userId: req.user?._id || req.user?.id,
organizationId: req.organization?._id || req.organization?.id,
ip: config.audit?.captureIp !== false ? (req.ip || req.connection?.remoteAddress || '') : undefined,
userAgent: config.audit?.captureUserAgent !== false ? (req.headers?.['user-agent'] || '') : undefined,
targetType,
targetId,
}).catch((err) => logger.error('audit.middleware: audit log write failed', { message: err?.message, stack: err?.stack }));
Expand Down
20 changes: 9 additions & 11 deletions modules/audit/services/audit.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ import AuditRepository from '../repositories/audit.repository.js';
* @description Record an audit log entry. No-op when audit is disabled.
* @param {Object} params
* @param {string} params.action - Action identifier (e.g. 'auth.login', 'billing.subscribe')
* @param {Object} [params.req] - Express request object (extracts userId, orgId, ip, userAgent)
* @param {string} [params.userId] - ID of the acting user
* @param {string} [params.organizationId] - ID of the organization context
* @param {string} [params.ip] - IP address of the request
* @param {string} [params.userAgent] - User-agent of the request
* @param {string} [params.targetType] - Type of the target entity
* @param {string} [params.targetId] - ID of the target entity
* @param {Object} [params.metadata] - Additional metadata
* @returns {Promise<Object|null>} The created audit log entry or null if disabled
*/
const log = async ({ action, req, targetType, targetId, metadata } = {}) => {
const log = async ({ action, userId, organizationId, ip, userAgent, targetType, targetId, metadata } = {}) => {
Comment thread
PierreBrisorgueil marked this conversation as resolved.
if (!config.audit?.enabled) return null;
if (!action) return null;

Expand All @@ -27,15 +30,10 @@ const log = async ({ action, req, targetType, targetId, metadata } = {}) => {
metadata: metadata || {},
};

// Extract context from request if available (coerce ObjectIds to strings)
if (req) {
const uid = req.user?._id || req.user?.id;
const oid = req.organization?._id || req.organization?.id;
if (uid) entry.userId = String(uid);
if (oid) entry.orgId = String(oid);
entry.ip = config.audit?.captureIp !== false ? (req.ip || req.connection?.remoteAddress || '') : '';
entry.userAgent = config.audit?.captureUserAgent !== false ? (req.headers?.['user-agent'] || '') : '';
}
if (userId) entry.userId = String(userId);
if (organizationId) entry.orgId = String(organizationId);
if (ip !== undefined) entry.ip = ip || '';
if (userAgent !== undefined) entry.userAgent = userAgent || '';

try {
return await AuditRepository.create(entry);
Expand Down
6 changes: 4 additions & 2 deletions modules/audit/tests/audit.integration.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,15 @@ describe('Audit integration tests:', () => {
// Create some audit entries
await AuditService.log({
action: 'auth.login',
req: { user: { _id: adminUser.id }, headers: { 'user-agent': 'test' } },
userId: adminUser.id,
userAgent: 'test',
targetType: 'User',
targetId: adminUser.id,
});
await AuditService.log({
action: 'auth.signup',
req: { user: { _id: adminUser.id }, headers: { 'user-agent': 'test' } },
userId: adminUser.id,
userAgent: 'test',
targetType: 'User',
targetId: adminUser.id,
});
Expand Down
6 changes: 5 additions & 1 deletion modules/audit/tests/audit.middleware.unit.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ describe('Audit middleware unit tests:', () => {
const call = mockLog.mock.calls[0][0];
expect(call.action).toBe('auth.signin');
expect(call.targetType).toBe('User');
expect(call.req).toBe(req);
expect(call.req).toBeUndefined();
expect(call.userId).toBe('507f1f77bcf86cd799439011');
expect(call.organizationId).toBe('507f1f77bcf86cd799439012');
expect(call.ip).toBe('127.0.0.1');
expect(call.userAgent).toBe('TestAgent/1.0');
});

test('should log PUT mutations', () => {
Expand Down
47 changes: 17 additions & 30 deletions modules/audit/tests/audit.service.unit.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,13 @@ describe('AuditService unit tests:', () => {
test('should log an audit entry with request context', async () => {
const userId = '507f1f77bcf86cd799439011';
const orgId = '507f1f77bcf86cd799439012';
const req = {
user: { _id: userId },
organization: { _id: orgId },
ip: '127.0.0.1',
headers: { 'user-agent': 'TestAgent/1.0' },
};

await AuditService.log({
action: 'auth.login',
req,
userId,
organizationId: orgId,
ip: '127.0.0.1',
userAgent: 'TestAgent/1.0',
targetType: 'User',
targetId: userId,
metadata: { foo: 'bar' },
Expand Down Expand Up @@ -115,47 +112,37 @@ describe('AuditService unit tests:', () => {
expect(mockList).toHaveBeenCalledWith({ userId }, 1, 10);
});

// GDPR config flag tests
test('should capture IP when captureIp is true (default)', async () => {
mockConfig.audit = { enabled: true, ttlDays: 90, captureIp: true, captureUserAgent: true };
const req = { ip: '10.0.0.1', headers: { 'user-agent': 'Bot/1.0' } };
await AuditService.log({ action: 'test.ip', req });
// GDPR config flag tests — ip/userAgent are now extracted by the caller
test('should store ip when provided', async () => {
await AuditService.log({ action: 'test.ip', ip: '10.0.0.1', userAgent: 'Bot/1.0' });
expect(mockCreate).toHaveBeenCalledTimes(1);
const arg = mockCreate.mock.calls[0][0];
expect(arg.ip).toBe('10.0.0.1');
});

test('should set IP to empty string when captureIp is false', async () => {
mockConfig.audit = { enabled: true, ttlDays: 90, captureIp: false, captureUserAgent: true };
const req = { ip: '10.0.0.1', headers: { 'user-agent': 'Bot/1.0' } };
await AuditService.log({ action: 'test.ip', req });
test('should omit ip when undefined is passed', async () => {
await AuditService.log({ action: 'test.ip', ip: undefined, userAgent: 'Bot/1.0' });
expect(mockCreate).toHaveBeenCalledTimes(1);
const arg = mockCreate.mock.calls[0][0];
expect(arg.ip).toBe('');
expect(arg.ip).toBeUndefined();
});

test('should capture User-Agent when captureUserAgent is true (default)', async () => {
mockConfig.audit = { enabled: true, ttlDays: 90, captureIp: true, captureUserAgent: true };
const req = { ip: '10.0.0.1', headers: { 'user-agent': 'Bot/1.0' } };
await AuditService.log({ action: 'test.ua', req });
test('should store userAgent when provided', async () => {
await AuditService.log({ action: 'test.ua', ip: '10.0.0.1', userAgent: 'Bot/1.0' });
expect(mockCreate).toHaveBeenCalledTimes(1);
const arg = mockCreate.mock.calls[0][0];
expect(arg.userAgent).toBe('Bot/1.0');
});

test('should set User-Agent to empty string when captureUserAgent is false', async () => {
mockConfig.audit = { enabled: true, ttlDays: 90, captureIp: true, captureUserAgent: false };
const req = { ip: '10.0.0.1', headers: { 'user-agent': 'Bot/1.0' } };
await AuditService.log({ action: 'test.ua', req });
test('should omit userAgent when undefined is passed', async () => {
await AuditService.log({ action: 'test.ua', ip: '10.0.0.1', userAgent: undefined });
expect(mockCreate).toHaveBeenCalledTimes(1);
const arg = mockCreate.mock.calls[0][0];
expect(arg.userAgent).toBe('');
expect(arg.userAgent).toBeUndefined();
});

test('should default to capturing IP and User-Agent when config keys are undefined', async () => {
mockConfig.audit = { enabled: true, ttlDays: 90 };
const req = { ip: '192.168.1.1', headers: { 'user-agent': 'DefaultBot/2.0' } };
await AuditService.log({ action: 'test.defaults', req });
test('should store ip and userAgent when provided', async () => {
await AuditService.log({ action: 'test.defaults', ip: '192.168.1.1', userAgent: 'DefaultBot/2.0' });
expect(mockCreate).toHaveBeenCalledTimes(1);
const arg = mockCreate.mock.calls[0][0];
expect(arg.ip).toBe('192.168.1.1');
Expand Down
2 changes: 1 addition & 1 deletion modules/billing/routes/billing.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import passport from 'passport';

import model from '../../../lib/middlewares/model.js';
import policy from '../../../lib/middlewares/policy.js';
import organization from '../../organizations/middleware/organizations.middleware.js';
import organization from '../../organizations/middlewares/organizations.middleware.js';
import billingSchema from '../models/billing.subscription.schema.js';
import billingPlans from '../controllers/billing.plans.controller.js';
import billing from '../controllers/billing.controller.js';
Expand Down
15 changes: 8 additions & 7 deletions modules/home/services/home.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import { Base64 } from 'js-base64';
import { promises as fs } from 'fs';
import mongoose from 'mongoose';

import AuthService from '../../auth/services/auth.service.js';
import config from '../../../config/index.js';
import mailer from '../../../lib/helpers/mailer/index.js';
import HomeRepository from '../repositories/home.repository.js';
import { removeSensitive } from '../../users/utils/sanitizeUser.js';

/**
* @desc Check whether a config value is meaningfully set (non-empty, not a DEVKIT placeholder).
Expand All @@ -21,19 +21,20 @@ import HomeRepository from '../repositories/home.repository.js';
const isSet = (value) => !!(value && typeof value === 'string' && value.trim() !== '' && !value.startsWith('DEVKIT_NODE_'));

/**
* @desc Function to get all admin users in db
* @return {Promise} All users
* @desc Function to get page content from markdown file
* @param {string} name - The name of the markdown file
* @returns {Promise<Array>} Page content array
*/
const page = async (name) => {
const markdown = await fs.readFile(path.resolve(`./config/markdown/${name}.md`), 'utf8');
const test = await fs.stat(path.resolve(`./config/markdown/${name}.md`));
return Promise.resolve([
return [
{
title: _.startCase(name),
updatedAt: test.mtime,
markdown,
},
]);
];
};

/**
Expand Down Expand Up @@ -87,11 +88,11 @@ const changelogs = async () => {

/**
* @desc Function to get all admin users in db
* @return {Promise} All users
* @returns {Promise<Array>} All users (sanitized)
*/
const team = async () => {
const result = await HomeRepository.team();
return Promise.resolve(result.map((user) => AuthService.removeSensitive(user)));
return result.map((user) => removeSensitive(user));
};
Comment thread
PierreBrisorgueil marked this conversation as resolved.

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import errors from '../../../lib/helpers/errors.js';
import responses from '../../../lib/helpers/responses.js';
import MembershipService from '../services/organizations.membership.service.js';
import { MEMBERSHIP_ROLES } from '../lib/constants.js';

/**
* @function list
Expand Down Expand Up @@ -34,7 +35,7 @@ const list = async (req, res) => {
const updateRole = async (req, res) => {
try {
// Belt-and-suspenders: only owners can change roles (CASL blocks admins via no 'update Membership')
if (req.membership && req.membership.role !== 'owner') {
if (req.membership && req.membership.role !== MEMBERSHIP_ROLES.OWNER) {
return responses.error(res, 403, 'Forbidden', 'Only owners can change member roles')();
}
Comment thread
PierreBrisorgueil marked this conversation as resolved.
const membership = await MembershipService.updateRole(req.membershipDoc, req.body.role);
Expand All @@ -54,7 +55,7 @@ const updateRole = async (req, res) => {
const remove = async (req, res) => {
try {
// Admins can only remove members, not other admins or owners
if (req.membership && req.membership.role !== 'owner' && req.membershipDoc.role !== 'member') {
if (req.membership && req.membership.role !== MEMBERSHIP_ROLES.OWNER && req.membershipDoc.role !== MEMBERSHIP_ROLES.MEMBER) {
return responses.error(res, 403, 'Forbidden', 'Only owners can remove admins or other owners')();
Comment thread
PierreBrisorgueil marked this conversation as resolved.
Outdated
}
Comment thread
PierreBrisorgueil marked this conversation as resolved.
const result = await MembershipService.remove(req.membershipDoc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import errors from '../../../lib/helpers/errors.js';
import responses from '../../../lib/helpers/responses.js';
import MembershipService from '../services/organizations.membership.service.js';
import { MEMBERSHIP_ROLES, MEMBERSHIP_STATUSES } from '../lib/constants.js';

/**
* @function create
Expand Down Expand Up @@ -33,7 +34,7 @@ const create = async (req, res) => {
*/
const listPending = async (req, res) => {
try {
if (!req.membership || req.membership.role === 'member') {
if (!req.membership || req.membership.role === MEMBERSHIP_ROLES.MEMBER) {
return responses.success(res, 'membership request list')([]);
}
const requests = await MembershipService.listPending(req.organization._id || req.organization.id);
Expand Down Expand Up @@ -165,7 +166,7 @@ const requestByID = async (req, res, next, id) => {
const membership = await MembershipService.get(id);
const organizationId = String(req.organization._id || req.organization.id);
const membershipOrgId = String(membership?.organizationId?._id || membership?.organizationId);
if (!membership || membership.status !== 'pending' || membershipOrgId !== organizationId) {
if (!membership || membership.status !== MEMBERSHIP_STATUSES.PENDING || membershipOrgId !== organizationId) {
return responses.error(res, 404, 'Not Found', 'No pending request with that identifier has been found')();
}
req.membershipRequest = membership;
Expand Down
12 changes: 12 additions & 0 deletions modules/organizations/lib/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export const MEMBERSHIP_STATUSES = {
ACTIVE: 'active',
PENDING: 'pending',
INVITED: 'invited',
REJECTED: 'rejected',
};

export const MEMBERSHIP_ROLES = {
OWNER: 'owner',
ADMIN: 'admin',
MEMBER: 'member',
};
Comment thread
PierreBrisorgueil marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
*/
import OrganizationsCrudService from '../services/organizations.crud.service.js';
import MembershipService from '../services/organizations.membership.service.js';

import responses from '../../../lib/helpers/responses.js';
import { MEMBERSHIP_ROLES } from '../lib/constants.js';

/**
* Middleware that resolves the current organization from a route param or
Expand Down Expand Up @@ -37,7 +37,7 @@ async function resolveOrganization(req, res, next) {

// Platform admin bypasses membership requirement
if (req.user && req.user.roles && req.user.roles.includes('admin')) {
req.membership = { role: 'owner', organizationId: organization._id };
req.membership = { role: MEMBERSHIP_ROLES.OWNER, organizationId: organization._id };
return next();
}

Expand Down
7 changes: 4 additions & 3 deletions modules/organizations/policies/organizations.policy.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* Organization ability definitions for CASL document-level authorization.
*/
import { MEMBERSHIP_ROLES } from '../lib/constants.js';

/**
* Register organization-related subjects for document-level and path-level resolution.
Expand Down Expand Up @@ -50,19 +51,19 @@ export function organizationAbilities(user, membership, { can, cannot }) {
if (!membership) return;

switch (membership.role) {
case 'owner':
case MEMBERSHIP_ROLES.OWNER:
can('manage', 'Organization', { _id: String(membership.organizationId._id || membership.organizationId) });
can('manage', 'Membership', { organizationId: String(membership.organizationId._id || membership.organizationId) });
break;
case 'admin':
case MEMBERSHIP_ROLES.ADMIN:
can('read', 'Organization', { _id: String(membership.organizationId._id || membership.organizationId) });
can('update', 'Organization', { _id: String(membership.organizationId._id || membership.organizationId) });
cannot('delete', 'Organization');
can('read', 'Membership', { organizationId: String(membership.organizationId._id || membership.organizationId) });
can('create', 'Membership', { organizationId: String(membership.organizationId._id || membership.organizationId) });
can('delete', 'Membership', { organizationId: String(membership.organizationId._id || membership.organizationId) });
break;
case 'member':
case MEMBERSHIP_ROLES.MEMBER:
can('read', 'Organization', { _id: String(membership.organizationId._id || membership.organizationId) });
can('read', 'Membership', { organizationId: String(membership.organizationId._id || membership.organizationId) });
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Module dependencies
*/
import mongoose from 'mongoose';
import { MEMBERSHIP_STATUSES } from '../lib/constants.js';

const Membership = mongoose.model('Membership');

Expand Down Expand Up @@ -98,7 +99,7 @@ const deleteMany = (filter) => {
*/
const aggregateCountByOrganizations = (orgIds) =>
Membership.aggregate([
{ $match: { organizationId: { $in: orgIds }, status: 'active' } },
{ $match: { organizationId: { $in: orgIds }, status: MEMBERSHIP_STATUSES.ACTIVE } },
{ $group: { _id: '$organizationId', count: { $sum: 1 } } },
]);

Expand All @@ -109,7 +110,7 @@ const aggregateCountByOrganizations = (orgIds) =>
* @returns {Promise<Array>} An array of memberships.
*/
const listByUsers = (userIds) =>
Membership.find({ userId: { $in: userIds }, status: 'active' }).populate(defaultPopulate).sort('-createdAt').exec();
Membership.find({ userId: { $in: userIds }, status: MEMBERSHIP_STATUSES.ACTIVE }).populate(defaultPopulate).sort('-createdAt').exec();

export default {
list,
Expand Down
Loading
Loading