Prevent user with empty uid #1616
Conversation
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
|
@blizzz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975, @nickvergessen and @owncloud-bot to be potential reviewers. |
|
👍 |
| @@ -111,6 +112,12 @@ public function __construct($username, $dn, IUserTools $access, | |||
| IConfig $config, FilesystemHelper $fs, Image $image, | |||
| LogWrapper $log, IAvatarManager $avatarManager, IUserManager $userManager) { | |||
|
|
|||
| if ($username === null) { | |||
| throw new \InvalidArgumentException("uid for '$dn' must not be null!"); | |||
There was a problem hiding this comment.
We had enough of problems with user controllable input in the past. Is that user controllable in any way? Or sensitive?
If so we should just log the detailled error message and have an exception that doesn't include the user input.
There was a problem hiding this comment.
Display name should be user controlled, but since you can not login anymore, you would need to set the malicious displayname via a secondary system.
Then again, only an admin could ever create an empty user...
There was a problem hiding this comment.
This one here comes in via LDAP. If somehow the attribute you as admin chose returns an empty string (or something that fails to transliterate i assume, the original PR was not referring to anything) it could end up here with an empty username.
Tbh, I did not dig into the details, but seems like an OK check.
There was a problem hiding this comment.
@blizzz I guess this is about escaping or not showing "$dn" to the user via an error page ;)
There was a problem hiding this comment.
@blizzz Could you fix the stuff @LukasReschke requested? Thanks :)
MorrisJobke
added
2. developing
Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
added
3. to review
|
Updated, review time @MorrisJobke @LukasReschke |
|
Tested and setup of LDAP and loging in still works 👍 |
MorrisJobke
added
4. to release
Current coverage is 30.82% (diff: 0.00%)@@ master #1616 diff @@
==========================================
Files 1081 1087 +6
Lines 60017 61115 +1098
Methods 6808 7030 +222
Messages 0 0
Branches 0 0
==========================================
+ Hits 18377 18840 +463
- Misses 41640 42275 +635
Partials 0 0
|
Downstream of owncloud/user_ldap#6
@nextcloud/ldap @LukasReschke @nickvergessen