-
Notifications
You must be signed in to change notification settings - Fork 86
Add access control tab content #992
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
Changes from 5 commits
85619c8
4c720e2
3298dad
5337b71
9feb7dd
14f3ff2
7c40489
c5b6d51
fc96304
f3ff2f9
8fd7211
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { EuiRadioGroupOption } from '@elastic/eui'; | ||
|
|
||
| export interface PermissionsFlexItem { | ||
| roles: Array<{ label: string }>; | ||
| selectedRoles: Array<{ label: string }>; | ||
| selectedRadio: string; | ||
| setSelectedRoles: (selectedRoles: Array<{ label: string }>) => void; | ||
| setSelectedRadio: (selectedRadio: string) => void; | ||
| radios: EuiRadioGroupOption[]; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { | ||
| EuiComboBox, | ||
| EuiFlexGroup, | ||
| EuiFlexItem, | ||
| EuiLink, | ||
| EuiRadioGroup, | ||
| EuiSpacer, | ||
| EuiText, | ||
| } from '@elastic/eui'; | ||
| import React from 'react'; | ||
| import { PermissionsFlexItem } from '../../../../common/types/data_connections'; | ||
| import { | ||
| OPENSEARCH_DOCUMENTATION_URL, | ||
| QUERY_ALL, | ||
| } from '../../../../common/constants/data_connections'; | ||
|
|
||
| export const AccelerationPermissionsFlexItem = (props: PermissionsFlexItem) => { | ||
| const { roles, setSelectedRoles, selectedRoles, selectedRadio, radios, setSelectedRadio } = props; | ||
| return ( | ||
| <EuiFlexItem> | ||
| <EuiFlexGroup direction="row"> | ||
| <EuiFlexItem> | ||
| <EuiText className="overview-title">Acceleration Permissions</EuiText> | ||
| <EuiText size="s" className="overview-content"> | ||
| Control which OpenSearch roles have permissions to accelerate external data through | ||
| OpenSearch indexing.{' '} | ||
| <EuiLink external={true} href={OPENSEARCH_DOCUMENTATION_URL} target="_blank"> | ||
| Learn more | ||
| </EuiLink> | ||
| </EuiText> | ||
| </EuiFlexItem> | ||
| <EuiFlexItem> | ||
| <EuiRadioGroup | ||
| options={radios} | ||
| idSelected={selectedRadio} | ||
| onChange={(id) => setSelectedRadio(id)} | ||
| name="acceleration-radio-group" | ||
| legend={{ | ||
| children: <span>Access level</span>, | ||
| }} | ||
| /> | ||
| {selectedRadio === QUERY_ALL ? ( | ||
| <div> | ||
| <EuiSpacer size="s" /> | ||
| <EuiText>OpenSearch Roles</EuiText> | ||
| <EuiText size="xs"> | ||
| Select one or more OpenSearch roles that can query this data connection. | ||
| </EuiText> | ||
| <EuiComboBox | ||
| placeholder="Select one or more options" | ||
| options={roles} | ||
| selectedOptions={selectedRoles} | ||
| onChange={setSelectedRoles} | ||
| isClearable={true} | ||
| data-test-subj="query-permissions-combo-box" | ||
| /> | ||
| </div> | ||
| ) : ( | ||
| <></> | ||
| )} | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| </EuiFlexItem> | ||
| ); | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { EuiCallOut } from '@elastic/eui'; | ||
| import React from 'react'; | ||
|
|
||
| export const AccessControlCallout = () => { | ||
| return ( | ||
| <EuiCallOut title="Configurations may be managed elsewhere." iconType="iInCircle"> | ||
| Access to data can be managed in other systems outside of OpenSearch. Check with your | ||
| administrator for additional configurations. | ||
| </EuiCallOut> | ||
| ); | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,205 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { | ||
| EuiButton, | ||
| EuiFlexGroup, | ||
| EuiFlexItem, | ||
| EuiSpacer, | ||
| EuiText, | ||
| EuiHorizontalRule, | ||
| EuiBottomBar, | ||
| EuiButtonEmpty, | ||
| } from '@elastic/eui'; | ||
| import React, { useEffect, useState } from 'react'; | ||
| import { EuiPanel } from '@elastic/eui'; | ||
| import { | ||
| ACCELERATION_ALL, | ||
| ACCELERATION_RESTRICT, | ||
| QUERY_ALL, | ||
| QUERY_RESTRICT, | ||
| } from 'common/constants/data_connections'; | ||
| import { AccessControlCallout } from './access_control_callout'; | ||
| import { coreRefs } from '../../../../public/framework/core_refs'; | ||
| import { QueryPermissionsFlexItem } from './query_permissions_flex_item'; | ||
| import { AccelerationPermissionsFlexItem } from './acceleration_permissions_flex_item'; | ||
|
|
||
| export const AccessControlTab = () => { | ||
| const [mode, setMode] = useState<'view' | 'edit'>('view'); | ||
| const [roles, setRoles] = useState<Array<{ label: string }>>([]); | ||
| const [selectedQueryPermissionRoles, setSelectedQueryPermissionRoles] = useState< | ||
| Array<{ label: string }> | ||
| >([]); | ||
| const [selectedAccelerationPermissionRoles, setSelectedAccelerationPermissionRoles] = useState< | ||
| Array<{ label: string }> | ||
| >([]); | ||
| const [queryPermissionRadioSelected, setQueryRadioIdSelected] = useState(`query-1`); | ||
| const [accelerationPermissionRadioSelected, setAccelerationRadioIdSelected] = useState( | ||
| `acceleration-1` | ||
| ); | ||
|
|
||
| const [finalQueryPermissionsPlaceHolder, setFinalQueryPermissionsPlaceHolder] = useState< | ||
| Array<{ label: string }> | ||
| >([]); | ||
| const [ | ||
| finalAccelerationPermissionsPlaceHolder, | ||
| setFinalAccelerationPermissionsPlaceHolder, | ||
| ] = useState<Array<{ label: string }>>([]); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Placeholder to update the state (this will be replaced with API call to POST, and then showing the list will be the result of the API call to GET) |
||
|
|
||
| useEffect(() => { | ||
| coreRefs.http!.get('/api/v1/configuration/roles').then((data) => | ||
| setRoles( | ||
| Object.keys(data.data).map((key) => { | ||
| return { label: key }; | ||
| }) | ||
| ) | ||
| ); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assumes security plugin is installed and user has permission to access it (will be addressed with the permission PR)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if security plugin is not installed. Are we not showing the whole data connections plugin? or Do we have a fallback view?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think current plan is to show as much as possible:
|
||
| }, []); | ||
|
|
||
| const queryRadios = [ | ||
| { | ||
| id: QUERY_RESTRICT, | ||
| label: 'Restricted - accessible by users with specific OpenSearch roles', | ||
| }, | ||
| { | ||
| id: QUERY_ALL, | ||
| label: 'Everyone - accessible by all users on this cluster', | ||
| }, | ||
| ]; | ||
| const accelerationRadios = [ | ||
| { | ||
| id: ACCELERATION_RESTRICT, | ||
| label: 'Restricted - accessible by users with specific OpenSearch roles', | ||
| }, | ||
| { | ||
| id: ACCELERATION_ALL, | ||
| label: 'Everyone - accessible by all users on this cluster', | ||
| }, | ||
| ]; | ||
|
|
||
| const renderViewAccessControlDetails = () => { | ||
| return ( | ||
| <EuiFlexGroup> | ||
| <EuiFlexItem> | ||
| <EuiFlexGroup direction="column"> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiText className="overview-title">Query access</EuiText> | ||
| <EuiText size="s" className="overview-content"> | ||
| {finalQueryPermissionsPlaceHolder.length | ||
| ? `Restricted to ${finalQueryPermissionsPlaceHolder.map((role) => role.label)}` | ||
| : '-'} | ||
| </EuiText> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| </EuiFlexItem> | ||
| <EuiFlexItem> | ||
| <EuiFlexGroup direction="column"> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiText className="overview-title">Acceleration permissions</EuiText> | ||
| <EuiText size="s" className="overview-content"> | ||
| {finalAccelerationPermissionsPlaceHolder.length | ||
| ? `Restricted to ${finalAccelerationPermissionsPlaceHolder.map( | ||
| (role) => role.label | ||
| )}` | ||
| : '-'} | ||
| </EuiText> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| ); | ||
| }; | ||
|
|
||
| const renderEditAccessControlDetails = () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please turn this into a formal React FC.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to address this in: fc96304, please let me know if I understood what you meant correctly. |
||
| return ( | ||
| <EuiFlexGroup direction="column"> | ||
| <QueryPermissionsFlexItem | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do not call these components "...FlexItem". The component's purpose is not a consequence of it's inclusion in a flex-group.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree. Done in: fc96304, thanks for the suggestion. |
||
| roles={roles} | ||
| selectedRoles={selectedQueryPermissionRoles} | ||
| setSelectedRoles={setSelectedQueryPermissionRoles} | ||
| selectedRadio={queryPermissionRadioSelected} | ||
| setSelectedRadio={setQueryRadioIdSelected} | ||
| radios={queryRadios} | ||
| /> | ||
| <AccelerationPermissionsFlexItem | ||
| roles={roles} | ||
| selectedRoles={selectedAccelerationPermissionRoles} | ||
| setSelectedRoles={setSelectedAccelerationPermissionRoles} | ||
| selectedRadio={accelerationPermissionRadioSelected} | ||
| setSelectedRadio={setAccelerationRadioIdSelected} | ||
| radios={accelerationRadios} | ||
| /> | ||
| </EuiFlexGroup> | ||
| ); | ||
| }; | ||
|
|
||
| const saveChanges = () => { | ||
| setFinalAccelerationPermissionsPlaceHolder(selectedAccelerationPermissionRoles); | ||
| setFinalQueryPermissionsPlaceHolder(selectedQueryPermissionRoles); | ||
| setMode('view'); | ||
| }; | ||
|
|
||
| return ( | ||
| <> | ||
| <EuiSpacer /> | ||
| <AccessControlCallout /> | ||
| <EuiSpacer /> | ||
| <EuiPanel> | ||
| <EuiFlexGroup direction="row"> | ||
| <EuiFlexItem> | ||
| <EuiText size="m"> | ||
| <h2 className="panel-title">Access Control</h2> | ||
| Control which OpenSearch users have access to this data source. | ||
| </EuiText> | ||
| </EuiFlexItem> | ||
|
|
||
| <EuiFlexItem grow={false}> | ||
| <EuiButton | ||
| data-test-subj="createButton" | ||
| onClick={ | ||
| mode === 'view' | ||
| ? () => { | ||
| setMode('edit'); | ||
| } | ||
| : () => { | ||
| setMode('view'); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can make it more readable or change it to a function.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in: fc96304, thanks! |
||
| } | ||
| fill={mode === 'view' ? true : false} | ||
| > | ||
| {mode === 'view' ? 'Edit' : 'Cancel'} | ||
| </EuiButton> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| <EuiHorizontalRule /> | ||
| {mode === 'view' ? renderViewAccessControlDetails() : renderEditAccessControlDetails()} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider setting this value to a component-constant ( Containing these conditions into one location will also raise the visibility of refactoring / simplification opportunities.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not quite sure I understand what you are suggesting/how it would be cleaner than having a state variable. Can you point me to some examples I can look at? |
||
| </EuiPanel> | ||
| <EuiSpacer /> | ||
| {mode === 'edit' ? ( | ||
| <EuiBottomBar affordForDisplacement={false}> | ||
| <EuiFlexGroup justifyContent="flexEnd"> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiButtonEmpty | ||
| onClick={() => { | ||
| setMode('view'); | ||
| }} | ||
| color="ghost" | ||
| size="s" | ||
| iconType="cross" | ||
| > | ||
| Discard change(s) | ||
| </EuiButtonEmpty> | ||
| </EuiFlexItem> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiButton onClick={saveChanges} size="s" iconType="check" fill> | ||
| Save | ||
| </EuiButton> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| </EuiBottomBar> | ||
| ) : null} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor, this can be changed to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in: fc96304, thanks! |
||
| </> | ||
| ); | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, but Ideally types here should be something like:
React.Dispatch<React.SetStateAction<Array<{ label: string }>>There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in: fc96304, thanks! I didn't know this