Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usernames containing mxids are overzealous at disambiguation #5914

Closed
turt2live opened this issue Jan 3, 2018 · 20 comments
Closed

Usernames containing mxids are overzealous at disambiguation #5914

turt2live opened this issue Jan 3, 2018 · 20 comments
Labels
A-Timeline O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@turt2live
Copy link
Member

turt2live commented Jan 3, 2018

Description

image

There's only one RSS bot in the room.

Introduced by matrix-org/matrix-js-sdk#588

Version information

  • Platform: web (in-browser)
  • Browser: Chrome 63
  • OS: Windows 10
  • URL: riot.im/develop
@pafcu
Copy link
Contributor

pafcu commented Jan 3, 2018

This is by design, so "overzeleous" is up for debate.

@t3chguy
Copy link
Member

t3chguy commented Jan 3, 2018

Why would it care about MXID's in square brackets?
Disambiguation uses round brackets in js-sdk

@turt2live
Copy link
Member Author

I thought the original aim of matrix-org/matrix-js-sdk#588 was to make sure that people don't set their display name to @travis:t2l.io, not make reading a room harder.

@t3chguy
Copy link
Member

t3chguy commented Jan 3, 2018

I think it was more to make manual disambiguation clear

Say I changed my display name to TravisR (@travis:t2l.io) (which would not conflict with yours yet make me look like you)

@turt2live
Copy link
Member Author

That's what I meant, I'm just bad at words. It makes perfect sense to throw in disambiguation for that, but the RSS Bot is clearly not trying to impersonate me.

@pafcu
Copy link
Contributor

pafcu commented Jan 3, 2018

If I didn't know what disambiguation looks like I would surely be tricked. Do we want to demand users to remember wether disambig uses brackets or parens, even when they might never have seen it before? And what about all the nearly identical parentheses? This is by far the easiest and secure solution. The only problem is if the display name is abused to display metadata about the user like above. I don't see this as a problem.

@turt2live
Copy link
Member Author

The display name isn't abused in this case - it's an intentional decision as well. The user is added to the notification bots for moderation purposes as there's no other way to know who decided to add the RSS feed to the room without using Scalar/Modular.

I'm not sure how best to handle this as it's clear that the definitions of what is clear and not are different for us. I'd argue that RSS Bot [@user:domain.com] is very clearly a bot, however someone trying to work around the disambiguation as TravisR [@travis:t2l.io] would be malicious.

There's also the concern of people using the display name to communicate account migration. For example, someone might set their display name to TravisR - Contact @travis:t2l.io instead to show that this account is dead. That would lead to a very cluttered and lengthy name, punishing people who aren't trying to be malicious.

Realistically, I feel a solution towards https://github.com/matrix-org/matrix-doc/issues/538 is better than forcing disambiguation as much as possible. I've yet to see a case where someone actually exploited the disambiguation algorithm. Now, that's not to say it's not a problem, it's just to illustrate that a proper disambiguation solution would be preferred in my eyes instead of making rooms hard to parse mentally.

@pafcu
Copy link
Contributor

pafcu commented Jan 4, 2018

I'd count both cases as "abusing" the display name by using it to display additional non-name information because there is no where else to put it. A simple solution would be to have a see_also event which could store your new mxid, who added the bot, etc. The meaning of this data could be left intentionally vague, just as it is in the name currently.

@lampholder
Copy link
Member

I think we've done ourselves a disservice by having the automatically-assigned nonoptional disambiguation text be appended to the display name as an unformatted, unstyled text string.

What if we styled the disambiguation suffix? Something like:
disambiguation_pill

(Not this exactly, since it muddles the pill semantics somewhat, but something like this.

@lampholder lampholder added T-Defect ui/ux P2 S-Major Severely degrades major functionality or product features, with no satisfactory workaround S-Tolerable Low/no impact on users labels Jan 11, 2018
@pafcu
Copy link
Contributor

pafcu commented Jan 11, 2018

@lampholder Definitely that too. However, I'm still a bit weary regarding solutions that require people to already know what disambiguation should look like. If I never (or rarely) see it, how would I know that it should look like a pill, rather than the way it now looks (especially since we've already used a particular style that would now suddenly be "dangerous").

I think it's much more worthwhile to fix @turt2live's underlying issue: The need to indicate a related mxid (i.e. not the actual mxid of the user) associated with a user. The problem would be solved by displaying this kind of information separately, which would prevent the "unnecessary" disambiguation, while still making spoofing difficult.

edit: Something like

image

where the text could be something like "Added by someone@example.org" for integrations.

@pafcu
Copy link
Contributor

pafcu commented Jan 11, 2018

Another way to avoid "safety disambiguation" because of the presence of an mxid in the name would be to display a small warning icon (instead of showing the full mxid), which on hover would explain what is going on, and that the mxid in the name might be there to trick you. This would make spoofing harder, while not adding the full mxid which some people might find "too much".

@turt2live
Copy link
Member Author

The pill-like disambiguation is quite distracting to me. @pafcu is right in that my problem now is that accounts which are not trying to impersonate users are victims of the algorithm, although I'd seriously recommend fixing disambiguation as a concept rather than just for integrations (as that's another can of worms).

Having user IDs in any way displayed is not a good solution to me: they are often cluttered or hard to read, particularly in rooms that are bridged. The display name is supposed to be front-facing to people. The disambiguation should also be done in a way where someone doesn't ask questions about how it works (most of these questions end up being sessions on how decentralized systems work - not something the average user is going to understand). I'm honestly not sure about what that would look like, but it is infuriating that many rooms are unreadable under the current semantics. Colors seems like the leading choice amoung the other issues, although it has the risk of colors becoming too close to distinguish. Some kind of identifier under the avatar may also work.

@pafcu
Copy link
Contributor

pafcu commented Jan 11, 2018

One possibility would be to automatically give the option to change your display name in a specific room, if there is already someone with that name there. This might reduce triggering disambiguation a bit. (Although not really relevant for integrations or this specific issue).

E.g. if your preferred name is John, and there is already a John in the room you enter, you might want to use the name John D. instead. Again, just as an option. If you really, really, want to be called just John, go ahead, but there might be disambiguation involved.

This might be good if you are the only John in your company (so you use that as your display name), but you end up in a conversation with a customer who is also called John.

edit:

Basically encourage people to use slightly different names in different rooms. This is different than how much chat systems work, so it might not even occur to people that this is possible.

@Half-Shot
Copy link
Member

This is a much greater problem now bridges are having to encode quite complex ids into a user id, for example:
example

The bridged user is already adding "(XMPP)" to separate it. If we absolutely must ignore bracketed strings, then at least shorten the userid a bit so it doesn't take up the whole line.

@t3chguy
Copy link
Member

t3chguy commented Jan 11, 2019

It shouldn't be ignoring bracketed strings, it should force disambiguation if there's anything that looks like an MXID in the displayname. I'm not sure that the screenshot shows the whole issue

@Half-Shot
Copy link
Member

it should force disambiguation if there's anything that looks like an MXID in the displayname

Neither "Half-Shot" or "Half-Shot (XMPP)" contain anything like an mxid. What are you missing from the screenshot? The memberlist of that room contains:

image

(The second user is doing the right thing in the memberlist, it's the third one that has the issue).

@neilisfragile
Copy link
Member

@ShadowJonathan
Copy link
Contributor

Isn't this technically fixed by #16897?

For the mention of the sidebar, there's also matrix-org/matrix-react-sdk#6328 in the works.

@t3chguy
Copy link
Member

t3chguy commented Jul 7, 2021

No, this issue is about the algorithm which decides when to kick in disambiguation being too sensitive (trigger happy)

@SimonBrandner SimonBrandner added A-Timeline O-Occasional Affects or can be seen by some users regularly or most users rarely and removed P2 S-Major Severely degrades major functionality or product features, with no satisfactory workaround labels Aug 22, 2021
@SimonBrandner SimonBrandner added S-Minor Impairs non-critical functionality or suitable workarounds exist and removed S-Tolerable Low/no impact on users labels Aug 22, 2021
@turt2live
Copy link
Member Author

Disambiguation is now different enough where I'm no longer concerned about this personally. If folks feel strongly otherwise, please open a new issue with fresh information for reconsideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Timeline O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Development

No branches or pull requests

9 participants