From ba8737f007210065b032816175b63069b5908ed7 Mon Sep 17 00:00:00 2001 From: amcmanu3 Date: Tue, 16 Jan 2024 19:21:00 -0500 Subject: [PATCH 1/3] Refactor subpage perm checks Fix bug where subpages people don't still have access to are retained --- app/classes/web/panel_handler.py | 150 +++++++++---------------------- 1 file changed, 43 insertions(+), 107 deletions(-) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index 8ac827c3..868bd5a3 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -28,6 +28,17 @@ from app.classes.web.base_handler import BaseHandler from app.classes.web.webhooks.webhook_factory import WebhookFactory logger = logging.getLogger(__name__) +SUBPAGE_PERMS = { + "term": EnumPermissionsServer.TERMINAL, + "logs": EnumPermissionsServer.LOGS, + "schedules": EnumPermissionsServer.SCHEDULE, + "backup": EnumPermissionsServer.BACKUP, + "files": EnumPermissionsServer.FILES, + "config": EnumPermissionsServer.CONFIG, + "admin_controls": EnumPermissionsServer.PLAYERS, + "metrics": EnumPermissionsServer.LOGS, + "webhooks": EnumPermissionsServer.CONFIG, +} class PanelHandler(BaseHandler): @@ -138,6 +149,11 @@ class PanelHandler(BaseHandler): # increasing and will eat up the RAM del chunk + def check_subpage_perms(self, user_perms, subpage): + if SUBPAGE_PERMS.get(subpage, False) in user_perms: + return True + return False + def check_server_id(self): server_id = self.get_argument("id", None) @@ -489,8 +505,17 @@ class PanelHandler(BaseHandler): server_id = self.check_server_id() # load page the user was on last server_subpage = self.controller.servers.server_subpage.get(server_id, "") - if subpage == "" and server_subpage != "": - subpage = self.controller.servers.server_subpage.get(server_id, "") + if ( + subpage == "" + and server_subpage != "" + and self.check_subpage_perms( + self.controller.server_perms.get_user_id_permissions_list( + exec_user["user_id"], server_id + ), + server_subpage, + ) + ): + subpage = server_subpage else: self.controller.servers.server_subpage[server_id] = subpage if server_id is None: @@ -502,16 +527,6 @@ class PanelHandler(BaseHandler): page_data["backup_failed"] = server_obj.last_backup_status() server_obj = None - valid_subpages = [ - "term", - "logs", - "backup", - "config", - "files", - "admin_controls", - "schedules", - "metrics", - ] if not self.failed_server: server = self.controller.servers.get_server_instance_by_id(server_id) # server_data isn't needed since the server_stats also pulls server data @@ -577,7 +592,6 @@ class PanelHandler(BaseHandler): page_data["get_players"] = server.get_server_players() else: page_data["get_players"] = [] - page_data["active_link"] = subpage page_data["permissions"] = { "Commands": EnumPermissionsServer.COMMANDS, "Terminal": EnumPermissionsServer.TERMINAL, @@ -601,83 +615,33 @@ class PanelHandler(BaseHandler): page_data["server_stats"][ "server_type" ] = self.controller.servers.get_server_type_by_id(server_id) - if subpage not in valid_subpages: - logger.debug("not a valid subpage") + if not subpage: - if ( - page_data["permissions"]["Terminal"] - in page_data["user_permissions"] - ): - subpage = "term" - elif page_data["permissions"]["Logs"] in page_data["user_permissions"]: - subpage = "logs" - elif ( - page_data["permissions"]["Schedule"] - in page_data["user_permissions"] - ): - subpage = "schedules" - elif ( - page_data["permissions"]["Backup"] in page_data["user_permissions"] - ): - subpage = "backup" - elif page_data["permissions"]["Files"] in page_data["user_permissions"]: - subpage = "files" - elif ( - page_data["permissions"]["Config"] in page_data["user_permissions"] - ): - subpage = "config" - elif ( - page_data["permissions"]["Players"] in page_data["user_permissions"] - ): - subpage = "admin_controls" - else: + for spage, perm in SUBPAGE_PERMS.items(): + if perm in page_data["user_permissions"]: + subpage = spage + break + # If we still don't have a subpage we're going to assume they + # have no perms + if not subpage: self.redirect("/panel/error?error=Unauthorized access to Server") + page_data["active_link"] = subpage logger.debug(f'Subpage: "{subpage}"') - if subpage == "term": - if ( - not page_data["permissions"]["Terminal"] - in page_data["user_permissions"] - ): - if not superuser: - self.redirect( - "/panel/error?error=Unauthorized access to Terminal" - ) - return - - if subpage == "logs": - if ( - not page_data["permissions"]["Logs"] - in page_data["user_permissions"] - ): - if not superuser: - self.redirect("/panel/error?error=Unauthorized access to Logs") - return + if ( + not self.check_subpage_perms(page_data["user_permissions"], subpage) + and not superuser + ): + return self.redirect( + f"/panel/error?error=Unauthorized access to {subpage}" + ) if subpage == "schedules": - if ( - not page_data["permissions"]["Schedule"] - in page_data["user_permissions"] - ): - if not superuser: - self.redirect( - "/panel/error?error=Unauthorized access To Schedules" - ) - return page_data["schedules"] = HelpersManagement.get_schedules_by_server( server_id ) if subpage == "config": - if ( - not page_data["permissions"]["Config"] - in page_data["user_permissions"] - ): - if not superuser: - self.redirect( - "/panel/error?error=Unauthorized access Server Config" - ) - return page_data["java_versions"] = Helpers.find_java_installs() server_obj: Servers = self.controller.servers.get_server_obj(server_id) page_data["failed"] = self.failed_server @@ -691,26 +655,7 @@ class PanelHandler(BaseHandler): page_java.append(version) page_data["java_versions"] = page_java - - if subpage == "files": - if ( - not page_data["permissions"]["Files"] - in page_data["user_permissions"] - ): - if not superuser: - self.redirect("/panel/error?error=Unauthorized access Files") - return - if subpage == "backup": - if ( - not page_data["permissions"]["Backup"] - in page_data["user_permissions"] - ): - if not superuser: - self.redirect( - "/panel/error?error=Unauthorized access to Backups" - ) - return server_info = self.controller.servers.get_server_data_by_id(server_id) page_data[ "backup_config" @@ -761,15 +706,6 @@ class PanelHandler(BaseHandler): server_id, hours=(days * 24) ) if subpage == "webhooks": - if ( - not page_data["permissions"]["Config"] - in page_data["user_permissions"] - ): - if not superuser: - self.redirect( - "/panel/error?error=Unauthorized access to Webhooks Config" - ) - return page_data[ "webhooks" ] = self.controller.management.get_webhooks_by_server( From 02fbc5056d09ebfed408e790e6b9e2e47b2b70c3 Mon Sep 17 00:00:00 2001 From: amcmanu3 Date: Sun, 28 Jan 2024 17:15:20 -0500 Subject: [PATCH 2/3] Redirect 404 on non-existant page --- app/classes/web/panel_handler.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index 868bd5a3..02a5dbc1 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -28,6 +28,7 @@ from app.classes.web.base_handler import BaseHandler from app.classes.web.webhooks.webhook_factory import WebhookFactory logger = logging.getLogger(__name__) +# You must put any new subpages in here SUBPAGE_PERMS = { "term": EnumPermissionsServer.TERMINAL, "logs": EnumPermissionsServer.LOGS, @@ -625,6 +626,14 @@ class PanelHandler(BaseHandler): # have no perms if not subpage: self.redirect("/panel/error?error=Unauthorized access to Server") + if subpage not in SUBPAGE_PERMS.keys(): + self.set_status(404) + page_data["background"] = self.controller.cached_login + return self.render( + "public/404.html", + data=page_data, + translate=self.translator.translate, + ) page_data["active_link"] = subpage logger.debug(f'Subpage: "{subpage}"') From 71ab03c614f7a51a17157abde1cac8f6a80d358a Mon Sep 17 00:00:00 2001 From: Zedifus Date: Sun, 28 Jan 2024 23:11:47 +0000 Subject: [PATCH 3/3] Update changelog !695 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35257c29..3645c9ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ### New features - Use Papermc Group's API for `paper` & `folia` builds in server builder ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/688)) - Allow omission of player count from Dashboard (e.g. for proxy servers) ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/692)) +### Refactor +- Refactor subpage perm checks ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/695)) ### Bug fixes - Fix bukkit and downstream fork MOTD crash ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/686)) - Fix bug where invalid server Id leads to stack ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/690))