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

Disable tab links if unauthorized #3260

Merged
merged 3 commits into from
May 22, 2024

Conversation

adrw
Copy link
Collaborator

@adrw adrw commented May 9, 2024

Currently, even if a caller is unauthorized to access a tab, they can click the link in the menu and navigate to it. When they do, they encounter the non-UI Misk standard text unauthorized. When they try to navigate back, it does not force a full page load due to the Turbo pre-load enhancements on the base links.

This PR disables the links (they route to the dashboard home which may have access information) and provides on hover tooltip text to explain why the link is not clickable. This will avoid this hard edge case while preserving the pre-loading benefits of Turbo links for tabs that the caller is authorized to access.

Screenshot 2024-05-16 at 09 38 52

Copy link
Collaborator

@uayyaz uayyaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@adrw adrw force-pushed the adrw.2024-05-09.disabledIfUnauthorizedMenu branch 2 times, most recently from f5c7b71 to 417699c Compare May 14, 2024 17:52
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@adrw adrw force-pushed the adrw.2024-05-09.disabledIfUnauthorizedMenu branch 3 times, most recently from 0f881d5 to e849cb5 Compare May 16, 2024 07:40
@adrw adrw changed the title WIP DNR Disable tab links if unauthorized Disable tab links if unauthorized May 16, 2024
@adrw adrw force-pushed the adrw.2024-05-09.disabledIfUnauthorizedMenu branch from e849cb5 to 3f8f8b2 Compare May 16, 2024 07:41
@adrw adrw force-pushed the adrw.2024-05-09.disabledIfUnauthorizedMenu branch from 3f8f8b2 to c5d0042 Compare May 16, 2024 07:44
@adrw adrw enabled auto-merge (squash) May 16, 2024 07:45
@adrw
Copy link
Collaborator Author

adrw commented May 20, 2024

  • Investigate setting up an exception mapper to catch 401s for /_admin/ requests instead of this custom logic which then serves a more helpful 401 page, this will also cover cases where someone directly accesses the tab from a runbook link which they don't have access to

@adrw adrw merged commit c31e57e into master May 22, 2024
12 checks passed
@adrw adrw deleted the adrw.2024-05-09.disabledIfUnauthorizedMenu branch May 22, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants