Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security decorator should log client IP address #415

Closed
wants to merge 7 commits into from
Closed
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
25 changes: 21 additions & 4 deletions connexion/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ class Api(object):
def __init__(self, specification, base_url=None, arguments=None,
swagger_json=None, swagger_ui=None, swagger_path=None, swagger_url=None,
validate_responses=False, strict_validation=False, resolver=None,
auth_all_paths=False, debug=False, resolver_error_handler=None, validator_map=None):
auth_all_paths=False, debug=False, resolver_error_handler=None,
validator_map=None, pythonic_params=False, trusted_ips=None):
"""
:type specification: pathlib.Path | dict
:type base_url: str | None
Expand All @@ -81,6 +82,12 @@ def __init__(self, specification, base_url=None, arguments=None,
:param resolver_error_handler: If given, a callable that generates an
Operation used for handling ResolveErrors
:type resolver_error_handler: callable | None
:param pythonic_params: When True CamelCase parameters are converted to snake_case and an underscore is appended
to any shadowed built-ins
:type pythonic_params: bool
:param trusted_ips: A list of trusted IPs. If request.remote_addr is in this list (i.e. it's a proxy we control)
then we'll also trust the X-Forwarded-For HTTP header.
:type trusted_ips: list
"""
self.debug = debug
self.validator_map = validator_map
Expand Down Expand Up @@ -142,6 +149,12 @@ def __init__(self, specification, base_url=None, arguments=None,
logger.debug('Strict Request Validation: %s', str(validate_responses))
self.strict_validation = strict_validation

logger.debug('Pythonic params: %s', str(pythonic_params))
self.pythonic_params = pythonic_params

logger.debug('Trusted IPs: %s', str(trusted_ips))
self.trusted_ips = trusted_ips or []

# Create blueprint and endpoints
self.blueprint = self.create_blueprint()

Expand Down Expand Up @@ -186,7 +199,9 @@ def add_operation(self, method, path, swagger_operation, path_parameters):
validate_responses=self.validate_responses,
validator_map=self.validator_map,
strict_validation=self.strict_validation,
resolver=self.resolver)
resolver=self.resolver,
pythonic_params=self.pythonic_params,
trusted_ips=self.trusted_ips)
self._add_operation_internal(method, path, operation)

def _add_resolver_error_handler(self, method, path, err):
Expand Down Expand Up @@ -266,8 +281,10 @@ def add_auth_on_not_found(self):
Adds a 404 error handler to authenticate and only expose the 404 status if the security validation pass.
"""
logger.debug('Adding path not found authentication')
not_found_error = AuthErrorHandler(werkzeug.exceptions.NotFound(), security=self.security,
security_definitions=self.security_definitions)
not_found_error = AuthErrorHandler(werkzeug.exceptions.NotFound(),
security=self.security,
security_definitions=self.security_definitions,
trusted_ips=self.trusted_ips)
endpoint_name = "{name}_not_found".format(name=self.blueprint.name)
self.blueprint.add_url_rule('/<path:invalid_path>', endpoint_name, not_found_error.function)

Expand Down
7 changes: 5 additions & 2 deletions connexion/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def common_error_handler(exception):

def add_api(self, specification, base_path=None, arguments=None, auth_all_paths=None, swagger_json=None,
swagger_ui=None, swagger_path=None, swagger_url=None, validate_responses=False,
strict_validation=False, resolver=Resolver(), resolver_error=None):
strict_validation=False, resolver=Resolver(), resolver_error=None, pythonic_params=False):
"""
Adds an API to the application based on a swagger file or API dict

Expand Down Expand Up @@ -129,6 +129,8 @@ def add_api(self, specification, base_path=None, arguments=None, auth_all_paths=
:param resolver_error: If specified, turns ResolverError into error
responses with the given status code.
:type resolver_error: int | None
:param pythonic_params: When True CamelCase parameters are converted to snake_case
:type pythonic_params: bool
:rtype: Api
"""
# Turn the resolver_error code into a handler object
Expand Down Expand Up @@ -165,7 +167,8 @@ def add_api(self, specification, base_path=None, arguments=None, auth_all_paths=
strict_validation=strict_validation,
auth_all_paths=auth_all_paths,
debug=self.debug,
validator_map=self.validator_map)
validator_map=self.validator_map,
pythonic_params=pythonic_params)
self.app.register_blueprint(api.blueprint)
return api

Expand Down
34 changes: 31 additions & 3 deletions connexion/decorators/parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,18 @@
import re

import flask
import inflection
import six
import werkzeug.exceptions as exceptions

from ..utils import all_json, boolean, is_null, is_nullable

try:
import builtins
except ImportError:
import __builtin__ as builtins


logger = logging.getLogger(__name__)

# https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#data-types
Expand Down Expand Up @@ -59,11 +66,21 @@ def get_val_from_param(value, query_param):
return make_type(value, query_param["type"])


def sanitize_param(name):
return name and re.sub('^[^a-zA-Z_]+', '', re.sub('[^0-9a-zA-Z_]', '', name))
def snake_and_shadow(name):
"""
Converts the given name into Pythonic form. Firstly it converts CamelCase names to snake_case. Secondly it looks to
see if the name matches a known built-in and if it does it appends an underscore to the name.
:param name: The parameter name
:type name: str
:return:
"""
snake = inflection.underscore(name)
if snake in builtins.__dict__.keys():
return "{}_".format(snake)
return snake


def parameter_to_arg(parameters, consumes, function):
def parameter_to_arg(parameters, consumes, function, pythonic_params=False):
"""
Pass query and body parameters as keyword arguments to handler function.

Expand All @@ -73,8 +90,16 @@ def parameter_to_arg(parameters, consumes, function):
:param consumes: The list of content types the operation consumes
:type consumes: list
:param function: The handler function for the REST endpoint.
:param pythonic_params: When True CamelCase parameters are converted to snake_case and an underscore is appended to
any shadowed built-ins
:type pythonic_params: bool
:type function: function|None
"""
def sanitize_param(name):
if name and pythonic_params:
name = snake_and_shadow(name)
return name and re.sub('^[^a-zA-Z_]+', '', re.sub('[^0-9a-zA-Z_]', '', name))

body_parameters = [parameter for parameter in parameters if parameter['in'] == 'body'] or [{}]
body_name = sanitize_param(body_parameters[0].get('name'))
default_body = body_parameters[0].get('schema', {}).get('default')
Expand Down Expand Up @@ -158,6 +183,9 @@ def wrapper(*args, **kwargs):
logger.debug("File parameter (formData) '%s' in function arguments", key)
kwargs[key] = value

# optionally convert parameter variable names to un-shadowed, snake_case form
if pythonic_params:
kwargs = {snake_and_shadow(k): v for k, v in kwargs.items()}
return function(*args, **kwargs)

return wrapper
34 changes: 27 additions & 7 deletions connexion/decorators/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,27 @@ def get_tokeninfo_url(security_definition):
return token_info_url


def get_client_ip(request, trusted_ips):
if request.headers.get("X-Forwarded-For") and request.remote_addr in trusted_ips:
return request.headers.get("X-Forwarded-For")
return request.remote_addr


def nice_scopes(scopes):
if not scopes:
return "None"
return ','.join(scopes)


def security_passthrough(function):
"""
:type function: types.FunctionType
:rtype: types.FunctionType
:rtype: types.FunctionTyperequest.headers
"""
return function


def verify_oauth(token_info_url, allowed_scopes, function):
def verify_oauth(token_info_url, allowed_scopes, trusted_ips, function):
"""
Decorator to verify oauth

Expand All @@ -49,7 +61,11 @@ def verify_oauth(token_info_url, allowed_scopes, function):
:type allowed_scopes: set
:type function: types.FunctionType
:rtype: types.FunctionType
:param trusted_ips: A list of trusted IPs. If request.remote_addr is in this list (i.e. it's a proxy we control)
then we'll also trust the X-Forwarded-For HTTP header.
:type trusted_ips: list
"""
trusted_ips = trusted_ips or []

@functools.wraps(function)
def wrapper(*args, **kwargs):
Expand All @@ -65,21 +81,25 @@ def wrapper(*args, **kwargs):
raise OAuthProblem(description='Invalid authorization header')
logger.debug("... Getting token from %s", token_info_url)
token_request = session.get(token_info_url, params={'access_token': token}, timeout=5)
logger.debug("... Token info (%d): %s", token_request.status_code, token_request.text)
client_ip = get_client_ip(request, trusted_ips)
logger.debug("... Token info (%d): %s for client IP '%s'",
token_request.status_code,
token_request.text,
client_ip)
if not token_request.ok:
raise OAuthResponseProblem(
description='Provided oauth token is not valid',
token_response=token_request
)
token_info = token_request.json() # type: dict
user_scopes = set(token_info['scope'])
logger.debug("... Scopes required: %s", allowed_scopes)
logger.debug("... User scopes: %s", user_scopes)
logger.debug("... Scopes required: %s", nice_scopes(allowed_scopes))
logger.debug("... User scopes: %s", nice_scopes(user_scopes))
if not allowed_scopes <= user_scopes:
logger.info(textwrap.dedent("""
... User scopes (%s) do not match the scopes necessary to call endpoint (%s).
Aborting with 403.""").replace('\n', ''),
user_scopes, allowed_scopes)
Aborting with 403 for client IP '%s'""").replace('\n', ''),
nice_scopes(user_scopes), nice_scopes(allowed_scopes), client_ip)
raise OAuthScopeProblem(
description='Provided token doesn\'t have the required scope',
required_scopes=allowed_scopes,
Expand Down
8 changes: 6 additions & 2 deletions connexion/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class AuthErrorHandler(SecureOperation):
Wraps an error with authentication.
"""

def __init__(self, exception, security, security_definitions):
def __init__(self, exception, security, security_definitions, trusted_ips=None):
"""
This class uses the exception instance to produce the proper response problem in case the
request is authenticated.
Expand All @@ -23,9 +23,13 @@ def __init__(self, exception, security, security_definitions):
:param security_definitions: `Security Definitions Object
<https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#security-definitions-object>`_
:type security_definitions: dict
:param trusted_ips: A list of trusted IPs. If request.remote_addr is in this list (i.e. it's a proxy we control)
then we'll also trust the X-Forwarded-For HTTP header.
:type trusted_ips: list
"""
self.exception = exception
SecureOperation.__init__(self, security, security_definitions)
trusted_ips = trusted_ips or []
SecureOperation.__init__(self, security, security_definitions, trusted_ips)

@property
def function(self):
Expand Down
20 changes: 16 additions & 4 deletions connexion/operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,20 @@

class SecureOperation(object):

def __init__(self, security, security_definitions):
def __init__(self, security, security_definitions, trusted_ips=None):
"""
:param security: list of security rules the application uses by default
:type security: list
:param security_definitions: `Security Definitions Object
<https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#security-definitions-object>`_
:type security_definitions: dict
:param trusted_ips: A list of trusted IPs. If request.remote_addr is in this list (i.e. it's a proxy we control)
then we'll also trust the X-Forwarded-For HTTP header.
:type trusted_ips: list
"""
self.security = security
self.security_definitions = security_definitions
self.trusted_ips = trusted_ips or []

@property
def security_decorator(self):
Expand Down Expand Up @@ -84,7 +88,7 @@ def security_decorator(self):
token_info_url = get_tokeninfo_url(security_definition)
if token_info_url:
scopes = set(scopes) # convert scopes to set because this is needed for verify_oauth
return functools.partial(verify_oauth, token_info_url, scopes)
return functools.partial(verify_oauth, token_info_url, scopes, self.trusted_ips)
else:
logger.warning("... OAuth2 token info URL missing. **IGNORING SECURITY REQUIREMENTS**",
extra=vars(self))
Expand Down Expand Up @@ -133,7 +137,7 @@ def __init__(self, method, path, operation, resolver, app_produces, app_consumes
path_parameters=None, app_security=None, security_definitions=None,
definitions=None, parameter_definitions=None, response_definitions=None,
validate_responses=False, strict_validation=False, randomize_endpoint=None,
validator_map=None):
validator_map=None, pythonic_params=False, trusted_ips=None):
"""
This class uses the OperationID identify the module and function that will handle the operation

Expand Down Expand Up @@ -177,13 +181,20 @@ def __init__(self, method, path, operation, resolver, app_produces, app_consumes
:type validate_responses: bool
:param strict_validation: True enables validation on invalid request parameters
:type strict_validation: bool
:param pythonic_params: When True CamelCase parameters are converted to snake_case and an underscore is appended
to any shadowed built-ins
:type pythonic_params: bool
:param trusted_ips: A list of trusted IPs. If request.remote_addr is in this list (i.e. it's a proxy we control)
then we'll also trust the X-Forwarded-For HTTP header.
:type trusted_ips: list
"""

self.method = method
self.path = path
self.validator_map = dict(VALIDATOR_MAP)
self.validator_map.update(validator_map or {})
self.security_definitions = security_definitions or {}
self.trusted_ips = trusted_ips or []
self.definitions = definitions or {}
self.parameter_definitions = parameter_definitions or {}
self.response_definitions = response_definitions or {}
Expand All @@ -196,6 +207,7 @@ def __init__(self, method, path, operation, resolver, app_produces, app_consumes
self.strict_validation = strict_validation
self.operation = operation
self.randomize_endpoint = randomize_endpoint
self.pythonic_params = pythonic_params

# todo support definition references
# todo support references to application level parameters
Expand Down Expand Up @@ -361,7 +373,7 @@ def function(self):
"""

function = parameter_to_arg(
self.parameters, self.consumes, self.__undecorated_function)
self.parameters, self.consumes, self.__undecorated_function, self.pythonic_params)
function = self._request_begin_lifecycle_decorator(function)

if self.validate_responses:
Expand Down
4 changes: 3 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def read_version(package):
'six>=1.9',
'strict-rfc3339>=0.6',
'swagger-spec-validator>=2.0.2',
'inflection>=0.3.1'
]

if py_major_minor_version < (3, 4):
Expand All @@ -42,7 +43,8 @@ def read_version(package):
'decorator',
'mock',
'pytest',
'pytest-cov'
'pytest-cov',
'testfixtures'
]


Expand Down
21 changes: 21 additions & 0 deletions tests/api/test_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,3 +309,24 @@ def test_param_sanitization(simple_app):
headers={'Content-Type': 'application/json'})
assert resp.status_code == 200
assert json.loads(resp.data.decode('utf-8', 'replace')) == body


def test_parameters_snake_case(snake_case_app):
app_client = snake_case_app.app.test_client()
headers = {'Content-type': 'application/json'}
resp = app_client.post('/v1.0/test-post-path-snake/123', headers=headers, data=json.dumps({"a": "test"}))
assert resp.status_code == 200
resp = app_client.post('/v1.0/test-post-path-shadow/123', headers=headers, data=json.dumps({"a": "test"}))
assert resp.status_code == 200
resp = app_client.post('/v1.0/test-post-query-snake?someId=123', headers=headers, data=json.dumps({"a": "test"}))
assert resp.status_code == 200
resp = app_client.post('/v1.0/test-post-query-shadow?id=123', headers=headers, data=json.dumps({"a": "test"}))
assert resp.status_code == 200
resp = app_client.get('/v1.0/test-get-path-snake/123')
assert resp.status_code == 200
resp = app_client.get('/v1.0/test-get-path-shadow/123')
assert resp.status_code == 200
resp = app_client.get('/v1.0/test-get-query-snake?someId=123')
assert resp.status_code == 200
resp = app_client.get('/v1.0/test-get-query-shadow?list=123')
assert resp.status_code == 200
5 changes: 5 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ def simple_app():
return build_app_from_fixture('simple', validate_responses=True)


@pytest.fixture(scope="session")
def snake_case_app():
return build_app_from_fixture('snake_case', validate_responses=True, pythonic_params=True)


@pytest.fixture(scope="session")
def invalid_resp_allowed_app():
return build_app_from_fixture('simple', validate_responses=False)
Expand Down
Loading