Team leads can add any user except the core admin; allow email usernames

app/controllers/team.controller.js

@@ -6,6 +6,8 @@ const {
   ConflictError, NotFoundError, ForbiddenError, handleError,
 } = require('../exceptions/errors.js');
 const authMiddleware = require('../utils/auth-middleware.js');
+const validationUtils = require('../utils/validation-utils.js');
+const { getCoreAdminUsername } = require('../utils/admin-seed-service.js');
 
 const log = debug('team:controller');
 
@@ -153,14 +155,6 @@ exports.delete = (req, res) => {
  */
 const MEMBER_FIELDS = '_id username role authProvider';
 
-/**
- * Team leads manage plain users only. An admin's access is global, and a team lead's
- * `team_lead` role is what grants lead powers over every team they belong to - so letting
- * a lead add or remove another lead would let them hand out (or take away) lead access.
- * Those memberships stay with admins.
- */
-const canManageMembership = (caller, targetUser) => caller.role === 'admin' || targetUser.role === 'user';
-
 exports.getMembers = async (req, res) => {
   const errors = validationResult(req);
   if (!errors.isEmpty()) {
@@ -177,7 +171,11 @@ exports.getMembers = async (req, res) => {
       throw new NotFoundError(`Team not found with id ${teamId}`);
     }
     const members = await User.find({ teams: teamId }, MEMBER_FIELDS).sort('username').lean();
-    return res.status(200).send(members);
+    // Flag the core admin so a client can show it as not removable by a team lead.
+    const coreAdmin = getCoreAdminUsername();
+    return res.status(200).send(members.map((member) => ({
+      ...member, isCoreAdmin: member.username === coreAdmin,
+    })));
   } catch (err) {
     return handleError(err, res);
   }
@@ -203,8 +201,8 @@ exports.addMember = async (req, res) => {
     if (!user) {
       throw new NotFoundError(`User "${username}" not found.`);
     }
-    if (!canManageMembership(req.user, user)) {
-      throw new ForbiddenError('Forbidden. Only an admin can add team leads or admins to a team.');
+    if (req.user.role !== 'admin' && user.username === getCoreAdminUsername()) {
+      throw new ForbiddenError('Forbidden. The core admin account cannot be added to a team by a team lead.');
     }
     if (user.teams.some((id) => id.toString() === teamId)) {
       throw new ConflictError(`User "${username}" is already a member of this team.`);
@@ -243,8 +241,8 @@ exports.removeMember = async (req, res) => {
     if (req.user.role !== 'admin' && req.user._id.toString() === userId) {
       throw new ForbiddenError('Forbidden. You cannot remove yourself from a team you lead; ask an admin.');
     }
-    if (!canManageMembership(req.user, user)) {
-      throw new ForbiddenError('Forbidden. Only an admin can remove team leads or admins from a team.');
+    if (req.user.role !== 'admin' && user.username === getCoreAdminUsername()) {
+      throw new ForbiddenError('Forbidden. The core admin account cannot be removed from a team by a team lead.');
     }
     await User.updateOne({ _id: user._id }, { $pull: { teams: teamId } });
     log(`Removed user "${user.username}" from team "${team.name}" (${teamId}).`);
@@ -262,9 +260,9 @@ exports.searchMemberCandidates = async (req, res) => {
     return res.status(422).json({ errors: errors.array() });
   }
   const { teamId } = req.params;
-  // The validator limits q to [A-Za-z0-9-_], none of which is a regex metacharacter, so it
-  // can go into the pattern as is. Usernames are stored lowercase.
-  const term = req.query.q.trim().toLowerCase();
+  // Usernames are stored lowercase. The term can contain regex metacharacters (. + in an
+  // email address), so it is escaped and matched literally.
+  const term = validationUtils.escapeRegex(req.query.q.trim().toLowerCase());
 
   try {
     if (!authMiddleware.hasTeamLeadAccess(req.user, teamId)) {
@@ -274,14 +272,12 @@ exports.searchMemberCandidates = async (req, res) => {
     if (!team) {
       throw new NotFoundError(`Team not found with id ${teamId}`);
     }
+    // Everyone except people already in the team and the core admin, whatever their role or
+    // how they sign in (local, LDAP, SSO).
     const filter = {
-      username: new RegExp(term),
+      username: { $regex: term, $ne: getCoreAdminUsername() },
       teams: { $ne: teamId },
     };
-    // Same rule as addMember: only an admin can add team leads or admins.
-    if (req.user.role !== 'admin') {
-      filter.role = 'user';
-    }
     const candidates = await User.find(filter, '_id username role')
       .sort('username')
       .limit(CANDIDATE_LIMIT)

app/routes/team.routes.js

@@ -2,6 +2,11 @@ const { check, param, query } = require('express-validator');
 const teamController = require('../controllers/team.controller.js');
 const authMiddleware = require('../utils/auth-middleware.js');
 
+// What a username can contain when it is looked up for team membership. Local accounts are
+// letters, digits, hyphens and underscores, but accounts created by a single sign-on login
+// take the identifier the provider reports, which is often an email address.
+const USERNAME_CHARACTERS = /^[A-Za-z0-9._@+-]+$/;
+
 module.exports = (app, path) => {
   app.post(`${path}/team`, authMiddleware.authorizeAdmin, [
     check('name')
@@ -54,27 +59,28 @@ module.exports = (app, path) => {
 
   // Predictive search for the "add member" box. Deliberately narrow, because it lets a team
   // lead look up accounts they otherwise cannot list: it needs at least three characters of
-  // the username, returns a handful of matches, and (for a team lead) only users they could
-  // add anyway - never admins, team leads, or people already in the team.
+  // the username and returns a handful of matches, never the core admin account or people
+  // already in the team.
   app.get(`${path}/team/:teamId/members/candidates`, authMiddleware.preventTokenAuth, [
     param('teamId').isMongoId(),
     query('q')
       .exists({ checkFalsy: true })
       .isString()
       .trim()
-      .isLength({ min: 3, max: 50 })
-      .withMessage('q must be between 3 and 50 characters.')
-      .matches(/^[A-Za-z0-9-_]+$/)
-      .withMessage('q must only contain letters, numbers, hyphens or underscores.'),
+      .isLength({ min: 3, max: 254 })
+      .withMessage('q must be between 3 and 254 characters.')
+      .matches(USERNAME_CHARACTERS)
+      .withMessage('q must only contain letters, numbers and . _ @ + - characters.'),
   ], teamController.searchMemberCandidates);
 
   app.post(`${path}/team/:teamId/members`, authMiddleware.preventTokenAuth, [
     param('teamId').isMongoId(),
     check('username')
       .exists({ checkFalsy: true })
       .isString()
-      .matches(/^[A-Za-z0-9-_]{2,50}$/)
-      .withMessage('Username must only contain letters, numbers, hyphens or underscores (2–50 characters).'),
+      .isLength({ min: 2, max: 254 })
+      .matches(USERNAME_CHARACTERS)
+      .withMessage('Username must be 2 to 254 characters: letters, numbers and . _ @ + - only.'),
   ], teamController.addMember);
 
   app.delete(`${path}/team/:teamId/members/:userId`, authMiddleware.preventTokenAuth, [

app/utils/admin-seed-service.js

@@ -20,9 +20,17 @@ const BCRYPT_ROUNDS = 10;
  * 'no-password' (env var unset), 'weak-password' (fails the strength policy, with the unmet
  * requirements in violations), or 'exists' (admin already present).
  */
+/**
+ * Username of the core admin: the account seeded at first run (ANGLES_ADMIN_USERNAME,
+ * default 'admin'). It is the deployment's break-glass account, so it is kept out of the
+ * team-membership features that team leads can use.
+ * @returns {string}
+ */
+const getCoreAdminUsername = () => (process.env.ANGLES_ADMIN_USERNAME || 'admin').toLowerCase();
+
 const ensureAdminUser = async () => {
   const password = process.env.ANGLES_ADMIN_PASSWORD;
-  const username = (process.env.ANGLES_ADMIN_USERNAME || 'admin').toLowerCase();
+  const username = getCoreAdminUsername();
 
   if (!password) {
     log('ANGLES_ADMIN_PASSWORD not set; skipping admin seed.');
@@ -56,4 +64,5 @@ const ensureAdminUser = async () => {
 
 module.exports = {
   ensureAdminUser,
+  getCoreAdminUsername,
 };

swagger/swagger.json

@@ -412,7 +412,7 @@
           "Team"
         ],
         "summary": "Add a user to a team",
-        "description": "Session authentication only - API tokens (x-api-key) are rejected on this endpoint with a 403. Callable by an admin or a team lead of this team. A team lead can only add users with the 'user' role; adding team leads or admins is admin-only.",
+        "description": "Session authentication only - API tokens (x-api-key) are rejected on this endpoint with a 403. Callable by an admin or a team lead of this team. A team lead can add any existing user except the core admin account (the seeded ANGLES_ADMIN_USERNAME account); an admin can add anyone. The username is matched case-insensitively and can be an email address.",
         "requestBody": {
           "required": true,
           "content": {
@@ -424,7 +424,7 @@
                 "properties": {
                   "username": {
                     "type": "string",
-                    "description": "Username of the existing user to add (case-insensitive)."
+                    "description": "Username of the existing user to add (case-insensitive). Letters, numbers and . _ @ + - characters, 2 to 254 characters, so email-style usernames work."
                   }
                 }
               }
@@ -443,7 +443,7 @@
             }
           },
           "403": {
-            "description": "Caller may not manage this team, or may not add a user with that role",
+            "description": "Caller may not manage this team, or a team lead tried to add the core admin account",
             "content": {
               "application/json": {
                 "schema": {
@@ -501,17 +501,17 @@
           "Team"
         ],
         "summary": "Search for users that can be added to a team",
-        "description": "Session authentication only - API tokens (x-api-key) are rejected on this endpoint with a 403. Callable by an admin or a team lead of this team. Powers the type-ahead on the add-member box. Matches part of the username (case-insensitive), returns at most 10 users ordered by username, and leaves out users who are already in the team. For a team lead only users with the 'user' role are returned, since those are the only ones a lead can add; an admin also sees team leads and admins.",
+        "description": "Session authentication only - API tokens (x-api-key) are rejected on this endpoint with a 403. Callable by an admin or a team lead of this team. Powers the type-ahead on the add-member box. Matches part of the username (case-insensitive, literal - characters such as . and + are not wildcards), returns at most 10 users ordered by username, and leaves out users who are already in the team and the core admin account. Every other user is offered, whatever their role or whether they sign in locally, with LDAP or with SSO.",
         "parameters": [
           {
             "name": "q",
             "in": "query",
             "required": true,
-            "description": "Part of the username: 3 to 50 letters, numbers, hyphens or underscores.",
+            "description": "Part of the username: 3 to 254 letters, numbers and . _ @ + - characters (email-style usernames are supported).",
             "schema": {
               "type": "string",
               "minLength": 3,
-              "maxLength": 50
+              "maxLength": 254
             }
           }
         ],
@@ -601,7 +601,7 @@
           "Team"
         ],
         "summary": "Remove a user from a team",
-        "description": "Session authentication only - API tokens (x-api-key) are rejected on this endpoint with a 403. Callable by an admin or a team lead of this team. A team lead can only remove users with the 'user' role, and cannot remove themselves; removing team leads or admins is admin-only. The user's membership of other teams is unaffected.",
+        "description": "Session authentication only - API tokens (x-api-key) are rejected on this endpoint with a 403. Callable by an admin or a team lead of this team. A team lead can remove any member except themselves and the core admin account; an admin can remove anyone. The user's membership of other teams is unaffected.",
         "responses": {
           "200": {
             "description": "The user was removed from the team",
@@ -614,7 +614,7 @@
             }
           },
           "403": {
-            "description": "Caller may not manage this team, or may not remove that user",
+            "description": "Caller may not manage this team, or a team lead tried to remove themselves or the core admin account",
             "content": {
               "application/json": {
                 "schema": {
@@ -10038,6 +10038,10 @@
           "authProvider": {
             "type": "string",
             "description": "'local' or the id of the SSO provider that authenticates the user."
+          },
+          "isCoreAdmin": {
+            "type": "boolean",
+            "description": "Present in the member list: true for the core admin account, which a team lead cannot remove from a team."
           }
         }
       }