From d7907f3f8125438321f98d4e2202701f25b0619b Mon Sep 17 00:00:00 2001 From: Naomi Hidalgo Date: Thu, 21 Apr 2022 12:56:03 +0200 Subject: [PATCH] refactor(api,webapp): check database queries instead of user id --- .../api/libv2/api_desktops_nonpersistent.py | 31 ++++++++--- api/src/api/libv2/api_desktops_persistent.py | 55 ++++++++++++------- api/src/api/libv2/helpers.py | 8 ++- api/src/api/views/AdminMediaViews.py | 24 ++++++-- api/src/api/views/UsersView.py | 12 +++- api/src/api/views/decorators.py | 20 +++---- .../webapp/webapp/admin/views/decorators.py | 4 +- .../templates/admin/pages/users_modals.html | 2 +- webapp/webapp/webapp/views/decorators.py | 15 +++-- 9 files changed, 121 insertions(+), 50 deletions(-) diff --git a/api/src/api/libv2/api_desktops_nonpersistent.py b/api/src/api/libv2/api_desktops_nonpersistent.py index 819329023e..0c72511641 100644 --- a/api/src/api/libv2/api_desktops_nonpersistent.py +++ b/api/src/api/libv2/api_desktops_nonpersistent.py @@ -118,16 +118,33 @@ class ApiDesktopsNonPersistent: ds.WaitStatus(desktop_id, "Any", "Any", "Started") return desktop_id - def _nonpersistent_desktop_from_tmpl(self, user_id, category, group, template_id): + def _nonpersistent_desktop_from_tmpl(self, user_id, template_id): with app.app_context(): template = r.table("domains").get(template_id).run(db.conn) - if template == None: - raise Error("not_found", "Template not found", traceback.format_stack()) + if not template: + raise Error("not_found", "Template not found", traceback.format_stack()) + user = r.table("users").get(user_id).run(db.conn) + if not user: + raise Error("not_found", "NewNonPersistent: user id not found.") + group = r.table("groups").get(user["group"]).run(db.conn) + if not group: + raise Error("not_found", "NewNonPersistent: group id not found.") + timestamp = time.strftime("%Y%m%d%H%M%S") parsed_name = (timestamp + "-" + _parse_string(template["name"]))[:40] parent_disk = template["hardware"]["disks"][0]["file"] - dir_disk = "volatiles/" + category + "/" + group + "/" + user_id + + dir_disk = "/".join( + ( + "volatiles", + user["category"], + group["uid"], + user["provider"], + user["username"], + ) + ) + disk_filename = parsed_name + ".qcow2" create_dict = template["create_dict"] @@ -145,11 +162,11 @@ class ApiDesktopsNonPersistent: "description": template["description"], "kind": "desktop", "user": user_id, - "username": user_id.split("-")[-1], + "username": user["username"], "status": "CreatingAndStarting", "detail": None, - "category": category, - "group": group, + "category": user["category"], + "group": user["group"], "xml": None, "icon": template["icon"], "image": template["image"], diff --git a/api/src/api/libv2/api_desktops_persistent.py b/api/src/api/libv2/api_desktops_persistent.py index efe59a2d20..6e8940c5ef 100644 --- a/api/src/api/libv2/api_desktops_persistent.py +++ b/api/src/api/libv2/api_desktops_persistent.py @@ -64,38 +64,57 @@ class ApiDesktopsPersistent: ): with app.app_context(): template = r.table("domains").get(template_id).run(db.conn) - if template == None: - raise Error("not_found", "Template not found", traceback.format_stack()) + if not template: + raise Error("not_found", "Template not found", traceback.format_stack()) + user = r.table("users").get(payload["user_id"]).run(db.conn) + if not user: + raise Error("not_found", "NewFromTemplate: user id not found.") + group = r.table("groups").get(user["group"]).run(db.conn) + if not group: + raise Error("not_found", "NewFromTemplate: group id not found.") parsed_name = _parse_string(desktop_name) + new_desktop_id = "_" + payload["user_id"] + "-" + parsed_name + with app.app_context(): + if r.table("domains").get(new_desktop_id).run(db.conn): + raise Error( + "conflict", + "NewFromTemplate: user already has a desktop with the same id.", + ) parent_disk = template["hardware"]["disks"][0]["file"] dir_disk = "/".join( ( payload["category_id"], - payload["group_id"].replace(f"{payload['category_id']}-", ""), - payload["user_id"].split("-", 1)[0], - "-".join(payload["user_id"].rsplit("-", 2)[-2:]), + group["uid"], + user["provider"], + user["username"], ) ) + disk_filename = parsed_name + ".qcow2" create_dict = template["create_dict"] create_dict["hardware"]["disks"] = [ {"file": dir_disk + "/" + disk_filename, "parent": parent_disk} ] - create_dict = _parse_media_info(create_dict) + try: + create_dict = _parse_media_info(create_dict) + except: + raise Error( + "internal_server", "NewFromTemplate: unable to parse media info." + ) if "interfaces_mac" in create_dict["hardware"].keys(): create_dict["hardware"].pop("interfaces_mac") new_desktop = { - "id": "_" + payload["user_id"] + "-" + parsed_name, + "id": new_desktop_id, "name": desktop_name, "description": desktop_description, "kind": "desktop", "user": payload["user_id"], - "username": payload["user_id"].split("-")[-1], + "username": user["username"], "status": "Creating", "detail": None, "category": payload["category_id"], @@ -126,18 +145,16 @@ class ApiDesktopsPersistent: new_desktop = {**new_desktop, **deployment} with app.app_context(): - if r.table("domains").get(new_desktop["id"]).run(db.conn) == None: - if ( - _check( - r.table("domains").insert(new_desktop).run(db.conn), "inserted" - ) - == False - ): - raise NewDesktopNotInserted - else: - return new_desktop["id"] + if ( + _check(r.table("domains").insert(new_desktop).run(db.conn), "inserted") + == False + ): + raise Error( + "internal_server", + "NewFromTemplate: unable to insert new desktop in database", + ) else: - raise DesktopExists + return new_desktop_id def NewFromScratch( self, diff --git a/api/src/api/libv2/helpers.py b/api/src/api/libv2/helpers.py index 48e11eb2de..ea3a7539f6 100644 --- a/api/src/api/libv2/helpers.py +++ b/api/src/api/libv2/helpers.py @@ -287,6 +287,12 @@ def generate_db_media(path_downloaded, filesize): "Skipping uploaded file as has unknown extension", traceback.format_stack(), ) + + with app.app_context(): + username = r.table("users").get(parts[-2])["username"].run(db.conn) + if username == None: + raise Error("not_found", "Username not found", traceback.format_stack()) + return { "accessed": time.time(), "allowed": { @@ -330,5 +336,5 @@ def generate_db_media(path_downloaded, filesize): + parts[-5] + "-" + parts[-2], # "local-default-admin-admin" , - "username": parts[-2].split("-")[1], + "username": username, } diff --git a/api/src/api/views/AdminMediaViews.py b/api/src/api/views/AdminMediaViews.py index ec9beb1de1..af4c347b7b 100644 --- a/api/src/api/views/AdminMediaViews.py +++ b/api/src/api/views/AdminMediaViews.py @@ -5,6 +5,7 @@ import time import traceback from flask import request +from rethinkdb import RethinkDB #!flask/bin/python # coding=utf-8 @@ -12,9 +13,14 @@ from api import app from ..libv2.api_admin import admin_table_insert from ..libv2.api_exceptions import Error +from ..libv2.flask_rethink import RDB from ..libv2.validators import _validate_item from .decorators import is_admin_or_manager +r = RethinkDB() +db = RDB(app) +db.init_app(app) + # Add media @app.route("/api/v3/admin/media", methods=["POST"]) @@ -26,10 +32,18 @@ def api_v3_admin_media_insert(payload): raise Error( "bad_request", "Unable to parse body data.", traceback.format_stack() ) - log.error(payload) + + with app.app_context(): + username = r.table("users").get(payload["user_id"])["username"].run(db.conn) + uid = r.table("users").get(payload["user_id"])["uid"] + if username == None: + raise Error("not_found", "User not found", traceback.format_stack()) + group = r.table("groups").get(payload["group_id"])["uid"].run(db.conn) + if group == None: + raise Error("not_found", "Group not found", traceback.format_stack()) data["user"] = payload["user_id"] - data["username"] = payload["user_id"].split("-")[3] + data["username"] = username data["category"] = payload["category_id"] data["group"] = payload["group_id"] data["url-web"] = data["url"] @@ -40,13 +54,13 @@ def api_v3_admin_media_insert(payload): urlpath = ( data["category"] + "/" - + data["group"].split("-")[1] + + group + "/" + payload["provider"] + "/" - + data["user"].split("-")[2] + + uid + "-" - + data["user"].split("-")[3] + + username + "/" + data["name"].replace(" ", "_") ) diff --git a/api/src/api/views/UsersView.py b/api/src/api/views/UsersView.py index 0525efac15..06af01d689 100644 --- a/api/src/api/views/UsersView.py +++ b/api/src/api/views/UsersView.py @@ -12,6 +12,7 @@ import traceback from uuid import uuid4 from flask import jsonify, request +from rethinkdb import RethinkDB #!flask/bin/python # coding=utf-8 @@ -21,6 +22,11 @@ from ..libv2.api_exceptions import Error from ..libv2.quotas import Quotas quotas = Quotas() +from ..libv2.flask_rethink import RDB + +r = RethinkDB() +db = RDB(app) +db.init_app(app) from ..libv2.api_users import ApiUsers, check_category_domain @@ -157,13 +163,17 @@ def api_v3_user_delete(payload): @app.route("/api/v3/user/templates", methods=["GET"]) @has_token def api_v3_user_templates(payload): + with app.app_context(): + group = r.table("groups").get(payload["group_id"])["uid"].run(db.conn) + if group == None: + raise Error("not_found", "Group not found", traceback.format_stack()) templates = users.Templates(payload) dropdown_templates = [ { "id": t["id"], "name": t["name"], "category": t["category"], - "group": t["group"].split("-")[1], + "group": group, "user_id": t["user"], "user_name": t["username"], "icon": t["icon"], diff --git a/api/src/api/views/decorators.py b/api/src/api/views/decorators.py index 059287425d..9e887c48a8 100644 --- a/api/src/api/views/decorators.py +++ b/api/src/api/views/decorators.py @@ -149,10 +149,11 @@ def is_hyper(f): def ownsUserId(payload, user_id): if payload["role_id"] == "admin": return True - if ( - payload["role_id"] == "manager" - and user_id.split("-")[1] == payload["category_id"] - ): + if payload["role_id"] == "manager": + with app.app_context(): + category = r.table("users").get(payload["user_id"])["category"].run(db.conn) + if category == payload["category_id"]: + return True return True if payload["user_id"] == user_id: return True @@ -196,12 +197,11 @@ def ownsDomainId(payload, desktop_id): ).startswith(payload["user_id"]): return True # User is manager and the desktop is from its categories - if ( - payload["role_id"] == "manager" - and payload["category_id"] == desktop_id.split("-")[1] - ): - return True - + if payload["role_id"] == "manager": + with app.app_context(): + category = r.table("users").get(payload["user_id"])["category"].run(db.conn) + if category == payload["category_id"]: + return True # User is admin if payload["role_id"] == "admin": return True diff --git a/webapp/webapp/webapp/admin/views/decorators.py b/webapp/webapp/webapp/admin/views/decorators.py index 48fc3bd243..a49a4cdc32 100644 --- a/webapp/webapp/webapp/admin/views/decorators.py +++ b/webapp/webapp/webapp/admin/views/decorators.py @@ -81,7 +81,9 @@ def ownsidortag(fn): id = myargs["pk"] except: id = myargs["id"] - if current_user.role == "manager" and current_user.category == id.split("-")[1]: + with app.app_context(): + category = r.table("users").get(id)["category"].run(db.conn) + if current_user.role == "manager" and current_user.category == category: return fn(*args, **kwargs) if current_user.role == "advanced": with app.app_context(): diff --git a/webapp/webapp/webapp/templates/admin/pages/users_modals.html b/webapp/webapp/webapp/templates/admin/pages/users_modals.html index 847539dfbf..8ef2a310a2 100644 --- a/webapp/webapp/webapp/templates/admin/pages/users_modals.html +++ b/webapp/webapp/webapp/templates/admin/pages/users_modals.html @@ -32,7 +32,7 @@
- +
diff --git a/webapp/webapp/webapp/views/decorators.py b/webapp/webapp/webapp/views/decorators.py index 2ed16b59a3..b2fdaaf10c 100644 --- a/webapp/webapp/webapp/views/decorators.py +++ b/webapp/webapp/webapp/views/decorators.py @@ -42,10 +42,13 @@ def ownsid(fn): id = myargs["pk"] except: id = myargs["id"] - if id.startswith("_" + current_user.id) or ( - current_user.role == "manager" and current_user.category == id.split("-")[1] - ): - return fn(*args, **kwargs) + if id.startswith("_" + current_user.id): + + with app.app_context(): + category = r.table("users").get(id)["category"].run(db.conn) + + if current_user.role == "manager" and current_user.category == category: + return fn(*args, **kwargs) logout_user() return render_template("login_category.html", category=False) @@ -69,7 +72,9 @@ def ownsidortag(fn): id = myargs["pk"] except: id = myargs["id"] - if current_user.role == "manager" and current_user.category == id.split("-")[1]: + with app.app_context(): + category = r.table("users").get(id)["category"].run(db.conn) + if current_user.role == "manager" and current_user.category == category: return fn(*args, **kwargs) with app.app_context(): domain = r.table("domains").get(id).pluck("id", "tag").run(db.conn) -- GitLab