Mypy-enabled PyBankID (#68)

* Update README.rst Corrected typo in examples * Update __init__.py Fix breakage with urllib 2.0.x * Github Action fixes * Version bump * Bump reqs for example * Bundle the BankID Test certificate The BankID pages now returns a captcha instead of the actual certificate when fetching with requests. The actual cert is now bundled instead of fetched each time. * Failure detection in openssl test cert conversion * Add possibility to provide p12 test cert through existing file * Bundle the BankID Test certificate in pem format Bundle pem formats as well. * Python 2.7 compat. fix * Rmoving certutils test for the time being * Remove Python 2.7 support * Remove six dependency * Async client using httpx (#55) * Test against Python 3.12 * Install setuptools after testing * Swap out pkg_resources for importlib * Downgrade importlib-resources to 5.12.0 * Always use compat package * Read required packages in setup.py from requirements.txt * Drop unused six and update docs * Async client * Tidy up async wrapper * Install requirements-dev.txt on CI * Add two more packages to requirements-dev.txt from CI * Update bankid/jsonclient.py Co-authored-by: David Svenson <[email protected]> * Update bankid/jsonclient.py Co-authored-by: David Svenson <[email protected]> * Drop unused TypeVar * Update bankid/jsonclient.py Co-authored-by: David Svenson <[email protected]> --------- Co-authored-by: David Svenson <[email protected]> * Add support for RP v6.0 * First draft of v6 clients Sync and Async clients Implementing parts of v6 API Removing all v5 and v5.1 API implementations Lacking documentation rewrite Builds on #53, #54, #56, #57, #58 * Corrected the example app to work with v1.0.0 * Documentation update * Cleanup before PR Documentation fixes Renaming and docstring fixes Demo app modifications Version bump * Remove .vscode folder * Minor doc change * Updated README.rst * CI changes Removed testing in windows and macos Also removed 3.7 and 3.8 from test matrix. * Upgrading CI action versions * Implemented phone/auth and phone/sign * Update certutils.py Make it even easier to retrieve the test certificate by writing it into the current directory if no path is supplied. * Expose QR code helper explicitly. This simplifies making use of it without having access to a client instance. * Update README.rst - use pytest instead of py.test pytest is the "new" name :) * Version 1.0.1 - Docfix and QR method separate * Cache ip addresses in test suite. Also, only keep a sync version of the ip_address fixture. This avoids httpbin flakyness/unrelibility since the ip address fetch only needs to happen once. * Use builtin importlib.resources. Supporting Python >=3.9 does not require using the backport. Also, use joinpath() to simplify the retrival of the path+return pathlib.Path instead of str. * Documentation updates * Dropping use of httpbin for external ip * Fix for async test * Add mypy to dev deps. * mypy --install-types * Drop duplicate method. * Fix type errors and add type annotations. * Add type checking to CI. * Add CONTRIBUTING.md --------- Co-authored-by: Simon Olofsson <[email protected]> Co-authored-by: Colin 't Hart <[email protected]> Co-authored-by: Stefan Berg <[email protected]> Co-authored-by: William Tisäter <[email protected]> Co-authored-by: David Svenson <[email protected]> Co-authored-by: Amin Solhizadeh <[email protected]> Co-authored-by: Andreas Pelme <[email protected]>
hbldh · Apr 24, 2024 · 1394921 · 1394921
1 parent 49ab8da
commit 1394921
Show file tree

Hide file tree

Showing 24 changed files with 265 additions and 241 deletions.
diff --git 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

diff --git 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/CONTRIBUTING.md 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
@@ -2,9 +2,8 @@
 
 ![Build and Test](https://github.com/hbldh/pybankid/workflows/Build%20and%20Test/badge.svg)
 ![Documentation Status](https://readthedocs.org/projects/pybankid/badge/?version=latest)
-![PyPI Version](http://img.shields.io/pypi/v/pybankid.svg)
-![PyPI License](http://img.shields.io/pypi/l/pybankid.svg)
-![Coverage](https://coveralls.io/repos/github/hbldh/pybankid/badge.svg?branch=master)
+![PyPI Version](https://img.shields.io/pypi/v/pybankid)
+![PyPI License](https://img.shields.io/pypi/l/pybankid)
 
 PyBankID is a client for providing BankID services as a Relying Party, i.e., providing authentication and signing functionality to end users. This package provides a simplifying interface for initiating authentication and signing orders and then collecting the results from the BankID servers.
 
@@ -22,21 +21,21 @@ pip install pybankid
 
 ## Usage
 
-PyBankID provides both a synchronous and an asynchronous client for communication with BankID services. The example below will use the asynchronous client, but the synchronous client is used in the same way by merely omitting the `await` keyword.
+PyBankID provides both a synchronous and an asynchronous client for communication with BankID services. The example below will use the synchronous client, but the asynchronous client is used in an identical fashion, only using the `await` keyword before each function call.
 
 ### Synchronous client
 
 ```python
 from bankid import BankIDClient
 client = BankIDClient(certificates=(
-    'path/to/certificate.pem', 
-    'path/to/key.pem', 
+    'path/to/certificate.pem',
+    'path/to/key.pem',
 ))
 ```
 
 Connection to the production server is the default in the client. If a test server is desired, send in the `test_server=True` keyword in the init of the client.
 
-When using the JSON client, authentication and signing calls require the end user's IP address to be included in all calls. An authentication order is initiated as such:
+All authentication and signing calls require the end user's IP address to be included. An authentication order is initiated as such:
 
 ```python
 client.authenticate(end_user_ip='194.168.2.25')
@@ -79,7 +78,7 @@ client.sign(
 }
 ```
 
-If someone else than the one you specified tries to authenticate or sign, the BankID app will state that the request is not intended for the user.
+If someone other than the one you specified tries to authenticate or sign, the BankID app will state that the request is not intended for the user.
 
 The status of an order can then be studied by polling with the `collect` method using the received `orderRef`:
 
@@ -126,20 +125,22 @@ 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
 
-The asynchronous client is used in the same way as the synchronous client, but the methods are blocking.
+The asynchronous client is used in the same way as the synchronous client, with the difference that all request are performed asynchronously.
 
 The synchronous guide above can be used as a reference for the asynchronous client as well, by simply adding the `await` keyword:
 
 ```python
-from bankid import BankIDClientAsync
-client = BankIDClientAsync(certificates=(
-    'path/to/certificate.pem', 
-    'path/to/key.pem', 
+from bankid import BankIDAsyncClient
+client = BankIDAsyncClient(certificates=(
+    'path/to/certificate.pem',
+    'path/to/key.pem',
 ))
 
 await client.authenticate(end_user_ip='194.168.2.25')
@@ -151,10 +152,12 @@ await client.authenticate(end_user_ip='194.168.2.25')
 }
 ```
 
-
 ## PyBankID and QR codes
 
-PyBankID can generate QR codes for you, and there is an example application in the [examples folder of the repo](https://github.com/hbldh/pybankid/tree/master/examples) where a Flask application called `qrdemo` shows one way to do authentication with animated QR codes.
+PyBankID can generate QR codes for you, and there is an example application in the [examples folder of the repo](https://github.com/hbldh/pybankid/tree/master/examples), where a Flask application called `qrdemo` shows one way to do authentication with animated QR codes.
+
+The QR code content generation is done with the `generate_qr_code_content` method on the BankID Client instances, or directly
+through the identically named method in `bankid.qr` module.
 
 ## Certificates
 
@@ -179,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/
-```
diff --git a/bankid/asyncclient.py b/bankid/asyncclient.py
@@ -1,12 +1,12 @@
-from typing import Optional, Tuple, Dict, Any
+from typing import Any, Dict, Tuple, Union
 
 import httpx
 
 from bankid.baseclient import BankIDClientBaseclass
 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,18 +79,18 @@ 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)
 
     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
@@ -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
@@ -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(),