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

feat(bulk-import): add permission checks in bulk import UI #2034

Merged
merged 1 commit into from
Aug 28, 2024
Merged
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
24 changes: 24 additions & 0 deletions plugins/bulk-import/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,30 @@ p, role:default/team_a, catalog-entity.create, create, allow
g, user:default/<login-id/user-name>, role:default/team_a
```

#### Installing as a dynamic plugin?

The sections below are relevant for static plugins. If the plugin is expected to be installed as a dynamic one:

- follow https://github.com/janus-idp/backstage-showcase/blob/main/showcase-docs/dynamic-plugins.md#installing-a-dynamic-plugin-package-in-the-showcase
- add content of `app-config.janus-idp.yaml` into `app-config.local.yaml`.

#### Prerequisites

Follow the Bulk import backend plugin [README](https://github.com/janus-idp/backstage-plugins/blob/main/plugins/bulk-import-backend/README.md) to integrate bulk import in your Backstage instance.

---

**NOTE**

- When RBAC permission framework is enabled, for non-admin users to access bulk import UI, the role associated with your user should have the following permission policies associated with it. Add the following in your permission policies configuration file:

```CSV
p, role:default/team_a, bulk.import, use, allow
p, role:default/team_a, catalog-entity.read, read, allow
p, role:default/team_a, catalog-entity.create, create, allow
g, user:default/<login-id/user-name>, role:default/team_a
```

#### Procedure

1. Install the Bulk import UI plugin using the following command:
Expand Down
17 changes: 16 additions & 1 deletion plugins/bulk-import/dev/index.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import React from 'react';

import { configApiRef } from '@backstage/core-plugin-api';
import { createDevApp } from '@backstage/dev-utils';
import { catalogApiRef } from '@backstage/plugin-catalog-react';
import { TestApiProvider } from '@backstage/test-utils';
import { permissionApiRef } from '@backstage/plugin-permission-react';
import {
MockConfigApi,
MockPermissionApi,
TestApiProvider,
} from '@backstage/test-utils';

import { getAllThemes } from '@redhat-developer/red-hat-developer-hub-theme';

Expand Down Expand Up @@ -101,6 +107,13 @@ class MockBulkImportApi implements BulkImportAPI {
}

const mockBulkImportApi = new MockBulkImportApi();
const mockPermissionApi = new MockPermissionApi();

const mockConfigApi = new MockConfigApi({
permission: {
enabled: true,
},
});

createDevApp()
.registerPlugin(bulkImportPlugin)
Expand All @@ -109,8 +122,10 @@ createDevApp()
element: (
<TestApiProvider
apis={[
[permissionApiRef, mockPermissionApi],
[catalogApiRef, mockCatalogApi],
[bulkImportApiRef, mockBulkImportApi],
[configApiRef, mockConfigApi],
]}
>
<BulkImportPage />
Expand Down
5 changes: 4 additions & 1 deletion plugins/bulk-import/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,17 @@
"@backstage/core-plugin-api": "^1.9.3",
"@backstage/plugin-catalog-import": "^0.12.1",
"@backstage/plugin-catalog-react": "^1.12.2",
"@backstage/plugin-permission-react": "^0.4.24",
"@backstage/theme": "^0.5.6",
"@janus-idp/backstage-plugin-bulk-import-common": "0.2.0",
"@janus-idp/shared-react": "2.10.0",
"@material-ui/core": "^4.9.13",
"@material-ui/icons": "^4.11.3",
"@material-ui/lab": "^4.0.0-alpha.61",
"@mui/icons-material": "5.16.4",
"@mui/material": "^5.12.2",
"formik": "^2.4.5",
"js-yaml": "^4.1.0",
"lodash": "^4.17.21",
"react-use": "^17.2.4",
"yaml": "^2.0.0"
Expand Down Expand Up @@ -95,5 +98,5 @@
"maintainers": [
"@janus-idp/maintainers-plugins"
],
"author": "The Backstage Community"
"author": "Red Hat"
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ import { StatusRunning } from '@backstage/core-components';

import { useFormikContext } from 'formik';

import { AddRepositoriesFormValues, AddRepositoryData } from '../../types';
import {
AddRepositoriesFormValues,
AddRepositoryData,
RepositoryStatus,
} from '../../types';
import {
areAllRowsSelected,
getImportStatus,
shouldExcludeRepositories,
} from '../../utils/repository-utils';
import { PreviewFile } from '../PreviewFile/PreviewFile';

Expand All @@ -31,7 +34,7 @@ export const CatalogInfoStatus = ({
useFormikContext<AddRepositoriesFormValues>();

React.useEffect(() => {
if (shouldExcludeRepositories(importStatus || '')) {
if (importStatus === RepositoryStatus.ADDED) {
setFieldValue(`excludedRepositories.${data.id}`, {
repoId: data.id,
orgName: data.orgName,
Expand Down Expand Up @@ -71,16 +74,13 @@ export const CatalogInfoStatus = ({
}

if (
values?.repositories?.[data.id]?.catalogInfoYaml?.status ||
importStatus
(!isDrawer && importStatus) ||
(isDrawer &&
importStatus &&
importStatus !== RepositoryStatus.WAIT_PR_APPROVAL)
) {
return (
<span style={{ color: '#6A6E73' }}>
{getImportStatus(
(values?.repositories?.[data.id]?.catalogInfoYaml?.status ||
importStatus) as string,
)}
</span>
<span style={{ color: '#6A6E73' }}>{getImportStatus(importStatus)}</span>
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,8 @@ import TableCell from '@mui/material/TableCell';
import TableRow from '@mui/material/TableRow';

import { bulkImportApiRef } from '../../api/BulkImportBackendClient';
import { AddRepositoryData } from '../../types';
import {
shouldExcludeRepositories,
urlHelper,
} from '../../utils/repository-utils';
import { AddRepositoryData, RepositoryStatus } from '../../types';
import { urlHelper } from '../../utils/repository-utils';
import { CatalogInfoStatus } from './CatalogInfoStatus';

export const RepositoryTableRow = ({
Expand All @@ -39,7 +36,7 @@ export const RepositoryTableRow = ({
data.repoUrl || '',
data?.defaultBranch || 'main',
);
return result.status;
return result;
});

return (
Expand All @@ -55,13 +52,9 @@ export const RepositoryTableRow = ({
disableRipple
color="primary"
checked={
shouldExcludeRepositories((value as string) || '')
? true
: isItemSelected
}
disabled={
loading || shouldExcludeRepositories((value as string) || '')
value?.status === RepositoryStatus.ADDED ? true : isItemSelected
}
disabled={loading || value?.status === RepositoryStatus.ADDED}
onClick={event => handleClick(event, data)}
style={{ padding: '0 12px' }}
/>
Expand Down Expand Up @@ -93,7 +86,7 @@ export const RepositoryTableRow = ({
<TableCell align="left" sx={tableCellStyle}>
<CatalogInfoStatus
data={data}
importStatus={value as string}
importStatus={value?.status as string}
isLoading={loading}
isItemSelected={isItemSelected}
isDrawer={isDrawer}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,6 @@ describe('Select Repositories', () => {
});

it('should allow users to edit repositories if repositories are selected', () => {
const mockAsyncData = {
loading: false,
value: {
totalCount: 5,
},
};
(useAsync as jest.Mock).mockReturnValue(mockAsyncData);
const { getByText, getByTestId } = render(
<BrowserRouter>
<SelectRepositories
Expand Down
57 changes: 57 additions & 0 deletions plugins/bulk-import/src/components/BulkImportPage.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import React from 'react';

import {
RequirePermission,
usePermission,
} from '@backstage/plugin-permission-react';
import { renderInTestApp } from '@backstage/test-utils';

import { screen } from '@testing-library/react';

import { useAddedRepositories } from '../hooks';
import { BulkImportPage } from './BulkImportPage';

jest.mock('@backstage/plugin-permission-react', () => ({
usePermission: jest.fn(),
RequirePermission: jest.fn(),
}));

jest.mock('../hooks/useAddedRepositories', () => ({
useAddedRepositories: jest.fn(),
}));

const mockUsePermission = usePermission as jest.MockedFunction<
typeof usePermission
>;

const mockUseAddedRepositories = useAddedRepositories as jest.MockedFunction<
typeof useAddedRepositories
>;

const RequirePermissionMock = RequirePermission as jest.MockedFunction<
typeof RequirePermission
>;

describe('BulkImport Page', () => {
it('should render if user authorized to access bulk import plugin', async () => {
RequirePermissionMock.mockImplementation(props => <>{props.children}</>);
mockUsePermission.mockReturnValue({ loading: false, allowed: true });
mockUseAddedRepositories.mockReturnValue({
loading: true,
data: [],
retry: jest.fn(),
error: undefined,
});
await renderInTestApp(<BulkImportPage />);
expect(screen.getByText('Added repositories')).toBeInTheDocument();
});

it('should not render if user is not authorized to access the bulk import plugin', async () => {
RequirePermissionMock.mockImplementation(_props => <>Not Found</>);
mockUsePermission.mockReturnValue({ loading: false, allowed: false });

await renderInTestApp(<BulkImportPage />);
expect(screen.getByText('Not Found')).toBeInTheDocument();
expect(screen.queryByText('Added repositories')).not.toBeInTheDocument();
});
});
44 changes: 26 additions & 18 deletions plugins/bulk-import/src/components/BulkImportPage.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import React from 'react';

import { Header, Page, TabbedLayout } from '@backstage/core-components';
import { RequirePermission } from '@backstage/plugin-permission-react';

import FormControl from '@mui/material/FormControl';
import { Formik } from 'formik';

import { bulkImportPermission } from '@janus-idp/backstage-plugin-bulk-import-common';

import {
AddRepositoriesFormValues,
ApprovalTool,
Expand All @@ -22,23 +25,28 @@ export const BulkImportPage = () => {
};

return (
<Page themeId="tool">
<Header title="Bulk import" />
<DeleteDialogContextProvider>
<TabbedLayout>
<TabbedLayout.Route path="/" title="Repositories">
<Formik
initialValues={initialValues}
enableReinitialize
onSubmit={async (_values: AddRepositoriesFormValues) => {}}
>
<FormControl fullWidth>
<RepositoriesList />
</FormControl>
</Formik>
</TabbedLayout.Route>
</TabbedLayout>
</DeleteDialogContextProvider>
</Page>
<RequirePermission
permission={bulkImportPermission}
resourceRef={bulkImportPermission.resourceType}
>
<Page themeId="tool">
<Header title="Bulk import" />
<DeleteDialogContextProvider>
<TabbedLayout>
<TabbedLayout.Route path="/" title="Repositories">
<Formik
initialValues={initialValues}
enableReinitialize
onSubmit={async (_values: AddRepositoriesFormValues) => {}}
>
<FormControl fullWidth>
<RepositoriesList />
</FormControl>
</Formik>
</TabbedLayout.Route>
</TabbedLayout>
</DeleteDialogContextProvider>
</Page>
</RequirePermission>
);
};
Loading