From 7ea31e1cba8651f5f3d295e71a79d102c6dff9ba Mon Sep 17 00:00:00 2001 From: David Svenson Date: Tue, 16 Apr 2024 09:07:42 +0200 Subject: [PATCH 1/6] Add mypy to dev deps. --- requirements-dev.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements-dev.txt b/requirements-dev.txt index 7aae967..0f90b53 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -5,3 +5,4 @@ pytest-asyncio pytest-cov sphinx sphinx-rtd-theme +mypy From 06213ae1c91c8690cb6e72d5b0f2db9ecfb08d6b Mon Sep 17 00:00:00 2001 From: David Svenson Date: Tue, 16 Apr 2024 09:29:13 +0200 Subject: [PATCH 2/6] mypy --install-types --- requirements-dev.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/requirements-dev.txt b/requirements-dev.txt index 0f90b53..96953eb 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -6,3 +6,9 @@ pytest-cov sphinx sphinx-rtd-theme mypy +types-docutils +types-mock +types-Pygments +types-pyOpenSSL +types-pytz +types-setuptools From aed72c6cc2fe1e38b81a31e0cacd3b5cfa953b0a Mon Sep 17 00:00:00 2001 From: David Svenson Date: Tue, 16 Apr 2024 09:07:49 +0200 Subject: [PATCH 3/6] Drop duplicate method. --- bankid/experimental/helper.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/bankid/experimental/helper.py b/bankid/experimental/helper.py index 33ef57d..cf35e4c 100644 --- a/bankid/experimental/helper.py +++ b/bankid/experimental/helper.py @@ -25,10 +25,6 @@ def __init__(self, signature): self.root = ET.fromstring(signature.decode) self.raw = signature - @property - def signature_value(self): - return B64Value(self.root[1].text).decode - @property def signed_data_digest(self): return self.root[0][2][2] From 63e2c33f36b3cfeb40ce5cb083e4019382bb80e3 Mon Sep 17 00:00:00 2001 From: David Svenson Date: Tue, 16 Apr 2024 11:10:24 +0200 Subject: [PATCH 4/6] Fix type errors and add type annotations. --- .gitignore | 4 +-- bankid/asyncclient.py | 58 +++++++++++++++----------------- bankid/baseclient.py | 38 ++++++++++----------- bankid/certs/__init__.py | 5 +-- bankid/certutils.py | 20 +++++++----- bankid/exceptions.py | 30 +++++++++-------- bankid/qr.py | 4 ++- bankid/syncclient.py | 56 ++++++++++++++----------------- mypy.ini | 13 ++++++++ py.typed | 0 tests/conftest.py | 12 +++---- tests/test_asyncclient.py | 43 ++++++++++++------------ tests/test_certutils.py | 2 +- tests/test_exceptions.py | 10 +++--- tests/test_syncclient.py | 69 ++++++++++++++++++++------------------- 15 files changed, 188 insertions(+), 176 deletions(-) create mode 100644 mypy.ini create mode 100644 py.typed diff --git a/.gitignore b/.gitignore index cc484ff..fd6451c 100644 --- a/.gitignore +++ b/.gitignore @@ -63,6 +63,8 @@ docs/_build/ # PyBuilder target/ +# mypy +.mypy_cache ### Vagrant template .vagrant/ @@ -125,5 +127,3 @@ atlassian-ide-plugin.xml com_crashlytics_export_strings.xml crashlytics.properties crashlytics-build.properties - - diff --git a/bankid/asyncclient.py b/bankid/asyncclient.py index 0625546..0f2b88a 100644 --- a/bankid/asyncclient.py +++ b/bankid/asyncclient.py @@ -1,4 +1,4 @@ -from typing import Optional, Tuple, Dict, Any +from typing import Any, Dict, Tuple, Union import httpx @@ -6,7 +6,7 @@ from bankid.exceptions import get_json_error_class -class BankIDAsyncClient(BankIDClientBaseclass): +class BankIDAsyncClient(BankIDClientBaseclass[httpx.AsyncClient]): """The asynchronous client to use for communicating with BankID servers via the v6 API. :param certificates: Tuple of string paths to the certificate to use and @@ -19,25 +19,19 @@ class BankIDAsyncClient(BankIDClientBaseclass): """ - def __init__(self, certificates: Tuple[str, str], test_server: bool = False, request_timeout: Optional[int] = None): + def __init__(self, certificates: Tuple[str, str], test_server: bool = False, request_timeout: int = 5): super().__init__(certificates, test_server, request_timeout) - kwargs = { - "cert": self.certs, - "headers": {"Content-Type": "application/json"}, - "verify": self.verify_cert, - } - if request_timeout: - kwargs["timeout"] = request_timeout - self.client = httpx.AsyncClient(**kwargs) + headers = {"Content-Type": "application/json"} + self.client = httpx.AsyncClient(cert=self.certs, headers=headers, verify=str(self.verify_cert), timeout=request_timeout) async def authenticate( self, end_user_ip: str, - requirement: Dict[str, Any] = None, - user_visible_data: str = None, - user_non_visible_data: str = None, - user_visible_data_format: str = None, + requirement: Union[Dict[str, Any], None] = None, + user_visible_data: Union[str, None] = None, + user_non_visible_data: Union[str, None] = None, + user_visible_data_format: Union[str, None] = None, ) -> Dict[str, str]: """Request an authentication order. The :py:meth:`collect` method is used to query the status of the order. @@ -85,7 +79,7 @@ async def authenticate( response = await self.client.post(self._auth_endpoint, json=data) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) @@ -93,10 +87,10 @@ async def phone_authenticate( self, personal_number: str, call_initiator: str, - requirement: Dict[str, Any] = None, - user_visible_data: str = None, - user_non_visible_data: str = None, - user_visible_data_format: str = None, + requirement: Union[Dict[str, Any], None] = None, + user_visible_data: Union[str, None] = None, + user_non_visible_data: Union[str, None] = None, + user_visible_data_format: Union[str, None] = None, ) -> Dict[str, str]: """Initiates an authentication order when the user is talking to the RP over the phone. The :py:meth:`collect` method @@ -150,17 +144,17 @@ async def phone_authenticate( response = await self.client.post(self._phone_auth_endpoint, json=data) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) async def sign( self, - end_user_ip, + end_user_ip: str, user_visible_data: str, - requirement: Dict[str, Any] = None, - user_non_visible_data: str = None, - user_visible_data_format: str = None, + requirement: Union[Dict[str, Any], None] = None, + user_non_visible_data: Union[str, None] = None, + user_visible_data_format: Union[str, None] = None, ) -> Dict[str, str]: """Request a signing order. The :py:meth:`collect` method is used to query the status of the order. @@ -206,7 +200,7 @@ async def sign( response = await self.client.post(self._sign_endpoint, json=data) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) @@ -215,9 +209,9 @@ async def phone_sign( personal_number: str, call_initiator: str, user_visible_data: str, - requirement: Dict[str, Any] = None, - user_non_visible_data: str = None, - user_visible_data_format: str = None, + requirement: Union[Dict[str, Any], None] = None, + user_non_visible_data: Union[str, None] = None, + user_visible_data_format: Union[str, None] = None, ) -> Dict[str, str]: """Initiates an authentication order when the user is talking to the RP over the phone. The :py:meth:`collect` method @@ -269,7 +263,7 @@ async def phone_sign( response = await self.client.post(self._phone_sign_endpoint, json=data) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) @@ -341,7 +335,7 @@ async def collect(self, order_ref: str) -> dict: response = await self.client.post(self._collect_endpoint, json={"orderRef": order_ref}) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) @@ -362,6 +356,6 @@ async def cancel(self, order_ref: str) -> bool: response = await self.client.post(self._cancel_endpoint, json={"orderRef": order_ref}) if response.status_code == 200: - return response.json() == {} + return response.json() == {} # type: ignore[no-any-return] else: raise get_json_error_class(response) diff --git a/bankid/baseclient.py b/bankid/baseclient.py index 95c9103..4d4a2ca 100644 --- a/bankid/baseclient.py +++ b/bankid/baseclient.py @@ -1,26 +1,31 @@ import base64 from datetime import datetime -from typing import Tuple, Optional, Dict, Any +from typing import Tuple, Dict, Any, Union, TypeVar, Generic from urllib.parse import urljoin from bankid.qr import generate_qr_code_content from bankid.certutils import resolve_cert_path +import httpx -class BankIDClientBaseclass: +TClient = TypeVar("TClient", httpx.AsyncClient, httpx.Client) + + +class BankIDClientBaseclass(Generic[TClient]): """Baseclass for BankID clients. Both the synchronous and asynchronous clients inherit from this base class and has the methods implemented here. """ + client: TClient + def __init__( self, certificates: Tuple[str, str], test_server: bool = False, - request_timeout: Optional[int] = None, + request_timeout: int = 5, ): self.certs = certificates - self._request_timeout = request_timeout if test_server: self.api_url = "https://appapi2.test.bankid.com/rp/v6.0/" @@ -36,28 +41,23 @@ def __init__( self._collect_endpoint = urljoin(self.api_url, "collect") self._cancel_endpoint = urljoin(self.api_url, "cancel") - self.client = None - @staticmethod - def generate_qr_code_content(qr_start_token: str, start_t: [float, datetime], qr_start_secret: str) -> str: + def generate_qr_code_content(qr_start_token: str, start_t: Union[float, datetime], qr_start_secret: str) -> str: return generate_qr_code_content(qr_start_token, start_t, qr_start_secret) @staticmethod - def _encode_user_data(user_data): - if isinstance(user_data, str): - return base64.b64encode(user_data.encode("utf-8")).decode("ascii") - else: - return base64.b64encode(user_data).decode("ascii") + def _encode_user_data(user_data: str) -> str: + return base64.b64encode(user_data.encode("utf-8")).decode("ascii") def _create_payload( self, - end_user_ip: str = None, - requirement: Dict[str, Any] = None, - user_visible_data: str = None, - user_non_visible_data: str = None, - user_visible_data_format: str = None, - ): - data = {} + end_user_ip: Union[str, None] = None, + requirement: Union[Dict[str, Any], None] = None, + user_visible_data: Union[str, None] = None, + user_non_visible_data: Union[str, None] = None, + user_visible_data_format: Union[str, None] = None, + ) -> Dict[str, str]: + data: Dict[str, Any] = {} if end_user_ip: data["endUserIp"] = end_user_ip if requirement and isinstance(requirement, dict): diff --git a/bankid/certs/__init__.py b/bankid/certs/__init__.py index 427af6c..52dd66c 100644 --- a/bankid/certs/__init__.py +++ b/bankid/certs/__init__.py @@ -2,13 +2,14 @@ # We have to pin these to prevent basic MITM attacks. from pathlib import Path +from typing import Tuple -def get_test_cert_p12(): +def get_test_cert_p12() -> Path: return (Path(__file__).parent / "FPTestcert4_20230629.p12").resolve() -def get_test_cert_and_key(): +def get_test_cert_and_key() -> Tuple[Path, Path]: return ( (Path(__file__).parent / "FPTestcert4_20230629_cert.pem").resolve(), (Path(__file__).parent / "FPTestcert4_20230629_key.pem").resolve(), diff --git a/bankid/certutils.py b/bankid/certutils.py index 5edebf5..d5a6cb6 100644 --- a/bankid/certutils.py +++ b/bankid/certutils.py @@ -7,7 +7,7 @@ import os import subprocess -from typing import Tuple +from typing import Tuple, Union import pathlib import importlib.resources @@ -19,10 +19,12 @@ def resolve_cert_path(file: str) -> pathlib.Path: - return importlib.resources.files("bankid.certs").joinpath(file) + path = importlib.resources.files("bankid.certs").joinpath(file) + assert isinstance(path, pathlib.Path) + return path -def create_bankid_test_server_cert_and_key(destination_path: str = ".") -> Tuple[str]: +def create_bankid_test_server_cert_and_key(destination_path: str = ".") -> Tuple[str, str]: """Split the bundled test certificate into certificate and key parts and save them as separate files, stored in PEM format. @@ -35,9 +37,9 @@ def create_bankid_test_server_cert_and_key(destination_path: str = ".") -> Tuple :rtype: tuple """ - if os.getenv("TEST_CERT_FILE"): + if test_cert_file := os.getenv("TEST_CERT_FILE"): certificate, key = split_certificate( - os.getenv("TEST_CERT_FILE"), destination_path, password=_TEST_CERT_PASSWORD + test_cert_file, destination_path, password=_TEST_CERT_PASSWORD ) else: @@ -48,7 +50,7 @@ def create_bankid_test_server_cert_and_key(destination_path: str = ".") -> Tuple return certificate, key -def split_certificate(certificate_path, destination_folder, password=None): +def split_certificate(certificate_path: str, destination_folder: str, password: Union[str, None] = None) -> Tuple[str, str]: """Splits a PKCS12 certificate into Base64-encoded DER certificate and key. This method splits a potentially password-protected @@ -64,7 +66,7 @@ def split_certificate(certificate_path, destination_folder, password=None): try: # Attempt Linux and Darwin call first. p = subprocess.Popen(["openssl", "version"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) - sout, serr = p.communicate() + sout, _ = p.communicate() openssl_executable_version = sout.decode().lower() if not (openssl_executable_version.startswith("openssl") or openssl_executable_version.startswith("libressl")): raise BankIDError("OpenSSL executable could not be found. " "Splitting cannot be performed.") @@ -76,7 +78,7 @@ def split_certificate(certificate_path, destination_folder, password=None): stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) - sout, serr = p.communicate() + sout, _ = p.communicate() if not sout.decode().lower().startswith("openssl"): raise BankIDError("OpenSSL executable could not be found. " "Splitting cannot be performed.") openssl_executable = "C:\\Program Files\\Git\\mingw64\\bin\\openssl.exe" @@ -129,7 +131,7 @@ def split_certificate(certificate_path, destination_folder, password=None): return out_cert_path, out_key_path -def main(verbose=True): +def main(verbose: bool = True) -> Tuple[str, str]: paths = create_bankid_test_server_cert_and_key(os.path.expanduser("~")) if verbose: print("Saved certificate as {0}".format(paths[0])) diff --git a/bankid/exceptions.py b/bankid/exceptions.py index 383d9e5..1d5e535 100644 --- a/bankid/exceptions.py +++ b/bankid/exceptions.py @@ -1,7 +1,9 @@ -# -*- coding: utf-8 -*- +from __future__ import annotations +import httpx +from typing import Any, Dict, Union -def get_json_error_class(response): +def get_json_error_class(response: httpx.Response) -> BankIDError: data = response.json() error_class = _JSON_ERROR_CODE_TO_CLASS.get(data.get("errorCode"), BankIDError) return error_class("{0}: {1}".format(data.get("errorCode"), data.get("details")), raw_data=data) @@ -10,10 +12,10 @@ def get_json_error_class(response): class BankIDError(Exception): """Parent exception class for all PyBankID errors.""" - def __init__(self, *args, **kwargs): - super(BankIDError, self).__init__(*args) - self.rfa = None - self.json = kwargs.get("raw_data", {}) + def __init__(self, *args: Any, raw_data: Union[Dict[str, Any], None] = None, **kwargs: Any) -> None: + super(BankIDError, self).__init__(*args, **kwargs) + self.rfa: Union[int, None] = None + self.json = raw_data or {} class BankIDWarning(Warning): @@ -35,7 +37,7 @@ class InvalidParametersError(BankIDError): """ - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) @@ -53,7 +55,7 @@ class AlreadyInProgressError(BankIDError): """ - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) self.rfa = 4 @@ -71,7 +73,7 @@ class InternalError(BankIDError): """ - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) self.rfa = 5 @@ -87,7 +89,7 @@ class MaintenanceError(BankIDError): """ - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) self.rfa = 5 @@ -103,7 +105,7 @@ class UnauthorizedError(BankIDError): """ - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) @@ -118,7 +120,7 @@ class NotFoundError(BankIDError): """ - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) @@ -133,12 +135,12 @@ class RequestTimeoutError(BankIDError): """ - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) self.rfa = 5 -_JSON_ERROR_CODE_TO_CLASS = { +_JSON_ERROR_CODE_TO_CLASS: Dict[str, type[BankIDError]] = { "invalidParameters": InvalidParametersError, "alreadyInProgress": AlreadyInProgressError, "unauthorized": UnauthorizedError, diff --git a/bankid/qr.py b/bankid/qr.py index d1e375c..47bc490 100644 --- a/bankid/qr.py +++ b/bankid/qr.py @@ -1,3 +1,5 @@ +from typing import Union + import hashlib import hmac import time @@ -5,7 +7,7 @@ from math import floor -def generate_qr_code_content(qr_start_token: str, start_t: [float, datetime], qr_start_secret: str) -> str: +def generate_qr_code_content(qr_start_token: str, start_t: Union[float, datetime], qr_start_secret: str) -> str: """Given QR start token, time.time() or UTC datetime when initiated authentication call was made and the QR start secret, calculate the current QR code content to display. """ diff --git a/bankid/syncclient.py b/bankid/syncclient.py index 2e44a6f..1dfea32 100644 --- a/bankid/syncclient.py +++ b/bankid/syncclient.py @@ -1,4 +1,4 @@ -from typing import Optional, Tuple, Dict, Any +from typing import Any, Dict, Tuple, Union import httpx @@ -6,7 +6,7 @@ from bankid.exceptions import get_json_error_class -class BankIDClient(BankIDClientBaseclass): +class BankIDClient(BankIDClientBaseclass[httpx.Client]): """The synchronous client to use for communicating with BankID servers via the v6 API. :param certificates: Tuple of string paths to the certificate to use and @@ -19,25 +19,19 @@ class BankIDClient(BankIDClientBaseclass): """ - def __init__(self, certificates: Tuple[str, str], test_server: bool = False, request_timeout: Optional[int] = None): + def __init__(self, certificates: Tuple[str, str], test_server: bool = False, request_timeout: int = 5): super().__init__(certificates, test_server, request_timeout) - kwargs = { - "cert": self.certs, - "headers": {"Content-Type": "application/json"}, - "verify": self.verify_cert, - } - if request_timeout: - kwargs["timeout"] = request_timeout - self.client = httpx.Client(**kwargs) + headers= {"Content-Type": "application/json"} + self.client = httpx.Client(cert=self.certs, headers=headers, verify=str(self.verify_cert), timeout=request_timeout) def authenticate( self, end_user_ip: str, - requirement: Dict[str, Any] = None, - user_visible_data: str = None, - user_non_visible_data: str = None, - user_visible_data_format: str = None, + requirement: Union[Dict[str, Any], None] = None, + user_visible_data: Union[str, None] = None, + user_non_visible_data: Union[str, None] = None, + user_visible_data_format: Union[str, None] = None, ) -> Dict[str, str]: """Request an authentication order. The :py:meth:`collect` method is used to query the status of the order. @@ -85,7 +79,7 @@ def authenticate( response = self.client.post(self._auth_endpoint, json=data) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) @@ -93,10 +87,10 @@ def phone_authenticate( self, personal_number: str, call_initiator: str, - requirement: Dict[str, Any] = None, - user_visible_data: str = None, - user_non_visible_data: str = None, - user_visible_data_format: str = None, + requirement: Union[Dict[str, Any], None] = None, + user_visible_data: Union[str, None] = None, + user_non_visible_data: Union[str, None] = None, + user_visible_data_format: Union[str, None] = None, ) -> Dict[str, str]: """Initiates an authentication order when the user is talking to the RP over the phone. The :py:meth:`collect` method @@ -150,7 +144,7 @@ def phone_authenticate( response = self.client.post(self._phone_auth_endpoint, json=data) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) @@ -158,9 +152,9 @@ def sign( self, end_user_ip: str, user_visible_data: str, - requirement: Dict[str, Any] = None, - user_non_visible_data: str = None, - user_visible_data_format: str = None, + requirement: Union[Dict[str, Any], None] = None, + user_non_visible_data: Union[str, None] = None, + user_visible_data_format: Union[str, None] = None, ) -> Dict[str, str]: """Request a signing order. The :py:meth:`collect` method is used to query the status of the order. @@ -205,7 +199,7 @@ def sign( response = self.client.post(self._sign_endpoint, json=data) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) @@ -214,9 +208,9 @@ def phone_sign( personal_number: str, call_initiator: str, user_visible_data: str, - requirement: Dict[str, Any] = None, - user_non_visible_data: str = None, - user_visible_data_format: str = None, + requirement: Union[Dict[str, Any], None] = None, + user_non_visible_data: Union[str, None] = None, + user_visible_data_format: Union[str, None] = None, ) -> Dict[str, str]: """Initiates an authentication order when the user is talking to the RP over the phone. The :py:meth:`collect` method @@ -268,7 +262,7 @@ def phone_sign( response = self.client.post(self._phone_sign_endpoint, json=data) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) @@ -340,7 +334,7 @@ def collect(self, order_ref: str) -> dict: response = self.client.post(self._collect_endpoint, json={"orderRef": order_ref}) if response.status_code == 200: - return response.json() + return response.json() # type: ignore[no-any-return] else: raise get_json_error_class(response) @@ -361,6 +355,6 @@ def cancel(self, order_ref: str) -> bool: response = self.client.post(self._cancel_endpoint, json={"orderRef": order_ref}) if response.status_code == 200: - return response.json() == {} + return response.json() == {} # type: ignore[no-any-return] else: raise get_json_error_class(response) diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 0000000..d4a0d0b --- /dev/null +++ b/mypy.ini @@ -0,0 +1,13 @@ +[mypy] +files = bankid,tests +exclude = bankid/experimental +show_error_codes = true +warn_redundant_casts = true +warn_unused_ignores = true +warn_return_any = true +disallow_untyped_calls = true +enable_error_code = truthy-bool +disallow_incomplete_defs = true +disallow_untyped_defs = true +strict_equality = true +warn_unreachable = true diff --git a/py.typed b/py.typed new file mode 100644 index 0000000..e69de29 diff --git a/tests/conftest.py b/tests/conftest.py index dfdc05e..67c73f2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,7 @@ import random import pytest +from typing import List, Tuple from bankid.certs import get_test_cert_and_key @@ -11,16 +12,16 @@ def ip_address() -> str: @pytest.fixture() -def cert_and_key(): +def cert_and_key() -> Tuple[str, str]: cert, key = get_test_cert_and_key() return str(cert), str(key) @pytest.fixture() -def random_personal_number(): +def random_personal_number() -> str: """Simple random Swedish personal number generator.""" - def _luhn_digit(id_): + def _luhn_digit(id_: str) -> int: """Calculate Luhn control digit for personal number. Code adapted from `Faker @@ -34,11 +35,10 @@ def _luhn_digit(id_): """ - def digits_of(n): + def digits_of(n: int) -> List[int]: return [int(i) for i in str(n)] - id_ = int(id_) * 10 - digits = digits_of(id_) + digits = digits_of(int(id_) * 10) checksum = sum(digits[-1::-2]) for k in digits[-2::-2]: checksum += sum(digits_of(k * 2)) diff --git a/tests/test_asyncclient.py b/tests/test_asyncclient.py index f8b51c3..a66e06c 100644 --- a/tests/test_asyncclient.py +++ b/tests/test_asyncclient.py @@ -15,12 +15,13 @@ import uuid import pytest +from typing import Tuple from bankid import BankIDAsyncClient, exceptions @pytest.mark.asyncio -async def test_authentication_and_collect(cert_and_key, ip_address): +async def test_authentication_and_collect(cert_and_key: Tuple[str, str], ip_address: str) -> None: """Authenticate call and then collect with the returned orderRef UUID.""" c = BankIDAsyncClient(certificates=cert_and_key, test_server=True) assert "appapi2.test.bankid.com.pem" in str(c.verify_cert) @@ -28,14 +29,14 @@ async def test_authentication_and_collect(cert_and_key, ip_address): assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - uuid.UUID(out.get("orderRef"), version=4) - collect_status = await c.collect(out.get("orderRef")) + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = await c.collect(str(order_ref)) assert collect_status.get("status") == "pending" assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") @pytest.mark.asyncio -async def test_sign_and_collect(cert_and_key, ip_address): +async def test_sign_and_collect(cert_and_key: Tuple[str, str], ip_address: str) -> None: """Sign call and then collect with the returned orderRef UUID.""" c = BankIDAsyncClient(certificates=cert_and_key, test_server=True) @@ -46,35 +47,35 @@ async def test_sign_and_collect(cert_and_key, ip_address): ) assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - uuid.UUID(out.get("orderRef"), version=4) - collect_status = await c.collect(out.get("orderRef")) + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = await c.collect(str(order_ref)) assert collect_status.get("status") == "pending" assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") @pytest.mark.asyncio -async def test_phone_sign_and_collect(cert_and_key, random_personal_number): +async def test_phone_sign_and_collect(cert_and_key: Tuple[str, str], random_personal_number: str) -> None: """Phone sign call and then collect with the returned orderRef UUID.""" c = BankIDAsyncClient(certificates=cert_and_key, test_server=True) out = await c.phone_sign(random_personal_number, "RP", user_visible_data="The data to be signed") assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - order_ref = uuid.UUID(out.get("orderRef"), version=4) - collect_status = await c.collect(out.get("orderRef")) + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = await c.collect(str(order_ref)) assert collect_status.get("status") == "pending" assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") @pytest.mark.asyncio -async def test_invalid_orderref_raises_error(cert_and_key): +async def test_invalid_orderref_raises_error(cert_and_key: Tuple[str, str]) -> None: c = BankIDAsyncClient(certificates=cert_and_key, test_server=True) with pytest.raises(exceptions.InvalidParametersError): await c.collect("invalid-uuid") @pytest.mark.asyncio -async def test_already_in_progress_raises_error(cert_and_key, ip_address, random_personal_number): +async def test_already_in_progress_raises_error(cert_and_key: Tuple[str, str], ip_address: str, random_personal_number: str) -> None: c = BankIDAsyncClient(certificates=cert_and_key, test_server=True) await c.authenticate(ip_address, requirement={"personalNumber": random_personal_number}) with pytest.raises(exceptions.AlreadyInProgressError): @@ -82,7 +83,7 @@ async def test_already_in_progress_raises_error(cert_and_key, ip_address, random @pytest.mark.asyncio -async def test_already_in_progress_raises_error_2(cert_and_key, ip_address, random_personal_number): +async def test_already_in_progress_raises_error_2(cert_and_key: Tuple[str, str], ip_address: str, random_personal_number: str) -> None: c = BankIDAsyncClient(certificates=cert_and_key, test_server=True) await c.sign( ip_address, @@ -96,42 +97,42 @@ async def test_already_in_progress_raises_error_2(cert_and_key, ip_address, rand @pytest.mark.asyncio -async def test_authentication_and_cancel(cert_and_key, ip_address): +async def test_authentication_and_cancel(cert_and_key: Tuple[str, str], ip_address: str) -> None: """Authenticate call and then cancel it""" c = BankIDAsyncClient(certificates=cert_and_key, test_server=True) out = await c.authenticate(ip_address) assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - order_ref = uuid.UUID(out.get("orderRef"), version=4) - collect_status = await c.collect(out.get("orderRef")) + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = await c.collect(str(order_ref)) assert collect_status.get("status") == "pending" assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") success = await c.cancel(str(order_ref)) assert success with pytest.raises(exceptions.InvalidParametersError): - collect_status = await c.collect(out.get("orderRef")) + collect_status = await c.collect(str(order_ref)) @pytest.mark.asyncio -async def test_phone_authentication_and_cancel(cert_and_key, random_personal_number): +async def test_phone_authentication_and_cancel(cert_and_key: Tuple[str, str], random_personal_number: str) -> None: """Phone authenticate call and then cancel it""" c = BankIDAsyncClient(certificates=cert_and_key, test_server=True) out = await c.phone_authenticate(random_personal_number, "user") assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - order_ref = uuid.UUID(out.get("orderRef"), version=4) - collect_status = await c.collect(out.get("orderRef")) + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = await c.collect(str(order_ref)) assert collect_status.get("status") == "pending" assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") success = await c.cancel(str(order_ref)) assert success with pytest.raises(exceptions.InvalidParametersError): - collect_status = await c.collect(out.get("orderRef")) + collect_status = await c.collect(str(order_ref)) @pytest.mark.asyncio -async def test_cancel_with_invalid_uuid(cert_and_key): +async def test_cancel_with_invalid_uuid(cert_and_key: Tuple[str, str]) -> None: c = BankIDAsyncClient(certificates=cert_and_key, test_server=True) invalid_order_ref = uuid.uuid4() with pytest.raises(exceptions.InvalidParametersError): diff --git a/tests/test_certutils.py b/tests/test_certutils.py index 53be7d0..b68ebcb 100644 --- a/tests/test_certutils.py +++ b/tests/test_certutils.py @@ -5,7 +5,7 @@ import bankid -def test_create_bankid_test_server_cert_and_key(tmpdir_factory: TempdirFactory): +def test_create_bankid_test_server_cert_and_key(tmpdir_factory: TempdirFactory) -> None: paths = bankid.certutils.create_bankid_test_server_cert_and_key(tmpdir_factory.mktemp("certs")) assert os.path.exists(paths[0]) assert os.path.exists(paths[1]) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 95fb4af..badee33 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -1,4 +1,5 @@ from collections import namedtuple +from typing import Union import pytest @@ -18,10 +19,10 @@ (bankid.exceptions.BankIDError, None), ], ) -def test_exceptions(exception_class, rfa): +def test_exceptions(exception_class: type[Exception], rfa: Union[int, None]) -> None: e = exception_class() - assert e.rfa == rfa assert isinstance(e, bankid.exceptions.BankIDError) + assert e.rfa == rfa @pytest.mark.parametrize( @@ -37,8 +38,9 @@ def test_exceptions(exception_class, rfa): (bankid.exceptions.BankIDError, "Unknown error code"), ], ) -def test_error_class_factory(exception_class, error_code): +def test_error_class_factory(exception_class: type[Exception], error_code: str) -> None: MockResponse = namedtuple("MockResponse", ["json"]) response = MockResponse(json=lambda: {"errorCode": error_code}) - e_class = bankid.exceptions.get_json_error_class(response) + # error: Argument 1 to "get_json_error_class" has incompatible type "MockResponse@41"; expected "Response" [arg-type] + e_class = bankid.exceptions.get_json_error_class(response) # type: ignore[arg-type] assert isinstance(e_class, exception_class) diff --git a/tests/test_syncclient.py b/tests/test_syncclient.py index b0a0f02..2d850d3 100644 --- a/tests/test_syncclient.py +++ b/tests/test_syncclient.py @@ -16,30 +16,31 @@ import uuid import pytest +from typing import Tuple try: from unittest import mock except: - import mock + import mock # type: ignore[no-redef] from bankid import BankIDClient, exceptions -def test_authentication_and_collect(cert_and_key, ip_address, random_personal_number): +def test_authentication_and_collect(cert_and_key: Tuple[str, str], ip_address: str, random_personal_number: str) -> None: """Authenticate call and then collect with the returned orderRef UUID.""" c = BankIDClient(certificates=cert_and_key, test_server=True) assert "appapi2.test.bankid.com.pem" in str(c.verify_cert) - out = c.authenticate(ip_address, random_personal_number) + out = c.authenticate(ip_address, requirement={"personalNumber": random_personal_number}) assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - order_ref = uuid.UUID(out.get("orderRef"), version=4) - collect_status = c.collect(out.get("orderRef")) - assert collect_status.get("status") == "pending" - assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = c.collect(str(order_ref)) + assert collect_status["status"] == "pending" + assert collect_status["hintCode"] in ("outstandingTransaction", "noClient") -def test_sign_and_collect(cert_and_key, ip_address): +def test_sign_and_collect(cert_and_key: Tuple[str, str], ip_address: str) -> None: """Sign call and then collect with the returned orderRef UUID.""" c = BankIDClient(certificates=cert_and_key, test_server=True) @@ -50,39 +51,39 @@ def test_sign_and_collect(cert_and_key, ip_address): ) assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - order_ref = uuid.UUID(out.get("orderRef"), version=4) - collect_status = c.collect(out.get("orderRef")) - assert collect_status.get("status") == "pending" - assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = c.collect(str(order_ref)) + assert collect_status["status"] == "pending" + assert collect_status["hintCode"] in ("outstandingTransaction", "noClient") -def test_phone_sign_and_collect(cert_and_key, random_personal_number): +def test_phone_sign_and_collect(cert_and_key: Tuple[str, str], random_personal_number: str) -> None: """Phone sign call and then collect with the returned orderRef UUID.""" c = BankIDClient(certificates=cert_and_key, test_server=True) out = c.phone_sign(random_personal_number, "user", user_visible_data="The data to be signed") assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - order_ref = uuid.UUID(out.get("orderRef"), version=4) - collect_status = c.collect(out.get("orderRef")) - assert collect_status.get("status") == "pending" - assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = c.collect(str(order_ref)) + assert collect_status["status"] == "pending" + assert collect_status["hintCode"] in ("outstandingTransaction", "noClient") -def test_invalid_orderref_raises_error(cert_and_key): +def test_invalid_orderref_raises_error(cert_and_key: Tuple[str, str]) -> None: c = BankIDClient(certificates=cert_and_key, test_server=True) with pytest.raises(exceptions.InvalidParametersError): collect_status = c.collect("invalid-uuid") -def test_already_in_progress_raises_error(cert_and_key, ip_address, random_personal_number): +def test_already_in_progress_raises_error(cert_and_key: Tuple[str, str], ip_address: str, random_personal_number: str) -> None: c = BankIDClient(certificates=cert_and_key, test_server=True) out = c.authenticate(ip_address, requirement={"personalNumber": random_personal_number}) with pytest.raises(exceptions.AlreadyInProgressError): out2 = c.authenticate(ip_address, requirement={"personalNumber": random_personal_number}) -def test_already_in_progress_raises_error_2(cert_and_key, ip_address, random_personal_number): +def test_already_in_progress_raises_error_2(cert_and_key: Tuple[str, str], ip_address: str, random_personal_number: str) -> None: c = BankIDClient(certificates=cert_and_key, test_server=True) out = c.sign(ip_address, requirement={"personalNumber": random_personal_number}, user_visible_data="Text to sign") with pytest.raises(exceptions.AlreadyInProgressError): @@ -91,41 +92,41 @@ def test_already_in_progress_raises_error_2(cert_and_key, ip_address, random_per ) -def test_authentication_and_cancel(cert_and_key, ip_address, random_personal_number): +def test_authentication_and_cancel(cert_and_key: Tuple[str, str], ip_address: str, random_personal_number: str) -> None: """Authenticate call and then cancel it""" c = BankIDClient(certificates=cert_and_key, test_server=True) out = c.authenticate(ip_address, requirement={"personalNumber": random_personal_number}) assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - order_ref = uuid.UUID(out.get("orderRef"), version=4) - collect_status = c.collect(out.get("orderRef")) - assert collect_status.get("status") == "pending" - assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = c.collect(str(order_ref)) + assert collect_status["status"] == "pending" + assert collect_status["hintCode"] in ("outstandingTransaction", "noClient") success = c.cancel(str(order_ref)) assert success with pytest.raises(exceptions.InvalidParametersError): - collect_status = c.collect(out.get("orderRef")) + collect_status = c.collect(str(order_ref)) -def test_phone_authentication_and_cancel(cert_and_key, random_personal_number): +def test_phone_authentication_and_cancel(cert_and_key: Tuple[str, str], random_personal_number: str) -> None: """Phone authenticate call and then cancel it""" c = BankIDClient(certificates=cert_and_key, test_server=True) out = c.phone_authenticate(random_personal_number, "user") assert isinstance(out, dict) # UUID.__init__ performs the UUID compliance assertion. - order_ref = uuid.UUID(out.get("orderRef"), version=4) - collect_status = c.collect(out.get("orderRef")) - assert collect_status.get("status") == "pending" - assert collect_status.get("hintCode") in ("outstandingTransaction", "noClient") + order_ref = uuid.UUID(out["orderRef"], version=4) + collect_status = c.collect(str(order_ref)) + assert collect_status["status"] == "pending" + assert collect_status["hintCode"] in ("outstandingTransaction", "noClient") success = c.cancel(str(order_ref)) assert success with pytest.raises(exceptions.InvalidParametersError): - collect_status = c.collect(out.get("orderRef")) + collect_status = c.collect(str(order_ref)) -def test_cancel_with_invalid_uuid(cert_and_key): +def test_cancel_with_invalid_uuid(cert_and_key: Tuple[str, str]) -> None: c = BankIDClient(certificates=cert_and_key, test_server=True) invalid_order_ref = uuid.uuid4() with pytest.raises(exceptions.InvalidParametersError): @@ -136,7 +137,7 @@ def test_cancel_with_invalid_uuid(cert_and_key): "test_server, endpoint", [(False, "appapi2.bankid.com"), (True, "appapi2.test.bankid.com")], ) -def test_correct_prod_server_urls(cert_and_key, test_server, endpoint): +def test_correct_prod_server_urls(cert_and_key: Tuple[str, str], test_server: bool, endpoint: str) -> None: c = BankIDClient(certificates=cert_and_key, test_server=test_server) assert c.api_url == "https://{0}/rp/v6.0/".format(endpoint) assert "{0}.pem".format(endpoint) in str(c.verify_cert) From 0938ecde9b433632a6f456f5bfc3eb54cb7eaa47 Mon Sep 17 00:00:00 2001 From: David Svenson Date: Tue, 16 Apr 2024 14:20:47 +0200 Subject: [PATCH 5/6] Add type checking to CI. --- .github/workflows/build_and_test.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 300b4c9..b4027ea 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -40,6 +40,9 @@ jobs: # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide flake8 ./bankid --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + - name: Look for type errors + run: mypy + - name: Test with pytest run: | pytest tests --junitxml=junit/test-results-${{ matrix.os }}-${{ matrix.python-version }}.xml --cov=bankid --cov-report=xml --cov-report=html From aa4d46c390190818397584233ead37ef75135bdf Mon Sep 17 00:00:00 2001 From: David Svenson Date: Tue, 16 Apr 2024 16:52:26 +0200 Subject: [PATCH 6/6] Add CONTRIBUTING.md --- CONTRIBUTING.md | 27 +++++++++++++++++++++++++++ README.md | 16 ++++------------ 2 files changed, 31 insertions(+), 12 deletions(-) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..410df34 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,27 @@ +# PyBankID + +Pull requests are welcome! They should target the [develop](https://github.com/hbldh/pybankid/tree/develop) branch. + +## Development + +Dependencies needed for development can be installed through pip: + +```bash +pip install -r requirements-dev.txt +``` + +## Testing + +The PyBankID solution can be tested with [pytest](https://pytest.org/): + +```bash +pytest +``` + +## Type checking + +PyBankID is annotated with types and [mypy](https://www.mypy-lang.org/) is used as type-checker. All contributions should include type annotations. + +```bash +mypy +``` diff --git a/README.md b/README.md index 49430c8..08e6ce5 100644 --- a/README.md +++ b/README.md @@ -28,8 +28,8 @@ PyBankID provides both a synchronous and an asynchronous client for communicatio ```python from bankid import BankIDClient client = BankIDClient(certificates=( - 'path/to/certificate.pem', - 'path/to/key.pem', + 'path/to/certificate.pem', + 'path/to/key.pem', )) ``` @@ -125,12 +125,12 @@ client.collect(order_ref="a9b791c3-459f-492b-bf61-23027876140b") } ``` -Please note that the `collect` method should be used sparingly: in the [BankID Integration Guide](https://www.bankid.com/en/utvecklare/guider/teknisk-integrationsguide) it is specified that *"collect should be called every two seconds and must not be called more frequent than once per second"*. +Please note that the `collect` method should be used sparingly: in the [BankID Integration Guide](https://www.bankid.com/en/utvecklare/guider/teknisk-integrationsguide) it is specified that _"collect should be called every two seconds and must not be called more frequent than once per second"_. PyBankID also implements the `phone/auth` and `phone/sign` methods, for performing authentication and signing with users that are contacted through phone. For documentation on this, see [PyBankID's Read the Docs page](https://pybankid.readthedocs.io/en/latest/). -### Asynchronous client +### Asynchronous client The asynchronous client is used in the same way as the synchronous client, with the difference that all request are performed asynchronously. @@ -182,11 +182,3 @@ print(cert_and_key) client = bankid.BankIDClient( certificates=cert_and_key, test_server=True) ``` - -## Testing - -The PyBankID solution can be tested with [pytest](https://pytest.org/): - -```bash -pytest tests/ -```