From d7390f00b0d0bd1b1ad28977e49b0b99a7fbb7fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mark=20Lis=C3=A9?= Date: Mon, 21 Oct 2024 17:30:42 -0700 Subject: [PATCH 1/5] TRACK-175: Fix grabbing permissions from KC. --- .../schemas/response/user_group_response.py | 26 +++++++-- epictrack-api/src/api/services/keycloak.py | 14 +++++ epictrack-api/src/api/services/user.py | 55 ++++++++++++++----- 3 files changed, 78 insertions(+), 17 deletions(-) diff --git a/epictrack-api/src/api/schemas/response/user_group_response.py b/epictrack-api/src/api/schemas/response/user_group_response.py index 1d670f095..8d830fcfa 100644 --- a/epictrack-api/src/api/schemas/response/user_group_response.py +++ b/epictrack-api/src/api/schemas/response/user_group_response.py @@ -26,12 +26,30 @@ class UserGroupResponseSchema(Schema): display_name = fields.Method("get_display_name") def get_level(self, instance): - """Get the full name""" - return int(instance["attributes"]["level"][0]) + """ + Retrieve the level attribute from the given instance. + + Args: + instance (dict): A dictionary representing the instance, which is expected to have an "attributes" key. + + Returns: + int: The level value extracted from the instance's attributes. Defaults to 0 if not found. + """ + return int(instance.get("attributes", {}).get("level", [0])[0] or 0) def get_display_name(self, instance): - """Get the display name of the group""" - return instance["attributes"]["display_name"][0] + """ + Retrieve the display name of the group from the given instance. + + Args: + instance (dict): A dictionary representing the group instance, + which should contain an "attributes" key. + + Returns: + str: The display name of the group. If the display name is not + found, an empty string is returned. + """ + return instance.get("attributes", {}).get("display_name", [""])[0] or "" def get_path(self, instance): """Format the path of the group from keycloak""" diff --git a/epictrack-api/src/api/services/keycloak.py b/epictrack-api/src/api/services/keycloak.py index 1baae99a6..687eb1658 100644 --- a/epictrack-api/src/api/services/keycloak.py +++ b/epictrack-api/src/api/services/keycloak.py @@ -26,6 +26,20 @@ def get_groups(brief_representation: bool = False): response = KeycloakService._request_keycloak(f'groups?briefRepresentation={brief_representation}') return response.json() + @staticmethod + def get_sub_groups(group_id): + """ + Return the subgroups of a given group. + + Args: + group_id (str): The ID of the group for which to retrieve subgroups. + + Returns: + list: A list of subgroups for the given group. + """ + response = KeycloakService._request_keycloak(f"groups/{group_id}/children") + return response.json() + @staticmethod def get_users(): """Get users""" diff --git a/epictrack-api/src/api/services/user.py b/epictrack-api/src/api/services/user.py index 1a13a8d34..8e85aea32 100644 --- a/epictrack-api/src/api/services/user.py +++ b/epictrack-api/src/api/services/user.py @@ -16,7 +16,7 @@ from api.utils import TokenInfo from .keycloak import KeycloakService - +from flask import current_app class UserService: """User Service""" @@ -39,23 +39,37 @@ def get_all_users(cls): @classmethod def get_groups(cls): - """Get groups that has "level" attribute set up""" + """ + Retrieve groups that have the "level" attribute set up. + + This method fetches all groups from the Keycloak service and filters them + to include only those groups that have sub-groups. It logs the groups and + sub-groups at various stages for debugging purposes. + + Returns: + list: A list of filtered groups that have sub-groups. + """ + # Fetch all groups from the Keycloak service groups = KeycloakService.get_groups() + current_app.logger.debug(f"Groups: {groups}") filtered_groups = [] + for group in groups: - if group.get("subGroups"): - filtered_groups = filtered_groups + [ - sub_group - for sub_group in group.get("subGroups") - if "level" in sub_group["attributes"] - ] - elif "level" in group["attributes"]: - filtered_groups.append(group) + current_app.logger.info(f"group: {group}") + + # Check if the group has sub-groups by looking at the "subGroupCount" attribute + if group.get("subGroupCount", 0) > 0: + + # Fetch the sub-groups for the current group + sub_groups = KeycloakService.get_sub_groups(group["id"]) + current_app.logger.debug(f"sub_groups: {sub_groups}") + filtered_groups.extend(sub_groups) + + current_app.logger.debug(f"filtered_groups: {filtered_groups}") return filtered_groups @classmethod def update_user_group(cls, user_id, user_group_request): - """Update the group of a user""" token_groups = TokenInfo.get_user_data()["groups"] groups = cls.get_groups() requesters_group = next( @@ -118,5 +132,20 @@ def _delete_from_all_epictrack_subgroups(user_id): @classmethod def _get_level(cls, group): - """Gets the level from the group""" - return group["attributes"]["level"][0] + """ + Retrieves the level from the given group. + + Args: + group (dict): A dictionary representing the group, which should contain + an "attributes" key with a nested "level" key. + + Returns: + int: The level extracted from the group. If the level is not found or + cannot be converted to an integer, returns 0. + """ + level_str = group["attributes"].get("level", [0])[0] + try: + return int(level_str) + except (KeyError, IndexError, TypeError) as e: + current_app.logger.error(f"Error getting level from group: {e}. Returning 0.") + return 0 From 26ab6ac90baf901c4fdfc91842c250491c65246c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mark=20Lis=C3=A9?= Date: Tue, 22 Oct 2024 09:45:48 -0700 Subject: [PATCH 2/5] Fix linting. --- epictrack-api/src/api/services/user.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/epictrack-api/src/api/services/user.py b/epictrack-api/src/api/services/user.py index 8e85aea32..c739e3db6 100644 --- a/epictrack-api/src/api/services/user.py +++ b/epictrack-api/src/api/services/user.py @@ -15,8 +15,8 @@ from api.exceptions import BusinessError, PermissionDeniedError from api.utils import TokenInfo -from .keycloak import KeycloakService from flask import current_app +from .keycloak import KeycloakService class UserService: """User Service""" @@ -60,16 +60,29 @@ def get_groups(cls): # Check if the group has sub-groups by looking at the "subGroupCount" attribute if group.get("subGroupCount", 0) > 0: - # Fetch the sub-groups for the current group - sub_groups = KeycloakService.get_sub_groups(group["id"]) - current_app.logger.debug(f"sub_groups: {sub_groups}") - filtered_groups.extend(sub_groups) + # Fetch the sub-groups for the current group + sub_groups = KeycloakService.get_sub_groups(group["id"]) + current_app.logger.debug(f"sub_groups: {sub_groups}") + filtered_groups.extend(sub_groups) current_app.logger.debug(f"filtered_groups: {filtered_groups}") return filtered_groups @classmethod def update_user_group(cls, user_id, user_group_request): + """ + Updates the user's group based on the provided user group request. + Args: + cls: The class instance. + user_id (str): The ID of the user to update. + user_group_request (dict): A dictionary containing the group update request details. + Expected keys: + - "group_id_to_update" (str): The ID of the group to update. + Raises: + PermissionDeniedError: If the requester does not have permission to update the group. + Returns: + dict: The result of the group update operation from KeycloakService. + """ token_groups = TokenInfo.get_user_data()["groups"] groups = cls.get_groups() requesters_group = next( From a576319d43a3ed1ef7991ee2f300e82d19a15ef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mark=20Lis=C3=A9?= Date: Tue, 22 Oct 2024 09:48:44 -0700 Subject: [PATCH 3/5] Fix linting. --- epictrack-api/src/api/services/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/epictrack-api/src/api/services/user.py b/epictrack-api/src/api/services/user.py index c739e3db6..86cb28a26 100644 --- a/epictrack-api/src/api/services/user.py +++ b/epictrack-api/src/api/services/user.py @@ -12,10 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. """User service""" +from flask import current_app from api.exceptions import BusinessError, PermissionDeniedError from api.utils import TokenInfo -from flask import current_app from .keycloak import KeycloakService class UserService: From 899cc8e6dd5ade5bad158ef4cd84aded9dfaca75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mark=20Lis=C3=A9?= Date: Tue, 22 Oct 2024 10:19:55 -0700 Subject: [PATCH 4/5] Fix linting. --- epictrack-api/src/api/services/user.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/epictrack-api/src/api/services/user.py b/epictrack-api/src/api/services/user.py index 86cb28a26..99181baf5 100644 --- a/epictrack-api/src/api/services/user.py +++ b/epictrack-api/src/api/services/user.py @@ -18,6 +18,7 @@ from .keycloak import KeycloakService + class UserService: """User Service""" @@ -71,6 +72,7 @@ def get_groups(cls): @classmethod def update_user_group(cls, user_id, user_group_request): """ + Updates the user's group based on the provided user group request. Args: cls: The class instance. From cb27a163def90944522040fce05e7e2a1b81c64d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mark=20Lis=C3=A9?= Date: Tue, 22 Oct 2024 10:23:46 -0700 Subject: [PATCH 5/5] Fix linting. --- epictrack-api/src/api/schemas/response/user_group_response.py | 4 ++-- epictrack-api/src/api/services/user.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/epictrack-api/src/api/schemas/response/user_group_response.py b/epictrack-api/src/api/schemas/response/user_group_response.py index 8d830fcfa..c643b7889 100644 --- a/epictrack-api/src/api/schemas/response/user_group_response.py +++ b/epictrack-api/src/api/schemas/response/user_group_response.py @@ -42,11 +42,11 @@ def get_display_name(self, instance): Retrieve the display name of the group from the given instance. Args: - instance (dict): A dictionary representing the group instance, + instance (dict): A dictionary representing the group instance, which should contain an "attributes" key. Returns: - str: The display name of the group. If the display name is not + str: The display name of the group. If the display name is not found, an empty string is returned. """ return instance.get("attributes", {}).get("display_name", [""])[0] or "" diff --git a/epictrack-api/src/api/services/user.py b/epictrack-api/src/api/services/user.py index 99181baf5..232bd3afa 100644 --- a/epictrack-api/src/api/services/user.py +++ b/epictrack-api/src/api/services/user.py @@ -72,8 +72,8 @@ def get_groups(cls): @classmethod def update_user_group(cls, user_id, user_group_request): """ - Updates the user's group based on the provided user group request. + Args: cls: The class instance. user_id (str): The ID of the user to update.