feat: [M3-7921] - Added Dialog to Refresh Proxy Tokens as Time Expires by jaalah-akamai · Pull Request #10361 · linode/manager

jaalah-akamai · 2024-04-08T18:02:41Z

Description 📝

When time is about to expire, we want to prompt the user with a modal window that indicates they will be logged out soon (show when 5min is remaining) and ask them if they want to continue working (refresh their token).

Note

I also did refactoring in SwitchAccountDrawer to implement the useParentChildAuthentication, hopefully making the file more readable.

Additional Issues

Parent Account: The drawer lacks a loading indicator when choosing a child account. This can lead to multiple selections and redundant requests for slow networks.
Proxy Account: Slow networks can cause the "Switch back to your account" action to initiate multiple requests.
Token Revocation: Currently, a toast notification appears when revoking a token. However, it often fails to display before the page is refreshed during the switch. Given the refresh, the toast notification is unnecessary.

Changes 🔄

New Files:

SessionExpirationDialog: Displayed for users logged into a proxy account. It performs three main functions:
- Initiates a countdown timer based on the token's expiration time, displaying the dialog when only 5 minutes remain.
- Provides options to either refresh the token or dismiss the dialog, allowing the token to expire naturally.
- Provides option to logout (returns to parent account if parent token is valid or logs out if not)
useInterval: Introduced a familiar hook for reliable timer management.
useParentChildAuthentication: In order to make the creation/revoking/updating/validating of tokens, I created this hook to make things more DRY. This allows us to do a lot of cleanup and in several files (SessionExpirationDialog, SwitchAccountDrawer, AccountLanding).

Deleted:

usePendingRevocationToken: We are now using usePersonalAccessTokensQuery directly in places it's needed.

Changes:

Renamed useParentTokenManagement to useIsParentTokenExpired for clarity

Target release date 🗓️

4/29

Preview 📷

feature.mp4

How to test 🧪

Prerequisites

Log into Parent account using developer credentials

Reproduction steps

Loading Indicators:

Open user menu and "Switch Account"
Select a child company and observe there's no loading indicator
In addition, if you slowed network you can repeatedly send requests
In proxy account (after switching to child company), open user menu and "Switch Account"
Observe same issue with "Switch back to your account"

Verification steps

Loading Indicators:

Checkout branch and log into Parent account
Open user menu and "Switch Account"
Observe that selecting child company shows loading indicator
Observe the same process for Proxy account when trying to "Switch back to your account"

Session Expiry:

Checkout branch and log into Parent account
Change L70 to if (timeRemaining.minutes < 15) { to observe modal
Test that closing modal should do nothing allowing user to return to work without refreshing token
Test that logging out sends user back to parent account
Change L100 to true to test that logging out sends you to log in
Test that "Continue Working" refreshes token and closes modal.

As an Author I have considered 🤔

Check all that apply

jaalah-akamai · 2024-04-09T21:49:44Z

packages/manager/src/MainContent.tsx

-                  isSideMenuOpen={!desktopMenuIsOpen}
-                  openSideMenu={() => toggleMenu(true)}
-                  username={username}
+      <SessionExpirationProvider value={sessionExpirationContextValue}>


Only change in JSX was to add a new context provider

jaalah-akamai · 2024-04-09T21:51:19Z

packages/manager/src/features/Account/SwitchAccountDrawer.tsx

 interface Props {
  onClose: () => void;
  open: boolean;
-  proxyToken?: Token;


No need to pass token in anymore, we're handling this within component now.

jaalah-akamai · 2024-04-09T21:52:45Z

packages/manager/src/features/Account/SwitchAccountDrawer.tsx

  const currentParentTokenWithBearer =
    getStorage('authentication/parent_token/token') ?? '';

-  const handleProxyTokenRevocation = React.useCallback(async () => {


Handled by useParentChildAuthentication.revokeToken and removed toasts since they're not necessary when switching accounts.

jaalah-akamai · 2024-04-09T21:53:25Z

packages/manager/src/features/Account/SwitchAccountDrawer.tsx

+    validateParentToken,
+  } = useParentChildAuthentication({
+    tokenIdToRevoke: pendingRevocationToken?.id ?? -1,
+  });


This is how the new hook will be used

jaalah-akamai · 2024-04-09T21:58:30Z

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

- * For 'proxy' userType, use the stored parent token in the request.
+ * Determine whether the tokens used for switchable accounts are still valid.
+ */
+export const isParentTokenValid = (): boolean => {


These functions were moved from parent utils file since they pertain to Account Switching.

isParentTokenValid

setTokenInLocalStorage

updateCurrentTokenBasedOnUserType

getPersonalAccessTokenForRevocation

jaalah-akamai · 2024-04-09T22:00:47Z

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

-      token.token &&
-      currentTokenWithBearer.replace('Bearer ', '').startsWith(token.token)
-  );
-}


Moved all these to /SwitchAccounts/utils

packages/manager/src/features/Account/SwitchAccounts/useParentChildAuthentication.tsx

mjac0bs

Thanks again for the pair review on this; it was definitely helpful to digest these changes. Confirmed that the token will refresh if a user clicks "Continue Working", won't refresh if they cancel out, and will either log out the user and return them to the parent account or to login if the parent token has expired.

I wonder if the proxy log out and return to parent account will be surprising to the parent, since they may be expecting to be logged out of CM entirely. I would imagine it would be a welcome feature from their end, though I'm not sure if it would be immediately clear to them what happened. We could consider adding something like "If your own session is still valid, you will be returned to your parent account on logout." to the session dialog, but maybe we get some input from UX or wait to see if parent users report and feedback or confusion before making any adjustments.

I left comments on a few other things I've noticed while testing. It also looks like the unit test for SessionExpirationDialog is currently failing. I replicated that locally:

packages/manager/src/features/Account/SwitchAccountDrawer.tsx

jaalah-akamai · 2024-05-01T19:28:05Z

@bnussman-akamai Good suggestion on using the endpoint directly, was able to clean things up in several files
@mjac0bs Hitting the endpoints on slow network requests should be fixed with the way I switched things around, essentially the loading state from RQ was taking too long, so handling this with state

github-actions · 2024-05-01T19:50:13Z

Coverage Report: ✅
Base Coverage: 82%
Current Coverage: 82.01%

mjac0bs

Switching accounts from parent to proxy and back, as well as to other child accounts via proxy is still looking good with the latest. Confirmed the slow network edge case behavior I was seeing before is fixed. The dialog still works as expected; I set it to 14 minutes and confirmed it popped up on time. New dialog copy looks good to me.

This should be unrelated to this PR; I just happened to notice it while testing: token revoke events, which are plentiful if you switch between accounts or continue working many times, try to make a network request to /account/users/[proxy-username] and get 403 forbidden as a response. It looks gravatar related to me. 😓 (just furthering our hopes 🪓 )

mjac0bs · 2024-05-02T21:44:49Z

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

+  '&:disabled': {
+    color: theme.palette.text.disabled,
+    cursor: 'not-allowed',
+  },
+  '&:hover:not(:disabled)': {


This looks nice!

Screen.Recording.2024-05-02.at.2.25.02.PM.mov

jaalah added 3 commits April 8, 2024 08:49

feat: [M3-7921] - Refresh Proxy User Session

5f52bd0

Update and add some more tests

18d9d10

Update and add some more tests

738cc11

jaalah-akamai added the Parent / Child Account label Apr 8, 2024

jaalah-akamai self-assigned this Apr 8, 2024

jaalah added 4 commits April 8, 2024 15:03

Remove hook - no longer need

dc7911f

Add useInterval hook

33ebb65

Adjust options

f69aea9

Revert my testing code

ec2d75a

jaalah-akamai commented Apr 9, 2024

View reviewed changes

Minor cleanup

7017e40

jaalah-akamai commented Apr 9, 2024

View reviewed changes

mjac0bs self-requested a review April 11, 2024 15:09

jaalah added 2 commits April 11, 2024 11:40

Added changeset: Dialog to Refresh Proxy Tokens as Time Expires

2231880

Get latest from dev

4426240

jaalah-akamai marked this pull request as ready for review April 11, 2024 15:45

jaalah-akamai requested a review from a team as a code owner April 11, 2024 15:45

jaalah-akamai requested review from bnussman-akamai and cpathipa and removed request for a team April 11, 2024 15:45

bnussman-akamai reviewed Apr 17, 2024

View reviewed changes

packages/manager/src/features/Account/SwitchAccounts/useParentChildAuthentication.tsx Outdated Show resolved Hide resolved

mjac0bs reviewed Apr 17, 2024

View reviewed changes

abailly-akamai added the Target: 4/29 label Apr 24, 2024

mjac0bs removed the Target: 4/29 label Apr 24, 2024

jaalah added 2 commits May 1, 2024 14:00

Review updates

de5c809

Fix tests

8d82bb4

jaalah-akamai added the Ready for Review label May 1, 2024

jaalah-akamai requested a review from bnussman-akamai May 1, 2024 19:28

jaalah-akamai requested a review from mjac0bs May 1, 2024 19:28

fix e2e test

0168ad1

mjac0bs approved these changes May 2, 2024

View reviewed changes

bnussman-akamai approved these changes May 3, 2024

View reviewed changes

bnussman-akamai added the Approved Multiple approvals and ready to merge! label May 3, 2024

Get latest and fix conflict

71251b8

jaalah-akamai merged commit 423d84f into linode:develop May 6, 2024

This was referenced May 13, 2024

Release v1.119.0 - release → staging #10457

Merged

Release v1.119.0 - staging → master #10458

Merged

Conversation

jaalah-akamai commented Apr 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description 📝

Additional Issues

Changes 🔄

New Files:

Deleted:

Changes:

Target release date 🗓️

Preview 📷

How to test 🧪

Prerequisites

Reproduction steps

Loading Indicators:

Verification steps

Loading Indicators:

Session Expiry:

As an Author I have considered 🤔

Uh oh!

jaalah-akamai Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

jaalah-akamai Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

jaalah-akamai Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

jaalah-akamai Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

jaalah-akamai Apr 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaalah-akamai Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mjac0bs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaalah-akamai commented May 1, 2024

Uh oh!

github-actions bot commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjac0bs left a comment

Choose a reason for hiding this comment

Uh oh!

mjac0bs May 2, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jaalah-akamai commented Apr 8, 2024 •

edited

Loading

jaalah-akamai Apr 9, 2024 •

edited

Loading

github-actions bot commented May 1, 2024 •

edited

Loading