diff --git a/keepercommander/resources/service_config.ini b/keepercommander/resources/service_config.ini index 516f22da2..336c55ca5 100644 --- a/keepercommander/resources/service_config.ini +++ b/keepercommander/resources/service_config.ini @@ -4,6 +4,7 @@ ngrok_prompt = Enable Ngrok Tunneling? (y/n): ngrok_token_prompt = Enter Ngrok Auth Token: ngrok_custom_domain_prompt = Enter Ngrok Custom Domain: run_mode_prompt = Select run mode (foreground/background): +queue_enabled_prompt = Enable Request Queue? (y/n): tls_certificate = Enable TLS Certificate? (y/n): certfile = Enter Certificate Path: certpassword = Enter Certificate Password: @@ -25,4 +26,5 @@ invalid_rate_limit = Invalid rate limit: invalid_ip_list = Invalid IP list: invalid_encryption_key = Invalid encryption key: invalid_input = Invalid input. Please enter 'y' or 'n'. -invalid_format = Invalid format. Please enter either 'json' or 'yaml'. \ No newline at end of file +invalid_format = Invalid format. Please enter either 'json' or 'yaml'. +invalid_queue_setting = Invalid queue setting. Please enter 'y' or 'n'. \ No newline at end of file diff --git a/keepercommander/service/README.md b/keepercommander/service/README.md index 4a58af7ad..df3504a38 100644 --- a/keepercommander/service/README.md +++ b/keepercommander/service/README.md @@ -42,6 +42,7 @@ You'll be prompted to configure: - Enable TLS Certificate (y/n) - TLS Certificate path - TLS Certificate password +- Enable Request Queue (y/n) - Advanced Security (y/n) - Rate Limit - Allowed IP List (comma-separated) @@ -57,13 +58,13 @@ You'll be prompted to configure: Configure the service streamlined with TLS: ```bash - My Vault> service-create -p -f -c 'tree,ls,search,record-add,mkdir' -rm -crtf -crtp -aip -dip + My Vault> service-create -p -f -c 'tree,ls,search,record-add,mkdir' -rm -q -crtf -crtp -aip -dip ``` Configure the service streamlined with Ngrok: ```bash - My Vault> service-create -p -f -c 'tree,record-add,audit-report' -ng -cd -rm -aip -dip + My Vault> service-create -p -f -c 'tree,record-add,audit-report' -ng -cd -rm -q -aip -dip ``` Parameters: @@ -75,6 +76,7 @@ Parameters: - `-crtf, --certfile`: Certificate file path - `-crtp, --certpassword`: Certificate password - `-rm, --run_mode`: Run mode (foreground/background) +- `-q, --queue_enabled`: Enable request queue (y/n) - `-dip, --deniedip`: Denied IP list to access service - `-aip, --allowedip`: Allowed IP list to access service @@ -97,6 +99,12 @@ My Vault> service-stop ## API Usage +### API Versioning + +The service provides two API versions based on queue configuration: +- **`/api/v2/`** - Queue enabled (default): Asynchronous request processing with enhanced features +- **`/api/v1/`** - Queue disabled (legacy): Direct synchronous execution + ### Request Queue System The service uses an asynchronous request queue system that provides: @@ -109,7 +117,7 @@ The service uses an asynchronous request queue system that provides: **Submit Request:** ```bash -curl -X POST 'http://localhost:/api/v1/executecommand' \ +curl -X POST 'http://localhost:/api/v2/executecommand-async' \ --header 'Content-Type: application/json' \ --header 'api-key: ' \ --data '{"command": "tree"}' @@ -120,13 +128,13 @@ curl -X POST 'http://localhost:/api/v1/executecommand' \ "success": true, "request_id": "550e8400-e29b-41d4-a716-446655440000", "status": "queued", - "message": "Request queued successfully. Use /api/v1/status/ to check progress, /api/v1/result/ to get results, or /api/v1/queue/status for queue info." + "message": "Request queued successfully. Use /api/v2/status/ to check progress, /api/v2/result/ to get results, or /api/v2/queue/status for queue info." } ``` **Check Request Status:** ```bash -curl 'http://localhost:/api/v1/status/' \ +curl 'http://localhost:/api/v2/status/' \ --header 'api-key: ' ``` *Response:* @@ -144,7 +152,7 @@ curl 'http://localhost:/api/v1/status/' \ **Get Request Result:** ```bash -curl 'http://localhost:/api/v1/result/' \ +curl 'http://localhost:/api/v2/result/' \ --header 'api-key: ' ``` *Response (for completed request):* @@ -157,7 +165,7 @@ curl 'http://localhost:/api/v1/result/' \ **Get Queue Status:** ```bash -curl 'http://localhost:/api/v1/queue/status' \ +curl 'http://localhost:/api/v2/queue/status' \ --header 'api-key: ' ``` *Response:* @@ -311,6 +319,15 @@ The service includes robust error handling for: ### Execute Command Endpoint ```bash + # Queue enabled (v2 - async) + curl --location 'http://localhost:/api/v2/executecommand-async' \ + --header 'Content-Type: application/json' \ + --header 'api-key: ' \ + --data '{ + "command": "" + }' + + # Queue disabled (v1 - direct) curl --location 'http://localhost:/api/v1/executecommand' \ --header 'Content-Type: application/json' \ --header 'api-key: ' \ diff --git a/keepercommander/service/api/command.py b/keepercommander/service/api/command.py index 1916dc7fd..cc1b6f171 100644 --- a/keepercommander/service/api/command.py +++ b/keepercommander/service/api/command.py @@ -9,27 +9,73 @@ # Contact: ops@keepersecurity.com # -from flask import Blueprint, request, jsonify +from flask import Blueprint, request, jsonify, Response from html import escape import queue +from typing import Tuple, Union from ..decorators.unified import unified_api_decorator +from ..util.command_util import CommandExecutor from ..decorators.logging import logger from ..core.request_queue import queue_manager +from ..util.request_validation import RequestValidator + +def create_legacy_command_blueprint(): + """Create legacy blueprint for direct/synchronous command execution (non-queue mode).""" + bp = Blueprint("legacy_command_bp", __name__) + + @bp.after_request + def add_legacy_header(response): + """Add legacy header for legacy API.""" + response.headers['X-API-Legacy'] = 'true' + return response + + @bp.route("/executecommand", methods=["POST"]) + @unified_api_decorator() + def execute_command_direct(**kwargs) -> Tuple[Union[Response, bytes], int]: + """Execute command directly and return result immediately (legacy behavior).""" + try: + logger.warning("LEGACY: /api/v1/ usage - migrate to /api/v2/") + + json_error = RequestValidator.validate_request_json() + if json_error: + return json_error + + command, validation_error = RequestValidator.validate_and_escape_command(request.json) + if validation_error: + return validation_error + + response, status_code = CommandExecutor.execute(command) + + # If we get a busy response, add v1-specific message + if (isinstance(response, dict) and + "temporarily busy" in str(response.get("error", "")).lower()): + response["message"] = "Note: api/v1/executecommand only supports a single request at a time." + status_code = 503 + + return response if isinstance(response, bytes) else jsonify(response), status_code + + except Exception as e: + logger.error(f"Error executing command: {e}") + return jsonify({"success": False, "error": f"Error: {str(e)}"}), 500 + + return bp def create_command_blueprint(): """Create Blue Print for Keeper Commander Service.""" bp = Blueprint("command_bp", __name__) - @bp.route("/executecommand", methods=["POST"]) + @bp.route("/executecommand-async", methods=["POST"]) @unified_api_decorator() - def execute_command(**kwargs): + def execute_command(**kwargs) -> Tuple[Response, int]: """Submit a command for execution and return request ID immediately.""" try: - request_command = request.json.get("command") - if not request_command: - return jsonify({"success": False, "error": "Error: No command provided"}), 400 - - command = escape(request_command) + json_error = RequestValidator.validate_request_json() + if json_error: + return json_error + + command, validation_error = RequestValidator.validate_and_escape_command(request.json) + if validation_error: + return validation_error # Submit to queue and return request ID immediately try: @@ -38,7 +84,7 @@ def execute_command(**kwargs): "success": True, "request_id": request_id, "status": "queued", - "message": "Request queued successfully. Use /api/v1/status/ to check progress, /api/v1/result/ to get results, or /api/v1/queue/status for queue info." + "message": "Request queued successfully. Use /api/v2/status/ to check progress, /api/v2/result/ to get results, or /api/v2/queue/status for queue info." }), 202 # 202 Accepted except queue.Full: return jsonify({ @@ -52,7 +98,7 @@ def execute_command(**kwargs): @bp.route("/status/", methods=["GET"]) @unified_api_decorator() - def get_request_status(request_id, **kwargs): + def get_request_status(request_id: str, **kwargs) -> Tuple[Response, int]: """Get the status of a specific request.""" try: status_info = queue_manager.get_request_status(request_id) @@ -74,7 +120,7 @@ def get_request_status(request_id, **kwargs): @bp.route("/result/", methods=["GET"]) @unified_api_decorator() - def get_request_result(request_id, **kwargs): + def get_request_result(request_id: str, **kwargs) -> Tuple[Union[Response, bytes], int]: """Get the result of a completed request.""" try: result_data = queue_manager.get_request_result(request_id) @@ -102,7 +148,7 @@ def get_request_result(request_id, **kwargs): @bp.route("/queue/status", methods=["GET"]) @unified_api_decorator() - def get_queue_status(**kwargs): + def get_queue_status(**kwargs) -> Tuple[Response, int]: """Get overall queue status information.""" try: queue_status = queue_manager.get_queue_status() diff --git a/keepercommander/service/api/routes.py b/keepercommander/service/api/routes.py index 01ee51827..c1e85738f 100644 --- a/keepercommander/service/api/routes.py +++ b/keepercommander/service/api/routes.py @@ -11,19 +11,47 @@ from flask import Flask from typing import Optional -from .command import create_command_blueprint +from .command import create_command_blueprint, create_legacy_command_blueprint from ..decorators.logging import logger, debug_decorator +def _setup_queue_mode(app: Flask) -> None: + """Setup queue mode with v2 API endpoints.""" + from ..core.request_queue import queue_manager + queue_manager.start() + + command_bp = create_command_blueprint() + app.register_blueprint(command_bp, url_prefix='/api/v2') + logger.debug("Started queue manager and registered command blueprint with URL prefix '/api/v2'") + +def _setup_legacy_mode(app: Flask) -> None: + """Setup legacy mode with v1 API endpoints.""" + legacy_bp = create_legacy_command_blueprint() + app.register_blueprint(legacy_bp, url_prefix='/api/v1') + logger.info("Using LEGACY /api/v1 - Enable queue mode (-q y) for /api/v2") + @debug_decorator def init_routes(app: Optional[Flask] = None) -> None: - """Initialize routes for the Keeper Commander Service.""" + """Initialize routes and queue manager for the Keeper Commander Service.""" if app is None: raise ValueError("App instance is required") logger.debug("Starting route initialization") - command_bp = create_command_blueprint() - - logger.debug("Registering command blueprint with URL prefix '/api/v1'") - app.register_blueprint(command_bp, url_prefix='/api/v1') + + try: + from ..config.service_config import ServiceConfig + service_config = ServiceConfig() + config_data = service_config.load_config() + queue_enabled = config_data.get("queue_enabled", "y") # Default to enabled + + if queue_enabled == "y": + logger.debug("Queue enabled - setting up v2 API with request queue") + _setup_queue_mode(app) + else: + logger.debug("Queue disabled - setting up v1 API with direct execution") + _setup_legacy_mode(app) + + except Exception as e: + logger.warning(f"Could not load service config, defaulting to queue mode: {e}") + _setup_queue_mode(app) logger.debug("Route initialization completed successfully") \ No newline at end of file diff --git a/keepercommander/service/app.py b/keepercommander/service/app.py index 4bd68026f..06bdae21c 100644 --- a/keepercommander/service/app.py +++ b/keepercommander/service/app.py @@ -15,7 +15,6 @@ from .decorators.security import limiter from .api.routes import init_routes from .decorators.logging import logger -from .core.request_queue import queue_manager def create_app(): @@ -31,9 +30,6 @@ def create_app(): try: logger.debug("Configuring rate limiter") limiter.init_app(app) - - logger.debug("Starting request queue manager") - queue_manager.start() logger.debug("Initializing API routes") init_routes(app) diff --git a/keepercommander/service/commands/create_service.py b/keepercommander/service/commands/create_service.py index 7cbcb6219..b360d0361 100644 --- a/keepercommander/service/commands/create_service.py +++ b/keepercommander/service/commands/create_service.py @@ -31,6 +31,7 @@ class StreamlineArgs: certpassword : Optional[str] fileformat : Optional[str] run_mode: Optional[str] + queue_enabled: Optional[str] class CreateService(Command): """Command to create a new service configuration.""" @@ -67,6 +68,7 @@ def get_parser(self): parser.add_argument('-crtp', '--certpassword', type=str, help='certificate password') parser.add_argument('-f', '--fileformat', type=str, help='file format') parser.add_argument('-rm', '--run_mode', type=str, help='run mode') + parser.add_argument('-q', '--queue_enabled', type=str, help='enable request queue (y/n)') return parser def execute(self, params: KeeperParams, **kwargs) -> None: @@ -81,7 +83,7 @@ def execute(self, params: KeeperParams, **kwargs) -> None: config_data = self.service_config.create_default_config() - filtered_kwargs = {k: v for k, v in kwargs.items() if k in ['port', 'allowedip', 'deniedip', 'commands', 'ngrok', 'ngrok_custom_domain', 'certfile', 'certpassword', 'fileformat', 'run_mode']} + filtered_kwargs = {k: v for k, v in kwargs.items() if k in ['port', 'allowedip', 'deniedip', 'commands', 'ngrok', 'ngrok_custom_domain', 'certfile', 'certpassword', 'fileformat', 'run_mode', 'queue_enabled']} args = StreamlineArgs(**filtered_kwargs) self._handle_configuration(config_data, params, args) self._create_and_save_record(config_data, params, args) diff --git a/keepercommander/service/commands/service_config_handlers.py b/keepercommander/service/commands/service_config_handlers.py index 37824ad88..ab28a5302 100644 --- a/keepercommander/service/commands/service_config_handlers.py +++ b/keepercommander/service/commands/service_config_handlers.py @@ -39,6 +39,10 @@ def handle_streamlined_config(self, config_data: Dict[str, Any], args, params: K if args.fileformat is not None and args.fileformat not in ['json', 'yaml']: raise ValidationError(f"Invalid file format '{args.fileformat}'. Must be 'json' or 'yaml'.") + queue_enabled = args.queue_enabled if args.queue_enabled is not None else "y" + if args.queue_enabled is not None and queue_enabled not in ['y', 'n']: + raise ValidationError(f"Invalid queue setting '{queue_enabled}'. Must be 'y' or 'n'.") + config_data.update({ "port": self.service_config.validator.validate_port(args.port), "ip_allowed_list": self.service_config.validator.validate_ip_list(args.allowedip), @@ -52,7 +56,8 @@ def handle_streamlined_config(self, config_data: Dict[str, Any], args, params: K "certfile": args.certfile, "certpassword": args.certpassword, "fileformat": args.fileformat, # Keep original logic - can be None - "run_mode": run_mode + "run_mode": run_mode, + "queue_enabled": queue_enabled }) @debug_decorator @@ -60,6 +65,7 @@ def handle_interactive_config(self, config_data: Dict[str, Any], params: KeeperP self._configure_port(config_data) self._configure_ngrok(config_data) self._configure_tls(config_data) + self._configure_queue(config_data) config_data["fileformat"] = None @@ -108,6 +114,11 @@ def _configure_tls(self, config_data: Dict[str, Any]) -> None: config_data["certfile"] = "" config_data["certpassword"] = "" + def _configure_queue(self, config_data: Dict[str, Any]) -> None: + """Configure queue enabled setting with user prompt.""" + config_data["queue_enabled"] = self.service_config._get_yes_no_input(self.messages['queue_enabled_prompt']) + logger.debug(f"Queue enabled set to: {config_data['queue_enabled']}") + def _configure_run_mode(self, config_data: Dict[str, Any]) -> None: """Configure run mode with user prompt.""" while True: diff --git a/keepercommander/service/config/models.py b/keepercommander/service/config/models.py index 175b59dad..05b2f7e61 100644 --- a/keepercommander/service/config/models.py +++ b/keepercommander/service/config/models.py @@ -32,4 +32,5 @@ class ServiceConfigData: encryption_private_key: str fileformat: str run_mode: str + queue_enabled: str records: List[Dict[str, Any]] \ No newline at end of file diff --git a/keepercommander/service/config/service_config.py b/keepercommander/service/config/service_config.py index 0fb9ee3ba..5857b3b0b 100644 --- a/keepercommander/service/config/service_config.py +++ b/keepercommander/service/config/service_config.py @@ -101,6 +101,7 @@ def create_default_config(self) -> Dict[str, Any]: encryption_private_key="", fileformat="yaml", run_mode="foreground", + queue_enabled="y", records=[] ).__dict__ return config @@ -216,6 +217,12 @@ def save_cert_data(self, config_data: Dict[str, Any], save_type: str = None) -> def load_config(self) -> Dict[str, Any]: """Load configuration from file.""" config = self.format_handler.load_config() + + # Add backwards compatibility for missing queue_enabled field + if 'queue_enabled' not in config: + config['queue_enabled'] = 'y' # Default to enabled for existing configs + logger.debug("Added default queue_enabled=y for backwards compatibility") + self._validate_config_structure(config) return config diff --git a/keepercommander/service/core/request_queue.py b/keepercommander/service/core/request_queue.py index 99c9dbc90..b4745f309 100644 --- a/keepercommander/service/core/request_queue.py +++ b/keepercommander/service/core/request_queue.py @@ -93,6 +93,7 @@ def __init__(self): self.current_request_id = None self.request_timeout = DEFAULT_REQUEST_TIMEOUT self.result_retention = DEFAULT_RESULT_RETENTION + self.data_lock = threading.Lock() # Lock for shared data structures logger.debug("RequestQueueManager initialized") @@ -139,7 +140,8 @@ def submit_request(self, command: str) -> str: try: self.request_queue.put(request, block=False) - self.active_requests[request_id] = request + with self.data_lock: + self.active_requests[request_id] = request logger.info(f"Request {request_id} queued: {command}") return request_id except queue.Full: @@ -156,15 +158,16 @@ def get_request_status(self, request_id: str) -> Optional[Dict[str, Any]]: Returns: Dict containing request status and metadata, or None if not found """ - # Check active requests - if request_id in self.active_requests: - return self.active_requests[request_id].to_dict() - - # Check completed requests - if request_id in self.completed_requests: - return self.completed_requests[request_id].to_dict() - - return None + with self.data_lock: + # Check active requests + if request_id in self.active_requests: + return self.active_requests[request_id].to_dict() + + # Check completed requests + if request_id in self.completed_requests: + return self.completed_requests[request_id].to_dict() + + return None @debug_decorator def get_request_result(self, request_id: str) -> Optional[Tuple[Any, int]]: @@ -176,13 +179,14 @@ def get_request_result(self, request_id: str) -> Optional[Tuple[Any, int]]: Returns: Tuple of (result, status_code) or None if not found/not completed """ - if request_id in self.completed_requests: - request = self.completed_requests[request_id] - if request.status == RequestStatus.COMPLETED: - return request.result, 200 - elif request.status == RequestStatus.FAILED: - return {"error": request.error_message}, 500 - return None + with self.data_lock: + if request_id in self.completed_requests: + request = self.completed_requests[request_id] + if request.status == RequestStatus.COMPLETED: + return request.result, 200 + elif request.status == RequestStatus.FAILED: + return {"error": request.error_message}, 500 + return None @debug_decorator def get_queue_status(self) -> Dict[str, Any]: @@ -191,13 +195,14 @@ def get_queue_status(self) -> Dict[str, Any]: Returns: Dict containing queue statistics """ - return { - "queue_size": self.request_queue.qsize(), - "active_requests": len(self.active_requests), - "completed_requests": len(self.completed_requests), - "currently_processing": self.current_request_id, - "worker_running": self.is_running and self.worker_thread and self.worker_thread.is_alive() - } + with self.data_lock: + return { + "queue_size": self.request_queue.qsize(), + "active_requests": len(self.active_requests), + "completed_requests": len(self.completed_requests), + "currently_processing": self.current_request_id, + "worker_running": self.is_running and self.worker_thread and self.worker_thread.is_alive() + } def _process_queue(self): """Main worker thread loop for processing queued requests.""" @@ -253,39 +258,41 @@ def _process_request(self, request: QueuedRequest): finally: # Move from active to completed - if request.request_id in self.active_requests: - del self.active_requests[request.request_id] - self.completed_requests[request.request_id] = request - self.current_request_id = None + with self.data_lock: + if request.request_id in self.active_requests: + del self.active_requests[request.request_id] + self.completed_requests[request.request_id] = request + self.current_request_id = None def _cleanup_expired_requests(self): """Clean up expired and old completed requests.""" now = datetime.now() - # Find expired active requests - expired_ids = [] - for request_id, request in self.active_requests.items(): - if request.status == RequestStatus.QUEUED: - age = (now - request.created_at).total_seconds() - if age > self.request_timeout: - request.status = RequestStatus.EXPIRED - expired_ids.append(request_id) - - # Move expired requests to completed - for request_id in expired_ids: - request = self.active_requests.pop(request_id) - self.completed_requests[request_id] = request - logger.warning(f"Request {request_id} expired after {self.request_timeout}s") - - # Clean up old completed requests - cutoff_time = now - timedelta(seconds=self.result_retention) - old_ids = [] - for request_id, request in self.completed_requests.items(): - if request.completed_at and request.completed_at < cutoff_time: - old_ids.append(request_id) - - for request_id in old_ids: - del self.completed_requests[request_id] - logger.debug(f"Cleaned up old request {request_id}") + with self.data_lock: + # Find expired active requests + expired_ids = [] + for request_id, request in self.active_requests.items(): + if request.status == RequestStatus.QUEUED: + age = (now - request.created_at).total_seconds() + if age > self.request_timeout: + request.status = RequestStatus.EXPIRED + expired_ids.append(request_id) + + # Move expired requests to completed + for request_id in expired_ids: + request = self.active_requests.pop(request_id) + self.completed_requests[request_id] = request + logger.warning(f"Request {request_id} expired after {self.request_timeout}s") + + # Clean up old completed requests + cutoff_time = now - timedelta(seconds=self.result_retention) + old_ids = [] + for request_id, request in self.completed_requests.items(): + if request.completed_at and request.completed_at < cutoff_time: + old_ids.append(request_id) + + for request_id in old_ids: + del self.completed_requests[request_id] + logger.debug(f"Cleaned up old request {request_id}") queue_manager = RequestQueueManager() \ No newline at end of file diff --git a/keepercommander/service/core/service_manager.py b/keepercommander/service/core/service_manager.py index dd040229a..47f7a0a86 100644 --- a/keepercommander/service/core/service_manager.py +++ b/keepercommander/service/core/service_manager.py @@ -100,7 +100,10 @@ def start_service(cls) -> None: is_running = True - print(f"Commander Service starting on https://localhost:{port}") + queue_enabled = config_data.get("queue_enabled", "y") + api_version = "v2" if queue_enabled == "y" else "v1" + + print(f"Commander Service starting on https://localhost:{port}/api/{api_version}/") ngrok_pid = NgrokConfigurator.configure_ngrok(config_data, service_config) diff --git a/keepercommander/service/decorators/security.py b/keepercommander/service/decorators/security.py index 2a57a6082..3deb007bd 100644 --- a/keepercommander/service/decorators/security.py +++ b/keepercommander/service/decorators/security.py @@ -82,14 +82,17 @@ def is_ip_in_range(ip, ip_range): def security_check(fn): @wraps(fn) def get_multiplied_rate_limit(): - """Get rate limit multiplied by 4 to account for 4 endpoints sharing the limit""" + """Get rate limit with appropriate multiplier based on API version""" + from flask import request base_limit = ConfigReader.read_config("rate_limiting") if base_limit: import re match = re.match(r'(\d+)(/\w+)', base_limit) if match: number, unit = match.groups() - return f"{int(number) * 4}{unit}" + # v2 API has 4 endpoints sharing the limit, v1 API has only 1 + multiplier = 1 if request.path.startswith('/api/v1') else 4 + return f"{int(number) * multiplier}{unit}" return base_limit @limiter.limit(get_multiplied_rate_limit) diff --git a/keepercommander/service/util/command_util.py b/keepercommander/service/util/command_util.py index 4f9e7ad48..e2dd24f8d 100644 --- a/keepercommander/service/util/command_util.py +++ b/keepercommander/service/util/command_util.py @@ -103,6 +103,10 @@ def execute(cls, command: str) -> Tuple[Any, int]: logger.debug("Command executed successfully") return response, 200 else: - return "Internal Server Error", 500 + busy_response = { + "success": False, + "error": "The server is temporarily busy. Please try again shortly." + } + return busy_response, 503 except Exception as e: raise CommandExecutionError(f"Command execution failed: {str(e)}") \ No newline at end of file diff --git a/keepercommander/service/util/parse_keeper_response.py b/keepercommander/service/util/parse_keeper_response.py index e736492ae..ead5b3cac 100644 --- a/keepercommander/service/util/parse_keeper_response.py +++ b/keepercommander/service/util/parse_keeper_response.py @@ -26,7 +26,10 @@ def parse_response(command: str, response: Any) -> Dict[str, Any]: Dict[str, Any]: Structured JSON response """ if not response: - return {"status": "success", "data": None} + return { + "success": False, + "error": "The server is temporarily busy. Please try again shortly." + } response_str = str(response).strip() if '--format=json' in command: diff --git a/keepercommander/service/util/request_validation.py b/keepercommander/service/util/request_validation.py new file mode 100644 index 000000000..d7cff5bae --- /dev/null +++ b/keepercommander/service/util/request_validation.py @@ -0,0 +1,64 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' Tuple[Optional[str], Optional[Tuple]]: + """Validate and escape command from request data. + + Args: + request_data: The request JSON data + + Returns: + Tuple of (escaped_command, error_response) where error_response is None on success + """ + if not request_data: + logger.info("Request validation failed: No JSON data provided") + return None, (jsonify({"success": False, "error": "Error: No JSON data provided"}), 400) + + command = request_data.get("command") + if not command: + logger.info("Request validation failed: No command provided") + return None, (jsonify({"success": False, "error": "Error: No command provided"}), 400) + + if not isinstance(command, str): + logger.warning("Request validation failed: Command must be a string") + return None, (jsonify({"success": False, "error": "Error: Command must be a string"}), 400) + + # Escape HTML to prevent XSS + escaped_command = escape(command) + logger.debug(f"Command validated and escaped: {escaped_command}") + return escaped_command, None + + @staticmethod + def validate_request_json() -> Optional[Tuple]: + """Validate that request contains valid JSON. + + Returns: + Error response tuple if validation fails, None if valid + """ + if not request.is_json: + logger.info("Request validation failed: Content-Type must be application/json") + return jsonify({"success": False, "error": "Error: Content-Type must be application/json"}), 400 + + if not request.json: + logger.info("Request validation failed: Invalid or empty JSON") + return jsonify({"success": False, "error": "Error: Invalid or empty JSON"}), 400 + + return None diff --git a/unit-tests/service/test_create_service.py b/unit-tests/service/test_create_service.py index 967a3b1f9..42e1f3b27 100644 --- a/unit-tests/service/test_create_service.py +++ b/unit-tests/service/test_create_service.py @@ -35,7 +35,7 @@ def test_execute_service_already_running(self, mock_service_manager): def test_handle_configuration_streamlined(self): """Test streamlined configuration handling.""" config_data = self.command.service_config.create_default_config() - args = StreamlineArgs(port=8080, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground') + args = StreamlineArgs(port=8080, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled='y') with patch.object(self.command.config_handler, 'handle_streamlined_config') as mock_streamlined: self.command._handle_configuration(config_data, self.params, args) @@ -44,7 +44,7 @@ def test_handle_configuration_streamlined(self): def test_handle_configuration_interactive(self): """Test interactive configuration handling.""" config_data = self.command.service_config.create_default_config() - args = StreamlineArgs(port=None, commands=None, ngrok=None, allowedip='' ,deniedip='', ngrok_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground') + args = StreamlineArgs(port=None, commands=None, ngrok=None, allowedip='' ,deniedip='', ngrok_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled=None) with patch.object(self.command.config_handler, 'handle_interactive_config') as mock_interactive, \ patch.object(self.command.security_handler, 'configure_security') as mock_security: @@ -55,7 +55,7 @@ def test_handle_configuration_interactive(self): def test_create_and_save_record(self): """Test record creation and saving.""" config_data = self.command.service_config.create_default_config() - args = StreamlineArgs(port=8080, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground') + args = StreamlineArgs(port=8080, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled='y') with patch.object(self.command.service_config, 'create_record') as mock_create_record, \ patch.object(self.command.service_config, 'save_config') as mock_save_config: @@ -75,7 +75,7 @@ def test_create_and_save_record(self): def test_validation_error_handling(self): """Test handling of validation errors during execution.""" - args = StreamlineArgs(port=-1, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground') + args = StreamlineArgs(port=-1, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled='y') with patch('builtins.print') as mock_print: with patch.object(self.command.service_config, 'create_default_config') as mock_create_config: diff --git a/unit-tests/service/test_service_config.py b/unit-tests/service/test_service_config.py index cb3e3dcc2..538f6d56d 100644 --- a/unit-tests/service/test_service_config.py +++ b/unit-tests/service/test_service_config.py @@ -28,7 +28,8 @@ def setUp(self): "certfile": "", "certpassword": "", "fileformat": "", - "run_mode": "" + "run_mode": "", + "queue_enabled": "y" } def test_create_default_config(self):