Fix/account settings

acl/drumate.json

@@ -63,6 +63,16 @@
         }
       },
       "errors": [
+        {
+          "code": "USERNAME_INVALID",
+          "message": "Username must be 2-80 letters, digits, dot, dash or underscore, starting with a letter or digit",
+          "http_status": 200
+        },
+        {
+          "code": "USERNAME_TAKEN",
+          "message": "Username is already used in this domain",
+          "http_status": 200
+        },
         {
           "code": "WRONG_PASSWORD",
           "message": "OTP code is incorrect",
@@ -696,8 +706,8 @@
         },
         "id": {
           "type": "string",
-          "required": true,
-          "doc": "User ID to update"
+          "required": false,
+          "doc": "Ignored. The caller is always the account renamed"
         }
       },
       "returns": {

service/lib/password-policy.js

@@ -0,0 +1,30 @@
+// service/lib/password-policy.js
+// The one password policy for every endpoint that sets a password: signup,
+// drumate.change_password and drumate.set_initial_password.
+//
+// KEEP IN STEP with PW_RULES in the signup UI (signup/src/widgets/form/index.js)
+// and the settings change-password modal
+// (ui-team/src/drumee/builtins/widget/settings/password-policy.js). The UIs
+// check first for the inline message; this is the authoritative check, since
+// the endpoints can be called directly. Keys double as the UIs' LOCALE keys
+// for the "Your password still needs: …" list.
+
+const PW_SPECIALS = /[\[\]\{\}\'\"\ \-\_\+\=\|\!\:\;\,\?\.\/\*\%\$\&\#\(\)\@]/;
+
+const PW_RULES = [
+  { key: "PW_NEEDS_MIN", test: (v) => v.length >= 8 },
+  { key: "PW_NEEDS_UPPERCASE", test: (v) => /[A-Z]/.test(v) },
+  { key: "PW_NEEDS_NUMBER", test: (v) => /[0-9]/.test(v) },
+  { key: "PW_NEEDS_SYMBOL", test: (v) => PW_SPECIALS.test(v) },
+];
+
+/**
+ * @param {String} password already trimmed by the caller (login trims too)
+ * @returns {String[]} keys of the unmet rules, empty when compliant
+ */
+function missingPasswordRules(password) {
+  const v = String(password == null ? "" : password);
+  return PW_RULES.filter((r) => !r.test(v)).map((r) => r.key);
+}
+
+module.exports = { PW_SPECIALS, PW_RULES, missingPasswordRules };

service/private/drumate.js

@@ -32,6 +32,7 @@
 const { Entity, Generator, MfsTools } = require("@drumee/server-core");
 const { get_node_content } = MfsTools;
 const { purge_account } = require("../lib/account-purge");
+const { missingPasswordRules } = require("../lib/password-policy");
 
 // Contextual tutorial tour ids. See tutorial_seen() below for why this list is
 // duplicated in acl/drumate.json and in ui-team's tours.js, and what a
@@ -184,7 +185,17 @@
    * @returns 
    */
   async change_password() {
-    const new_password = this.input.need(Attr.new_password);
+    // Trimmed like signup: that is the value login compares against.
+    const new_password = String(this.input.need(Attr.new_password)).trim();
+    // Same policy as signup, checked BEFORE the credential so a weak password
+    // never burns a single-use email OTP. `missing` lists the unmet rule keys
+    // (the UI's PW_NEEDS_* LOCALE keys); the error code stays
+    // uncompliant_password so older clients keep their existing message.
+    const missing = missingPasswordRules(new_password);
+    if (missing.length) {
+      this.output.data({ error: 'uncompliant_password', missing });
+      return
+    }
     // Accept EITHER credential, strictly verifying whichever was sent —
     // same contract as unlink_oauth. The FE picks by the ACCOUNT's state:
     // password-backed accounts send old_password, accounts that never set
@@ -209,25 +220,21 @@
       }
       await this.yp.await_proc('secret_clear', this.uid, 'all');
     }
-    if (!new_password.match(/(.+){8,}/)) { //(/(.+){2,} +(.+){4,}/)
-      this.output.data({ error: 'uncompliant_password' });
-    } else {
-      r = await this.yp.await_proc('set_password', this.uid, new_password);
-      // Flag the account as password-backed so step-up flows
-      // (delete_account, change_email) gate on password rather than OTP.
-      await this.yp.call_proc('drumate_update_profile', this.uid, { password_set: 1 });
-      // "Log out of other devices": drop every other session's cookie and
-      // socket; the calling session (input.sid) survives. Best-effort — a
-      // cleanup failure must not report the password change as failed.
-      if (parseInt(this.input.use('logout_others', 0))) {
-        try {
-          await this.yp.await_proc('session_logout_others', this.uid, this.input.sid());
-        } catch (e) {
-          this.warn('change_password: session_logout_others failed:', e && e.message);
-        }
+    r = await this.yp.await_proc('set_password', this.uid, new_password);
+    // Flag the account as password-backed so step-up flows
+    // (delete_account, change_email) gate on password rather than OTP.
+    await this.yp.call_proc('drumate_update_profile', this.uid, { password_set: 1 });
+    // "Log out of other devices": drop every other session's cookie and
+    // socket; the calling session (input.sid) survives. Best-effort — a
+    // cleanup failure must not report the password change as failed.
+    if (parseInt(this.input.use('logout_others', 0))) {
+      try {
+        await this.yp.await_proc('session_logout_others', this.uid, this.input.sid());
+      } catch (e) {
+        this.warn('change_password: session_logout_others failed:', e && e.message);
       }
-      this.output.data(r)
     }
+    this.output.data(r)
   }
 
   /**
@@ -318,7 +325,7 @@
         }
       })
       tips = phone.match(/(^.+)([0-9]{4,4})$/)[3];
       tips = tips;
     } else if (email) {
       const lang = this.client_language();
       const subject = DrumeeCache.message("_your_otp", lang);
@@ -400,8 +407,7 @@
   /** do_update_profile
    * 
    */
-  async do_update_profile() {
-    let profile = this.input.need(Attr.profile);
+  async do_update_profile(profile = this.input.need(Attr.profile)) {
     const profile_str = JSON.stringify(profile);
     let data = await this.yp.await_proc(
       'drumate_update_profile',
@@ -457,6 +463,14 @@
       cur_profile = {};
     }
     if (await this.check_otp_and_change()) return;
+    // A username is not just a profile field: login, the directory and
+    // avatars read the drumate.username COLUMN (plus the -u vhost built from
+    // it), which drumate_update_profile never touches. Change it first so a
+    // taken/invalid name rejects the whole save instead of half-applying it.
+    if (profile && profile.username !== undefined) {
+      const failed = await this._changeUsername(profile.username);
+      if (failed) return this.output.data(failed);
+    }
     for (let key in profile) {
       if (['otp'].includes(key)) {
         if (cur_profile.otp != null) {
@@ -468,6 +482,44 @@
     await this.do_update_profile();
   }
 
+  /**
+   * Rename the caller's account: validate, check it is free in the caller's
+   * domain, then update the username column and vhost (drumate_change_username).
+   * No-op when the name is unchanged (compared case-insensitively, like the
+   * column's collation).
+   * @param {String} username
+   * @returns {Object|null} { error } to send back, or null on success/no-op
+   */
+  async _changeUsername(username) {
+    username = String(username == null ? '' : username).trim();
+    const rows = toArray(await this.yp.await_query(
+      'SELECT username, domain_id FROM drumate WHERE id = ?', this.uid
+    ));
+    const cur = rows[0] || {};
+    if (username.toLowerCase() === String(cur.username || '').toLowerCase()) {
+      return null;
+    }
+    // 2-80 chars: letters, digits, dot, dash, underscore; must start with a
+    // letter or digit. It ends up in a hostname label (<name>-u.<domain>).
+    if (!/^[A-Za-z0-9][A-Za-z0-9._-]{1,79}$/.test(username)) {
+      return { error: 'USERNAME_INVALID' };
+    }
+    // get_user_in_domain also matches by id/email; any hit that is not the
+    // caller means the name is not available.
+    const chk = await this.yp.await_proc('get_user_in_domain', username, cur.domain_id || 1);
+    if (chk && chk.exists == 1 && chk.id !== this.uid) {
+      return { error: 'USERNAME_TAKEN' };
+    }
+    try {
+      await this.yp.await_proc('drumate_change_username', this.uid, username);
+    } catch (e) {
+      // UNIQUE (username, domain_id) lost a race with another rename.
+      this.warn('_changeUsername failed', e && e.message);
+      return { error: 'USERNAME_TAKEN' };
+    }
+    return null;
+  }
+
   /**
    * Get or create the signed-in user's referral code + link.
    * Reads the live reward DB from reward_hub_conf, calls the reward-hub
@@ -1023,9 +1075,10 @@
       return;
     }
 
-    const password = this.input.need(Attr.password);
-    if (!password.match(/(.+){8,}/)) {
-      this.output.data({ error: "uncompliant_password" });
+    const password = String(this.input.need(Attr.password)).trim();
+    const missing = missingPasswordRules(password);
+    if (missing.length) {
+      this.output.data({ error: "uncompliant_password", missing });
       return;
     }
 
@@ -1236,7 +1289,9 @@
    */
   async update_ident() {
     const ident = this.input.need(Attr.ident);
-    const id = this.input.need(Attr.id);
+    // Always the caller: this used to rename whatever `id` the client sent,
+    // letting any signed-in user rename any other account.
+    const id = this.uid;
     let chk;
     let my_org = await this.yp.await_proc('my_organisation', id)
     if (isEmpty(my_org)) {

service/signup.js

@@ -23,17 +23,9 @@ const { isEmpty } = require("lodash");
 const { notifyMemberJoined } = require("./lib/notify-member-joined");
 const { Mfs } = require("@drumee/server-core");
 
-// Password policy — KEEP IN STEP with PW_RULES in the signup UI
-// (signup/src/widgets/form/index.js). The UI checks first for the inline
-// message; this is the authoritative check, since the endpoint can be called
-// directly. Keys double as the UI's LOCALE keys for the "still needs" list.
-const PW_SPECIALS = /[\[\]\{\}\'\"\ \-\_\+\=\|\!\:\;\,\?\.\/\*\%\$\&\#\(\)\@]/;
-const PW_RULES = [
-  { key: "PW_NEEDS_MIN", test: (v) => v.length >= 8 },
-  { key: "PW_NEEDS_UPPERCASE", test: (v) => /[A-Z]/.test(v) },
-  { key: "PW_NEEDS_NUMBER", test: (v) => /[0-9]/.test(v) },
-  { key: "PW_NEEDS_SYMBOL", test: (v) => PW_SPECIALS.test(v) },
-];
+// Password policy — shared with drumate.change_password; see the lib for the
+// UI copies it must stay in step with.
+const { missingPasswordRules } = require("./lib/password-policy");
 
 class __signup extends Mfs {
 
@@ -48,7 +40,7 @@ class __signup extends Mfs {
 
     // Checked on the TRIMMED value — the one that is hashed and that login
     // (yp.login, session.signin) compares against.
-    const missing = PW_RULES.filter((r) => !r.test(password)).map((r) => r.key);
+    const missing = missingPasswordRules(password);
     if (missing.length) {
       return this.output.data({ status: "weak_password", missing });
     }

test/change-username.test.js

@@ -0,0 +1,68 @@
+const assert = require('node:assert/strict');
+const test = require('node:test');
+
+global.myDrumee = {arch: 'pod', useEmail: 0};
+global.verbosity = 0;
+global.debug = {};
+
+const DrumatePrivate = require('../service/private/drumate');
+const {_changeUsername} = DrumatePrivate.prototype;
+
+const UID = 'aaaaaaaaaaaaaaaa';
+
+// Stub `this`: the caller is `john` in domain 1. `taken` is the answer
+// get_user_in_domain gives; `renameFails` makes drumate_change_username throw.
+function worker({taken = null, renameFails = false} = {}) {
+  const calls = [];
+  return {
+    calls,
+    uid: UID,
+    warn() {},
+    yp: {
+      async await_query() {
+        return {username: 'john', domain_id: 1};
+      },
+      async await_proc(name, ...args) {
+        calls.push([name, ...args]);
+        if (name === 'get_user_in_domain') {
+          return taken ? {id: taken, exists: 1} : {id: 'ffffffffffffffff', exists: 0};
+        }
+        if (name === 'drumate_change_username' && renameFails) {
+          throw new Error('Duplicate entry');
+        }
+        return {};
+      },
+    },
+  };
+}
+
+test('an unchanged username (any case) touches nothing', async () => {
+  const w = worker();
+  assert.equal(await _changeUsername.call(w, ' John '), null);
+  assert.deepEqual(w.calls, []);
+});
+
+test('a free, valid username renames the column and vhost', async () => {
+  const w = worker();
+  assert.equal(await _changeUsername.call(w, 'john.doe'), null);
+  assert.deepEqual(w.calls.at(-1), ['drumate_change_username', UID, 'john.doe']);
+});
+
+test('a username used by someone else is rejected before any write', async () => {
+  const w = worker({taken: 'bbbbbbbbbbbbbbbb'});
+  assert.deepEqual(await _changeUsername.call(w, 'jane'), {error: 'USERNAME_TAKEN'});
+  assert.ok(!w.calls.some(([n]) => n === 'drumate_change_username'));
+});
+
+test('invalid usernames never reach the database', async () => {
+  for (const bad of ['', 'a', 'john doe', '-john', 'john@x.com', 'x'.repeat(81)]) {
+    const w = worker();
+    assert.deepEqual(await _changeUsername.call(w, bad), {error: 'USERNAME_INVALID'}, bad);
+    assert.deepEqual(w.calls, [], bad);
+  }
+});
+
+test('losing a rename race reports the name as taken', async () => {
+  const w = worker({renameFails: true});
+  assert.deepEqual(await _changeUsername.call(w, 'jane'), {error: 'USERNAME_TAKEN'});
+});

test/password-policy.test.js

@@ -0,0 +1,26 @@
+const assert = require('node:assert/strict');
+const test = require('node:test');
+
+const { missingPasswordRules } = require('../service/lib/password-policy');
+
+test('a password meeting every signup rule has nothing missing', () => {
+  assert.deepEqual(missingPasswordRules('Abcdefg1!'), []);
+});
+
+test('the old 8-character-only check no longer passes', () => {
+  assert.deepEqual(missingPasswordRules('abcdefgh'), [
+    'PW_NEEDS_UPPERCASE', 'PW_NEEDS_NUMBER', 'PW_NEEDS_SYMBOL',
+  ]);
+});
+
+test('each rule is reported on its own', () => {
+  assert.deepEqual(missingPasswordRules('Abc1!'), ['PW_NEEDS_MIN']);
+  assert.deepEqual(missingPasswordRules('abcdefg1!'), ['PW_NEEDS_UPPERCASE']);
+  assert.deepEqual(missingPasswordRules('Abcdefgh!'), ['PW_NEEDS_NUMBER']);
+  assert.deepEqual(missingPasswordRules('Abcdefgh1'), ['PW_NEEDS_SYMBOL']);
+});
+
+test('empty and missing input fail every rule instead of throwing', () => {
+  assert.equal(missingPasswordRules('').length, 4);
+  assert.equal(missingPasswordRules(undefined).length, 4);
+});