Fix external auth (REMOTE_USER) plugin support

According to the WSGI specification "REMOTE_USER should be the string
username of the user, nothing more" [1], therefore no modifications
should be made to the REMOTE_USER variable and it should be fully
considered as the username. Otherwise the expected semantics of the
REMOTE_USER variable change, and an site administrator could get
undesirable side-effects.

[1] http://wsgi.readthedocs.org/en/latest/specifications/simple_authentication.html#specification

Moreover, it is important to have a consistent behaviour regarding
external authentication in V2 (not domain aware), V3 with default
domain and V3 with domain (see Bug #1253484) so that we produce similar
results with the three methods.

This change aims to solve this issues by removing the split of the
REMOTE_USER variable by "@" at all:

- In external.DefaultDomain, we cannot split REMOTE_USER by "@". This split
  will cause errors for remote users containing an "@" (not only
  emails, but also X.509 subjects, etc). The external.DefaultDomain plugin
  considers the REMOTE_USER variable as the username, and the configured
  default domain as the domain

- In external.Domain we should not split also the REMOTE_USER by "@". A
  new environment variable (REMOTE_DOMAIN) is introduced, so that any
  external plugin can pass down the right domain for the user. The
  external.Domain plugin considers the REMOTE_USER as the username, the
  REMOTE_DOMAIN as the domain if it is present, otherwise it takes the
  configured default domain.

- Two legacy plugins are also provided with the same behaviour as the
  Havana shipped ones. This plugins should not be used and are provided
  for compatibility reasons (see Bug #1254619)

Closes-Bug: #1254619
Closes-Bug: #1211233
Closes-Bug: #1253484

DocImpact: This change breaks backwards compatibility in favour of
security (see bug #1254619), therefore an upgrade not is needed. It is
needed to document the new plugins and state clearly the semantics of
the REMOTE_USER and REMOTE_DOMAIN variable for the WSGI filters. The
default external authentication plugin has been changed from
exernal.ExternalDefault to external.Default.

Change-Id: I1b2521a526fa976146dfe2fcf4d4c1851416d8ae

doc/source/configuration.rst

@@ -129,7 +129,8 @@ file. It is up to the plugin to register its own configuration options.
 Keystone provides three authentication methods by default. ``password`` handles password
 authentication and ``token`` handles token authentication.  ``external`` is used in conjunction
 with authentication performed by a container web server that sets the ``REMOTE_USER``
-environment variable.
+environment variable. For more details, refer to :doc:`External Authentication
+<external-auth>`.
 
 How to Implement an Authentication Plugin
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -174,7 +175,8 @@ agree on the ``user_id`` in the ``auth_context``.
 The ``REMOTE_USER`` environment variable is only set from a containing webserver.  However,
 to ensure that a user must go through other authentication mechanisms, even if this variable
 is set, remove ``external`` from the list of plugins specified in ``methods``.  This effectively
-disables external authentication.
+disables external authentication.  For more details, refer to :doc:`External
+Authentication <external-auth>`.
 
 
 Token Provider

doc/source/external-auth.rst

@@ -1,21 +1,51 @@
 ===========================================
 Using external authentication with Keystone
 ===========================================
+.. _external-auth:
 
-When Keystone is executed in :doc:`HTTPD <apache-httpd>` it is possible to
+When Keystone is executed in a web server like :doc:`Apache HTTPD
-use external authentication methods different from the authentication
+<apache-httpd>` it is possible to use external authentication methods different
-provided by the identity store backend. For example, this makes possible to
+from the authentication provided by the identity store backend or the different
-use a SQL identity backend together with X.509 authentication, Kerberos, etc.
+authentication plugins. For example, this makes possible to use an SQL identity
-instead of using the username/password combination.
+backend together with, X.509 authentication or Kerberos, for example, instead
+of using the username and password combination.
+
+When a web server is in charge of authentication, it is normally possible to
+set the ``REMOTE_USER`` environment variable so that it can be used in the
+underlying application. Keystone can be configured to use that environment
+variable if set, so that the authentication is handled by the web server.
+
+Configuration
+=============
+
+In Identity API v2, there is no way to disable external authentication. In
+order to activate the external authentication mechanism for Identity API v3,
+the ``external`` method must be in the list of enabled authentication methods.
+By default it is enabled, so if you don't want to use external authentication,
+remove it from the ``methods`` option in the ``auth`` section.
+
+To configure the plugin that should be used set the ``external`` option again
+in the ``auth`` section. There are two external authentication method plugins
+provided by Keystone:
+
+* ``keystone.auth.plugins.external.Default``: This plugin won't take into
+  account the domain information that the external authentication method may
+  pass down to Keystone and will always use the configured default domain. The
+  ``REMOTE_USER`` variable is the username.
+
+* ``keystone.auth.plugins.external.Domain``: This plugin expects that the
+  ``REMOTE_DOMAIN`` variable contains the domain for the user. If this variable
+  is not present, the configured default domain will be used. The
+  ``REMOTE_USER`` variable is the username.
 
 Using HTTPD authentication
 ==========================
 
-Webservers like Apache HTTP support many methods of authentication. Keystone can
+Web servers like Apache HTTP support many methods of authentication. Keystone
-profit from this feature and let the authentication be done in the webserver,
+can profit from this feature and let the authentication be done in the web
-that will pass down the authenticated user to Keystone using the ``REMOTE_USER``
+server, that will pass down the authenticated user to Keystone using the
-environment variable. This user must exist in advance in the identity backend
+``REMOTE_USER`` environment variable. This user must exist in advance in the
-so as to get a token from the controller.
+identity backend so as to get a token from the controller.
 
 To use this method, Keystone should be running on :doc:`HTTPD <apache-httpd>`.
 
@@ -47,13 +77,14 @@ custom authentication mechanisms using the ``REMOTE_USER`` WSGI environment
 variable.
 
 .. ATTENTION::
+
     Please note that even if it is possible to develop a custom authentication
     module, it is preferable to use the modules in the HTTPD server. Such
     authentication modules in webservers like Apache have normally undergone
-    years of development and use in production systems and are actively maintained
+    years of development and use in production systems and are actively
-    upstream. Developing a custom authentication module that implements the same
+    maintained upstream. Developing a custom authentication module that
-    authentication as an existing Apache module likely introduces a higher
+    implements the same authentication as an existing Apache module likely
-    security risk.
+    introduces a higher security risk.
 
 If you find you must implement a custom authentication mechanism, you will need
 to develop a custom WSGI middleware pipeline component. This middleware should
@@ -94,19 +125,21 @@ Pipeline configuration
 ----------------------
 
 Once you have your WSGI middleware component developed you have to add it to
-your pipeline. The first step is to add the middleware to your configuration file.
+your pipeline. The first step is to add the middleware to your configuration
-Assuming that your middleware module is ``keystone.middleware.MyMiddlewareAuth``,
+file. Assuming that your middleware module is
-you can configure it in your ``keystone-paste.ini`` as::
+``keystone.middleware.MyMiddlewareAuth``, you can configure it in your
+``keystone-paste.ini`` as::
 
     [filter:my_auth]
     paste.filter_factory = keystone.middleware.MyMiddlewareAuth.factory
 
-The second step is to add your middleware to the pipeline. The exact place where
+The second step is to add your middleware to the pipeline. The exact place
-you should place it will depend on your code (i.e. if you need for example that
+where you should place it will depend on your code (i.e. if you need for
-the request body is converted from JSON before perform the authentication you
+example that the request body is converted from JSON before perform the
-should place it after the ``json_body`` filter) but it should be set before the
+authentication you should place it after the ``json_body`` filter) but it
-``public_service`` (for the ``public_api`` pipeline) or ``admin_service`` (for
+should be set before the ``public_service`` (for the ``public_api`` pipeline)
-the ``admin_api`` pipeline), since they consume authentication.
+or ``admin_service`` (for the ``admin_api`` pipeline), since they consume
+authentication.
 
 For example, if the original pipeline looks like this::
 

keystone/auth/plugins/external.py

@@ -14,7 +14,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-"""Keystone External Authentication Plugin"""
+"""Keystone External Authentication Plugins"""
 
 import abc
 
@@ -24,6 +24,7 @@ from keystone import auth
 from keystone.common import config
 from keystone import exception
 from keystone.openstack.common import log as logging
+from keystone.openstack.common import versionutils
 
 
 LOG = logging.getLogger(__name__)
@@ -45,7 +46,7 @@ class Base(auth.AuthMethodHandler):
             msg = _('No authenticated user')
             raise exception.Unauthorized(msg)
         try:
-            user_ref = self._authenticate(REMOTE_USER, auth_info)
+            user_ref = self._authenticate(REMOTE_USER, context, auth_info)
             auth_context['user_id'] = user_ref['id']
             if ('kerberos' in CONF.token.bind and
                 (context['environment'].get('AUTH_TYPE', '').lower()
@@ -56,7 +57,7 @@ class Base(auth.AuthMethodHandler):
             raise exception.Unauthorized(msg)
 
     @abc.abstractmethod
-    def _authenticate(self, remote_user):
+    def _authenticate(self, remote_user, context, auth_info):
         """Look up the user in the identity backend.
 
         Return user_ref
@@ -64,9 +65,78 @@ class Base(auth.AuthMethodHandler):
         pass
 
 
-class Default(Base):
+class DefaultDomain(Base):
-    def _authenticate(self, remote_user, auth_info):
+    def _authenticate(self, remote_user, context, auth_info):
         """Use remote_user to look up the user in the identity backend."""
+        domain_id = CONF.identity.default_domain_id
+        user_ref = auth_info.identity_api.get_user_by_name(remote_user,
+                                                           domain_id)
+        return user_ref
+
+
+class Domain(Base):
+    def _authenticate(self, remote_user, context, auth_info):
+        """Use remote_user to look up the user in the identity backend.
+
+        The domain will be extracted from the REMOTE_DOMAIN environment
+        variable if present. If not, the default domain will be used.
+        """
+
+        username = remote_user
+        try:
+            domain_name = context['environment']['REMOTE_DOMAIN']
+        except KeyError:
+            domain_id = CONF.identity.default_domain_id
+        else:
+            domain_ref = (auth_info.identity_api.
+                          get_domain_by_name(domain_name))
+            domain_id = domain_ref['id']
+
+        user_ref = auth_info.identity_api.get_user_by_name(username,
+                                                           domain_id)
+        return user_ref
+
+
+class ExternalDefault(DefaultDomain):
+    """Deprecated. Please use keystone.auth.external.DefaultDomain instead."""
+
+    @versionutils.deprecated(
+        as_of=versionutils.deprecated.ICEHOUSE,
+        in_favor_of='keystone.auth.external.DefaultDomain',
+        remove_in=+1)
+    def __init__(self):
+        super(ExternalDefault, self).__init__()
+
+
+class ExternalDomain(Domain):
+    """Deprecated. Please use keystone.auth.external.Domain instead."""
+
+    @versionutils.deprecated(
+        as_of=versionutils.deprecated.ICEHOUSE,
+        in_favor_of='keystone.auth.external.Domain',
+        remove_in=+1)
+    def __init__(self):
+        super(ExternalDomain, self).__init__()
+
+
+class LegacyDefaultDomain(Base):
+    """Deprecated. Please use keystone.auth.external.DefaultDomain instead.
+
+    This plugin exists to provide compatibility for the unintended behavior
+    described here: https://bugs.launchpad.net/keystone/+bug/1253484
+
+    """
+
+    @versionutils.deprecated(
+        as_of=versionutils.deprecated.ICEHOUSE,
+        in_favor_of='keystone.auth.external.DefaultDomain',
+        remove_in=+1)
+    def __init__(self):
+        super(LegacyDefaultDomain, self).__init__()
+
+    def _authenticate(self, remote_user, context, auth_info):
+        """Use remote_user to look up the user in the identity backend."""
+        # NOTE(dolph): this unintentionally discards half the REMOTE_USER value
         names = remote_user.split('@')
         username = names.pop(0)
         domain_id = CONF.identity.default_domain_id
@@ -75,8 +145,17 @@ class Default(Base):
         return user_ref
 
 
-class Domain(Base):
+class LegacyDomain(Base):
-    def _authenticate(self, remote_user, auth_info):
+    """Deprecated. Please use keystone.auth.external.Domain instead."""
+
+    @versionutils.deprecated(
+        as_of=versionutils.deprecated.ICEHOUSE,
+        in_favor_of='keystone.auth.external.Domain',
+        remove_in=+1)
+    def __init__(self):
+        super(LegacyDomain, self).__init__()
+
+    def _authenticate(self, remote_user, context, auth_info):
         """Use remote_user to look up the user in the identity backend.
 
         If remote_user contains an `@` assume that the substring before the
@@ -95,21 +174,3 @@ class Domain(Base):
         user_ref = auth_info.identity_api.get_user_by_name(username,
                                                            domain_id)
         return user_ref
-
-
-# NOTE(aloga): ExternalDefault and External have been renamed to Default and
-# Domain.
-class ExternalDefault(Default):
-    """Deprecated. Please use keystone.auth.external.Default instead."""
-    def __init__(self):
-        msg = _('keystone.auth.external.ExternalDefault is deprecated in'
-                'favor of keystone.auth.external.Default')
-        LOG.warning(msg)
-
-
-class ExternalDomain(Domain):
-    """Deprecated. Please use keystone.auth.external.Domain instead."""
-    def __init__(self):
-        msg = _('keystone.auth.external.ExternalDomain is deprecated in'
-                'favor of keystone.auth.external.Domain')
-        LOG.warning(msg)

keystone/common/config.py

@@ -256,7 +256,7 @@ FILE_OPTIONS = {
                    default='keystone.auth.plugins.token.Token'),
         #deals with REMOTE_USER authentication
         cfg.StrOpt('external',
-                   default='keystone.auth.plugins.external.Default')],
+                   default='keystone.auth.plugins.external.DefaultDomain')],
     'paste_deploy': [
         cfg.StrOpt('config_file', default=None)],
     'memcache': [

keystone/tests/auth_plugin_external_default_legacy.conf

@@ -0,0 +1,3 @@
+[auth]
+methods = external, password, token
+external = keystone.auth.plugins.external.LegacyDefaultDomain

keystone/tests/auth_plugin_external_domain_legacy.conf

@@ -0,0 +1,3 @@
+[auth]
+methods = external, password, token
+external = keystone.auth.plugins.external.LegacyDomain

keystone/tests/test_v3.py

@@ -1043,8 +1043,11 @@ class RestfulTestCase(rest.RestfulTestCase):
             auth_data['scope'] = self.build_auth_scope(**kwargs)
         return {'auth': auth_data}
 
-    def build_external_auth_request(self, remote_user, auth_data=None):
+    def build_external_auth_request(self, remote_user,
+                                    remote_domain=None, auth_data=None):
         context = {'environment': {'REMOTE_USER': remote_user}}
+        if remote_domain:
+            context['environment']['REMOTE_DOMAIN'] = remote_domain
         if not auth_data:
             auth_data = self.build_authentication_request()['auth']
         no_context = None

keystone/tests/test_v3_auth.py

@@ -1094,12 +1094,42 @@ class TestAuthExternalDisabled(test_v3.RestfulTestCase):
                           auth_context)
 
 
-class TestAuthExternalDomain(test_v3.RestfulTestCase):
+class TestAuthExternalLegacyDefaultDomain(test_v3.RestfulTestCase):
     content_type = 'json'
 
     def config_files(self):
         cfg_list = self._config_file_list[:]
-        cfg_list.append(tests.dirs.tests('auth_plugin_external_domain.conf'))
+        cfg_list.append(
+            tests.dirs.tests('auth_plugin_external_default_legacy.conf'))
+        return cfg_list
+
+    def test_remote_user_no_realm(self):
+        CONF.auth.methods = 'external'
+        api = auth.controllers.Auth()
+        context, auth_info, auth_context = self.build_external_auth_request(
+            self.default_domain_user['name'])
+        api.authenticate(context, auth_info, auth_context)
+        self.assertEqual(auth_context['user_id'],
+                         self.default_domain_user['id'])
+
+    def test_remote_user_no_domain(self):
+        api = auth.controllers.Auth()
+        context, auth_info, auth_context = self.build_external_auth_request(
+            self.user['name'])
+        self.assertRaises(exception.Unauthorized,
+                          api.authenticate,
+                          context,
+                          auth_info,
+                          auth_context)
+
+
+class TestAuthExternalLegacyDomain(test_v3.RestfulTestCase):
+    content_type = 'json'
+
+    def config_files(self):
+        cfg_list = self._config_file_list[:]
+        cfg_list.append(
+            tests.dirs.tests('auth_plugin_external_domain_legacy.conf'))
         return cfg_list
 
     def test_remote_user_with_realm(self):
@@ -1144,6 +1174,61 @@ class TestAuthExternalDomain(test_v3.RestfulTestCase):
         self.assertEqual(token['bind']['kerberos'], self.user['name'])
 
 
+class TestAuthExternalDomain(test_v3.RestfulTestCase):
+    content_type = 'json'
+
+    def config_files(self):
+        cfg_list = self._config_file_list[:]
+        cfg_list.append(tests.dirs.tests('auth_plugin_external_domain.conf'))
+        return cfg_list
+
+    def test_remote_user_with_realm(self):
+        api = auth.controllers.Auth()
+        remote_user = self.user['name']
+        remote_domain = self.domain['name']
+        context, auth_info, auth_context = self.build_external_auth_request(
+            remote_user, remote_domain=remote_domain)
+
+        api.authenticate(context, auth_info, auth_context)
+        self.assertEqual(auth_context['user_id'], self.user['id'])
+
+        # Now test to make sure the user name can, itself, contain the
+        # '@' character.
+        user = {'name': 'myname@mydivision'}
+        self.identity_api.update_user(self.user['id'], user)
+        remote_user = user["name"]
+        context, auth_info, auth_context = self.build_external_auth_request(
+            remote_user, remote_domain=remote_domain)
+
+        api.authenticate(context, auth_info, auth_context)
+        self.assertEqual(auth_context['user_id'], self.user['id'])
+
+    def test_project_id_scoped_with_remote_user(self):
+        CONF.token.bind = ['kerberos']
+        auth_data = self.build_authentication_request(
+            project_id=self.project['id'])
+        remote_user = self.user['name']
+        remote_domain = self.domain['name']
+        self.admin_app.extra_environ.update({'REMOTE_USER': remote_user,
+                                             'REMOTE_DOMAIN': remote_domain,
+                                             'AUTH_TYPE': 'Negotiate'})
+        r = self.post('/auth/tokens', body=auth_data)
+        token = self.assertValidProjectScopedTokenResponse(r)
+        self.assertEqual(token['bind']['kerberos'], self.user['name'])
+
+    def test_unscoped_bind_with_remote_user(self):
+        CONF.token.bind = ['kerberos']
+        auth_data = self.build_authentication_request()
+        remote_user = self.user['name']
+        remote_domain = self.domain['name']
+        self.admin_app.extra_environ.update({'REMOTE_USER': remote_user,
+                                             'REMOTE_DOMAIN': remote_domain,
+                                             'AUTH_TYPE': 'Negotiate'})
+        r = self.post('/auth/tokens', body=auth_data)
+        token = self.assertValidUnscopedTokenResponse(r)
+        self.assertEqual(token['bind']['kerberos'], self.user['name'])
+
+
 class TestAuthJSON(test_v3.RestfulTestCase):
     content_type = 'json'
 
@@ -1608,6 +1693,15 @@ class TestAuthJSON(test_v3.RestfulTestCase):
         api.authenticate(context, auth_info, auth_context)
         self.assertEqual(auth_context['user_id'],
                          self.default_domain_user['id'])
+        # Now test to make sure the user name can, itself, contain the
+        # '@' character.
+        user = {'name': 'myname@mydivision'}
+        self.identity_api.update_user(self.default_domain_user['id'], user)
+        context, auth_info, auth_context = self.build_external_auth_request(
+            user["name"])
+        api.authenticate(context, auth_info, auth_context)
+        self.assertEqual(auth_context['user_id'],
+                         self.default_domain_user['id'])
 
     def test_remote_user_no_domain(self):
         api = auth.controllers.Auth()