fix credential check for pki systems (#1563)

Author: rstijerina

designsafe/apps/api/datafiles/handlers.py

@@ -41,7 +41,7 @@ def datafiles_get_handler(api, client, scheme, system, path, operation, username
     try:
         return op(client, system, path, username=username, tapis_tracking_id=tapis_tracking_id, **kwargs)
     except (InternalServerError, UnauthorizedError):
-        system_needs_keys = test_system_needs_keys(client, username, system)
+        system_needs_keys = test_system_needs_keys(client, username, system, path)
         if system_needs_keys:
             logger.error(
                 f"Keys for user {username} must be manually pushed to system: {system_needs_keys.id}"

designsafe/apps/workspace/api/views.py

@@ -13,7 +13,7 @@
 from django.db.models.lookups import GreaterThan
 from django.urls import reverse
 from tapipy.tapis import TapisResult
-from tapipy.errors import InternalServerError, UnauthorizedError
+from tapipy.errors import InternalServerError, UnauthorizedError, BaseTapyException
 from designsafe.apps.api.exceptions import ApiException
 from designsafe.apps.api.users.utils import get_user_data
 from designsafe.apps.api.views import AuthenticatedApiView
@@ -114,7 +114,27 @@ def _get_app(app_id, app_version, user):
     return data
 
 
-def test_system_needs_keys(tapis, username, system_id):
+def test_system_access_ok(
+    tapis: object, username: str, system_id: str, path: str = "/"
+) -> bool:
+    """
+    Test system access by attempting to list files in a given path.
+    """
+    try:
+        tapis.files.listFiles(systemId=system_id, path=path, limit=1)
+        return True
+    except UnauthorizedError:
+        return False
+    except BaseTapyException:
+        logger.exception(
+            "System access check failed for user: %s on system: %s", username, system_id
+        )
+        raise
+
+
+def test_system_needs_keys(
+    tapis: object, username: str, system_id: str, path: str = "/"
+) -> bool:
     """Tests a Tapis system by making a file listing call.
 
     If the system is TMS_KEYS-based, it attempts to create credentials before listing files.
@@ -130,19 +150,32 @@ def test_system_needs_keys(tapis, username, system_id):
     try:
         tapis.systems.checkUserCredential(systemId=system_id, userName=username)
         return False
-    except (InternalServerError, UnauthorizedError):
-        # Check if the system uses TMS_KEYS and create credentials if necessary
+    except (InternalServerError, UnauthorizedError) as exc:
         system_def = tapis.systems.getSystem(systemId=system_id)
+
+        # Check if the system uses TMS_KEYS and create credentials if necessary
         if system_def.get("defaultAuthnMethod") == "TMS_KEYS":
             try:
                 create_system_credentials(
                     tapis, username, system_id, createTmsKeys=True
                 )
                 return False
             except (InternalServerError, UnauthorizedError):
-                logger.error(f"TMS_KEYS credential generation failed for system: {system_id}, user: {username}")
+                logger.exception(
+                    f"TMS_KEYS credential generation failed for system: {system_id}, user: {username}"
+                )
                 raise
-        return True
+
+        # If the system uses PKI_KEYS, check if the user has access
+        if (
+            system_def.get("effectiveUserId") == username
+            and system_def.get("defaultAuthnMethod") == "PKI_KEYS"
+        ):
+            return test_system_access_ok(tapis, username, system_id, path)
+
+        raise ApiException(
+            f"User {username} does not have system credentials and cannot push keys or create credentials for system {system_id}."
+        ) from exc
 
 
 class SystemListingView(AuthenticatedApiView):
@@ -373,8 +406,7 @@ def _get_public_apps(self, user, verbose):
                     bundle_label=F("bundle__label"),
                     bundle_license_type=F("bundle__license_type"),
                     bundle_user_guide_link=F("bundle__user_guide_link"),
-                )
-                .values(*values)
+                ).values(*values)
             )
 
             html_apps = list(