feat(chat): join users to rooms on sign-in

apps/edr-freight-api/src/modules/chat/chat-provisioning.service.ts

@@ -67,7 +67,8 @@ export class ChatProvisioningService {
     }
   }
 
-  private async currentHolders(): Promise<PositionHolder[]> {
+  /** Every current holder in the unit, or just one person's rows when `userId` is given. */
+  private async currentHolders(userId?: string): Promise<PositionHolder[]> {
     return this.dataSource.query(
       `SELECT p.key AS "positionKey",
               COALESCE(p.name->>'en', p.key) AS "positionName",
@@ -82,11 +83,53 @@ export class ChatProvisioningService {
         WHERE ep.is_current = true
           AND e.is_current = true
           AND o.key = $1
-          AND u.key = $2`,
-      [ORG_KEY, UNIT_KEY],
+          AND u.key = $2
+          ${userId ? 'AND e.user_id = $3' : ''}`,
+      userId ? [ORG_KEY, UNIT_KEY, userId] : [ORG_KEY, UNIT_KEY],
     );
   }
 
+  /**
+   * Put one person in their rooms right now.
+   *
+   * {@link reconcile} is nightly, so without this a new employee's first
+   * sign-in shows an empty client until 3AM — the SSO handoff creates their
+   * account but joins them to nothing. Called on every /chat/sso, so it is
+   * scoped to the one user (a full reconcile per click would be a room-count
+   * multiple of Matrix calls) and every step is get-or-create.
+   *
+   * Someone holding no current position in the unit joins nothing, by the same
+   * rule the reconcile uses — chat membership follows the org tree.
+   */
+  async joinUserRooms(userId: string, displayName: string): Promise<number> {
+    const positions = await this.currentHolders(userId);
+    if (positions.length === 0) return 0;
+
+    const mxid = this.matrix.mxidFor(userId, displayName);
+    // The JWT login auto-registers too, but that happens after this runs and
+    // the admin join API 404s on an account that does not exist yet.
+    await this.matrix.ensureUser(mxid, displayName);
+
+    const spaceId = await this.matrix.ensureRoom(SPACE_ALIAS, 'EDR Freight', {
+      isSpace: true,
+    });
+    const generalRoomId = await this.matrix.ensureRoom(GENERAL_ALIAS, 'General', {
+      parentSpaceId: spaceId,
+    });
+    await this.matrix.ensureJoined(generalRoomId, mxid);
+
+    for (const position of positions) {
+      const roomId = await this.matrix.ensureRoom(
+        `dept-${position.positionKey}`,
+        position.positionName,
+        { parentSpaceId: spaceId },
+      );
+      await this.matrix.ensureJoined(roomId, mxid);
+    }
+
+    return positions.length + 1;
+  }
+
   /** Force-joins additions, kicks+deactivates users no longer entitled anywhere. */
   private async syncMembership(
     roomId: string,
@@ -99,7 +142,7 @@ export class ChatProvisioningService {
     let joined = 0;
     for (const userId of desiredUserIds) {
       if (!currentSet.has(userId)) {
-        await this.matrix.forceJoin(roomId, userId);
+        await this.matrix.ensureJoined(roomId, userId);
         joined += 1;
       }
     }
@@ -126,14 +169,16 @@ export class ChatProvisioningService {
       parentSpaceId: spaceId,
     });
 
-    const allUserIds = new Set(holders.map((h) => this.matrix.mxid(h.userId)));
+    const allUserIds = new Set(
+      holders.map((h) => this.matrix.mxidFor(h.userId, h.userName)),
+    );
 
     // Accounts are otherwise only created lazily on first JWT login (see
     // ChatSsoService) — force-joining someone who has never clicked "Chat"
     // yet 404s ("User not found") without this.
     const seenUserIds = new Set<string>();
     for (const h of holders) {
-      const mxid = this.matrix.mxid(h.userId);
+      const mxid = this.matrix.mxidFor(h.userId, h.userName);
       if (seenUserIds.has(mxid)) continue;
       seenUserIds.add(mxid);
       await this.matrix.ensureUser(mxid, h.userName);
@@ -158,7 +203,7 @@ export class ChatProvisioningService {
         name: h.positionName,
         userIds: new Set<string>(),
       };
-      entry.userIds.add(this.matrix.mxid(h.userId));
+      entry.userIds.add(this.matrix.mxidFor(h.userId, h.userName));
       byPosition.set(h.positionKey, entry);
     }
 

apps/edr-freight-api/src/modules/chat/chat-sso.service.ts

@@ -1,12 +1,13 @@
-import { Inject, Injectable } from '@nestjs/common';
+import { Inject, Injectable, Logger } from '@nestjs/common';
 import type { ConfigType } from '@nestjs/config';
 import type { TCurrentUser } from '@tria-plc/api-common/modules/auth/types/current-user.type';
 import { SignJWT } from 'jose';
 
 import chatConfig from '../../config/chat.config';
-import { MatrixClient } from './matrix.client';
+import { ChatProvisioningService } from './chat-provisioning.service';
+import { MatrixClient, chatLocalpart } from './matrix.client';
 
-/** Matrix login_tokens are single-use and expire in 5 minutes (Synapse default). */
+/** Long enough for one login call, short enough to be worthless if it leaks. */
 const JWT_TTL_SECONDS = 60;
 
 function displayName(user: TCurrentUser): string {
@@ -24,36 +25,67 @@ function displayName(user: TCurrentUser): string {
  *
  *   1. Sign a short-lived JWT asserting this user's id (Synapse's
  *      org.matrix.login.jwt auto-registers the account on first use).
- *   2. Trade that JWT for a real Matrix access token.
- *   3. Trade the access token for a one-shot login_token.
- *   4. Hand the caller a link to Element's sso.html shim, which seeds
- *      localStorage and forwards the token into Element's own login flow.
+ *   2. Trade that JWT for a real Matrix session.
+ *   3. Hand the caller a link to Element's sso.html shim, which writes that
+ *      session into localStorage and drops the user straight into Element.
+ *
+ * Step 3 used to mint a one-shot login_token and let Element redeem it. That
+ * path is capped at one request per user per minute by a limiter hardcoded in
+ * Synapse, so a second click inside a minute returned M_LIMIT_EXCEEDED — and a
+ * spent token surfaces in Element as "Incorrect username and/or password".
+ * Element accepts a plaintext token out of localStorage (Lifecycle.ts
+ * getStoredToken/tryDecryptToken), so handing over the session we already hold
+ * removes both failure modes and one round-trip.
  */
 @Injectable()
 export class ChatSsoService {
+  private readonly logger = new Logger(ChatSsoService.name);
+
   constructor(
     @Inject(chatConfig.KEY)
     private readonly config: ConfigType<typeof chatConfig>,
     private readonly matrix: MatrixClient,
+    private readonly provisioning: ChatProvisioningService,
   ) {}
 
   async getSsoUrl(user: TCurrentUser): Promise<{ url: string }> {
     const secret = new TextEncoder().encode(this.config.jwtSecret);
-    const jwt = await new SignJWT({ name: displayName(user) })
+    const name = displayName(user);
+
+    // Before the link, not after: the reconcile that fills rooms is nightly, so
+    // a first sign-in would otherwise open an empty client. Best-effort —
+    // failing to join a room is no reason to refuse someone a sign-in link.
+    try {
+      await this.provisioning.joinUserRooms(user.id, name);
+    } catch (err) {
+      this.logger.error(
+        `Room join on sign-in failed for ${user.id}: ${(err as Error).message}`,
+      );
+    }
+    // Synapse takes the localpart straight from `sub` on auto-registration, so
+    // this must be byte-identical to what ChatProvisioningService derives for
+    // the same person — otherwise SSO signs them into one account while the
+    // reconcile force-joins a different one into the rooms.
+    const jwt = await new SignJWT({ name })
       .setProtectedHeader({ alg: 'HS256' })
-      .setSubject(user.id)
+      .setSubject(chatLocalpart(user.id, name))
       .setIssuer('edr-freight-api')
       .setAudience('matrix')
       .setIssuedAt()
       .setExpirationTime(`${JWT_TTL_SECONDS}s`)
       .sign(secret);
 
-    const { access_token } = await this.matrix.loginWithJwt(jwt);
-    const { login_token } = await this.matrix.getLoginToken(access_token);
+    const session = await this.matrix.loginWithJwt(jwt);
 
-    const url = new URL(`${this.config.webUrl}/sso.html`);
-    url.searchParams.set('t', login_token);
-    url.searchParams.set('hs', this.config.publicBaseUrl);
-    return { url: url.toString() };
+    // Session goes in the URL fragment, never the query: a fragment is not sent
+    // to any server, so the token stays out of Element's access log, and
+    // sso.html replaces the entry so it does not linger in history either.
+    const params = new URLSearchParams({
+      hs: this.config.publicBaseUrl,
+      t: session.access_token,
+      u: session.user_id,
+      d: session.device_id,
+    });
+    return { url: `${this.config.webUrl}/sso.html#${params.toString()}` };
   }
 }

apps/edr-freight-api/src/modules/chat/matrix.client.spec.ts

@@ -0,0 +1,35 @@
+import { chatLocalpart } from './matrix.client';
+
+describe('chatLocalpart', () => {
+  it('reads from the name, not the id', () => {
+    expect(
+      chatLocalpart('03f5eb9e-23a0-4413-8d98-8de4b98b1be2', 'Nati Wondi'),
+    ).toBe('nati-wondi.03f5eb');
+  });
+
+  it('separates two people who share a name', () => {
+    // Both of these are real dev rows — same name, different employees.
+    const a = chatLocalpart('11111111-1111-4111-8111-111111111111', 'MARKOS REGASA');
+    const b = chatLocalpart('22222222-2222-4222-8222-222222222222', 'Markos  REGASA');
+    expect(a).not.toBe(b);
+  });
+
+  it('is stable for the same person', () => {
+    const id = '7d798218-09de-47a1-98eb-f61ec44e9280';
+    expect(chatLocalpart(id, 'Naod')).toBe(chatLocalpart(id, 'Naod'));
+  });
+
+  it('still yields a usable localpart for a name that slugs to nothing', () => {
+    expect(chatLocalpart('7d798218-09de-47a1-98eb-f61ec44e9280', 'ናኦድ')).toBe(
+      'user.7d7982',
+    );
+  });
+
+  it('only emits characters Matrix accepts in a localpart', () => {
+    for (const name of ['Mubarek Jemal Hassen', "N'gozi  O_Brien", 'ናኦድ', 'José']) {
+      expect(chatLocalpart('7d798218-09de-47a1-98eb-f61ec44e9280', name)).toMatch(
+        /^[a-z0-9._=\-/]+$/,
+      );
+    }
+  });
+});

apps/edr-freight-api/src/modules/chat/matrix.client.ts

@@ -14,6 +14,32 @@ import chatConfig from '../../config/chat.config';
  * ChatBridgeService) — one bot/admin account covers both jobs, no separate
  * bot user needed.
  */
+/**
+ * Localpart of a staff member's MXID: their name, plus the first 6 hex of
+ * their freight user id.
+ *
+ * The tail is not decoration. Names collide — 19 of the 114 users in the dev
+ * IAM share a slug with someone else ("MARKOS REGASA" and "Markos  REGASA" are
+ * two different people) — and an MXID is permanent, so a bare slug would hand
+ * two employees the same Matrix account and each other's rooms. The id is
+ * already random, so 6 hex of it separates them without a lookup or a mapping
+ * table, and keeps the derivation pure: ChatSsoService (which mints the JWT
+ * `sub`) and ChatProvisioningService (which force-joins rooms) must agree on
+ * this string exactly or they provision two accounts per person.
+ */
+export function chatLocalpart(userId: string, displayName: string): string {
+  const slug = displayName
+    // NFKD splits an accent off its letter; the non-alnum sweep below then
+    // folds the leftover mark into the same `-` run as the neighbouring space.
+    .normalize('NFKD')
+    .toLowerCase()
+    .replace(/[^a-z0-9]+/g, '-')
+    .replace(/^-+|-+$/g, '')
+    .slice(0, 40);
+  // Amharic-only names slug to nothing — the tail still makes it unique.
+  return `${slug || 'user'}.${userId.replace(/-/g, '').slice(0, 6)}`;
+}
+
 @Injectable()
 export class MatrixClient {
   constructor(
@@ -21,11 +47,28 @@ export class MatrixClient {
     private readonly config: ConfigType<typeof chatConfig>,
   ) {}
 
+  /**
+   * Alias localparts go in a URL path segment, so a `/` in one is fatal:
+   * Synapse decodes the path before routing, and `%2F` splits the request into
+   * a route that doesn't exist ("M_UNRECOGNIZED"). resolveAlias reads that 404
+   * as "no such room" and ensureRoom then tries to create the same broken alias
+   * on every run. Position keys are `edr_freight_app/opn` shaped, so this hits
+   * every dept room but the handful whose key happens to be a bare word.
+   */
+  private static aliasSafe(alias: string): string {
+    return alias.replace(/[^A-Za-z0-9._=-]/g, '-');
+  }
+
   /** `@<localpart>:<server_name>` — the one place this format is assembled. */
   mxid(localpart: string): string {
     return `@${localpart}:${this.config.serverName}`;
   }
 
+  /** The MXID of a freight user — see {@link chatLocalpart}. */
+  mxidFor(userId: string, displayName: string): string {
+    return this.mxid(chatLocalpart(userId, displayName));
+  }
+
   get serverName(): string {
     return this.config.serverName;
   }
@@ -123,15 +166,13 @@ export class MatrixClient {
     return Object.keys(res.joined);
   }
 
-  /** Exchange a fresh access token for a one-shot login_token (5 min TTL). */
-  getLoginToken(accessToken: string): Promise<{ login_token: string }> {
-    return this.request(
-      'POST',
-      '/_matrix/client/v1/login/get_token',
-      {},
-      accessToken,
-    );
-  }
+  // No getLoginToken here on purpose. POST /_matrix/client/v1/login/get_token
+  // is rate limited to 1 request per user per MINUTE, hardcoded in Synapse
+  // (rest/client/login_token_request.py: "Ratelimit aggressively … could be
+  // abused by a malicious client to create many sessions") and not settable
+  // from homeserver.yaml. A second click inside a minute got M_LIMIT_EXCEEDED.
+  // ChatSsoService hands Element the session from loginWithJwt directly
+  // instead, which needs no second call.
 
   /** null when the alias doesn't resolve to a room yet. */
   resolveAlias(alias: string): Promise<{ room_id: string } | null> {
@@ -180,10 +221,11 @@ export class MatrixClient {
    * on every reconcile run and every bridged notification alike.
    */
   async ensureRoom(
-    alias: string,
+    rawAlias: string,
     name: string,
     opts: { isSpace?: boolean; parentSpaceId?: string } = {},
   ): Promise<string> {
+    const alias = MatrixClient.aliasSafe(rawAlias);
     const existing = await this.resolveAlias(`#${alias}:${this.config.serverName}`);
     if (existing) return existing.room_id;
 
@@ -228,6 +270,21 @@ export class MatrixClient {
     );
   }
 
+  /**
+   * Force-join, treating "already a member" as success. Synapse answers a
+   * repeat join with 403 `M_FORBIDDEN: "<user> is already in the room."`, which
+   * is a failure only if you assumed you knew the membership first. Callers
+   * that just want someone in a room (sign-in, reconcile racing itself) want
+   * this; the raw 403 tells them nothing they can act on.
+   */
+  async ensureJoined(roomIdOrAlias: string, userId: string): Promise<void> {
+    try {
+      await this.forceJoin(roomIdOrAlias, userId);
+    } catch (err) {
+      if (!/already in the room/i.test((err as Error).message)) throw err;
+    }
+  }
+
   kick(roomId: string, userId: string, reason: string): Promise<void> {
     return this.request(
       'POST',