Bug description

We get a forbidden toaster when users without link share permission click on the link icon. Even though this is an expected behavior, a link icon is unnecessary if it does not work. This is creating confusions

How to reproduce the bug

  1. Take away link share permission from the user
  2. Go to any dashboard
  3. Create a new tab in the dashboard.
  4. Click on share link Icon

Screenshots/recordings

Superset version

3.1.3

Python version

3.9

Node version

16

Browser

Chrome

Additional context

No response

Checklist

  • [X] I have searched Superset docs and Slack and didn't find a solution to my problem.
  • [X] I have searched the GitHub issue tracker and didn't find a similar bug report.
  • [X] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

Comment From: dosubot[bot]

Hello, @ssabhijith2000! I'm here to help you with any bugs, questions, or contributions you have for the repository. Let me know how I can assist you!

To address the issue of the forbidden toaster error when a user without link share permission clicks on the link icon in Superset version 3.1.3, you can add a permission check before attempting to generate the permalink. If the user does not have the required permission, you can show an appropriate message or disable the link icon.

Here is an example of how you can modify the ShareMenuItems component to include a permission check:

import React, { useEffect, useState } from 'react';
import { Menu } from 'src/components/Menu';
import { useSelector } from 'react-redux';
import { SupersetClient } from '@superset-ui/core';
import { t } from '@superset-ui/core';
import PropTypes from 'prop-types';

const ShareMenuItems = ({
  addDangerToast,
  addSuccessToast,
  url,
  copyMenuItemTitle,
  emailMenuItemTitle,
  emailSubject,
  emailBody,
  dashboardId,
}) => {
  const [hasPermission, setHasPermission] = useState(false);

  useEffect(() => {
    // Fetch user permissions
    SupersetClient.get({
      endpoint: `/api/v1/dashboard/${dashboardId}/permissions`,
    })
      .then(({ json }) => {
        if (json.permissions.includes('can_share_dashboard')) {
          setHasPermission(true);
        }
      })
      .catch(() => {
        addDangerToast(t('Sorry, something went wrong. Try again later.'));
      });
  }, [dashboardId, addDangerToast]);

  const handleCopyLink = async () => {
    if (!hasPermission) {
      addDangerToast(t('You do not have permission to share this dashboard.'));
      return;
    }

    try {
      const response = await SupersetClient.post({
        endpoint: `/api/v1/dashboard/${dashboardId}/permalink`,
      });
      const { url } = await response.json();
      await copyTextToClipboard(url);
      addSuccessToast(t('Copied to clipboard!'));
    } catch (error) {
      addDangerToast(t('Sorry, something went wrong. Try again later.'));
    }
  };

  return (
    <Menu onClick={jest.fn()} selectable={false} data-test="main-menu">
      <Menu.Item key="copy-link" onClick={handleCopyLink}>
        {copyMenuItemTitle}
      </Menu.Item>
      <Menu.Item key="share-email">
        <a
          href={`mailto:?Subject=${emailSubject}&Body=${emailBody}${url}`}
          target="_blank"
          rel="noopener noreferrer"
        >
          {emailMenuItemTitle}
        </a>
      </Menu.Item>
    </Menu>
  );
};

ShareMenuItems.propTypes = {
  addDangerToast: PropTypes.func.isRequired,
  addSuccessToast: PropTypes.func.isRequired,
  url: PropTypes.string.isRequired,
  copyMenuItemTitle: PropTypes.string.isRequired,
  emailMenuItemTitle: PropTypes.string.isRequired,
  emailSubject: PropTypes.string.isRequired,
  emailBody: PropTypes.string.isRequired,
  dashboardId: PropTypes.string.isRequired,
};

export default ShareMenuItems;

Additionally, you can refer to the URLShortLinkButton component in the Superset codebase for a similar implementation. This component includes a mechanism to handle the sharing of dashboard links:

import { useState } from 'react';
import { getClientErrorObject, t } from '@superset-ui/core';
import Popover, { PopoverProps } from 'src/components/Popover';
import CopyToClipboard from 'src/components/CopyToClipboard';
import { getDashboardPermalink } from 'src/utils/urlUtils';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import { useSelector } from 'react-redux';
import { RootState } from 'src/dashboard/types';

export type URLShortLinkButtonProps = {
  dashboardId: number;
  anchorLinkId?: string;
  emailSubject?: string;
  emailContent?: string;
  placement?: PopoverProps['placement'];
};

export default function URLShortLinkButton({
  dashboardId,
  anchorLinkId,
  placement = 'right',
  emailContent = '',
  emailSubject = '',
}: URLShortLinkButtonProps) {
  const [shortUrl, setShortUrl] = useState('');
  const { addDangerToast } = useToasts();
  const { dataMask, activeTabs } = useSelector((state: RootState) => ({
    dataMask: state.dataMask,
    activeTabs: state.dashboardState.activeTabs,
  }));

  const getCopyUrl = async () => {
    try {
      const url = await getDashboardPermalink({
        dashboardId,
        dataMask,
        activeTabs,
        anchor: anchorLinkId,
      });
      setShortUrl(url);
    } catch (error) {
      if (error) {
        addDangerToast(
          (await getClientErrorObject(error)).error ||
            t('Something went wrong.'),
        );
      }
    }
  };

  const emailBody = `${emailContent}${shortUrl || ''}`;
  const emailLink = `mailto:?Subject=${emailSubject}%20&Body=${emailBody}`;

  return (
    <Popover
      trigger="click"
      placement={placement}
      content={
        <div
          id="shorturl-popover"
          data-test="shorturl-popover"
          onClick={e => {
            e.stopPropagation();
          }}
        >
          <CopyToClipboard
            text={shortUrl}
            copyNode={
              <i className="fa fa-clipboard" title={t('Copy to clipboard')} />
            }
          />
          &nbsp;&nbsp;
          <a href={emailLink}>
            <i className="fa fa-envelope" />
          </a>
        </div>
      }
    >
      <span
        className="short-link-trigger btn btn-default btn-sm"
        tabIndex={-1}
        role="button"
        onClick={e => {
          e.stopPropagation();
          getCopyUrl();
        }}
      >
        <i className="short-link-trigger fa fa-link" />
        &nbsp;
      </span>
    </Popover>
  );
}

In this example, the getCopyUrl function handles the URL generation and error handling. This approach ensures that users without the necessary permissions are informed appropriately and prevents the forbidden toaster error [1][2][3][4].

To continue talking to Dosu, mention @dosu.

Comment From: rusackas

It looks like Dosu is on the right path here... would you be willing to open a PR that renders the link conditionally based on permission rather than letting the user try/fail upon click?

Comment From: rusackas

This probably could/should be addressed in a similar fashion to https://github.com/apache/superset/pull/32995 (on which we're still aligning on the implementation plan, so let's merge that first)

Comment From: rusackas

We're still open to a PR here, if anyone is interested, but otherwise we might consider this 'not planned' or move it to a Discussion of the "Ideas" variety.