-
-
Notifications
You must be signed in to change notification settings - Fork 834
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 cannot be numeric #1356
Comments
In my opinion, numeric usernames are allowed on most of software and disallow them is not the way. Maybe, when someone build an extension to integrate login with service "A" and this service allow numeric usernames, what will happen? The problem here, is not the numeric username, is how API works. |
Open to discussing alternatives, I think the main question is what should the API call be to retrieve a single user by their username? I guess it could be |
Wouldn't be enough to obtain user details by its id? |
Not always. Prime example is when you browse to a user page like http://discuss.flarum.org/u/Toby - we need to look up the user by their username! |
Right now, we are generating duplicate content with http://discuss.flarum.org/u/Toby and http://discuss.flarum.org/u/1. Shouldn't one of them redirect to the other one or at least a canonical URL direct search engines to the version to be indexed? |
Now my question changes to: Wouldn't be enough to obtain user details by its username? |
Sometimes you need to look up a user by their ID too. eg. when the API returns discussions/posts, they have relationships with their authors which specify a user ID. |
There could be a separate endpoint to lookup by username ? like Or tweak the existing endpoint to accept the username query with a different syntax. For example if you query It seems only profile page makes use of the endpoint with a username ? |
|
Not sure if this is the right place, but I think it's the closest we have: Ugh, I just gave this great article a good read - turns out there are some more things we need to consider in terms of username validation. And now is the best time to do so, because we can still break backwards compatibility. 😁 |
If you really think about breaking the backwards compatiblity please consider replacing the numeric IDs with something different - like UUIDs. It´s no good idea to have a public available system and to be able to identify all users in a row by guessing IDs. Also you can guess admin-IDs (will be the first ones). like: I could now write a script and grab all user ID´s & names. Currently this is not a huge problem - but consider flarum grows and the userpage will eventually someday show personal data like email-adress or real name or so.... Perhaps you could for now just disable access by ID? Because.... what is it good for? |
Oh... and we really would later need a user-flag like I´ll lock down my installation with fail2ban and use an external auth provider... but currently there is no protection of flooding password guesses. Ok,... that is an indirect feature-request ;-) |
+1 for disable member profile access by user ID link. +1 for |
I would prefer access by UUID instead of username... because even "admin" etc are all guessable... |
@cljk I don't think this issue is about hiding usernames. You do want usernames to be searchable on a forum (in particular they are the only identification in the profile url). The question is just how to offer both id and username based queries... I thought about this again and in my opinion the only good |
@clarkwinkelmann Alright I'm sold let's go with that 👍 |
I actually prefer the new endpoint, as filters seem to be the wrong tool, when they will only ever return one result (at most). 🙈 |
I have been in doubt as well mostly for the same reason as @franzliedke mentions. Also using the filter on the users endpoint could have the following unwanted side effect; users:
Result of the query will be two users not one. |
Oh I forgot it was only partially matching. What about adding a new filter for exact matching to the existing endpoint ? |
Wait, I didn't realize |
@franzliedke i think we're talking about the @clarkwinkelmann i'd like to propose the following then:
|
I don´t like the idea to present an API to NON-Admins that is able to SEARCH in a userbase. I find it problematic enough that the user-IDs ARE inremental (!) numeric and are guessable AND are presented in an API
Thanks god the API response doesn´t seem to contain email addresses. Then it would be a 10-liner to grab all email-adresses in you discussion-forum. To come back to the issue.... which was "usernames cannot be numeric" ... okay ... |
Using numeric IDs everywhere would definitely be the easiest I guess, but it's not as good looking ✨ As the reason both need to be usable via the API: clean urls with username as key, and numeric ID for all other API calls to improve performance. Regarding user enumeration I don't think numeric vs uuids vs whatever is relevant. As soon as user listing is enabled for guests or members, you can easily scan through every member and check their role anyway. And even if user listing is disabled you can also just crawl discussions via the API and check which users have posted something. @cljk if you don't like the information the API exposes you can easily alter it via an extension 😉 |
@clarkwinkelmann That has nothing to do with "liking an API" but with the problem of a possible data leak. And I don´t want to tweak a system to become secure. But I see... I´m the only one seeing this problem. So don´t care about me. |
Even if we used uuids for user IDs, you could still mine users from a forum by going through all of its discussions/posts. Regardless, that is not the issue at hand - that can be discussed elsewhere. The issue at hand is how to structure the API endpoint so that you can look up users by both their ID and their username. Currently we have As per the JSON-API spec, I still support Whereas |
Good enough for me. |
Just an observation from today: If you view the profile of the user 010101 at discuss.flarum.org, you will see his posts as expected. The discussions and mentions are shown correctly as well. The given URL is https://discuss.flarum.org/u/010101. If you, however, reload this page the same URL shows the user profile of the user dkio whose user ID seems to be 10101. As I understand it, the api call for the user profile must be different when called from within the application (URL is updated by the application) compared to loading the user profile URL directly. |
I experienced something similar to this. If you search for 010101 via search and go to his profile, it will launch, but clicking on its profile picture via the discussion list will give the red error dialog. |
@pflstr @TurboProgramming indeed that's the case. The behaviour you both mention was the reason for investigating the cause and this confirmed issue in the first place. |
I just encountered this problem on my forum and found the discussion here. If I understand correctly, the proposed solution would be to disable the possibility to view a profile page using |
Maybe a good compromise is usernames cannot start with numeric symbols (thinking about variable name restrictions in programming languages)? That should be enough to efficiently distinguish numeric strings and text strings? (Looking at the whole string should be better, but more complicated to implement perhaps?) |
@luceos you removed needs-discussion, are we going with shangjiaxuan's idea? |
This is the final solution: #1356 (comment) Clark's PR is a good solution for now. |
Fixed as a side effect of #2456 |
Currently username can be any matching
regex:/^[a-z0-9_-]+$/i
and the other rules (eg, min: 3). A drawback of this regex is that it allows for numeric usernames. As a consequence consider:User
Hitting
/api/user/1337 returns user 1337 for any other user.
What we have to do is disallow any numeric usernames, the UserValidator has to be modified. A consideration is what to do with existing users.
The text was updated successfully, but these errors were encountered: