From 95160d1812104d90ff096cb2d32390b065bfeaae Mon Sep 17 00:00:00 2001
From: Matthew Edmonds <edmondsw@us.ibm.com>
Date: Fri, 24 Feb 2017 00:41:11 -0500
Subject: [PATCH] Fix MFA rule checks for LDAP auth

LDAP authentication was broken by the addition of MFA rule checking.
This patch fixes that.

Change-Id: I4efe4b1b90c93110509cd599f9dd047c313dade3
Closes-Bug: #1662762
(cherry picked from commit 4e0029455ab45e3b9a15fe9fc151c14c502b7bdd)
---
 keystone/identity/backends/ldap/core.py       | 27 +++++++++++++++++++
 keystone/tests/unit/default_fixtures.py       |  7 ++++-
 keystone/tests/unit/identity/test_backends.py |  4 +++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/keystone/identity/backends/ldap/core.py b/keystone/identity/backends/ldap/core.py
index 0e17fb866c..d029ddcd5e 100644
--- a/keystone/identity/backends/ldap/core.py
+++ b/keystone/identity/backends/ldap/core.py
@@ -313,6 +313,8 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap):
         del values['enabled_nomask']
 
     def create(self, values):
+        if 'options' in values:
+            values.pop('options')  # can't specify options
         if self.enabled_mask:
             orig_enabled = values['enabled']
             self.mask_enabled_attribute(values)
@@ -326,12 +328,25 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap):
         if self.enabled_mask or (self.enabled_invert and
                                  not self.enabled_emulation):
             values['enabled'] = orig_enabled
+        values['options'] = {}  # options always empty
         return values
 
+    def get(self, user_id, ldap_filter=None):
+        obj = super(UserApi, self).get(user_id, ldap_filter=ldap_filter)
+        obj['options'] = {}  # options always empty
+        return obj
+
     def get_filtered(self, user_id):
         user = self.get(user_id)
         return self.filter_attributes(user)
 
+    def get_all(self, ldap_filter=None, hints=None):
+        objs = super(UserApi, self).get_all(ldap_filter=ldap_filter,
+                                            hints=hints)
+        for obj in objs:
+            obj['options'] = {}  # options always empty
+        return objs
+
     def get_all_filtered(self, hints):
         query = self.filter_query(hints, self.ldap_filter)
         return [self.filter_attributes(user)
@@ -349,6 +364,18 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap):
 
         return common_ldap.dn_startswith(dn, self.tree_dn)
 
+    def update(self, user_id, values, old_obj=None):
+        if old_obj is None:
+            old_obj = self.get(user_id)
+        # don't support updating options
+        if 'options' in old_obj:
+            old_obj.pop('options')
+        if 'options' in values:
+            values.pop('options')
+        values = super(UserApi, self).update(user_id, values, old_obj)
+        values['options'] = {}  # options always empty
+        return values
+
 
 class GroupApi(common_ldap.BaseLdap):
     DEFAULT_OU = 'ou=UserGroups'
diff --git a/keystone/tests/unit/default_fixtures.py b/keystone/tests/unit/default_fixtures.py
index 7f6619867d..1fd48f028e 100644
--- a/keystone/tests/unit/default_fixtures.py
+++ b/keystone/tests/unit/default_fixtures.py
@@ -67,7 +67,8 @@ USERS = [
         'domain_id': DEFAULT_DOMAIN_ID,
         'password': 'password',
         'tenants': [],
-        'enabled': True
+        'enabled': True,
+        'options': {},
     },
     {
         'id': 'foo',
@@ -77,6 +78,7 @@ USERS = [
         'tenants': [BAR_TENANT_ID],
         'enabled': True,
         'email': 'foo@bar.com',
+        'options': {},
     }, {
         'id': 'two',
         'name': 'TWO',
@@ -86,6 +88,7 @@ USERS = [
         'default_project_id': BAZ_TENANT_ID,
         'tenants': [BAZ_TENANT_ID],
         'email': 'two@three.com',
+        'options': {},
     }, {
         'id': 'badguy',
         'name': 'BadGuy',
@@ -95,6 +98,7 @@ USERS = [
         'default_project_id': BAZ_TENANT_ID,
         'tenants': [BAZ_TENANT_ID],
         'email': 'bad@guy.com',
+        'options': {},
     }, {
         'id': 'sna',
         'name': 'SNA',
@@ -103,6 +107,7 @@ USERS = [
         'enabled': True,
         'tenants': [BAR_TENANT_ID],
         'email': 'sna@snl.coom',
+        'options': {},
     }
 ]
 
diff --git a/keystone/tests/unit/identity/test_backends.py b/keystone/tests/unit/identity/test_backends.py
index 9bfd0c8609..f074906ebd 100644
--- a/keystone/tests/unit/identity/test_backends.py
+++ b/keystone/tests/unit/identity/test_backends.py
@@ -115,6 +115,10 @@ class IdentityTests(object):
         #               it easier to authenticate in tests, but should
         #               not be returned by the api
         self.user_foo.pop('password')
+        # NOTE(edmondsw): check that options is set, even if it's just an
+        # empty dict, because otherwise auth will blow up for whatever
+        # case misses this.
+        self.assertIn('options', user_ref)
         self.assertDictEqual(self.user_foo, user_ref)
 
     def test_get_user_returns_required_attributes(self):