-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Increase username flexibility/privacy with mentions and user profile #254
Comments
@tobscure you might have an opinion on this as you might have already ran into this with that client, maybe I missed something that core allows here. |
My feeling is that usernames should always be considered public, they are like a friendly "user ID". We could add an option to only allow logging in via email address for enhanced security? |
Doesn't that complicate the idea of display- vs username? Isn't displayname supposed to be public, not the other way around? Using email as sole authorization might not be the best bet either, communities might allow multiple users with the same mail address (role playing boards for instance). |
Not necessarily, the distinction is not intended to be public vs. private but just machine-friendly (something that can be used for @mentions - eg. Toby001) vs human-friendly (eg. a full name like Toby Zerner)
I don't know if we should allow this, people can always sign up multiple accounts by using |
I think we differ in opinion where we talk about "we should allow this", in my opinion I would formulate this "we should allow this by default". Display name really feels like something publicly usable and visible, whereas username is something that could/should be private (by default). |
If username is private, how do you @mention a user? Typing their display name is not an option as it could have whitespace in it. The other option is to use their user ID (@123) but that's not very nice. (One other option is to implement a rich text editor by default which can do fancy highlighted @mentions like Facebook does them, showing the display name but storing the user ID internally, but that is very complicated and a large amount of dev work.) Hence why I think we should just keep it simple, make usernames always public, and display names are just an extra thing on top of that if you want to allow people to show their full name etc. |
Let's agree to disagree 👍 To me the Facebook way of mentioning is what I had in mind. We can always revisit this later. |
Actually I agree if we were to implement that then we no longer need usernames to be public. It's just a challenging thing to implement, but we should aim for it in the long-term. |
So basically, we have three things here, right?
Does this cover all the aspects mentioned here and in related issues? |
According to our code, a login identifier is either a username or email address. So your summary requires specification. I personally think both of them should be kept private. |
I think it'd be best to use Discord's approach, where the actual content of mentions is |
If we're changing this, should we consider storing it as <@U:ID> to provide flexibility for group mentions down the line? |
Can we unparse text content? @<"Daniel Klabbers"(12434234324)> potential format? |
Trying to catch up with this issue. Whichever format we choose, it'll essentially be what the user sees in the editor correct ? I think we should probably aim for the most user friendly format possible, couldn't we go for something more simple like: |
That format makes sense to me, I think I was being a bit too safe with my suggestion. As long as it can be parsed (which surrounding the display name part with parentheses accomplishes) it should be good. One thing I'd like to make sure is that the format we choose hypothetically supports groups, but what you proposed should work: @g"Mods"#12 (or some other modification that either goes before the hash or after the @. |
I think that works well enough. I can provide regex for those if it's needed, in my head at the moment regex seems like the easiest method to parse that? |
We'll just need to modify the current system, which uses regex yes. |
Yeah, for groups I would personally go for something like
I'm sure there is regex involved yes, so feel free! So essentially this is what needs to be done: Mentions:
Slugs
Other
Anything I'm missing ? |
Why? Isn't the whole point of this that we include the ID in mentions so that we can entirely rely on that, and an arbitrary display name can be injected? Also, one thing we're missing here is a syntax for post mentions. Not sure if 2 pound signs in a row would be that good a UI. |
I didn't say we'd be relying on it for mentions, it's only a nice-to-have since hiding usernames will mean using the ID only slug for URLs, it's not a necessity so we don't have to do it for now, but it's easy to implement anyway.
2 pound signs ? what's that.. |
Thing is, nicknames are not required to be unique. Even if that option is enabled, it's possible that non unique nicknames have been used.
Hash signs (#), sorry. This symbol has too many names lol. Rigjt now post mentions are @username#POSTID. If we switch to the proposed system, we'd get @"Display Name"#UserID#PostID |
Why would we need to recalculate every mentioned user on every post load? I didn't say anything about recalculating. The post content will contain its ID, and the post_mentions table contains the relationship - the users are loaded with the post now, and so we'd use that information to display it to the user. |
Ah, so we store it as just the user ID on the backend and then convert it to include the display name as we send it to the front-end? I don't think that we should start adding display names that stay the same if a user changed username. That seems like an easy way to confuse people looking at older posts. If I changed my name to MrJeeves on a forum, I'd expect previous mentions to update too. If it has davwheat in some places and MrJeeves in others, that's asking for confusion. |
I have confirmed from testing that the displayed mention will automatically update when the display name is changed. It needs an adjustment to handle deleted users, but that should be pretty easy. |
When you talk about displayed you're talking about a normal post right? If the user edits the post though, don't they see their original text with an old username/display name? |
If that's how that works, what's the point of including the display name in the first place? Can't that be easily fetched using the user ID? |
I think for the user to know who they're mentioning, but we'd want to parse this out in the DB and then replace the mention w/ USER ID with the syntax we choose for mentions when showing he post content when editing (or hitting the endpoint for normal |
Ah, I see what you mean now. Yeah that's something we'd want to adjust in the new system. |
@askvortsov1 Yeah, sorry I explained myself poorly. I realized late that we were talking about different things - rendered post VS editing. |
So can't we just store the mention with the user ID in the DB, then add in their current display name when we send it off to the front end? Would that be too difficult? |
We'd need some way to hook into the unparsing process. We already wrap unparsing in our own method (https://github.com/flarum/core/blob/d642fb531c1335c33ff7515da367f6815ea93b12/src/Formatter/Formatter.php#L97-L106), so we could insert an extender there, allow a chance to edit the raw XML before proceeding? The utils we use for rendering should work there too. |
Just chiming in to say that this is also a privacy concern. If you are requested to remove PII for a user there's currently no way to make the username disappear from mentions, which could be the full name of the person, for example when using Facebook login. |
Absolutely agreed. In planning discussions for the GDPR extension's erasure feature this was discussed as a necessity, but if we can get it implemented in the mention extension itself (which we should be able to do), that'd be even cleaner. |
Assuming someone has a display name of |
We could just escape double quotes, maybe? |
I'm afraid that would make the regular expressions more complicated because of how complex escaping really is, we'd also have to account for escaping at the start and at the end of the real double quotes. |
Since the User serializer now contains slug and display name, would the next step be here? I suppose we should stop serializing the username unless the actor has the edit credentials permission, but there are still several bits that rely on that, such as the post/discussion filters. |
Yes, what would we replace the username filter with ? I think the slug is what makes sense here ? |
Slug makes sense because it's a unique identifier, but it's not ideal because we'd have to make one query per-username, whereas currently we can wrap everything up into a single query. |
Would something like user:"Display Name" not work? |
Display names are not a one to one relation. Also, that still has the issue of needing to query users individually. What we could do is support both ID and slug, and convert any slugs to IDs? Alternatively, people are unlikely to search for THAT many authors at once, so the multiple queries could be fine for slug. |
Feature Request
Is your feature request related to a problem? Please describe.
When you have a forum that tries to protect usernames (as they are used for logging in too) by adding the display_name (eg I've migrated that column onto the users table), the profile url and mentioning logic should be able to use that column instead of the username column.
Describe the solution you'd like
I'm pretty blocked right now and would like your input for the best solution:
Justify why this feature belongs in Flarum's core, rather than in a third-party extension
Ease of configuration and protecting user privacy. This might also relate to the PR Toby did flarum/framework#1721 (see point two in flarum/framework#1721 (comment))
Describe alternatives you've considered
🤷♀️
The text was updated successfully, but these errors were encountered: