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

Shared user to START shared app #553

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 40 additions & 40 deletions jhub_apps/static/js/index.js

Large diffs are not rendered by default.

22 changes: 15 additions & 7 deletions jupyterhub_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,28 +65,36 @@ def service_for_jhub_apps(name, url):
# This should be handled by the users of jhub-apps,
# jhub-apps won't make this decision for the users, so that
# they can define permissions as per their preferences

for role in c.JupyterHub.load_roles:
if role["name"] == "user":
role["scopes"].extend(
[
# Need scope 'read:users:name' to share with users by name
# Scopes required for shared servers
"access:servers",
"servers",
# Allow reading user and group names for sharing
"read:users:name",
# Need scope 'read:groups:name' to share with groups by name
"read:groups:name",
# Allow sharing functionality (if on JupyterHub 5.x+)
"shares!user",
]
+ ["shares!user"]
if is_jupyterhub_5()
else []
if is_jupyterhub_5() else []
)
break

# Add permission to start shared servers
c.JupyterHub.load_roles = c.JupyterHub.load_roles + [
{
"name": "allow-access-to-start-shared-server",
"description": "Allows users to start shared server",
"description": "Allows users to start shared servers",
"scopes": [
"servers",
# This scope allows access to servers owned by other users (shared servers)
"access:servers!server={server_owner}/{server_name}",
# This scope enables starting those servers
"servers!server={server_owner}/{server_name}",
],
# Specify users or groups allowed to start shared servers
"users": ["user-with-permission-to-start-shared"],
}
]
Expand Down
60 changes: 60 additions & 0 deletions ui/src/components/app-card/app-card.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -804,4 +804,64 @@ describe('AppCard', () => {
}),
);
});
test('shows error snackbar when trying to stop a shared app without admin rights', async () => {
const { getByTestId, getByText, queryByText } = render(
<RecoilRoot>
<QueryClientProvider client={queryClient}>
<AppCard
id="1"
title="Shared App"
username="Non-Admin"
framework="Some Framework"
url="/some-url"
serverStatus="Running"
isShared={true} // Shared app
app={{
id: '1',
name: 'Shared App',
framework: 'Some Framework',
description: 'Test App 1',
url: '/user/test/shared-app/',
thumbnail: '',
username: 'test',
ready: true,
public: false,
shared: true,
last_activity: new Date(),
pending: false,
stopped: false,
status: 'false',
full_name: 'test/shared-app',
}}
/>
</QueryClientProvider>
</RecoilRoot>,
);

// Open context menu first
const contextMenuButton = getByTestId('context-menu-button-card-menu-1');
act(() => {
contextMenuButton.click();
});

const stopMenuItem = await waitFor(() => getByText('Stop'));
expect(stopMenuItem).toBeInTheDocument();

// Simulate clicking the "Stop" button
await act(async () => {
stopMenuItem.click();
});

// Verify that the Snackbar appears with the correct message
const snackbar = await waitFor(() =>
queryByText(
"You don't have permission to stop this app. Please ask the owner to stop it.",
),
);
expect(snackbar).toBeInTheDocument();

// Verify that the Snackbar has the correct severity styling
const snackbarAlert = snackbar?.closest('.MuiAlert-root');
expect(snackbarAlert).toHaveClass('MuiAlert-standardError'); // Check for the correct class
});
});
48 changes: 36 additions & 12 deletions ui/src/components/app-card/app-card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import GroupRoundedIcon from '@mui/icons-material/GroupRounded';
import LockRoundedIcon from '@mui/icons-material/LockRounded';
import PublicRoundedIcon from '@mui/icons-material/PublicRounded';
import PushPinRoundedIcon from '@mui/icons-material/PushPinRounded';
import { Box, Link, Tooltip } from '@mui/material';
import { Alert, Box, Link, Snackbar, Tooltip } from '@mui/material';
import Card from '@mui/material/Card';
import CardContent from '@mui/material/CardContent';
import CardMedia from '@mui/material/CardMedia';
Expand All @@ -18,7 +18,6 @@ import { UserState } from '@src/types/user';
import axios from 'axios';
import React, { useEffect, useState } from 'react';
import { useRecoilState } from 'recoil';

import {
currentApp,
currentNotification,
Expand Down Expand Up @@ -65,6 +64,12 @@ export const AppCard = ({
const [appStatus, setAppStatus] = useState('');
const [currentProfiles] = useRecoilState<AppProfileProps[]>(defaultProfiles);
const [, setCurrentApp] = useRecoilState<JhApp | undefined>(currentApp);
const [snackbarOpen, setSnackbarOpen] = useState(false);
const [snackbarMessage, setSnackbarMessage] = useState('');
const [snackbarSeverity, setSnackbarSeverity] = useState<'success' | 'error'>(
'success',
);

const [, setNotification] = useRecoilState<string | undefined>(
currentNotification,
);
Expand Down Expand Up @@ -124,29 +129,24 @@ export const AppCard = ({
id: 'start',
title: 'Start',
onClick: async () => {
// Allow admins to start shared apps
if (isShared && !currentUserData?.admin) {
// Show error if it's a shared app
setNotification(
"You don't have permission to start this app. Please ask the owner to start it.",
);
return;
}
setIsStartOpen(true);
setCurrentApp(app!); // Add the non-null assertion operator (!) to ensure that app is not undefined
setCurrentApp(app!); // Ensure app is defined
},
visible: true,
disabled: serverStatus !== 'Ready', // Disable start if the app is already running
},

{
id: 'stop',
title: 'Stop',
onClick: async () => {
// Allow admins to stop shared apps
if (isShared && !currentUserData?.admin) {
setNotification(
setSnackbarMessage(
"You don't have permission to stop this app. Please ask the owner to stop it.",
);
setSnackbarSeverity('error');
setSnackbarOpen(true);
return;
}
setIsStopOpen(true);
Expand Down Expand Up @@ -370,6 +370,30 @@ export const AppCard = ({
</div>
</Card>
</Link>
<Snackbar
open={snackbarOpen}
autoHideDuration={6000}
onClose={() => setSnackbarOpen(false)}
anchorOrigin={{ vertical: 'top', horizontal: 'center' }}
sx={{
top: '90px !important',
}}
>
<Alert
onClose={() => setSnackbarOpen(false)}
severity={snackbarSeverity}
sx={{
width: '100%',
fontFamily: 'Inter, sans-serif',
fontWeight: 600,
backgroundColor:
snackbarSeverity === 'success' ? '#D1FAE5' : '#FEE2E2',
color: snackbarSeverity === 'success' ? '#065F46' : '#B91C1C',
}}
>
{snackbarMessage}
</Alert>
</Snackbar>
</Box>
);
};
Expand Down
109 changes: 59 additions & 50 deletions ui/src/pages/home/home.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -720,56 +720,6 @@ describe('Home', () => {
});
});

test('should show 403 error snackbar when trying to start a shared app', async () => {
mock.onPost('/server/test-app-1').reply(403); // Mocking 403 Forbidden for shared app

const { baseElement, getByText } = render(
<RecoilRoot
initializeState={({ set }) => {
set(isStartOpen, true);
set(defaultApp, {
id: 'test-app-1',
name: 'Test App',
framework: 'JupyterLab',
url: 'https://example.com',
ready: true,
public: false,
shared: true,
last_activity: new Date(),
status: 'Ready',
}); // App is shared
}}
>
<QueryClientProvider client={queryClient}>
<BrowserRouter>
<Home />
</BrowserRouter>
</QueryClientProvider>
</RecoilRoot>,
);

// Modal should be present
await waitFor(() => {
const startModal = within(baseElement).getByTestId('StartModal');
expect(startModal).toBeInTheDocument();
});

// Simulate clicking the Start button
const startBtn = baseElement.querySelector(
'#start-btn',
) as HTMLButtonElement;
await act(async () => {
startBtn.click();
});

// Expect the snackbar to display the 403 Forbidden error message
await waitFor(() => {
const snackbar = getByText(
/You don't have permission to start this app. Please ask the owner to start it./,
);
expect(snackbar).toBeInTheDocument();
});
});
test('should show 403 error snackbar when trying to stop a shared app', async () => {
mock.onDelete('/server/test-app-1').reply(403); // Mocking 403 Forbidden for shared app

Expand Down Expand Up @@ -860,4 +810,63 @@ describe('Home', () => {
startBtn.click();
});
});
test('should update query client state after successful app start', async () => {
mock.onPost('/server/test-app-1').reply(200);

// Use vi.spyOn to spy on invalidateQueries
const invalidateQueriesSpy = vi.spyOn(queryClient, 'invalidateQueries');

const { baseElement } = render(
<RecoilRoot
initializeState={({ set }) => {
set(isStartOpen, true);
set(defaultApp, apps[0]); // Simulating the app being set in Recoil state
}}
>
<QueryClientProvider client={queryClient}>
<BrowserRouter>
<Home />
</BrowserRouter>
</QueryClientProvider>
</RecoilRoot>,
);

const startBtn = baseElement.querySelector(
'#start-btn',
) as HTMLButtonElement;

await act(async () => {
startBtn.click();
});

// Wait for invalidateQueries to be called with the correct arguments
await waitFor(() => {
expect(invalidateQueriesSpy).toHaveBeenCalledWith({
queryKey: ['app-state'],
});
});

// Restore the spy
invalidateQueriesSpy.mockRestore();
});
test('should close start modal when cancel button is clicked', async () => {
const { baseElement } = render(
<RecoilRoot initializeState={({ set }) => set(isStartOpen, true)}>
<QueryClientProvider client={queryClient}>
<BrowserRouter>
<Home />
</BrowserRouter>
</QueryClientProvider>
</RecoilRoot>,
);

const cancelBtn = baseElement.querySelector(
'#cancel-btn',
) as HTMLButtonElement;
await act(async () => {
cancelBtn.click();
});

expect(baseElement.querySelector('#StartModal')).not.toBeInTheDocument();
});
});
20 changes: 2 additions & 18 deletions ui/src/pages/home/home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ export const Home = (): React.ReactElement => {
url: `/server/${id}`,
params: { owner: creatorName || '' },
};

const response = await axios(requestConfig);

return response;
Expand Down Expand Up @@ -165,6 +164,7 @@ export const Home = (): React.ReactElement => {
): error is { response: { status: number } } => {
return typeof error === 'object' && error !== null && 'response' in error;
};

const handleStart = async () => {
const appId = currentApp?.id || '';
const fullName = currentApp?.full_name || '';
Expand All @@ -174,18 +174,6 @@ export const Home = (): React.ReactElement => {
setIsStartOpen(false);
setIsStartNotRunningOpen(false);

// Check if the app is shared and if the user has permissions
const sharedApp = currentApp?.shared;
if (sharedApp && !currentUserData?.admin) {
setSubmitting(false);
setSnackbarMessage(
"You don't have permission to start this app. Please ask the owner to start it.",
);
setSnackbarSeverity('error');
setSnackbarOpen(true);
return;
}

startQuery(
{ id: appId, full_name: fullName },
{
Expand All @@ -201,11 +189,7 @@ export const Home = (): React.ReactElement => {

if (isErrorWithResponse(error)) {
const status = error.response?.status;
if (status === 403) {
setSnackbarMessage(
"You don't have permission to start this app. Please ask the owner to start it.",
);
} else if (status === 404) {
if (status === 404) {
setSnackbarMessage('App not found (404).');
} else if (status === 500) {
setSnackbarMessage('Internal server error (500).');
Expand Down
Loading