feat: authorize the use of dot in a username (#6177)

PR #6177

---------

Co-authored-by: Skylsmoi <come.huguies@algoo.fr>

.pre-commit-config.yaml

@@ -28,7 +28,7 @@ repos:
       hooks:
           - id: isort
             files: 'backend'
-    - repo: https://gitlab.com/pycqa/flake8
+    - repo: https://github.com/pycqa/flake8
       rev: 3.7.9
       hooks:
           - id: flake8

backend/development.ini.sample

@@ -367,10 +367,12 @@ api.key = %(basic_setup.api_key)s
 ; ldap_user_base_dn =
 # example:
 # ldap_user_base_dn = ou=Users,dc=directory,dc=fsf,dc=org
-## Attribute name of user record who contain user login (email)
-; ldap_login_attribute = mail
-## Attribute for default name of new user from ldap
-; ldap_name_attribute = givenName
+## Attribute name of user record who contain username (login)
+; ldap_username_attribute = givenName
+## Attribute name of user record who contain user mail (email)
+; ldap_mail_attribute = mail
+## Attribute for default Public Name of new user from ldap
+; ldap_name_attributes = displayName
 ## TLS usage to communicate with your LDAP server
 ; ldap_tls = False
 

backend/tests_configs.ini

@@ -64,8 +64,9 @@ ldap_url = ldap://localhost:3890
 ldap_bind_dn = cn=admin,dc=planetexpress,dc=com
 ldap_bind_pass = GoodNewsEveryone
 ldap_user_base_dn = ou=people,dc=planetexpress,dc=com
-ldap_login_attribute = mail
-ldap_name_attribute = givenName
+ldap_mail_attribute = mail
+ldap_username_attribute = givenName
+ldap_name_attribute = displayName
 ldap_profile_attribute = profile
 ldap_tls = False
 
@@ -92,8 +93,9 @@ ldap_url = ldap://localhost:3890
 ldap_bind_dn = cn=admin,dc=planetexpress,dc=com
 ldap_bind_pass = GoodNewsEveryone
 ldap_user_base_dn = ou=people,dc=planetexpress,dc=com
-ldap_login_attribute = mail
-ldap_name_attribute = givenName
+ldap_mail_attribute = mail
+ldap_username_attribute = givenName
+ldap_name_attribute = displayName
 ldap_tls = False
 
 [functional_ldap_anonymous_test]
@@ -105,8 +107,9 @@ ldap_bind_anonymous = True
 ldap_bind_dn =
 ldap_bind_pass =
 ldap_user_base_dn = ou=people,dc=planetexpress,dc=com
-ldap_login_attribute = mail
-ldap_name_attribute = givenName
+ldap_mail_attribute = mail
+ldap_username_attribute = givenName
+ldap_name_attribute = displayName
 ldap_tls = False
 
 [functional_ldap_test_username]
@@ -117,8 +120,9 @@ ldap_url = ldap://localhost:3890
 ldap_bind_dn = cn=admin,dc=planetexpress,dc=com
 ldap_bind_pass = GoodNewsEveryone
 ldap_user_base_dn = ou=people,dc=planetexpress,dc=com
-ldap_login_attribute = givenName
-ldap_name_attribute = givenName
+ldap_mail_attribute = mail
+ldap_username_attribute = givenName
+ldap_name_attribute = displayName
 ldap_tls = False
 email.required = False
 
@@ -131,8 +135,9 @@ ldap_url = ldap://localhost:3890
 ldap_bind_dn = cn=admin,dc=planetexpress,dc=com
 ldap_bind_pass = GoodNewsEveryone
 ldap_user_base_dn = ou=people,dc=planetexpress,dc=com
-ldap_login_attribute = mail
-ldap_name_attribute = givenName
+ldap_mail_attribute = mail
+ldap_username_attribute = givenName
+ldap_name_attribute = displayName
 ldap_tls = False
 email.notification.activated = true
 email.notification.from.email = test_user_from+{user_id}@localhost
@@ -172,8 +177,9 @@ ldap_url = ldap://localhost:3890
 ldap_bind_dn = cn=admin,dc=planetexpress,dc=com
 ldap_bind_pass = GoodNewsEveryone
 ldap_user_base_dn = ou=people,dc=planetexpress,dc=com
-ldap_login_attribute = mail
-ldap_name_attribute = givenName
+ldap_mail_attribute = mail
+ldap_username_attribute = givenName
+ldap_name_attribute = displayName
 ldap_tls = False
 
 
@@ -801,8 +807,9 @@ ldap_url = ldap://localhost:3890
 ldap_bind_dn = cn=admin,dc=planetexpress,dc=com
 ldap_bind_pass = GoodNewsEveryone
 ldap_user_base_dn = ou=people,dc=planetexpress,dc=com
-ldap_login_attribute = mail
-ldap_name_attribute = givenName
+ldap_mail_attribute = mail
+ldap_username_attribute = givenName
+ldap_name_attribute = displayName
 ldap_tls = False
 
 [functional_test_with_no_email_notif_but_invitation_email_notif]

backend/tracim_backend/config.py

@@ -911,9 +911,10 @@ def _load_ldap_config(self) -> None:
         self.LDAP_BIND_ANONYMOUS = asbool(self.get_raw_config("ldap_bind_anonymous", "False"))
         self.LDAP_TLS = asbool(self.get_raw_config("ldap_tls", "False"))
         self.LDAP_USER_BASE_DN = self.get_raw_config("ldap_user_base_dn")
-        self.LDAP_LOGIN_ATTRIBUTE = self.get_raw_config("ldap_login_attribute", "mail")
+        self.LDAP_MAIL_ATTRIBUTE = self.get_raw_config("ldap_mail_attribute", "mail")
+        self.LDAP_USERNAME_ATTRIBUTE = self.get_raw_config("ldap_username_attribute", "givenName")
         # TODO - G.M - 16-11-2018 - Those prams are only use at account creation
-        self.LDAP_NAME_ATTRIBUTE = self.get_raw_config("ldap_name_attribute", "givenName")
+        self.LDAP_NAME_ATTRIBUTE = self.get_raw_config("ldap_name_attribute", "displayName")
         # TODO - G.M - 2018-12-05 - [ldap_profile]
         # support for profile attribute disabled
         # Should be reenabled later probably with a better code
@@ -922,7 +923,9 @@ def _load_ldap_config(self) -> None:
         # TODO - G.M - 2019-04-05 - keep as parameters
         # or set it as constant,
         # see https://github.com/tracim/tracim/issues/1569
-        self.LDAP_USER_FILTER = "({}=%(login)s)".format(self.LDAP_LOGIN_ATTRIBUTE)
+        self.LDAP_USER_FILTER = "({}=%(login)s)".format(self.LDAP_USERNAME_ATTRIBUTE)
+        if self.EMAIL__REQUIRED:
+            self.LDAP_USER_FILTER = "({}=%(login)s)".format(self.LDAP_MAIL_ATTRIBUTE)
         self.LDAP_USE_POOL = True
         self.LDAP_POOL_SIZE = 10 if self.LDAP_USE_POOL else None
         self.LDAP_POOL_LIFETIME = 3600 if self.LDAP_USE_POOL else None
@@ -1434,8 +1437,13 @@ def _check_ldap_config_validity(self):
                 when_str="when ldap is in available auth method",
             )
             self.check_mandatory_param(
-                "LDAP_LOGIN_ATTRIBUTE",
-                self.LDAP_LOGIN_ATTRIBUTE,
+                "LDAP_USERNAME_ATTRIBUTE",
+                self.LDAP_USERNAME_ATTRIBUTE,
+                when_str="when ldap is in available auth method",
+            )
+            self.check_mandatory_param(
+                "LDAP_MAIL_ATTRIBUTE",
+                self.LDAP_MAIL_ATTRIBUTE,
                 when_str="when ldap is in available auth method",
             )
             self.check_mandatory_param(

backend/tracim_backend/lib/core/user.py

@@ -477,15 +477,18 @@ def _ldap_authenticate(
             #             )
             #         )
             name = None
-            if self._config.LDAP_NAME_ATTRIBUTE:
+            mail = None
+            username = None
+            if self._config.LDAP_NAME_ATTRIBUTE and self._config.LDAP_NAME_ATTRIBUTE in ldap_data:
                 name = ldap_data[self._config.LDAP_NAME_ATTRIBUTE][0]
+            if self._config.LDAP_MAIL_ATTRIBUTE:
+                mail = ldap_data[self._config.LDAP_MAIL_ATTRIBUTE][0]
+            if self._config.LDAP_USERNAME_ATTRIBUTE:
+                username = ldap_data[self._config.LDAP_USERNAME_ATTRIBUTE][0]
             # INFO - G.M - 2018-11-08 - Create new user from ldap credentials
-            use_email = False
-            if "@" in login:
-                use_email = True
             user = self.create_user(
-                email=login if use_email else None,
-                username=login if not use_email else None,
+                email=mail,
+                username=username,
                 name=name,
                 profile=profile,
                 auth_type=AuthType.LDAP,
@@ -861,7 +864,7 @@ def _check_username_correctness(self, username: str) -> bool:
         if len(username) < User.MIN_USERNAME_LENGTH or len(username) > User.MAX_USERNAME_LENGTH:
             return False
         for char in username:
-            if char not in "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_":
+            if char not in "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_.":
                 return False
         return True
 

backend/tracim_backend/tests/functional/test_reset_password.py

@@ -376,10 +376,10 @@ class TestResetPasswordExternalAuthUser(object):
     def test_api__reset_password_request__err__external_auth_ldap_cant_change_password(
         self, web_testapp
     ):
-        web_testapp.authorization = ("Basic", ("hubert@planetexpress.com", "professor"))
+        web_testapp.authorization = ("Basic", ("professor@planetexpress.com", "professor"))
         web_testapp.get("/api/auth/whoami", status=200)
 
-        params = {"email": "hubert@planetexpress.com"}
+        params = {"email": "professor@planetexpress.com"}
         web_testapp.post_json("/api/auth/password/reset/request", status=204, params=params)
 
     @pytest.mark.email_notification
@@ -388,10 +388,10 @@ def test_api__reset_password_check_token__err__external_auth_ldap_cant_change_pa
         self, web_testapp
     ):
         # precreate user
-        web_testapp.authorization = ("Basic", ("hubert@planetexpress.com", "professor"))
+        web_testapp.authorization = ("Basic", ("professor@planetexpress.com", "professor"))
         web_testapp.get("/api/auth/whoami", status=200)
 
-        params = {"email": "hubert@planetexpress.com", "reset_password_token": "unknown"}
+        params = {"email": "professor@planetexpress.com", "reset_password_token": "unknown"}
         res = web_testapp.post_json(
             "/api/auth/password/reset/token/check", status=400, params=params
         )
@@ -405,11 +405,11 @@ def test_api__reset_password_modify__err__external_auth_ldap_cant_change_passwor
         self, web_testapp
     ):
         # precreate user
-        web_testapp.authorization = ("Basic", ("hubert@planetexpress.com", "professor"))
+        web_testapp.authorization = ("Basic", ("professor@planetexpress.com", "professor"))
         web_testapp.get("/api/auth/whoami", status=200)
 
         params = {
-            "email": "hubert@planetexpress.com",
+            "email": "professor@planetexpress.com",
             "reset_password_token": "unknown",
             "new_password": "mynewpassword",
             "new_password2": "mynewpassword",