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

fix: [M3-8590] - Restrict access to Database Create page for restricted users #11137

Merged

Conversation

zaenab-akamai
Copy link
Contributor

@zaenab-akamai zaenab-akamai commented Oct 22, 2024

Description 📝

Discovered during release testing for v1.128.0:

A recent PR updated the UI for users who have restricted access (read only or no access) to Databases: #10794

This included disabling buttons and providing helper tooltip text to explain the reason why restricted users couldn't access these features.

The PR did not include the same treatment for the databases/create page, which a restricted user can still navigate to via the URL (https://cloud.linode.com/databases/create) or via the top menu's Create button. The restricted user is able to fill out the form and submit it, and only then do they receive an error.

Thank you @mjac0bs for your inputs! :)

Changes 🔄

List any change relevant to the reviewer.

  • Added an error message on the create database page for restricted users
  • Disabled all controls on /databases/create
  • Updated DatabaseCreateAccessControls to disable inputs through a prop

Target release date 🗓️

NA

Preview 📷

Screenshot 2024-10-23 at 4 29 20 PM

Screenshot 2024-10-23 at 4 29 30 PM

How to test 🧪

Reproduction steps

  1. Login as a user with full access and add databases. Grant read_only permissions to these new resources to the restricted user
  2. Login as the restricted user and navigate to the databases page. The "Create Database Cluster" button should be disabled, but if you go to directly to https://cloud.linode.com/databases/create via the URL or via the top menu, notice that the form is accessible.

Verification steps

Pull the PR, login as a restricted.
Navigate to /databases/create via the URL or via the top menu. The form should be disabled and an error notice should appear on top.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@mjac0bs mjac0bs added the Restricted User Access Improve UX surrounding restricted access to features label Oct 22, 2024
@zaenab-akamai zaenab-akamai marked this pull request as ready for review October 23, 2024 11:06
@zaenab-akamai zaenab-akamai requested a review from a team as a code owner October 23, 2024 11:06
@zaenab-akamai zaenab-akamai requested review from dwiley-akamai and mjac0bs and removed request for a team October 23, 2024 11:06
Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

Access to Database Create page restricted appropriately for restricted users with no adverse impact on non-restricted users ✅

We'll want to get a changeset added for this PR

@@ -570,6 +590,8 @@ const DatabaseCreate = () => {
}}
className={classes.selectPlanPanel}
data-qa-select-plan
disabled={isRestricted}
disabledTabs={isRestricted ? ['shared', 'dedicated'] : []}
Copy link
Contributor

@dwiley-akamai dwiley-akamai Oct 23, 2024

Choose a reason for hiding this comment

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

I would be in favor in getting right of this line, right now there's odd behavior where you can click "Premium CPU" as a restricted user but not be able to click back to the "Dedicated CPU" tab.

Even without this line, a restricted user wouldn't be able to select a plan, and the only way they'd even be able to reach the page is by navigating to it directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Looks good - error message is visible for restricted users and form fields are disabled.

Other than Dajahi's comments (we can categorize a Changed changeset), this should be good to go. Thanks @zaenab-akamai!

@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Oct 23, 2024
@zaenab-akamai
Copy link
Contributor Author

Added the changeset. I missed this - thanks for pointing it out @dwiley-akamai !
@mjac0bs I marked it as a fix since this was meant to fix a bug on a change released earlier. Let me know if you still prefer Changed. I'll update the PR title as well to keep things consistent

Copy link

Coverage Report:
Base Coverage: 86.99%
Current Coverage: 86.99%

@mjac0bs
Copy link
Contributor

mjac0bs commented Oct 25, 2024

@mjac0bs I marked it as a fix since this was meant to fix a bug on a change released earlier. Let me know if you still prefer Changed. I'll update the PR title as well to keep things consistent

@zaenab-akamai Good call on Fixed since there was a previous PR for this. That works for me. 👍🏼

@zaenab-akamai zaenab-akamai merged commit b5da866 into linode:develop Oct 28, 2024
20 checks passed
Copy link

cypress bot commented Oct 28, 2024

Cloud Manager E2E    Run #6736

Run Properties:  status check passed Passed #6736  •  git commit b5da866ae2: fix: [M3-8590] - Restrict access to Database Create page for restricted users (#...
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6736
Run duration 26m 18s
Commit git commit b5da866ae2: fix: [M3-8590] - Restrict access to Database Create page for restricted users (#...
Committer zaenab-akamai
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 445
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Restricted User Access Improve UX surrounding restricted access to features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants