all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH backup] REST server: avoid hard coding world readable API endpoints
@ 2020-10-02 11:17 Thomas Lamprecht
  2020-10-05  6:33 ` [pbs-devel] applied: " Dietmar Maurer
  0 siblings, 1 reply; 2+ messages in thread
From: Thomas Lamprecht @ 2020-10-02 11:17 UTC (permalink / raw)
  To: pbs-devel

while we probably do not add much more to them, it still looks ugly.

If this was made so that adding a World readable API call is "hard"
and not done by accident, it rather should be done as a test on build
time. But, IMO, the API permission schema definitions are easy to
review, and not often changed/added - so any wrong World readable API
call will normally still caught.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

the need to method.clone() to make the borrow checker happy isn't to nice,
but figured it isn't to expensive, maybe there's a nicer way..

 src/server/rest.rs | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/server/rest.rs b/src/server/rest.rs
index feb0b462..4cfdd6c0 100644
--- a/src/server/rest.rs
+++ b/src/server/rest.rs
@@ -22,6 +22,7 @@ use proxmox::api::{
     ApiHandler,
     ApiMethod,
     HttpError,
+    Permission,
     RpcEnvironment,
     RpcEnvironmentType,
     check_api_permission,
@@ -546,13 +547,16 @@ pub async fn handle_request(api: Arc<ApiConfig>, req: Request<Body>) -> Result<R
             };
 
             let mut uri_param = HashMap::new();
+            let api_method = api.find_method(&components[2..], method.clone(), &mut uri_param);
 
-            if comp_len == 4 && components[2] == "access" && (
-                (components[3] == "ticket" && method ==  hyper::Method::POST) ||
-                (components[3] == "domains" && method ==  hyper::Method::GET)
-            ) {
-                // explicitly allow those calls without auth
-            } else {
+            let mut auth_required = true;
+            if let Some(api_method) = api_method {
+                if let Permission::World = *api_method.access.permission {
+                    auth_required = false; // no auth for endpoints with World permission
+                }
+            }
+
+            if auth_required {
                 let (ticket, token, _) = extract_auth_data(&parts.headers);
                 match check_auth(&method, &ticket, &token, &user_info) {
                     Ok(userid) => rpcenv.set_user(Some(userid.to_string())),
@@ -565,7 +569,7 @@ pub async fn handle_request(api: Arc<ApiConfig>, req: Request<Body>) -> Result<R
                 }
             }
 
-            match api.find_method(&components[2..], method, &mut uri_param) {
+            match api_method {
                 None => {
                     let err = http_err!(NOT_FOUND, "Path '{}' not found.", path);
                     return Ok((formatter.format_error)(err));
-- 
2.27.0





^ permalink raw reply	[flat|nested] 2+ messages in thread

* [pbs-devel] applied: [PATCH backup] REST server: avoid hard coding world readable API endpoints
  2020-10-02 11:17 [pbs-devel] [PATCH backup] REST server: avoid hard coding world readable API endpoints Thomas Lamprecht
@ 2020-10-05  6:33 ` Dietmar Maurer
  0 siblings, 0 replies; 2+ messages in thread
From: Dietmar Maurer @ 2020-10-05  6:33 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Thomas Lamprecht

applied




^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-10-05  6:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-02 11:17 [pbs-devel] [PATCH backup] REST server: avoid hard coding world readable API endpoints Thomas Lamprecht
2020-10-05  6:33 ` [pbs-devel] applied: " Dietmar Maurer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal