From 238d2839a8c6d141109528883d41a937e944f298 Mon Sep 17 00:00:00 2001 From: Noley Holland Date: Thu, 21 May 2026 14:28:44 -0700 Subject: [PATCH 1/5] Push live team-state and membership updates to members via MQTT/WS --- forge/comms/aclManager.js | 42 +++ forge/comms/index.js | 12 + forge/comms/v2AuthRoutes.js | 2 + forge/db/controllers/BrokerClient.js | 27 +- forge/ee/routes/billing/index.js | 6 + forge/routes/api/team.js | 59 ++++ forge/routes/api/teamMembers.js | 4 + frontend/src/api/team.js | 6 + frontend/src/services/service.registry.ts | 4 +- frontend/src/services/team-channel.service.ts | 197 ++++++++++++ frontend/src/stores/account-auth.js | 6 +- frontend/src/stores/account.js | 14 + frontend/src/types/services/index.ts | 1 + frontend/src/types/services/service.types.ts | 3 +- .../src/types/services/team-channel.types.ts | 13 + test/unit/forge/comms/authRoutesV2_spec.js | 74 +++++ .../services/services.orchestrator.spec.js | 3 +- .../services/team-channel.service.spec.js | 284 ++++++++++++++++++ .../unit/frontend/stores/account-auth.spec.js | 38 +++ 19 files changed, 790 insertions(+), 5 deletions(-) create mode 100644 frontend/src/services/team-channel.service.ts create mode 100644 frontend/src/types/services/team-channel.types.ts create mode 100644 test/unit/frontend/services/team-channel.service.spec.js diff --git a/forge/comms/aclManager.js b/forge/comms/aclManager.js index bf2ad49b1f..05cedec15e 100644 --- a/forge/comms/aclManager.js +++ b/forge/comms/aclManager.js @@ -115,6 +115,30 @@ module.exports = function (app) { return false } }, + checkUserIsTeamMember: async function (requestParts, usernameParts) { + // requestParts = [ fullTopic , [, ] ] + // usernameParts = [ 'team-frontend', , , ] + const topicTeamHash = requestParts[1] + const usernameUserHash = usernameParts[1] + const usernameTeamHash = usernameParts[2] + if (topicTeamHash !== usernameTeamHash) { + return false + } + // membership topic: user capture must match the credential's user + if (requestParts[2] !== undefined && requestParts[2] !== usernameUserHash) { + return false + } + try { + const team = await app.db.models.Team.byId(usernameTeamHash) + if (!team) return false + const user = await app.db.models.User.byId(usernameUserHash) + if (!user) return false + const membership = await app.db.models.TeamMember.getTeamMembership(user.id, team.id, false) + return !!membership + } catch (err) { + return false + } + }, checkExpertTopic: async function (topicParts, usernameParts, acl) { // topicParts = [ fullTopic , , , , [, ] ] // usernameParts = [ 'expert-client' | 'expert-agent', [, ] ] @@ -290,6 +314,11 @@ module.exports = function (app) { // Send commands to all application-assigned devices // - ff/v1/+/a/+/command { topic: /^ff\/v1\/[^/]+\/a\/[^/]+\/command$/ }, + // Team channel broadcasts to subscribed team members + // - ff/v1//team/updated + { topic: /^ff\/v1\/[^/]+\/team\/updated$/ }, + // - ff/v1//u//membership + { topic: /^ff\/v1\/[^/]+\/u\/[^/]+\/membership$/ }, // ff/v1/platform/sync { topic: /^ff\/v1\/platform\/sync$/ }, // ff/v1/platform/leader @@ -347,6 +376,16 @@ module.exports = function (app) { { topic: /^ff\/v1\/([^/]+)\/d\/([^/]+)\/resources\/heartbeat$/, verify: 'checkDeviceIsAssigned' } ] }, + // browser-side team channel (per-tab, per-team) + teamFrontend: { + sub: [ + // - ff/v1//team/updated + { topic: /^ff\/v1\/([^/]+)\/team\/updated$/, verify: 'checkUserIsTeamMember' }, + // - ff/v1//u//membership + { topic: /^ff\/v1\/([^/]+)\/u\/([^/]+)\/membership$/, verify: 'checkUserIsTeamMember' } + ], + pub: [] + }, // frontend client (user) expertClient: { sub: [ @@ -384,6 +423,7 @@ module.exports = function (app) { // - project:: // - device:: // - frontend:: + // - team-frontend::: // - expert-client:: // - expert-agent:: @@ -399,6 +439,8 @@ module.exports = function (app) { aclList = ACLS.project[aclType] } else if (/^device:/.test(username)) { aclList = ACLS.device[aclType] + } else if (/^team-frontend:/.test(username)) { + aclList = ACLS.teamFrontend[aclType] } else if (/^frontend:/.test(username)) { aclList = ACLS.frontend[aclType] } else if (/^expert-agent:/.test(username)) { diff --git a/forge/comms/index.js b/forge/comms/index.js index 1bb6a6fdac..4cb7bf79c5 100644 --- a/forge/comms/index.js +++ b/forge/comms/index.js @@ -60,6 +60,18 @@ module.exports = fp(async function (app, _opts) { client.publish('ff/v1/platform/leader', JSON.stringify(msg)) } } + }, + team: { + notify: function (teamHash, reason, srcId) { + if (!teamHash) return + const msg = { reason: reason || null, srcId: srcId || null } + client.publish(`ff/v1/${teamHash}/team/updated`, JSON.stringify(msg)) + }, + notifyMembership: function (teamHash, userHash, reason, srcId) { + if (!teamHash || !userHash) return + const msg = { reason: reason || null, srcId: srcId || null } + client.publish(`ff/v1/${teamHash}/u/${userHash}/membership`, JSON.stringify(msg)) + } } }) diff --git a/forge/comms/v2AuthRoutes.js b/forge/comms/v2AuthRoutes.js index 9387b4af25..89b244c949 100644 --- a/forge/comms/v2AuthRoutes.js +++ b/forge/comms/v2AuthRoutes.js @@ -34,6 +34,7 @@ module.exports = async function (app) { if ((username.startsWith('device:') && password.startsWith('ffbd_')) || (username.startsWith('project:') && password.startsWith('ffbp_')) || (username.startsWith('frontend:') && password.startsWith('ffbf_')) || + (username.startsWith('team-frontend:') && password.startsWith('ffbtf_')) || (username.startsWith('expert-agent:') && password.startsWith('ffbea_')) || (username.startsWith('expert-client:') && password.startsWith('ffbec_')) || (username === 'forge_platform')) { @@ -138,6 +139,7 @@ module.exports = async function (app) { if ((username.startsWith('device:') || username.startsWith('project:') || username.startsWith('frontend:') || + username.startsWith('team-frontend:') || username.startsWith('expert-agent:') || username.startsWith('expert-client:') || username === 'forge_platform') && !username.includes('@')) { diff --git a/forge/db/controllers/BrokerClient.js b/forge/db/controllers/BrokerClient.js index 4af37e2de5..b56bfd6cbd 100644 --- a/forge/db/controllers/BrokerClient.js +++ b/forge/db/controllers/BrokerClient.js @@ -10,7 +10,7 @@ module.exports = { attributes: ['username', 'password'] }) if (compareHash(password || '', user ? user.password : '')) { - if (username.startsWith('frontend:') || username.startsWith('expert-client:')) { + if (username.startsWith('frontend:') || username.startsWith('expert-client:') || username.startsWith('team-frontend:')) { await user.destroy() } return true @@ -182,6 +182,31 @@ module.exports = { return null }, + createClientForTeamFrontend: async function (app, user, team, sessionId) { + if (app.comms) { + const username = `team-frontend:${user.hashid}:${team.hashid}:${sessionId}` + const existingClient = await app.db.models.BrokerClient.findOne({ + where: { username } + }) + if (existingClient) { + await existingClient.destroy() + } + const password = generateToken(32, 'ffbtf') + await app.db.models.BrokerClient.create({ + username, + password, + ownerId: '' + user.id, + ownerType: 'team-frontend' + }) + return { + url: app.config.broker.public_url || app.config.broker.url || null, + username, + password + } + } + return null + }, + createClientForExpertClient: async function (app, user, sessionId) { if (app.comms) { const existingClient = await app.db.models.BrokerClient.findOne({ diff --git a/forge/ee/routes/billing/index.js b/forge/ee/routes/billing/index.js index da60e0b5bd..ff77abb813 100644 --- a/forge/ee/routes/billing/index.js +++ b/forge/ee/routes/billing/index.js @@ -268,6 +268,9 @@ module.exports = async function (app) { } else { app.log.warn(`Stripe subscription ${stripeSubscriptionId} has transitioned in Stripe to a state not currently handled: '${stripeSubscriptionStatus}'`) } + if (team) { + app.comms?.team?.notify(team.hashid, 'billing-updated') + } break } @@ -286,6 +289,7 @@ module.exports = async function (app) { response.status(200).send() return } + app.comms?.team?.notify(team.hashid, 'billing-deleted') // Suspend all projects of that team const projects = await app.db.models.Project.byTeam(team.hashid) @@ -444,6 +448,7 @@ module.exports = async function (app) { await team.save() } } + app.comms?.team?.notify(team.hashid, 'billing-manual-enabled') response.code(200).send({}) } catch (err) { // Standard errors @@ -472,6 +477,7 @@ module.exports = async function (app) { const team = request.team try { await app.billing.disableManualBilling(team) + app.comms?.team?.notify(team.hashid, 'billing-manual-disabled') response.code(200).send({}) } catch (err) { // Standard errors diff --git a/forge/routes/api/team.js b/forge/routes/api/team.js index dad4969932..703793a3c0 100644 --- a/forge/routes/api/team.js +++ b/forge/routes/api/team.js @@ -841,6 +841,7 @@ module.exports = async function (app) { } teamAuditFunc(request.session.User, null, request.team) platformAuditFunc(request.session.User, null, request.team) + app.comms?.team?.notify(request.team.hashid, request.body.suspended ? 'suspended' : 'unsuspended') reply.send(app.db.views.Team.team(request.team)) return } catch (err) { @@ -894,6 +895,7 @@ module.exports = async function (app) { // Only log if something changes if (updates.length > 0) { auditLogFunc(request.session.User, null, request.team, updates) + app.comms?.team?.notify(request.team.hashid, 'updated') } reply.send(app.db.views.Team.team(request.team)) } catch (err) { @@ -914,6 +916,63 @@ module.exports = async function (app) { } }) + /** + * Issue MQTT/WS credentials for the team-level browser channel. + * @name /api/v1/teams/:teamId/comms-credentials + */ + app.post('/:teamId/comms-credentials', { + preHandler: app.needsPermission('team:read'), + schema: { + summary: 'Issue team-channel broker credentials for the current user/session', + tags: ['Teams'], + params: { + type: 'object', + properties: { + teamId: { type: 'string' } + } + }, + body: { + type: 'object', + properties: { + sessionId: { type: 'string' } + } + }, + response: { + 200: { + type: 'object', + properties: { + url: { type: 'string' }, + username: { type: 'string' }, + password: { type: 'string' } + } + }, + '4xx': { + $ref: 'APIError' + } + } + } + }, async (request, reply) => { + if (!app.comms) { + reply.code(503).send({ code: 'comms_unavailable', error: 'Broker not configured' }) + return + } + const sessionId = request.body?.sessionId + if (!sessionId || typeof sessionId !== 'string' || sessionId.length < 8) { + reply.code(400).send({ code: 'invalid_request', error: 'sessionId is required' }) + return + } + const creds = await app.db.controllers.BrokerClient.createClientForTeamFrontend( + request.session.User, + request.team, + sessionId + ) + if (!creds) { + reply.code(503).send({ code: 'comms_unavailable', error: 'Broker not configured' }) + return + } + reply.send(creds) + }) + /** * Get the session users team membership * @name /api/v1/team/:teamId/user diff --git a/forge/routes/api/teamMembers.js b/forge/routes/api/teamMembers.js index 43b70687bf..147a20f6f0 100644 --- a/forge/routes/api/teamMembers.js +++ b/forge/routes/api/teamMembers.js @@ -117,6 +117,9 @@ module.exports = async function (app) { // the needsPermission handler will have ensured the requesting user is allowed // to make this request. All we have to do try { + // Cue the targeted user's session to redirect before we delete the + // membership row (so their next API call doesn't 404 them mid-action). + app.comms?.team?.notifyMembership(request.team.hashid, request.user.hashid, 'removed') const result = await app.db.controllers.Team.removeUser(request.team, request.user, request.userRole) if (result) { await app.auditLog.Team.team.user.removed(request.session.User, null, request.team, request.user) @@ -188,6 +191,7 @@ module.exports = async function (app) { // might want to make this only if it drop under Member await app.db.controllers.StorageSession.removeUserFromTeamSessions(request.user, request.team) } + app.comms?.team?.notifyMembership(request.team.hashid, request.user.hashid, 'role-changed') } reply.send({ status: 'okay' }) } catch (err) { diff --git a/frontend/src/api/team.js b/frontend/src/api/team.js index 362d649a83..686a6f9767 100644 --- a/frontend/src/api/team.js +++ b/frontend/src/api/team.js @@ -320,6 +320,11 @@ const getTeamAuditLog = async (teamId, params, cursor, limit) => { const url = paginateUrl(`/api/v1/teams/${teamId}/audit-log`, cursor, limit) return client.get(url, { params }).then(res => res.data) } +const getTeamCommsCreds = (teamId, sessionId) => { + return client.post(`/api/v1/teams/${teamId}/comms-credentials`, { sessionId }) + .then(res => res.data) +} + const getTeamUserMembership = (teamId) => { return client.get(`/api/v1/teams/${teamId}/user`).then(res => res.data) } @@ -564,6 +569,7 @@ export default { resendTeamInvitation, getTeamAuditLog, getTeamUserMembership, + getTeamCommsCreds, getTeamDevices, getTeamRegistry, generateRegistryUserToken, diff --git a/frontend/src/services/service.registry.ts b/frontend/src/services/service.registry.ts index b49a5bb8b5..d1c4aa7ea8 100644 --- a/frontend/src/services/service.registry.ts +++ b/frontend/src/services/service.registry.ts @@ -1,9 +1,11 @@ import { createBootstrapService } from './bootstrap.service' import { createMqttService } from './mqtt.service' import { createMessagingService } from './post-message.service' +import { createTeamChannelService } from './team-channel.service' export default [ { key: 'bootstrap' as const, create: createBootstrapService, requiredLifecycle: ['init', 'destroy'] as const }, { key: 'postMessage' as const, create: createMessagingService, requiredLifecycle: ['destroy'] as const }, - { key: 'mqtt' as const, create: createMqttService, requiredLifecycle: ['destroy'] as const } + { key: 'mqtt' as const, create: createMqttService, requiredLifecycle: ['destroy'] as const }, + { key: 'teamChannel' as const, create: createTeamChannelService, requiredLifecycle: ['destroy'] as const } ] diff --git a/frontend/src/services/team-channel.service.ts b/frontend/src/services/team-channel.service.ts new file mode 100644 index 0000000000..3a0cee82fc --- /dev/null +++ b/frontend/src/services/team-channel.service.ts @@ -0,0 +1,197 @@ +import { v4 as uuidv4 } from 'uuid' + +import { BaseService } from './service.contract' + +import teamApi from '@/api/team.js' +import { useAccountAuthStore } from '@/stores/account-auth.js' +import { useContextStore } from '@/stores/context.js' +import { Maybe } from '@/types/common/types' +import type { CreateServiceOptions } from '@/types/services/service.types' +import type { TeamChannelServiceI, TeamRef } from '@/types/services/team-channel.types' + +const SESSION_KEY = 'ff-team-channel-session-id' +const MEMBERSHIP_TOPIC_REGEX = /^ff\/v1\/[^/]+\/u\/([^/]+)\/membership$/ + +function connectionKey (teamId: string): string { + return `team:${teamId}` +} + +class TeamChannelService extends BaseService implements TeamChannelServiceI { + protected $sessionId: Maybe = null + protected $connectedTeamId: Maybe = null + + constructor ({ app, router, services }: CreateServiceOptions) { + super({ + name: 'teamChannel', + app, + router, + services + }) + } + + getSessionId (): string { + if (this.$sessionId) return this.$sessionId + try { + let sessionId = sessionStorage.getItem(SESSION_KEY) + if (!sessionId) { + sessionId = uuidv4() + sessionStorage.setItem(SESSION_KEY, sessionId) + } + this.$sessionId = sessionId + } catch { + this.$sessionId = uuidv4() + } + return this.$sessionId as string + } + + isConnected (): boolean { + return this.$connectedTeamId !== null + } + + async connect (team: Maybe): Promise { + if (!team?.id || typeof team.id !== 'string' || team.id.length === 0) return + const authStore = useAccountAuthStore() + const userId = authStore.user?.id + if (!userId) return + if (this.$connectedTeamId === team.id) return + + await this.disconnect() + + const mqtt = this.$services?.mqtt + if (!mqtt) return + + const teamId = team.id + const sessionId = this.getSessionId() + const key = connectionKey(teamId) + + try { + await mqtt.createClient(key, { + getCredentials: () => teamApi.getTeamCommsCreds(teamId, sessionId), + onMessage: (topic: string, message: Buffer) => this._onMqttMessage(topic, message), + onConnect: () => this._onMqttConnect(teamId, userId), + onClose: () => this._onMqttClose(), + onOffline: () => this._onMqttOffline(), + onError: (err: Error) => this._onMqttError(err), + onDisconnect: () => this._onMqttDisconnect() + }) + this.$connectedTeamId = teamId + } catch { + // graceful degrade — no broker, bad creds, or network + this.$connectedTeamId = null + } + } + + async disconnect (): Promise { + if (!this.$connectedTeamId) return + const mqtt = this.$services?.mqtt + const key = connectionKey(this.$connectedTeamId) + this.$connectedTeamId = null + if (!mqtt) return + try { + await mqtt.destroyClient(key) + } catch { + // ignore teardown failures + } + } + + async destroy (): Promise { + await this.disconnect() + this.$sessionId = null + } + + protected async _onMqttConnect (teamId: string, userId: string): Promise { + const mqtt = this.$services?.mqtt + if (!mqtt) return + try { + await mqtt.subscribe(connectionKey(teamId), [ + `ff/v1/${teamId}/team/updated`, + `ff/v1/${teamId}/u/${userId}/membership` + ], { qos: 1 }) + } catch (err) { + // non-fatal — mqtt.service replays subscriptions on reconnect + this._onMqttError(err instanceof Error ? err : new Error(String(err))) + } + } + + protected _onMqttClose (): void { + // no-op — reconnect handled by mqtt.service + } + + protected _onMqttOffline (): void { + // no-op — reconnect handled by mqtt.service + } + + protected _onMqttDisconnect (): void { + // no-op — reconnect handled by mqtt.service + } + + protected _onMqttError (err: Error): undefined { + // no-op — terminal failures handled by connect()'s catch; transients retried by mqtt.service + return err && undefined + } + + protected _onMqttMessage (topic: string, message: Buffer | Uint8Array | string): void { + let payload: { reason?: string } = {} + try { + const raw = typeof message === 'string' + ? message + : (message?.toString ? message.toString() : '') + payload = raw ? JSON.parse(raw) : {} + } catch { + payload = {} + } + + const membershipMatch = MEMBERSHIP_TOPIC_REGEX.exec(topic) + if (membershipMatch) { + this._handleMembership(payload) + return + } + if (topic.endsWith('/team/updated')) { + this._handleTeamUpdated() + } + } + + protected _handleMembership (payload: { reason?: string }): void { + if (payload?.reason === 'removed') { + // Only redirect if the user is on a team-scoped route — leave them + // alone if they're already on /account or another non-team page. + const path = this.$router?.currentRoute?.value?.path + if (typeof path === 'string' && path.startsWith('/team/')) { + try { + this.$router?.push({ name: 'Home' }) + } catch {} + } + return + } + try { + useContextStore().refreshTeamMembership().catch(() => undefined) + } catch {} + } + + protected _handleTeamUpdated (): void { + try { + useContextStore().refreshTeam().catch(() => undefined) + } catch {} + } +} + +let TeamChannelServiceInstance: Maybe = null + +export function createTeamChannelService ({ app, router, services }: CreateServiceOptions): TeamChannelService { + if (!TeamChannelServiceInstance) { + TeamChannelServiceInstance = new TeamChannelService({ + app, + router, + services + }) + } + return TeamChannelServiceInstance +} + +export async function destroyTeamChannelService (): Promise { + if (!TeamChannelServiceInstance) return + await TeamChannelServiceInstance.destroy() + TeamChannelServiceInstance = null +} + +export default createTeamChannelService diff --git a/frontend/src/stores/account-auth.js b/frontend/src/stores/account-auth.js index cc60a600d7..6f62353ccd 100644 --- a/frontend/src/stores/account-auth.js +++ b/frontend/src/stores/account-auth.js @@ -5,6 +5,7 @@ import settingsApi from '../api/settings.js' import teamApi from '../api/team.js' import userApi from '../api/user.js' +import getServicesOrchestrator from '@/services/service.orchestrator' import { useAccountSettingsStore } from '@/stores/account-settings.js' import { useAccountStore } from '@/stores/account.js' import { useContextStore } from '@/stores/context.js' @@ -192,7 +193,10 @@ export const useAccountAuthStore = defineStore('account-auth', { if (useAccountSettingsStore().settings['platform:sso:only']) { logoutURL = useAccountSettingsStore().settings['platform:sso:only:logoutURL'] || '/' } - return userApi.logout() + const teamChannel = getServicesOrchestrator().$serviceInstances.teamChannel + const disconnect = teamChannel ? teamChannel.disconnect().catch(() => {}) : Promise.resolve() + return disconnect + .then(() => userApi.logout()) .then(() => { useAccountAuthStore().$reset() useAccountStore().$reset() diff --git a/frontend/src/stores/account.js b/frontend/src/stores/account.js index f55ec45eee..e3c4097665 100644 --- a/frontend/src/stores/account.js +++ b/frontend/src/stores/account.js @@ -4,10 +4,17 @@ import flowBlueprintsApi from '@/api/flowBlueprints.js' import teamApi from '@/api/team.js' import userApi from '@/api/user.js' import product from '@/services/product.js' +import getServicesOrchestrator from '@/services/service.orchestrator' import { useAccountAuthStore } from '@/stores/account-auth.js' import { useContextStore } from '@/stores/context.js' import { useProductTablesStore } from '@/stores/product-tables.js' +function ensureTeamChannelConnected (team) { + if (!team?.id) return + const teamChannel = getServicesOrchestrator().$serviceInstances.teamChannel + teamChannel?.connect(team).catch(() => {}) +} + export const useAccountStore = defineStore('account', { state: () => ({ teams: [], @@ -56,6 +63,7 @@ export const useAccountStore = defineStore('account', { if (!currentTeam || currentTeam.slug !== team) { team = await teamApi.getTeam({ slug: team }) } else { + ensureTeamChannelConnected(currentTeam) this.pendingTeamChange = false return } @@ -69,6 +77,7 @@ export const useAccountStore = defineStore('account', { if (team?.id) { context.setTeamMembership(await teamApi.getTeamUserMembership(team.id)) } + ensureTeamChannelConnected(team || currentTeam) this.pendingTeamChange = false return } @@ -80,6 +89,11 @@ export const useAccountStore = defineStore('account', { context.setTeam(team) this.clearOtherStores() context.setTeamMembership(teamMembership) + if (team?.id) { + ensureTeamChannelConnected(team) + } else { + getServicesOrchestrator().$serviceInstances.teamChannel?.disconnect().catch(() => {}) + } this.pendingTeamChange = false }, async refreshTeams () { diff --git a/frontend/src/types/services/index.ts b/frontend/src/types/services/index.ts index 130d91b51e..a7ddff9f09 100644 --- a/frontend/src/types/services/index.ts +++ b/frontend/src/types/services/index.ts @@ -2,3 +2,4 @@ export * from './service.types.js' export * from './mqtt.types.js' export * from './post-message.types.js' export * from './bootstrap.types.js' +export * from './team-channel.types.js' diff --git a/frontend/src/types/services/service.types.ts b/frontend/src/types/services/service.types.ts index 5972a71b50..788ceb39c6 100644 --- a/frontend/src/types/services/service.types.ts +++ b/frontend/src/types/services/service.types.ts @@ -1,7 +1,7 @@ import type { App } from 'vue' import type { Router } from 'vue-router' -import type { BootstrapServiceI, MqttServiceI, PostMessageServiceI } from '@/types' +import type { BootstrapServiceI, MqttServiceI, PostMessageServiceI, TeamChannelServiceI } from '@/types' /** * Minimal lifecycle contract for app services. @@ -15,6 +15,7 @@ export type ServiceInstances = { bootstrap: BootstrapServiceI | null postMessage: PostMessageServiceI | null mqtt: MqttServiceI | null + teamChannel: TeamChannelServiceI | null } export interface CreateServiceOptions { diff --git a/frontend/src/types/services/team-channel.types.ts b/frontend/src/types/services/team-channel.types.ts new file mode 100644 index 0000000000..5201e1a88a --- /dev/null +++ b/frontend/src/types/services/team-channel.types.ts @@ -0,0 +1,13 @@ +import type { AppService } from '@/types' + +export interface TeamRef { + id: string +} + +export interface TeamChannelServiceI extends AppService { + connect(team: TeamRef | null | undefined): Promise + disconnect(): Promise + destroy(): Promise + isConnected(): boolean + getSessionId(): string +} diff --git a/test/unit/forge/comms/authRoutesV2_spec.js b/test/unit/forge/comms/authRoutesV2_spec.js index e3eaa62b4f..1c4df923bb 100644 --- a/test/unit/forge/comms/authRoutesV2_spec.js +++ b/test/unit/forge/comms/authRoutesV2_spec.js @@ -1315,5 +1315,79 @@ describe('Broker Auth v2 API', async function () { // TODO: tests for Application RBACs (ensure project/device in an application with reduced permissions are suitably restricted in the ACLs) }) + + describe('Team Frontend', async function () { + // checkUserIsTeamMember verifier coverage — the security gate for the + // browser team-channel subscriptions + let teamFrontendUsername + let teamUpdatedTopic + let membershipTopic + let otherTeam + let bob + + before(async function () { + await setupCE() + bob = await factory.createUser({ username: 'bob', name: 'Bob', email: 'bob@example.com', password: 'bbPassword1!' }) + await TestObjects.ATeam.addUser(bob, { through: { role: Roles.Member } }) + otherTeam = await factory.createTeam({ name: 'BTeam' }) + teamFrontendUsername = `team-frontend:${TestObjects.alice.hashid}:${TestObjects.ATeam.hashid}:session-1234567890` + teamUpdatedTopic = `ff/v1/${TestObjects.ATeam.hashid}/team/updated` + membershipTopic = `ff/v1/${TestObjects.ATeam.hashid}/u/${TestObjects.alice.hashid}/membership` + }) + + after(async function () { + await app.close() + }) + + it('allows a team member to subscribe to their team/updated topic', async function () { + await allowRead({ + username: teamFrontendUsername, + topic: teamUpdatedTopic + }) + }) + it('allows a team member to subscribe to their own membership topic', async function () { + await allowRead({ + username: teamFrontendUsername, + topic: membershipTopic + }) + }) + it('denies subscribe when the topic team-hash mismatches the credential', async function () { + await denyRead({ + username: teamFrontendUsername, + topic: `ff/v1/${otherTeam.hashid}/team/updated` + }) + }) + it('denies subscribe to another user\'s membership topic', async function () { + await denyRead({ + username: teamFrontendUsername, + topic: `ff/v1/${TestObjects.ATeam.hashid}/u/${bob.hashid}/membership` + }) + }) + it('denies subscribe for a user who is not a member of the team', async function () { + const charlie = await factory.createUser({ username: 'charlie', name: 'Charlie', email: 'charlie@example.com', password: 'ccPassword1!' }) + // charlie is not in ATeam; he holds a (theoretically) valid credential username + const charlieUsername = `team-frontend:${charlie.hashid}:${TestObjects.ATeam.hashid}:session-1234567890` + await denyRead({ + username: charlieUsername, + topic: teamUpdatedTopic + }) + }) + it('denies team-frontend from publishing (read-only client)', async function () { + await denyWrite({ + username: teamFrontendUsername, + topic: teamUpdatedTopic + }) + }) + it('allows forge_platform to publish team-channel topics', async function () { + await allowWrite({ + username: 'forge_platform', + topic: teamUpdatedTopic + }) + await allowWrite({ + username: 'forge_platform', + topic: membershipTopic + }) + }) + }) }) }) diff --git a/test/unit/frontend/services/services.orchestrator.spec.js b/test/unit/frontend/services/services.orchestrator.spec.js index 9692146d8a..bcdcdab251 100644 --- a/test/unit/frontend/services/services.orchestrator.spec.js +++ b/test/unit/frontend/services/services.orchestrator.spec.js @@ -104,7 +104,8 @@ describe('ServicesOrchestrator', () => { expect(orchestrator.$serviceInstances).toEqual({ bootstrap: null, postMessage: null, - mqtt: null + mqtt: null, + teamChannel: null }) expect(orchestrator.$app).toBeNull() expect(orchestrator.$router).toBeNull() diff --git a/test/unit/frontend/services/team-channel.service.spec.js b/test/unit/frontend/services/team-channel.service.spec.js new file mode 100644 index 0000000000..214969d21c --- /dev/null +++ b/test/unit/frontend/services/team-channel.service.spec.js @@ -0,0 +1,284 @@ +/* eslint-env browser */ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' + +const getTeamCommsCreds = vi.fn() +const refreshTeam = vi.fn().mockResolvedValue(undefined) +const refreshTeamMembership = vi.fn().mockResolvedValue(undefined) +const useContextStore = vi.fn(() => ({ refreshTeam, refreshTeamMembership })) +const useAccountAuthStore = vi.fn(() => ({ user: { id: 'user-hashid-1' } })) + +vi.mock('@/api/team.js', () => ({ + default: { getTeamCommsCreds: (...args) => getTeamCommsCreds(...args) } +})) +vi.mock('@/stores/context.js', () => ({ useContextStore })) +vi.mock('@/stores/account-auth.js', () => ({ useAccountAuthStore })) + +function makeMqttService () { + return { + createClient: vi.fn(), + destroyClient: vi.fn().mockResolvedValue(undefined), + subscribe: vi.fn().mockResolvedValue(undefined) + } +} + +function makeRouter (path = '/team/dev/instances') { + return { + push: vi.fn(), + currentRoute: { value: { path } } + } +} + +describe('TeamChannelService', async () => { + const mod = await import('../../../../frontend/src/services/team-channel.service.ts') + const { createTeamChannelService, destroyTeamChannelService } = mod + + function createService ({ mqtt = makeMqttService(), router = makeRouter() } = {}) { + const services = { mqtt, postMessage: null, bootstrap: null, teamChannel: null } + const service = createTeamChannelService({ app: {}, router, services }) + services.teamChannel = service + return { service, mqtt, router } + } + + beforeEach(async () => { + getTeamCommsCreds.mockReset() + refreshTeam.mockClear() + refreshTeamMembership.mockClear() + useContextStore.mockClear() + useAccountAuthStore.mockClear().mockReturnValue({ user: { id: 'user-hashid-1' } }) + sessionStorage.clear() + await destroyTeamChannelService() + }) + + afterEach(async () => { + await destroyTeamChannelService() + }) + + describe('getSessionId', () => { + test('mints a sessionId and persists it to sessionStorage', () => { + const { service } = createService() + const id = service.getSessionId() + expect(id).toMatch(/^[0-9a-f-]{36}$/) + expect(sessionStorage.getItem('ff-team-channel-session-id')).toBe(id) + }) + + test('returns the same sessionId on subsequent calls', () => { + const { service } = createService() + const first = service.getSessionId() + const second = service.getSessionId() + expect(second).toBe(first) + }) + + test('reuses an existing sessionStorage value (same browser tab)', () => { + sessionStorage.setItem('ff-team-channel-session-id', 'existing-session') + const { service } = createService() + expect(service.getSessionId()).toBe('existing-session') + }) + }) + + describe('connect', () => { + test('no-ops without a team', async () => { + const { service, mqtt } = createService() + await service.connect(null) + await service.connect({}) + await service.connect({ id: '' }) + await service.connect({ id: 123 }) + expect(mqtt.createClient).not.toHaveBeenCalled() + expect(service.isConnected()).toBe(false) + }) + + test('no-ops without a logged-in user', async () => { + useAccountAuthStore.mockReturnValue({ user: null }) + const { service, mqtt } = createService() + await service.connect({ id: 'team-1' }) + expect(mqtt.createClient).not.toHaveBeenCalled() + expect(service.isConnected()).toBe(false) + }) + + test('creates an mqtt client keyed by team id', async () => { + const { service, mqtt } = createService() + mqtt.createClient.mockResolvedValue(undefined) + await service.connect({ id: 'team-1' }) + expect(mqtt.createClient).toHaveBeenCalledTimes(1) + const [key, opts] = mqtt.createClient.mock.calls[0] + expect(key).toBe('team:team-1') + expect(typeof opts.getCredentials).toBe('function') + expect(typeof opts.onMessage).toBe('function') + expect(typeof opts.onConnect).toBe('function') + expect(service.isConnected()).toBe(true) + }) + + test('wires the full mqtt lifecycle handler surface', async () => { + const { service, mqtt } = createService() + mqtt.createClient.mockResolvedValue(undefined) + await service.connect({ id: 'team-1' }) + const opts = mqtt.createClient.mock.calls[0][1] + expect(typeof opts.onMessage).toBe('function') + expect(typeof opts.onConnect).toBe('function') + expect(typeof opts.onClose).toBe('function') + expect(typeof opts.onOffline).toBe('function') + expect(typeof opts.onError).toBe('function') + expect(typeof opts.onDisconnect).toBe('function') + }) + + test('quiet mqtt lifecycle handlers do not throw', async () => { + const { service, mqtt } = createService() + mqtt.createClient.mockResolvedValue(undefined) + await service.connect({ id: 'team-1' }) + const opts = mqtt.createClient.mock.calls[0][1] + expect(() => opts.onClose()).not.toThrow() + expect(() => opts.onOffline()).not.toThrow() + expect(() => opts.onDisconnect()).not.toThrow() + expect(() => opts.onError(new Error('boom'))).not.toThrow() + }) + + test('credential callback forwards team id + per-tab session id', async () => { + getTeamCommsCreds.mockResolvedValue({ url: 'wss://broker', username: 'u', password: 'p' }) + const { service, mqtt } = createService() + mqtt.createClient.mockResolvedValue(undefined) + await service.connect({ id: 'team-1' }) + const opts = mqtt.createClient.mock.calls[0][1] + await opts.getCredentials() + expect(getTeamCommsCreds).toHaveBeenCalledWith('team-1', service.getSessionId()) + }) + + test('skips reconnect when already connected to the same team', async () => { + const { service, mqtt } = createService() + mqtt.createClient.mockResolvedValue(undefined) + await service.connect({ id: 'team-1' }) + await service.connect({ id: 'team-1' }) + expect(mqtt.createClient).toHaveBeenCalledTimes(1) + }) + + test('disconnects from the previous team when switching teams', async () => { + const { service, mqtt } = createService() + mqtt.createClient.mockResolvedValue(undefined) + await service.connect({ id: 'team-1' }) + await service.connect({ id: 'team-2' }) + expect(mqtt.destroyClient).toHaveBeenCalledWith('team:team-1') + expect(mqtt.createClient).toHaveBeenCalledTimes(2) + expect(mqtt.createClient.mock.calls[1][0]).toBe('team:team-2') + }) + + test('degrades gracefully when createClient rejects (e.g. no broker)', async () => { + const { service, mqtt } = createService() + mqtt.createClient.mockRejectedValue(new Error('comms_unavailable')) + await expect(service.connect({ id: 'team-1' })).resolves.toBeUndefined() + expect(service.isConnected()).toBe(false) + }) + }) + + describe('subscribe on connect', () => { + test('subscribes to team/updated and membership topics with qos 1', async () => { + const { service, mqtt } = createService() + let onConnect + mqtt.createClient.mockImplementation(async (_key, opts) => { + onConnect = opts.onConnect + }) + await service.connect({ id: 'team-1' }) + await onConnect() + expect(mqtt.subscribe).toHaveBeenCalledWith( + 'team:team-1', + ['ff/v1/team-1/team/updated', 'ff/v1/team-1/u/user-hashid-1/membership'], + { qos: 1 } + ) + }) + + test('swallows subscribe errors (managed reconnect will retry)', async () => { + const { service, mqtt } = createService() + mqtt.subscribe.mockRejectedValue(new Error('subscribe failed')) + let onConnect + mqtt.createClient.mockImplementation(async (_key, opts) => { + onConnect = opts.onConnect + }) + await service.connect({ id: 'team-1' }) + await expect(onConnect()).resolves.toBeUndefined() + }) + }) + + describe('message routing', () => { + async function connectAndCaptureOnMessage () { + const { service, mqtt, router } = createService() + let onMessage + mqtt.createClient.mockImplementation(async (_key, opts) => { + onMessage = opts.onMessage + }) + await service.connect({ id: 'team-1' }) + return { service, mqtt, router, onMessage } + } + + test('team/updated triggers refreshTeam', async () => { + const { onMessage } = await connectAndCaptureOnMessage() + onMessage('ff/v1/team-1/team/updated', Buffer.from(JSON.stringify({}))) + expect(refreshTeam).toHaveBeenCalledTimes(1) + expect(refreshTeamMembership).not.toHaveBeenCalled() + }) + + test('membership update triggers refreshTeamMembership', async () => { + const { onMessage, router } = await connectAndCaptureOnMessage() + onMessage('ff/v1/team-1/u/user-hashid-1/membership', Buffer.from(JSON.stringify({ reason: 'role-changed' }))) + expect(refreshTeamMembership).toHaveBeenCalledTimes(1) + expect(refreshTeam).not.toHaveBeenCalled() + expect(router.push).not.toHaveBeenCalled() + }) + + test('membership "removed" redirects to Home when on a team route', async () => { + const { onMessage, router } = await connectAndCaptureOnMessage() + onMessage('ff/v1/team-1/u/user-hashid-1/membership', Buffer.from(JSON.stringify({ reason: 'removed' }))) + expect(router.push).toHaveBeenCalledWith({ name: 'Home' }) + expect(refreshTeamMembership).not.toHaveBeenCalled() + }) + + test('membership "removed" does not redirect when on a non-team route', async () => { + const router = makeRouter('/account') + const { service, mqtt } = createService({ router }) + let onMessage + mqtt.createClient.mockImplementation(async (_key, opts) => { + onMessage = opts.onMessage + }) + await service.connect({ id: 'team-1' }) + onMessage('ff/v1/team-1/u/user-hashid-1/membership', Buffer.from(JSON.stringify({ reason: 'removed' }))) + expect(router.push).not.toHaveBeenCalled() + }) + + test('does not throw on malformed JSON payloads', async () => { + const { onMessage } = await connectAndCaptureOnMessage() + expect(() => onMessage('ff/v1/team-1/team/updated', Buffer.from('not json'))).not.toThrow() + expect(refreshTeam).toHaveBeenCalledTimes(1) + }) + + test('ignores topics outside the team-channel scheme', async () => { + const { onMessage } = await connectAndCaptureOnMessage() + onMessage('ff/v1/team-1/some/other/topic', Buffer.from('{}')) + expect(refreshTeam).not.toHaveBeenCalled() + expect(refreshTeamMembership).not.toHaveBeenCalled() + }) + }) + + describe('disconnect / destroy', () => { + test('disconnect destroys the client and clears connected state', async () => { + const { service, mqtt } = createService() + mqtt.createClient.mockResolvedValue(undefined) + await service.connect({ id: 'team-1' }) + await service.disconnect() + expect(mqtt.destroyClient).toHaveBeenCalledWith('team:team-1') + expect(service.isConnected()).toBe(false) + }) + + test('disconnect is a no-op when not connected', async () => { + const { service, mqtt } = createService() + await service.disconnect() + expect(mqtt.destroyClient).not.toHaveBeenCalled() + }) + + test('destroy disconnects and clears the sessionId cache', async () => { + const { service, mqtt } = createService() + mqtt.createClient.mockResolvedValue(undefined) + await service.connect({ id: 'team-1' }) + const sessionIdBefore = service.getSessionId() + await service.destroy() + expect(mqtt.destroyClient).toHaveBeenCalledWith('team:team-1') + // per-tab id survives destroy (logout/login on same tab) + expect(sessionStorage.getItem('ff-team-channel-session-id')).toBe(sessionIdBefore) + }) + }) +}) diff --git a/test/unit/frontend/stores/account-auth.spec.js b/test/unit/frontend/stores/account-auth.spec.js index f2e5720d31..ade8457f59 100644 --- a/test/unit/frontend/stores/account-auth.spec.js +++ b/test/unit/frontend/stores/account-auth.spec.js @@ -19,6 +19,13 @@ vi.mock('@/api/user.js', () => ({ } })) +const teamChannelDisconnect = vi.fn().mockResolvedValue(undefined) +vi.mock('@/services/service.orchestrator', () => ({ + default: () => ({ + $serviceInstances: { teamChannel: { disconnect: teamChannelDisconnect } } + }) +})) + // imported after mocks so vi.mock hoisting resolves correctly const { useAccountAuthStore } = await import('@/stores/account-auth.js') const userApi = (await import('@/api/user.js')).default @@ -138,6 +145,37 @@ describe('account-auth store', () => { }) }) + describe('logout', () => { + beforeEach(async () => { + // Avoid the redirect at the end of logout() + Object.defineProperty(window, 'location', { + writable: true, + value: { ...window.location, assign: vi.fn() } + }) + userApi.logout.mockResolvedValue(undefined) + // logout() reads platform:sso:only from settings — seed an empty object + const { useAccountSettingsStore } = await import('@/stores/account-settings.js') + useAccountSettingsStore().setSettings({}) + }) + + it('disconnects the team channel before calling userApi.logout', async () => { + const store = useAccountAuthStore() + store.user = { id: '1' } + const order = [] + teamChannelDisconnect.mockImplementation(async () => { order.push('disconnect') }) + userApi.logout.mockImplementation(async () => { order.push('logout') }) + await store.logout() + expect(order).toEqual(['disconnect', 'logout']) + }) + + it('tolerates a missing team-channel service', async () => { + const store = useAccountAuthStore() + store.user = { id: '1' } + teamChannelDisconnect.mockRejectedValueOnce(new Error('boom')) + await expect(store.logout()).resolves.toBeUndefined() + }) + }) + describe('$reset', () => { it('restores default state', () => { const store = useAccountAuthStore() From 962c440665f568478db8d0bd10222e9c7c6ddc45 Mon Sep 17 00:00:00 2001 From: Noley Holland Date: Fri, 22 May 2026 07:30:26 -0700 Subject: [PATCH 2/5] Update generated types --- frontend/src/types/generated.ts | 57 +++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/frontend/src/types/generated.ts b/frontend/src/types/generated.ts index 91e65b7ffb..f90ac6f4ca 100644 --- a/frontend/src/types/generated.ts +++ b/frontend/src/types/generated.ts @@ -2007,6 +2007,63 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/teams/{teamId}/comms-credentials": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** Issue team-channel broker credentials for the current user/session */ + post: { + parameters: { + query?: never; + header?: never; + path: { + teamId: string; + }; + cookie?: never; + }; + requestBody?: { + content: { + "application/json": { + sessionId?: string; + }; + }; + }; + responses: { + /** @description Default Response */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + url?: string; + username?: string; + password?: string; + }; + }; + }; + /** @description Default Response */ + "4XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + }; + }; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/teams/{teamId}/user": { parameters: { query?: never; From 9ff800c1315d68d7e70c1d74193bd91cda6db4ff Mon Sep 17 00:00:00 2001 From: Noley Holland Date: Fri, 22 May 2026 08:41:52 -0700 Subject: [PATCH 3/5] Fix team-channel removal redirect and duplicated-tab session collision --- frontend/src/services/team-channel.service.ts | 24 ++++-------- .../services/team-channel.service.spec.js | 37 ++++++++++++------- 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/frontend/src/services/team-channel.service.ts b/frontend/src/services/team-channel.service.ts index 3a0cee82fc..bd1a05e227 100644 --- a/frontend/src/services/team-channel.service.ts +++ b/frontend/src/services/team-channel.service.ts @@ -9,7 +9,6 @@ import { Maybe } from '@/types/common/types' import type { CreateServiceOptions } from '@/types/services/service.types' import type { TeamChannelServiceI, TeamRef } from '@/types/services/team-channel.types' -const SESSION_KEY = 'ff-team-channel-session-id' const MEMBERSHIP_TOPIC_REGEX = /^ff\/v1\/[^/]+\/u\/([^/]+)\/membership$/ function connectionKey (teamId: string): string { @@ -29,19 +28,11 @@ class TeamChannelService extends BaseService implements TeamChannelServiceI { }) } + // Minted per page-load so duplicated tabs (which clone sessionStorage in + // most browsers) still get distinct credentials and don't kick each other. getSessionId (): string { - if (this.$sessionId) return this.$sessionId - try { - let sessionId = sessionStorage.getItem(SESSION_KEY) - if (!sessionId) { - sessionId = uuidv4() - sessionStorage.setItem(SESSION_KEY, sessionId) - } - this.$sessionId = sessionId - } catch { - this.$sessionId = uuidv4() - } - return this.$sessionId as string + if (!this.$sessionId) this.$sessionId = uuidv4() + return this.$sessionId } isConnected (): boolean { @@ -153,13 +144,12 @@ class TeamChannelService extends BaseService implements TeamChannelServiceI { protected _handleMembership (payload: { reason?: string }): void { if (payload?.reason === 'removed') { - // Only redirect if the user is on a team-scoped route — leave them + // Only act if the user is on a team-scoped route — leave them // alone if they're already on /account or another non-team page. const path = this.$router?.currentRoute?.value?.path if (typeof path === 'string' && path.startsWith('/team/')) { - try { - this.$router?.push({ name: 'Home' }) - } catch {} + // Hard reload — a soft push to Home bounces back to the current team + try { window.location.assign('/') } catch {} } return } diff --git a/test/unit/frontend/services/team-channel.service.spec.js b/test/unit/frontend/services/team-channel.service.spec.js index 214969d21c..f6bbc66753 100644 --- a/test/unit/frontend/services/team-channel.service.spec.js +++ b/test/unit/frontend/services/team-channel.service.spec.js @@ -54,24 +54,23 @@ describe('TeamChannelService', async () => { }) describe('getSessionId', () => { - test('mints a sessionId and persists it to sessionStorage', () => { + test('mints a uuid sessionId on first call', () => { const { service } = createService() const id = service.getSessionId() expect(id).toMatch(/^[0-9a-f-]{36}$/) - expect(sessionStorage.getItem('ff-team-channel-session-id')).toBe(id) }) - test('returns the same sessionId on subsequent calls', () => { + test('returns the same sessionId on subsequent calls within the same instance', () => { const { service } = createService() const first = service.getSessionId() const second = service.getSessionId() expect(second).toBe(first) }) - test('reuses an existing sessionStorage value (same browser tab)', () => { - sessionStorage.setItem('ff-team-channel-session-id', 'existing-session') + test('does not persist to sessionStorage (so duplicated tabs get distinct ids)', () => { const { service } = createService() - expect(service.getSessionId()).toBe('existing-session') + service.getSessionId() + expect(sessionStorage.getItem('ff-team-channel-session-id')).toBeNull() }) }) @@ -221,14 +220,24 @@ describe('TeamChannelService', async () => { expect(router.push).not.toHaveBeenCalled() }) - test('membership "removed" redirects to Home when on a team route', async () => { - const { onMessage, router } = await connectAndCaptureOnMessage() + test('membership "removed" hard-reloads to / when on a team route', async () => { + const assign = vi.fn() + Object.defineProperty(window, 'location', { + writable: true, + value: { ...window.location, assign } + }) + const { onMessage } = await connectAndCaptureOnMessage() onMessage('ff/v1/team-1/u/user-hashid-1/membership', Buffer.from(JSON.stringify({ reason: 'removed' }))) - expect(router.push).toHaveBeenCalledWith({ name: 'Home' }) + expect(assign).toHaveBeenCalledWith('/') expect(refreshTeamMembership).not.toHaveBeenCalled() }) - test('membership "removed" does not redirect when on a non-team route', async () => { + test('membership "removed" does not reload when on a non-team route', async () => { + const assign = vi.fn() + Object.defineProperty(window, 'location', { + writable: true, + value: { ...window.location, assign } + }) const router = makeRouter('/account') const { service, mqtt } = createService({ router }) let onMessage @@ -237,7 +246,7 @@ describe('TeamChannelService', async () => { }) await service.connect({ id: 'team-1' }) onMessage('ff/v1/team-1/u/user-hashid-1/membership', Buffer.from(JSON.stringify({ reason: 'removed' }))) - expect(router.push).not.toHaveBeenCalled() + expect(assign).not.toHaveBeenCalled() }) test('does not throw on malformed JSON payloads', async () => { @@ -270,15 +279,15 @@ describe('TeamChannelService', async () => { expect(mqtt.destroyClient).not.toHaveBeenCalled() }) - test('destroy disconnects and clears the sessionId cache', async () => { + test('destroy disconnects and clears the in-memory sessionId', async () => { const { service, mqtt } = createService() mqtt.createClient.mockResolvedValue(undefined) await service.connect({ id: 'team-1' }) const sessionIdBefore = service.getSessionId() await service.destroy() expect(mqtt.destroyClient).toHaveBeenCalledWith('team:team-1') - // per-tab id survives destroy (logout/login on same tab) - expect(sessionStorage.getItem('ff-team-channel-session-id')).toBe(sessionIdBefore) + // next caller mints a fresh id rather than reusing the destroyed one + expect(service.getSessionId()).not.toBe(sessionIdBefore) }) }) }) From 9a4001a382fbd62bc51ea676c8fbfdb6328046bf Mon Sep 17 00:00:00 2001 From: Noley Holland Date: Fri, 22 May 2026 09:28:25 -0700 Subject: [PATCH 4/5] Self review - log errors, gate notify behind successful result, update specs --- forge/comms/aclManager.js | 3 ++- forge/routes/api/teamMembers.js | 4 +--- .../services/services.orchestrator.spec.js | 22 ++++++++++++++++++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/forge/comms/aclManager.js b/forge/comms/aclManager.js index 05cedec15e..ec816e2b25 100644 --- a/forge/comms/aclManager.js +++ b/forge/comms/aclManager.js @@ -135,7 +135,8 @@ module.exports = function (app) { if (!user) return false const membership = await app.db.models.TeamMember.getTeamMembership(user.id, team.id, false) return !!membership - } catch (err) { + } catch (error) { + app.log.error('Unexpected error during team-channel ACL check', { requestParts, usernameParts, error }) return false } }, diff --git a/forge/routes/api/teamMembers.js b/forge/routes/api/teamMembers.js index 147a20f6f0..ea925a3ff5 100644 --- a/forge/routes/api/teamMembers.js +++ b/forge/routes/api/teamMembers.js @@ -117,12 +117,10 @@ module.exports = async function (app) { // the needsPermission handler will have ensured the requesting user is allowed // to make this request. All we have to do try { - // Cue the targeted user's session to redirect before we delete the - // membership row (so their next API call doesn't 404 them mid-action). - app.comms?.team?.notifyMembership(request.team.hashid, request.user.hashid, 'removed') const result = await app.db.controllers.Team.removeUser(request.team, request.user, request.userRole) if (result) { await app.auditLog.Team.team.user.removed(request.session.User, null, request.team, request.user) + app.comms?.team?.notifyMembership(request.team.hashid, request.user.hashid, 'removed') } reply.send({ status: 'okay' }) } catch (err) { diff --git a/test/unit/frontend/services/services.orchestrator.spec.js b/test/unit/frontend/services/services.orchestrator.spec.js index bcdcdab251..492b839046 100644 --- a/test/unit/frontend/services/services.orchestrator.spec.js +++ b/test/unit/frontend/services/services.orchestrator.spec.js @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, test, vi } from 'vitest' const mockCreateBootstrapService = vi.fn() const mockCreateMessagingService = vi.fn() const mockCreateMqttService = vi.fn() +const mockCreateTeamChannelService = vi.fn() vi.mock('../../../../frontend/src/services/bootstrap.service.js', () => { return { @@ -22,6 +23,12 @@ vi.mock('../../../../frontend/src/services/mqtt.service.js', () => { } }) +vi.mock('../../../../frontend/src/services/team-channel.service.js', () => { + return { + createTeamChannelService: mockCreateTeamChannelService + } +}) + async function loadOrchestratorModule () { vi.resetModules() return await import('../../../../frontend/src/services/service.orchestrator.ts') @@ -32,16 +39,19 @@ describe('ServicesOrchestrator', () => { mockCreateBootstrapService.mockReset() mockCreateMessagingService.mockReset() mockCreateMqttService.mockReset() + mockCreateTeamChannelService.mockReset() }) test('init boots services, injects shared instances, and provides them on the app', async () => { const bootstrapService = { name: 'bootstrap', init: vi.fn(), destroy: vi.fn().mockResolvedValue() } const postMessageService = { name: 'postMessage', destroy: vi.fn().mockResolvedValue() } const mqttService = { name: 'mqtt', destroy: vi.fn().mockResolvedValue() } + const teamChannelService = { name: 'teamChannel', destroy: vi.fn().mockResolvedValue() } mockCreateBootstrapService.mockReturnValue(bootstrapService) mockCreateMessagingService.mockReturnValue(postMessageService) mockCreateMqttService.mockReturnValue(mqttService) + mockCreateTeamChannelService.mockReturnValue(teamChannelService) const app = { provide: vi.fn(), @@ -57,17 +67,21 @@ describe('ServicesOrchestrator', () => { const bootstrapArgs = mockCreateBootstrapService.mock.calls[0][0] const messagingArgs = mockCreateMessagingService.mock.calls[0][0] const mqttArgs = mockCreateMqttService.mock.calls[0][0] + const teamChannelArgs = mockCreateTeamChannelService.mock.calls[0][0] expect(bootstrapArgs).toMatchObject({ app, router }) expect(messagingArgs).toMatchObject({ app, router }) expect(mqttArgs).toMatchObject({ app, router }) + expect(teamChannelArgs).toMatchObject({ app, router }) expect(messagingArgs.services).toBe(bootstrapArgs.services) expect(mqttArgs.services).toBe(bootstrapArgs.services) + expect(teamChannelArgs.services).toBe(bootstrapArgs.services) expect(bootstrapArgs.services).toMatchObject({ bootstrap: bootstrapService, postMessage: postMessageService, - mqtt: mqttService + mqtt: mqttService, + teamChannel: teamChannelService }) expect(bootstrapService.init).toHaveBeenCalledTimes(1) @@ -78,10 +92,12 @@ describe('ServicesOrchestrator', () => { const bootstrapService = { name: 'bootstrap', init: vi.fn(), destroy: vi.fn().mockResolvedValue() } const postMessageService = { name: 'postMessage', destroy: vi.fn().mockResolvedValue() } const mqttService = { name: 'mqtt', destroy: vi.fn().mockRejectedValue(new Error('destroy failed')) } + const teamChannelService = { name: 'teamChannel', destroy: vi.fn().mockResolvedValue() } mockCreateBootstrapService.mockReturnValue(bootstrapService) mockCreateMessagingService.mockReturnValue(postMessageService) mockCreateMqttService.mockReturnValue(mqttService) + mockCreateTeamChannelService.mockReturnValue(teamChannelService) const app = { provide: vi.fn(), @@ -96,11 +112,13 @@ describe('ServicesOrchestrator', () => { bootstrapService.destroy.mockClear() postMessageService.destroy.mockClear() mqttService.destroy.mockClear() + teamChannelService.destroy.mockClear() await orchestrator.dispose() expect(bootstrapService.destroy).toHaveBeenCalledTimes(1) expect(postMessageService.destroy).toHaveBeenCalledTimes(1) expect(mqttService.destroy).toHaveBeenCalledTimes(1) + expect(teamChannelService.destroy).toHaveBeenCalledTimes(1) expect(orchestrator.$serviceInstances).toEqual({ bootstrap: null, postMessage: null, @@ -116,10 +134,12 @@ describe('ServicesOrchestrator', () => { const bootstrapService = { name: 'bootstrap', init: vi.fn(), destroy: vi.fn().mockResolvedValue() } const postMessageService = { name: 'postMessage', destroy: vi.fn().mockResolvedValue() } const mqttService = { name: 'mqtt', destroy: vi.fn().mockResolvedValue() } + const teamChannelService = { name: 'teamChannel', destroy: vi.fn().mockResolvedValue() } mockCreateBootstrapService.mockReturnValue(bootstrapService) mockCreateMessagingService.mockReturnValue(postMessageService) mockCreateMqttService.mockReturnValue(mqttService) + mockCreateTeamChannelService.mockReturnValue(teamChannelService) const app = { provide: vi.fn(), From 7cfa5c6cf69fc0ac707bcca08ff53d1bd864494c Mon Sep 17 00:00:00 2001 From: Noley Holland Date: Thu, 28 May 2026 11:10:06 -0700 Subject: [PATCH 5/5] Implement tests to cover team-channel comms-credentials endpoint, BrokerClient creator, and team-frontend ACL paths --- test/unit/forge/comms/authRoutesV2_spec.js | 35 ++++++ .../forge/db/controllers/BrokerClient_spec.js | 47 +++++++ test/unit/forge/routes/api/team_spec.js | 115 ++++++++++++++++++ 3 files changed, 197 insertions(+) diff --git a/test/unit/forge/comms/authRoutesV2_spec.js b/test/unit/forge/comms/authRoutesV2_spec.js index 1c4df923bb..29f1e5007d 100644 --- a/test/unit/forge/comms/authRoutesV2_spec.js +++ b/test/unit/forge/comms/authRoutesV2_spec.js @@ -1,4 +1,6 @@ const should = require('should') // eslint-disable-line +const sinon = require('sinon') + const { Roles } = require('../../../../forge/lib/roles') const setup = require('../routes/setup') const TestModelFactory = require('../../../lib/TestModelFactory') // eslint-disable-line @@ -1388,6 +1390,39 @@ describe('Broker Auth v2 API', async function () { topic: membershipTopic }) }) + it('denies subscribe when the credential\'s team hash does not resolve to a team', async function () { + const teamLookupStub = sinon.stub(app.db.models.Team, 'byId').resolves(null) + try { + await denyRead({ + username: teamFrontendUsername, + topic: teamUpdatedTopic + }) + } finally { + teamLookupStub.restore() + } + }) + it('denies subscribe when the credential\'s user hash does not resolve to a user', async function () { + const userLookupStub = sinon.stub(app.db.models.User, 'byId').resolves(null) + try { + await denyRead({ + username: teamFrontendUsername, + topic: teamUpdatedTopic + }) + } finally { + userLookupStub.restore() + } + }) + it('denies subscribe when the membership lookup throws', async function () { + const teamLookupStub = sinon.stub(app.db.models.Team, 'byId').rejects(new Error('boom')) + try { + await denyRead({ + username: teamFrontendUsername, + topic: teamUpdatedTopic + }) + } finally { + teamLookupStub.restore() + } + }) }) }) }) diff --git a/test/unit/forge/db/controllers/BrokerClient_spec.js b/test/unit/forge/db/controllers/BrokerClient_spec.js index 683a747ea8..827e6cfc17 100644 --- a/test/unit/forge/db/controllers/BrokerClient_spec.js +++ b/test/unit/forge/db/controllers/BrokerClient_spec.js @@ -130,6 +130,53 @@ describe('BrokerClient', function () { app.settings.set.calledWith('platform:expert-agent:creds', false).should.be.true() }) }) + describe('createClientForTeamFrontend', function () { + it('should create broker client for team frontend', async function () { + const fakeClient = fakeBrokerClient() + app.db.models.BrokerClient.findOne.resolves(null) + app.db.models.BrokerClient.create.resolves(fakeClient) + const user = { id: 7, hashid: 'userHash' } + const team = { id: 11, hashid: 'teamHash' } + const sessionId = 'tab-1234567890' + const result = await BrokerClient.createClientForTeamFrontend(app, user, team, sessionId) + const expectedUsername = `team-frontend:${user.hashid}:${team.hashid}:${sessionId}` + should(result).have.property('username', expectedUsername) + should(result).have.property('password') + result.password.should.match(/^ffbtf_/) + should(result).have.property('url', 'http://public.url') + app.db.models.BrokerClient.create.calledWith({ + username: expectedUsername, + password: result.password, + ownerId: '' + user.id, + ownerType: 'team-frontend' + }).should.be.true() + }) + it('should destroy existing client for same session before creating a new one', async function () { + const existingClient = fakeBrokerClient() + app.db.models.BrokerClient.findOne.resolves(existingClient) + app.db.models.BrokerClient.create.resolves(fakeBrokerClient()) + const user = { id: 7, hashid: 'userHash' } + const team = { id: 11, hashid: 'teamHash' } + const result = await BrokerClient.createClientForTeamFrontend(app, user, team, 'tab-1234567890') + existingClient.destroy.called.should.be.true() + should(result).have.property('username', `team-frontend:${user.hashid}:${team.hashid}:tab-1234567890`) + }) + it('should look up existing client scoped to the full username (including sessionId) so different tabs do not revoke each other', async function () { + app.db.models.BrokerClient.findOne.resolves(null) + app.db.models.BrokerClient.create.resolves(fakeBrokerClient()) + const user = { id: 7, hashid: 'userHash' } + const team = { id: 11, hashid: 'teamHash' } + await BrokerClient.createClientForTeamFrontend(app, user, team, 'tab-A') + const findOneArgs = app.db.models.BrokerClient.findOne.firstCall.args[0] + findOneArgs.should.have.property('where') + findOneArgs.where.should.have.property('username', `team-frontend:${user.hashid}:${team.hashid}:tab-A`) + }) + it('should return null if app.comms is not available', async function () { + app.comms = null + const result = await BrokerClient.createClientForTeamFrontend(app, { id: 7, hashid: 'userHash' }, { id: 11, hashid: 'teamHash' }, 'tab-1234567890') + should(result).be.null() + }) + }) describe('createClientForExpertClient', function () { it('should create broker client for expert client', async function () { const fakeClient = fakeBrokerClient() diff --git a/test/unit/forge/routes/api/team_spec.js b/test/unit/forge/routes/api/team_spec.js index 4bc8e47b94..53a6337a64 100644 --- a/test/unit/forge/routes/api/team_spec.js +++ b/test/unit/forge/routes/api/team_spec.js @@ -1460,6 +1460,121 @@ describe('Team API', function () { // GET /api/v1/teams/:teamId/user }) + describe('Comms credentials', async function () { + // POST /api/v1/teams/:teamId/comms-credentials + it('issues team-channel credentials for a team member', async function () { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${TestObjects.ATeam.hashid}/comms-credentials`, + payload: { sessionId: 'tab-1234567890' }, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(200) + const result = response.json() + result.should.have.property('username', `team-frontend:${TestObjects.bob.hashid}:${TestObjects.ATeam.hashid}:tab-1234567890`) + result.should.have.property('password') + result.password.should.match(/^ffbtf_/) + result.should.have.property('url') + }) + it('rejects a non-member with 404', async function () { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${TestObjects.ATeam.hashid}/comms-credentials`, + payload: { sessionId: 'tab-1234567890' }, + cookies: { sid: TestObjects.tokens.chris } + }) + response.statusCode.should.equal(404) + }) + it('rejects when sessionId is missing', async function () { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${TestObjects.ATeam.hashid}/comms-credentials`, + payload: {}, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(400) + response.json().should.have.property('code', 'invalid_request') + }) + it('rejects when sessionId is too short', async function () { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${TestObjects.ATeam.hashid}/comms-credentials`, + payload: { sessionId: 'short' }, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(400) + response.json().should.have.property('code', 'invalid_request') + }) + it('returns 503 when comms is not configured', async function () { + const originalComms = app.comms + app.comms = null + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${TestObjects.ATeam.hashid}/comms-credentials`, + payload: { sessionId: 'tab-1234567890' }, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(503) + response.json().should.have.property('code', 'comms_unavailable') + } finally { + app.comms = originalComms + } + }) + it('returns 503 when broker client cannot be created', async function () { + const stub = sinon.stub(app.db.controllers.BrokerClient, 'createClientForTeamFrontend').resolves(null) + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${TestObjects.ATeam.hashid}/comms-credentials`, + payload: { sessionId: 'tab-1234567890' }, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(503) + response.json().should.have.property('code', 'comms_unavailable') + } finally { + stub.restore() + } + }) + }) + + describe('Team-channel notifications', async function () { + it('publishes a team-channel notify on team update', async function () { + const team = await app.db.models.Team.create({ name: generateName('notify-team'), slug: generateName('notify-slug'), TeamTypeId: app.defaultTeamType.id }) + await team.addUser(TestObjects.bob, { through: { role: Roles.Owner } }) + const notifyStub = sinon.stub(app.comms.team, 'notify') + try { + const response = await app.inject({ + method: 'PUT', + url: `/api/v1/teams/${team.hashid}`, + payload: { name: team.name + '-updated' }, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(200) + notifyStub.calledWith(team.hashid, 'updated').should.be.true() + } finally { + notifyStub.restore() + } + }) + it('publishes a team-channel notify when a team is suspended', async function () { + const team = await app.db.models.Team.create({ name: generateName('notify-suspend'), slug: generateName('notify-suspend-slug'), TeamTypeId: app.defaultTeamType.id }) + await team.addUser(TestObjects.bob, { through: { role: Roles.Owner } }) + const notifyStub = sinon.stub(app.comms.team, 'notify') + try { + const response = await app.inject({ + method: 'PUT', + url: `/api/v1/teams/${team.hashid}`, + payload: { suspended: true }, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(200) + notifyStub.calledWith(team.hashid, 'suspended').should.be.true() + } finally { + notifyStub.restore() + } + }) + }) + describe('Get team audit-log', async function () { // GET /api/v1/teams/:teamId/audit-log })