feat: [M3-7490] - Improve NodeBalancer Restricted User Experience #10095

jaalah-akamai · 2024-01-22T22:04:01Z

Description 📝

To prevent unauthorized access to specific flows and provide clearer guidance, we aim to restrict entry to users without the required permissions while providing new notices.

Tooltips specifically designed for disabled buttons will persist within action menus. These tooltips prove valuable in scenarios where a restricted user has permission to delete their personal resources but lacks the permission to delete those belonging to the admin user. In such cases, we refrain from displaying notices on the page.

For restricted users:

Error Notices:
- Drawers - Error notifications without an icon:
- Create Pages - Error notifications with an icon
Warning Notices w/ Icon:
- Detail Pages
Disabled Create Button
- Empty State Landing Pages
- Landing

Changes 🔄

Added useIsResourceRestricted hook that verifies if a user with restricted access can edit specific cloud resources (Linodes, NodeBalancers, Volumes, etc.) Admin-created resources may be restricted, but those created by the restricted user are editable.
Changed entityNameMap to grantTypeMap and added as const in order to generate a union of the mapped names to provide type-safety to useIsResourceRestricted and allow for capitalization/plurality in the names.
Added StyledTagButton to replace our one-off usages of adding tags.
Added getRestrictedResourceText utility to help generate resource-specific notices.
Added isRestrictedGlobalGrantType util which can be used to throughout Cloud Manager (+15 occurrences we can update in the future)

Preview 📷

Note

We changed the banner slightly from the screenshots. The text will now say You don't have permissions to ${action} ${resourceType}. Please contact your account administrator to request the necessary permissions.

Page	Before	After
Empty State Landing
Landing: Unable to delete admin NodeBalancer

Page	Before	After
Landing	No Restrictions	No Notice, Can Add Linodes, Can Add NodeBalancers
Create Page
Configurations
Settings
Summary: Tags

Before	After

How to test 🧪

Prerequisites

(How to setup test environment)

Log into two accounts side by side:
- An unrestricted admin user account: full access
- A restricted user account (use Incognito for this)
  - Start with Read Only for everything
  - After we'll be toggling permissions

Reproduction steps

Restricted User Perm: "Read Only"

Empty State Landing:
- Start with no NodeBalancers and perms set to "Read Only" on restricted account
- Observe as restricted user, you cannot create a NodeBalancer
- Change perm to include "Can add Linodes" and "Can add NodeBalancers"
- Observe as restricted user, you can create a NodeBalancer
- Reset settings for restricted user back to "Read Only"
Create:
- As admin user, create a Nodebalancer
- Observe as restricted user, you shouldn't be able to reach this page except by changing URL
- Observe that when changing URL, all fields and actions are disabled (including firewalls)
Landing:
- Observe as restricted user, notice shows and you cannot delete or create NodeBalancers
Detail:
- Observe as restricted user, notice shows
- Observe on Summary tab, you cannot create tags
- Observe on Configurations tab, all fields and actions are disabled
- Observe on Settings tab, all fields and actions are disabled

Restricted User Perm: "Read Only" w/ Can add Linodes, Can add NodeBalancers

Create:
- Observe as restricted user, you should be able to create a NodeBalancer
- Observe that there's no notices
Landing:
- Observe as restricted user, you should be able to create a NodeBalancer
- Observe there's no notices
- Observe you cannot delete the admin user's NodeBalancer
- Observe you can delete your own NodeBalancer
Detail:
- Observe no notices show for your own NodeBalancer, but that they do for admin user's NodeBalancer
- Observe you can edit your own NodeBalancer, but that you cannot edit the admin user's NodeBalancer

As an Author I have considered 🤔

Check all that apply

jaalah-akamai · 2024-01-23T18:46:38Z

packages/manager/src/components/ActionMenu/ActionMenu.tsx

    '& :hover': {
      color: '#4d99f1',
    },
+    '&& .MuiSvgIcon-root': {
+      fill: theme.color.disabledText,


We only display the tooltip for when things are disabled

jaalah-akamai · 2024-01-23T18:47:09Z

packages/manager/src/components/Button/StyledTagButton.ts

+ * spreading excessive styles for everywhere this is used.
+ *
+ */
+export const StyledTagButton = styled(Button, {


We can use this in other places as well, but I didn't want to expand the scope of this PR too much.

👍 that being said we should both add a unit test and add/modify a story if it is meant to stay

Agree with this new component and thanks for creating a test/story as per @abailly-akamai. Should we open a ticket for the other places?

jaalah-akamai · 2024-01-23T18:48:04Z

packages/manager/src/components/SelectFirewallPanel/SelectFirewallPanel.tsx

@@ -78,7 +81,7 @@ export const SelectFirewallPanel = (props: Props) => {
          value={selectedFirewall}
        />
        <StyledLinkButtonBox>
-          <LinkButton onClick={handleCreateFirewallClick}>
+          <LinkButton isDisabled={disabled} onClick={handleCreateFirewallClick}>


To disable this link in create flows for restricted users, probably should've been passed in to begin with.

packages/manager/src/features/Account/constants.ts

packages/manager/src/features/Account/types.ts

jaalah-akamai · 2024-01-23T18:53:50Z

packages/manager/src/features/NodeBalancers/NodeBalancerDetail/NodeBalancerConfigurations.tsx

@@ -87,6 +88,7 @@ const StyledConfigsButton = styled(Button, {
 }));

 interface Props {
+  grants: Grants | undefined;


Passing in grants due to the fact this is a class component and it was available from it's parent.

jaalah-akamai · 2024-01-23T19:03:19Z

packages/manager/src/features/ObjectStorage/BucketLanding/CreateBucketDrawer.tsx

@@ -123,7 +123,6 @@ export const CreateBucketDrawer = (props: Props) => {
        {isRestrictedUser && (
          <Notice
            data-qa-permissions-notice
-            important


Don't want the icon version for drawers anymore.

jaalah-akamai · 2024-01-23T19:03:42Z

packages/manager/src/features/Users/UserPermissionsEntitySection.tsx

-  firewall: 'Firewalls',
-  database: 'Databases',
-  vpc: 'VPCs',
-};


Moved to /src/features/Account/constants

github-actions · 2024-01-24T02:48:27Z

Coverage Report: ❌
Base Coverage: 81.19%
Current Coverage: 81.15%

bnussman-akamai · 2024-01-25T16:05:12Z

I'm not a huge fan of how we force a restricted user to look at a big yellow banner on landing pages. I think it would be nice if we had a more subtile way to indicate they have read-only permissions for example.

For the load balancer empty state, should we disable the "Create" button with a tooltip instead of putting a yellow banner at the top?

I still feel like "Access is restricted" is too generic. I think it would be nice to let the user know they have "read only permission" or that they "do not have permission to create"...

Those are just my opinions on the UX aspect of this, will continue to review regardless.

jaalah-akamai · 2024-01-26T14:30:44Z

@bnussman-akamai I'll circle back to UX about the wording, but if you can review the functionality that would be awesome!

bnussman-akamai

General functionality looks good! Have a few comments/questions

bnussman-akamai · 2024-01-26T15:33:21Z

packages/manager/src/features/NodeBalancers/NodeBalancerCreate.tsx

  const disabled =
-    Boolean(profile?.restricted) && !grants?.global.add_nodebalancers;
+    Boolean(profile?.restricted) &&
+    !(grants?.global.add_nodebalancers && grants?.global.add_linodes);


I confirmed I was able to create a NodeBalancer as a restricted user without add_linodes permission.

I understand why we might want to take add_linodes into account for UX reasons, but I'd prefer if we just check add_nodebalancers and not add_linodes so we are in parity with how the API works.

Screen.Recording.2024-01-26.at.10.32.01.AM.mov

+1 this. Any reason why the Add Linodes permission should be required for adding/modifying NodeBalancers?

if that's the case we should document this in code more thouroughly

Ahh yea I was under the impression that linodes with read_only perms couldn't be added to NodeBalancers, I guess that's not the case.

packages/manager/src/features/NodeBalancers/NodeBalancerDetail/NodeBalancerConfigurations.tsx

packages/manager/src/hooks/useIsResourceRestricted.ts

packages/manager/src/features/Account/utils.ts

abailly-akamai

Nice work! Added comments to clean things up and some questions. I'll give this a more thorough review once code improvements from reviewers have been addressed 👍

abailly-akamai · 2024-01-29T16:39:21Z

packages/manager/src/components/Button/StyledTagButton.ts

+ * spreading excessive styles for everywhere this is used.
+ *
+ */
+export const StyledTagButton = styled(Button, {


👍 that being said we should both add a unit test and add/modify a story if it is meant to stay

packages/manager/src/features/Account/constants.ts

packages/manager/src/features/Account/types.ts

abailly-akamai · 2024-01-29T16:44:48Z

packages/manager/src/features/NodeBalancers/NodeBalancerCreate.tsx

  const disabled =
-    Boolean(profile?.restricted) && !grants?.global.add_nodebalancers;
+    Boolean(profile?.restricted) &&
+    !(grants?.global.add_nodebalancers && grants?.global.add_linodes);


if that's the case we should document this in code more thouroughly

packages/manager/src/features/NodeBalancers/NodeBalancerCreate.tsx

abailly-akamai · 2024-01-29T16:48:54Z

packages/manager/src/features/NodeBalancers/NodeBalancerDetail/NodeBalancerConfigurations.tsx

@@ -628,6 +645,8 @@ class NodeBalancerConfigurations extends React.Component<CombinedProps, State> {
      ? parseInt(expandedConfigId, 10) === config.id
      : false;

+    const isRestricted = this.isRestrictedUser();
+
    const L = {


Not related to this PR, but const L 👎

packages/manager/src/features/NodeBalancers/NodeBalancersLanding/NodeBalancersLanding.tsx

...s/manager/src/features/NodeBalancers/NodeBalancersLanding/NodeBalancersLandingEmptyState.tsx

jaalah-akamai · 2024-01-30T16:26:51Z

Added storybook for StyledTagButton (No unit tests since it essentially inherits from Button and I'll be working with UX to make this comply to a standard button style)
Added isRestrictedGlobalGrantType util which can be used to throughout Cloud Manager (+15 occurrences we can update in the future)
getRestrictedResourceText now supports singular or plural instances
For useIsResourceRestricted hook, removed default for grantLevel and useEffect. Updated variable name to isNodeBalancerReadOnly

hkhalil-akamai

Verified testing steps, LGTM! Thanks for considering others' feedback.

hkhalil-akamai · 2024-01-30T19:02:16Z

packages/manager/src/components/Button/StyledTagButton.ts

+ * spreading excessive styles for everywhere this is used.
+ *
+ */
+export const StyledTagButton = styled(Button, {


Agree with this new component and thanks for creating a test/story as per @abailly-akamai. Should we open a ticket for the other places?

abailly-akamai

Reads so much better, thanks for the changes!

Feature behaves as described in the PR and looks great from my vantage point ✅

abailly-akamai · 2024-01-30T19:40:53Z

packages/manager/src/features/Account/utils.ts

+  profile: Profile | undefined;
+}) => {
+  return Boolean(profile?.restricted) && !grants?.global[globalGrantType];
+};


I know it's clear from the name however

can we add return types here?

i think it's always super useful to complement those utils with JSDocs

jaalah-akamai · 2024-01-31T12:48:46Z

Agree with this new component and thanks for creating a test/story as per @abailly-akamai. Should we open a ticket for the other places?

I updated it here, it was only one reference.

jaalah added 3 commits January 22, 2024 10:59

Work in progress

b69c6c1

feat: [M3-7490] - Improve Nodebalancer restricted user experience

40c7d96

feat: [M3-7490] - Improve Nodebalancer restricted user experience

3109cd7

jaalah-akamai added the Work in Progress label Jan 22, 2024

jaalah-akamai self-assigned this Jan 22, 2024

jaalah added 2 commits January 23, 2024 12:24

More updates...

b6ac3e4

Added utils and types

4a3f190

jaalah-akamai commented Jan 23, 2024

View reviewed changes

packages/manager/src/features/Account/constants.ts Show resolved Hide resolved

jaalah-akamai commented Jan 23, 2024

View reviewed changes

packages/manager/src/features/Account/types.ts Show resolved Hide resolved

jaalah-akamai commented Jan 23, 2024

View reviewed changes

Cleanup class method

87e34e1

jaalah-akamai commented Jan 23, 2024

View reviewed changes

Renamed hook

3d98268

jaalah-akamai added the Restricted User Access Improve UX surrounding restricted access to features label Jan 23, 2024

jaalah-akamai marked this pull request as ready for review January 24, 2024 02:25

jaalah-akamai requested a review from a team as a code owner January 24, 2024 02:25

jaalah-akamai requested review from bnussman-akamai and abailly-akamai and removed request for a team January 24, 2024 02:25

jaalah-akamai added Ready for Review and removed Work in Progress labels Jan 24, 2024

Get latest

924986d

jaalah added 2 commits January 23, 2024 23:08

Added changeset: Improve NodeBalancer Restricted User Experience

87a9451

Update changeset

ed92441

jaalah added 2 commits January 26, 2024 09:58

Update UX- remove banners from create/landing

35bf9f9

Get latest

b15eea4

bnussman-akamai reviewed Jan 26, 2024

View reviewed changes

abailly-akamai reviewed Jan 29, 2024

View reviewed changes

Review updates

c6e93b6

jaalah added 2 commits January 30, 2024 11:30

Simplify hook

8154589

Get latest

bb2a422

jaalah-akamai requested review from abailly-akamai, bnussman-akamai and hkhalil-akamai January 30, 2024 16:34

hkhalil-akamai approved these changes Jan 30, 2024

View reviewed changes

abailly-akamai approved these changes Jan 30, 2024

View reviewed changes

Update other reference for TagButton - return types

c5acd0a

jaalah-akamai merged commit b11bff9 into linode:develop Jan 31, 2024
17 of 18 checks passed

cpathipa mentioned this pull request Feb 1, 2024

Release v1.111.0 - release → staging #10136

Merged

12 tasks

jaalah-akamai mentioned this pull request Feb 16, 2024

refactor: [M3-7786] - Convert isRestrictedGlobalGrantType to Hook #10203

Merged

12 tasks

mjac0bs mentioned this pull request Apr 15, 2024

refactor: [M3-7380] - Use volumes/types endpoint for pricing data #10376

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [M3-7490] - Improve NodeBalancer Restricted User Experience #10095

feat: [M3-7490] - Improve NodeBalancer Restricted User Experience #10095

jaalah-akamai commented Jan 22, 2024 •

edited

Loading

jaalah-akamai Jan 23, 2024

jaalah-akamai Jan 23, 2024

abailly-akamai Jan 29, 2024

hkhalil-akamai Jan 30, 2024

jaalah-akamai Jan 23, 2024

jaalah-akamai Jan 23, 2024

jaalah-akamai Jan 23, 2024

jaalah-akamai Jan 23, 2024

github-actions bot commented Jan 24, 2024 •

edited

Loading

bnussman-akamai commented Jan 25, 2024

jaalah-akamai commented Jan 26, 2024

bnussman-akamai left a comment

bnussman-akamai Jan 26, 2024

hkhalil-akamai Jan 26, 2024

abailly-akamai Jan 29, 2024

jaalah-akamai Jan 30, 2024

abailly-akamai left a comment

abailly-akamai Jan 29, 2024

abailly-akamai Jan 29, 2024

abailly-akamai Jan 29, 2024

bnussman-akamai Jan 30, 2024

jaalah-akamai commented Jan 30, 2024 •

edited

Loading

hkhalil-akamai left a comment

hkhalil-akamai Jan 30, 2024

abailly-akamai left a comment

abailly-akamai Jan 30, 2024

jaalah-akamai commented Jan 31, 2024 •

edited

Loading

feat: [M3-7490] - Improve NodeBalancer Restricted User Experience #10095

feat: [M3-7490] - Improve NodeBalancer Restricted User Experience #10095

Conversation

jaalah-akamai commented Jan 22, 2024 • edited Loading

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Prerequisites

Reproduction steps

As an Author I have considered 🤔

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 24, 2024 • edited Loading

bnussman-akamai commented Jan 25, 2024

jaalah-akamai commented Jan 26, 2024

bnussman-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaalah-akamai commented Jan 30, 2024 • edited Loading

hkhalil-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaalah-akamai commented Jan 31, 2024 • edited Loading

jaalah-akamai commented Jan 22, 2024 •

edited

Loading

github-actions bot commented Jan 24, 2024 •

edited

Loading

jaalah-akamai commented Jan 30, 2024 •

edited

Loading

jaalah-akamai commented Jan 31, 2024 •

edited

Loading