Conversation
…d expert-agent roles
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7008 +/- ##
=======================================
Coverage 76.62% 76.62%
=======================================
Files 405 405
Lines 20566 20566
Branches 4972 4972
=======================================
Hits 15758 15758
Misses 4808 4808
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ssions and restrictions
…session management
hardillb
left a comment
There was a problem hiding this comment.
OK, I've read it all apart from the aclManager.js
What I've read looks OK, but need to spend time to actually step through the acl matching.
|
are we ok to mark this as ready for review and review it? |
@hardillb this has been marked as ready for review. To (hopefully) help with the understanding the acl matching, below may or may not be useful: ACL matching cheat sheet for
|
| Username | Who | |
|---|---|---|
expert-client:<userid>:<sessionid> |
A logged-in user's browser session | one per browser tab |
expert-agent:<userid>:<apiversion> |
The single backend "Expert" worker that services every user | one process |
The topic shape
ff/v1/expert/<userid>/<sessionid>/<entity>/<entityId>/support/<channel>/...
^entity [a|p|d|t] == app|project|device|team
Two channels:
- chat -
.../support/chat/request(client→agent) and.../support/chat/response(agent→client) - inflight -
.../support/inflight/<toolName>/request(agent→client) and.../inflight/<toolName>/response(client→agent)
The pub on one side mirrors the sub on the other - client pub chat/request pairs with agent sub chat/request, etc.
Two-stage match (in verify, aclManager.js:383-456)
- Pick the bucket by username prefix + sub/pub → e.g.
ACLS.expertClient.sub/ACLS.expertClient.pub. - First matching regex wins. If that ACL entry has a
verify, run it with the regex captures, the username parts, and the ACL's config flags. No regex match → denied.
What gets wildcarded, and why
+ (single-level MQTT wildcard) is only honored where the ACL config opts in:
| userid | sessionid | entity/entityId | |
|---|---|---|---|
| client sub | ❌ (must match own user) | ❌ (must match own session) | ✅ (one sub covers all entities in this session) |
| client pub | ❌ | ❌ | ❌ (publishes are always specific) |
| agent sub | ✅ | ✅ | ✅ (one agent fans-in everything) |
| agent pub | ❌ | ❌ | ❌ (replies are always specific) |
Entity wildcarding requires both entityType and entityId to be + together - you can't wildcard one half (aclManager.js:185-189).
What checkExpertTopic enforces (aclManager.js:118-253)
In order, denying on first failure:
- Client-type sanity: agent ACL must be hit by an agent username, client ACL by a client. Stops a client from ever falling into agent rules.
- Topic part count : chat=5 segments after the prefix, inflight=6. Cheap shape check.
- userid present and (for clients) matches the username's userid - a client cannot pub/sub on another user's topics.
- userid exists in DB: for clients always, for agents only on pub. Agent subs can use
+. - sessionid present; for clients must match the username's sessionid; for agents
+allowed; otherwise must be ≥ 8 chars. - Entity resolves :
p/d/a/tlooked up in DB to get the owningteamId(andapplicationHashwhere relevant). Unknown entityType or missing record → deny. - User is a team member of that owning team.
- For inflight only: RBAC check on the tool name via
expertRbacToolCheck-expert:status-messageis free,automation:select-nodes / get-nodes / get-flowsneedproject:flows:view, anything else falls back toproject:flows:edit.
Wildcarded-entity subs skip steps 6–8 (no entity to look up, no team to check) - that's safe because the agent is trusted infrastructure and clients can't wildcard the user/session, so a client wildcard sub only ever sees its own session's traffic.
The mental model in one line
Username decides who you are; the regex bucket decides what shape of topic you're allowed to touch;
checkExpertTopicdecides whether the userid/session/entity in the topic is actually yours and you have permission to see it - with carefully scoped wildcards so the agent can fan-in and a client can fan-out across its own session without ever crossing a user or team boundary.
hardillb
left a comment
There was a problem hiding this comment.
Approved, but please just check my comments
Description
Adds backend support for Expert requests/responses and inflight request/response messages over pubsub.
Key parts:
/api/v1/user/expert-creds(POST) inforge/routes/api/user.jsfor Expert User to get creds for MQTT client/api/v1/admin/expert-agent-creds(DELETE and POST) inforge/routes/api/admin.jscreateClientForExpertClientinforge/db/controllers/BrokerClient.jsfor FE broker clientcreateClientForExpertAgent,removeClientForExpertAgentfor generating BE agent broker clientforge/comms/aclManager.jscheckExpertTopicexpert-client:*asexpertClientexpert-agent:*asexpertAgentTests added:
test/unit/forge/db/controllers/BrokerClient_spec.jstest/unit/forge/comms/authRoutesV2_spec.jstest/unit/forge/routes/api/user_spec.jsRelated Issue(s)
Checklist
flowforge.yml?FlowFuse/helmto update ConfigMap TemplateFlowFuse/CloudProjectto update values for Staging/ProductionLabels
area:migrationlabel