From fd0da1ef203ab4a43990e944570ae4a88c6b0868 Mon Sep 17 00:00:00 2001 From: Andrew Date: Sat, 18 Jun 2022 16:20:57 -0400 Subject: [PATCH 1/9] Fix any user can recieve all api keys --- app/classes/web/panel_handler.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index bb44138f..4f6dfe87 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -1926,6 +1926,12 @@ class PanelHandler(BaseHandler): self.redirect("/panel/error?error=Invalid Key ID") return + if key.user_id != exec_user["user_id"]: + self.redirect( + "/panel/error?error=You are not authorized to access this key." + ) + return + self.controller.management.add_to_audit_log( exec_user["user_id"], f"Generated a new API token for the key {key.name} " From 72f97e4ff0cbca33a9c3008aa9fd11283d64474c Mon Sep 17 00:00:00 2001 From: Andrew Date: Sat, 18 Jun 2022 16:55:39 -0400 Subject: [PATCH 2/9] Fix issue where any user could add/remove api keys --- app/classes/web/panel_handler.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index 4f6dfe87..e234c03c 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -1893,6 +1893,13 @@ class PanelHandler(BaseHandler): self.redirect("/panel/error?error=Invalid User ID") return + if user_id != exec_user["user_id"] or not exec_user["superuser"]: + self.redirect( + "/panel/error?error=You do not have access to change" + + "this user's api key." + ) + return + crafty_permissions_mask = self.get_perms() server_permissions_mask = self.get_perms_server() @@ -2148,6 +2155,15 @@ class PanelHandler(BaseHandler): self.redirect("/panel/error?error=Invalid Key ID") return + key_obj = self.controller.users.get_user_api_key(key_id) + + if key_obj.user_id != exec_user["user_id"] or not exec_user["superuser"]: + self.redirect( + "/panel/error?error=You do not have access to change" + + "this user's api key." + ) + return + self.controller.users.delete_user_api_key(key_id) self.controller.management.add_to_audit_log( From 37765dbebc0c65e5511e836abb54eaf11fd5b6fb Mon Sep 17 00:00:00 2001 From: Andrew Date: Sat, 18 Jun 2022 17:02:10 -0400 Subject: [PATCH 3/9] Fix general user can view any api-key page --- app/classes/web/panel_handler.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index e234c03c..cbb5de0e 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -1058,6 +1058,9 @@ class PanelHandler(BaseHandler): if user_id is None: self.redirect("/panel/error?error=Invalid User ID") return + if user_id != exec_user["user_id"] or not exec_user["superuser"]: + self.redirect("/panel/error?error=Invalid User ID") + return template = "panel/panel_edit_user_apikeys.html" From a8cd982b966ceb5a0ac255e39bc196fd7e39b000 Mon Sep 17 00:00:00 2001 From: Andrew Date: Sat, 18 Jun 2022 17:03:44 -0400 Subject: [PATCH 4/9] Fix warning message --- app/classes/web/panel_handler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index cbb5de0e..a59ca8d7 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -1059,7 +1059,9 @@ class PanelHandler(BaseHandler): self.redirect("/panel/error?error=Invalid User ID") return if user_id != exec_user["user_id"] or not exec_user["superuser"]: - self.redirect("/panel/error?error=Invalid User ID") + self.redirect( + "/panel/error?error=You are not authorized to view this page." + ) return template = "panel/panel_edit_user_apikeys.html" From 31097da97164832ccbb77fe774e9f49a87a14f4e Mon Sep 17 00:00:00 2001 From: Andrew Date: Sat, 18 Jun 2022 17:06:44 -0400 Subject: [PATCH 5/9] Fix type issue comparing --- app/classes/web/panel_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index a59ca8d7..96053378 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -1058,7 +1058,7 @@ class PanelHandler(BaseHandler): if user_id is None: self.redirect("/panel/error?error=Invalid User ID") return - if user_id != exec_user["user_id"] or not exec_user["superuser"]: + if str(user_id) != str(exec_user["user_id"]) or not exec_user["superuser"]: self.redirect( "/panel/error?error=You are not authorized to view this page." ) From 8b6d70ba9a4325d607d6b0a6ee6a230194a294ec Mon Sep 17 00:00:00 2001 From: xithical <86810816+xithical@users.noreply.github.com> Date: Sat, 18 Jun 2022 16:27:06 -0500 Subject: [PATCH 6/9] Fix bug where non-superusers could not edit their own API keys --- app/classes/web/panel_handler.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index 96053378..cd5d8cf7 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -1058,7 +1058,11 @@ class PanelHandler(BaseHandler): if user_id is None: self.redirect("/panel/error?error=Invalid User ID") return - if str(user_id) != str(exec_user["user_id"]) or not exec_user["superuser"]: + if int(user_id) != exec_user["user_id"] and not exec_user["superuser"]: + print(f"{user_id} {type(user_id)}") + print(f"{exec_user['user_id']} {type(exec_user['user_id'])}") + print(int(user_id) != exec_user["user_id"]) + print((int(user_id) != exec_user["user_id"]) or (not exec_user["superuser"])) self.redirect( "/panel/error?error=You are not authorized to view this page." ) From 464428ea7e1e0d6003355ce6192d84e408948e39 Mon Sep 17 00:00:00 2001 From: xithical <86810816+xithical@users.noreply.github.com> Date: Sat, 18 Jun 2022 16:29:36 -0500 Subject: [PATCH 7/9] Remove erroneous print statements --- app/classes/web/panel_handler.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index cd5d8cf7..147b6606 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -1059,10 +1059,6 @@ class PanelHandler(BaseHandler): self.redirect("/panel/error?error=Invalid User ID") return if int(user_id) != exec_user["user_id"] and not exec_user["superuser"]: - print(f"{user_id} {type(user_id)}") - print(f"{exec_user['user_id']} {type(exec_user['user_id'])}") - print(int(user_id) != exec_user["user_id"]) - print((int(user_id) != exec_user["user_id"]) or (not exec_user["superuser"])) self.redirect( "/panel/error?error=You are not authorized to view this page." ) From 9569e760c9dfffa0effe9d85eb2e8aa892864e73 Mon Sep 17 00:00:00 2001 From: Andrew Date: Sat, 18 Jun 2022 17:40:50 -0400 Subject: [PATCH 8/9] Fix api key permission logic issue --- app/classes/web/panel_handler.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index 147b6606..e2e83e23 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -1898,7 +1898,7 @@ class PanelHandler(BaseHandler): self.redirect("/panel/error?error=Invalid User ID") return - if user_id != exec_user["user_id"] or not exec_user["superuser"]: + if str(user_id) != str(exec_user["user_id"]) and not exec_user["superuser"]: self.redirect( "/panel/error?error=You do not have access to change" + "this user's api key." @@ -2162,7 +2162,7 @@ class PanelHandler(BaseHandler): key_obj = self.controller.users.get_user_api_key(key_id) - if key_obj.user_id != exec_user["user_id"] or not exec_user["superuser"]: + if key_obj.user_id != exec_user["user_id"] and not exec_user["superuser"]: self.redirect( "/panel/error?error=You do not have access to change" + "this user's api key." @@ -2178,7 +2178,8 @@ class PanelHandler(BaseHandler): server_id=0, source_ip=self.get_remote_ip(), ) - self.redirect("/panel/panel_config") + self.finish() + self.redirect(f"/panel/edit_user_apikeys?id={key_obj.user_id}") else: self.set_status(404) self.render( From 27cba00e16ca86704fbdf87910b6a9d82cccaf8b Mon Sep 17 00:00:00 2001 From: Zedifus Date: Sat, 18 Jun 2022 23:03:23 +0100 Subject: [PATCH 9/9] Bump CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3fb4ebd..8cfa6417 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ None ### Bug fixes - Amend Java system variable fix to be more specfic since they only affect Oracle. ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/364)) - +- API Token authentication hardening ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/364)) ### Tweaks - Add better error logging for statistic collection ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/359))