[Update] 优化LDAPUtil逻辑 (#2728)

apps/authentication/backends/ldap.py

@@ -7,6 +7,8 @@ from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist
 from django_auth_ldap.backend import _LDAPUser, LDAPBackend
 from django_auth_ldap.config import _LDAPConfig, LDAPSearch, LDAPSearchUnion
 
+from users.utils import construct_user_email
+
 logger = _LDAPConfig.get_logger()
 
 
@@ -95,14 +97,9 @@ class LDAPUser(_LDAPUser):
                 if not hasattr(self._user, field):
                     continue
                 if isinstance(getattr(self._user, field), bool):
-                    value = bool(int(value == 'true'))
+                    value = value.lower() in ['true', '1']
                 setattr(self._user, field, value)
 
-        if not hasattr(self._user, 'email') or '@' not in self._user.email:
-            if '@' not in self._user.username:
-                email = '{}@{}'.format(self._user.username, settings.EMAIL_SUFFIX)
-            else:
-                email = self._user.username
-            setattr(self._user, 'email', email)
-
-
+        email = getattr(self._user, 'email', '')
+        email = construct_user_email(email, self._user.username)
+        setattr(self._user, 'email', email)

apps/settings/api.py

@@ -79,7 +79,7 @@ class LDAPTestingAPI(APIView):
         util = self.get_ldap_util(serializer)
 
         try:
-            users = util.get_search_user_items()
+            users = util.search_user_items()
         except Exception as e:
             return Response({"error": str(e)}, status=401)
 
@@ -95,7 +95,7 @@ class LDAPUserListApi(APIView):
     def get(self, request):
         util = LDAPUtil()
         try:
-            users = util.get_search_user_items()
+            users = util.search_user_items()
         except Exception as e:
             users = []
             logger.error(e, exc_info=True)
@@ -108,11 +108,11 @@ class LDAPUserSyncAPI(APIView):
     permission_classes = (IsOrgAdmin,)
 
     def post(self, request):
-        user_names = request.data.get('user_names', '')
+        username_list = request.data.get('username_list', [])
 
         util = LDAPUtil()
         try:
-            result = util.sync_users(username_set=user_names)
+            result = util.sync_users(username_list)
         except Exception as e:
             logger.error(e, exc_info=True)
             return Response({'error': str(e)}, status=401)

apps/settings/templates/settings/ldap_setting.html

@@ -107,13 +107,13 @@ $(document).ready(function () {
     });
 })
 .on("click","#btn_ldap_modal_confirm",function () {
-    var user_names=[];
+    var username_list=[];
     $("tbody input[type='checkbox']:checked").each(function () {
-        user_names.push($(this).attr('id'));
+        username_list.push($(this).attr('id'));
     });
 
-    if (user_names.length === 0){
-        var msg = "{% trans 'User is not currently selected, please check the user you want to import'%}"
+    if (username_list.length === 0){
+        var msg = "{% trans 'User is not currently selected, please check the user you want to import'%}";
         toastr.error(msg);
         return
      }
@@ -129,7 +129,7 @@ $(document).ready(function () {
     }
     APIUpdateAttr({
         url: the_url,
-        body: JSON.stringify({'user_names':user_names}),
+        body: JSON.stringify({'username_list':username_list}),
         method: "POST",
         flash_message: false,
         success: success,

apps/settings/utils.py

@@ -5,7 +5,9 @@ from ldap3 import Server, Connection
 from django.utils.translation import ugettext_lazy as _
 
 from users.models import User
+from users.utils import construct_user_email
 from common.utils import get_logger
+
 from .models import settings
 
 
@@ -17,11 +19,11 @@ class LDAPOUGroupException(Exception):
 
 
 class LDAPUtil:
+    _conn = None
 
     def __init__(self, use_settings_config=True, server_uri=None, bind_dn=None,
                  password=None, use_ssl=None, search_ougroup=None,
                  search_filter=None, attr_map=None, auth_ldap=None):
-
         # config
         if use_settings_config:
             self._load_config_from_settings()
@@ -45,6 +47,15 @@ class LDAPUtil:
         self.attr_map = settings.AUTH_LDAP_USER_ATTR_MAP
         self.auth_ldap = settings.AUTH_LDAP
 
+    @property
+    def connection(self):
+        if self._conn is None:
+            server = Server(self.server_uri, use_ssl=self.use_ssl)
+            conn = Connection(server, self.bind_dn, self.password)
+            conn.bind()
+            self._conn = conn
+        return self._conn
+
     @staticmethod
     def get_user_by_username(username):
         try:
@@ -55,45 +66,64 @@ class LDAPUtil:
         else:
             return user
 
+    def _ldap_entry_to_user_item(self, entry):
+        user_item = {}
+        for attr, mapping in self.attr_map.items():
+            if not hasattr(entry, mapping):
+                continue
+            user_item[attr] = getattr(entry, mapping).value or ''
+        return user_item
+
+    def search_user_items(self):
+        user_items = []
+        for search_ou in str(self.search_ougroup).split("|"):
+            ok = self.connection.search(
+                search_ou, self.search_filter % ({"user": "*"}),
+                attributes=list(self.attr_map.values())
+            )
+            if not ok:
+                error = _("Search no entry matched in ou {}".format(search_ou))
+                raise LDAPOUGroupException(error)
+            for entry in self.connection.entries:
+                user_item = self._ldap_entry_to_user_item(entry)
+                user = self.get_user_by_username(user_item['username'])
+                user_item['existing'] = bool(user)
+                user_items.append(user_item)
+        return user_items
+
+    def search_filter_user_items(self, username_list):
+        user_items = self.search_user_items()
+        if username_list:
+            user_items = [u for u in user_items if u['username'] in username_list]
+        return user_items
+
     @staticmethod
-    def _update_user(user, user_item):
+    def save_user(user, user_item):
         for field, value in user_item.items():
             if not hasattr(user, field):
                 continue
             if isinstance(getattr(user, field), bool):
-                value = bool(int(value == 'true'))
+                value = value.lower() in ['true', 1]
             setattr(user, field, value)
         user.save()
 
     def update_user(self, user_item):
         user = self.get_user_by_username(user_item['username'])
-        if not user:
-            msg = _('User does not exist')
-            return False, msg
         if user.source != User.SOURCE_LDAP:
             msg = _('The user source is not LDAP')
             return False, msg
-
         try:
-            self._update_user(user, user_item)
+            self.save_user(user, user_item)
         except Exception as e:
             logger.error(e, exc_info=True)
             return False, str(e)
         else:
             return True, None
 
-    @staticmethod
-    def create_user(user_item):
-        user = User()
-        for field, value in user_item.items():
-            if not hasattr(user, field):
-                continue
-            if isinstance(getattr(user, field), bool):
-                value = bool(int(value == 'true'))
-                user_item[field] = value
-        user_item['source'] = User.SOURCE_LDAP
+    def create_user(self, user_item):
+        user = User(source=User.SOURCE_LDAP)
         try:
-            User.objects.create(**user_item)
+            self.save_user(user, user_item)
         except Exception as e:
             logger.error(e, exc_info=True)
             return False, str(e)
@@ -101,26 +131,21 @@ class LDAPUtil:
             return True, None
 
     @staticmethod
-    def get_or_construct_email(user_item):
-        if not user_item.get('email', None):
-            if '@' in user_item['username']:
-                email = user_item['username']
-            else:
-                email = '{}@{}'.format(
-                    user_item['username'], settings.EMAIL_SUFFIX)
-        else:
-            email = user_item['email']
+    def construct_user_email(user_item):
+        username = user_item['username']
+        email = user_item.get('email', '')
+        email = construct_user_email(username, email)
         return email
 
     def create_or_update_users(self, user_items, force_update=True):
         succeed = failed = 0
         for user_item in user_items:
-            user_item['email'] = self.get_or_construct_email(user_item)
-            exist = user_item.pop('existing', None)
-            if exist:
-                ok, error = self.update_user(user_item)
-            else:
+            exist = user_item.pop('existing', False)
+            user_item['email'] = self.construct_user_email(user_item)
+            if not exist:
                 ok, error = self.create_user(user_item)
+            else:
+                ok, error = self.update_user(user_item)
             if not ok:
                 failed += 1
             else:
@@ -128,44 +153,7 @@ class LDAPUtil:
         result = {'total': len(user_items), 'succeed': succeed, 'failed': failed}
         return result
 
-    def _ldap_entry_to_user_item(self, entry):
-        user_item = {}
-        for attr, mapping in self.attr_map.items():
-            if not hasattr(entry, mapping):
-                continue
-            user_item[attr] = getattr(entry, mapping).value or ''
-        return user_item
-
-    def get_connection(self):
-        server = Server(self.server_uri, use_ssl=self.use_ssl)
-        conn = Connection(server, self.bind_dn, self.password)
-        conn.bind()
-        return conn
-
-    def get_search_user_items(self):
-        conn = self.get_connection()
-        user_items = []
-        search_ougroup = str(self.search_ougroup).split("|")
-        for search_ou in search_ougroup:
-            ok = conn.search(
-                search_ou, self.search_filter % ({"user": "*"}),
-                attributes=list(self.attr_map.values())
-            )
-            if not ok:
-                error = _("Search no entry matched in ou {}".format(search_ou))
-                raise LDAPOUGroupException(error)
-
-            for entry in conn.entries:
-                user_item = self._ldap_entry_to_user_item(entry)
-                user = self.get_user_by_username(user_item['username'])
-                user_item['existing'] = bool(user)
-                user_items.append(user_item)
-
-        return user_items
-
-    def sync_users(self, username_set):
-        user_items = self.get_search_user_items()
-        if username_set:
-            user_items = [u for u in user_items if u['username'] in username_set]
+    def sync_users(self, username_list):
+        user_items = self.search_filter_user_items(username_list)
         result = self.create_or_update_users(user_items)
         return result

apps/users/utils.py

@@ -313,3 +313,13 @@ def is_need_unblock(key_block):
     if not cache.get(key_block):
         return False
     return True
+
+
+def construct_user_email(username, email):
+    if '@' not in email:
+        if '@' in username:
+            email = username
+        else:
+            email = '{}@{}'.format(username, settings.EMAIL_SUFFIX)
+    return email
+