Fix #619: invitee username collisions no longer fail with a raw 403.

Inviting by email derives the new user's username from the email local
part; a collision with an existing user (cats@foo.com then
cats@facebook.com) made Accounts.createUser throw Meteor's raw
"403 Username already exists", shown to the inviter as just "403". The
username now probes cats, cats1, cats2, ... to the first free variant
(bounded), and exhaustion raises the translated error-username-taken.

Tests: tests/inviteeUsername.test.cjs (11, positive and negative,
including the literal reported scenario).

Closes #804: already fixed - every popup is viewport-clamped with
internal scrolling since ac368de06 (#5667 geometry rewrite: maxHeight
from live viewport space, bounded flex layout) plus the 04eb8a8fb
(#6221) global 90vh clamp; the member picker inherits the shell, so
neither standalone nor the Sandstorm iframe can grow past the viewport.

Thanks to lemoer, ocdtrekkie and xet7.

CHANGELOG.md

@@ -195,6 +195,14 @@ and fixes the following bugs:
   `tests/oauth2LoginStyle.test.cjs` (12 tests, positive + negative). Thanks to ArturRuta and
   xet7.
 
+- **Inviting a second user whose email shares a local part failed with a bare "403"**
+  ([#619](https://github.com/wekan/wekan/issues/619), `server/models/users.js`, new
+  `models/lib/inviteeUsername.js`): inviting `cats@foo.com` creates user "cats"; inviting
+  `cats@facebook.com` then crashed into Meteor's raw `403 Username already exists`. The
+  invitee's username now probes `cats`, `cats1`, `cats2`, … to the first free variant, and
+  exhaustion raises the translated `error-username-taken` instead of a number. Tests:
+  `tests/inviteeUsername.test.cjs` (11, incl. the literal reported scenario). Thanks to
+  lemoer and xet7.
 - **Date pickers ignored the configured default time and stored 12:00**
   ([#1502](https://github.com/wekan/wekan/issues/1502), `client/lib/datepicker.js`, new
   `imports/lib/datePickerTime.js`): the due-date picker configures a 17:00 default (and

models/lib/inviteeUsername.js

@@ -0,0 +1,71 @@
+'use strict';
+
+// Pure helpers for deriving a username for a NEW user invited to a board by
+// email address (Meteor method `inviteUserToBoard` in server/models/users.js).
+// Extracted so the username-collision rules are unit-testable without Meteor.
+//
+// #619 ("Not possible to invite multiple users with similar email
+// addresses"): inviting cats@foo.com creates a user named "cats" (the email
+// local part). Inviting cats@facebook.com afterwards passed the bare local
+// part to Accounts.createUser again, which threw Meteor's raw
+// `Error 403: Username already exists.` — surfaced in the Add-Member popup as
+// an "inaccurate error: 403". The invitee's username is an implementation
+// detail here (the human typed an EMAIL, not a username), so instead of
+// failing, the caller should pick the first free variant of the local part:
+// "cats", "cats1", "cats2", … up to a bounded number of attempts.
+
+// Lowercased local part of an email address ("Cats@Foo.com" -> "cats").
+// Mirrors the normalization inviteUserToBoard applies (whole address is
+// lowercased before splitting). Returns '' for non-strings and for strings
+// without a usable local part (no '@', or '@' first), so callers can treat
+// falsy as "not an email".
+function usernameFromEmail(email) {
+  if (typeof email !== 'string') {
+    return '';
+  }
+  const normalized = email.trim().toLowerCase();
+  const posAt = normalized.indexOf('@');
+  if (posAt <= 0) {
+    return '';
+  }
+  return normalized.substring(0, posAt);
+}
+
+// Derive a username for a new invitee that does not collide with an existing
+// account: the email local part itself when free, otherwise the first free
+// "<localPart><n>" (n = 1, 2, …). `isUsernameTaken(candidate)` is supplied by
+// the caller (sync or async, e.g. a ReactiveCache.getUser lookup) so this
+// stays a pure, Meteor-free module.
+//
+// Returns null when the email has no usable local part or when every
+// candidate within `maxAttempts` is taken — callers must treat null as
+// "could not create the user" and raise a proper, translatable error instead
+// of letting Accounts.createUser throw a raw 403.
+async function deriveUniqueInviteeUsername(
+  email,
+  isUsernameTaken,
+  { maxAttempts = 100 } = {},
+) {
+  if (typeof isUsernameTaken !== 'function') {
+    throw new TypeError('isUsernameTaken must be a function');
+  }
+  const base = usernameFromEmail(email);
+  if (!base) {
+    return null;
+  }
+  if (!(await isUsernameTaken(base))) {
+    return base;
+  }
+  for (let suffix = 1; suffix < maxAttempts; suffix += 1) {
+    const candidate = `${base}${suffix}`;
+    if (!(await isUsernameTaken(candidate))) {
+      return candidate;
+    }
+  }
+  return null;
+}
+
+module.exports = {
+  usernameFromEmail,
+  deriveUniqueInviteeUsername,
+};

server/models/users.js

@@ -905,7 +905,16 @@ Meteor.methods({
         throw new Meteor.Error('error-user-notCreated');
       }
       const email = username.toLowerCase();
-      username = email.substring(0, posAt);
+      // #619: the invitee's username is derived from the email local part; a
+      // collision with an existing user (e.g. cats@foo.com then
+      // cats@facebook.com) made Accounts.createUser throw a raw
+      // "403 Username already exists". Probe cats, cats1, cats2, ... instead.
+      const { deriveUniqueInviteeUsername } = require('/models/lib/inviteeUsername');
+      username = await deriveUniqueInviteeUsername(
+        email,
+        async candidate => !!(await ReactiveCache.getUser({ username: candidate })),
+      );
+      if (username === null) throw new Meteor.Error('error-username-taken');
       if (username.includes('/') || email.includes('/')) {
         return false;
       }

tests/inviteeUsername.test.cjs

@@ -0,0 +1,171 @@
+'use strict';
+
+// Plain-Node unit test (no Meteor) for models/lib/inviteeUsername.js.
+// Run: ELECTRON_RUN_AS_NODE=1 <node> tests/inviteeUsername.test.cjs
+//
+// Regression guard for #619 ("Not possible to invite multiple users with
+// similar email addresses"): inviting cats@foo.com to a board creates a user
+// named "cats"; inviting cats@facebook.com afterwards re-derived the same
+// username and Accounts.createUser threw a raw "403 Username already exists",
+// shown to the inviter as just "403". deriveUniqueInviteeUsername must hand
+// back a non-colliding variant ("cats1") instead.
+
+const assert = require('assert');
+const {
+  usernameFromEmail,
+  deriveUniqueInviteeUsername,
+} = require('../models/lib/inviteeUsername.js');
+
+let passed = 0;
+const tests = [];
+function test(name, fn) {
+  tests.push([name, fn]);
+}
+
+// ---------------------------------------------------------------- fixtures
+
+// A fake user directory: isUsernameTaken backed by a Set, like the
+// ReactiveCache.getUser({ username }) lookup the server method would use.
+function directory(existing) {
+  const names = new Set(existing);
+  return {
+    isTaken: candidate => names.has(candidate),
+    isTakenAsync: async candidate => names.has(candidate),
+    add: name => names.add(name),
+  };
+}
+
+// ------------------------------------------------------ usernameFromEmail
+
+test('usernameFromEmail extracts the lowercased local part', () => {
+  assert.strictEqual(usernameFromEmail('cats@foo.com'), 'cats');
+  assert.strictEqual(usernameFromEmail('Cats@Facebook.COM'), 'cats');
+  assert.strictEqual(usernameFromEmail('  spaced@x.org  '), 'spaced');
+});
+
+test('usernameFromEmail rejects non-emails (negative)', () => {
+  assert.strictEqual(usernameFromEmail('no-at-sign'), '');
+  assert.strictEqual(usernameFromEmail('@leading-at.com'), '');
+  assert.strictEqual(usernameFromEmail(''), '');
+  assert.strictEqual(usernameFromEmail(null), '');
+  assert.strictEqual(usernameFromEmail(42), '');
+});
+
+// -------------------------------------------- deriveUniqueInviteeUsername
+
+test('first invitee gets the plain local part', async () => {
+  const dir = directory([]);
+  const name = await deriveUniqueInviteeUsername('cats@foo.com', dir.isTaken);
+  assert.strictEqual(name, 'cats');
+});
+
+test('#619 scenario: second similar email gets a suffixed username', async () => {
+  const dir = directory([]);
+  // Invite 1: cats@foo.com -> "cats" (then the account exists).
+  const first = await deriveUniqueInviteeUsername('cats@foo.com', dir.isTaken);
+  assert.strictEqual(first, 'cats');
+  dir.add(first);
+  // Invite 2: cats@facebook.com must NOT collide (the old code re-derived
+  // "cats" here and Accounts.createUser threw the raw 403).
+  const second = await deriveUniqueInviteeUsername(
+    'cats@facebook.com',
+    dir.isTaken,
+  );
+  assert.strictEqual(second, 'cats1');
+  dir.add(second);
+  // Invite 3: yet another cats@… walks to the next free suffix.
+  const third = await deriveUniqueInviteeUsername('cats@bar.io', dir.isTaken);
+  assert.strictEqual(third, 'cats2');
+});
+
+test('collision with a pre-existing non-invited account is also avoided', async () => {
+  // e.g. someone registered the username "admin" long ago.
+  const dir = directory(['admin', 'admin1']);
+  const name = await deriveUniqueInviteeUsername(
+    'admin@example.com',
+    dir.isTaken,
+  );
+  assert.strictEqual(name, 'admin2');
+});
+
+test('async isUsernameTaken predicates are awaited', async () => {
+  const dir = directory(['cats']);
+  const name = await deriveUniqueInviteeUsername(
+    'cats@facebook.com',
+    dir.isTakenAsync,
+  );
+  assert.strictEqual(name, 'cats1');
+});
+
+test('email is normalized before deriving (mixed case still collides)', async () => {
+  const dir = directory(['cats']);
+  const name = await deriveUniqueInviteeUsername(
+    'CATS@Facebook.com',
+    dir.isTaken,
+  );
+  assert.strictEqual(name, 'cats1');
+});
+
+test('returns null for non-email input (negative)', async () => {
+  const dir = directory([]);
+  assert.strictEqual(
+    await deriveUniqueInviteeUsername('not-an-email', dir.isTaken),
+    null,
+  );
+  assert.strictEqual(
+    await deriveUniqueInviteeUsername('@nolocal.com', dir.isTaken),
+    null,
+  );
+  assert.strictEqual(
+    await deriveUniqueInviteeUsername('', dir.isTaken),
+    null,
+  );
+});
+
+test('returns null when every candidate within maxAttempts is taken (negative)', async () => {
+  const taken = ['cats'];
+  for (let i = 1; i < 5; i += 1) taken.push(`cats${i}`);
+  const dir = directory(taken);
+  const name = await deriveUniqueInviteeUsername('cats@x.com', dir.isTaken, {
+    maxAttempts: 5,
+  });
+  assert.strictEqual(name, null);
+});
+
+test('does not probe beyond maxAttempts candidates', async () => {
+  let calls = 0;
+  const alwaysTaken = () => {
+    calls += 1;
+    return true;
+  };
+  const name = await deriveUniqueInviteeUsername('cats@x.com', alwaysTaken, {
+    maxAttempts: 3,
+  });
+  assert.strictEqual(name, null);
+  // base + suffixes 1..2 = exactly maxAttempts probes.
+  assert.strictEqual(calls, 3);
+});
+
+test('throws when isUsernameTaken is not a function (negative)', async () => {
+  await assert.rejects(
+    () => deriveUniqueInviteeUsername('cats@x.com', null),
+    TypeError,
+  );
+});
+
+// ------------------------------------------------------------------ runner
+
+(async () => {
+  for (const [name, fn] of tests) {
+    try {
+      await fn();
+      passed += 1;
+      console.log(`ok - ${name}`);
+    } catch (err) {
+      console.error(`FAIL - ${name}`);
+      console.error(err && err.stack ? err.stack : err);
+      process.exit(1);
+    }
+  }
+  console.log(`\n${passed}/${tests.length} tests passed`);
+})();