Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions keepercommander/commands/pam/gateway_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ def create_gateway(params, gateway_name, ksm_app, config_init, ott_expire_in_min

one_time_token = config_str_and_config_dict.get('config_str')

# New controller may not appear in a warm get_all_gateways() cache.
invalidate_gateway_cache()
return one_time_token


Expand Down
6 changes: 5 additions & 1 deletion keepercommander/service/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ result_retention: 3600 # Result retention (1 hour)
- **Default limits**: 60/minute, 600/hour, 6000/day
- **Per-endpoint tracking**: Each API endpoint has independent rate limit counters
- **Example**: Setting `"20/minute"` provides 20 requests per minute per endpoint per IP address
- Service rate limits are local (per client IP/endpoint). They are not the same as Keeper’s per-user API throttle; one service HTTP call may trigger many Keeper API calls.

#### Error Responses

Expand All @@ -224,7 +225,10 @@ result_retention: 3600 # Result retention (1 hour)
- **401 Unauthorized**: Missing, invalid, or expired API key; no active session
- **403 Forbidden**: IP not allowed, access denied, or command not in allowed list
- **404 Not Found**: Request ID not found
- **429 Too Many Requests**: Rate limit exceeded
- **429 Too Many Requests**: JSON `{"status":"error","error":"<message>","result_code":"..."}`
- `rate_limited` — service-local Flask limiter
- `throttled` — upstream Keeper API throttle (same as Commander)
- `429` — upstream edge/gateway “Too Many Requests”

**Server Errors (5xx):**
- **500 Internal Server Error**: Command execution failed or unexpected server error
Expand Down
10 changes: 9 additions & 1 deletion keepercommander/service/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@
# Contact: ops@keepersecurity.com
#

from flask import Flask
from flask import Flask, jsonify
import logging
from werkzeug.middleware.proxy_fix import ProxyFix
from flask_limiter.errors import RateLimitExceeded
from .decorators.security import limiter, is_behind_proxy
from .decorators.api_logging import SSLHandshakeFilter
from .api.routes import init_routes
from .decorators.logging import logger
from .util.throttle import rate_limited_response


def create_app():
Expand All @@ -35,6 +37,12 @@ def create_app():
logger.debug("Configuring rate limiter")
limiter.init_app(app)

@app.errorhandler(RateLimitExceeded)
def handle_rate_limit_exceeded(e):
detail = getattr(e, 'description', None) or str(e)
body, status = rate_limited_response(detail)
return jsonify(body), status

logger.debug("Initializing API routes")
init_routes(app)

Expand Down
65 changes: 42 additions & 23 deletions keepercommander/service/util/command_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,18 @@
from .config_reader import ConfigReader
from .exceptions import CommandExecutionError
from .parse_keeper_response import parse_keeper_response, ensure_record_add_json_format
from .throttle import (
RESULT_EDGE_429,
RESULT_THROTTLED,
is_throttle_error,
throttle_error_response,
)
from ..core.globals import get_current_params
from ..decorators.logging import logger, debug_decorator, sanitize_debug_data
from ... import cli, utils
from ...crypto import encrypt_aes_v2
from ...error import KeeperApiError


class CommandExecutor:
@staticmethod
Expand Down Expand Up @@ -114,6 +122,28 @@ def encrypt_response(response: Any) -> bytes:
raise
return response

@staticmethod
def _status_code_from_response(response: dict) -> int:
if 'status_code' in response:
return response.pop('status_code')
if response.get('status') in ('error', 'warning'):
return 400
return 200

@classmethod
def _finalize_parsed_response(cls, response: Any) -> Tuple[Any, int]:
if not isinstance(response, dict):
return response, 200

status_code = cls._status_code_from_response(response)
result_code = response.get('result_code')
if status_code == 429 or result_code in (RESULT_THROTTLED, RESULT_EDGE_429):
body, status_code = throttle_error_response(response.get('error'), result_code)
if 'command' in response:
body['command'] = response['command']
return body, status_code
return response, status_code

@classmethod
def execute(cls, command: str) -> Tuple[Any, int]:
logger.debug(f"Executing command: {command}")
Expand Down Expand Up @@ -142,36 +172,25 @@ def execute(cls, command: str) -> Tuple[Any, int]:

# Always let the parser handle the response (including empty responses and logs)
response = parse_keeper_response(command, response, log_output)

if isinstance(response, dict):
# Extract status_code and remove it from response body
if 'status_code' in response:
status_code = response.pop('status_code')
elif response.get("status") == "error":
status_code = 400
elif response.get("status") == "warning":
status_code = 400
else:
status_code = 200
else:
status_code = 200
response, status_code = cls._finalize_parsed_response(response)

response = CommandExecutor.encrypt_response(response)
logger.debug(f"Command executed successfully")
return response, status_code
except CommandExecutionError as e:
# Return the actual command error instead of generic "server busy"
logger.error(f"Command execution error: {e}")
error_response = {
"status": "error",
"error": str(e)
}
return error_response, 400
if is_throttle_error(e):
return throttle_error_response(str(e))
return {"status": "error", "error": str(e)}, 400
except KeeperApiError as e:
if is_throttle_error(e):
return throttle_error_response(e.message or str(e), e.result_code)
logger.error(f"Unexpected error during command execution: {e}")
return {"status": "error", "error": f"Unexpected error: {str(e)}"}, 500
except Exception as e:
if is_throttle_error(e):
return throttle_error_response(str(e))
# Log unexpected errors and return a proper error response
logger.error(f"Unexpected error during command execution: {e}")
error_response = {
"status": "error",
"error": f"Unexpected error: {str(e)}"
}
return error_response, 500
return {"status": "error", "error": f"Unexpected error: {str(e)}"}, 500
13 changes: 12 additions & 1 deletion keepercommander/service/util/parse_keeper_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
from typing import Any, Dict, List, Optional, Tuple
import re, json

from .throttle import RESULT_THROTTLED, clean_throttle_message, is_throttle_text

class KeeperResponseParser:
@staticmethod
def _clean_ansi_codes(text: str) -> str:
Expand Down Expand Up @@ -1054,7 +1056,16 @@ def _parse_logging_based_command(command: str, response_str: str) -> Dict[str, A
]

has_success_indicator = any(indicator in response_lower for indicator in success_indicators)


if is_throttle_text(response_str):
return {
"status": "error",
"status_code": 429,
"command": command.split()[0] if command.split() else command,
"error": clean_throttle_message(response_str),
"result_code": RESULT_THROTTLED,
}

forbidden_patterns = [
"not an msp administrator",
"permission denied",
Expand Down
96 changes: 96 additions & 0 deletions keepercommander/service/util/throttle.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# _ __
# | |/ /___ ___ _ __ ___ _ _ ®
# | ' </ -_) -_) '_ \/ -_) '_|
# |_|\_\___\___| .__/\___|_|
# |_|
#
# Keeper Commander
# Copyright 2024 Keeper Security Inc.
# Contact: ops@keepersecurity.com
#

"""Service-mode mapping for local rate limits and upstream Keeper/edge throttling."""

from typing import Any, Optional, Tuple, Union

from ...error import KeeperApiError

RESULT_THROTTLED = 'throttled'
RESULT_RATE_LIMITED = 'rate_limited'
RESULT_EDGE_429 = '429'

_THROTTLE_TEXT_MARKERS = ('throttled', 'too many requests')


def is_throttle_text(text: str) -> bool:
if not text:
return False
lowered = text.lower()
return any(marker in lowered for marker in _THROTTLE_TEXT_MARKERS)


def is_throttle_error(exc_or_text: Union[BaseException, str]) -> bool:
if isinstance(exc_or_text, KeeperApiError):
code = exc_or_text.result_code
if code == RESULT_THROTTLED or code == 429 or str(code) == RESULT_EDGE_429:
return True
return is_throttle_text(exc_or_text.message or str(exc_or_text))
return is_throttle_text(str(exc_or_text or ''))


def normalize_throttle_result_code(result_code: Any = None) -> str:
if result_code in (None, ''):
return RESULT_THROTTLED
if result_code == 429 or str(result_code) == RESULT_EDGE_429:
return RESULT_EDGE_429
return str(result_code)


def clean_throttle_message(message: Optional[str]) -> str:
"""Prefer Keeper's API message; drop duplicated client retry log noise."""
if not message:
return 'Request throttled'

text = message.replace('\\n', '\n')
for line in text.splitlines():
line = line.strip()
if not line:
continue
lowered = line.lower()
# Skip Commander client retry warnings captured from logging
if 'retrying in' in lowered or ('throttled (' in lowered and 'attempt' in lowered):
continue
if 'too many requests' in lowered:
return 'Too Many Requests'
if 'throttled' in lowered or 'due to repeated attempts' in lowered:
if lowered.startswith('throttled:'):
return line.split(':', 1)[1].strip() or line
return line

if 'retrying in' in text.lower():
return 'Request throttled by Keeper API'

first_line = text.strip().splitlines()[0]
return first_line[:500]


def throttle_error_response(
message: Optional[str] = None,
result_code: Any = RESULT_THROTTLED,
) -> Tuple[dict, int]:
return {
'status': 'error',
'error': clean_throttle_message(message),
'result_code': normalize_throttle_result_code(result_code),
}, 429


def rate_limited_response(detail: Optional[str] = None) -> Tuple[dict, int]:
error = 'Service rate limit exceeded'
if detail:
error = f'{error}: {detail}'
return {
'status': 'error',
'error': error,
'result_code': RESULT_RATE_LIMITED,
}, 429
Loading