Add retry on network error capabilities to finish signing / authentication process

.pylintrc

@@ -60,97 +60,24 @@ confidence=
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use "--disable=all --enable=classes
 # --disable=W".
-disable=apply-builtin,
-        backtick,
-        bad-inline-option,
-        bad-python3-import,
-        basestring-builtin,
-        buffer-builtin,
-        cmp-builtin,
-        cmp-method,
-        coerce-builtin,
-        coerce-method,
-        comprehension-escape,
-        delslice-method,
-        deprecated-itertools-function,
-        deprecated-operator-function,
+disable=bad-inline-option,
         deprecated-pragma,
-        deprecated-str-translate-call,
-        deprecated-string-function,
-        deprecated-sys-function,
-        deprecated-types-field,
-        deprecated-urllib-function,
-        dict-items-not-iterating,
-        dict-iter-method,
-        dict-keys-not-iterating,
-        dict-values-not-iterating,
-        dict-view-method,
-        div-method,
-        eq-without-hash,
-        exception-escape,
-        exception-message-attribute,
-        execfile-builtin,
-        file-builtin,
         file-ignored,
-        filter-builtin-not-iterating,
-        getslice-method,
-        hex-method,
-        idiv-method,
-        import-star-module-level,
-        indexing-exception,
-        input-builtin,
-        intern-builtin,
-        invalid-str-codec,
         locally-disabled,
-        long-builtin,
-        long-suffix,
-        map-builtin-not-iterating,
-        metaclass-assignment,
         missing-class-docstring,
         missing-function-docstring,
         missing-module-docstring,
-        next-method-called,
-        next-method-defined,
-        no-absolute-import,
-        non-ascii-bytes-literal,
-        nonzero-method,
-        oct-method,
-        old-division,
-        old-ne-operator,
-        old-octal-literal,
-        old-raise-syntax,
-        parameter-unpacking,
-        raising-string,
-        range-builtin-not-iterating,
         raw-checker-failed,
-        raw_input-builtin,
-        rdiv-method,
-        reduce-builtin,
-        reload-builtin,
-        round-builtin,
-        setslice-method,
-        standarderror-builtin,
         suppressed-message,
-        sys-max-int,
         too-few-public-methods,
-        unichr-builtin,
-        unicode-builtin,
-        unpacking-in-except,
         use-symbolic-message-instead,
         useless-suppression,
-        using-cmp-argument,
-        xrange-builtin,
-        xreadlines-attribute,
-        zip-builtin-not-iterating,
-        # aka Wrong hanging indentation: Pylint disagrees with black, as seen in https://github.com/psf/black/issues/48
-        C0330,
         # these below are supposed to be dealt with at some point...
         abstract-method,
         arguments-differ,
         broad-except,
         fixme,
         invalid-name,
-        no-self-use,
         protected-access,
         super-with-arguments,
         too-many-arguments,
@@ -379,13 +306,6 @@ max-line-length=120
 # Maximum number of lines in a module.
 max-module-lines=1000
 
-# List of optional constructs for which whitespace checking is disabled. `dict-
-# separator` is used to allow tabulation in dicts, etc.: {1  : 1,\n222: 2}.
-# `trailing-comma` allows a space between comma and closing bracket: (a, ).
-# `empty-line` allows space-only lines.
-no-space-check=trailing-comma,
-               dict-separator
-
 # Allow the body of a class to be on the same line as the declaration if body
 # contains single statement.
 single-line-class-stmt=no
@@ -604,5 +524,5 @@ min-public-methods=2
 
 # Exceptions that will emit a warning when being caught. Defaults to
 # "BaseException, Exception".
-overgeneral-exceptions=BaseException,
-                       Exception
+overgeneral-exceptions=builtins.BaseException,
+                       builtins.Exception

CHANGELOG.md

@@ -0,0 +1,38 @@
+# Changelog
+
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
+
+
+## [4.3] - 2025-04-04
+### Added
+- `get_nonce` method on `AuthenticationViewMixin` that can be overridden to provide custom nonce
+- `login` method on `AuthenticationViewMixin` that allows to invoke django `login` while suppressing 
+session key rotation. It is recommended to use this method to avoid `Invalid authentication session` error
+on mobile devices with Smart ID app running on same device as the browser running your webapp; however, 
+you need to consider security risks.
+- Changelog
+
+### Changed
+- Upgrade `esteid-helper` to version [0.6.0](https://github.com/thorgate/esteid-helper/releases/tag/0.6.0)
+that supports retry on network error
+- Persist authentication session state after session completion, including authentication result details.
+This allows to execute multiple `patch` requests within the same session, which is useful for example in case
+of a network error that causes the FE to never receive the finalization response from BE.
+- Persist signing session state after session completion (but not the temporary containers), for same purpose
+as with authentication sessions above.
+
+### Fixed
+- Use unreleased version of `oscrypto`, dependency of pyasice, that fixes compatibility with some OpenSSL versions,
+for tests. If you are getting `oscrypto.errors.LibraryNotFoundError: Error detecting the version of libcrypto` 
+in your project you will need to fix your dependencies as well.
+- Coverage report including legacy compatibility code scheduled for removal (pragma: no cover not applying properly)
+- Exception messages being discarded and default exception message always being used
+
+## [4.3] - 2024-12-31
+### Added
+- Allow to provide custom random bytes to authenticate
+
+[4.3]: https://github.com/olivierlacan/keep-a-changelog/compare/v4.2...v4.3
+[4.2]: https://github.com/olivierlacan/keep-a-changelog/compare/v4.1...v4.2

esteid/actions.py

@@ -1,4 +1,3 @@
-# pragma: no cover
 import base64
 import binascii
 import logging

esteid/authentication/authenticator.py

@@ -2,7 +2,7 @@
 from time import time
 from typing import Dict, Optional, Type
 
-from esteid.authentication.types import AuthenticationResult, SessionData
+from esteid.authentication.types import AuthenticationResult, SessionData, Status
 from esteid.exceptions import EsteidError, SigningSessionDoesNotExist, SigningSessionExists
 
 
@@ -31,6 +31,10 @@ class Authenticator:
 
     AUTHENTICATION_METHODS: Dict[str, Type["Authenticator"]] = {}
 
+    # For some authentication mechanisms that involve talking to the backend, django session needs
+    # needs to be set before the authentication process starts.
+    DJANGO_SESSION_IS_NEEDED = False
+
     _SESSION_KEY = f"{__name__}.session"
 
     # timeout in seconds, after which a fresh session can be started even if old session data is present.
@@ -73,28 +77,35 @@ def setup(self, initial_data: dict = None):
     # Session management.
     # NOTE: this part is largely a copy-paste from signing.
 
-    def save_session_data(self, *, session_id, hash_value_b64):
+    @classmethod
+    def clean_session_data(cls):
         """
-        Saves the session data between initialization and polling requests
+        Creates a new session data object.
         """
-        session_data = self.session_data or SessionData()
-
-        session_data.session_id = session_id
-        session_data.hash_value_b64 = hash_value_b64
-        session_data.timestamp = int(time())
+        return SessionData(status=Status.PENDING, result=None)
 
-        self.session[self._SESSION_KEY] = dict(session_data)
+    def save_session_data(self):
+        """
+        Saves the session data between initialization and polling requests
+        """
+        self.session_data.timestamp = int(time())
+        self.session[self._SESSION_KEY] = dict(self.session_data)
 
     def load_session_data(self, session) -> SessionData:
         try:
             session_data = session[self._SESSION_KEY]
         except KeyError:
-            session_data = {}
+            return self.clean_session_data()
 
         try:
             session_data = SessionData(session_data)
-        except TypeError:
-            session_data = SessionData()
+            if getattr(session_data, "result", None) is not None:
+                session_data.result = AuthenticationResult(session_data.result)
+                session_data.result.is_valid()
+            session_data.is_valid()
+        except (ValueError, TypeError):
+            logging.exception("Invalid session data %r found, cleaning it up.", dict(session_data))
+            session_data = self.clean_session_data()
             self._cleanup_session(session)
 
         # Not doing session data validation here, because
@@ -116,11 +127,11 @@ def __init__(self, session, initial=False, origin=None):
                 except AttributeError:
                     timestamp = 0
 
-                if time() < timestamp + self.SESSION_VALIDITY_TIMEOUT:
+                if time() < timestamp + self.SESSION_VALIDITY_TIMEOUT and session_data.status == Status.PENDING:
                     raise SigningSessionExists("Another authentication session already in progress")
 
-                # session expired => create a fresh data store
-                session_data = SessionData()
+                # session expired or is complete => create a fresh data store
+                session_data = self.clean_session_data()
 
                 # wipe the old data from session.
                 self._cleanup_session(session)

esteid/authentication/tests/test_authenticator.py

@@ -4,13 +4,18 @@
 import pytest
 
 from esteid.authentication import Authenticator
-from esteid.authentication.types import SessionData
+from esteid.authentication.types import SessionData, Status
 from esteid.exceptions import EsteidError, SigningSessionDoesNotExist, SigningSessionExists
 
 
 @pytest.fixture()
 def test_session_data():
-    return SessionData(timestamp=11111, session_id="test", hash_value_b64="MQ==")  # a very old timestamp
+    return SessionData(
+        timestamp=11111,  # a very old timestamp
+        session_id="test",
+        hash_value_b64="MQ==",
+        status=Status.PENDING,
+    )
 
 
 @patch.object(Authenticator, "AUTHENTICATION_METHODS", {})
@@ -45,7 +50,7 @@ def test_authenticator_init__initial_true(test_session_data):
     session = {}
     authenticator = Authenticator(session, initial=True)
 
-    assert authenticator.session_data == {}
+    assert authenticator.session_data == authenticator.clean_session_data()
     assert authenticator.session is session
     assert session == {}
 
@@ -55,28 +60,58 @@ def test_authenticator_init__initial_true(test_session_data):
     session = {Authenticator._SESSION_KEY: wrong_data}
     authenticator = Authenticator(session, initial=True)
 
-    assert authenticator.session_data == {}
+    assert authenticator.session_data == authenticator.clean_session_data()
     assert authenticator.session is session
     assert session == {}
 
     # Expired session data, session is reset
     session = {Authenticator._SESSION_KEY: {**test_session_data}}
     authenticator = Authenticator(session, initial=True)
 
-    assert authenticator.session_data == {}
+    assert authenticator.session_data == authenticator.clean_session_data()
     assert authenticator.session is session
     assert session == {}
 
-    # Some (unvalidated) session data present, not expired => error
-    session = {Authenticator._SESSION_KEY: {"timestamp": int(time()), "key": "value"}}
-    with pytest.raises(SigningSessionExists):
-        Authenticator(session, initial=True)
+    # Some invalid session data present, session is reset
+    session = {
+        Authenticator._SESSION_KEY: {
+            "timestamp": int(time()),
+            "key": "value",
+        }
+    }
+    authenticator = Authenticator(session, initial=True)
+    assert authenticator.session_data == authenticator.clean_session_data()
+    assert authenticator.session is session
+    assert session == {}
 
     # Correct session data present, not expired => error
-    session = {Authenticator._SESSION_KEY: {**test_session_data, "timestamp": int(time()), "key": "value"}}
+    session = {
+        Authenticator._SESSION_KEY: {
+            "timestamp": int(time()),
+            "status": Status.PENDING,
+            "session_id": "test",
+            "result": None,
+            "hash_value_b64": "hash",
+        }
+    }
     with pytest.raises(SigningSessionExists):
         Authenticator(session, initial=True)
 
+    # Correct session data present, not expired but not pending anymore => success, session is reset
+    session = {
+        Authenticator._SESSION_KEY: {
+            "timestamp": int(time()),
+            "status": Status.SUCCESS,
+            "session_id": "test",
+            "result": None,
+            "hash_value_b64": "hash",
+        }
+    }
+    authenticator = Authenticator(session, initial=True)
+    assert authenticator.session_data == authenticator.clean_session_data()
+    assert authenticator.session is session
+    assert session == {}
+
 
 def test_authenticator_init__initial_false(test_session_data):
     # Wrong data: empty

esteid/authentication/types.py

@@ -1,6 +1,17 @@
+import typing as t
+from http import HTTPStatus
+
 from esteid.types import PredictableDict
 
 
+class AuthenticationResult(PredictableDict):
+    country: str
+    id_code: str
+    given_name: str
+    surname: str
+    certificate_b64: str
+
+
 class SessionData(PredictableDict):
     """
     Wrapper for temporary data stored between authentication polling requests.
@@ -12,11 +23,21 @@ class SessionData(PredictableDict):
     timestamp: int
     session_id: str
     hash_value_b64: str
+    status: str
+    result: t.Optional[AuthenticationResult]
 
 
-class AuthenticationResult(PredictableDict):
-    country: str
-    id_code: str
-    given_name: str
-    surname: str
-    certificate_b64: str
+class Status:
+    ERROR = "error"
+    PENDING = "pending"
+    SUCCESS = "success"
+    CANCELLED = "cancelled"
+
+    @classmethod
+    def http_status_for_status(cls, status: str) -> HTTPStatus:
+        return {
+            cls.ERROR: HTTPStatus.GONE,
+            cls.PENDING: HTTPStatus.ACCEPTED,
+            cls.SUCCESS: HTTPStatus.OK,
+            cls.CANCELLED: HTTPStatus.CONFLICT,
+        }.get(status, HTTPStatus.INTERNAL_SERVER_ERROR)