Skip to content

Commit

Permalink
Merge branch 'main' into bugs/WP-865
Browse files Browse the repository at this point in the history
  • Loading branch information
chandra-tacc authored Feb 11, 2025
2 parents f5b2ff4 + 0cd2c59 commit a2ba0dc
Show file tree
Hide file tree
Showing 12 changed files with 201 additions and 137 deletions.
8 changes: 2 additions & 6 deletions client/src/components/Applications/AppForm/AppForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ AppInfo.propTypes = {

export const AppSchemaForm = ({ app }) => {
const dispatch = useDispatch();
const { systemNeedsKeys, pushKeysSystem } = app;

useEffect(() => {
dispatch({ type: 'GET_SYSTEM_MONITOR' });
Expand All @@ -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 &&
Expand Down Expand Up @@ -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();
Expand Down
52 changes: 24 additions & 28 deletions client/src/components/DataFiles/DataFilesTable/DataFilesTable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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">
Expand Down
82 changes: 82 additions & 0 deletions client/src/hooks/datafiles/mutations/useUpload.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { uploadUtil } from './useUpload';
import { apiClient } from 'utils/apiClient';
import Cookies from 'js-cookie';
import { vi, Mock } from 'vitest';

vi.mock('utils/apiClient');
Cookies.get = vi.fn().mockImplementation(() => 'test-cookie');

describe('uploadUtil', () => {
const api = 'tapis';
const scheme = 'private';
const system = 'apcd.submissions';
const file = new FormData();
file.append(
'uploaded_file',
new Blob(['test content'], { type: 'text/plain' })
);

beforeEach(() => {
vi.clearAllMocks();
(Cookies.get as Mock).mockReturnValue('mockCsrfToken');
});

it('should construct the correct URL when path is an empty string', async () => {
vi.mocked(apiClient.post).mockResolvedValue({
data: { file: 'mockFile', path: '' },
});

await uploadUtil({ api, scheme, system, path: '', file });

expect(apiClient.post).toHaveBeenCalledWith(
'/api/datafiles/tapis/upload/private/apcd.submissions/',
expect.any(FormData),
expect.objectContaining({
headers: { 'X-CSRFToken': 'mockCsrfToken' },
withCredentials: true,
})
);
});

it('should not call post when path is "/"', async () => {
vi.mocked(apiClient.post).mockResolvedValue({
data: { file: 'mockFile', path: '' },
});
await uploadUtil({ api, scheme, system, path: '/', file });
expect(apiClient.post).not.toHaveBeenCalled();
});

it('should construct the correct URL when path is regular folder', async () => {
vi.mocked(apiClient.post).mockResolvedValue({
data: { file: 'mockFile', path: '/subdir' },
});

await uploadUtil({ api, scheme, system, path: 'subdir', file });

expect(apiClient.post).toHaveBeenCalledWith(
'/api/datafiles/tapis/upload/private/apcd.submissions/subdir/',
expect.any(FormData),
expect.objectContaining({
headers: { 'X-CSRFToken': 'mockCsrfToken' },
withCredentials: true,
})
);
});

it('should normalize multiple slashes in the URL', async () => {
vi.mocked(apiClient.post).mockResolvedValue({
data: { file: 'mockFile', path: '/nested/path' },
});

await uploadUtil({ api, scheme, system, path: 'nested//path', file });

expect(apiClient.post).toHaveBeenCalledWith(
'/api/datafiles/tapis/upload/private/apcd.submissions/nested/path/',
expect.any(FormData),
expect.objectContaining({
headers: { 'X-CSRFToken': 'mockCsrfToken' },
withCredentials: true,
})
);
});
});
2 changes: 1 addition & 1 deletion client/src/hooks/datafiles/mutations/useUpload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export async function uploadUtil({
const fileField = file.get('uploaded_file') as Blob;
formData.append('uploaded_file', fileField);
let url = `/api/datafiles/${api}/upload/${scheme}/${system}/${apiPath}/`;
url.replace(/\/{2,}/g, '/');
url = url.replace(/\/{2,}/g, '/');
const response = await apiClient.post(url, formData, {
headers: {
'X-CSRFToken': Cookies.get('csrftoken') || '',
Expand Down
14 changes: 6 additions & 8 deletions server/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions server/portal/apps/accounts/api/views/systems.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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(
{
Expand Down
71 changes: 37 additions & 34 deletions server/portal/apps/onboarding/steps/system_access_v3.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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):
Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down
Loading

0 comments on commit a2ba0dc

Please sign in to comment.