From 6f97e046d83f09e8d7e0e1aacfa359e89a50a45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Fri, 2 Feb 2024 18:55:12 +0100 Subject: [PATCH 1/4] added mypy type checker --- .dockerignore | 1 + Dockerfile-pr-check | 5 ++-- Makefile | 4 +++ Pipfile | 2 ++ Pipfile.lock | 62 +++++++++++++++++++++++++++++++++++++-------- mypy.ini | 4 +++ requirements.txt | 2 +- tox.ini | 8 +++++- 8 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 mypy.ini diff --git a/.dockerignore b/.dockerignore index 924e997b..e2790b43 100644 --- a/.dockerignore +++ b/.dockerignore @@ -12,6 +12,7 @@ !tests/ !requirements.txt !Makefile +!mypy.ini # keep the test files out of the final image #**/*test* diff --git a/Dockerfile-pr-check b/Dockerfile-pr-check index 12ed421a..ebfc520f 100644 --- a/Dockerfile-pr-check +++ b/Dockerfile-pr-check @@ -4,6 +4,7 @@ USER root COPY Pipfile . COPY Pipfile.lock . COPY tox.ini . +COPY mypy.ini . COPY requirements.txt . COPY Makefile . @@ -18,5 +19,5 @@ RUN pipenv install --deploy && \ #copy the src files COPY . . -#Add command to run tox -CMD ["tox"] \ No newline at end of file +#Add command to run tox +CMD ["tox"] diff --git a/Makefile b/Makefile index 5ead428e..3b4c2ef5 100644 --- a/Makefile +++ b/Makefile @@ -30,6 +30,7 @@ Please use \`make ' where is one of: html create html documentation for the project lint run linting against the project format format linting errors found by lint task + typecheck run type check --- Commands using local services --- create-test-db-file create a Postgres DB dump file for RBAC @@ -95,6 +96,9 @@ lint: format: black -t py39 -l 119 rbac tests +typecheck: + mypy --install-types --non-interactive rbac + reinitdb: make start-db make reset-db diff --git a/Pipfile b/Pipfile index 78c473fe..203b7940 100644 --- a/Pipfile +++ b/Pipfile @@ -48,6 +48,8 @@ flake8 = "==6.0.0" flake8-docstrings = "==1.7.0" flake8-import-order = "==0.18.2" flake8-quotes = "==3.3.2" +mypy = "==1.8.0" +types-pyyaml = "==6.0.12.12" [requires] python_version = "3.9" diff --git a/Pipfile.lock b/Pipfile.lock index ad7a2158..1f130035 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "9c2d76702552a12b6a6993583712186f343232e37d1fb1afaed2593cef09f3f5" + "sha256": "010894659b4482d3250a7b916d76a6126e29fc5109658211103c38bdd7aaa38a" }, "pipfile-spec": 6, "requires": { @@ -84,11 +84,11 @@ }, "certifi": { "hashes": [ - "sha256:9b469f3a900bf28dc19b8cfbf8019bf47f7fdd1a65a1d4ffb98fc14166beb4d1", - "sha256:e036ab49d5b79556f99cfc2d9320b34cfbe5be05c5871b51de9329f0603b0474" + "sha256:0569859f95fc761b18b45ef421b1290a0f65f147e92a1e5eb3e635f9a5e4e66f", + "sha256:dc383c07b76109f368f6106eee2b593b04a011ea4d55f652c6ca24a754d1cdd1" ], "markers": "python_version >= '3.6'", - "version": "==2023.11.17" + "version": "==2024.2.2" }, "cffi": { "hashes": [ @@ -844,11 +844,11 @@ }, "certifi": { "hashes": [ - "sha256:9b469f3a900bf28dc19b8cfbf8019bf47f7fdd1a65a1d4ffb98fc14166beb4d1", - "sha256:e036ab49d5b79556f99cfc2d9320b34cfbe5be05c5871b51de9329f0603b0474" + "sha256:0569859f95fc761b18b45ef421b1290a0f65f147e92a1e5eb3e635f9a5e4e66f", + "sha256:dc383c07b76109f368f6106eee2b593b04a011ea4d55f652c6ca24a754d1cdd1" ], "markers": "python_version >= '3.6'", - "version": "==2023.11.17" + "version": "==2024.2.2" }, "cfgv": { "hashes": [ @@ -1235,6 +1235,40 @@ "markers": "python_version >= '3.6'", "version": "==0.7.0" }, + "mypy": { + "hashes": [ + "sha256:028cf9f2cae89e202d7b6593cd98db6759379f17a319b5faf4f9978d7084cdc6", + "sha256:2afecd6354bbfb6e0160f4e4ad9ba6e4e003b767dd80d85516e71f2e955ab50d", + "sha256:2b5b6c721bd4aabaadead3a5e6fa85c11c6c795e0c81a7215776ef8afc66de02", + "sha256:42419861b43e6962a649068a61f4a4839205a3ef525b858377a960b9e2de6e0d", + "sha256:42c6680d256ab35637ef88891c6bd02514ccb7e1122133ac96055ff458f93fc3", + "sha256:485a8942f671120f76afffff70f259e1cd0f0cfe08f81c05d8816d958d4577d3", + "sha256:4c886c6cce2d070bd7df4ec4a05a13ee20c0aa60cb587e8d1265b6c03cf91da3", + "sha256:4e6d97288757e1ddba10dd9549ac27982e3e74a49d8d0179fc14d4365c7add66", + "sha256:4ef4be7baf08a203170f29e89d79064463b7fc7a0908b9d0d5114e8009c3a259", + "sha256:51720c776d148bad2372ca21ca29256ed483aa9a4cdefefcef49006dff2a6835", + "sha256:52825b01f5c4c1c4eb0db253ec09c7aa17e1a7304d247c48b6f3599ef40db8bd", + "sha256:538fd81bb5e430cc1381a443971c0475582ff9f434c16cd46d2c66763ce85d9d", + "sha256:5c1538c38584029352878a0466f03a8ee7547d7bd9f641f57a0f3017a7c905b8", + "sha256:6ff8b244d7085a0b425b56d327b480c3b29cafbd2eff27316a004f9a7391ae07", + "sha256:7178def594014aa6c35a8ff411cf37d682f428b3b5617ca79029d8ae72f5402b", + "sha256:720a5ca70e136b675af3af63db533c1c8c9181314d207568bbe79051f122669e", + "sha256:7f1478736fcebb90f97e40aff11a5f253af890c845ee0c850fe80aa060a267c6", + "sha256:855fe27b80375e5c5878492f0729540db47b186509c98dae341254c8f45f42ae", + "sha256:8963b83d53ee733a6e4196954502b33567ad07dfd74851f32be18eb932fb1cb9", + "sha256:9261ed810972061388918c83c3f5cd46079d875026ba97380f3e3978a72f503d", + "sha256:99b00bc72855812a60d253420d8a2eae839b0afa4938f09f4d2aa9bb4654263a", + "sha256:ab3c84fa13c04aeeeabb2a7f67a25ef5d77ac9d6486ff33ded762ef353aa5592", + "sha256:afe3fe972c645b4632c563d3f3eff1cdca2fa058f730df2b93a35e3b0c538218", + "sha256:d19c413b3c07cbecf1f991e2221746b0d2a9410b59cb3f4fb9557f0365a1a817", + "sha256:df9824ac11deaf007443e7ed2a4a26bebff98d2bc43c6da21b2b64185da011c4", + "sha256:e46f44b54ebddbeedbd3d5b289a893219065ef805d95094d16a0af6630f5d410", + "sha256:f5ac9a4eeb1ec0f1ccdc6f326bcdb464de5f80eb07fb38b5ddd7b0de6bc61e55" + ], + "index": "pypi", + "markers": "python_version >= '3.8'", + "version": "==1.8.0" + }, "mypy-extensions": { "hashes": [ "sha256:4392f6c0eb8a5668a69e23d168ffa70f0be9ccfd32b5cc2d26a34ae5b844552d", @@ -1269,11 +1303,11 @@ }, "platformdirs": { "hashes": [ - "sha256:11c8f37bcca40db96d8144522d925583bdb7a31f7b0e37e3ed4318400a8e2380", - "sha256:906d548203468492d432bcb294d4bc2fff751bf84971fbb2c10918cc206ee420" + "sha256:0614df2a2f37e1a662acbd8e2b25b92ccf8632929bc6d43467e17fe89c75e068", + "sha256:ef0cc731df711022c174543cb70a9b5bd22e5a9337c8624ef2c2ceb8ddad8768" ], "markers": "python_version >= '3.8'", - "version": "==4.1.0" + "version": "==4.2.0" }, "pluggy": { "hashes": [ @@ -1576,6 +1610,14 @@ "markers": "implementation_name == 'cpython'", "version": "==1.5.5" }, + "types-pyyaml": { + "hashes": [ + "sha256:334373d392fde0fdf95af5c3f1661885fa10c52167b14593eb856289e1855062", + "sha256:c05bc6c158facb0676674b7f11fe3960db4f389718e19e62bd2b84d6205cfd24" + ], + "index": "pypi", + "version": "==6.0.12.12" + }, "typing-extensions": { "hashes": [ "sha256:23478f88c37f27d76ac8aee6c905017a143b0b1b886c3c9f66bc2fd94f9f5783", diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 00000000..5b458079 --- /dev/null +++ b/mypy.ini @@ -0,0 +1,4 @@ +[mypy] +python_version = 3.9 +ignore_missing_imports = True +disable_error_code = annotation-unchecked, var-annotated, misc, index diff --git a/requirements.txt b/requirements.txt index fc8143af..98a58112 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ billiard==4.2.0; python_version >= '3.7' boto3==1.24.24; python_version >= '3.7' botocore==1.27.96; python_version >= '3.7' celery==5.3.0b2; python_version >= '3.7' -certifi==2023.11.17; python_version >= '3.6' +certifi==2024.2.2; python_version >= '3.6' cffi==1.16.0; platform_python_implementation != 'PyPy' charset-normalizer==3.3.2; python_full_version >= '3.7.0' click==8.1.7; python_version >= '3.7' diff --git a/tox.ini b/tox.ini index e1acf799..6720b63a 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = lint, py39 +envlist = lint, py39, mypy skipsdist = True [flake8] @@ -55,3 +55,9 @@ setenv = commands = flake8 rbac black --check -t py39 -l 119 rbac tests --diff + +[testenv:mypy] +deps = + mypy +commands = + mypy --install-types --non-interactive rbac From d77a21895a20b7e6016b08db956baa082af43337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Fri, 2 Feb 2024 18:56:50 +0100 Subject: [PATCH 2/4] fixed errors found by mypy type checker --- rbac/api/models.py | 2 +- rbac/management/authorization/token_validator.py | 9 ++++----- rbac/management/group/view.py | 9 +++++---- rbac/management/principal/it_service.py | 11 ++++++----- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/rbac/api/models.py b/rbac/api/models.py index 11f54c6b..733dd161 100644 --- a/rbac/api/models.py +++ b/rbac/api/models.py @@ -72,5 +72,5 @@ class User: is_active = True org_id = None # Service account properties. - client_id: str = None + client_id: str = "" is_service_account: bool = False diff --git a/rbac/management/authorization/token_validator.py b/rbac/management/authorization/token_validator.py index 8e515fa9..1b389ede 100644 --- a/rbac/management/authorization/token_validator.py +++ b/rbac/management/authorization/token_validator.py @@ -67,8 +67,7 @@ def _get_json_web_keyset(self) -> KeySet: except Exception as e: logger.debug( "Fetching the JSON Web Key Set from Redis raised an exception, attempting to fetch the keys from the" - " OIDC configuration instead. Raised error: ", - e, + f" OIDC configuration instead. Raised error: {e}" ) if jwks_certificates_response: @@ -79,7 +78,7 @@ def _get_json_web_keyset(self) -> KeySet: if not status.is_success(oidc_response.status_code): logger.error( f"Unable to get the OIDC configuration payload when attempting to validate a JWT token. Response" - f" code: {oidc_response.status_code}. Response body: {oidc_response.content}" + f" code: {oidc_response.status_code!r}. Response body: {oidc_response.content!r}" ) raise UnableMeetPrerequisitesError() @@ -90,7 +89,7 @@ def _get_json_web_keyset(self) -> KeySet: if not jwks_uri: logger.error( f"Unable to extract the JWKs' URI when attempting to validate a JWT token. Actual payload:" - f"{oidc_response.content}" + f"{oidc_response.content!r}" ) raise UnableMeetPrerequisitesError() @@ -117,7 +116,7 @@ def _get_json_web_keyset(self) -> KeySet: try: return KeySet.import_key_set(jwks_certificates_response) except Exception as e: - logger.error("Unable to import IT's public keys to validate the token: {token:%s}".format(token=str(e))) + logger.error(f"Unable to import IT's public keys to validate the token: {e}") raise UnableMeetPrerequisitesError() def validate_token(self, request: Request) -> str: diff --git a/rbac/management/group/view.py b/rbac/management/group/view.py index 8aa0a166..d660a3aa 100644 --- a/rbac/management/group/view.py +++ b/rbac/management/group/view.py @@ -17,6 +17,7 @@ """View for group management.""" import logging +from typing import Iterable import requests from django.conf import settings @@ -397,9 +398,9 @@ def add_service_accounts( user: User, group: Group, bearer_token: str, - service_accounts: [dict], - account_name: str = None, - org_id: str = None, + service_accounts: Iterable[dict], + account_name: str = "", + org_id: str = "", ) -> Group: """Process the list of service accounts and add them to the group.""" # Fetch all the user's service accounts from IT. If we are on a development or testing environment, we might @@ -1061,7 +1062,7 @@ def obtain_roles_with_exclusion(self, request, group): return roles.exclude(uuid__in=roles_for_group) def remove_service_accounts( - self, user: User, group: Group, service_accounts: [str], account_name: str = None, org_id: str = None + self, user: User, group: Group, service_accounts: Iterable[str], account_name: str = "", org_id: str = "" ) -> None: """Remove the given service accounts from the tenant.""" # Log our intention. diff --git a/rbac/management/principal/it_service.py b/rbac/management/principal/it_service.py index def9a60b..be170517 100644 --- a/rbac/management/principal/it_service.py +++ b/rbac/management/principal/it_service.py @@ -17,6 +17,7 @@ """Class to manage interactions with the IT service accounts service.""" import logging import time +from typing import Tuple import requests from django.conf import settings @@ -154,7 +155,7 @@ def request_service_accounts(self, bearer_token: str) -> list[dict]: return service_accounts - def get_service_accounts(self, user: User, bearer_token: str, options: dict = {}) -> (list[dict], int): + def get_service_accounts(self, user: User, bearer_token: str, options: dict = {}) -> Tuple[list[dict], int]: """Request and returns the service accounts for the given tenant.""" # We might want to bypass calls to the IT service on ephemeral or test environments. it_service_accounts: list[dict] = [] @@ -173,7 +174,7 @@ def get_service_accounts(self, user: User, bearer_token: str, options: dict = {} # - Admin only # - Email # - Status - usernames: [str] = [] + usernames: list[str] = [] specified_usernames = options.get("usernames") if specified_usernames: usernames = specified_usernames.split(",") @@ -222,7 +223,7 @@ def get_service_accounts(self, user: User, bearer_token: str, options: dict = {} # Filter the incoming service accounts. Also, transform them to the payload we will # be returning. - service_accounts: [dict] = self._merge_principals_it_service_accounts( + service_accounts: list[dict] = self._merge_principals_it_service_accounts( service_account_principals=sap_dict, it_service_accounts=it_service_accounts, options=options ) @@ -279,7 +280,7 @@ def get_service_accounts_group(self, group: Group, bearer_token: str, options: d # Filter the incoming service accounts. Also, transform them to the payload we will # be returning. - service_accounts: [dict] = self._merge_principals_it_service_accounts( + service_accounts: list[dict] = self._merge_principals_it_service_accounts( service_account_principals=sap_dict, it_service_accounts=it_service_accounts, options=options ) @@ -348,7 +349,7 @@ def _merge_principals_it_service_accounts( self, service_account_principals: dict[str, dict], it_service_accounts: list[dict], options: dict ) -> list[dict]: """Merge the database principals with the service account principals and return the response payload.""" - service_accounts: [dict] = [] + service_accounts: list[dict] = [] # If the "username_only" parameter was set, we should only return that for the user. username_only = options.get("username_only") From 8bb762177b08f54079ff6b3e94b7009551ddc671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Fri, 2 Feb 2024 18:57:34 +0100 Subject: [PATCH 3/4] removed not needed variable (is created with same value 2 rows above) --- rbac/management/principal/it_service.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rbac/management/principal/it_service.py b/rbac/management/principal/it_service.py index be170517..fd601b59 100644 --- a/rbac/management/principal/it_service.py +++ b/rbac/management/principal/it_service.py @@ -182,7 +182,6 @@ def get_service_accounts(self, user: User, bearer_token: str, options: dict = {} # If "match_criteria" is specified, only the first username is taken into account. match_criteria = options.get("match_criteria") if match_criteria: - match_criteria: str = options["match_criteria"] username = usernames[0] if match_criteria == "partial": From c3da32c6433dff0ddbea7ac36e2f8904ac0dc81b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Fri, 2 Feb 2024 18:58:22 +0100 Subject: [PATCH 4/4] ignore few type errors --- rbac/management/group/view.py | 2 +- rbac/management/principal/it_service.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rbac/management/group/view.py b/rbac/management/group/view.py index d660a3aa..5e33d113 100644 --- a/rbac/management/group/view.py +++ b/rbac/management/group/view.py @@ -1128,7 +1128,7 @@ def user_has_permission_act_on_service_account(self, user: User, service_account is_user_owner = user.username == owner # Check if the user has the "User Access administrator" permission. Leaving the RAW query here - username: str = user.username + username: str = user.username # type: ignore query = ( "SELECT EXISTS ( " "SELECT " diff --git a/rbac/management/principal/it_service.py b/rbac/management/principal/it_service.py index fd601b59..1bc314f5 100644 --- a/rbac/management/principal/it_service.py +++ b/rbac/management/principal/it_service.py @@ -358,10 +358,10 @@ def _merge_principals_it_service_accounts( sa_principal = service_account_principals[it_service_account["clientID"]] if username_only and username_only == "true": - service_accounts.append({"username": sa_principal.username}) + service_accounts.append({"username": sa_principal.username}) # type: ignore else: # Get the principal's username from the database and set it in the response for the user. - it_service_account["username"] = sa_principal.username + it_service_account["username"] = sa_principal.username # type: ignore service_accounts.append(it_service_account) # If we cannot find a requested service account to IT in the database, we simply