feat: allow for ExApps to call Admin endpoints marked with specific attribute #46539
Conversation
|
@nickvergessen What do you think about that? |
|
Will have a look and answer tomorrow. |
There was a problem hiding this comment.
Since this opens a wide angle on our APIs, we should make sure that logging (and admin audit logging) is also aware of $this->userSession->getSession()->get('app_api') and properly logs that an action was performed "on behalf of a user" and not "by a user".
But be very careful in the logging so it never fails (catch throwable, etc.) as otherwise logging breaks completely.
lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
Outdated
Show resolved
Hide resolved
|
Additionally this would allow a malicious admin to escape |
…ttribute Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
bigcat88
force-pushed
the
feat/AppApiAdminAccessWithoutUser-attribute
branch
from
defc4e0
to
3aec0da
Compare
If I correctly understood the situation you describe, then the answer is probably “no”:
should this be done in a separate PR for cleanliness? |
Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
Might be a "no brainer", but I meant if they can install exapps they can now also perform admin actions on the server (on any level), by writing their own exapp and enabling it. |
If you can do it in a separate PR before beta1 next week? |
|
I will re-create this PR as I messed it with merging commits from master branch. |
1. Logs are written to a separate file 2. Log level - Warning 3. Only for those requests where "user" is set Reference: nextcloud/server#46539 (review) this is how logs look like: ```json {"reqId":"t9ThOI2CheVn6sDUsUKO","level":2,"time":"2024-08-13T10:08:42+00:00","remoteAddr":"192.168.65.1","user":"admin","app":"nc_py_api","method":"GET","url":"/ocs/v1.php/cloud/capabilities?format=json","message":"impersonation request","userAgent":"python-httpx/0.25.2","version":"30.0.0.7","data":{"app":"nc_py_api"}} {"reqId":"tVtHIEwQ5YKhbWUFTmKF","level":2,"time":"2024-08-13T10:08:42+00:00","remoteAddr":"192.168.65.1","user":"admin","app":"nc_py_api","method":"PROPFIND","url":"/remote.php/dav/files/admin","message":"impersonation request","userAgent":"python-httpx/0.25.2","version":"30.0.0.7","data":{"app":"nc_py_api"}} ``` Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Summary
We need this for the Workflow Engine Project, and not only.
The option of duplicating ednpoints where they do not require the admin flag to be set seems to us not quite the right solution; sometimes for ExApps you still need to check whether the user is an administrator or not.
Checklist