You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by am...@apache.org on 2021/03/03 10:32:18 UTC

[superset] branch master updated: feat(dashboard_rbac): manage roles for dashboard (#13145)

This is an automated email from the ASF dual-hosted git repository.

amitmiran pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new dc17039  feat(dashboard_rbac): manage roles for dashboard (#13145)
dc17039 is described below

commit dc170397c595d1357a2afa5b2410e37cf82c8311
Author: simcha90 <56...@users.noreply.github.com>
AuthorDate: Wed Mar 3 12:31:41 2021 +0200

    feat(dashboard_rbac): manage roles for dashboard (#13145)
    
    * feat(dashboard_rbac): manage roles for dashboard
    
    * test: fix tests
    
    * fix: fix empty roles
    
    Co-authored-by: Amit Miran <47...@users.noreply.github.com>
---
 .../dashboard/components/PropertiesModal_spec.jsx  |  24 +++
 .../views/CRUD/dashboard/DashboardList_spec.jsx    |   1 +
 .../src/dashboard/components/PropertiesModal.jsx   | 163 +++++++++++++++++----
 superset-frontend/src/featureFlags.ts              |   1 +
 4 files changed, 157 insertions(+), 32 deletions(-)

diff --git a/superset-frontend/spec/javascripts/dashboard/components/PropertiesModal_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/PropertiesModal_spec.jsx
index 07dcf7d..92f7887 100644
--- a/superset-frontend/spec/javascripts/dashboard/components/PropertiesModal_spec.jsx
+++ b/superset-frontend/spec/javascripts/dashboard/components/PropertiesModal_spec.jsx
@@ -38,6 +38,7 @@ const dashboardResult = {
       slug: '/new',
       json_metadata: '{"something":"foo"}',
       owners: [],
+      roles: [],
     },
   },
 };
@@ -54,6 +55,7 @@ fetchMock.get('glob:*/api/v1/dashboard/*', {
     slug: '/new',
     json_metadata: '{"something":"foo"}',
     owners: [],
+    roles: [],
   },
 });
 
@@ -207,6 +209,7 @@ describe('PropertiesModal', () => {
             slug: '/new',
             json_metadata: '{"something":"foo"}',
             owners: [{ id: 1, first_name: 'Al', last_name: 'Pacino' }],
+            roles: [],
           },
         },
       });
@@ -219,6 +222,27 @@ describe('PropertiesModal', () => {
       ]);
     });
 
+    it('should call onRolesChange', async () => {
+      const wrapper = setup();
+      const modalInstance = wrapper.find('PropertiesModal').instance();
+      const fetchSpy = jest.spyOn(SupersetClient, 'get').mockResolvedValue({
+        json: {
+          result: {
+            dashboard_title: 'New Title',
+            slug: '/new',
+            json_metadata: '{"something":"foo"}',
+            owners: [],
+            roles: [{ id: 1, name: 'Alpha' }],
+          },
+        },
+      });
+      const onRolwesSpy = jest.spyOn(modalInstance, 'onRolesChange');
+      modalInstance.fetchDashboardDetails();
+      await fetchSpy();
+      expect(modalInstance.state.values.colorScheme).toBeUndefined();
+      expect(onRolwesSpy).toHaveBeenCalledWith([{ value: 1, label: 'Alpha' }]);
+    });
+
     describe('when colorScheme is undefined as a prop', () => {
       describe('when color_scheme is defined in json_metadata', () => {
         const wrapper = setup();
diff --git a/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx
index 2af4dc1..c8d2686 100644
--- a/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx
+++ b/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx
@@ -56,6 +56,7 @@ const mockDashboards = [...new Array(3)].map((_, i) => ({
   changed_on_utc: new Date().toISOString(),
   changed_on_delta_humanized: '5 minutes ago',
   owners: [{ id: 1, first_name: 'admin', last_name: 'admin_user' }],
+  roles: [{ id: 1, name: 'adminUser' }],
   thumbnail_url: '/thumbnail',
 }));
 
diff --git a/superset-frontend/src/dashboard/components/PropertiesModal.jsx b/superset-frontend/src/dashboard/components/PropertiesModal.jsx
index 411eda0..481893f 100644
--- a/superset-frontend/src/dashboard/components/PropertiesModal.jsx
+++ b/superset-frontend/src/dashboard/components/PropertiesModal.jsx
@@ -38,6 +38,7 @@ import ColorSchemeControlWrapper from 'src/dashboard/components/ColorSchemeContr
 import { getClientErrorObject } from '../../utils/getClientErrorObject';
 import withToasts from '../../messageToasts/enhancers/withToasts';
 import '../stylesheets/buttons.less';
+import { FeatureFlag, isFeatureEnabled } from '../../featureFlags';
 
 const StyledJsonEditor = styled(JsonEditor)`
   border-radius: ${({ theme }) => theme.borderRadius}px;
@@ -85,10 +86,10 @@ const handleErrorResponse = async response => {
   });
 };
 
-const loadOwnerOptions = (input = '') => {
+const loadAccessOptions = accessType => (input = '') => {
   const query = rison.encode({ filter: input });
   return SupersetClient.get({
-    endpoint: `/api/v1/dashboard/related/owners?q=${query}`,
+    endpoint: `/api/v1/dashboard/related/${accessType}?q=${query}`,
   }).then(
     response =>
       response.json.result.map(item => ({
@@ -111,6 +112,7 @@ class PropertiesModal extends React.PureComponent {
         dashboard_title: '',
         slug: '',
         owners: [],
+        roles: [],
         json_metadata: '',
         colorScheme: props.colorScheme,
       },
@@ -120,9 +122,12 @@ class PropertiesModal extends React.PureComponent {
     this.onChange = this.onChange.bind(this);
     this.onMetadataChange = this.onMetadataChange.bind(this);
     this.onOwnersChange = this.onOwnersChange.bind(this);
+    this.onRolesChange = this.onRolesChange.bind(this);
     this.submit = this.submit.bind(this);
     this.toggleAdvanced = this.toggleAdvanced.bind(this);
     this.onColorSchemeChange = this.onColorSchemeChange.bind(this);
+    this.getRowsWithRoles = this.getRowsWithRoles.bind(this);
+    this.getRowsWithoutRoles = this.getRowsWithoutRoles.bind(this);
   }
 
   componentDidMount() {
@@ -163,6 +168,10 @@ class PropertiesModal extends React.PureComponent {
     this.updateFormState('owners', value);
   }
 
+  onRolesChange(value) {
+    this.updateFormState('roles', value);
+  }
+
   onMetadataChange(metadata) {
     this.updateFormState('json_metadata', metadata);
   }
@@ -202,7 +211,12 @@ class PropertiesModal extends React.PureComponent {
         value: owner.id,
         label: `${owner.first_name} ${owner.last_name}`,
       }));
+      const initialSelectedRoles = dashboard.roles.map(role => ({
+        value: role.id,
+        label: `${role.name}`,
+      }));
       this.onOwnersChange(initialSelectedOwners);
+      this.onRolesChange(initialSelectedRoles);
     }, handleErrorResponse);
   }
 
@@ -231,10 +245,12 @@ class PropertiesModal extends React.PureComponent {
         dashboard_title: dashboardTitle,
         colorScheme,
         owners: ownersValue,
+        roles: rolesValue,
       },
     } = this.state;
     const { onlyApply } = this.props;
-    const owners = ownersValue.map(o => o.value);
+    const owners = ownersValue?.map(o => o.value) ?? [];
+    const roles = rolesValue?.map(o => o.value) ?? [];
     let metadataColorScheme;
 
     // update color scheme to match metadata
@@ -247,6 +263,12 @@ class PropertiesModal extends React.PureComponent {
       }
     }
 
+    const moreProps = {};
+    const morePutProps = {};
+    if (isFeatureEnabled(FeatureFlag.DASHBOARD_RBAC)) {
+      moreProps.rolesIds = roles;
+      morePutProps.roles = roles;
+    }
     if (onlyApply) {
       this.props.onSubmit({
         id: this.props.dashboardId,
@@ -255,6 +277,7 @@ class PropertiesModal extends React.PureComponent {
         jsonMetadata,
         ownerIds: owners,
         colorScheme: metadataColorScheme || colorScheme,
+        ...moreProps,
       });
       this.props.onHide();
     } else {
@@ -266,8 +289,13 @@ class PropertiesModal extends React.PureComponent {
           slug: slug || null,
           json_metadata: jsonMetadata || null,
           owners,
+          ...morePutProps,
         }),
       }).then(({ json: { result } }) => {
+        const moreResultProps = {};
+        if (isFeatureEnabled(FeatureFlag.DASHBOARD_RBAC)) {
+          moreResultProps.rolesIds = result.roles;
+        }
         this.props.addSuccessToast(t('The dashboard has been saved'));
         this.props.onSubmit({
           id: this.props.dashboardId,
@@ -276,12 +304,109 @@ class PropertiesModal extends React.PureComponent {
           jsonMetadata: result.json_metadata,
           ownerIds: result.owners,
           colorScheme: metadataColorScheme || colorScheme,
+          ...moreResultProps,
         });
         this.props.onHide();
       }, handleErrorResponse);
     }
   }
 
+  getRowsWithoutRoles() {
+    const { values, isDashboardLoaded } = this.state;
+    return (
+      <Row>
+        <Col md={6}>
+          <h3 style={{ marginTop: '1em' }}>{t('Access')}</h3>
+          <FormLabel htmlFor="owners">{t('Owners')}</FormLabel>
+          <AsyncSelect
+            name="owners"
+            isMulti
+            value={values.owners}
+            loadOptions={loadAccessOptions('owners')}
+            defaultOptions // load options on render
+            cacheOptions
+            onChange={this.onOwnersChange}
+            disabled={!isDashboardLoaded}
+            filterOption={null} // options are filtered at the api
+          />
+          <p className="help-block">
+            {t(
+              'Owners is a list of users who can alter the dashboard. Searchable by name or username.',
+            )}
+          </p>
+        </Col>
+        <Col md={6}>
+          <h3 style={{ marginTop: '1em' }}>{t('Colors')}</h3>
+          <ColorSchemeControlWrapper
+            onChange={this.onColorSchemeChange}
+            colorScheme={values.colorScheme}
+          />
+        </Col>
+      </Row>
+    );
+  }
+
+  getRowsWithRoles() {
+    const { values, isDashboardLoaded } = this.state;
+    return (
+      <>
+        <Row>
+          <Col md={12}>
+            <h3 style={{ marginTop: '1em' }}>{t('Access')}</h3>
+          </Col>
+        </Row>
+        <Row>
+          <Col md={6}>
+            <FormLabel htmlFor="owners">{t('Owners')}</FormLabel>
+            <AsyncSelect
+              name="owners"
+              isMulti
+              value={values.owners}
+              loadOptions={loadAccessOptions('owners')}
+              defaultOptions // load options on render
+              cacheOptions
+              onChange={this.onOwnersChange}
+              disabled={!isDashboardLoaded}
+              filterOption={null} // options are filtered at the api
+            />
+            <p className="help-block">
+              {t(
+                'Owners is a list of users who can alter the dashboard. Searchable by name or username.',
+              )}
+            </p>
+          </Col>
+          <Col md={6}>
+            <FormLabel htmlFor="roles">{t('Roles')}</FormLabel>
+            <AsyncSelect
+              name="roles"
+              isMulti
+              value={values.roles}
+              loadOptions={loadAccessOptions('roles')}
+              defaultOptions // load options on render
+              cacheOptions
+              onChange={this.onRolesChange}
+              disabled={!isDashboardLoaded}
+              filterOption={null} // options are filtered at the api
+            />
+            <p className="help-block">
+              {t(
+                'Roles is a list which defines access to the dashboard. These roles are always applied in addition to restrictions on dataset level access. If no roles defined then the dashboard is available to all roles.',
+              )}
+            </p>
+          </Col>
+        </Row>
+        <Row>
+          <Col md={6}>
+            <ColorSchemeControlWrapper
+              onChange={this.onColorSchemeChange}
+              colorScheme={values.colorScheme}
+            />
+          </Col>
+        </Row>
+      </>
+    );
+  }
+
   render() {
     const { values, isDashboardLoaded, isAdvancedOpen, errors } = this.state;
     const { onHide, onlyApply } = this.props;
@@ -352,35 +477,9 @@ class PropertiesModal extends React.PureComponent {
               </p>
             </Col>
           </Row>
-          <Row>
-            <Col md={6}>
-              <h3 style={{ marginTop: '1em' }}>{t('Access')}</h3>
-              <FormLabel htmlFor="owners">{t('Owners')}</FormLabel>
-              <AsyncSelect
-                name="owners"
-                isMulti
-                value={values.owners}
-                loadOptions={loadOwnerOptions}
-                defaultOptions // load options on render
-                cacheOptions
-                onChange={this.onOwnersChange}
-                disabled={!isDashboardLoaded}
-                filterOption={null} // options are filtered at the api
-              />
-              <p className="help-block">
-                {t(
-                  'Owners is a list of users who can alter the dashboard. Searchable by name or username.',
-                )}
-              </p>
-            </Col>
-            <Col md={6}>
-              <h3 style={{ marginTop: '1em' }}>{t('Colors')}</h3>
-              <ColorSchemeControlWrapper
-                onChange={this.onColorSchemeChange}
-                colorScheme={values.colorScheme}
-              />
-            </Col>
-          </Row>
+          {isFeatureEnabled(FeatureFlag.DASHBOARD_RBAC)
+            ? this.getRowsWithRoles()
+            : this.getRowsWithoutRoles()}
           <Row>
             <Col md={12}>
               <h3 style={{ marginTop: '1em' }}>
diff --git a/superset-frontend/src/featureFlags.ts b/superset-frontend/src/featureFlags.ts
index 7cf3c1a..6c95213 100644
--- a/superset-frontend/src/featureFlags.ts
+++ b/superset-frontend/src/featureFlags.ts
@@ -36,6 +36,7 @@ export enum FeatureFlag {
   ESCAPE_MARKDOWN_HTML = 'ESCAPE_MARKDOWN_HTML',
   DASHBOARD_NATIVE_FILTERS = 'DASHBOARD_NATIVE_FILTERS',
   DASHBOARD_CROSS_FILTERS = 'DASHBOARD_CROSS_FILTERS',
+  DASHBOARD_RBAC = 'DASHBOARD_RBAC',
   DASHBOARD_NATIVE_FILTERS_SET = 'DASHBOARD_NATIVE_FILTERS_SET',
   VERSIONED_EXPORT = 'VERSIONED_EXPORT',
   GLOBAL_ASYNC_QUERIES = 'GLOBAL_ASYNC_QUERIES',