Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Feb 10, 2025
Merged

Conversation

chandra-tacc
Copy link
Collaborator

@chandra-tacc chandra-tacc commented Feb 6, 2025

Overview

Core Portal changes for TMS Keys migration process.
Adjusted Scenarios:

  1. Onboarding:

    • System supporting TMS - credentials are created with createTmsKeys flag set.
    • System not supporting TMS - credentials are created with portal generated public and private key.
  2. Data files

    • when listing files fails, pushkeySystem is sent back to the client.
    • for tms based systems, a credentials is created preemptively to check if that fixes listing.
    • UI will not show push modal for tms key based system. Instead it shows an error status asking to contact support because this is an api error.
  3. App retrieval

    • when listing files fails, pushkeySystem is sent back to the client.
    • for tms based systems, a credentials is created preemptively to check if that fixes listing.
    • UI will not show push modal for tms key based system. Instead it shows an error status asking to contact support because this is an api error.

Related

Changes

  1. Add additonal createUserCredentials to use createTmsKeys set to true
  2. Stop generating and using private and public keys
  3. On file listing failures, create credential.

Testing

Look for staging tenant DEV.CEP.STAGING in https://stache.utexas.edu/entry/bedc97190d3a907cb44488785440595c
and setup cep.test and login
See scenarios in Overview section for testing

  1. Remove credentials.

Removing credential to test onboarding

from django.contrib.auth import get_user_model
user = get_user_model().objects.get(username='')
tapis = user.tapis_oauth.client
tapis.systems.removeUserCredential(systemId='cloud.data', userName='')
  1. reset onboarding for system setup. onboarding should work
  2. Remove credentials again
  3. go to data files and check that credentials should be created again. No push modal is shown, but the listing fails due to other reasons(There was a problem accessing this file system.).
  4. Remove credentials again
  5. go to app and see that the credentials work but wma-exec-01 host related message shows up.(which is expected)

Copy link

codecov bot commented Feb 6, 2025

Codecov Report

Attention: Patch coverage is 47.45763% with 31 lines in your changes missing coverage. Please review.

Project coverage is 70.48%. Comparing base (44e2dff) to head (d9dc92e).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
server/portal/apps/workspace/api/utils.py 37.50% 10 Missing ⚠️
...r/portal/apps/onboarding/steps/system_access_v3.py 30.76% 9 Missing ⚠️
...onents/DataFiles/DataFilesTable/DataFilesTable.jsx 60.00% 8 Missing ⚠️
server/portal/apps/workspace/api/views.py 50.00% 3 Missing ⚠️
server/portal/apps/accounts/api/views/systems.py 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1051      +/-   ##
==========================================
+ Coverage   70.47%   70.48%   +0.01%     
==========================================
  Files         538      538              
  Lines       34028    34018      -10     
  Branches     2941     2941              
==========================================
- Hits        23980    23979       -1     
+ Misses       9848     9839       -9     
  Partials      200      200              
Flag Coverage Δ
javascript 72.66% <63.63%> (-0.01%) ⬇️
unittests 60.88% <37.83%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...nt/src/components/Applications/AppForm/AppForm.jsx 87.90% <100.00%> (-0.04%) ⬇️
...orkspace_operations/shared_workspace_operations.py 94.69% <ø> (ø)
server/portal/apps/accounts/api/views/systems.py 60.86% <50.00%> (ø)
server/portal/apps/workspace/api/views.py 57.63% <50.00%> (+2.39%) ⬆️
...onents/DataFiles/DataFilesTable/DataFilesTable.jsx 85.97% <60.00%> (-0.04%) ⬇️
...r/portal/apps/onboarding/steps/system_access_v3.py 29.50% <30.76%> (+0.52%) ⬆️
server/portal/apps/workspace/api/utils.py 51.72% <37.50%> (-17.51%) ⬇️

@chandra-tacc chandra-tacc marked this pull request as ready for review February 6, 2025 21:18
@chandra-tacc chandra-tacc marked this pull request as draft February 6, 2025 21:37
Copy link
Contributor

@fnets fnets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just added a comment so that we don't lose track of a TODO

server/portal/apps/workspace/api/views.py Show resolved Hide resolved
server/portal/apps/workspace/api/utils.py Show resolved Hide resolved
@chandra-tacc chandra-tacc marked this pull request as ready for review February 10, 2025 15:18
@chandra-tacc chandra-tacc merged commit 83dabae into main Feb 10, 2025
5 of 6 checks passed
@chandra-tacc chandra-tacc deleted the tasks/tms-update branch February 10, 2025 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants