merge changes to upstream: ldap secure connections, log/exception han…

…dling, AWS SessionDurations, userPrincipalName as LDAP search filter

.travis.yml

@@ -16,7 +16,7 @@ install:
  - pip install pybuilder
  - pip install coveralls
  - pyb install_dependencies
- 
+
 script: "pyb -X verify analyze"
 after_success:
  - coveralls --verbose

README.rst

@@ -10,7 +10,7 @@
 .. image:: https://landscape.io/github/ImmobilienScout24/afp-core/master/landscape.svg?style=flat
  :target: https://landscape.io/github/ImmobilienScout24/afp-core/master
  :alt: Code Health
- 
+
 ========
 afp-core
 ========

build.py

@@ -8,7 +8,6 @@
 use_plugin("python.coverage")
 use_plugin("python.distutils")
 use_plugin("copy_resources")
-use_plugin("filter_resources")
 
 name = 'afp-core'
 summary = 'API and backend for temporary authentication to aws services'
@@ -40,6 +39,7 @@ def set_properties(project):
  project.depends_on("yamlreader")
  project.depends_on("bottle")
  project.depends_on("boto>=2.38.0")
+ project.depends_on("ldap")
 
  project.set_property("verbose", True)
  project.set_property('flake8_include_test_sources', True)

src/main/python/aws_federation_proxy/aws_federation_proxy.py

@@ -142,10 +142,8 @@ def get_aws_credentials(self, account_alias, role):
  role_session_name=self.user)
  except Exception as error:
  if getattr(error, 'status', None) == 403:
- raise PermissionError(str(error))
- self.logger.exception("AWS STS failed with: {exc_vars}".format(
- exc_vars=vars(error)))
- raise AWSError(str(error))
+ raise PermissionError(error.message)
+ raise AWSError(error.message)
  return assumed_role_object.credentials
 
  @staticmethod
@@ -171,7 +169,6 @@ def _get_signin_token(cls, credentials):
  request_url = (
  "https://signin.aws.amazon.com/federation"
  "?Action=getSigninToken"
- "&SessionDuration=43200"
  "&Session=" +
  cls._generate_urlencoded_json_credentials(credentials))
  reply = requests.get(request_url)

src/main/python/aws_federation_proxy/provider/base_provider.py

@@ -94,13 +94,11 @@ def get_accounts_and_roles(self):
 
 
 class SingleAccountSingleRoleProvider(BaseProvider):
- """A sample Provider, for testing only"""
  def get_accounts_and_roles(self):
  return {'the_only_account': set([('the_only_role', 'because I said so')])}
 
 
 class NoAccountNoRoleProvider(BaseProvider):
- """A sample Provider, for testing only"""
  def get_accounts_and_roles(self):
  return {}
 

src/main/python/aws_federation_proxy/provider/ldap_provider.py

@@ -15,16 +15,23 @@ def get_group_list(self):
  ldap_base_groups = self.config['ldap_base_groups']
  ldap_bind_dn = self.config['ldap_bind_dn']
  ldap_bind_password = self.config['ldap_bind_password']
+ ldap_starttls = self.config['ldap_starttls']
 
  l = ldap.initialize(ldap_uri)
 
  self.logger.debug('User: "%s"', self.user.lower())
  search_filter = '(|(&(objectClass=user)' \
- '(sAMAccountName=%s)))' \
+ '(userPrincipalName=%s)))' \
  % self.user.lower()
 
  self.logger.debug('User DN Search Filter: "%s"', search_filter)
  try:
+ l.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, ldap.OPT_X_TLS_DEMAND)
+
+ # start TLS onlyif requested and not on ldaps connects (already SSL!)
+ if ldap_starttls and not ldap_uri.startswith("ldaps:"):
+ l.start_tls_s()
+
  l.simple_bind_s(ldap_bind_dn, ldap_bind_password)
 
  # Find user's DN

src/main/python/aws_federation_proxy/provider/provider_by_ip.py

@@ -4,7 +4,6 @@
 
 from socket import gethostbyaddr
 
-from aws_federation_proxy import PermissionError
 from aws_federation_proxy.provider import BaseProvider
 
 
@@ -18,13 +17,7 @@ class ProviderByIP(BaseProvider):
  def get_accounts_and_roles(self):
  """Return a dict with one account and one aws role"""
  self.role_prefix = self.config.get('role_prefix', "")
- try:
- self.client_fqdn = gethostbyaddr(self.user)[0]
- except Exception as exc:
- # The exception message of gethostbyaddr() is quite useless since
- # it does not include the address that was looked up.
- message = "Lookup for '{0}' failed: {1}".format(self.user, exc)
- raise Exception(message)
+ self.client_fqdn = gethostbyaddr(self.user)[0]
  self.check_host_allowed()
  self._get_role_name()
  reason = "Machine {0} (FQDN {1}) matched the role {2}".format(
@@ -35,7 +28,7 @@ def check_host_allowed(self):
  self.client_host, self.client_domain = self.client_fqdn.split(".", 1)
  allowed_domains = self.config['allowed_domains']
  if self.client_domain not in allowed_domains:
- raise PermissionError("Client IP {0} (FQDN {1}) is not permitted".format(
+ raise Exception("Client IP {0} (FQDN {1}) is not permitted".format(
  self.user, self.client_fqdn))
 
  def _get_role_name(self):
@@ -54,8 +47,7 @@ def _get_role_name(self):
  def check_host_allowed(self):
  super(Provider, self).check_host_allowed()
  if len(self.client_host) != 8:
- raise PermissionError(
- "Client {0} has an invalid name".format(self.client_fqdn))
+ raise Exception("Client {0} has an invalid name".format(self.client_fqdn))
 
  def _normalize_loctyp(self):
  """Return the normalized (ber/ham -> pro) loctyp of self.client_host"""

src/main/python/aws_federation_proxy/wsgi_api/__init__.py

@@ -2,6 +2,6 @@
 # -*- coding: utf-8 -*-
 from __future__ import print_function, absolute_import, unicode_literals, division
 
-from aws_federation_proxy.wsgi_api.wsgi_api import get_webapp, LOGGER_NAME
+from aws_federation_proxy.wsgi_api.wsgi_api import get_webapp
 
-__all__ = ['get_webapp', 'LOGGER_NAME']
+__all__ = ['get_webapp']

src/main/python/aws_federation_proxy/wsgi_api/wsgi_api.py

@@ -3,7 +3,7 @@
 from __future__ import print_function, absolute_import, division
 
 import datetime
-import logging
+import traceback
 import simplejson
 
 from aws_federation_proxy import (
@@ -18,28 +18,21 @@
 from aws_federation_proxy.util import setup_logging
 
 
-LOGGER_NAME = 'AWSFederationProxy'
-
-
 def with_exception_handling(old_function):
  """Decorator function to ensure proper exception handling"""
  @wraps(old_function)
  def new_function(*args, **kwargs):
- logger = logging.getLogger(LOGGER_NAME)
  try:
  result = old_function(*args, **kwargs)
- except ConfigurationError:
- logger.exception("Call to '%s' failed:", old_function.__name__)
- abort(404, "ConfigurationError")
- except AWSError:
- logger.exception("AWS call in '%s' failed:", old_function.__name__)
- abort(502, "Call to AWS failed")
- except PermissionError:
- logger.exception("Permission denied:")
- abort(403, "Permission Denied")
+ except ConfigurationError as err:
+ abort(404, err)
+ except AWSError as err:
+ abort(502, err)
+ except PermissionError as err:
+ abort(403, err)
  except Exception:
- logger.exception("Call to '%s' failed:", old_function.__name__)
- abort(500, "Internal Server Error")
+ message = traceback.format_exc()
+ abort(500, "Exception caught {0}".format(message))
  return result
  return new_function
 
@@ -50,18 +43,18 @@ def initialize_federation_proxy(user=None):
  if config_path is None:
  raise Exception("No Config Path specified")
  config = yaml_load(config_path)
-
- try:
- logger = setup_logging(config, logger_name=LOGGER_NAME)
- except Exception as exc:
- raise ConfigurationError(str(exc))
-
- if user is None:
- user = get_user(config['api']['user_identification'])
  account_config_path = request.environ.get('ACCOUNT_CONFIG_PATH')
  if account_config_path is None:
  raise Exception("No Account Config Path specified")
  account_config = yaml_load(account_config_path)
+
+ if user is None:
+ user = get_user(config['api']['user_identification'])
+
+ try:
+ logger = setup_logging(config, logger_name='AWSFederationProxy')
+ except Exception as exc:
+ raise ConfigurationError(str(exc))
  proxy = AWSFederationProxy(user=user, config=config,
  account_config=account_config, logger=logger)
  return proxy

src/unittest/python/api_endpoint_tests.py

@@ -198,8 +198,7 @@ def test_status_broken_providerconfig_must_be_reported(self):
  }
  }
  self._create_app()
- with self.assertLogs(wsgi_api.LOGGER_NAME, logging.ERROR):
- result = self.app.get('/status', expect_errors=True)
+ result = self.app.get('/status', expect_errors=True)
  self.assertEqual(result.status_int, 404)
 
  def test_account_broken_providerconfig_must_be_reported(self):
@@ -361,17 +360,17 @@ def test_403_on_illegal_role(self, mock_get):
  self.assertEqual(result.status_int, 403)
  self.assertEqual(self.user, result.headers['X-Username'])
 
- result.mustcontain("Permission Denied")
+ result.mustcontain("Forbidden")
  result = self.app.get('/account/testaccount/illegalrole/credentials',
  expect_errors=True)
  self.assertEqual(result.status_int, 403)
- result.mustcontain("Permission Denied")
+ result.mustcontain("Forbidden")
  self.assertEqual(self.user, result.headers['X-Username'])
 
  result = self.app.get('/account/testaccount/illegalrole/consoleurl',
  expect_errors=True)
  self.assertEqual(result.status_int, 403)
- result.mustcontain("Permission Denied")
+ result.mustcontain("Forbidden")
  self.assertEqual(self.user, result.headers['X-Username'])
 
  logged_data = str(self.log_file.read())
@@ -390,19 +389,19 @@ def test_403_on_illegal_account(self, mock_get):
  result = self.app.get('/account/testaccount1/testrole',
  expect_errors=True)
  self.assertEqual(result.status_int, 403)
- result.mustcontain("Permission Denied")
+ result.mustcontain("Forbidden")
  self.assertEqual(self.user, result.headers['X-Username'])
 
  result = self.app.get('/account/testaccount1/testrole/credentials',
  expect_errors=True)
  self.assertEqual(result.status_int, 403)
- result.mustcontain("Permission Denied")
+ result.mustcontain("Forbidden")
  self.assertEqual(self.user, result.headers['X-Username'])
 
  result = self.app.get('/account/testaccount1/testrole/consoleurl',
  expect_errors=True)
  self.assertEqual(result.status_int, 403)
- result.mustcontain("Permission Denied")
+ result.mustcontain("Forbidden")
  self.assertEqual(self.user, result.headers['X-Username'])
 
  logged_data = str(self.log_file.read())
@@ -415,16 +414,13 @@ def test_403_on_illegal_account(self, mock_get):
  def test_502_on_aws_failure(self, mock_get_aws_credentials):
  mock_get_aws_credentials.side_effect = AWSError("aws is down")
 
- with self.assertLogs(wsgi_api.LOGGER_NAME, logging.ERROR):
- result = self.app.get('/account/testaccount/testrole',
- expect_errors=True)
+ result = self.app.get('/account/testaccount/testrole', expect_errors=True)
  self.assertEqual(result.status_int, 502)
- result.mustcontain("Call to AWS failed")
+ result.mustcontain("aws is down")
  self.assertEqual(self.user, result.headers['X-Username'])
 
  @patch("aws_federation_proxy.aws_federation_proxy.AWSFederationProxy.get_aws_credentials")
  def test_all_exceptions_are_loggged(self, mock_get_aws_credentials):
  mock_get_aws_credentials.side_effect = Exception("some random exception")
 
- with self.assertLogs(wsgi_api.LOGGER_NAME, logging.ERROR):
- self.app.get('/account/testaccount/testrole', expect_errors=True)
+ self.app.get('/account/testaccount/testrole', expect_errors=True)

src/unittest/python/aws_federation_proxy_tests.py

@@ -265,17 +265,16 @@ class FakeHTTPError(Exception):
  @patch("aws_federation_proxy.AWSFederationProxy.check_user_permissions")
  def test_get_aws_credentials_handles_sts_errors(
  self, mock_check_user_permissions, mock_sts_connection):
- """Other errors must raise AWSError and log the AWS request ID"""
- fake_boto_error = boto.exception.AWSConnectionError("AWS message")
- fake_boto_error.request_id = "111222333"
+ """Other errors must raise AWSError and contain the Error """
+ error_message = 'AWS message'
+ fake_boto_error = boto.exception.AWSConnectionError(error_message)
  mock_sts_connection.side_effect = fake_boto_error
 
- with self.assertLogs(level='ERROR') as cm:
- self.assertRaises(
- AWSError,
- self.proxy.get_aws_credentials, self.account_alias, self.role)
- all_log_messages = "".join(cm.output)
- self.assertIn(fake_boto_error.request_id, all_log_messages)
+ try:
+ self.proxy.get_aws_credentials(self.account_alias, self.role)
+ self.assertFail()
+ except AWSError as error:
+ self.assertIn(error_message, error)
 
  @mock_sts
  @patch("aws_federation_proxy.AWSFederationProxy.check_user_permissions")

src/unittest/python/machine_provider_tests.py

@@ -4,7 +4,6 @@
 
 from mock import patch
 from base_provider_tests import BaseProviderTest
-from aws_federation_proxy import PermissionError
 from aws_federation_proxy.provider.provider_by_ip import Provider
 
 ACCOUNT_NAME = "testaccount"
@@ -63,12 +62,12 @@ def test_forbidden_domains_raise_exception(self, mock_gethostbyaddr):
  mock_gethostbyaddr.return_value = ["tuvfoo42.invalid"]
  self.config['allowed_domains'] = ["valid"]
  provider = self.testclass("tuvfoo42.invalid", self.config)
- self.assertRaises(PermissionError, provider.get_accounts_and_roles)
+ self.assertRaises(Exception, provider.get_accounts_and_roles)
 
  @patch("aws_federation_proxy.provider.provider_by_ip.gethostbyaddr")
  def test_invalid_names_raise_exception(self, mock_gethostbyaddr):
  # host name is one character too short
  mock_gethostbyaddr.return_value = ["tuvfoo4.valid"]
  self.config['allowed_domains'] = ["something.else", "valid"]
  provider = self.testclass("tuvfoo42.valid", self.config)
- self.assertRaises(PermissionError, provider.get_accounts_and_roles)
+ self.assertRaises(Exception, provider.get_accounts_and_roles)