From ef80471a39318b740c1bd7e9079bfa08924f9277 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <josephschorr@users.noreply.github.com>
Date: Wed, 21 Dec 2016 15:00:55 -0500
Subject: [PATCH] fix(136521333): Handle None email_or_id in avatar code

Fixes https://www.pivotaltracker.com/story/show/136521333
---
 avatars/avatars.py         |  6 +++++-
 test/test_endtoend_auth.py | 14 +++++++++++++-
 test/test_ldap.py          | 31 +++++++++++++++++++++----------
 3 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/avatars/avatars.py b/avatars/avatars.py
index 51f6605ac..737b51191 100644
--- a/avatars/avatars.py
+++ b/avatars/avatars.py
@@ -77,7 +77,11 @@ class BaseAvatar(object):
         }
     """
     colors = self.colors
-    hash_value = hashlib.md5(email_or_id.strip().lower()).hexdigest()
+
+    # Note: email_or_id may be None if gotten from external auth when email is disabled,
+    # so use the username in that case.
+    username_email_or_id = email_or_id or name
+    hash_value = hashlib.md5(username_email_or_id.strip().lower()).hexdigest()
 
     byte_count = int(math.ceil(math.log(len(colors), 16)))
     byte_data = hash_value[0:byte_count]
diff --git a/test/test_endtoend_auth.py b/test/test_endtoend_auth.py
index 33df27bda..a66339da7 100644
--- a/test/test_endtoend_auth.py
+++ b/test/test_endtoend_auth.py
@@ -48,10 +48,22 @@ class TestJWTEndToEnd(ApiTestCase, EndToEndAuthMixin):
   def get_authentication(self):
     return fake_jwt()
 
-class TestKeystoneEndToEnd(ApiTestCase, EndToEndAuthMixin):
+class TestKeystone3EndToEnd(ApiTestCase, EndToEndAuthMixin):
   def get_authentication(self):
     return fake_keystone(3)
 
+class TestLDAPNoEmailEndToEnd(ApiTestCase, EndToEndAuthMixin):
+  def get_authentication(self):
+    return mock_ldap(requires_email=False)
+
+class TestJWTNoEmailEndToEnd(ApiTestCase, EndToEndAuthMixin):
+  def get_authentication(self):
+    return fake_jwt(requires_email=False)
+
+class TestKeystone3NoEmailEndToEnd(ApiTestCase, EndToEndAuthMixin):
+  def get_authentication(self):
+    return fake_keystone(3, requires_email=False)
+
 
 if __name__ == '__main__':
   unittest.main()
\ No newline at end of file
diff --git a/test/test_ldap.py b/test/test_ldap.py
index d04e1b97f..6e2a5d914 100644
--- a/test/test_ldap.py
+++ b/test/test_ldap.py
@@ -24,7 +24,7 @@ def _create_ldap(requires_email=True):
 
 @contextmanager
 def mock_ldap(requires_email=True):
-  mockldap = MockLdap({
+  mock_data = {
     'dc=quay,dc=io': {'dc': ['quay', 'io']},
     'ou=employees,dc=quay,dc=io': {
       'dc': ['quay', 'io'],
@@ -77,27 +77,38 @@ def mock_ldap(requires_email=True):
       'uid': ['multientry'],
       'another': ['key']
     },
-     'uid=secondaryuser,ou=otheremployees,dc=quay,dc=io': {
+    'uid=secondaryuser,ou=otheremployees,dc=quay,dc=io': {
       'dc': ['quay', 'io'],
       'ou': 'otheremployees',
       'uid': ['secondaryuser'],
       'userPassword': ['somepass'],
       'mail': ['foosecondary@bar.com']
     },
-  })
+  }
+
+  if not requires_email:
+    for path in mock_data:
+      mock_data[path].pop('mail', None)
+
+  mockldap = MockLdap(mock_data)
 
   def initializer(uri, trace_level=0):
     obj = mockldap[uri]
 
     # Seed to "support" wildcard queries, which MockLDAP does not support natively.
+    cool_block = {
+      'dc': ['quay', 'io'],
+      'ou': 'employees',
+      'uid': ['cool.user', 'referred'],
+      'userPassword': ['somepass'],
+      'mail': ['foo@bar.com']
+    }
+
+    if not requires_email:
+      cool_block.pop('mail', None)
+
     obj.search_s.seed('ou=employees,dc=quay,dc=io', 2, '(|(uid=cool*)(mail=cool*))')([
-      ('uid=cool.user,ou=employees,dc=quay,dc=io', {
-        'dc': ['quay', 'io'],
-        'ou': 'employees',
-        'uid': ['cool.user', 'referred'],
-        'userPassword': ['somepass'],
-        'mail': ['foo@bar.com']
-      })
+      ('uid=cool.user,ou=employees,dc=quay,dc=io', cool_block)
     ])
 
     obj.search_s.seed('ou=otheremployees,dc=quay,dc=io', 2, '(|(uid=cool*)(mail=cool*))')([])