Task/WP-832: switching to TMS for credentials (#1051)

* switching to tms for credentials * lint fix * Address review comments * Update server/portal/apps/workspace/api/utils.py Co-authored-by: fnets <fnetscher@tacc.utexas.edu> * Client adjustments for Keys * linting and onboarding adjustment * Update to latest tapipy * fixing poetry error * Fix testing credentials --------- Co-authored-by: fnets <fnetscher@tacc.utexas.edu>
TACC · Feb 10, 2025 · 83dabae · 83dabae
1 parent 44e2dff
commit 83dabae
Show file tree

Hide file tree

Showing 10 changed files with 118 additions and 136 deletions.
diff --git a/client/src/components/Applications/AppForm/AppForm.jsx b/client/src/components/Applications/AppForm/AppForm.jsx
@@ -233,6 +233,7 @@ AppInfo.propTypes = {
 
 export const AppSchemaForm = ({ app }) => {
   const dispatch = useDispatch();
+  const { systemNeedsKeys, pushKeysSystem } = app;
 
   useEffect(() => {
     dispatch({ type: 'GET_SYSTEM_MONITOR' });
@@ -255,10 +256,7 @@ export const AppSchemaForm = ({ app }) => {
       state.allocations
     );
     const { defaultHost, configuration, defaultSystem } = state.systems.storage;
-
-    const keyService = state.systems.storage.configuration.find(
-      (sys) => sys.system === defaultSystem && sys.default
-    )?.keyservice;
+    const keyService = pushKeysSystem?.defaultAuthnMethod === 'TMS_KEYS';
 
     const hasCorral =
       configuration.length &&
@@ -299,8 +297,6 @@ export const AppSchemaForm = ({ app }) => {
     (state) => state.workbench.config.hideManageAccount
   );
 
-  const { systemNeedsKeys, pushKeysSystem } = app;
-
   const missingLicense = app.license.type && !app.license.enabled;
   const pushKeys = (e) => {
     e.preventDefault();

diff --git a/client/src/components/DataFiles/DataFilesTable/DataFilesTable.jsx b/client/src/components/DataFiles/DataFilesTable/DataFilesTable.jsx
@@ -34,11 +34,6 @@ const DataFilesTablePlaceholder = ({ section, data }) => {
       (sysDef) => sysDef.id === state.files.params.FilesListing.system
     )
   );
-
-  const { fetchSelectedSystem } = useSystems();
-
-  const selectedSystem = fetchSelectedSystem(params ?? {});
-
   const currSystemHost = currSystem ? currSystem.host : '';
 
   const modalRefs = useSelector((state) => state.files.refs);
@@ -123,29 +118,30 @@ const DataFilesTablePlaceholder = ({ section, data }) => {
         ['private', 'projects'].includes(scheme) &&
         currSystem?.effectiveUserId === currentUser
       ) {
-        const sectionMessage = selectedSystem?.keyservice ? (
-          <span>
-            For help, please{' '}
-            <Link
-              className="wb-link"
-              to={`${ROUTES.WORKBENCH}${ROUTES.DASHBOARD}${ROUTES.TICKETS}/create`}
-            >
-              submit a ticket.
-            </Link>
-          </span>
-        ) : (
-          <span>
-            If this is your first time accessing this system, you may need to{' '}
-            <a
-              className="data-files-nav-link"
-              type="button"
-              href="#"
-              onClick={pushKeys}
-            >
-              push your keys
-            </a>
-          </span>
-        );
+        const sectionMessage =
+          currSystem?.defaultAuthnMethod === 'TMS_KEYS' ? (
+            <span>
+              For help, please{' '}
+              <Link
+                className="wb-link"
+                to={`${ROUTES.WORKBENCH}${ROUTES.DASHBOARD}${ROUTES.TICKETS}/create`}
+              >
+                submit a ticket.
+              </Link>
+            </span>
+          ) : (
+            <span>
+              If this is your first time accessing this system, you may need to{' '}
+              <a
+                className="data-files-nav-link"
+                type="button"
+                href="#"
+                onClick={pushKeys}
+              >
+                push your keys
+              </a>
+            </span>
+          );
 
         return (
           <div className="h-100 listing-placeholder">

diff --git a/server/poetry.lock b/server/poetry.lock
diff --git a/server/portal/apps/accounts/api/views/systems.py b/server/portal/apps/accounts/api/views/systems.py
@@ -9,7 +9,7 @@
 from django.utils.decorators import method_decorator
 from portal.views.base import BaseApiView
 from portal.apps.accounts.managers import accounts as AccountsManager
-from portal.apps.onboarding.steps.system_access_v3 import create_system_credentials
+from portal.apps.onboarding.steps.system_access_v3 import create_system_credentials_with_keys
 from portal.utils.encryption import createKeyPair
 
 logger = logging.getLogger(__name__)
@@ -52,11 +52,11 @@ def push(self, request, system_id, body):
             hostname=body['form']['hostname']
         )
 
-        create_system_credentials(request.user.tapis_oauth.client,
-                                  request.user.username,
-                                  publ_key_str,
-                                  priv_key_str,
-                                  system_id)
+        create_system_credentials_with_keys(request.user.tapis_oauth.client,
+                                            request.user.username,
+                                            publ_key_str,
+                                            priv_key_str,
+                                            system_id)
 
         return JsonResponse(
             {

diff --git a/server/portal/apps/onboarding/steps/system_access_v3.py b/server/portal/apps/onboarding/steps/system_access_v3.py
@@ -1,27 +1,23 @@
 import logging
-import requests
-from requests.exceptions import HTTPError
 from portal.apps.onboarding.steps.abstract import AbstractStep
 from portal.apps.onboarding.state import SetupState
-from django.conf import settings
-from portal.utils.encryption import createKeyPair
 from portal.libs.agave.utils import service_account
 from tapipy.errors import BaseTapyException
-
+from portal.utils.encryption import createKeyPair
 
 logger = logging.getLogger(__name__)
 
 
-def create_system_credentials(client,
-                              username,
-                              public_key,
-                              private_key,
-                              system_id,
-                              skipCredentialCheck=False) -> int:
+def create_system_credentials_with_keys(client,
+                                        username,
+                                        public_key,
+                                        private_key,
+                                        system_id,
+                                        skipCredentialCheck=False) -> int:
     """
     Set an RSA key pair as the user's auth credential on a Tapis system.
     """
-    logger.info(f"Creating user credential for {username} on Tapis system {system_id}")
+    logger.info(f"Creating user credential for {username} on Tapis system {system_id} using keys")
     data = {'privateKey': private_key, 'publicKey': public_key}
     client.systems.createUserCredential(
         systemId=system_id,
@@ -31,16 +27,21 @@ def create_system_credentials(client,
     )
 
 
-def register_public_key(username, publicKey, system_id) -> int:
+def create_system_credentials(client,
+                              username,
+                              system_id,
+                              createTmsKeys,
+                              skipCredentialCheck=False) -> int:
     """
-    Push a public key to the Key Service API.
+    Create user's auth credential on a Tapis system. This Tapis API uses TMS.
     """
-    url = "https://api.tacc.utexas.edu/keys/v2/" + username
-    headers = {"Authorization": "Bearer {}".format(settings.KEY_SERVICE_TOKEN)}
-    data = {"key_value": publicKey, "tags": [{"name": "system", "value": system_id}]}
-    response = requests.post(url, json=data, headers=headers)
-    response.raise_for_status()
-    return response.status_code
+    logger.info(f"Creating user credential for {username} on Tapis system {system_id} using TMS")
+    client.systems.createUserCredential(
+        systemId=system_id,
+        userName=username,
+        createTmsKeys=createTmsKeys,
+        skipCredentialCheck=skipCredentialCheck,
+    )
 
 
 def set_user_permissions(user, system_id):
@@ -77,6 +78,12 @@ def prepare(self):
         self.state = SetupState.PENDING
         self.log("Awaiting TACC systems access.")
 
+    def get_system(self, system_id) -> None:
+        """
+        Get the system definition
+        """
+        return self.user.tapis_oauth.client.systems.getSystem(systemId=system_id)
+
     def check_system(self, system_id) -> None:
         """
         Check whether a user already has access to a storage system by attempting a listing.
@@ -101,21 +108,17 @@ def process(self):
             except BaseTapyException:
                 self.log(f"Creating credentials for system: {system}")
 
-            (priv, pub) = createKeyPair()
-
-            try:
-                register_public_key(self.user.username, pub, system)
-                self.log(f"Successfully registered public key for system: {system}")
-            except HTTPError as e:
-                logger.error(e)
-                self.fail(f"Failed to register public key with key service for system: {system}")
-
+            system_definition = self.get_system(system)
             try:
-                create_system_credentials(self.user.tapis_oauth.client,
-                                          self.user.username,
-                                          pub,
-                                          priv,
-                                          system)
+                if system_definition.get("defaultAuthnMethod") != 'TMS_KEYS':
+                    (priv, pub) = createKeyPair()
+                    create_system_credentials_with_keys(
+                        self.user.tapis_oauth.client, self.user.username, pub, priv, system
+                    )
+                else:
+                    create_system_credentials(
+                        self.user.tapis_oauth.client, self.user.username, system, createTmsKeys=True
+                    )
                 self.log(f"Successfully created credentials for system: {system}")
             except BaseTapyException as e:
                 logger.error(e)

diff --git a/server/portal/apps/projects/workspace_operations/shared_workspace_operations.py b/server/portal/apps/projects/workspace_operations/shared_workspace_operations.py
@@ -4,7 +4,6 @@
 from typing import Literal
 from django.conf import settings
 from django.contrib.auth import get_user_model
-# from portal.apps.onboarding.steps.system_access_v3 import create_system_credentials, register_public_key
 
 
 import logging
@@ -181,16 +180,6 @@ def create_shared_workspace(client: Tapis, title: str, owner: str):
 
     # User creates the system and adds their credential
     system_id = create_workspace_system(client, workspace_id, title)
-    # priv_key, pub_key = createKeyPair()
-    # register_public_key(owner,
-    #                     pub_key,
-    #                     system_id)
-    # create_system_credentials(client, owner, pub_key, priv_key, system_id)
-    # create_system_credentials(service_client,
-    #                           settings.PORTAL_ADMIN_USERNAME,
-    #                          settings.PORTAL_PROJECTS_PUBLIC_KEY,
-    #                           settings.PORTAL_PROJECTS_PRIVATE_KEY,
-    #                           system_id)
 
     return system_id
 
@@ -210,14 +199,6 @@ def add_user_to_workspace(client: Tapis,
                        "add",
                        role)
 
-    # Code to generate/push user keys to a workspace
-    # (uncomment to add per-user credentials)
-    # priv_key, pub_key = createKeyPair()
-    # register_public_key(username,
-    #                     pub_key,
-    #                     system_id)
-    # create_system_credentials(client, username, pub_key, priv_key, system_id)
-
     # Share system to allow listing of users
     client.systems.shareSystem(systemId=system_id, users=[username])
     set_workspace_permissions(client, username, system_id, role)

diff --git a/server/portal/apps/workspace/api/utils.py b/server/portal/apps/workspace/api/utils.py
@@ -1,4 +1,9 @@
+from portal.apps.onboarding.steps.system_access_v3 import create_system_credentials
+from tapipy.errors import BaseTapyException
 import json
+import logging
+
+logger = logging.getLogger(__name__)
 
 
 def get_tapis_timeout_error_messages(job_id):
@@ -26,3 +31,30 @@ def check_job_for_timeout(job):
             job.remoteOutcome = 'FINISHED'
 
     return job
+
+
+def should_push_keys(system):
+    """
+    If defaultAuthnMethod is not TMS_KEYS, return true. Otherwise, false.
+    """
+    return system.get("defaultAuthnMethod") != 'TMS_KEYS'
+
+
+def test_system_credentials(system, user):
+    """
+    If system does not support TMS, create keys and
+    tapis system credentials using keys, otherwise create
+    credentials with TMS.
+    """
+    # TODOv3: Add Tapis system test utility method with proper error handling https://tacc-main.atlassian.net/browse/WP-101
+    tapis = user.tapis_oauth.client
+    if should_push_keys(system):
+        return False
+    else:
+        try:
+            create_system_credentials(user.tapis_oauth.client, user.username, system.id, createTmsKeys=True)
+            tapis.files.listFiles(systemId=system.id, path="/")
+        except BaseTapyException:
+            return False
+
+    return True