From b1ed9ba2bd6b39c48a8a8a8167ed731aae9fd107 Mon Sep 17 00:00:00 2001 From: Andrew Date: Sun, 10 Apr 2022 19:39:31 +0000 Subject: [PATCH] Add API routes from 3.x Enhance security for permissions on API requests Fix bug where server permissions and crafty permissions were flipped upon making a new token Fix bug where new secret key would be created every time Crafty was started. Fix bug where DB locks will occur with concurrent writes to the DB. --- app/classes/controllers/users_controller.py | 7 +- app/classes/models/crafty_permissions.py | 15 +- app/classes/models/server_permissions.py | 4 +- app/classes/shared/authentication.py | 4 +- app/classes/shared/helpers.py | 25 ++ app/classes/shared/main_models.py | 11 +- app/classes/web/api_handler.py | 262 +++++++++++++++++++- app/classes/web/panel_handler.py | 6 +- app/classes/web/tornado_handler.py | 26 +- app/config/config.json | 45 ++-- 10 files changed, 362 insertions(+), 43 deletions(-) diff --git a/app/classes/controllers/users_controller.py b/app/classes/controllers/users_controller.py index 1a6575a7..96169911 100644 --- a/app/classes/controllers/users_controller.py +++ b/app/classes/controllers/users_controller.py @@ -161,9 +161,14 @@ class Users_Controller: @staticmethod def get_user_by_api_token(token: str): - _, user = authentication.check(token) + _, _, user = authentication.check(token) return user + @staticmethod + def get_api_key_by_token(token: str): + key, _, _ = authentication.check(token) + return key + # ********************************************************************************** # User Roles Methods # ********************************************************************************** diff --git a/app/classes/models/crafty_permissions.py b/app/classes/models/crafty_permissions.py index fbe8ab0e..665dad87 100644 --- a/app/classes/models/crafty_permissions.py +++ b/app/classes/models/crafty_permissions.py @@ -2,7 +2,7 @@ import logging from app.classes.shared.helpers import helper from app.classes.shared.permission_helper import permission_helper -from app.classes.models.users import Users, ApiKeys +from app.classes.models.users import Users, ApiKeys, users_helper try: from peewee import ( @@ -213,13 +213,16 @@ class Permissions_Crafty: @staticmethod def get_api_key_permissions_list(key: ApiKeys): - user = key.user - if user.superuser and key.superuser: + user = users_helper.get_user(key.user_id) + if user["superuser"] and key.superuser: return crafty_permissions.get_permissions_list() else: - user_permissions_mask = crafty_permissions.get_crafty_permissions_mask( - user.user_id - ) + if user["superuser"]: + user_permissions_mask = "111" + else: + user_permissions_mask = crafty_permissions.get_crafty_permissions_mask( + user["user_id"] + ) key_permissions_mask: str = key.crafty_permissions permissions_mask = permission_helper.combine_masks( user_permissions_mask, key_permissions_mask diff --git a/app/classes/models/server_permissions.py b/app/classes/models/server_permissions.py index 6f5a0241..12f42b2f 100644 --- a/app/classes/models/server_permissions.py +++ b/app/classes/models/server_permissions.py @@ -263,8 +263,8 @@ class Permissions_Servers: @staticmethod def get_api_key_permissions_list(key: ApiKeys, server_id: str): - user = key.user - if user.superuser and key.superuser: + user = users_helper.get_user(key.user_id) + if user["superuser"] and key.superuser: return server_permissions.get_permissions_list() else: roles_list = users_helper.get_user_roles_id(user["user_id"]) diff --git a/app/classes/shared/authentication.py b/app/classes/shared/authentication.py index 6f4d5bcc..5a3d334a 100644 --- a/app/classes/shared/authentication.py +++ b/app/classes/shared/authentication.py @@ -22,16 +22,18 @@ class Authentication: if self.secret is None or self.secret == "random": self.secret = helper.random_string_generator(64) + helper.set_setting("apikey_secret", self.secret) @staticmethod def generate(user_id, extra=None): if extra is None: extra = {} - return jwt.encode( + jwt_encoded = jwt.encode( {"user_id": user_id, "iat": int(time.time()), **extra}, authentication.secret, algorithm="HS256", ) + return jwt_encoded @staticmethod def read(token): diff --git a/app/classes/shared/helpers.py b/app/classes/shared/helpers.py index 6deb85ee..58f3f1fe 100644 --- a/app/classes/shared/helpers.py +++ b/app/classes/shared/helpers.py @@ -206,6 +206,31 @@ class Helpers: return default_return + def set_setting(self, key, new_value, default_return=False): + + try: + with open(self.settings_file, "r", encoding="utf-8") as f: + data = json.load(f) + + if key in data.keys(): + data[key] = new_value + + else: + logger.error(f"Config File Error: setting {key} does not exist") + console.error(f"Config File Error: setting {key} does not exist") + return default_return + + with open(self.settings_file, "w", encoding="utf-8") as f: + json.dump(data, f, indent=1) + + except Exception as e: + logger.critical( + f"Config File Error: Unable to read {self.settings_file} due to {e}" + ) + console.critical( + f"Config File Error: Unable to read {self.settings_file} due to {e}" + ) + def get_local_ip(self): s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) try: diff --git a/app/classes/shared/main_models.py b/app/classes/shared/main_models.py index dcd4296f..155892a7 100644 --- a/app/classes/shared/main_models.py +++ b/app/classes/shared/main_models.py @@ -10,7 +10,8 @@ Users = Users try: # pylint: disable=unused-import - from peewee import SqliteDatabase, fn + from peewee import fn + from playhouse.sqliteq import SqliteQueueDatabase from playhouse.shortcuts import model_to_dict except ModuleNotFoundError as err: @@ -19,8 +20,12 @@ except ModuleNotFoundError as err: logger = logging.getLogger(__name__) peewee_logger = logging.getLogger("peewee") peewee_logger.setLevel(logging.INFO) -database = SqliteDatabase( - helper.db_path, pragmas={"journal_mode": "wal", "cache_size": -1024 * 10} +database = SqliteQueueDatabase( + helper.db_path + # This is commented out after presenting issues when + # moving from SQLiteDatabase to SqliteQueueDatabase + # //TODO Enable tuning + # pragmas={"journal_mode": "wal", "cache_size": -1024 * 10} ) diff --git a/app/classes/web/api_handler.py b/app/classes/web/api_handler.py index ae4b101f..53194fb2 100644 --- a/app/classes/web/api_handler.py +++ b/app/classes/web/api_handler.py @@ -1,6 +1,9 @@ +from datetime import datetime import logging import re +from app.classes.controllers.crafty_perms_controller import Enum_Permissions_Crafty +from app.classes.controllers.server_perms_controller import Enum_Permissions_Server from app.classes.web.base_handler import BaseHandler logger = logging.getLogger(__name__) @@ -13,6 +16,10 @@ class ApiHandler(BaseHandler): self.set_status(status) self.write(data) + def check_xsrf_cookie(self): + # Disable CSRF protection on API routes + pass + def access_denied(self, user, reason=""): if reason: reason = " because " + reason @@ -34,10 +41,24 @@ class ApiHandler(BaseHandler): ) def authenticate_user(self) -> bool: + self.permissions = { + "Commands": Enum_Permissions_Server.Commands, + "Terminal": Enum_Permissions_Server.Terminal, + "Logs": Enum_Permissions_Server.Logs, + "Schedule": Enum_Permissions_Server.Schedule, + "Backup": Enum_Permissions_Server.Backup, + "Files": Enum_Permissions_Server.Files, + "Config": Enum_Permissions_Server.Config, + "Players": Enum_Permissions_Server.Players, + "Server_Creation": Enum_Permissions_Crafty.Server_Creation, + "User_Config": Enum_Permissions_Crafty.User_Config, + "Roles_Config": Enum_Permissions_Crafty.Roles_Config, + } try: logger.debug("Searching for specified token") api_token = self.get_argument("token", "") + self.api_token = api_token if api_token is None and self.request.headers.get("Authorization"): api_token = bearer_pattern.sub( "", self.request.headers.get("Authorization") @@ -50,7 +71,6 @@ class ApiHandler(BaseHandler): if user_data: # Login successful! Check perms logger.info(f"User {user_data['username']} has authenticated to API") - # TODO: Role check return True # This is to set the "authenticated" else: @@ -77,10 +97,20 @@ class ServersStats(ApiHandler): authenticated = self.authenticate_user() if not authenticated: return + raw_stats = self.controller.servers.get_all_servers_stats() + stats = [] + for rs in raw_stats: + s = {} + for k, v in rs["server_data"].items(): + if isinstance(v, datetime): + s[k] = v.timestamp() + else: + s[k] = v + stats.append(s) # Get server stats # TODO Check perms - self.finish(self.write({"servers": self.controller.stats.get_servers_stats()})) + self.finish(self.write({"servers": stats})) class NodeStats(ApiHandler): @@ -92,5 +122,229 @@ class NodeStats(ApiHandler): # Get node stats node_stats = self.controller.stats.get_node_stats() - node_stats.pop("servers") - self.finish(self.write(node_stats)) + self.return_response(200, {"code": node_stats["node_stats"]}) + + +class SendCommand(ApiHandler): + def post(self): + user = self.authenticate_user() + + if user is None: + self.access_denied("unknown") + server_id = self.get_argument("id") + + if not self.permissions[ + "Commands" + ] in self.controller.server_perms.get_api_key_permissions_list( + self.controller.users.get_api_key_by_token(self.api_token), server_id + ): + self.access_denied(user) + + command = self.get_argument("command", default=None, strip=True) + server_id = self.get_argument("id") + if command: + server = self.controller.get_server_obj(server_id) + if server.check_running: + server.send_command(command) + self.return_response(200, {"run": True}) + else: + self.return_response(200, {"error": "SER_NOT_RUNNING"}) + else: + self.return_response(200, {"error": "NO_COMMAND"}) + + +class ServerBackup(ApiHandler): + def post(self): + user = self.authenticate_user() + + if user is None: + self.access_denied("unknown") + server_id = self.get_argument("id") + + if not self.permissions[ + "Backup" + ] in self.controller.server_perms.get_api_key_permissions_list( + self.controller.users.get_api_key_by_token(self.api_token), server_id + ): + self.access_denied(user) + + server = self.controller.get_server_obj(server_id) + + server.backup_server() + + self.return_response(200, {"code": "SER_BAK_CALLED"}) + + +class StartServer(ApiHandler): + def post(self): + user = self.authenticate_user() + remote_ip = self.get_remote_ip() + + user_obj = self.controller.users.get_user_by_api_token(self.api_token) + + if user is None: + self.access_denied("unknown") + + server_id = self.get_argument("id") + + if not self.permissions[ + "Commands" + ] in self.controller.server_perms.get_api_key_permissions_list( + self.controller.users.get_api_key_by_token(self.api_token), server_id + ): + self.access_denied(user) + + server = self.controller.get_server_obj(server_id) + + if not server.check_running(): + self.controller.management.send_command( + user_obj["user_id"], server_id, remote_ip, "start_server" + ) + self.return_response(200, {"code": "SER_START_CALLED"}) + else: + self.return_response(500, {"error": "SER_RUNNING"}) + + +class StopServer(ApiHandler): + def post(self): + user = self.authenticate_user() + remote_ip = self.get_remote_ip() + + if user is None: + self.access_denied("unknown") + server_id = self.get_argument("id") + + if not self.permissions[ + "Commands" + ] in self.controller.server_perms.get_api_key_permissions_list( + self.controller.users.get_api_key_by_token(self.api_token), server_id + ): + self.access_denied(user) + + server = self.controller.get_server_obj(server_id) + + if server.check_running(): + self.controller.management.send_command( + user, server_id, remote_ip, "stop_server" + ) + + self.return_response(200, {"code": "SER_STOP_CALLED"}) + else: + self.return_response(500, {"error": "SER_NOT_RUNNING"}) + + +class RestartServer(ApiHandler): + def post(self): + user = self.authenticate_user() + remote_ip = self.get_remote_ip() + server_id = self.get_argument("id") + + if user is None: + self.access_denied("unknown") + + if not self.permissions[ + "Commands" + ] in self.controller.server_perms.get_api_key_permissions_list( + self.controller.users.get_api_key_by_token(self.api_token), server_id + ): + self.access_denied(user) + + self.controller.management.send_command( + user, server_id, remote_ip, "restart_server" + ) + self.return_response(200, {"code": "SER_RESTART_CALLED"}) + + +class CreateUser(ApiHandler): + def post(self): + user = self.authenticate_user() + + if user is None: + self.access_denied("unknown") + + if not self.permissions[ + "User_Config" + ] in self.controller.crafty_perms.get_api_key_permissions_list( + self.controller.users.get_api_key_by_token(self.api_token) + ): + self.access_denied(user) + + new_username = self.get_argument("username") + new_pass = self.get_argument("password") + + if new_username: + self.controller.users.add_user( + new_username, new_pass, "default@example.com", True, False + ) + + self.return_response( + 200, + { + "code": "COMPLETE", + "username": new_username, + "password": new_pass, + }, + ) + else: + self.return_response( + 500, + { + "error": "MISSING_PARAMS", + "info": "Some paramaters failed validation", + }, + ) + + +class DeleteUser(ApiHandler): + def post(self): + user = self.authenticate_user() + + if user is None: + self.access_denied("unknown") + + if not self.permissions[ + "User_Config" + ] in self.controller.crafty_perms.get_api_key_permissions_list( + self.controller.users.get_api_key_by_token(self.api_token) + ): + self.access_denied(user) + + user_id = self.get_argument("user_id", None, True) + user_to_del = self.controller.users.get_user_by_id(user_id) + + if user_to_del["superuser"]: + self.return_response( + 500, + {"error": "NOT_ALLOWED", "info": "You cannot delete a super user"}, + ) + else: + if user_id: + self.controller.users.remove_user(user_id) + self.return_response(200, {"code": "COMPLETED"}) + + +class ListServers(ApiHandler): + def get(self): + user = self.authenticate_user() + user_obj = self.controller.users.get_user_by_api_token(self.api_token) + + if user is None: + self.access_denied("unknown") + + if self.api_token is None: + self.access_denied("unknown") + + if user_obj["superuser"]: + servers = self.controller.servers.get_all_defined_servers() + servers = [str(i) for i in servers] + else: + servers = self.controller.servers.get_all_defined_servers() + servers = [str(i) for i in servers] + + self.return_response( + 200, + { + "code": "COMPLETED", + "servers": servers, + }, + ) diff --git a/app/classes/web/panel_handler.py b/app/classes/web/panel_handler.py index a1071a21..8175d058 100644 --- a/app/classes/web/panel_handler.py +++ b/app/classes/web/panel_handler.py @@ -1856,8 +1856,8 @@ class PanelHandler(BaseHandler): name, user_id, superuser, - crafty_permissions_mask, server_permissions_mask, + crafty_permissions_mask, ) self.controller.management.add_to_audit_log( @@ -1886,13 +1886,13 @@ class PanelHandler(BaseHandler): self.controller.management.add_to_audit_log( exec_user["user_id"], f"Generated a new API token for the key {key.name} " - f"from user with UID: {key.user.user_id}", + f"from user with UID: {key.user_id}", server_id=0, source_ip=self.get_remote_ip(), ) self.write( - authentication.generate(key.user.user_id, {"token_id": key.token_id}) + authentication.generate(key.user_id.user_id, {"token_id": key.token_id}) ) self.finish() diff --git a/app/classes/web/tornado_handler.py b/app/classes/web/tornado_handler.py index 805820ac..65075850 100644 --- a/app/classes/web/tornado_handler.py +++ b/app/classes/web/tornado_handler.py @@ -13,7 +13,18 @@ from app.classes.web.panel_handler import PanelHandler from app.classes.web.default_handler import DefaultHandler from app.classes.web.server_handler import ServerHandler from app.classes.web.ajax_handler import AjaxHandler -from app.classes.web.api_handler import ServersStats, NodeStats +from app.classes.web.api_handler import ( + ServersStats, + NodeStats, + ServerBackup, + StartServer, + StopServer, + RestartServer, + CreateUser, + DeleteUser, + ListServers, + SendCommand, +) from app.classes.web.websocket_handler import SocketHandler from app.classes.web.static_handler import CustomStaticHandler from app.classes.web.upload_handler import UploadHandler @@ -139,11 +150,20 @@ class Webserver: (r"/server/(.*)", ServerHandler, handler_args), (r"/ajax/(.*)", AjaxHandler, handler_args), (r"/files/(.*)", FileHandler, handler_args), - (r"/api/stats/servers", ServersStats, handler_args), - (r"/api/stats/node", NodeStats, handler_args), (r"/ws", SocketHandler, handler_args), (r"/upload", UploadHandler, handler_args), (r"/status", StatusHandler, handler_args), + # API Routes + (r"/api/v1/stats/servers", ServersStats, handler_args), + (r"/api/v1/stats/node", NodeStats, handler_args), + (r"/api/v1/server/send_command", SendCommand, handler_args), + (r"/api/v1/server/backup", ServerBackup, handler_args), + (r"/api/v1/server/start", StartServer, handler_args), + (r"/api/v1/server/stop", StopServer, handler_args), + (r"/api/v1/server/restart", RestartServer, handler_args), + (r"/api/v1/list_servers", ListServers, handler_args), + (r"/api/v1/users/create_user", CreateUser, handler_args), + (r"/api/v1/users/delete_user", DeleteUser, handler_args), ] app = tornado.web.Application( diff --git a/app/config/config.json b/app/config/config.json index 93b3c7e3..20921ee7 100644 --- a/app/config/config.json +++ b/app/config/config.json @@ -1,21 +1,26 @@ { - "https": true, - "http_port": 8000, - "https_port": 8443, - "language": "en_EN", - "cookie_expire": 30, - "cookie_secret": "random", - "apikey_secret": "random", - "show_errors": true, - "history_max_age": 7, - "stats_update_frequency": 30, - "delete_default_json": false, - "show_contribute_link": true, - "virtual_terminal_lines": 70, - "max_log_lines": 700, - "max_audit_entries": 300, - "disabled_language_files": ["lol_EN.json", ""], - "stream_size_GB": 1, - "keywords": ["help", "chunk"], - "allow_nsfw_profile_pictures": false -} + "http_port": 8000, + "https_port": 8443, + "language": "en_EN", + "cookie_expire": 30, + "cookie_secret": "random", + "apikey_secret": "random", + "show_errors": true, + "history_max_age": 7, + "stats_update_frequency": 30, + "delete_default_json": false, + "show_contribute_link": true, + "virtual_terminal_lines": 70, + "max_log_lines": 700, + "max_audit_entries": 300, + "disabled_language_files": [ + "lol_EN.json", + "" + ], + "stream_size_GB": 1, + "keywords": [ + "help", + "chunk" + ], + "allow_nsfw_profile_pictures": false +} \ No newline at end of file