Merge pull request #68 from IATI/sk-error-handling--count-bug--user-reporting-orgs-endpoint

Add `/users/{id}/reporting-orgs' endpoint, improve handling of unknown clients, fix dataset count bug

Author: simon-20

CHANGELOG.md

@@ -19,6 +19,39 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Security
 
+## [0.3.0]
+
+### Added
+
+ - Added the /users/{id}/reporting-orgs endpoint which allows lookup of a user's
+   orgs by user id.
+
+### Fixed
+
+ - Updated short name validation so that it rejects values with uppercase
+   characters.
+ - Improves error handling and logging for when clients connect with a
+   misconfigured client ID.
+
+## [0.2.9]
+
+### Added
+
+ - Emails notifications to Secretariat admins when a user creates a new
+   organisation and to organisation admins when a user requests to join their
+   organisation.
+
+## [0.2.8]
+
+### Added
+
+ - Improved and adding more audit logging.
+
+### Fixed
+
+ - Fixed the validation on dataset short_name which wasn't checking whether the
+   record in the CRM was deleted.
+
 ## [0.2.7]
 
 ### Fixed

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "register-your-data-api"
-version = "0.2.9"
+version = "0.3.0"
 requires-python = ">= 3.12.11"
 readme = "README.md"
 authors = [{name="IATI Secretariat", email="support@iatistandard.org"}]

src/register_your_data_api/auth/authz.py

@@ -18,16 +18,14 @@ async def get_user_authnz(
     if user.sub is None:
         raise RuntimeError  # TODO: handle in proper way
 
-    # Read the user's details from Asgardeo to get their SuiteCRM UUID
-    # user.user_id_crm = context._crm_uuid_provider.get_crm_uuid(user)
+    current_user_id = UUID(user.user_id_crm)
 
-    # Read the user's fine grained authorisations
-    users_fgas = context.fine_grained_auth_provider.get_user_fine_grained_permissions(UUID(user.user_id_crm))
+    users_fgas = context.fine_grained_auth_provider.get_user_fine_grained_permissions(current_user_id)
 
-    is_superadmin = context.fine_grained_auth_provider.is_user_a_superadmin(UUID(user.user_id_crm))
+    is_superadmin = context.fine_grained_auth_provider.is_user_a_superadmin(current_user_id)
 
     user.fga_user_validator = FineGrainedAuthorisationUserValidator(
-        fine_grained_authorisations=users_fgas, is_superadmin=is_superadmin
+        user_id=current_user_id, fine_grained_authorisations=users_fgas, is_superadmin=is_superadmin
     )
 
     return user

src/register_your_data_api/auth/fga/fga_validator.py

@@ -7,6 +7,8 @@
 
 class FineGrainedAuthorisationUserValidator(pydantic.BaseModel):
 
+    user_id: UUID
+
     fine_grained_authorisations: list[FineGrainedAuthorisationRoleAssociation] | None
 
     is_superadmin: bool
@@ -172,6 +174,12 @@ def user_can_modify_user_roles_for_reporting_org(self, reporting_org_id: UUID) -
 
         return False
 
+    def user_can_read_users_reporting_orgs(self, user_id: UUID) -> bool:
+        if self.is_superadmin:
+            return True
+
+        return self.user_id == user_id
+
     def get_users_fine_grained_associations(self) -> list[FineGrainedAuthorisationRoleAssociation]:
         if self.fine_grained_authorisations is None:
             return []

src/register_your_data_api/client_application_details_provider.py

@@ -54,8 +54,6 @@ def get_client_application_details(self, client_id: str) -> ClientApplicationDet
 
         if client_id not in self._CLIENT_APPLICATION_DETAILS:
             error_message = f"Unknown client application. Client id: {client_id} is not found in the list of clients"
-            self._app_logger.error(error_message)
-            self._audit_logger.error(error_message)
             raise ValueError(error_message)
 
         return self._CLIENT_APPLICATION_DETAILS[client_id]

src/register_your_data_api/data_handling/data_schemas.py

@@ -4,12 +4,12 @@
 
 import pydantic
 
-alpha_numeric_hyphen_regex = re.compile(r"^[a-zA-Z0-9-_]+$")
+alpha_lowercase_numeric_hyphen_regex = re.compile(r"^[a-z0-9-_]+$")
 
 
-def validate_alpha_numeric_hyphen_str(value: str) -> str:
-    if not alpha_numeric_hyphen_regex.match(value):
-        raise ValueError("String must contain only alphanumeric characters, hyphens, or underscores")
+def validate_alpha_numeric_hyphen_lowercase_str(value: str) -> str:
+    if not alpha_lowercase_numeric_hyphen_regex.match(value):
+        raise ValueError("String must contain only lowercase alphanumeric characters, hyphens, or underscores")
     return value
 
 
@@ -62,7 +62,7 @@ class DatasetCreateModel(pydantic.BaseModel):
     human_readable_name: str
     licence_id: str
     owner_organisation_id: str
-    short_name: Annotated[str, pydantic.AfterValidator(validate_alpha_numeric_hyphen_str)]
+    short_name: Annotated[str, pydantic.AfterValidator(validate_alpha_numeric_hyphen_lowercase_str)]
     source_type: str
     url: str
     visibility: str
@@ -71,7 +71,7 @@ class DatasetCreateModel(pydantic.BaseModel):
 class DatasetUpdateModel(pydantic.BaseModel):
     human_readable_name: str
     licence_id: str
-    short_name: Annotated[str, pydantic.AfterValidator(validate_alpha_numeric_hyphen_str)]
+    short_name: Annotated[str, pydantic.AfterValidator(validate_alpha_numeric_hyphen_lowercase_str)]
     source_type: str
     url: str
     visibility: Literal["public", "private"] | None = None
@@ -124,7 +124,7 @@ class ReportingOrgUserCreateModel(pydantic.BaseModel):
     phone: str | None
     region: str | None
     reporting_source_type: str | None
-    short_name: Annotated[str, pydantic.AfterValidator(validate_alpha_numeric_hyphen_str)]
+    short_name: Annotated[str, pydantic.AfterValidator(validate_alpha_numeric_hyphen_lowercase_str)]
     website: str | None
 
 

src/register_your_data_api/dependencies.py

@@ -1,7 +1,10 @@
+import uuid
+
 import starlette.requests
 from fastapi import Security
 
 from register_your_data_api.auth import authz
+from register_your_data_api.exceptions import RYDUserException
 from register_your_data_api.util import Context
 
 
@@ -13,7 +16,20 @@ def get_suitecrm_audit_headers(
 
     context: Context = request.app.state.context
 
-    client_app_details = context.get_client_application_details(user.client_id)
+    try:
+        client_app_details = context.get_client_application_details(user.client_id)
+    except ValueError as e:
+        trace_id = uuid.uuid4()
+        raise RYDUserException(
+            user,
+            400,
+            f"trace_id: {trace_id} - details: unknown client ID",
+            f"trace_id: {trace_id} - details: {e.args[0]}",
+            (
+                "There was a problem processing your request (invalid client ID). "
+                f"Please contact IATI Support quoting trace_id: {trace_id}"
+            ),
+        ) from e
 
     return {
         "IATI-Person-ID": user.user_id_crm,

src/register_your_data_api/routers/reporting_orgs.py

@@ -8,12 +8,13 @@
 from fastapi import BackgroundTasks, Depends, Security
 from fastapi.exceptions import HTTPException
 from fastapi.responses import JSONResponse
-from libsuitecrm import Filter, RequestFailed, SuiteCRM  # type: ignore
+from libsuitecrm import Filter, SuiteCRM  # type: ignore
 
 from register_your_data_api import email_generator
 from register_your_data_api.background_tasks import ActionType, enqueue_task
 from register_your_data_api.dependencies import get_suitecrm_audit_headers
 from register_your_data_api.exceptions import RYDUserException
+from register_your_data_api.services.suitecrm_common import get_reporting_orgs_for_user
 
 from ..auth import authz
 from ..auth import models as auth_models
@@ -22,7 +23,6 @@
     SUITECRM_REPORTING_ORG_FIELDS,
     get_dataset_actions_from_suitecrm_response,
     get_dataset_list_from_suitecrm_response,
-    get_discoverable_reporting_org_meta_from_suitecrm_response,
     get_fga_role_as_str,
     get_reporting_org_meta_from_suitecrm_response,
     get_suitecrm_dict_from_reporting_org,
@@ -31,11 +31,9 @@
     CRMUser,
     CRMUserListResponse,
     DatasetReadModel,
-    DiscoverableReportingOrgMetadata,
     PaginationQueryParams,
     ReportingOrgAction,
     ReportingOrgCreateModel,
-    ReportingOrgMetadata,
     ReportingOrgUpdateModel,
     ReportingOrgUserCreateModel,
     UserReportingOrgDiscoverableMetadataRelation,
@@ -60,74 +58,11 @@ def get_reporting_orgs(
 
     context: Context = request.app.state.context
 
-    crm: SuiteCRM = context.suitecrm_client_factory.get_client()
-
-    try:
-        orgs_for_user = crm.get_relationship(
-            "Contacts",
-            user.user_id_crm,
-            "Accounts",
-            page_number=paging.page,
-            page_size=paging.page_size,
-            sort_field="name",
-            sort_dir="ascending",
-            filters=Filter().equal("iati_registry_discoverable", "1"),
-        )
-    except RequestFailed as e:
-        error_id = uuid.uuid4()
-        public_error_message = (
-            "There was a problem fetching the list of reporting orgs you are associated with. "
-            f"Please try again later, or contact IATI Support quoting error id: {error_id}"
-        )
-        context.app_logger.error(
-            f"error id: {error_id} - user id: {user.user_id_crm} - GET /reporting-orgs - Problem when fetching "
-            "the list of reporting organisations for this user from SuiteCRM. "
-            f"Details: {str(e)}"
-        )
-        raise HTTPException(status_code=fastapi.status.HTTP_500_INTERNAL_SERVER_ERROR, detail=public_error_message)
-
-    total_records = orgs_for_user.get("meta", {}).get("total-records", 0)
-
-    reporting_orgs_list: list[UserReportingOrgRelation | UserReportingOrgDiscoverableMetadataRelation] = []
-
-    for reporting_org_from_suitecrm in orgs_for_user["data"]:
-        role_for_org = user.validator.get_user_role_for_reporting_org(reporting_org_from_suitecrm["id"])
-
-        if role_for_org is None:
-            context.app_logger.info(
-                f"user id: {user.user_id_crm} - GET /reporting-orgs - user is associated with organisation "
-                f"{reporting_org_from_suitecrm["id"]} in the CRM but has no role for that organisation in the FGA DB. "
-                "Organisation was omitted from the list returned to the user."
-            )
-            continue
-
-        reporting_org_obj: ReportingOrgMetadata | DiscoverableReportingOrgMetadata
-
-        if role_for_org == fga_models.FineGrainedAuthorisationRole.CONTRIBUTOR_PENDING:
-            reporting_org_obj = get_discoverable_reporting_org_meta_from_suitecrm_response(
-                reporting_org_from_suitecrm["attributes"]
-            )
-        else:
-            reporting_org_obj = get_reporting_org_meta_from_suitecrm_response(
-                reporting_org_from_suitecrm["attributes"]
-            )
-
-        reporting_orgs_list.append(
-            UserReportingOrgRelation(
-                id=reporting_org_from_suitecrm["id"],
-                user_role=get_fga_role_as_str(role_for_org),
-                metadata=reporting_org_obj,
-                reporting_org_actions=(
-                    get_reporting_org_actions(crm, reporting_org_from_suitecrm["id"])
-                    if include_actions == "yes"
-                    else []
-                ),
-            )
-        )
-
-    reporting_orgs_list.sort(key=lambda org: org.metadata.human_readable_name.lower())
+    total_records, reporting_orgs = get_reporting_orgs_for_user(
+        context, user, uuid.UUID(user.user_id_crm), paging.page, paging.page_size
+    )
 
-    return PaginatedResultsPage.create(reporting_orgs_list, paging.page, paging.page_size, total_records, request)
+    return PaginatedResultsPage.create(reporting_orgs, paging.page, paging.page_size, total_records, request)
 
 
 @router.get("/{org_id}")
@@ -648,7 +583,7 @@ def get_reporting_org_datasets(
 
     # 4. SuiteCRM doesn't tell us the total number of records, so we set page size = 1 and make a request
     total_records_resp = crm.get_records("IATI_Datasets", filters=filters, fields=["id"], page_number=1, page_size=1)
-    total_records = total_records_resp.get("meta", {}).get("total-pages", 1)
+    total_records = total_records_resp.get("meta", {}).get("total-pages", 0)
 
     datasets = get_dataset_list_from_suitecrm_response(datasets_from_suitecrm)
 

src/register_your_data_api/routers/users.py

@@ -13,13 +13,18 @@
 from register_your_data_api import email_generator
 from register_your_data_api.background_tasks import ActionType, enqueue_task
 from register_your_data_api.dependencies import get_suitecrm_audit_headers
+from register_your_data_api.response_schemas import PaginatedResultsPage
+from register_your_data_api.services.suitecrm_common import get_reporting_orgs_for_user
 
 from ..auth import authz
 from ..auth.fga import models as fga_models
 from ..auth.models import UserAndCredentials
 from ..data_handling.converters import get_fga_role_from_str
 from ..data_handling.data_schemas import (
     OrganisationId,
+    PaginationQueryParams,
+    UserReportingOrgDiscoverableMetadataRelation,
+    UserReportingOrgRelation,
     UserRoleUpdateModel,
 )
 from ..exception_handlers import format_log_msg
@@ -221,6 +226,52 @@ def add_user_to_reporting_org(
     return JSONResponse({"data": None, "error": None, "status": "success"}, 200)
 
 
+@router.get("/{user_id}/reporting-orgs")
+def get_reporting_orgs(
+    user_id: uuid.UUID,
+    request: starlette.requests.Request,
+    user: UserAndCredentials = Security(authz.get_user_authnz, scopes=["ryd", "ryd:reporting_org"]),
+    paging: PaginationQueryParams = fastapi.Depends(),
+) -> PaginatedResultsPage[UserReportingOrgRelation | UserReportingOrgDiscoverableMetadataRelation]:
+
+    context: Context = request.app.state.context
+
+    crm: SuiteCRM = context.suitecrm_client_factory.get_client()
+
+    # 1. Check that the requested user exists in CRM
+    assert_precondition_met(
+        context,
+        user,
+        condition_func=lambda: check_crm_record_exists(crm, "Contacts", str(user_id)),
+        public_msg=f"There is no user with ID {str(user_id)}.",
+        status_code=fastapi.status.HTTP_404_NOT_FOUND,
+        audit_log_msg=(
+            f"user id: {user.user_id_crm} - GET /users/{user_id}reporting-orgs - Request to read reporting orgs for "
+            f"user {user_id} but there is no such user in the CRM."
+        ),
+    )
+
+    # 2. Check that the user has permission to the the requested user's reporting orgs
+    assert_precondition_met(
+        context,
+        user,
+        condition_func=lambda: user.validator.user_can_read_users_reporting_orgs(user_id),
+        status_code=fastapi.status.HTTP_403_FORBIDDEN,
+        public_msg=(
+            "There is a problem with your credentials. If this persists please report this "
+            "error to the provider of the tool you are using to access the IATI Register Your Data."
+        ),
+        audit_log_msg=(
+            f"user id: {user.user_id_crm} - GET /users/{user_id}reporting-orgs - Request to read reporting orgs for "
+            f"user {user_id} by unauthorised user."
+        ),
+    )
+
+    total_records, reporting_orgs = get_reporting_orgs_for_user(context, user, user_id, paging.page, paging.page_size)
+
+    return PaginatedResultsPage.create(reporting_orgs, paging.page, paging.page_size, total_records, request)
+
+
 @router.put("/{user_id}/reporting-org/{org_id}")
 def update_user_role_in_reporting_org(
     user_id: uuid.UUID,