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

Trust & Decoration | Missing mxId in the Banner for unverified user identity change (displayName+avatar is not enough) #2565

Closed
3 tasks done
BillCarsonFr opened this issue Oct 9, 2024 · 17 comments

Comments

@BillCarsonFr
Copy link
Member

BillCarsonFr commented Oct 9, 2024

Follow up of #2544

The current design only shows the user display name
image

If several users are using the same display name and no avatar it would be impossible to know which one it is talking about. Also a user could change it's display name and avatar to masqarade another user.

Show the MXID in bold and parentheses after the display name. In case display name is missing, omit the parentheses.

image

Designs:

Tasks

Preview Give feedback
  1. T-Task
    pixlwave
  2. T-Task
    bmarty
@BillCarsonFr
Copy link
Member Author

Clarification: It appears that this is the disambiguatedDisplayName that is used here. But anyhow we want also the mxid here.

@mxandreas
Copy link

@americanrefugee Could you please update the designs accordingly? I think you can then do it also for the verified identity change case if we are doing it anyway.

@BillCarsonFr Besides the example you listed - do you know if there is already some sort of de facto standard here? It would be trivial to just add the MXID in partentheses but would be nice to make sure it is identical in all cases.

disambiguatedDisplayName

Also, if this does not include the MXID then how does it make the display name unambiguous?

@bmarty
Copy link
Member

bmarty commented Oct 10, 2024

@mxandreas disambiguatedDisplayName may contain the mxId if the room have multiple users with the same display name.

If you read code, this is the Kotlin implementation: https://github.com/element-hq/element-x-android/blob/develop/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMember.kt#L43

    /**
     * Disambiguated display name for the RoomMember.
     * If the display name is null, the user ID is returned.
     * If the display name is ambiguous, the user ID is appended in parentheses.
     * Otherwise, the display name is returned.
     */
    val disambiguatedDisplayName: String = when {
        displayName == null -> userId.value // will be "alice:server.org"
        isNameAmbiguous -> "$displayName ($userId)" // will be "Alice (alice:server.org)"
        else -> displayName // will be "Alice"
    }

But it is definitely possible to always include the mxId. We will need to decide how it will render if the user does not have a display name.

Maybe something like:

    val displayNameWithMxId: String = when {
        displayName == null -> userId.value // will be "alice:server.org"
        else -> "$displayName ($userId)" // will be "Alice (alice:server.org)"
    }

@mxandreas
Copy link

Thanks for the clarifications! To me it seems the safest and most straightforward option to always include MXID in this scenario.

@bmarty
Copy link
Member

bmarty commented Oct 10, 2024

Clarification: It appears that this is the disambiguatedDisplayName that is used here. But anyhow we want also the mxid here.

@BillCarsonFr Can you change the issue title please?

@BillCarsonFr BillCarsonFr changed the title Trust & Decoration | Missing display name disambiguation in the Banner for unverified user identity change Trust & Decoration | Missing mxId in the Banner for unverified user identity change (displayName+avatar is not enough) Oct 10, 2024
@BillCarsonFr
Copy link
Member Author

Clarification: It appears that this is the disambiguatedDisplayName that is used here. But anyhow we want also the mxid here.

@BillCarsonFr Can you change the issue title please?

Done

@mxandreas
Copy link

mxandreas commented Oct 10, 2024

We had a short sync and the designs are updated accordingly:

Android sample:
image

@bmarty
Copy link
Member

bmarty commented Oct 10, 2024

What happen if the user does not have a display name? We repeat the user id like this?

alice:domain.org's (alice:domain.org) identity appears to have changed

It's fine to me, but please confirm.

Also please be aware that translation may look differently, for instance in French this will be something like:

L'identité de Alice (alice:domain.org) semble avoir changé.

Tech note: since there is a new template (for the userId) we will need to delete all the existing translations.

@mxandreas
Copy link

What happen if the user does not have a display name? We repeat the user id like this?
alice:domain.org's (alice:domain.org) identity appears to have changed

If that is an approach that we're already using somewhere, then I think this is preferred. @americanrefugee WDYT?

@ara4n
Copy link
Member

ara4n commented Oct 12, 2024

the idiomatic thing to do if the user has no displayname is to just show the matrix id (in the alternative font).

@americanrefugee
Copy link

If the user doesn't have a display name, maybe just show the Matrix ID in bold (no parentheses)? Like this:

([email protected])'s verified identity has changed.

@mxandreas
Copy link

mxandreas commented Oct 14, 2024

maybe just show the Matrix ID in bold

Generally this an approach that apps use in such cases - when something is missing they omit this in a graceful manner. However, I wonder if in this case it is better to "maintain the structure" (e.g. always have 2 fields - like Benoit suggested) to reduce the potential ways how one can trick somebody with this.

Having said that, it looks safe to start with just showing the MXID in bold (without parentheses) when display name is missing. We can improve later, unless anyone can immediately name any easy attacks.

@bmarty
Copy link
Member

bmarty commented Oct 14, 2024

Just a small tech note, we will need 2 different Localazy entries to manage all the cases here:

<displayName>'s (<userIdInBold>) identity appears to have changed

when a display name is available, and

<userIdInBold>'s identity appears to have changed

when there is no display name.

Not a big deal, but we take the risk that the strings are not translated (or even updated) the same way.

@andybalaam
Copy link
Member

@mxandreas will create tasks for the 3 platforms.

@mxandreas
Copy link

Created tasks for iOS, Android - linked above. For Web - mentioned it in #2565 as it is still in progress.

cc: @manuroe @andybalaam

@pixlwave pixlwave removed their assignment Oct 15, 2024
@manuroe
Copy link
Member

manuroe commented Oct 17, 2024

@BillCarsonFr for info, all the work is done on the EX side. Thanks @bmarty and @pixlwave ! There is no more EX team members assigned to this issue.

@mxandreas
Copy link

Closing this as iOS and Android are done, and for Web there is a separate issue for tracking this which has made it to the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants