From 457a9ddf515c09e8c8a2adab949450b678e026d0 Mon Sep 17 00:00:00 2001 From: Emmanuel Viennet Date: Wed, 22 Nov 2023 17:55:15 +0100 Subject: [PATCH] =?UTF-8?q?Am=C3=A9liore=20code=20et=20tests=20gestion=20U?= =?UTF-8?q?ser?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/api/users.py | 2 +- app/auth/models.py | 32 ++++++++++++++++++++-------- app/scodoc/sco_import_users.py | 2 +- app/scodoc/sco_users.py | 12 ----------- app/templates/auth/user_info_page.j2 | 8 ++++--- app/views/users.py | 17 +++++++++------ tests/unit/test_users.py | 5 ++--- 7 files changed, 42 insertions(+), 36 deletions(-) diff --git a/app/api/users.py b/app/api/users.py index 3bbc96d3..3dffd277 100644 --- a/app/api/users.py +++ b/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() diff --git a/app/auth/models.py b/app/auth/models.py index 38bad097..9f84196d 100644 --- a/app/auth/models.py +++ b/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: diff --git a/app/scodoc/sco_import_users.py b/app/scodoc/sco_import_users.py index af0cef3f..1f06f028 100644 --- a/app/scodoc/sco_import_users.py +++ b/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() diff --git a/app/scodoc/sco_users.py b/app/scodoc/sco_users.py index fad28632..158378ce 100644 --- a/app/scodoc/sco_users.py +++ b/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() diff --git a/app/templates/auth/user_info_page.j2 b/app/templates/auth/user_info_page.j2 index e21da165..54f83007 100644 --- a/app/templates/auth/user_info_page.j2 +++ b/app/templates/auth/user_info_page.j2 @@ -9,9 +9,11 @@
Login : {{user.user_name}}
CAS id: {{user.cas_id or "(aucun)"}} - (CAS {{'autorisé' if user.cas_allow_login else 'interdit'}} pour cet utilisateur) - {% if user.cas_allow_scodoc_login %} - (connexion sans CAS autorisée) + {% 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 %}
Nom : {{user.nom or ""}}
diff --git a/app/views/users.py b/app/views/users.py index 3f360c45..143a5e4c 100644 --- a/app/views/users.py +++ b/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, ) diff --git a/tests/unit/test_users.py b/tests/unit/test_users.py index 8f4d8a84..f947bd3b 100644 --- a/tests/unit/test_users.py +++ b/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"] == ""