Skip to content

Commit

Permalink
Prevent starting a Git+SSH factory if no SSH keys configured (#1043)
Browse files Browse the repository at this point in the history 
* feat: prevent starting a Git+SSH factory if no SSH keys configured

Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>

* fix: set public SSH key max length 4096

Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>

---------

Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
  • Loading branch information
@akurinnoy
akurinnoy authored Jan 24, 2024
1 parent 85ae365 commit c2f1799
Show file tree
Hide file tree
Showing 23 changed files with 761 additions and 283 deletions.
1 change: 1 addition & 0 deletions packages/dashboard-frontend/src/Routes/routes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export enum ROUTE {
FACTORY_LOADER = '/load-factory',
FACTORY_LOADER_URL = '/load-factory?url=:url',
USER_PREFERENCES = '/user-preferences',
USER_PREFERENCES_TAB = '/user-preferences?tab=:tabId',
USER_ACCOUNT = '/user-account',
}

Expand Down
20 changes: 15 additions & 5 deletions packages/dashboard-frontend/src/__tests__/workspaceCreationTimeCheck.check.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,22 @@ describe('Workspace creation time', () => {
const { rerender } = render(
getComponent(
`/load-factory?url=${url}`,
new FakeStoreBuilder().withInfrastructureNamespace([namespace]).build(),
new FakeStoreBuilder()
.withInfrastructureNamespace([namespace])
.withSshKeys({
keys: [
{
name: 'test',
keyPub: 'test',
},
],
})
.build(),
),
);

await waitFor(
() => expect(mockPost).toBeCalledWith('/api/factory/resolver', expect.anything()),
() => expect(mockPost).toHaveBeenCalledWith('/api/factory/resolver', expect.anything()),
{ timeout: 8000 },
);
expect(mockPost).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -205,7 +215,7 @@ describe('Workspace creation time', () => {
]),
{ timeout: 1500 },
);
expect(mockPost).toBeCalledTimes(3);
expect(mockPost).toHaveBeenCalledTimes(3);

await waitFor(
() =>
Expand All @@ -216,8 +226,8 @@ describe('Workspace creation time', () => {
]),
{ timeout: 1500 },
);
expect(mockPatch).toBeCalledTimes(1);
expect(mockGet).not.toBeCalled();
expect(mockPatch).toHaveBeenCalledTimes(1);
expect(mockGet).not.toHaveBeenCalled();
expect(screen.queryByTestId('fallback-spinner')).not.toBeInTheDocument();

expect(execTime).toBeLessThan(TIME_LIMIT);
Expand Down
2 changes: 1 addition & 1 deletion ...frontend/src/components/WorkspaceProgress/CreatingSteps/CheckExistingWorkspaces/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class CreatingStepCheckExistingWorkspaces extends ProgressStep<Props, State> {
}

let newWorkspaceName: string;
if (factoryParams.useDevworkspaceResources === true) {
if (factoryParams.useDevWorkspaceResources === true) {
const resources = devWorkspaceResources[factoryParams.sourceUrl]?.resources;
if (resources === undefined) {
// going to use the default devfile in the next step
Expand Down
66 changes: 63 additions & 3 deletions ...ontend/src/components/WorkspaceProgress/CreatingSteps/Initialize/__tests__/index.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
FACTORY_URL_ATTR,
POLICIES_CREATE_ATTR,
} from '@/services/helpers/factoryFlow/buildFactoryParams';
import { AlertItem } from '@/services/helpers/types';
import { AlertItem, UserPreferencesTab } from '@/services/helpers/types';
import { DevWorkspaceBuilder } from '@/store/__mocks__/devWorkspaceBuilder';
import { FakeStoreBuilder } from '@/store/__mocks__/storeBuilder';

Expand All @@ -36,20 +36,35 @@ const mockOnRestart = jest.fn();
const mockOnError = jest.fn();
const mockOnHideError = jest.fn();

jest.mock('@/services/helpers/location', () => ({
toHref: (_: unknown, location: string) => 'http://localhost/' + location,
buildUserPreferencesLocation: (tab: UserPreferencesTab) => 'user-preferences?tab=' + tab,
}));

describe('Creating steps, initializing', () => {
const factoryUrl = 'https://factory-url';
const { reload } = window.location;

let store: Store;

beforeEach(() => {
store = new FakeStoreBuilder()
.withInfrastructureNamespace([{ name: 'user-che', attributes: { phase: 'Active' } }])
.withSshKeys({
keys: [{ name: 'key1', keyPub: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD' }],
})
.build();

delete (window as any).location;
(window.location as any) = { reload: jest.fn() };
window.open = jest.fn();

jest.useFakeTimers();
});

afterEach(() => {
window.location.reload = reload;

jest.clearAllMocks();
jest.clearAllTimers();
jest.useRealTimers();
Expand Down Expand Up @@ -83,7 +98,7 @@ describe('Creating steps, initializing', () => {
});
await waitFor(() => expect(mockOnError).toHaveBeenCalledWith(expectAlertItem));

expect(mockOnRestart).toHaveBeenCalled();
expect(window.location.reload).toHaveBeenCalled();
expect(mockOnNextStep).not.toHaveBeenCalled();
});

Expand All @@ -99,7 +114,7 @@ describe('Creating steps, initializing', () => {

const expectAlertItem = expect.objectContaining({
title: 'Failed to create the workspace',
children: expect.stringContaining('Devworkspace resources URL is missing.'),
children: expect.stringContaining('DevWorkspace resources URL is missing.'),
actionCallbacks: [
expect.objectContaining({
title: 'Click to try again',
Expand Down Expand Up @@ -213,6 +228,9 @@ describe('Creating steps, initializing', () => {
.withInfrastructureNamespace([{ name: 'user-che', attributes: { phase: 'Active' } }])
.withClusterConfig({ allWorkspacesLimit: 1 })
.withDevWorkspaces({ workspaces: [new DevWorkspaceBuilder().build()] })
.withSshKeys({
keys: [{ name: 'key1', keyPub: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD' }],
})
.build();
const searchParams = new URLSearchParams({
[FACTORY_URL_ATTR]: factoryUrl,
Expand All @@ -236,6 +254,48 @@ describe('Creating steps, initializing', () => {

expect(mockOnNextStep).not.toHaveBeenCalled();
});

test('no SSH keys with Git+SSH factory URL', async () => {
const store = new FakeStoreBuilder()
.withInfrastructureNamespace([{ name: 'user-che', attributes: { phase: 'Active' } }])
.build();
const searchParams = new URLSearchParams({
[FACTORY_URL_ATTR]: factoryUrl,
});

// this will help test the case when the user clicks on the "Add SSH Keys" button
mockOnError.mockImplementation((alertItem: AlertItem) => {
if (alertItem.actionCallbacks) {
alertItem.actionCallbacks[1].callback();
}
});

renderComponent(store, searchParams);

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

const expectAlertItem = expect.objectContaining({
title: 'No SSH keys found',
children: 'No SSH keys found. Please add your SSH keys and then try again.',
actionCallbacks: [
expect.objectContaining({
title: 'Click to try again',
callback: expect.any(Function),
}),
expect.objectContaining({
title: 'Add SSH Keys',
callback: expect.any(Function),
}),
],
});
await waitFor(() => expect(mockOnError).toHaveBeenCalledWith(expectAlertItem));

expect(window.open).toHaveBeenCalledWith(
'http://localhost/user-preferences?tab=SshKeys',
'_blank',
);
expect(mockOnNextStep).not.toHaveBeenCalled();
});
});

function getComponent(store: Store, searchParams: URLSearchParams): React.ReactElement {
Expand Down
66 changes: 55 additions & 11 deletions ...es/dashboard-frontend/src/components/WorkspaceProgress/CreatingSteps/Initialize/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ import {
FactoryParams,
PoliciesCreate,
} from '@/services/helpers/factoryFlow/buildFactoryParams';
import { AlertItem } from '@/services/helpers/types';
import { buildUserPreferencesLocation, toHref } from '@/services/helpers/location';
import { AlertItem, UserPreferencesTab } from '@/services/helpers/types';
import { AppState } from '@/store';
import { selectAllWorkspacesLimit } from '@/store/ClusterConfig/selectors';
import { selectInfrastructureNamespaces } from '@/store/InfrastructureNamespaces/selectors';
import { selectSshKeys } from '@/store/SshKeys/selectors';
import { selectAllWorkspaces } from '@/store/Workspaces/selectors';

export type Props = MappedProps &
Expand Down Expand Up @@ -90,12 +92,12 @@ class CreatingStepInitialize extends ProgressStep<Props, State> {
}

protected async runStep(): Promise<boolean> {
const { useDevworkspaceResources, sourceUrl, errorCode, policiesCreate, remotes } =
const { useDevWorkspaceResources, sourceUrl, errorCode, policiesCreate, remotes } =
this.state.factoryParams;

if (useDevworkspaceResources === true && sourceUrl === '') {
throw new Error('Devworkspace resources URL is missing.');
} else if (useDevworkspaceResources === false && sourceUrl === '' && !remotes) {
if (useDevWorkspaceResources === true && sourceUrl === '') {
throw new Error('DevWorkspace resources URL is missing.');
} else if (useDevWorkspaceResources === false && sourceUrl === '' && !remotes) {
const factoryPath = generatePath(ROUTE.FACTORY_LOADER_URL, {
url: 'your-repository-url',
});
Expand Down Expand Up @@ -126,6 +128,11 @@ class CreatingStepInitialize extends ProgressStep<Props, State> {
);
}

// check for SSH keys availability
if (this.props.sshKeys.length === 0) {
throw new NoSshKeysError('No SSH keys found.');
}

this.checkAllWorkspacesLimitExceeded();

return true;
Expand All @@ -135,13 +142,38 @@ class CreatingStepInitialize extends ProgressStep<Props, State> {
return (val && (val as PoliciesCreate) === 'perclick') || (val as PoliciesCreate) === 'peruser';
}

private handleRestart(alertKey: string): void {
this.props.onHideError(alertKey);
this.props.onRestart();
private handleRestart(): void {
window.location.reload();
}

private handleOpenUserPreferences(): void {
const location = buildUserPreferencesLocation(UserPreferencesTab.SSH_KEYS);
const link = toHref(this.props.history, location);
window.open(link, '_blank');
}

protected buildAlertItem(error: Error): AlertItem {
const key = this.name;

if (error instanceof NoSshKeysError) {
return {
key,
title: 'No SSH keys found',
variant: AlertVariant.warning,
children: 'No SSH keys found. Please add your SSH keys and then try again.',
actionCallbacks: [
{
title: 'Click to try again',
callback: () => this.handleRestart(),
},
{
title: 'Add SSH Keys',
callback: () => this.handleOpenUserPreferences(),
},
],
};
}

return {
key,
title: 'Failed to create the workspace',
Expand All @@ -150,7 +182,7 @@ class CreatingStepInitialize extends ProgressStep<Props, State> {
actionCallbacks: [
{
title: 'Click to try again',
callback: () => this.handleRestart(key),
callback: () => this.handleRestart(),
},
],
};
Expand All @@ -171,8 +203,12 @@ class CreatingStepInitialize extends ProgressStep<Props, State> {
const { distance, hasChildren } = this.props;
const { name, lastError } = this.state;

const isError = lastError !== undefined;
const isWarning = false;
let isError = lastError !== undefined;
let isWarning = false;
if (lastError instanceof NoSshKeysError) {
isWarning = true;
isError = false;
}

return (
<React.Fragment>
Expand All @@ -196,10 +232,18 @@ export class AllWorkspacesExceededError extends Error {
}
}

export class NoSshKeysError extends Error {
constructor(message: string) {
super(message);
this.name = 'NoSshKeysError';
}
}

const mapStateToProps = (state: AppState) => ({
infrastructureNamespaces: selectInfrastructureNamespaces(state),
allWorkspaces: selectAllWorkspaces(state),
allWorkspacesLimit: selectAllWorkspacesLimit(state),
sshKeys: selectSshKeys(state),
});

const connector = connect(mapStateToProps, {}, null, {
Expand Down
2 changes: 1 addition & 1 deletion packages/dashboard-frontend/src/components/WorkspaceProgress/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ class Progress extends React.Component<Props, State> {
const { history, searchParams } = this.props;
const { factoryParams } = this.state;

const usePrebuiltResources = factoryParams.useDevworkspaceResources;
const usePrebuiltResources = factoryParams.useDevWorkspaceResources;
const steps = [
usePrebuiltResources ? this.getFactoryFetchResources() : this.getFactoryFetchDevfile(),
this.getCheckExistingWorkspacesStep(),
Expand Down
Loading

0 comments on commit c2f1799

Please sign in to comment.