Améliore code et tests gestion User

app/api/users.py

@@ -138,7 +138,7 @@ def user_create():
     ok, msg = _is_allowed_user_edit(args)
     if not ok:
         return json_error(403, f"user_create: {msg}")
-    user = User()
+    user = User(user_name=user_name)
     user.from_dict(args, new_user=True)
     db.session.add(user)
     db.session.commit()

app/auth/models.py

@@ -12,7 +12,6 @@ from typing import Optional
 
 import cracklib  # pylint: disable=import-error
 
-import flask
 from flask import current_app, g
 from flask_login import UserMixin, AnonymousUserMixin
 
@@ -53,7 +52,8 @@ def is_valid_password(cleartxt) -> bool:
 def invalid_user_name(user_name: str) -> bool:
     "Check that user_name (aka login) is invalid"
     return (
-        (len(user_name) < 2)
+        not user_name
+        or (len(user_name) < 2)
         or (len(user_name) >= USERNAME_STR_LEN)
         or not VALID_LOGIN_EXP.match(user_name)
     )
@@ -116,11 +116,16 @@ class User(UserMixin, db.Model, ScoDocModel):
     )
 
     def __init__(self, **kwargs):
+        "user_name:str is mandatory"
         self.roles = []
         self.user_roles = []
         # check login:
-        if kwargs.get("user_name") and invalid_user_name(kwargs["user_name"]):
+        if not "user_name" in kwargs:
+            raise ValueError("missing user_name argument")
+        if invalid_user_name(kwargs["user_name"]):
             raise ValueError(f"invalid user_name: {kwargs['user_name']}")
+        kwargs["nom"] = kwargs.get("nom", "") or ""
+        kwargs["prenom"] = kwargs.get("prenom", "") or ""
         super().__init__(**kwargs)
         # Ajoute roles:
         if (
@@ -279,6 +284,7 @@ class User(UserMixin, db.Model, ScoDocModel):
         Convert boolean values to bools.
         """
         args_dict = args
+        # Dates
         if "date_expiration" in args:
             date_expiration = args.get("date_expiration")
             if isinstance(date_expiration, str):
@@ -287,27 +293,33 @@ class User(UserMixin, db.Model, ScoDocModel):
                     if date_expiration
                     else None
                 )
-
+        # booléens:
         for field in ("active", "cas_allow_login", "cas_allow_scodoc_login"):
             if field in args:
                 args_dict[field] = scu.to_bool(args.get(field))
+
+        # chaines ne devant pas être NULLs
+        for field in ("nom", "prenom"):
+            if field in args:
+                args[field] = args[field] or ""
+
         return args_dict
 
     def from_dict(self, data: dict, new_user=False):
         """Set users' attributes from given dict values.
-        Roles must be encoded as "roles_string", like "Ens_RT, Secr_CJ"
+        - roles_string : roles, encoded like "Ens_RT, Secr_CJ"
+        - date_expiration is a dateime object.
         Does not check permissions here.
         """
-        super().from_dict(data, excluded=("user_name", "roles_string"))
-
         if new_user:
             if "user_name" in data:
                 # never change name of existing users
+                if invalid_user_name(data["user_name"]):
+                    raise ValueError(f"invalid user_name: {data['user_name']}")
                 self.user_name = data["user_name"]
             if "password" in data:
                 self.set_password(data["password"])
-        if invalid_user_name(self.user_name):
+
-            raise ValueError(f"invalid user_name: {self.user_name}")
         # Roles: roles_string is "Ens_RT, Secr_RT, ..."
         if "roles_string" in data:
             self.user_roles = []
@@ -316,6 +328,8 @@ class User(UserMixin, db.Model, ScoDocModel):
                     role, dept = UserRole.role_dept_from_string(r_d)
                     self.add_role(role, dept)
 
+        super().from_dict(data, excluded={"user_name", "roles_string", "roles"})
+
         # Set cas_id using regexp if configured:
         exp = ScoDocSiteConfig.get("cas_uid_from_mail_regexp")
         if exp and self.email_institutionnel:

app/scodoc/sco_import_users.py

@@ -254,7 +254,7 @@ def import_users(users, force="") -> tuple[bool, list[str], int]:
         if import_ok:
             for u in created.values():
                 # Création de l'utilisateur (via SQLAlchemy)
-                user = User()
+                user = User(user_name=u["user_name"])
                 user.from_dict(u, new_user=True)
                 db.session.add(user)
                 db.session.commit()

app/scodoc/sco_users.py

@@ -432,15 +432,3 @@ def check_modif_user(
         )
     # Roles ?
     return True, ""
-
-
-def user_edit(user_name, vals):
-    """Edit the user specified by user_name
-    (ported from Zope to SQLAlchemy, hence strange !)
-    """
-    u: User = User.query.filter_by(user_name=user_name).first()
-    if not u:
-        raise ScoValueError("Invalid user_name")
-    u.from_dict(vals)
-    db.session.add(u)
-    db.session.commit()

app/templates/auth/user_info_page.j2

@@ -9,10 +9,12 @@
 <div class="user_basics">
     <b>Login :</b> {{user.user_name}}<br>
     <b>CAS id:</b> {{user.cas_id or "(aucun)"}}
+    {% if ScoDocSiteConfig.is_cas_enabled() %}
         (CAS {{'autorisé' if user.cas_allow_login else 'interdit'}} pour cet utilisateur)
         {% if user.cas_allow_scodoc_login %}
             (connexion sans CAS autorisée)
         {% endif %}
+    {% endif %}
     <br>
     <b>Nom :</b> {{user.nom or ""}}<br>
     <b>Prénom :</b> {{user.prenom or ""}}<br>

app/views/users.py

@@ -701,10 +701,12 @@ def create_user_form(user_name=None, edit=0, all_roles=True):
                 log(f"sco_users: editing {user_name} by {current_user.user_name}")
                 log(f"sco_users: previous_values={initvalues}")
                 log(f"sco_users: new_values={vals}")
-                sco_users.user_edit(user_name, vals)
-                flash(f"Utilisateur {user_name} modifié")
             else:
-                sco_users.user_edit(user_name, {"roles_string": vals["roles_string"]})
+                vals = {"roles_string": vals["roles_string"]}
+            the_user.from_dict(vals)
+            db.session.add(the_user)
+            db.session.commit()
+            flash(f"Utilisateur {user_name} modifié")
             return flask.redirect(
                 url_for(
                     "users.user_info_page",
@@ -760,7 +762,7 @@ def create_user_form(user_name=None, edit=0, all_roles=True):
             log(
                 f"""sco_users: new_user {vals["user_name"]} by {current_user.user_name}"""
             )
-            the_user = User()
+            the_user = User(user_name=user_name)
             the_user.from_dict(vals, new_user=True)
             db.session.add(the_user)
             db.session.commit()
@@ -927,11 +929,12 @@ def user_info_page(user_name=None):
 
     return render_template(
         "auth/user_info_page.j2",
-        user=user,
-        title=f"Utilisateur {user.user_name}",
-        Permission=Permission,
         dept=dept,
+        Permission=Permission,
+        ScoDocSiteConfig=ScoDocSiteConfig,
         session_info=session_info,
+        title=f"Utilisateur {user.user_name}",
+        user=user,
     )
 
 

tests/unit/test_users.py

@@ -128,19 +128,18 @@ def test_create_delete(test_client):
 def test_edit(test_client):
     "test edition object utlisateur"
     args = {
-        "user_name": "Tonari",
         "prenom": "No Totoro",
         "edt_id": "totorito",
         "cas_allow_login": 1,  # boolean
         "irrelevant": "..",  # intentionnellement en dehors des attributs
     }
-    u = User()
+    u = User(user_name="Tonari")
     u.from_dict(args)
     db.session.add(u)
     db.session.commit()
     db.session.refresh(u)
     assert u.edt_id == "totorito"
-    assert u.nom is None
+    assert u.nom == ""
     assert u.cas_allow_login is True
     d = u.to_dict()
     assert d["nom"] == ""