TCHAP: fix external detection for prod HS#1598
Open
thistehneisen wants to merge 1 commit into
Open
Conversation
doesTargetsContainsExternal used a hardcoded domain list that omitted the production external homeserver (agent.externe.tchap.gouv.fr), so inviting a production external user by Matrix ID into a Restricted room did not trigger the external-access warning or the accessRules update. Derive the external homeserver domain(s) from the runtime config (homeserver_list entries whose server_name contains "Externes") via the new TchapUtils.getExternalHomeServerDomains() helper, so detection stays correct across prod/preprod/dev. Also remove a debug console.log that leaked member objects to the console.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: external-user detection in invite flow misses the production external homeserver
Summary
InviteDialog.doesTargetsContainsExternal()decides whether selected invitetargets include an external user. That decision drives the entire
external-invite safeguard introduced in the new invite flow (PR #1565):
irreversible" confirmation modal, and
m.room.accessRules→Unrestrictedstate event so the room iscorrectly classified as containing external participants.
The external check used a hardcoded domain list:
This list does not contain the production external homeserver domain.
Per
config.prod.json(andconfig.prod.lab.json) the external homeserver is:so production external accounts are
@user:agent.externe.tchap.gouv.fr. Thematch is an exact
Array.includes, and"agent.externe.tchap.gouv.fr"is notin the list (the list has
externe.tchap.gouv.frwith noagent.prefix, plusthe preprod and dev domains).
Impact
In production, inviting an existing external user by Matrix ID / directory
pick (a resolved user — not a
ThreepidMember, display name is not an email)into a
Restrictedinternal-only room is not detected as external.Consequently:
Internal members lose the signal that an external party is present in the room —
the exact internal/external segregation guarantee Tchap is built around.
Fix
Derive the external homeserver domain(s) from the runtime config instead of
a hardcoded list. A new helper
TchapUtils.getExternalHomeServerDomains()returns the MXID domain(s) of the
homeserver_listentries whoseserver_namecontains
"Externes", by stripping the scheme and the leadingmatrix.labelfrom
base_url.This stays correct across every environment without code changes:
https://matrix.agent.externe.tchap.gouv.fragent.externe.tchap.gouv.frhttps://matrix.e.tchap.gouv.fre.tchap.gouv.frhttps://matrix.ext01.tchap.incubateur.netext01.tchap.incubateur.netInviteDialog.doesTargetsContainsExternal()now uses that helper. The strayconsole.log("in doesTargetsContainsExternal member", m)(which leaked memberobjects to the browser console) was also removed.
Changes
src/tchap/util/TchapUtils.ts— addgetExternalHomeServerDomains().src/components/views/dialogs/InviteDialog.tsx— use config-derived domains;remove debug
console.log.Nils Putnins / OffSeq Cybersecurity
npu@offseq.com / https://offseq.com / https://radar.offseq.com