From 435530d842c4278a6563dc3770b9dcf3bd1c900f Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Wed, 18 Sep 2024 09:09:40 +0000 Subject: [PATCH 1/7] add FLY ruleset, more f-string use for readability --- ruff.toml | 2 +- src/eduid/workers/msg/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ruff.toml b/ruff.toml index e43d6b495..9ea698db6 100644 --- a/ruff.toml +++ b/ruff.toml @@ -3,4 +3,4 @@ line-length = 120 target-version = "py310" [lint] -select = ["E4", "E7", "E9", "F", "W", "I", "ASYNC", "UP"] +select = ["E4", "E7", "E9", "F", "W", "I", "ASYNC", "UP", "FLY"] diff --git a/src/eduid/workers/msg/utils.py b/src/eduid/workers/msg/utils.py index d474fa534..8e91771bb 100644 --- a/src/eduid/workers/msg/utils.py +++ b/src/eduid/workers/msg/utils.py @@ -26,7 +26,7 @@ def load_template(template_dir: str, filename: str, message_dict: Mapping[str, s if os.path.isdir(template_dir): try: - f = ".".join([filename, lang]) + f = f"{filename}.{lang}" if os.path.exists(os.path.join(template_dir, f)): filename = f template = Environment(loader=FileSystemLoader(template_dir)).get_template(filename) From 93ab89dbb63fce4caeacd9b5af598a330a60b26a Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Wed, 18 Sep 2024 09:38:39 +0000 Subject: [PATCH 2/7] use keys() or values() instead of item() when only one is used as per the documentation, a micro-optimization but more readable code --- src/eduid/webapp/common/api/testing.py | 2 +- src/eduid/webapp/common/authn/fido_tokens.py | 2 +- src/eduid/webapp/common/authn/utils.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/eduid/webapp/common/api/testing.py b/src/eduid/webapp/common/api/testing.py index a0fd48e13..10c107a5d 100644 --- a/src/eduid/webapp/common/api/testing.py +++ b/src/eduid/webapp/common/api/testing.py @@ -455,7 +455,7 @@ def _check_api_response( def _assure_not_in_dict(d: Mapping[str, Any], unwanted_key: str): assert unwanted_key not in d, f"Key {unwanted_key} should not be in payload, but it is: {payload}" v2: Mapping[str, Any] - for _k2, v2 in d.items(): + for v2 in d.values(): if isinstance(v2, dict): _assure_not_in_dict(v2, unwanted_key) diff --git a/src/eduid/webapp/common/authn/fido_tokens.py b/src/eduid/webapp/common/authn/fido_tokens.py index 84ecb0d51..b768ac92a 100644 --- a/src/eduid/webapp/common/authn/fido_tokens.py +++ b/src/eduid/webapp/common/authn/fido_tokens.py @@ -79,7 +79,7 @@ def _get_fido2server(credentials: dict[ElementKey, FidoCred], fido2rp: PublicKey # (assume all app-ids are the same - authenticating with a mix of different # app-ids isn't supported in current Webauthn) app_id = None - for _k, v in credentials.items(): + for v in credentials.values(): if v.app_id: app_id = v.app_id break diff --git a/src/eduid/webapp/common/authn/utils.py b/src/eduid/webapp/common/authn/utils.py index 38f28bc93..beb20c15f 100644 --- a/src/eduid/webapp/common/authn/utils.py +++ b/src/eduid/webapp/common/authn/utils.py @@ -71,7 +71,7 @@ def get_saml_attribute(session_info: SessionInfo, attr_name: str) -> list[str] | logger.debug(f"SAML attributes received: {attributes}") # Look for the canonicalized attribute in the SAML assertion attributes - for saml_attr, _ in attributes.items(): + for saml_attr in attributes.keys(): if saml_attr.lower() == attr_name.lower(): return attributes[saml_attr] return None From 9e1681604f1f390cc5fa85402e01f5ce773aee69 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Wed, 18 Sep 2024 11:01:34 +0000 Subject: [PATCH 3/7] add first group of PERF checks --- ruff.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruff.toml b/ruff.toml index 9ea698db6..eff3a05ee 100644 --- a/ruff.toml +++ b/ruff.toml @@ -3,4 +3,4 @@ line-length = 120 target-version = "py310" [lint] -select = ["E4", "E7", "E9", "F", "W", "I", "ASYNC", "UP", "FLY"] +select = ["E4", "E7", "E9", "F", "W", "I", "ASYNC", "UP", "FLY", "PERF1"] From 84373ef6cb154fdd35b08fa9c4ad6cc934d9ceb0 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Wed, 18 Sep 2024 11:04:06 +0000 Subject: [PATCH 4/7] use log2 as it is usually more accurate --- ruff.toml | 2 +- src/eduid/webapp/common/api/validation.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ruff.toml b/ruff.toml index eff3a05ee..4494b82cf 100644 --- a/ruff.toml +++ b/ruff.toml @@ -3,4 +3,4 @@ line-length = 120 target-version = "py310" [lint] -select = ["E4", "E7", "E9", "F", "W", "I", "ASYNC", "UP", "FLY", "PERF1"] +select = ["E4", "E7", "E9", "F", "W", "I", "ASYNC", "UP", "FLY", "PERF1", "FURB"] diff --git a/src/eduid/webapp/common/api/validation.py b/src/eduid/webapp/common/api/validation.py index 3df2cb2d2..ca5a7816f 100644 --- a/src/eduid/webapp/common/api/validation.py +++ b/src/eduid/webapp/common/api/validation.py @@ -58,7 +58,7 @@ def is_valid_password(password: str, user_info: Sequence[str], min_entropy: int, # Check password complexity with zxcvbn result = zxcvbn(password, user_inputs=user_info) _guesses = result.get("guesses", 1) - _pw_entropy = math.log(_guesses, 2) + _pw_entropy = math.log2(_guesses) if _pw_entropy < min_entropy: raise ValueError("The password complexity is too weak.") # This is the SWAMID requirement for zxcvbn since 2021: From 3ac16b885dd48e97ea7cb17d6bdb239e76c948db Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Wed, 18 Sep 2024 11:25:47 +0000 Subject: [PATCH 5/7] consolidate E rules and skip E501 only --- ruff.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ruff.toml b/ruff.toml index 4494b82cf..08df0e8b9 100644 --- a/ruff.toml +++ b/ruff.toml @@ -3,4 +3,7 @@ line-length = 120 target-version = "py310" [lint] -select = ["E4", "E7", "E9", "F", "W", "I", "ASYNC", "UP", "FLY", "PERF1", "FURB"] +select = ["E", "F", "W", "I", "ASYNC", "UP", "FLY", "PERF1", "FURB"] + +# For now, ignore E501 (line too long) errors. They are in strings and comments. +ignore = ["E501"] From 90ffa40329120bedb1fd9815f86c677cc923b735 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Wed, 18 Sep 2024 12:25:16 +0000 Subject: [PATCH 6/7] skip 2 perf rules for now --- ruff.toml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ruff.toml b/ruff.toml index 08df0e8b9..ae0c39c41 100644 --- a/ruff.toml +++ b/ruff.toml @@ -3,7 +3,9 @@ line-length = 120 target-version = "py310" [lint] -select = ["E", "F", "W", "I", "ASYNC", "UP", "FLY", "PERF1", "FURB"] +select = ["E", "F", "W", "I", "ASYNC", "UP", "FLY", "PERF", "FURB"] # For now, ignore E501 (line too long) errors. They are in strings and comments. -ignore = ["E501"] +# For now, ignore PERF203 (try-except-in-loop), TODO: fix them. +# For now, ignore PERF401 (manual-list-comprehension), TODO: fix them. +ignore = ["E501", "PERF203", "PERF401"] From 597119a2fc80147c51ae7e33f84c9b8b235a4979 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Wed, 18 Sep 2024 14:00:47 +0000 Subject: [PATCH 7/7] re-add list comprehension rule and patches --- ruff.toml | 3 +-- src/eduid/graphdb/groupdb/db.py | 12 ++++++++---- src/eduid/scimapi/routers/groups.py | 9 +++------ src/eduid/scimapi/test-scripts/scim-util.py | 9 ++++----- src/eduid/userdb/group_management/db.py | 11 +++++------ src/eduid/webapp/common/api/helpers.py | 4 +--- src/eduid/webapp/common/api/utils.py | 3 +-- src/eduid/webapp/security/helpers.py | 7 +++---- .../lookup_mobile/client/mobile_lookup_client.py | 4 +--- 9 files changed, 27 insertions(+), 35 deletions(-) diff --git a/ruff.toml b/ruff.toml index ae0c39c41..518e55bb2 100644 --- a/ruff.toml +++ b/ruff.toml @@ -7,5 +7,4 @@ select = ["E", "F", "W", "I", "ASYNC", "UP", "FLY", "PERF", "FURB"] # For now, ignore E501 (line too long) errors. They are in strings and comments. # For now, ignore PERF203 (try-except-in-loop), TODO: fix them. -# For now, ignore PERF401 (manual-list-comprehension), TODO: fix them. -ignore = ["E501", "PERF203", "PERF401"] +ignore = ["E501", "PERF203"] diff --git a/src/eduid/graphdb/groupdb/db.py b/src/eduid/graphdb/groupdb/db.py index 938d5d3d8..5d754c63d 100644 --- a/src/eduid/graphdb/groupdb/db.py +++ b/src/eduid/graphdb/groupdb/db.py @@ -284,8 +284,10 @@ def get_groups_by_property(self, key: str, value: str, skip=0, limit=100): RETURN g as group SKIP $skip LIMIT $limit """ with self.db.driver.session(default_access_mode=READ_ACCESS) as session: - for record in session.run(q, scope=self.scope, value=value, skip=skip, limit=limit): - res.append(self._load_group(record.data()["group"])) + res = [ + self._load_group(record.data()["group"]) + for record in session.run(q, scope=self.scope, value=value, skip=skip, limit=limit) + ] return res def get_groups(self, skip: int = 0, limit: int = 100) -> list[Group]: @@ -295,8 +297,10 @@ def get_groups(self, skip: int = 0, limit: int = 100) -> list[Group]: RETURN g as group SKIP $skip LIMIT $limit """ with self.db.driver.session(default_access_mode=READ_ACCESS) as session: - for record in session.run(q, scope=self.scope, skip=skip, limit=limit): - res.append(self._load_group(record.data()["group"])) + res = [ + self._load_group(record.data()["group"]) + for record in session.run(q, scope=self.scope, skip=skip, limit=limit) + ] return res def _get_groups_for_role(self, label: Label, identifier: str, role: Role): diff --git a/src/eduid/scimapi/routers/groups.py b/src/eduid/scimapi/routers/groups.py index a549b27ee..3bd96a365 100644 --- a/src/eduid/scimapi/routers/groups.py +++ b/src/eduid/scimapi/routers/groups.py @@ -30,9 +30,7 @@ @groups_router.get("/", response_model=ListResponse) async def on_get_all(req: ContextRequest) -> ListResponse: db_groups = req.context.groupdb.get_groups() - resources = [] - for db_group in db_groups: - resources.append({"id": str(db_group.scim_id), "displayName": db_group.graph.display_name}) + resources = [{"id": str(db_group.scim_id), "displayName": db_group.graph.display_name} for db_group in db_groups] return ListResponse(total_results=len(db_groups), resources=resources) @@ -302,7 +300,6 @@ async def search(req: ContextRequest, query: SearchRequest) -> ListResponse: else: raise BadRequest(scim_type="invalidFilter", detail=f"Can't filter on attribute {_filter.attr}") - resources = [] - for this in groups: - resources.append({"id": str(this.scim_id), "displayName": this.display_name}) + resources = [{"id": str(this.scim_id), "displayName": this.display_name} for this in groups] + return ListResponse(total_results=total_count, resources=resources) diff --git a/src/eduid/scimapi/test-scripts/scim-util.py b/src/eduid/scimapi/test-scripts/scim-util.py index f1df093fe..2e38e24f4 100755 --- a/src/eduid/scimapi/test-scripts/scim-util.py +++ b/src/eduid/scimapi/test-scripts/scim-util.py @@ -194,11 +194,10 @@ def put_group(api: Api, scim_id: str, data: dict[str, Any], token: str | None = if display_name: scim["displayName"] = display_name if members: - new_members = [] - for member in members: - new_members.append( - {"$ref": f'{api}/Users/{member["id"]}', "value": member["id"], "display": member["display_name"]} - ) + new_members = [ + {"$ref": f'{api}/Users/{member["id"]}', "value": member["id"], "display": member["display_name"]} + for member in members + ] scim["members"] = new_members if "data" in data: if NUTID_GROUP_V1 not in scim["schemas"]: diff --git a/src/eduid/userdb/group_management/db.py b/src/eduid/userdb/group_management/db.py index c8889cce5..7e3ecc51e 100644 --- a/src/eduid/userdb/group_management/db.py +++ b/src/eduid/userdb/group_management/db.py @@ -60,12 +60,11 @@ def get_states_by_email_addresses(self, email_addresses: list[str]) -> list[Grou :raise self.DocumentDoesNotExist: No document match the search criteria """ - states: list[GroupInviteState] = [] - for email_address in email_addresses: - spec = {"email_address": email_address} - for this in self._get_documents_by_filter(spec): - states.append(GroupInviteState.from_dict(this)) - + states: list[GroupInviteState] = [ + GroupInviteState.from_dict(state) + for email_address in email_addresses + for state in self._get_documents_by_filter({"email_address": email_address}) + ] return states def save(self, state: GroupInviteState, is_in_database: bool) -> SaveResult: diff --git a/src/eduid/webapp/common/api/helpers.py b/src/eduid/webapp/common/api/helpers.py index 97f853830..15f184539 100644 --- a/src/eduid/webapp/common/api/helpers.py +++ b/src/eduid/webapp/common/api/helpers.py @@ -63,9 +63,7 @@ def get_marked_given_name(given_name: str, given_name_marking: str | None) -> st else: _given_names.append(name) - _optional_marked_names: list[str | None] = [] - for i in given_name_marking: - _optional_marked_names.append(_given_names[int(i)]) + _optional_marked_names: list[str | None] = [_given_names[int(i)] for i in given_name_marking] # remove None values # i.e. 0 index and hyphenated names second part placeholder _marked_names: list[str] = [name for name in _optional_marked_names if name is not None] diff --git a/src/eduid/webapp/common/api/utils.py b/src/eduid/webapp/common/api/utils.py index a836935af..6e1d7a6e4 100644 --- a/src/eduid/webapp/common/api/utils.py +++ b/src/eduid/webapp/common/api/utils.py @@ -271,8 +271,7 @@ def get_zxcvbn_terms(user: User) -> list[str]: # Mail addresses if user.mail_addresses.count: - for item in user.mail_addresses.to_list(): - user_input.append(item.email.split("@")[0]) + user_input.extend(item.email.split("@")[0] for item in user.mail_addresses.to_list()) return user_input diff --git a/src/eduid/webapp/security/helpers.py b/src/eduid/webapp/security/helpers.py index eeaa7a88f..5782acdd7 100644 --- a/src/eduid/webapp/security/helpers.py +++ b/src/eduid/webapp/security/helpers.py @@ -278,10 +278,9 @@ def get_approved_security_keys() -> dict[str, Any]: ) parsed_entries.append(authenticator_info) - approved_keys_list: list[str] = [] - for entry in parsed_entries: - if entry.description and is_authenticator_mfa_approved(entry): - approved_keys_list.append(entry.description) + approved_keys_list: list[str] = [ + entry.description for entry in parsed_entries if entry.description and is_authenticator_mfa_approved(entry) + ] # remove case-insensitive duplicates from list, while maintaining the original case marker = set() diff --git a/src/eduid/workers/lookup_mobile/client/mobile_lookup_client.py b/src/eduid/workers/lookup_mobile/client/mobile_lookup_client.py index 8c839b2be..2f1300241 100644 --- a/src/eduid/workers/lookup_mobile/client/mobile_lookup_client.py +++ b/src/eduid/workers/lookup_mobile/client/mobile_lookup_client.py @@ -95,9 +95,7 @@ def _search_by_SSNo(self, national_identity_number: str) -> list[str]: if record is None: return [] - mobile_numbers = [] - for r in record: - mobile_numbers.append(r.Mobiles) + mobile_numbers = [r.Mobiles for r in record] return mobile_numbers