Unexpected user ID shown instead of name

fetimo commented 1 year ago

How to use GitHub

Please use the 👍 reaction to show that you are affected by the same issue.
Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
Subscribe to receive notifications on status change and new comments.

Hi, apologies if this is a duplicate issue, I have looked but couldn't see anything that looked similar.

Steps to reproduce

Create a SAML user
Write a message to them

Expected behaviour

The message preview for a user what is sent should be consistent. E.g. when I @[person] I expect the message to show the same, rather than be replaced by their user ID.

Preview Capture-2023-07-14-134418

Sent Capture-2023-07-14-134452

Actual behaviour

The message is not consistent with the preview.

Talk app

Talk app version: 15.0.6

Custom Signaling server configured: yes

Custom TURN server configured: yes

Custom STUN server configured: yes

Browser

Microphone available: yes

Camera available: yes

Operating system: macOS

Browser name: Chrome

Browser version: 114

Nextcloud Version: 25.0.7 Enterprise

nickvergessen commented 1 year ago

I'm not aware that | is a valid character for user ids: https://github.com/nextcloud/server/blob/346344c15371597ec825059abdf7564da9334b7e/lib/private/User/Manager.php#L710-L715

So it also is not matched by the regular expression later on that tries to match mentions:

https://github.com/nextcloud/server/blob/7aa97dcc2369c421b6d32ad32c33d357e582af4a/lib/private/Comments/Comment.php#L222

fetimo commented 1 year ago

Interesting, thanks for the quick reply Nick! I'm not familiar with this code but I guess that SAML users don't have the same restrictions? What do you suggest in this scenario?

nickvergessen commented 1 year ago

The auth0/saml user backend could change them before creating the users in Nextcloud. Or someone sends a PR to all place

nickvergessen commented 1 year ago

Moving to server therefore

szaimen commented 1 year ago

Which NC version?

nickvergessen commented 1 year ago

Also happens on master. The problem is the auth0 user backend contains | which is not matched by the regex when finding mentions in comments and anything else because the user default user backends don't support the character and it's not listed as supported in the manager.

The question is whether we want to extend the character list and allow it, or not, in which vase the auth0 backend should try to change it's account pattern and migrate existing users over

nextcloud / server