From 1be186242d5713ed0f931e328db0c8a59e1fb1aa Mon Sep 17 00:00:00 2001
From: AA Turner <9087854+aa-turner@users.noreply.github.com>
Date: Fri, 5 Feb 2021 13:41:12 +0000
Subject: [PATCH 1/8] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor=20internal=20?=
 =?UTF-8?q?login=20implementation?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 compass/core/logon.py | 126 ++++++++++++++++++++++--------------------
 1 file changed, 65 insertions(+), 61 deletions(-)

diff --git a/compass/core/logon.py b/compass/core/logon.py
index 16c17fce..d19dc629 100644
--- a/compass/core/logon.py
+++ b/compass/core/logon.py
@@ -37,6 +37,8 @@ def __init__(self, credentials: tuple[str, str], role_to_use: str = None):
         self.sto_thread = PeriodicTimer(150, self._extend_session_timeout)
         # self.sto_thread.start()
 
+    # properties/accessors code:
+
     @property
     def mrn(self) -> int:
         return self.compass_dict["Master.User.MRN"]  # Member Role Number
@@ -70,6 +72,8 @@ def hierarchy(self) -> schemas.hierarchy.HierarchyLevel:
 
         return schemas.hierarchy.HierarchyLevel(id=unit_number, level=level_map[unit_level])
 
+    # _get override code:
+
     def _get(self, url: str, auth_header: bool = False, session: Optional[requests.Session] = None, **kwargs) -> requests.Response:
         """Override get method with custom auth_header logic."""
         # pylint: disable=arguments-differ
@@ -98,18 +102,22 @@ def _jk_hash(self) -> str:
         self._post(f"{Settings.base_url}/System/Preflight", json=data)
         return key_hash
 
+    # Timeout code:
+
     def _extend_session_timeout(self, sto: Literal[None, "0", "5", "X"] = "0"):
         # Session time out. 4 values: None (normal), 0 (STO prompt) 5 (Extension, arbitrary constant) X (Hard limit)
         logger.debug(f"Extending session length {datetime.datetime.now()}")
         return self._get(f"{Settings.web_service_path}/STO_CHK", auth_header=True, params={"pExtend": sto})
 
+    # Core login code:
+
     def _do_logon(self, credentials: tuple[str, str] = None, role_to_use: str = None) -> requests.Session:
         """Log in to Compass, change role and confirm success."""
         session = self._create_session()
 
         # Log in and try to confirm success
         self._logon_remote(credentials, session)
-        self._confirm_login_success(session)
+        self._verify_success_update_properties(session)
 
         # Session contains updated auth headers from role change
         session = self.change_role(role_to_use, session=session)
@@ -156,33 +164,46 @@ def _logon_remote(auth: tuple[str, str], session: requests.Session) -> requests.
         response = session.post(f"{Settings.base_url}/Login.ashx", headers=headers, data=credentials)
         return response
 
-    def change_role(self, new_role: Optional[str], session: Optional[requests.Session] = None) -> requests.Session:
-        """Update role information."""
-        if session is None:
-            try:
-                session = self.s
-            except AttributeError:
-                raise ValueError("No session! session object must be passed or self.s set.") from None
+    def _verify_success_update_properties(self, session: requests.Session, check_role_number: int = None) -> requests.Session:
+        """Confirms success and updates authorisation."""
+        # Test 'get' for an exemplar page that needs authorisation.
+        portal_url = f"{Settings.base_url}/ScoutsPortal.aspx"  # TODO use roles page for roles dict?
+        response = self._get(portal_url, session=session)
 
-        if new_role is not None:
-            logger.info("Changing role")
-            new_role = new_role.strip()
+        # Naive check for error, Compass redirects to an error page when something goes wrong
+        # TODO what is the error page URL - what do we expect? From memory Error.aspx
+        # # Response body is login page for failure (~8Kb), but success is a 300 byte page.
+        # if int(post_response.headers.get("content-length", 901)) > 900:
+        #     raise CompassAuthenticationError("Login has failed")
+        if response.url != portal_url:
+            raise CompassAuthenticationError("Login has failed")
 
-            # Change role to the specified role number
-            member_role_number = {v: k for k, v in self.roles_dict.items()}[new_role]
-            session.post(f"{Settings.base_url}/API/ChangeRole", json={"MRN": str(member_role_number)})
-        else:
-            logger.info("not changing role")
-            member_role_number = self.current_role_number
+        # Create lxml html.FormElement
+        form = html.fromstring(response.content).forms[0]
 
-        # Confirm Compass is reporting the changed role number, update auth headers
-        form_tree = self._confirm_role_change(session, member_role_number)
-        session = self._update_authorisation(form_tree, session)
+        # Update session dicts with new role
+        self.compass_dict = self._create_compass_dict(form)  # Updates MRN property etc.
+        self.roles_dict = self._create_roles_dict(form)
 
-        if member_role_number != self.mrn:
-            raise CompassAuthenticationError("Compass Authentication failed to update")
+        # Set auth headers for new role
+        auth_headers = {
+            "Authorization": f"{self.cn}~{self.mrn}",
+            "SID": self.compass_dict["Master.Sys.SessionID"],  # Session ID
+        }
+        session.headers.update(auth_headers)
 
-        logger.info(f"Role updated successfully! Role is now {self.current_role}.")
+        # Update current role properties
+        # TODO is this get role bit needed given that we change the role?
+        self.current_role = self.roles_dict[int(self.mrn)]
+        self.current_role_number = self._get_active_role_number(form)  # TODO mrn property?
+        logger.debug(f"Using Role: {self.current_role}")
+
+        # Verify role number against test value
+        if check_role_number is not None:
+            logger.debug("Confirming role has been changed")
+            # Check that the role has been changed to the desired role. If not, raise exception.
+            if self.current_role_number != check_role_number:
+                raise CompassError("Role failed to update in Compass")
 
         return session
 
@@ -208,48 +229,31 @@ def _get_active_role_number(form_tree: html.FormElement) -> int:
         """Gets active (selected) role from FormElement."""
         return int(form_tree.inputs["ctl00$UserTitleMenu$cboUCRoles"].value)
 
-    def _confirm_login_success(self, session: requests.Session) -> None:
-        """Confirms success and updates authorisation."""
-        portal_url = f"{Settings.base_url}/ScoutsPortal.aspx"
-        response = self._get(portal_url, session=session)
-
-        # # Response body is login page for failure (~8Kb), but success is a 300 byte page.
-        # if int(post_response.headers.get("content-length", 901)) > 900:
-        #     raise CompassAuthenticationError("Login has failed")
-        if response.url != portal_url:
-            raise CompassAuthenticationError("Login has failed")
-
-        form = html.fromstring(response.content).forms[0]
-        self._update_authorisation(form, session)
-
-    def _confirm_role_change(self, session: requests.Session, check_role_number: int) -> html.FormElement:
-        """Confirms success and updates authorisation."""
-        portal_url = f"{Settings.base_url}/ScoutsPortal.aspx"
-        response = self._get(portal_url, session=session)
-        form = html.fromstring(response.content).forms[0]
+    def change_role(self, new_role: Optional[str], session: Optional[requests.Session] = None) -> requests.Session:
+        """Update role information."""
+        if session is None:
+            try:
+                session = self.s
+            except AttributeError:
+                raise ValueError("No session! session object must be passed or self.s set.") from None
 
-        logger.debug("Confirming role has been changed")
-        # Check that the role has been changed to the desired role. If not, raise exception.
-        if self._get_active_role_number(form) != check_role_number:
-            raise CompassError("Role failed to update in Compass")
+        if new_role is not None:
+            logger.info("Changing role")
+            new_role = new_role.strip()
 
-        return form
+            # Change role to the specified role number
+            member_role_number = {v: k for k, v in self.roles_dict.items()}[new_role]
+            session.post(f"{Settings.base_url}/API/ChangeRole", json={"MRN": str(member_role_number)})
+        else:
+            logger.info("not changing role")
+            member_role_number = self.current_role_number
 
-    def _update_authorisation(self, form: html.FormElement, session: requests.Session) -> requests.Session:
-        """Update authorisation data."""
-        self.compass_dict = self._create_compass_dict(form)  # Updates MRN property etc.
-        self.roles_dict = self._create_roles_dict(form)
+        # Confirm Compass is reporting the changed role number, update auth headers
+        session = self._verify_success_update_properties(session, check_role_number=member_role_number)
 
-        # Set auth headers for new role
-        auth_headers = {
-            "Authorization": f"{self.cn}~{self.mrn}",
-            "SID": self.compass_dict["Master.Sys.SessionID"],  # Session ID
-        }
-        session.headers.update(auth_headers)
+        if member_role_number != self.mrn:
+            raise CompassAuthenticationError("Compass Authentication failed to update")
 
-        # TODO is this get role bit needed given that we change the role?
-        self.current_role = self.roles_dict[int(self.mrn)]
-        self.current_role_number = self._get_active_role_number(form)
-        logger.debug(f"Using Role: {self.current_role}")
+        logger.info(f"Role updated successfully! Role is now {self.current_role}.")
 
         return session

From c9e78e8f1af16c86501c024b30904aaab8a34281 Mon Sep 17 00:00:00 2001
From: AA Turner <9087854+aa-turner@users.noreply.github.com>
Date: Fri, 5 Feb 2021 15:23:29 +0000
Subject: [PATCH 2/8] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Move=20change=5Frole?=
 =?UTF-8?q?=20outside=20of=20=5Fdo=5Flogon?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 compass/core/logon.py | 50 ++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 31 deletions(-)

diff --git a/compass/core/logon.py b/compass/core/logon.py
index d19dc629..e615401b 100644
--- a/compass/core/logon.py
+++ b/compass/core/logon.py
@@ -32,7 +32,11 @@ def __init__(self, credentials: tuple[str, str], role_to_use: str = None):
         self.current_role_number: int = 0
         self.roles_dict: dict = {}
 
-        super().__init__(self._do_logon(credentials, role_to_use))
+        super().__init__(self._do_logon(credentials))
+
+        if role_to_use is not None:
+            # Session contains updated auth headers from role change
+            self.change_role(role_to_use)
 
         self.sto_thread = PeriodicTimer(150, self._extend_session_timeout)
         # self.sto_thread.start()
@@ -111,7 +115,7 @@ def _extend_session_timeout(self, sto: Literal[None, "0", "5", "X"] = "0"):
 
     # Core login code:
 
-    def _do_logon(self, credentials: tuple[str, str] = None, role_to_use: str = None) -> requests.Session:
+    def _do_logon(self, credentials: tuple[str, str] = None) -> requests.Session:
         """Log in to Compass, change role and confirm success."""
         session = self._create_session()
 
@@ -119,9 +123,6 @@ def _do_logon(self, credentials: tuple[str, str] = None, role_to_use: str = None
         self._logon_remote(credentials, session)
         self._verify_success_update_properties(session)
 
-        # Session contains updated auth headers from role change
-        session = self.change_role(role_to_use, session=session)
-
         return session
 
     @staticmethod
@@ -202,8 +203,8 @@ def _verify_success_update_properties(self, session: requests.Session, check_rol
         if check_role_number is not None:
             logger.debug("Confirming role has been changed")
             # Check that the role has been changed to the desired role. If not, raise exception.
-            if self.current_role_number != check_role_number:
-                raise CompassError("Role failed to update in Compass")
+            if not (check_role_number == self.current_role_number == self.mrn):
+                raise CompassAuthenticationError("Role failed to update in Compass")
 
         return session
 
@@ -229,31 +230,18 @@ def _get_active_role_number(form_tree: html.FormElement) -> int:
         """Gets active (selected) role from FormElement."""
         return int(form_tree.inputs["ctl00$UserTitleMenu$cboUCRoles"].value)
 
-    def change_role(self, new_role: Optional[str], session: Optional[requests.Session] = None) -> requests.Session:
-        """Update role information."""
-        if session is None:
-            try:
-                session = self.s
-            except AttributeError:
-                raise ValueError("No session! session object must be passed or self.s set.") from None
-
-        if new_role is not None:
-            logger.info("Changing role")
-            new_role = new_role.strip()
-
-            # Change role to the specified role number
-            member_role_number = {v: k for k, v in self.roles_dict.items()}[new_role]
-            session.post(f"{Settings.base_url}/API/ChangeRole", json={"MRN": str(member_role_number)})
-        else:
-            logger.info("not changing role")
-            member_role_number = self.current_role_number
+    def change_role(self, new_role: str) -> None:
+        """Update role information.
 
-        # Confirm Compass is reporting the changed role number, update auth headers
-        session = self._verify_success_update_properties(session, check_role_number=member_role_number)
+        If the user has multiple roles with the same role title, the first is used.
+        """
+        logger.info("Changing role")
 
-        if member_role_number != self.mrn:
-            raise CompassAuthenticationError("Compass Authentication failed to update")
+        # Change role to the specified role number
+        member_role_number = next(num for num, name in self.roles_dict.items() if name == new_role.strip())
+        self._post(f"{Settings.base_url}/API/ChangeRole", json={"MRN": member_role_number})
 
-        logger.info(f"Role updated successfully! Role is now {self.current_role}.")
+        # Confirm Compass is reporting the changed role number, update auth headers
+        self._verify_success_update_properties(self.s, check_role_number=member_role_number)
 
-        return session
+        logger.info(f"Role updated successfully! Role is now {self.current_role}.")

From ae0fce0e2e5730b9e53b20681ad46cb7c2d6f713 Mon Sep 17 00:00:00 2001
From: AA Turner <9087854+aa-turner@users.noreply.github.com>
Date: Fri, 5 Feb 2021 16:09:46 +0000
Subject: [PATCH 3/8] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Simplify=20=5Fverify?=
 =?UTF-8?q?=5Fsuccess=5Fupdate=5Fproperties?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 compass/core/logon.py | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/compass/core/logon.py b/compass/core/logon.py
index e615401b..35754388 100644
--- a/compass/core/logon.py
+++ b/compass/core/logon.py
@@ -29,7 +29,6 @@ def __init__(self, credentials: tuple[str, str], role_to_use: str = None):
         self.role_to_use: str = role_to_use
 
         self.current_role: str = ""
-        self.current_role_number: int = 0
         self.roles_dict: dict = {}
 
         super().__init__(self._do_logon(credentials))
@@ -168,14 +167,14 @@ def _logon_remote(auth: tuple[str, str], session: requests.Session) -> requests.
     def _verify_success_update_properties(self, session: requests.Session, check_role_number: int = None) -> requests.Session:
         """Confirms success and updates authorisation."""
         # Test 'get' for an exemplar page that needs authorisation.
-        portal_url = f"{Settings.base_url}/ScoutsPortal.aspx"  # TODO use roles page for roles dict?
+        portal_url = f"{Settings.base_url}/MemberProfile.aspx?Page=ROLES&TAB"
         response = self._get(portal_url, session=session)
 
-        # Naive check for error, Compass redirects to an error page when something goes wrong
-        # TODO what is the error page URL - what do we expect? From memory Error.aspx
         # # Response body is login page for failure (~8Kb), but success is a 300 byte page.
         # if int(post_response.headers.get("content-length", 901)) > 900:
         #     raise CompassAuthenticationError("Login has failed")
+        # Naive check for error, Compass redirects to an error page when something goes wrong
+        # TODO what is the error page URL - what do we expect? From memory Error.aspx
         if response.url != portal_url:
             raise CompassAuthenticationError("Login has failed")
 
@@ -194,16 +193,15 @@ def _verify_success_update_properties(self, session: requests.Session, check_rol
         session.headers.update(auth_headers)
 
         # Update current role properties
-        # TODO is this get role bit needed given that we change the role?
-        self.current_role = self.roles_dict[int(self.mrn)]
-        self.current_role_number = self._get_active_role_number(form)  # TODO mrn property?
-        logger.debug(f"Using Role: {self.current_role}")
+        self.current_role = self.roles_dict[self.mrn]
+        location = next((row[2].text_content().strip() for row in form.xpath(f"//tbody/tr") if int(row.get("data-pk")) == self.mrn), "")
+        logger.debug(f"Using Role: {self.current_role} ({location})")
 
         # Verify role number against test value
         if check_role_number is not None:
             logger.debug("Confirming role has been changed")
             # Check that the role has been changed to the desired role. If not, raise exception.
-            if not (check_role_number == self.current_role_number == self.mrn):
+            if check_role_number != self.mrn:
                 raise CompassAuthenticationError("Role failed to update in Compass")
 
         return session
@@ -214,8 +212,8 @@ def _create_compass_dict(form_tree: html.FormElement) -> dict:
         compass_dict = {}
         compass_vars = form_tree.fields["ctl00$_POST_CTRL"]
         for pair in compass_vars.split("~"):
-            key, value, *_ = pair.split("#")
-            compass_dict[key] = cast(value)
+            key, value = pair.split("#", 1)
+            compass_dict[key] = cast(value)  # int or str
 
         return compass_dict
 
@@ -225,11 +223,6 @@ def _create_roles_dict(form_tree: html.FormElement) -> dict:
         roles_selector = form_tree.inputs["ctl00$UserTitleMenu$cboUCRoles"]  # get roles from compass page (list of option tags)
         return {int(role.get("value")): role.text.strip() for role in roles_selector}
 
-    @staticmethod
-    def _get_active_role_number(form_tree: html.FormElement) -> int:
-        """Gets active (selected) role from FormElement."""
-        return int(form_tree.inputs["ctl00$UserTitleMenu$cboUCRoles"].value)
-
     def change_role(self, new_role: str) -> None:
         """Update role information.
 
@@ -239,7 +232,8 @@ def change_role(self, new_role: str) -> None:
 
         # Change role to the specified role number
         member_role_number = next(num for num, name in self.roles_dict.items() if name == new_role.strip())
-        self._post(f"{Settings.base_url}/API/ChangeRole", json={"MRN": member_role_number})
+        response = self._post(f"{Settings.base_url}/API/ChangeRole", json={"MRN": member_role_number})  # b"false"
+        logger.debug(f"Compass ChangeRole call returned: {response.json()}")
 
         # Confirm Compass is reporting the changed role number, update auth headers
         self._verify_success_update_properties(self.s, check_role_number=member_role_number)

From c04192a79429b1a4dab39a24e386c35e2f13ef2d Mon Sep 17 00:00:00 2001
From: AA Turner <9087854+aa-turner@users.noreply.github.com>
Date: Fri, 5 Feb 2021 16:19:22 +0000
Subject: [PATCH 4/8] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Use=20object=20session?=
 =?UTF-8?q?=20property?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 compass/core/logon.py | 44 ++++++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/compass/core/logon.py b/compass/core/logon.py
index 35754388..f61ae00d 100644
--- a/compass/core/logon.py
+++ b/compass/core/logon.py
@@ -31,7 +31,11 @@ def __init__(self, credentials: tuple[str, str], role_to_use: str = None):
         self.current_role: str = ""
         self.roles_dict: dict = {}
 
-        super().__init__(self._do_logon(credentials))
+        # Create session
+        super().__init__(self._create_session())
+
+        # Log in and try to confirm success
+        self._logon_remote(credentials)
 
         if role_to_use is not None:
             # Session contains updated auth headers from role change
@@ -77,7 +81,7 @@ def hierarchy(self) -> schemas.hierarchy.HierarchyLevel:
 
     # _get override code:
 
-    def _get(self, url: str, auth_header: bool = False, session: Optional[requests.Session] = None, **kwargs) -> requests.Response:
+    def _get(self, url: str, auth_header: bool = False, **kwargs) -> requests.Response:
         """Override get method with custom auth_header logic."""
         # pylint: disable=arguments-differ
         if auth_header:
@@ -90,10 +94,7 @@ def _get(self, url: str, auth_header: bool = False, session: Optional[requests.S
             kwargs["headers"] = {**kwargs.get("headers", {}), **headers}
             kwargs["params"] = {**kwargs.get("params", {}), **params}
 
-        if session is not None:
-            return session.get(url, **kwargs)
-        else:
-            return super(Logon, self)._get(url, **kwargs)
+        return super(Logon, self)._get(url, **kwargs)
 
     def _jk_hash(self) -> str:
         """Generate JK Hash needed by Compass."""
@@ -114,16 +115,6 @@ def _extend_session_timeout(self, sto: Literal[None, "0", "5", "X"] = "0"):
 
     # Core login code:
 
-    def _do_logon(self, credentials: tuple[str, str] = None) -> requests.Session:
-        """Log in to Compass, change role and confirm success."""
-        session = self._create_session()
-
-        # Log in and try to confirm success
-        self._logon_remote(credentials, session)
-        self._verify_success_update_properties(session)
-
-        return session
-
     @staticmethod
     def _create_session() -> requests.Session:
         """Create a session and get ASP.Net Session ID cookie from the compass server."""
@@ -146,9 +137,8 @@ def _create_session() -> requests.Session:
 
         return session
 
-    @staticmethod
-    def _logon_remote(auth: tuple[str, str], session: requests.Session) -> requests.Response:
-        """Interface code with Compass."""
+    def _logon_remote(self, auth: tuple[str, str]) -> requests.Response:
+        """Log in to Compass and confirm success."""
         # Referer is genuinely needed otherwise login doesn't work
         headers = {"Referer": f"{Settings.base_url}/login/User/Login"}
 
@@ -161,14 +151,18 @@ def _logon_remote(auth: tuple[str, str], session: requests.Session) -> requests.
 
         # log in
         logger.info("Logging in")
-        response = session.post(f"{Settings.base_url}/Login.ashx", headers=headers, data=credentials)
+        response = self._post(f"{Settings.base_url}/Login.ashx", headers=headers, data=credentials)
+
+        # verify log in was successful
+        self._verify_success_update_properties()
+
         return response
 
-    def _verify_success_update_properties(self, session: requests.Session, check_role_number: int = None) -> requests.Session:
+    def _verify_success_update_properties(self, check_role_number: int = None) -> None:
         """Confirms success and updates authorisation."""
         # Test 'get' for an exemplar page that needs authorisation.
         portal_url = f"{Settings.base_url}/MemberProfile.aspx?Page=ROLES&TAB"
-        response = self._get(portal_url, session=session)
+        response = self._get(portal_url)
 
         # # Response body is login page for failure (~8Kb), but success is a 300 byte page.
         # if int(post_response.headers.get("content-length", 901)) > 900:
@@ -190,7 +184,7 @@ def _verify_success_update_properties(self, session: requests.Session, check_rol
             "Authorization": f"{self.cn}~{self.mrn}",
             "SID": self.compass_dict["Master.Sys.SessionID"],  # Session ID
         }
-        session.headers.update(auth_headers)
+        self._update_headers(auth_headers)
 
         # Update current role properties
         self.current_role = self.roles_dict[self.mrn]
@@ -204,8 +198,6 @@ def _verify_success_update_properties(self, session: requests.Session, check_rol
             if check_role_number != self.mrn:
                 raise CompassAuthenticationError("Role failed to update in Compass")
 
-        return session
-
     @staticmethod
     def _create_compass_dict(form_tree: html.FormElement) -> dict:
         """Create Compass info dict from FormElement."""
@@ -236,6 +228,6 @@ def change_role(self, new_role: str) -> None:
         logger.debug(f"Compass ChangeRole call returned: {response.json()}")
 
         # Confirm Compass is reporting the changed role number, update auth headers
-        self._verify_success_update_properties(self.s, check_role_number=member_role_number)
+        self._verify_success_update_properties(check_role_number=member_role_number)
 
         logger.info(f"Role updated successfully! Role is now {self.current_role}.")

From f96bfa2b7067e5e8032cc14ce9e19479e2454df7 Mon Sep 17 00:00:00 2001
From: AA Turner <9087854+aa-turner@users.noreply.github.com>
Date: Fri, 5 Feb 2021 16:32:39 +0000
Subject: [PATCH 5/8] =?UTF-8?q?=F0=9F=93=9D=20Add=20class=20docstring?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 compass/core/logon.py | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/compass/core/logon.py b/compass/core/logon.py
index f61ae00d..3cbdb7eb 100644
--- a/compass/core/logon.py
+++ b/compass/core/logon.py
@@ -1,6 +1,6 @@
 import datetime
 import time
-from typing import Literal, Optional
+from typing import Literal
 
 import certifi
 from lxml import html
@@ -21,6 +21,27 @@
 
 
 class Logon(InterfaceBase):
+    """Create connection to Compass and authenticate. Holds session state.
+
+    Logon flow is:
+    1. Create a persistent state object (Session) to hold headers, cookies etc.
+      a. Check and verify that TLS certificates for Compass have been set up.
+      b. Get ASP.NET session cookie from Compass. (HTTP request #1)
+    2. Post login data to Compass. (HTTP request #2)
+    3. Get sample page from Compass. (HTTP request #3)
+    4. Verify login was successful
+      a. Create dict of compass internal variables, e.g. Master.User.CN (POST_CTRL).
+      b. Create dict of user's role title to internal role number
+      c. Update data for authorisation headers for requests to Compass.
+    IF Role title specified:
+    5. Post new role number to Compass (HTTP request #4)
+    6. Get sample page from Compass. (HTTP request #5)
+    7. Get currently active role number, and check this equals the requested role number.
+      a. Create dict of compass internal variables, e.g. Master.User.CN (POST_CTRL).
+      b. Create dict of user's role title to internal role number
+      c. Update data for authorisation headers for requests to Compass.
+
+    """
     def __init__(self, credentials: tuple[str, str], role_to_use: str = None):
         """Constructor for Logon."""
         self._member_role_number = 0

From 79037bb72874c772c1ea0a65ead775456dbb34c1 Mon Sep 17 00:00:00 2001
From: AA Turner <9087854+aa-turner@users.noreply.github.com>
Date: Fri, 5 Feb 2021 16:36:28 +0000
Subject: [PATCH 6/8] =?UTF-8?q?=F0=9F=8E=A8=20Format?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 compass/core/logon.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/compass/core/logon.py b/compass/core/logon.py
index 3cbdb7eb..06880140 100644
--- a/compass/core/logon.py
+++ b/compass/core/logon.py
@@ -42,6 +42,7 @@ class Logon(InterfaceBase):
       c. Update data for authorisation headers for requests to Compass.
 
     """
+
     def __init__(self, credentials: tuple[str, str], role_to_use: str = None):
         """Constructor for Logon."""
         self._member_role_number = 0
@@ -209,8 +210,8 @@ def _verify_success_update_properties(self, check_role_number: int = None) -> No
 
         # Update current role properties
         self.current_role = self.roles_dict[self.mrn]
-        location = next((row[2].text_content().strip() for row in form.xpath(f"//tbody/tr") if int(row.get("data-pk")) == self.mrn), "")
-        logger.debug(f"Using Role: {self.current_role} ({location})")
+        location = next(row[2].text_content() for row in form.xpath(f"//tbody/tr") if int(row.get("data-pk")) == self.mrn)
+        logger.debug(f"Using Role: {self.current_role} ({location.strip()})")
 
         # Verify role number against test value
         if check_role_number is not None:

From b9485355376206ed6fb6d8f16e0561e4f67a6afa Mon Sep 17 00:00:00 2001
From: AA Turner <9087854+aa-turner@users.noreply.github.com>
Date: Fri, 5 Feb 2021 16:39:02 +0000
Subject: [PATCH 7/8] =?UTF-8?q?=F0=9F=8F=B7=EF=B8=8F=20Add=20logon.TYPES?=
 =?UTF-8?q?=5FSTO?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 compass/core/logon.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/compass/core/logon.py b/compass/core/logon.py
index 06880140..217f2268 100644
--- a/compass/core/logon.py
+++ b/compass/core/logon.py
@@ -18,6 +18,7 @@
 from compass.core.utility import setup_tls_certs
 
 TYPES_UNIT_LEVELS = Literal["Group", "District", "County", "Region", "Country", "Organisation"]
+TYPES_STO = Literal[None, "0", "5", "X"]
 
 
 class Logon(InterfaceBase):
@@ -130,7 +131,7 @@ def _jk_hash(self) -> str:
 
     # Timeout code:
 
-    def _extend_session_timeout(self, sto: Literal[None, "0", "5", "X"] = "0"):
+    def _extend_session_timeout(self, sto: TYPES_STO = "0"):
         # Session time out. 4 values: None (normal), 0 (STO prompt) 5 (Extension, arbitrary constant) X (Hard limit)
         logger.debug(f"Extending session length {datetime.datetime.now()}")
         return self._get(f"{Settings.web_service_path}/STO_CHK", auth_header=True, params={"pExtend": sto})

From 5b348cee952edd6b24d1c408f4ab7648d3f0f5ca Mon Sep 17 00:00:00 2001
From: AA Turner <9087854+aa-turner@users.noreply.github.com>
Date: Fri, 5 Feb 2021 16:47:28 +0000
Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=9A=A8=20Fix=20pylint=20error?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 compass/core/logon.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compass/core/logon.py b/compass/core/logon.py
index 217f2268..62d764e6 100644
--- a/compass/core/logon.py
+++ b/compass/core/logon.py
@@ -211,7 +211,7 @@ def _verify_success_update_properties(self, check_role_number: int = None) -> No
 
         # Update current role properties
         self.current_role = self.roles_dict[self.mrn]
-        location = next(row[2].text_content() for row in form.xpath(f"//tbody/tr") if int(row.get("data-pk")) == self.mrn)
+        location = next(row[2].text_content() for row in form.xpath("//tbody/tr") if int(row.get("data-pk")) == self.mrn)
         logger.debug(f"Using Role: {self.current_role} ({location.strip()})")
 
         # Verify role number against test value