From ff2389f1268144e920a8d1feee58de217752ba18 Mon Sep 17 00:00:00 2001 From: Dominika Trojanowska Date: Mon, 21 Oct 2024 10:59:47 +0200 Subject: [PATCH 1/6] adding comments and simplifying the code --- mxcubeweb/core/components/user/usermanager.py | 135 +++++++++++++----- 1 file changed, 103 insertions(+), 32 deletions(-) diff --git a/mxcubeweb/core/components/user/usermanager.py b/mxcubeweb/core/components/user/usermanager.py index e04133f5a..fdad6dcf8 100644 --- a/mxcubeweb/core/components/user/usermanager.py +++ b/mxcubeweb/core/components/user/usermanager.py @@ -19,14 +19,19 @@ class BaseUserManager(ComponentBase): def __init__(self, app, config): super().__init__(app, config) - def get_observers(self): + def get_observers(self) -> list[User]: + """ + Return users that are in observer mode: logged in + (authenticated and active) but not in control of the beamline. + """ return [ user for user in User.query.all() if ((not user.in_control) and user.is_authenticated and user.is_active) ] - def get_operator(self): + def get_operator(self) -> User: + """Return user (operator) that is controlling the beamline.""" user = None for _u in User.query.all(): @@ -36,10 +41,15 @@ def get_operator(self): return user - def is_operator(self): + def is_operator(self) -> bool: + """Return True if the current_user is an operator.""" return getattr(current_user, "in_control", False) - def active_logged_in_users(self, exclude_inhouse=False): + def active_logged_in_users(self, exclude_inhouse: bool = False) -> list[User]: + """ + Return a list of active logged in users. With or without inhouse, + based on the exclude_inhouse parameter. + """ self.update_active_users() if exclude_inhouse: @@ -51,7 +61,8 @@ def active_logged_in_users(self, exclude_inhouse=False): return users - def get_user(self, username): + def get_user(self, username: str) -> User | None: + """Return user model instance based on username.""" user = None for _u in User.query.all(): @@ -60,7 +71,8 @@ def get_user(self, username): return user - def set_operator(self, username): + def set_operator(self, username: str) -> User | None: + """Set the user with the given username to be an operator.""" user = None for _u in User.query.all(): @@ -72,7 +84,12 @@ def set_operator(self, username): return user - def update_active_users(self): + def update_active_users(self) -> None: + """ + Check if any user have been inactive for a period longer than the + session lifetime. If so, deactivate the user in datastore and emit + the relvant signals `userChanged` and `observersChanged` to the client. + """ for _u in User.query.all(): if ( _u.active @@ -90,7 +107,14 @@ def update_active_users(self): self.app.server.emit("observersChanged", namespace="/hwr") - def update_operator(self, new_login=False): + def update_operator(self, new_login: bool = False) -> None: + """ + Set the operator based on the logged in users. If no user is currently + in control, the first logged in user is set. Additionally, proposal + is set based on the operator selected_proposal field. + + :param bool new_login: True if updating operator was invoked with new user logging in + """ active_in_control = False for _u in User.query.all(): @@ -119,10 +143,11 @@ def update_operator(self, new_login=False): if _u.is_authenticated and _u.in_control: if HWR.beamline.lims.loginType.lower() != "user": self.app.lims.select_proposal(self.app.lims.get_proposal(_u)) - elif _u.selected_proposal is not None: + elif _u.selected_proposal: self.app.lims.select_proposal(_u.selected_proposal) - def is_inhouse_user(self, user_id): + def is_inhouse_user(self, user_id: str) -> bool: + """Retrun True if the user_id is in the in-house user list.""" user_id_list = [ "%s%s" % (code, number) for (code, number) in HWR.beamline.session.in_house_users @@ -131,10 +156,18 @@ def is_inhouse_user(self, user_id): return user_id in user_id_list # Abstract method to be implemented by concrete implementation - def _login(self, login_id, password): + def _login(self, login_id: str, password: str): pass - def login(self, login_id: str, password: str): + def login(self, login_id: str, password: str) -> None: + """ + Create new session for the user if it does not exist. Activate user in + data store. If a sample is loaded in sample changer but not mounted, + mount it and update the smaple list. Try update the operator. + + :param str login_id: username + :param str password: password + """ try: login_res = self._login(login_id, password) except Exception: @@ -170,6 +203,12 @@ def _signout(self): pass def signout(self): + """ + Signing out the current user: if the user was an operator, the queue + and samples restored to init values, the session is cleared, the user + is not an operator anymore. Log out and deactivte the user, and emit + 'observersChanged' signal. + """ self._signout() user = current_user @@ -191,15 +230,19 @@ def signout(self): msg = "User %s signed out" % user.username logging.getLogger("MX3.HWR").info(msg) + # change current_user.active to False self.app.server.user_datastore.deactivate_user(user) flask_security.logout_user() self.app.server.emit("observersChanged", namespace="/hwr") - def is_authenticated(self): + def is_authenticated(self) -> bool: + """Return True whether the current user is authenticated.""" return current_user.is_authenticated() - def force_signout_user(self, username): + def force_signout_user(self, username: str) -> None: + """Force signout of the annonymous or non operating user with the given username. + """ user = self.get_user(username) if not user.in_control or current_user.is_anonymous: @@ -208,7 +251,11 @@ def force_signout_user(self, username): self.app.server.user_datastore.commit() self.app.server.emit("forceSignout", room=socketio_sid, namespace="/hwr") - def login_info(self): + def login_info(self) -> dict: + """ + Return a dictionary with the login information to be displayed in the + application, such as: synchrotron and beamline names, proposal list etc. + """ if not current_user.is_anonymous: login_info = convert_to_dict(json.loads(current_user.limsdata)) @@ -246,7 +293,8 @@ def login_info(self): return res - def update_user(self, user): + def update_user(self, user: User) -> None: + """Update user information in datastore.""" self.app.server.user_datastore.put(user) self.app.server.user_datastore.commit() @@ -265,15 +313,24 @@ def _get_configured_roles(self, user): return list(roles) - def db_create_user(self, user: str, password: str, lims_data: dict): + def db_create_user(self, user: str, password: str, lims_data: dict) -> User: + """ + Create new user in datastore. If the user already exists, + update the user information. + + :param str user: representation of (patial) username and nickname of new user + :param str password: password + :param dict lims_data: dictionary with the lims data to be updated + :return: User model instance existing / added to datastore + """ sid = flask.session["sid"] user_datastore = self.app.server.user_datastore - username = f"{user}-{str(uuid.uuid4())}" if HWR.beamline.lims.loginType.lower() == "user": username = f"{user}" + else: + username = f"{user}-{str(uuid.uuid4())}" - # Make sure that the roles staff and incontrol always - # exists + # Make sure that the roles staff and incontrol always exists if not user_datastore.find_role("staff"): user_datastore.create_role(name="staff") user_datastore.create_role(name="incontrol") @@ -304,7 +361,16 @@ def db_create_user(self, user: str, password: str, lims_data: dict): return user_datastore.find_user(username=username) - def db_set_in_control(self, user, control): + def db_set_in_control(self, user: User, control: bool) -> None: + """ + Update users (their in_control field) in the datastore. If the passed + user becomes an operator (control=True), the remaining users' + in_control fields are set to False. If passed user stops being + an operator, only its in_control field is set to False. + + :param User user: User model instance + :param bool control: the user becomes an operator (Ture) or not (False) + """ user_datastore = self.app.server.user_datastore if control: @@ -317,7 +383,7 @@ def db_set_in_control(self, user, control): user_datastore.put(_u) else: _u = user_datastore.find_user(username=user.username) - _u.in_control = control + _u.in_control = False user_datastore.put(_u) self.app.server.user_datastore.commit() @@ -327,7 +393,15 @@ class UserManager(BaseUserManager): def __init__(self, app, config): super().__init__(app, config) - def _login(self, login_id: str, password: str): + def _login(self, login_id: str, password: str) -> dict: + """ + Check loging conditions (anonymous, local/remote, existing session) + and return the login response information. + + :param str login_id: username + :param str password: password + :return: login response information + """ login_res = self.app.lims.lims_login(login_id, password, create_session=False) inhouse = self.is_inhouse_user(login_id) @@ -383,17 +457,14 @@ def _login(self, login_id: str, password: str): raise Exception("Remote access disabled") # Only allow remote logins with existing sessions - if self.app.lims.lims_valid_login(login_res) and is_local_host(): + if self.app.lims.lims_valid_login(login_res): if not self.app.lims.lims_existing_session(login_res): login_res = self.app.lims.create_lims_session(login_res) - - msg = "[LOGIN] Valid login from local host (%s)" % str(info) - logging.getLogger("MX3.HWR").info(msg) - elif self.app.lims.lims_valid_login( - login_res - ) and self.app.lims.lims_existing_session(login_res): - msg = "[LOGIN] Valid remote login from %s with existing session (%s)" - msg += msg % (remote_addr(), str(info)) + if is_local_host(): + msg = "[LOGIN] Valid login from local host (%s)" % str(info) + else: + msg = "[LOGIN] Valid remote login from %s with existing session (%s)" + msg += msg % (remote_addr(), str(info)) logging.getLogger("MX3.HWR").info(msg) else: logging.getLogger("MX3.HWR").info("Invalid login %s" % info) From 3b1cb4ff33d13472d9a58ef57059bbc2247184c8 Mon Sep 17 00:00:00 2001 From: Dominika Trojanowska Date: Tue, 22 Oct 2024 11:27:05 +0200 Subject: [PATCH 2/6] docstrings in google style --- mxcubeweb/core/components/user/usermanager.py | 120 ++++++++++++------ 1 file changed, 78 insertions(+), 42 deletions(-) diff --git a/mxcubeweb/core/components/user/usermanager.py b/mxcubeweb/core/components/user/usermanager.py index fdad6dcf8..3d17b46f5 100644 --- a/mxcubeweb/core/components/user/usermanager.py +++ b/mxcubeweb/core/components/user/usermanager.py @@ -20,9 +20,10 @@ def __init__(self, app, config): super().__init__(app, config) def get_observers(self) -> list[User]: - """ - Return users that are in observer mode: logged in - (authenticated and active) but not in control of the beamline. + """List users that are in observer mode. + + Observer mode means user is logged in (authenticated and active) but not in + control of the application. """ return [ user @@ -31,7 +32,7 @@ def get_observers(self) -> list[User]: ] def get_operator(self) -> User: - """Return user (operator) that is controlling the beamline.""" + """Return user object that is controlling the beamline (operator).""" user = None for _u in User.query.all(): @@ -46,9 +47,10 @@ def is_operator(self) -> bool: return getattr(current_user, "in_control", False) def active_logged_in_users(self, exclude_inhouse: bool = False) -> list[User]: - """ - Return a list of active logged in users. With or without inhouse, - based on the exclude_inhouse parameter. + """List of active and logged in users. + + Attributes: + exclude_inhouse (bool): exclude inhouse users from the list """ self.update_active_users() @@ -113,7 +115,8 @@ def update_operator(self, new_login: bool = False) -> None: in control, the first logged in user is set. Additionally, proposal is set based on the operator selected_proposal field. - :param bool new_login: True if updating operator was invoked with new user logging in + Attributes: + new_login: True if method was invoked with new user login. """ active_in_control = False @@ -147,7 +150,11 @@ def update_operator(self, new_login: bool = False) -> None: self.app.lims.select_proposal(_u.selected_proposal) def is_inhouse_user(self, user_id: str) -> bool: - """Retrun True if the user_id is in the in-house user list.""" + """Retrun True if the user_id is in the in-house user list. + + Attributes: + user_id: user id composed from code and number. + """ user_id_list = [ "%s%s" % (code, number) for (code, number) in HWR.beamline.session.in_house_users @@ -165,8 +172,9 @@ def login(self, login_id: str, password: str) -> None: data store. If a sample is loaded in sample changer but not mounted, mount it and update the smaple list. Try update the operator. - :param str login_id: username - :param str password: password + Attributes: + login_id: username. + password: password. """ try: login_res = self._login(login_id, password) @@ -203,11 +211,11 @@ def _signout(self): pass def signout(self): - """ - Signing out the current user: if the user was an operator, the queue - and samples restored to init values, the session is cleared, the user - is not an operator anymore. Log out and deactivte the user, and emit - 'observersChanged' signal. + """Sign out the current user. + + If the user was an operator, the queue and samples are restored to init values, + the session is cleared, the user is not an operator anymore. Log out and + deactivte the user, and emit 'observersChanged' signal. """ self._signout() user = current_user @@ -241,7 +249,10 @@ def is_authenticated(self) -> bool: return current_user.is_authenticated() def force_signout_user(self, username: str) -> None: - """Force signout of the annonymous or non operating user with the given username. + """Force signout of the annonymous or non operating user. + + Attributes: + username: username of the user to be signed out. """ user = self.get_user(username) @@ -252,9 +263,12 @@ def force_signout_user(self, username: str) -> None: self.app.server.emit("forceSignout", room=socketio_sid, namespace="/hwr") def login_info(self) -> dict: - """ - Return a dictionary with the login information to be displayed in the - application, such as: synchrotron and beamline names, proposal list etc. + """Login information to be displayed in the application. + + Infomration such as: synchrotron and beamline names, user infromation, proposals list, selected proposal etc. + + Returns: + dictionary with login information. """ if not current_user.is_anonymous: login_info = convert_to_dict(json.loads(current_user.limsdata)) @@ -294,11 +308,22 @@ def login_info(self) -> dict: return res def update_user(self, user: User) -> None: - """Update user information in datastore.""" + """Update user information in datastore. + + Attributes: + user: User model instance. + """ self.app.server.user_datastore.put(user) self.app.server.user_datastore.commit() - def _get_configured_roles(self, user): + def _get_configured_roles(self, user: str) -> list[str]: + """Get the roles configured for the user. + + Inhouse user is always assigned additionaly a staff role. + + Attributes: + user: username. + """ roles = set() _ihs = ["%s%s" % prop for prop in HWR.beamline.session.in_house_users] @@ -314,14 +339,20 @@ def _get_configured_roles(self, user): return list(roles) def db_create_user(self, user: str, password: str, lims_data: dict) -> User: - """ - Create new user in datastore. If the user already exists, - update the user information. + """Create or update user in datastore. + + If the user already exists, update the user information. If not create new one. + Assign roles to the user, prevoiusly making sure the roles of 'staff' and + 'incontrol' existis in data store. If not create them also. - :param str user: representation of (patial) username and nickname of new user - :param str password: password - :param dict lims_data: dictionary with the lims data to be updated - :return: User model instance existing / added to datastore + Attributes: + user: representation of username (eventually part of it). Also a nickname + for new users. + password (unused): password. + lims_data: dictionary with the lims data to be updated. + + Returns: + User model instance existing in or added to datastore. """ sid = flask.session["sid"] user_datastore = self.app.server.user_datastore @@ -362,14 +393,15 @@ def db_create_user(self, user: str, password: str, lims_data: dict) -> User: return user_datastore.find_user(username=username) def db_set_in_control(self, user: User, control: bool) -> None: - """ - Update users (their in_control field) in the datastore. If the passed - user becomes an operator (control=True), the remaining users' - in_control fields are set to False. If passed user stops being - an operator, only its in_control field is set to False. + """Update users (their in_control field) in the datastore. + + If the passed user becomes an operator (control=True), the remaining users' + in_control fields are set to False. If passed user stops being an operator, + only its in_control field is set to False. - :param User user: User model instance - :param bool control: the user becomes an operator (Ture) or not (False) + Attributes: + user: User model instance. + control: the user becomes an operator (Ture) or not (False). """ user_datastore = self.app.server.user_datastore @@ -394,13 +426,17 @@ def __init__(self, app, config): super().__init__(app, config) def _login(self, login_id: str, password: str) -> dict: - """ - Check loging conditions (anonymous, local/remote, existing session) - and return the login response information. + """Check loging conditions + + Login conditions are: active, anonymous, inhouse, local/remote login, within + existing session or not. + + Attributes: + login_id: username + password: password - :param str login_id: username - :param str password: password - :return: login response information + Returns: + dictionary with login response information. """ login_res = self.app.lims.lims_login(login_id, password, create_session=False) inhouse = self.is_inhouse_user(login_id) From cea590dcf0f1efbaffa1f39ded8b7d9fd1544562 Mon Sep 17 00:00:00 2001 From: Dominika Trojanowska Date: Tue, 22 Oct 2024 12:09:06 +0200 Subject: [PATCH 3/6] adjusting for pytest --- mxcubeweb/core/components/user/usermanager.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mxcubeweb/core/components/user/usermanager.py b/mxcubeweb/core/components/user/usermanager.py index 3d17b46f5..93af06fee 100644 --- a/mxcubeweb/core/components/user/usermanager.py +++ b/mxcubeweb/core/components/user/usermanager.py @@ -1,5 +1,6 @@ import logging import json +from typing import List, Union import uuid import datetime @@ -19,7 +20,7 @@ class BaseUserManager(ComponentBase): def __init__(self, app, config): super().__init__(app, config) - def get_observers(self) -> list[User]: + def get_observers(self) -> List[User]: """List users that are in observer mode. Observer mode means user is logged in (authenticated and active) but not in @@ -46,7 +47,7 @@ def is_operator(self) -> bool: """Return True if the current_user is an operator.""" return getattr(current_user, "in_control", False) - def active_logged_in_users(self, exclude_inhouse: bool = False) -> list[User]: + def active_logged_in_users(self, exclude_inhouse: bool = False) -> List[User]: """List of active and logged in users. Attributes: @@ -63,7 +64,7 @@ def active_logged_in_users(self, exclude_inhouse: bool = False) -> list[User]: return users - def get_user(self, username: str) -> User | None: + def get_user(self, username: str) -> Union[User, None]: """Return user model instance based on username.""" user = None @@ -73,7 +74,7 @@ def get_user(self, username: str) -> User | None: return user - def set_operator(self, username: str) -> User | None: + def set_operator(self, username: str) -> Union[User, None]: """Set the user with the given username to be an operator.""" user = None From 411012dd9c25fe08c3e25d59df737f8ff8a104e8 Mon Sep 17 00:00:00 2001 From: Dominika Trojanowska Date: Thu, 24 Oct 2024 14:45:43 +0200 Subject: [PATCH 4/6] simplify even more --- mxcubeweb/core/components/user/usermanager.py | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/mxcubeweb/core/components/user/usermanager.py b/mxcubeweb/core/components/user/usermanager.py index 93af06fee..32eae486e 100644 --- a/mxcubeweb/core/components/user/usermanager.py +++ b/mxcubeweb/core/components/user/usermanager.py @@ -441,11 +441,13 @@ def _login(self, login_id: str, password: str) -> dict: """ login_res = self.app.lims.lims_login(login_id, password, create_session=False) inhouse = self.is_inhouse_user(login_id) + valid_login = self.app.lims.lims_valid_login(login_res) + existing_session = self.app.lims.lims_existing_session(login_res) info = { - "valid": self.app.lims.lims_valid_login(login_res), + "valid": valid_login, "local": is_local_host(), - "existing_session": self.app.lims.lims_existing_session(login_res), + "existing_session": existing_session, "inhouse": inhouse, } @@ -465,7 +467,7 @@ def _login(self, login_id: str, password: str) -> dict: ) # Only allow in-house log-in from local host - if inhouse and not (inhouse and is_local_host()): + if inhouse and not is_local_host(): raise Exception("In-house only allowed from localhost") non_inhouse_active_users = self.active_logged_in_users(exclude_inhouse=True) @@ -494,19 +496,18 @@ def _login(self, login_id: str, password: str) -> dict: raise Exception("Remote access disabled") # Only allow remote logins with existing sessions - if self.app.lims.lims_valid_login(login_res): - if not self.app.lims.lims_existing_session(login_res): + if valid_login and is_local_host(): + if not existing_session: login_res = self.app.lims.create_lims_session(login_res) - if is_local_host(): - msg = "[LOGIN] Valid login from local host (%s)" % str(info) - else: - msg = "[LOGIN] Valid remote login from %s with existing session (%s)" - msg += msg % (remote_addr(), str(info)) - logging.getLogger("MX3.HWR").info(msg) + msg = "[LOGIN] Valid login from local host (%s)" % str(info) + elif valid_login and existing_session: + msg = "[LOGIN] Valid remote login from %s with existing session (%s)" + msg += msg % (remote_addr(), str(info)) else: logging.getLogger("MX3.HWR").info("Invalid login %s" % info) raise Exception(str(info)) - + logging.getLogger("MX3.HWR").info(msg) + return login_res def _signout(self): From 49c6c02add84a38c40af4fc13f335a5cc6d7094f Mon Sep 17 00:00:00 2001 From: Dominika Trojanowska Date: Thu, 31 Oct 2024 16:10:49 +0100 Subject: [PATCH 5/6] precommit --- mxcubeweb/core/components/user/usermanager.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mxcubeweb/core/components/user/usermanager.py b/mxcubeweb/core/components/user/usermanager.py index 86517ac65..5e3dfe5c4 100644 --- a/mxcubeweb/core/components/user/usermanager.py +++ b/mxcubeweb/core/components/user/usermanager.py @@ -2,7 +2,10 @@ import json import logging import uuid -from typing import List, Union +from typing import ( + List, + Union, +) import flask import flask_security @@ -509,7 +512,7 @@ def _login(self, login_id: str, password: str) -> dict: logging.getLogger("MX3.HWR").info("Invalid login %s" % info) raise Exception(str(info)) logging.getLogger("MX3.HWR").info(msg) - + return login_res def _signout(self): From 7eaa50334324bab6492c073a31b3cde0cc189fad Mon Sep 17 00:00:00 2001 From: Dominika Trojanowska Date: Mon, 4 Nov 2024 11:10:00 +0100 Subject: [PATCH 6/6] list[str] -> List[str] --- mxcubeweb/core/components/user/usermanager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mxcubeweb/core/components/user/usermanager.py b/mxcubeweb/core/components/user/usermanager.py index 5e3dfe5c4..b587b5c6b 100644 --- a/mxcubeweb/core/components/user/usermanager.py +++ b/mxcubeweb/core/components/user/usermanager.py @@ -322,7 +322,7 @@ def update_user(self, user: User) -> None: self.app.server.user_datastore.put(user) self.app.server.user_datastore.commit() - def _get_configured_roles(self, user: str) -> list[str]: + def _get_configured_roles(self, user: str) -> List[str]: """Get the roles configured for the user. Inhouse user is always assigned additionaly a staff role.