From 91a66e5b335e2de41b0611571881eae9fa980354 Mon Sep 17 00:00:00 2001 From: AZ0228 <53315675+AZ0228@users.noreply.github.com> Date: Mon, 27 Apr 2026 14:05:08 -0400 Subject: [PATCH 1/4] MER-189: strengthen org role and permission backend invariants. Add multi-role compatibility, owner invariants/migration coverage, hardened role/member authorization gates, and route/schema test coverage for org permission enforcement. --- backend/docs/org-permissions-test-matrix.md | 41 +++ backend/middlewares/orgPermissions.js | 46 ++- backend/package.json | 3 +- backend/routes/adminRoutes.js | 62 ++++ backend/routes/orgInviteRoutes.js | 4 +- backend/routes/orgRoleRoutes.js | 338 ++++++++++++++++-- backend/routes/orgRoutes.js | 24 +- backend/schemas/org.js | 35 +- backend/schemas/orgInvite.js | 13 + backend/schemas/orgMember.js | 48 ++- backend/services/orgInviteService.js | 35 +- .../orgRoleRoutes.auth.outcomes.test.js | 21 ++ .../tests/unit/orgPermissionModels.test.js | 61 ++++ 13 files changed, 649 insertions(+), 82 deletions(-) create mode 100644 backend/docs/org-permissions-test-matrix.md create mode 100644 backend/tests/route-outcomes/orgRoleRoutes.auth.outcomes.test.js create mode 100644 backend/tests/unit/orgPermissionModels.test.js diff --git a/backend/docs/org-permissions-test-matrix.md b/backend/docs/org-permissions-test-matrix.md new file mode 100644 index 00000000..2c747b9b --- /dev/null +++ b/backend/docs/org-permissions-test-matrix.md @@ -0,0 +1,41 @@ +# Org Permissions Test Matrix + +## Route Authorization + +- `GET /org-roles/:orgId/members` + - non-member -> `403` + - member without `manage_members` -> `403` + - manager/owner -> `200` +- `POST /org-roles/:orgId/members/:userId/role` + - non-member -> `403` + - manager assigning above own order -> `403` + - assigning `owner` without transfer -> `403` + - valid manager assignment -> `200` +- `GET /org-roles/:orgId/roles/:roleName/members` + - non-member -> `403` + - member without `manage_members` -> `403` + - manager/owner -> `200` +- `GET /org/:orgId/forms` + - member without `manage_members` -> `403` + - manager/owner -> `200` + +## Ownership Invariants + +- transfer ownership endpoint updates: + - `Org.owner` to target user + - target `OrgMember` includes `roles: ['owner', ...]` + - previous owner loses `owner` role +- `owner` role cannot be assigned through general role assignment endpoint + +## Multi-Role Compatibility + +- existing records with only `role` are backfilled to `roles: [role]` +- permission checks resolve from union of `roles[]` +- invite acceptance copies `roles[]` and sets legacy `role` to first element + +## Frontend Regression Checks + +- `RoleManager` shows owner role as visible immutable system role +- member role assignment modal supports selecting multiple roles +- role options above actor hierarchy are hidden/disabled +- application approval includes selected role payload diff --git a/backend/middlewares/orgPermissions.js b/backend/middlewares/orgPermissions.js index 9d6acb8f..9d06a0cd 100644 --- a/backend/middlewares/orgPermissions.js +++ b/backend/middlewares/orgPermissions.js @@ -34,13 +34,22 @@ function requireOrgPermission(permission, orgParam = 'orgId') { return next(); } + const org = await Org.findById(orgId); + if (!org) { + return res.status(404).json({ + success: false, + message: 'Organization not found' + }); + } + const member = await OrgMember.findOne({ org_id: orgId, user_id: req.user.userId, status: 'active' }); - if (!member) { + const isRecordOwner = String(org.owner) === String(req.user.userId); + if (!member && !isRecordOwner) { console.log('Denied, You are not a member of this organization'); return res.status(403).json({ success: false, @@ -48,13 +57,10 @@ function requireOrgPermission(permission, orgParam = 'orgId') { }); } - // Get the organization to check permissions - const org = await Org.findById(orgId); - if (!org) { - return res.status(404).json({ - success: false, - message: 'Organization not found' - }); + if (isRecordOwner) { + req.orgMember = member || { role: 'owner', roles: ['owner'] }; + req.org = org; + return next(); } const hasPermission = await member.hasPermissionWithOrg(permission, org); @@ -112,27 +118,31 @@ function requireAnyOrgPermission(permissions, orgParam = 'orgId') { return next(); } + const org = await Org.findById(orgId); + if (!org) { + return res.status(404).json({ + success: false, + message: 'Organization not found' + }); + } + const member = await OrgMember.findOne({ org_id: orgId, user_id: req.user.userId, status: 'active' }); - - if (!member) { + const isRecordOwner = String(org.owner) === String(req.user.userId); + if (!member && !isRecordOwner) { console.log('Denied, You are not a member of this organization'); return res.status(403).json({ success: false, message: 'You are not a member of this organization' }); } - - // Get the organization to check permissions - const org = await Org.findById(orgId); - if (!org) { - return res.status(404).json({ - success: false, - message: 'Organization not found' - }); + if (isRecordOwner) { + req.orgMember = member || { role: 'owner', roles: ['owner'] }; + req.org = org; + return next(); } // Check if user has any of the required permissions diff --git a/backend/package.json b/backend/package.json index a867615a..3c16312a 100644 --- a/backend/package.json +++ b/backend/package.json @@ -47,7 +47,8 @@ "test:integration": "NODE_ENV=test jest --runInBand --testPathPatterns=tests/integration", "test:routes": "NODE_ENV=test jest --runInBand --testPathPatterns=tests/route-outcomes", "test:coverage": "NODE_ENV=test jest --runInBand --coverage", - "setup-saml": "node scripts/setupSAML.js" + "setup-saml": "node scripts/setupSAML.js", + "migrate:org-member-roles": "node scripts/backfillOrgMemberRoles.js" }, "devDependencies": { "jest": "^30.3.0", diff --git a/backend/routes/adminRoutes.js b/backend/routes/adminRoutes.js index 15824956..da6f7845 100644 --- a/backend/routes/adminRoutes.js +++ b/backend/routes/adminRoutes.js @@ -359,6 +359,68 @@ router.post('/admin/migrate-classroom-building-refs', verifyToken, requireAdmin, } }); +/** + * POST /admin/migrate-org-owner-roles + * Ensure every org owner has an active OrgMember record with owner role. + * Also repairs existing owner memberships that drifted away from owner role. + */ +router.post('/admin/migrate-org-owner-roles', verifyToken, requireAdmin, async (req, res) => { + try { + const getModels = require('../services/getModelService'); + const { Org, OrgMember } = getModels(req, 'Org', 'OrgMember'); + + const orgs = await Org.find({}, { _id: 1, owner: 1 }).lean(); + let createdOwnerMemberships = 0; + let repairedOwnerMemberships = 0; + + for (const org of orgs) { + if (!org?.owner) continue; + const ownerId = org.owner; + const ownerMembership = await OrgMember.findOne({ org_id: org._id, user_id: ownerId }); + + if (!ownerMembership) { + await OrgMember.create({ + org_id: org._id, + user_id: ownerId, + role: 'owner', + roles: ['owner'], + status: 'active', + assignedBy: ownerId + }); + createdOwnerMemberships += 1; + continue; + } + + const rolesArray = Array.isArray(ownerMembership.roles) ? ownerMembership.roles : []; + const hasOwnerRoleField = ownerMembership.role === 'owner'; + const hasOwnerInRolesArray = rolesArray.includes('owner'); + const isActive = ownerMembership.status === 'active'; + if (!hasOwnerRoleField || !hasOwnerInRolesArray || !isActive) { + const repairedRoles = [ + 'owner', + ...(rolesArray.filter((roleName) => roleName && roleName !== 'owner')) + ]; + ownerMembership.role = 'owner'; + ownerMembership.roles = repairedRoles; + ownerMembership.status = 'active'; + await ownerMembership.save(); + repairedOwnerMemberships += 1; + } + } + + const data = { + orgsScanned: orgs.length, + createdOwnerMemberships, + repairedOwnerMemberships + }; + console.log('POST /admin/migrate-org-owner-roles completed:', data); + res.json({ success: true, data }); + } catch (err) { + console.error('POST /admin/migrate-org-owner-roles failed:', err); + res.status(500).json({ success: false, message: err.message }); + } +}); + router.get('/admin/tenant-config', verifyToken, requireAdmin, async (req, res) => { if (!req.user.platformRoles?.includes('platform_admin')) { return res.status(403).json({ success: false, message: 'Platform admin required.' }); diff --git a/backend/routes/orgInviteRoutes.js b/backend/routes/orgInviteRoutes.js index b4b75784..b7be6d78 100644 --- a/backend/routes/orgInviteRoutes.js +++ b/backend/routes/orgInviteRoutes.js @@ -141,7 +141,7 @@ router.post('/decline-by-token', verifyToken, async (req, res) => { router.post('/:orgId/invite', verifyToken, requireMemberManagement('orgId'), async (req, res) => { try { const { orgId } = req.params; - const { email, role = 'member' } = req.body; + const { email, role = 'member', roles = [] } = req.body; if (!email || !String(email).trim()) { return res.status(400).json({ @@ -150,7 +150,7 @@ router.post('/:orgId/invite', verifyToken, requireMemberManagement('orgId'), asy }); } - const result = await orgInviteService.createInvite(req, orgId, email.trim(), role); + const result = await orgInviteService.createInvite(req, orgId, email.trim(), roles.length > 0 ? roles : role); res.status(201).json({ success: true, diff --git a/backend/routes/orgRoleRoutes.js b/backend/routes/orgRoleRoutes.js index 7f33fcd3..a0ac19ff 100644 --- a/backend/routes/orgRoleRoutes.js +++ b/backend/routes/orgRoleRoutes.js @@ -19,6 +19,143 @@ const { recordMemberJoined, recordMemberRemoved } = require('../services/orgMemb const router = express.Router(); +const SYSTEM_ROLE_NAMES = new Set(['owner', 'member']); + +function normalizeRoleShape(role, previousRole = null) { + const permissions = Array.isArray(role.permissions) ? [...new Set(role.permissions)] : (previousRole?.permissions || []); + return { + ...role, + permissions, + canManageMembers: permissions.includes('manage_members'), + canManageRoles: permissions.includes('manage_roles'), + canManageEvents: permissions.includes('manage_events'), + canViewAnalytics: permissions.includes('view_analytics') + }; +} + +async function ensureOwnerMembership(OrgMember, orgId, ownerUserId) { + const ownerMembership = await OrgMember.findOne({ org_id: orgId, user_id: ownerUserId }); + if (!ownerMembership) { + const createdOwnerMembership = new OrgMember({ + org_id: orgId, + user_id: ownerUserId, + role: 'owner', + roles: ['owner'], + status: 'active', + assignedBy: ownerUserId + }); + await createdOwnerMembership.save(); + return createdOwnerMembership; + } + if (ownerMembership.role !== 'owner' || !ownerMembership.roles?.includes('owner')) { + ownerMembership.role = 'owner'; + ownerMembership.roles = ['owner', ...(ownerMembership.roles || []).filter((r) => r !== 'owner')]; + ownerMembership.status = 'active'; + await ownerMembership.save(); + } + return ownerMembership; +} + +async function renameAssignedRoleAcrossRecords({ OrgMember, OrgInvite, orgId, oldName, newName }) { + if (!oldName || !newName || oldName === newName) { + return; + } + + const memberUpdatePipeline = [ + { + $set: { + role: { + $cond: [{ $eq: ['$role', oldName] }, newName, '$role'] + }, + roles: { + $let: { + vars: { + safeRoles: { + $cond: [{ $isArray: '$roles' }, '$roles', []] + } + }, + in: { + $setUnion: [ + { + $map: { + input: '$$safeRoles', + as: 'roleName', + in: { + $cond: [ + { $eq: ['$$roleName', oldName] }, + newName, + '$$roleName' + ] + } + } + }, + [] + ] + } + } + } + } + } + ]; + + await OrgMember.updateMany( + { + org_id: orgId, + $or: [{ role: oldName }, { roles: oldName }] + }, + memberUpdatePipeline + ); + + if (!OrgInvite) { + return; + } + + const inviteUpdatePipeline = [ + { + $set: { + role: { + $cond: [{ $eq: ['$role', oldName] }, newName, '$role'] + }, + roles: { + $let: { + vars: { + safeRoles: { + $cond: [{ $isArray: '$roles' }, '$roles', []] + } + }, + in: { + $setUnion: [ + { + $map: { + input: '$$safeRoles', + as: 'roleName', + in: { + $cond: [ + { $eq: ['$$roleName', oldName] }, + newName, + '$$roleName' + ] + } + } + }, + [] + ] + } + } + } + } + } + ]; + + await OrgInvite.updateMany( + { + org_id: orgId, + $or: [{ role: oldName }, { roles: oldName }] + }, + inviteUpdatePipeline + ); +} + /** * Check if user can edit/delete a role. Users cannot edit roles more privileged than their own. * Organization owners (record owner or membership role "owner") may edit/delete any non-system role. @@ -44,6 +181,35 @@ router.get('/:orgId/can-manage-roles', verifyToken, requireRoleManagement(), asy res.status(200).json({ success: true, canManageRoles: true }); }); +router.get('/:orgId/me/permissions', verifyToken, requireOrgPermission('view_events'), async (req, res) => { + try { + const member = req.orgMember; + const org = req.org; + const assignedRoles = member?.roles?.length ? member.roles : [member?.role || 'member']; + const effectivePermissions = new Set(); + for (const roleName of assignedRoles) { + const role = org.getRoleByName(roleName); + if (!role) continue; + (role.permissions || []).forEach((permission) => effectivePermissions.add(permission)); + } + if (effectivePermissions.has('manage_finances')) { + effectivePermissions.add('view_finances'); + } + res.status(200).json({ + success: true, + role: member?.role || 'member', + roles: assignedRoles, + permissions: Array.from(effectivePermissions) + }); + } catch (error) { + console.error('Error getting effective org permissions:', error); + res.status(500).json({ + success: false, + message: 'Error fetching permissions' + }); + } +}); + // Get all roles for an organization router.get('/:orgId/roles', verifyToken, requireOrgPermission('view_roles'), async (req, res) => { const { Org } = getModels(req, 'Org'); @@ -158,7 +324,7 @@ router.post('/:orgId/roles', verifyToken, requireRoleManagement(), async (req, r // Update all roles for an organization router.put('/:orgId/roles', verifyToken, requireRoleManagement(), async (req, res) => { - const { Org, OrgMember } = getModels(req, 'Org', 'OrgMember'); + const { Org, OrgMember, OrgInvite } = getModels(req, 'Org', 'OrgMember', 'OrgInvite'); const { orgId } = req.params; const { positions } = req.body; @@ -171,12 +337,13 @@ router.put('/:orgId/roles', verifyToken, requireRoleManagement(), async (req, re }); } - // Validate that owner role is preserved + // Validate that owner and member system roles are preserved const hasOwnerRole = positions.some(role => role.name === 'owner'); - if (!hasOwnerRole) { + const hasMemberRole = positions.some(role => role.name === 'member'); + if (!hasOwnerRole || !hasMemberRole) { return res.status(400).json({ success: false, - message: 'Owner role must be preserved' + message: 'Owner and member roles must be preserved' }); } @@ -230,20 +397,33 @@ router.put('/:orgId/roles', verifyToken, requireRoleManagement(), async (req, re } for (const { oldName, newName } of roleNameUpdates) { - await OrgMember.updateMany( - { org_id: orgId, role: oldName }, - { $set: { role: newName } } - ); + await renameAssignedRoleAcrossRecords({ + OrgMember, + OrgInvite, + orgId, + oldName, + newName + }); } + const previousByName = new Map(oldPositions.map((role) => [role.name, role])); + const normalizedPositions = positions.map((role) => { + if (SYSTEM_ROLE_NAMES.has(role.name)) { + const previousRole = previousByName.get(role.name); + return previousRole || role; + } + const previousRole = previousByName.get(role.name); + return normalizeRoleShape(role, previousRole); + }); + // Update all roles - org.positions = positions; + org.positions = normalizedPositions; await org.save(); res.status(200).json({ success: true, message: 'Roles updated successfully', - roles: positions + roles: normalizedPositions }); } catch (error) { console.error('Error updating roles:', error); @@ -256,7 +436,7 @@ router.put('/:orgId/roles', verifyToken, requireRoleManagement(), async (req, re // Update an existing role router.put('/:orgId/roles/:roleName', verifyToken, requireRoleManagement(), async (req, res) => { - const { Org, OrgMember } = getModels(req, 'Org', 'OrgMember'); + const { Org, OrgMember, OrgInvite } = getModels(req, 'Org', 'OrgMember', 'OrgInvite'); const { orgId, roleName } = req.params; const updates = req.body; @@ -287,6 +467,13 @@ router.put('/:orgId/roles/:roleName', verifyToken, requireRoleManagement(), asyn }); } + if (SYSTEM_ROLE_NAMES.has(roleName)) { + return res.status(400).json({ + success: false, + message: `${roleName} is an immutable system role` + }); + } + // Validate color format if provided if (updates.color && !/^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$/.test(updates.color)) { return res.status(400).json({ @@ -298,14 +485,17 @@ router.put('/:orgId/roles/:roleName', verifyToken, requireRoleManagement(), asyn // If role name is changing, update OrgMember documents first so members keep their role const newName = updates.name; if (newName && newName !== roleName) { - await OrgMember.updateMany( - { org_id: orgId, role: roleName }, - { $set: { role: newName } } - ); + await renameAssignedRoleAcrossRecords({ + OrgMember, + OrgInvite, + orgId, + oldName: roleName, + newName + }); } // Update role - await org.updateRole(roleName, updates); + await org.updateRole(roleName, normalizeRoleShape(updates, existingRole)); console.log('PUT /org-roles/', orgId, roleName, updates); res.status(200).json({ @@ -370,7 +560,7 @@ router.delete('/:orgId/roles/:roleName', verifyToken, requireRoleManagement(), a }); // Get members by role -router.get('/:orgId/roles/:roleName/members', verifyToken , async (req, res) => { +router.get('/:orgId/roles/:roleName/members', verifyToken, requireMemberManagement(), async (req, res) => { const { OrgMember } = getModels(req, 'OrgMember'); const { orgId, roleName } = req.params; @@ -393,10 +583,10 @@ router.get('/:orgId/roles/:roleName/members', verifyToken , async (req, res) = }); // Assign role to a member -router.post('/:orgId/members/:userId/role', verifyToken, async (req, res) => { +router.post('/:orgId/members/:userId/role', verifyToken, requireMemberManagement(), async (req, res) => { const { Org, OrgMember, User } = getModels(req, 'Org', 'OrgMember', 'User'); const { orgId, userId } = req.params; - const { role, reason } = req.body; + const { role, roles, reason } = req.body; try { // Verify organization exists @@ -408,9 +598,25 @@ router.post('/:orgId/members/:userId/role', verifyToken, async (req, res) => { }); } - // Verify role exists - const roleExists = org.getRoleByName(role); - if (!roleExists) { + const requestedRoles = Array.isArray(roles) && roles.length > 0 ? roles : [role]; + const normalizedRoles = [...new Set(requestedRoles.filter(Boolean))]; + if (normalizedRoles.length === 0) { + return res.status(400).json({ + success: false, + message: 'At least one role is required' + }); + } + const includesOwnerRole = normalizedRoles.includes('owner'); + if (includesOwnerRole && String(org.owner) !== String(userId)) { + return res.status(403).json({ + success: false, + message: 'Owner role can only be assigned through ownership transfer' + }); + } + + // Verify roles exist + const hasMissingRole = normalizedRoles.some((roleName) => !org.getRoleByName(roleName)); + if (hasMissingRole) { return res.status(404).json({ success: false, message: 'Role not found' @@ -426,6 +632,21 @@ router.post('/:orgId/members/:userId/role', verifyToken, async (req, res) => { }); } + const actorMember = req.orgMember; + const actorRole = org.getRoleByName(actorMember?.role); + const actorIsRecordOwner = String(org.owner) === String(req.user.userId); + const actorMaxPrivilege = actorIsRecordOwner ? -1 : (actorRole?.order ?? Number.MAX_SAFE_INTEGER); + const rolesOutOfReach = normalizedRoles.some((roleName) => { + const targetRole = org.getRoleByName(roleName); + return (targetRole?.order ?? Number.MAX_SAFE_INTEGER) < actorMaxPrivilege; + }); + if (rolesOutOfReach) { + return res.status(403).json({ + success: false, + message: 'You cannot assign roles above your own level' + }); + } + // Find or create member record let member = await OrgMember.findOne({ org_id: orgId, user_id: userId }); @@ -433,14 +654,15 @@ router.post('/:orgId/members/:userId/role', verifyToken, async (req, res) => { member = new OrgMember({ org_id: orgId, user_id: userId, - role: role, + role: normalizedRoles[0], + roles: normalizedRoles, assignedBy: req.user.userId, status: 'active' }); await member.save(); await recordMemberJoined(member, req.user.userId, reason || 'role_assigned'); } else { - await member.changeRole(role, req.user.userId, reason, { + await member.setRoles(normalizedRoles, req.user.userId, reason, { termStart: req.body.roleTermStart ? new Date(req.body.roleTermStart) : undefined, termEnd: req.body.roleTermEnd ? new Date(req.body.roleTermEnd) : undefined }); @@ -459,7 +681,7 @@ router.post('/:orgId/members/:userId/role', verifyToken, async (req, res) => { await user.save(); - console.log('POST /org-roles/members/:userId/role', orgId, userId, role, reason); + console.log('POST /org-roles/members/:userId/role', orgId, userId, normalizedRoles, reason); res.status(200).json({ success: true, @@ -467,6 +689,7 @@ router.post('/:orgId/members/:userId/role', verifyToken, async (req, res) => { member: { userId: member.user_id, role: member.role, + roles: member.roles, assignedAt: member.assignedAt } }); @@ -480,7 +703,7 @@ router.post('/:orgId/members/:userId/role', verifyToken, async (req, res) => { }); // Get all members of an organization -router.get('/:orgId/members', verifyToken, async (req, res) => { +router.get('/:orgId/members', verifyToken, requireMemberManagement(), async (req, res) => { const { OrgMember, OrgMemberApplication } = getModels(req, 'OrgMember', 'OrgMemberApplication'); const { orgId } = req.params; @@ -503,6 +726,60 @@ router.get('/:orgId/members', verifyToken, async (req, res) => { } }); +router.post('/:orgId/transfer-ownership/:newOwnerUserId', verifyToken, requireOrgOwner(), async (req, res) => { + const { Org, OrgMember } = getModels(req, 'Org', 'OrgMember'); + const { orgId, newOwnerUserId } = req.params; + + try { + const org = req.org || await Org.findById(orgId); + if (!org) { + return res.status(404).json({ success: false, message: 'Organization not found' }); + } + + if (String(org.owner) === String(newOwnerUserId)) { + return res.status(400).json({ success: false, message: 'User is already the organization owner' }); + } + + const newOwnerMembership = await ensureOwnerMembership(OrgMember, orgId, newOwnerUserId); + const previousOwnerUserId = org.owner; + org.owner = newOwnerUserId; + await org.save(); + + const previousOwnerMembership = await OrgMember.findOne({ + org_id: orgId, + user_id: previousOwnerUserId, + status: 'active' + }); + if (previousOwnerMembership) { + previousOwnerMembership.roles = (previousOwnerMembership.roles || []) + .filter((roleName) => roleName !== 'owner'); + if (previousOwnerMembership.roles.length === 0) { + previousOwnerMembership.roles = ['member']; + } + previousOwnerMembership.role = previousOwnerMembership.roles[0]; + await previousOwnerMembership.save(); + } + + await ensureOwnerMembership(OrgMember, orgId, newOwnerUserId); + + return res.status(200).json({ + success: true, + message: 'Organization ownership transferred successfully', + newOwnerMembership: { + userId: newOwnerMembership.user_id, + role: newOwnerMembership.role, + roles: newOwnerMembership.roles + } + }); + } catch (error) { + console.error('Error transferring organization ownership:', error); + return res.status(500).json({ + success: false, + message: error.message || 'Failed to transfer ownership' + }); + } +}); + // Remove member from organization router.delete('/:orgId/members/:userId', verifyToken, requireMemberManagement(), async (req, res) => { const { OrgMember, User } = getModels(req, 'OrgMember', 'User'); @@ -599,7 +876,7 @@ router.get('/:orgId/roles/:roleName/permissions', verifyToken, requireOrgPermiss router.post('/:orgId/applications/:applicationId/approve', verifyToken, requireMemberManagement(), async (req, res) => { const { OrgMember, OrgMemberApplication, User } = getModels(req, 'OrgMember', 'OrgMemberApplication', 'User'); const { orgId, applicationId } = req.params; - const { role = 'member', reason = '' } = req.body; + const { role = 'member', roles = [], reason = '' } = req.body; const userId = req.user.userId; try { @@ -644,11 +921,16 @@ router.post('/:orgId/applications/:applicationId/approve', verifyToken, requireM }); } + const requestedRoles = Array.isArray(roles) && roles.length > 0 ? roles : [role]; + const normalizedRoles = [...new Set(requestedRoles.filter(Boolean))]; + const safeRoles = normalizedRoles.length > 0 ? normalizedRoles : ['member']; + // Create new member const newMember = new OrgMember({ org_id: orgId, user_id: application.user_id._id, - role: role, + role: safeRoles[0], + roles: safeRoles, status: 'active', assignedBy: userId, assignedAt: new Date() diff --git a/backend/routes/orgRoutes.js b/backend/routes/orgRoutes.js index bbf66b3c..5f0b7ded 100644 --- a/backend/routes/orgRoutes.js +++ b/backend/routes/orgRoutes.js @@ -320,6 +320,7 @@ router.post("/create-org", verifyToken, upload.fields([ org_id: newOrg._id, user_id: userId, role: 'owner', // Set the creator as owner + roles: ['owner'], status: 'active', assignedBy: userId }); @@ -564,17 +565,10 @@ router.post("/edit-org", verifyToken, upload.fields([ // Update other fields only if they are provided if (positions) { - try { - // Parse positions if it's a JSON string - const parsedPositions = typeof positions === 'string' ? JSON.parse(positions) : positions; - org.positions = parsedPositions; - } catch (error) { - console.error('Error parsing positions:', error); - return res.status(400).json({ - success: false, - message: "Invalid positions data format" - }); - } + return res.status(400).json({ + success: false, + message: "Role updates must use /org-roles endpoints" + }); } if (requireApprovalForJoin) { org.requireApprovalForJoin = requireApprovalForJoin; @@ -873,6 +867,7 @@ router.post("/:orgId/apply-to-org", verifyToken, async (req, res) => { org_id: orgId, user_id: userId, role: 'member', + roles: ['member'], }); await newMember.save(); // Check auto-approve for org (Atlas: when member count reaches threshold) @@ -920,7 +915,10 @@ router.post("/:orgId/apply-to-org", verifyToken, async (req, res) => { // Send notification to org admins about the new member const orgAdmins = await OrgMember.find({ org_id: orgId, - role: { $in: ['owner', 'admin'] } + $or: [ + { role: { $in: ['owner', 'admin'] } }, + { roles: { $in: ['owner', 'admin'] } } + ] }); console.log('orgAdmins', orgAdmins); @@ -1530,7 +1528,7 @@ router.post('/:orgId/approval-settings', verifyToken, requireMemberManagement(), } }) -router.get('/:orgId/forms', verifyToken, async (req, res) => { +router.get('/:orgId/forms', verifyToken, requireMemberManagement(), async (req, res) => { const { Form } = getModels(req, 'Form'); const { orgId } = req.params; try{ diff --git a/backend/schemas/org.js b/backend/schemas/org.js index 7d3f21c8..16952fec 100644 --- a/backend/schemas/org.js +++ b/backend/schemas/org.js @@ -381,6 +381,35 @@ OrgSchema.pre('findOne', function () { this.where({ isDeleted: { $ne: true } }); }); +OrgSchema.pre('validate', function(next) { + // Normalize legacy/out-of-range values so unrelated saves (e.g. image updates) don't fail. + if (this.messageSettings && typeof this.messageSettings.characterLimit === 'number') { + this.messageSettings.characterLimit = Math.max( + 100, + Math.min(2000, this.messageSettings.characterLimit) + ); + } + + next(); +}); + +OrgSchema.pre('save', function(next) { + if (Array.isArray(this.positions)) { + this.positions = this.positions.map((role) => { + const normalizedPermissions = [...new Set((role.permissions || []).filter(Boolean))]; + return { + ...role.toObject?.() || role, + permissions: normalizedPermissions, + canManageMembers: normalizedPermissions.includes('manage_members'), + canManageRoles: normalizedPermissions.includes('manage_roles'), + canManageEvents: normalizedPermissions.includes('manage_events'), + canViewAnalytics: normalizedPermissions.includes('view_analytics') + }; + }); + } + next(); +}); + // Add methods for role management OrgSchema.methods.addCustomRole = function(roleData) { if (!this.roleManagement.allowCustomRoles) { @@ -446,12 +475,6 @@ OrgSchema.methods.hasPermission = function(roleName, permission) { // Finance: managing implies viewing (roles often grant manage_finances only) if (permission === 'view_finances' && role.permissions.includes('manage_finances')) return true; - // Also check boolean flags (roles may have canManageEvents etc. without permissions array) - if (permission === 'manage_events' && role.canManageEvents) return true; - if (permission === 'manage_members' && role.canManageMembers) return true; - if (permission === 'manage_roles' && role.canManageRoles) return true; - if (permission === 'view_analytics' && role.canViewAnalytics) return true; - return false; }; diff --git a/backend/schemas/orgInvite.js b/backend/schemas/orgInvite.js index 1a603047..4ce80f62 100644 --- a/backend/schemas/orgInvite.js +++ b/backend/schemas/orgInvite.js @@ -25,6 +25,10 @@ const orgInviteSchema = new Schema({ required: true, default: 'member' }, + roles: { + type: [String], + default: ['member'] + }, invited_by: { type: Schema.Types.ObjectId, required: true, @@ -55,6 +59,15 @@ orgInviteSchema.index({ token: 1 }); orgInviteSchema.index({ user_id: 1, status: 1 }); orgInviteSchema.index({ org_id: 1, status: 1 }); +orgInviteSchema.pre('save', function(next) { + if (!Array.isArray(this.roles) || this.roles.length === 0) { + this.roles = [this.role || 'member']; + } + this.roles = [...new Set(this.roles.filter(Boolean))]; + this.role = this.roles[0] || this.role || 'member'; + next(); +}); + orgInviteSchema.statics.generateToken = function () { return crypto.randomBytes(32).toString('hex'); }; diff --git a/backend/schemas/orgMember.js b/backend/schemas/orgMember.js index 89e5d4e9..6ee2629f 100644 --- a/backend/schemas/orgMember.js +++ b/backend/schemas/orgMember.js @@ -17,6 +17,10 @@ const memberSchema = new Schema({ required: true, default: 'member' }, + roles: { + type: [String], + default: ['member'] + }, status: { type: String, enum: ['active', 'inactive', 'pending', 'suspended'], @@ -108,6 +112,18 @@ memberSchema.methods.hasPermission = async function(permission) { }; // Method to check permissions with Org model provided +memberSchema.methods.getAssignedRoles = function() { + if (Array.isArray(this.roles) && this.roles.length > 0) { + return [...new Set(this.roles.filter(Boolean))]; + } + return this.role ? [this.role] : ['member']; +}; + +memberSchema.methods.getPrimaryRole = function() { + const assignedRoles = this.getAssignedRoles(); + return assignedRoles[0] || 'member'; +}; + memberSchema.methods.hasPermissionWithOrg = async function(permission, org) { // Check if permission is explicitly denied if (this.deniedPermissions.includes(permission)) { @@ -119,10 +135,10 @@ memberSchema.methods.hasPermissionWithOrg = async function(permission, org) { return true; } - // Check role permissions using the provided org + // Check role permissions using the provided org (union for multi-role members) if (!org) return false; - - return org.hasPermission(this.role, permission); + const assignedRoles = this.getAssignedRoles(); + return assignedRoles.some((roleName) => org.hasPermission(roleName, permission)); }; memberSchema.methods.canManageMembers = async function(org) { @@ -161,6 +177,7 @@ memberSchema.methods.changeRole = async function(newRole, assignedBy, reason = ' reason: reason || '' }); this.role = newRole; + this.roles = [newRole]; this.assignedBy = assignedBy; this.assignedAt = new Date(); if (termStart !== undefined) { @@ -172,9 +189,32 @@ memberSchema.methods.changeRole = async function(newRole, assignedBy, reason = ' return this.save(); }; +memberSchema.methods.setRoles = async function(newRoles, assignedBy, reason = '', options = {}) { + const normalizedRoles = [...new Set((newRoles || []).filter(Boolean))]; + const safeRoles = normalizedRoles.length > 0 ? normalizedRoles : ['member']; + const primaryRole = safeRoles[0]; + return this.changeRole(primaryRole, assignedBy, reason, options).then(async () => { + this.roles = safeRoles; + return this.save(); + }); +}; + +memberSchema.pre('save', function(next) { + if (!Array.isArray(this.roles) || this.roles.length === 0) { + this.roles = [this.role || 'member']; + } + this.roles = [...new Set(this.roles.filter(Boolean))]; + this.role = this.roles[0] || this.role || 'member'; + next(); +}); + // Static method to get members by role memberSchema.statics.getMembersByRole = function(orgId, role) { - return this.find({ org_id: orgId, role: role, status: 'active' }) + return this.find({ + org_id: orgId, + status: 'active', + $or: [{ role }, { roles: role }] + }) .populate('user_id', 'username name email picture') .populate('assignedBy', 'username name'); }; diff --git a/backend/services/orgInviteService.js b/backend/services/orgInviteService.js index f4847f75..e38b8b45 100644 --- a/backend/services/orgInviteService.js +++ b/backend/services/orgInviteService.js @@ -125,7 +125,15 @@ function getBaseUrl(req) { * Create a single invite. Sends notification (if user exists) and email. * @returns {{ userExists: boolean, inviteId: string }} */ -async function createInvite(req, orgId, email, role) { +function normalizeAssignedRoles(roleOrRoles) { + if (Array.isArray(roleOrRoles)) { + const normalized = [...new Set(roleOrRoles.filter(Boolean))]; + return normalized.length > 0 ? normalized : ['member']; + } + return [roleOrRoles || 'member']; +} + +async function createInvite(req, orgId, email, roleOrRoles) { const { OrgInvite, User, Org, OrgMember } = getModels(req, 'OrgInvite', 'User', 'Org', 'OrgMember'); const inviterId = req.user.userId; @@ -139,10 +147,14 @@ async function createInvite(req, orgId, email, role) { throw new Error('Organization not found'); } - const roleExists = org.getRoleByName ? org.getRoleByName(role) : org.positions?.find(p => p.name === role); - if (!roleExists) { + const normalizedRoles = normalizeAssignedRoles(roleOrRoles); + const hasMissingRole = normalizedRoles.some((roleName) => !(org.getRoleByName ? org.getRoleByName(roleName) : org.positions?.find((p) => p.name === roleName))); + if (hasMissingRole) { throw new Error('Role not found'); } + if (normalizedRoles.includes('owner')) { + throw new Error('Owner role can only be assigned through ownership transfer'); + } const memberEmails = (await OrgMember.find({ org_id: orgId, status: 'active' }) .populate('user_id', 'email')) @@ -172,7 +184,8 @@ async function createInvite(req, orgId, email, role) { org_id: orgId, email: normalizedEmail, user_id: user?._id || null, - role, + role: normalizedRoles[0], + roles: normalizedRoles, invited_by: inviterId, status: 'pending', token, @@ -183,8 +196,8 @@ async function createInvite(req, orgId, email, role) { const inviter = await User.findById(inviterId).select('name username'); const inviterName = inviter?.name || inviter?.username || 'Someone'; - const roleObj = roleExists; - const roleDisplayName = roleObj?.displayName || role; + const roleObj = org.getRoleByName ? org.getRoleByName(normalizedRoles[0]) : org.positions?.find((p) => p.name === normalizedRoles[0]); + const roleDisplayName = roleObj?.displayName || normalizedRoles[0]; const baseUrl = getBaseUrl(req); @@ -197,7 +210,7 @@ async function createInvite(req, orgId, email, role) { 'org_invitation', { orgName: org.org_name, - role, + role: normalizedRoles[0], invitationId: invite._id.toString(), metadata: { inviteId: invite._id.toString() } } @@ -207,7 +220,7 @@ async function createInvite(req, orgId, email, role) { const emailHTML = buildExistingUserInviteEmail({ orgName: org.org_name, orgDescription: org.org_description, - role, + role: normalizedRoles[0], roleDisplayName, inviterName, acceptUrl: inviteUrl, @@ -318,9 +331,9 @@ async function createBatchInvites(req, orgId, invites) { const result = { sent: 0, skipped: 0, errors: [] }; - for (const { email, role } of invites) { + for (const { email, role, roles } of invites) { try { - await createInvite(req, orgId, email, role || 'member'); + await createInvite(req, orgId, email, roles || role || 'member'); result.sent++; } catch (err) { if (err.message?.includes('already a member') || err.message?.includes('already sent')) { @@ -374,6 +387,7 @@ async function acceptInvite(req, inviteId) { org_id: invite.org_id, user_id: userId, role: invite.role, + roles: invite.roles || [invite.role || 'member'], status: 'active', assignedBy: invite.invited_by }); @@ -469,6 +483,7 @@ async function getPendingForUser(req) { _id: inv._id, org: inv.org_id, role: inv.role, + roles: inv.roles || [inv.role || 'member'], invitedBy: inv.invited_by })); } diff --git a/backend/tests/route-outcomes/orgRoleRoutes.auth.outcomes.test.js b/backend/tests/route-outcomes/orgRoleRoutes.auth.outcomes.test.js new file mode 100644 index 00000000..a4e1c5f8 --- /dev/null +++ b/backend/tests/route-outcomes/orgRoleRoutes.auth.outcomes.test.js @@ -0,0 +1,21 @@ +const fs = require('fs'); +const path = require('path'); + +const routeFile = fs.readFileSync( + path.join(__dirname, '../../routes/orgRoleRoutes.js'), + 'utf8' +); + +describe('orgRoleRoutes authorization coverage', () => { + test('member listing route includes member-management guard', () => { + expect(routeFile).toMatch(/router\.get\('\/:orgId\/members',\s*verifyToken,\s*requireMemberManagement\(\)/); + }); + + test('member role assignment route includes member-management guard', () => { + expect(routeFile).toMatch(/router\.post\('\/:orgId\/members\/:userId\/role',\s*verifyToken,\s*requireMemberManagement\(\)/); + }); + + test('members-by-role route includes member-management guard', () => { + expect(routeFile).toMatch(/router\.get\('\/:orgId\/roles\/:roleName\/members',\s*verifyToken,\s*requireMemberManagement\(\)/); + }); +}); diff --git a/backend/tests/unit/orgPermissionModels.test.js b/backend/tests/unit/orgPermissionModels.test.js new file mode 100644 index 00000000..202d7fdd --- /dev/null +++ b/backend/tests/unit/orgPermissionModels.test.js @@ -0,0 +1,61 @@ +const mongoose = require('mongoose'); +const orgSchema = require('../../schemas/org'); +const orgMemberSchema = require('../../schemas/orgMember'); + +describe('org permission model invariants', () => { + const Org = mongoose.models.TestOrg || mongoose.model('TestOrg', orgSchema); + const OrgMember = mongoose.models.TestOrgMember || mongoose.model('TestOrgMember', orgMemberSchema); + + test('org member evaluates permissions as union of assigned roles', async () => { + const org = new Org({ + org_name: 'Test Org', + org_profile_image: '/Logo.svg', + org_description: 'desc', + owner: new mongoose.Types.ObjectId(), + positions: [ + { name: 'owner', displayName: 'Owner', permissions: ['all'], order: 0 }, + { name: 'member', displayName: 'Member', permissions: ['view_events'], order: 1 }, + { name: 'treasurer', displayName: 'Treasurer', permissions: ['view_finances'], order: 2 } + ] + }); + const member = new OrgMember({ + org_id: new mongoose.Types.ObjectId(), + user_id: new mongoose.Types.ObjectId(), + role: 'member', + roles: ['member', 'treasurer'], + status: 'active' + }); + + await expect(member.hasPermissionWithOrg('view_finances', org)).resolves.toBe(true); + await expect(member.hasPermissionWithOrg('manage_roles', org)).resolves.toBe(false); + }); + + test('org pre-save normalizes boolean fields from permissions', async () => { + const org = new Org({ + org_name: 'Normalizer Org', + org_profile_image: '/Logo.svg', + org_description: 'desc', + owner: new mongoose.Types.ObjectId(), + positions: [ + { name: 'owner', displayName: 'Owner', permissions: ['all'], order: 0 }, + { name: 'member', displayName: 'Member', permissions: ['view_events'], order: 1 }, + { + name: 'manager', + displayName: 'Manager', + permissions: ['manage_members'], + canManageMembers: false, + canManageRoles: true, + order: 2 + } + ] + }); + + await org.validate(); + // Trigger pre-save logic without touching database connection. + await new Promise((resolve, reject) => org.schema.s.hooks.execPre('save', org, (err) => (err ? reject(err) : resolve()))); + + const managerRole = org.positions.find((role) => role.name === 'manager'); + expect(managerRole.canManageMembers).toBe(true); + expect(managerRole.canManageRoles).toBe(false); + }); +}); From 05a1e779712436eb02506529e38281cb2d22a6e6 Mon Sep 17 00:00:00 2001 From: AZ0228 <53315675+AZ0228@users.noreply.github.com> Date: Mon, 27 Apr 2026 14:05:12 -0400 Subject: [PATCH 2/4] MER-189: redesign role management UX around view-first workflows. Ship the new role overview, modal-based member assignment, multi-role-aware member surfaces, and in-memory rename propagation so role/member state stays consistent without refresh. --- .../AddMemberForm/AddMemberForm.jsx | 9 +- .../src/components/Dashboard/Dashboard.jsx | 37 ++- frontend/src/components/Popup/Popup.scss | 2 +- .../components/RoleManager/RoleManager.jsx | 272 +++++++++++++++-- .../components/RoleManager/RoleManager.scss | 278 ++++++++++++++++++ .../RoleManager/RoleMemberManagementPopup.jsx | 110 +++++++ .../RoleMemberManagementPopup.scss | 168 +++++++++++ .../MemberApplicationsViewer.jsx | 27 +- .../src/pages/ClubDash/Members/Members.jsx | 128 +++++--- .../src/pages/ClubDash/Members/Members.scss | 23 ++ .../OrgSettings/components/RolesSettings.jsx | 62 +++- frontend/src/pages/ClubDash/Roles/Roles.jsx | 191 ++++++++++-- 12 files changed, 1200 insertions(+), 107 deletions(-) create mode 100644 frontend/src/components/RoleManager/RoleMemberManagementPopup.jsx create mode 100644 frontend/src/components/RoleManager/RoleMemberManagementPopup.scss diff --git a/frontend/src/components/AddMemberForm/AddMemberForm.jsx b/frontend/src/components/AddMemberForm/AddMemberForm.jsx index 136420b9..174923fd 100644 --- a/frontend/src/components/AddMemberForm/AddMemberForm.jsx +++ b/frontend/src/components/AddMemberForm/AddMemberForm.jsx @@ -28,6 +28,7 @@ function validateEmails(emails) { function AddMemberForm({ orgId, roles = [], + assignableRoles = [], existingMembers = [], onMemberAdded, onClose, @@ -49,7 +50,7 @@ function AddMemberForm({ const searchWrapperRef = useRef(null); const debounceRef = useRef(null); - const roleOptions = roles.length ? roles : [{ name: 'member', displayName: 'Member' }]; + const roleOptions = assignableRoles.length ? assignableRoles : (roles.length ? roles : [{ name: 'member', displayName: 'Member' }]); const existingMemberIds = useMemo( () => existingMembers.map(m => m.user_id?._id || m.user_id).filter(Boolean), [existingMembers] @@ -144,7 +145,8 @@ function AddMemberForm({ if (selectedToInvite.length === 0 || !orgId) return; const invites = selectedToInvite.map(item => ({ email: item.email, - role: item.role || 'member' + role: item.role || 'member', + roles: [item.role || 'member'] })); setSending(true); try { @@ -172,7 +174,8 @@ function AddMemberForm({ if (!batchPreviewData?.toInvite?.length || !orgId) return; const invites = batchPreviewData.toInvite.map(item => ({ email: item.email, - role: batchInviteRoles[item.email] || item.role || 'member' + role: batchInviteRoles[item.email] || item.role || 'member', + roles: [batchInviteRoles[item.email] || item.role || 'member'] })); setSending(true); try { diff --git a/frontend/src/components/Dashboard/Dashboard.jsx b/frontend/src/components/Dashboard/Dashboard.jsx index fa9d9b15..f8301610 100644 --- a/frontend/src/components/Dashboard/Dashboard.jsx +++ b/frontend/src/components/Dashboard/Dashboard.jsx @@ -174,14 +174,45 @@ function Dashboard({ if (sub !== null) { // We're in a sub-menu context const subIndex = parseInt(sub); - if (subIndex >= 0) { + const pageItem = menuItems[page]; + const subItems = pageItem?.subItems; + + // Rehydrate sub-menu state from URL so refresh/direct links preserve submenu context + if ( + page >= 0 && + page < menuItems.length && + Array.isArray(subItems) && + subItems.length > 0 && + subIndex >= 0 && + subIndex < subItems.length + ) { + setNavigationStack([{ + parentIndex: page, + subIndex, + items: subItems, + parentLabel: pageItem.label + }]); + setCurrentSubItems(subItems); + setShowBackButton(true); setCurrentDisplay(subIndex); + } else if (page >= 0 && page < menuItems.length) { + // Invalid sub param for this page - recover to main page context + setNavigationStack([]); + setCurrentSubItems(null); + setShowBackButton(false); + setCurrentDisplay(page); } } else if (page >= 0 && page < menuItems.length) { // We're in the main menu + setNavigationStack([]); + setCurrentSubItems(null); + setShowBackButton(false); setCurrentDisplay(page); } else if (!hasInitializedRef.current) { // Only fallback to default page on initial load if the parsed page is invalid + setNavigationStack([]); + setCurrentSubItems(null); + setShowBackButton(false); setCurrentDisplay(defaultPage); } @@ -307,11 +338,15 @@ function Dashboard({ setContentOpacity(0); setTimeout(() => { + setNavigationStack([]); + setCurrentSubItems(null); + setShowBackButton(false); setCurrentDisplay(index); // Update URL with the new page number, preserving other search params (e.g. adminView) setSearchParams(prev => { const next = new URLSearchParams(prev); next.set('page', index.toString()); + next.delete('sub'); return next; }, { replace: true }); diff --git a/frontend/src/components/Popup/Popup.scss b/frontend/src/components/Popup/Popup.scss index 28d2e4f8..cf0c7d25 100644 --- a/frontend/src/components/Popup/Popup.scss +++ b/frontend/src/components/Popup/Popup.scss @@ -50,7 +50,7 @@ .popup-overlay .popup-content { background-color: var(--background); padding: 20px; - border-radius: 10px; + border-radius: 20px; position: relative; // max-width:500px; box-sizing: border-box; diff --git a/frontend/src/components/RoleManager/RoleManager.jsx b/frontend/src/components/RoleManager/RoleManager.jsx index 55958a25..1b86f420 100644 --- a/frontend/src/components/RoleManager/RoleManager.jsx +++ b/frontend/src/components/RoleManager/RoleManager.jsx @@ -3,17 +3,21 @@ import './RoleManager.scss'; import { Icon } from '@iconify-icon/react'; import { getOrgRoleColor } from '../../utils/orgUtils'; import DraggableList from '../DraggableList/DraggableList'; +import RoleMemberManagementPopup from './RoleMemberManagementPopup'; -const RoleManager = forwardRef(({ roles, onRolesChange, onDeleteRequest, isEditable = true, roleHighlight = false, saveImmediately = false, onDraftChange, userRoleData = null, isOwner = false }, ref) => { +const RoleManager = forwardRef(({ roles, onRolesChange, onDeleteRequest, isEditable = true, roleHighlight = false, saveImmediately = false, onDraftChange, userRoleData = null, isOwner = false, members = [], showHeaderEditToggle = false, onEditModeChange, onMemberRolesChange, memberRoleActionPending = {} }, ref) => { const [customRoles, setCustomRoles] = useState(roles || []); const userHasEditedRef = useRef(false); const [selectedRole, setSelectedRole] = useState(null); + const [internalEditMode, setInternalEditMode] = useState(false); + const [memberManagementRoleName, setMemberManagementRoleName] = useState(null); const [formData, setFormData] = useState({ name: '', permissions: [], color: '#a855f7', useCustomColor: false }); + const effectiveEditable = isEditable && (!showHeaderEditToggle || internalEditMode); // Plaeholder role color pallete, needs to be updated. const colorPalette = [ @@ -348,9 +352,15 @@ const RoleManager = forwardRef(({ roles, onRolesChange, onDeleteRequest, isEdita }; useImperativeHandle(ref, () => ({ - createDraftRole + createDraftRole, + setEditMode: (isEditing) => setInternalEditMode(Boolean(isEditing)) })); + useEffect(() => { + if (!onEditModeChange) return; + onEditModeChange(Boolean(internalEditMode)); + }, [internalEditMode, onEditModeChange]); + const handleRoleSelect = (role) => { userHasEditedRef.current = false; // Reset - we're loading role data, not user edit setSelectedRole(role); @@ -408,6 +418,25 @@ const RoleManager = forwardRef(({ roles, onRolesChange, onDeleteRequest, isEdita const permission = availablePermissions.find(p => p.key === permissionKey); return permission ? permission.label : permissionKey; }; + const getMemberDisplayName = (member) => { + const user = member?.user_id || member?.user || member; + if (!user) return 'Unknown User'; + return user.name || user.username || user.email || 'Unknown User'; + }; + const getMemberId = (member) => { + const user = member?.user_id || member?.user || member; + return user?._id || member?._id || `${member?.role || 'member'}-${getMemberDisplayName(member)}`; + }; + const getMemberInitial = (member) => { + const label = getMemberDisplayName(member); + return label?.charAt(0)?.toUpperCase() || 'U'; + }; + const getMemberRoles = (member) => { + const roleFromLegacyField = member?.role ? [member.role] : []; + const roleArray = Array.isArray(member?.roles) ? member.roles.filter(Boolean) : []; + const merged = [...new Set([...roleFromLegacyField, ...roleArray])]; + return merged.length > 0 ? merged : ['member']; + }; // Unless owner: cannot edit roles more privileged than yours. Same privilege level is allowed // (e.g. deleting a top custom role you assigned yourself). Lower order = higher privilege. @@ -416,6 +445,10 @@ const RoleManager = forwardRef(({ roles, onRolesChange, onDeleteRequest, isEdita const targetOrder = role?.order ?? 999; return targetOrder >= (userRoleData?.order ?? -1); }; + const isSystemRole = (role) => role?.name === 'owner' || role?.name === 'member'; + + const ownerRole = customRoles.find((role) => role.name === 'owner'); + const draftRoleLabel = formData.name?.trim() ? formData.name.trim() : 'New Role'; // Get all roles except owner, sorted by order (member always last) const editableRoles = customRoles @@ -428,23 +461,201 @@ const RoleManager = forwardRef(({ roles, onRolesChange, onDeleteRequest, isEdita return (a.order || 0) - (b.order || 0); }); + const orderedRoles = [...customRoles].sort((a, b) => { + if (a.name === 'owner') return -1; + if (b.name === 'owner') return 1; + if (a.name === 'member') return 1; + if (b.name === 'member') return -1; + return (a.order || 0) - (b.order || 0); + }); + + const managedRole = memberManagementRoleName + ? orderedRoles.find((role) => role.name === memberManagementRoleName) + : null; + const managedRoleMembers = managedRole + ? members.filter((member) => getMemberRoles(member).includes(managedRole.name)) + : []; + const managedRoleAssignableMembers = managedRole + ? members.filter((member) => !getMemberRoles(member).includes(managedRole.name)) + : []; + + if (!effectiveEditable) { + return ( +
View permissions and member distribution by role.
+{roleMembers.length} member{roleMembers.length === 1 ? '' : 's'}
+Members
+ {visibleMembers.length === 0 ? ( +No members currently assigned.
+ ) : ( +Choose a role from the list on the left to view and edit its permissions
- {isEditable && ( + {effectiveEditable && ( diff --git a/frontend/src/components/RoleManager/RoleManager.scss b/frontend/src/components/RoleManager/RoleManager.scss index 1e95964d..3292f66e 100644 --- a/frontend/src/components/RoleManager/RoleManager.scss +++ b/frontend/src/components/RoleManager/RoleManager.scss @@ -22,6 +22,234 @@ font-family: 'Inter', sans-serif; height: 100%; + &.role-manager--view { + border: 1px solid var(--rm-border-color); + border-left: none; + border-right: none; + background: var(--rm-background); + padding: 16px; + box-sizing: border-box; + + .role-overview-header { + margin-bottom: 12px; + + &__row { + display: flex; + align-items: flex-start; + justify-content: space-between; + gap: 10px; + } + + h3 { + margin: 0; + font-size: 18px; + color: var(--rm-text-primary); + } + + p { + margin: 4px 0 0; + font-size: 13px; + color: var(--rm-text-tertiary); + } + + &__edit-btn { + display: inline-flex; + align-items: center; + gap: 6px; + padding: 8px 12px; + border-radius: 8px; + border: 1px solid var(--rm-border-color); + background: var(--rm-background); + color: var(--rm-text-primary); + font-size: 12px; + font-weight: 600; + cursor: pointer; + transition: all 0.2s ease; + + &:hover { + border-color: var(--rm-primary); + color: var(--rm-primary); + } + } + } + + .role-overview-grid { + display: grid; + grid-template-columns: repeat(auto-fill, minmax(280px, 1fr)); + gap: 12px; + } + + .role-overview-card { + border: 1px solid var(--rm-border-color); + border-radius: 12px; + padding: 12px; + + &__header { + display: flex; + align-items: flex-start; + justify-content: space-between; + gap: 8px; + margin-bottom: 10px; + } + + &__identity { + display: flex; + align-items: center; + gap: 8px; + + h4 { + margin: 0; + font-size: 14px; + color: var(--rm-text-primary); + } + + p { + margin: 2px 0 0; + font-size: 12px; + color: var(--rm-text-tertiary); + } + } + + &__dot { + width: 22px; + height: 22px; + border-radius: 8px; + flex-shrink: 0; + } + + &__system-badge { + font-size: 11px; + padding: 2px 8px; + border-radius: 999px; + background: var(--rm-background); + border: 1px solid var(--rm-border-color); + color: var(--rm-text-tertiary); + } + + &__manage-icon-btn { + width: 28px; + height: 28px; + margin-left: auto; + border: 1px solid var(--rm-border-color); + border-radius: 8px; + background: var(--rm-background); + color: var(--rm-text-tertiary); + display: inline-flex; + align-items: center; + justify-content: center; + cursor: pointer; + transition: all 0.2s ease; + + &:hover { + border-color: var(--org-primary, #64AB6C); + color: var(--org-primary, #64AB6C); + background: var(--org-primary-transparent, rgba(100, 171, 108, 0.12)); + } + + iconify-icon, + svg { + font-size: 16px; + } + } + + &__permissions { + display: flex; + flex-wrap: wrap; + gap: 6px; + margin-bottom: 12px; + + .permission-chip { + font-size: 11px; + padding: 4px 8px; + border-radius: 999px; + background: var(--rm-background); + border: 1px solid var(--rm-border-color); + color: var(--rm-text-secondary); + + &--empty { + color: var(--rm-text-tertiary); + } + + &--more { + color: var(--rm-primary); + border-color: rgba(109, 142, 250, 0.35); + } + } + } + + &__members-title { + margin: 0 0 6px; + font-size: 12px; + color: var(--rm-text-tertiary); + font-weight: 600; + } + + &__empty-members { + margin: 0; + font-size: 12px; + color: var(--rm-text-quaternary); + } + + &__member-list { + display: flex; + flex-wrap: wrap; + gap: 6px; + } + + &__member-pill { + display: inline-flex; + align-items: center; + gap: 6px; + padding: 4px 8px; + border-radius: 999px; + background: var(--rm-background); + border: 1px solid var(--rm-border-color); + font-size: 12px; + color: var(--rm-text-secondary); + + .member-initial { + display: inline-flex; + width: 18px; + height: 18px; + border-radius: 50%; + align-items: center; + justify-content: center; + background: var(--rm-primary-light); + color: var(--rm-primary); + font-size: 10px; + font-weight: 700; + } + + .member-name { + max-width: 150px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + + &--more { + color: var(--rm-text-tertiary); + font-weight: 600; + } + + .member-pill-action { + border: none; + background: transparent; + color: #dc2626; + font-size: 11px; + cursor: pointer; + padding: 0 2px; + + &:disabled { + opacity: 0.5; + cursor: not-allowed; + } + } + } + + } + } + .role-manager-layout { display: flex; height: 100%; @@ -46,6 +274,30 @@ .role-list-header { padding: 12px; border-bottom: 1px solid var(--rm-border-color); + display: flex; + flex-direction: column; + gap: 8px; + + .cancel-edit-btn { + width: 100%; + display: inline-flex; + align-items: center; + justify-content: center; + padding: 8px 10px; + border-radius: 10px; + border: 1px solid var(--rm-border-color); + background: var(--rm-background); + color: var(--rm-text-secondary); + font-size: 12px; + font-weight: 600; + cursor: pointer; + transition: all 0.2s ease; + + &:hover { + border-color: var(--rm-text-tertiary); + color: var(--rm-text-primary); + } + } .add-role-btn { width: 100%; @@ -129,6 +381,32 @@ } } + &.system-role { + .role-list-item-info { + flex-direction: column; + align-items: flex-start; + gap: 2px; + + small { + font-size: 11px; + color: var(--rm-text-tertiary); + } + } + } + + &.draft-role-item { + .role-list-item-info { + flex-direction: column; + align-items: flex-start; + gap: 2px; + + small { + font-size: 11px; + color: var(--rm-text-tertiary); + } + } + } + .drag-handle { display: flex; align-items: center; diff --git a/frontend/src/components/RoleManager/RoleMemberManagementPopup.jsx b/frontend/src/components/RoleManager/RoleMemberManagementPopup.jsx new file mode 100644 index 00000000..559a917a --- /dev/null +++ b/frontend/src/components/RoleManager/RoleMemberManagementPopup.jsx @@ -0,0 +1,110 @@ +import React, { useMemo, useState } from 'react'; +import Popup from '../Popup/Popup'; +import './RoleMemberManagementPopup.scss'; + +function RoleMemberManagementPopup({ + role, + isOpen, + onClose, + assignedMembers = [], + assignableMembers = [], + getMemberId, + getMemberDisplayName, + getMemberInitial, + onAssignMember, + onRemoveMember, + memberRoleActionPending = {} +}) { + const [searchTerm, setSearchTerm] = useState(''); + const filteredAssignableMembers = useMemo(() => { + const query = searchTerm.trim().toLowerCase(); + if (!query) return assignableMembers; + return assignableMembers.filter((member) => getMemberDisplayName(member).toLowerCase().includes(query)); + }, [assignableMembers, getMemberDisplayName, searchTerm]); + const filteredAssignedMembers = useMemo(() => { + const query = searchTerm.trim().toLowerCase(); + if (!query) return assignedMembers; + return assignedMembers.filter((member) => getMemberDisplayName(member).toLowerCase().includes(query)); + }, [assignedMembers, getMemberDisplayName, searchTerm]); + + return ( +Search members to quickly assign or remove this role.
+No matching members.
+ ) : ( +No matching members.
+ ) : ( +
+ Backfills owner memberships so every organization owner also has the immutable owner role.
+