change: [M3-7729] - Improve UX of Personal Access Token and Access Key drawers #10338

mjac0bs · 2024-04-01T20:55:15Z

Description 📝

Based on discussion in cafe, we want to make a couple of improvements to the Personal Access Token and OBJ Access Key drawers to prevent users from unintentionally limiting their access (e.g. creating a PAT with all scopes set to None), while forcing the user to make a conscious decision about the access selections that they make.

Note: AccessKeyDrawer.tsx is going to be deprecated as a component once OBJ Multi-Cluster is live, so updates are limited to the OMC bucket access keys components.

Changes 🔄

In both the Create PAT Drawer and Create OBJ Access Key Drawer:

No scopes are selected by default
"None" is changed to "No Access" (already done in upcoming: [M3-7694] - OBJ Multi Cluster Copy updates #10188 for OBJ access keys; also done in ViewAPITokenDrawer)
The "Select All" row is separated from the other rows by a bolded bottom border
The submit button is disabled until every scope has a selection made
Tests are updated

Note: If anyone has suggestions on how to fix the padding for the Read/Write column, I'd appreciate it! Other than making the PAT drawer wide, I haven't been able to find a way to add spacing to that column that actually works with the table display CSS properties.

Preview 📷

Component	Before	After
Create a PAT
Create an OBJ Access Key	Screen.Recording.2024-04-09.at.12.02.05.PM.mov	Screen.Recording.2024-04-09.at.10.58.58.AM.mov

How to test 🧪

Reproduction steps

(How to reproduce the issue, if applicable)

Go to https://cloud.linode.com/profile/tokens and click "Create a Personal Access Token".
Observe the default scope selection is "None" and the Select All row is not obviously separated from the other scopes.
Check out develop and turn the OBJ Multi-Cluster feature flag on and the mocks on.
Go to http://localhost:3000/object-storage/access-keys/ and click "Create Access Key".
Observe that when there are no regions selected, all three radio buttons are selected in the Bucket Permissions Table.
Observe that once a region is selected, the radio button selection is "No Access" by default.
Observe that the Select All row is not obviously separated from the other scopes.

Verification steps

(How to verify changes)

Check out this PR and yarn up.
Go to http://localhost:3000/profile/tokens and click "Create a Personal Access Token".
Observe the Changes outlined above.
Confirm that you can still submit PATs with no scopes, read-only scopes, and * (all scopes).
Go to http://localhost:3000/object-storage/access-keys and click "Create Access Key".
Observe the Changes outlined above.
Confirm the request payload when creating access keys with various region and permissions selections.
- Example 1:
  - Limited Access field is toggled on with No Access selected
  - Payload contains bucket_access: [] empty array
- Example 2:
  - Limited Access field is toggled off (unlimited access)
  - Payload contains bucket_access: null
- Example 3:
  - Limited Access field toggled on with some read / read write
  - Payload contains bucket_access : [{....}]
Confirm all tests pass:

yarn test utils CreateAPITokenDrawer ViewAPITokenDrawer

yarn cy:run -s "cypress/e2e/core/account/personal-access-tokens.spec.ts, cypress/e2e/core/objectStorage/access-keys.smoke.spec.ts"

As an Author I have considered 🤔

Check all that apply

mjac0bs · 2024-04-04T17:03:13Z

packages/manager/src/features/ObjectStorage/AccessKeyLanding/utils.ts

  | { label: FormState['label'] }
+  | { label: FormState['label']; regions: FormState['regions'] }


mjac0bs · 2024-04-08T22:09:20Z

packages/manager/src/features/ObjectStorage/AccessKeyLanding/OMC_AccessKeyDrawer.tsx

+      !hasLabelOrRegionsChanged(formik.values, objectStorageKey)) ||
+    (mode === 'creating' &&
+      limitedAccessChecked &&
+      !hasAccessBeenSelectedForAllBuckets(formik.values.bucket_access));


Disables the Create button if some buckets have limited access and their scope selections have not been made.

packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx

mjac0bs · 2024-04-08T22:15:31Z

packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx

@@ -97,7 +99,7 @@ export const CreateAPITokenDrawer = (props: Props) => {
  const initialValues = {
    expiry: expiryTups[0][1],
    label: '',
-    scopes: scopeStringToPermTuples(''),
+    scopes: scopeStringToPermTuples('', true),


For isCreateFlow param, to determine permission default values.

packages/manager/src/features/Profile/APITokens/utils.ts

github-actions · 2024-04-09T20:58:29Z

Coverage Report: ✅
Base Coverage: 81.8%
Current Coverage: 81.8%

cpathipa

LGTM!

Confirming on visual changes on OBJ Access key drawer.
Able to create / update limited access key.
Create a Personal Access Token

cpathipa · 2024-04-10T16:17:33Z

@mjac0bs Hold on my approval, I think we should find a way to control default behavior of radio buttons with out diverging form API. I think there is an opportunity for little bit cleanup. I will take a deeper look later today.

dwiley-akamai

From the PR description: "The submit button is disabled until every scope has a selection made" -- is this the case for the PAT drawer currently? I'm observing this:

PAT drawer screenshots

Tried to actually submit on this one, but got an error about the child_account scope (which shouldn't be at play for this particular account since it isn't a parent/child scenario?)

packages/manager/.changeset/pr-10338-changed-1712685790473.md

carrillo-erik · 2024-04-10T19:38:49Z

Verified functionality and that all tests are passing. Below is a screenshot of the Access Keys (redacted) I created and the payload contents which show it aligns with the description of the PR.

packages/api-v4/src/object-storage/types.ts

Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>

bnussman-akamai · 2024-04-11T17:25:36Z

I really liked how we set all scopes to "None" by default in the old flow. I feel like this new UX creates slightly more work and confusion for the user.

For example if they fill out the form as shown in the screenshot, what will the expected behavior be? I don't think it's obvious to the user what permission will be set of the unselected scopes.

Screenshot

I think defaulting to "No Access" makes a lot of sense. This new flow also requires more clicks / interaction from the user.

I assume UX's argument is that they want the choices to be intentional. This is kind of fair, but I think it will create more work for the average user. This is speculation, but there's probably two types of users who create PATs.

A user who knows exactly what they are doing. I don't see a need to make them do an extra click to set all permissions to none
A user who is following some guide on an external site and the guide will tell them what scopes to grant. That guide should't have to have an extra step that says "first, set all grants to "No access", then select Read-Write for VPC"

Additionally, If a user wants to create a PAT with zero scopes, I think we should let them 🤷

I'm curious as to what prompted the change to have no defaults and if users had complaints / issues with the way it currently works.

Just my thoughts, they can be completely ignored if I'm the only one who feels this way.

…awers

mjac0bs · 2024-04-16T22:04:10Z

@bnussman-akamai - There's some history to this one. We used to default to "Read Write", rather than "No Access"/"None". The new default was a fairly recent change requested for security best practices that got rolled into some Parent/Child updates. Once we implemented that change, we received some reports that (internal, though I imagine there were also external) users were mistakenly creating useless keys because they didn't realize that the default had changed to "No Access"/"None". In the internal ticket, I linked a thread with some discussion on this, but we've got UX and backend folks on board with this solution: defaulting to no selections and disabling the button until the required selections have been made.

For example if they fill out the form as shown in the screenshot, what will the expected behavior be? I don't think it's obvious to the user what permission will be set of the unselected scopes.

You're right that this was confusing, and it was unintended. Thanks to you and Dajahi for pointing it out; I fixed this in 5e8b3e8. If we feel reason for the disabled button is not obvious to the user, we could add a help tooltip to the disabled button, but I'm not sure it's necessary.

Screenshot

Additionally, If a user wants to create a PAT with zero scopes, I think we should let them

DX did say there's potentially a legit use-case for this, so it will remain possible like it is today, it will just require the user to Select "No Access" for all scopes.

bnussman-akamai

Thanks for the history and context!

Changes look good ✅

mjac0bs · 2024-04-17T14:38:28Z

packages/manager/src/features/ObjectStorage/AccessKeyLanding/OMC_AccessKeyDrawer.tsx

@@ -52,6 +56,11 @@ export interface AccessKeyDrawerProps {
  open: boolean;
 }

+// Access key scopes displayed in the drawer can have no permission or "No Access" selected, which are not valid API permissions.
+export interface DisplayedAccessKeyScope extends Omit<Scope, 'permissions'> {
+  permissions: AccessType | null;


@cpathipa I looked into also removing 'none' from the AccessType, but it required more clean up in the original AccessKeyDrawer files, which I wanted to avoid touching since they will be soon deprecated. I did create the new interface here for the null value displayed scopes in the OMC drawer and can create a follow up ticket (edit: M3-8003) for correcting the AccessType to only the API's values once we switch to fully OMC components.

mjac0bs self-assigned this Apr 1, 2024

mjac0bs changed the title ~~change: [M3 7729] - Improve UX of Personal Access Token and Access Key drawers~~ change: [M3-7729] - Improve UX of Personal Access Token and Access Key drawers Apr 1, 2024

mjac0bs force-pushed the M3-7729-improve-pat-and-obj-access-key-drawers branch from 3a94ec1 to 5bb8095 Compare April 3, 2024 23:53

mjac0bs added 7 commits April 8, 2024 14:17

Update Select All from 'None' to 'No Access' and add border

2f22814

Disable the Create Token button until scope selection is made

97b2c3b

Add test coverage

b180ffa

Apply Select All row styling; fix radio state without regions

53f5470

Fix default bucket access selection to no selection

0c98fe5

Add util and logic to disable create button; add test

a370a95

Update PAT test to account for UI updates

cee94ab

mjac0bs force-pushed the M3-7729-improve-pat-and-obj-access-key-drawers branch from 3438391 to cee94ab Compare April 8, 2024 21:19

mjac0bs added 3 commits April 9, 2024 10:46

Update one more test

676c5ae

Fix type

0e0f0fc

Added changeset: Improve the UX of access key drawers

4c13e9e

mjac0bs commented Apr 9, 2024

View reviewed changes

mjac0bs marked this pull request as ready for review April 9, 2024 19:07

mjac0bs requested review from a team as code owners April 9, 2024 19:07

mjac0bs requested review from cliu-akamai, carrillo-erik and cpathipa and removed request for a team April 9, 2024 19:07

Clean up

4fbb60f

cpathipa approved these changes Apr 10, 2024

View reviewed changes

mjac0bs requested a review from dwiley-akamai April 10, 2024 14:47

dwiley-akamai reviewed Apr 10, 2024

View reviewed changes

packages/manager/.changeset/pr-10338-changed-1712685790473.md Outdated Show resolved Hide resolved

carrillo-erik approved these changes Apr 10, 2024

View reviewed changes

mjac0bs added the Needs extra approvals Use for PRs that have high-impact changes where > 2 approvals are needed label Apr 10, 2024

cpathipa reviewed Apr 10, 2024

View reviewed changes

packages/api-v4/src/object-storage/types.ts Outdated Show resolved Hide resolved

Update packages/manager/.changeset/pr-10338-changed-1712685790473.md

0255704

Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>

mjac0bs added 4 commits April 16, 2024 09:19

Merge branch 'develop' into M3-7729-improve-pat-and-obj-access-key-dr…

4ebb56b

…awers

Address feedback: disable button if any missing scope selections

5e8b3e8

Update PAT test coverage

cb831e4

Use new scope interface for displayed access key scopes

7403ee1

bnussman-akamai approved these changes Apr 16, 2024

View reviewed changes

Clean up types and exports

75480fc

mjac0bs commented Apr 17, 2024

View reviewed changes

mjac0bs requested review from dwiley-akamai and cpathipa April 17, 2024 14:38

cpathipa approved these changes Apr 17, 2024

View reviewed changes

mjac0bs added Approved Multiple approvals and ready to merge! and removed Needs extra approvals Use for PRs that have high-impact changes where > 2 approvals are needed labels Apr 18, 2024

mjac0bs merged commit 6529346 into linode:develop Apr 18, 2024
17 of 18 checks passed

This was referenced Apr 25, 2024

Release v.118.0 - release → staging #10409

Closed

Release v1.118.0 - release → staging #10410

Merged

Release v1.118.0 - staging → master #10416

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change: [M3-7729] - Improve UX of Personal Access Token and Access Key drawers #10338

change: [M3-7729] - Improve UX of Personal Access Token and Access Key drawers #10338

mjac0bs commented Apr 1, 2024 •

edited

mjac0bs Apr 4, 2024

mjac0bs Apr 8, 2024

mjac0bs Apr 8, 2024

github-actions bot commented Apr 9, 2024 •

edited

cpathipa left a comment •

edited

cpathipa commented Apr 10, 2024

dwiley-akamai left a comment

carrillo-erik commented Apr 10, 2024

bnussman-akamai commented Apr 11, 2024 •

edited

mjac0bs commented Apr 16, 2024

bnussman-akamai left a comment

mjac0bs Apr 17, 2024 •

edited

		\| { label: FormState['label'] }
		\| { label: FormState['label']; regions: FormState['regions'] }

change: [M3-7729] - Improve UX of Personal Access Token and Access Key drawers #10338

change: [M3-7729] - Improve UX of Personal Access Token and Access Key drawers #10338

Conversation

mjac0bs commented Apr 1, 2024 • edited

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Reproduction steps

Verification steps

As an Author I have considered 🤔

mjac0bs Apr 4, 2024

Choose a reason for hiding this comment

mjac0bs Apr 8, 2024

Choose a reason for hiding this comment

mjac0bs Apr 8, 2024

Choose a reason for hiding this comment

github-actions bot commented Apr 9, 2024 • edited

cpathipa left a comment • edited

Choose a reason for hiding this comment

cpathipa commented Apr 10, 2024

dwiley-akamai left a comment

Choose a reason for hiding this comment

carrillo-erik commented Apr 10, 2024

bnussman-akamai commented Apr 11, 2024 • edited

mjac0bs commented Apr 16, 2024

bnussman-akamai left a comment

Choose a reason for hiding this comment

mjac0bs Apr 17, 2024 • edited

Choose a reason for hiding this comment

mjac0bs commented Apr 1, 2024 •

edited

github-actions bot commented Apr 9, 2024 •

edited

cpathipa left a comment •

edited

bnussman-akamai commented Apr 11, 2024 •

edited

mjac0bs Apr 17, 2024 •

edited