-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Properly throw errors on users management #16746
Conversation
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@@ -458,7 +459,7 @@ public function editUser(string $userId, string $key, string $value): DataRespon | |||
|
|||
$targetUser = $this->userManager->get($userId); | |||
if ($targetUser === null) { | |||
throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); | |||
throw new OCSException('Unknown user', 101); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be consistent with others methods on this page 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But now it reveals to anyone if a user exists, while before it required it to be your own user or you having subadmin/admin permissions
At least its documented ;) |
Lol, wrongly! It says |
@@ -638,7 +639,7 @@ public function deleteUser(string $userId): DataResponse { | |||
$targetUser = $this->userManager->get($userId); | |||
|
|||
if ($targetUser === null || $targetUser->getUID() === $currentLoggedInUser->getUID()) { | |||
throw new OCSException('', 101); | |||
throw new OCSException('Unknown user', 101); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unknown is not correct for the second part of the if condition
Nothing for 20 -> move to 21 |
The list of error codes is a mess btw, should we fix them as well?
What is the proper way of doing this? One error code per error type? Bad email 102, bad quota, 103.. etc?