fix: [UIE-9661] - oAuth callback improvements - #13105
Conversation
| ? getQueryParamsFromQueryString(options.params) | ||
| : options.params | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Just added some typeguard here to satisfy the new callback strict route types
| const authenticate = async () => { | ||
| try { | ||
| const { returnTo } = await handleOAuthCallback({ | ||
| params: location.search, |
There was a problem hiding this comment.
This is one potential issue:
here we were using windows.location which means it would get evaluated (and return a value) before router initiation.
| '/admin/callback', | ||
| '/oauth/callback', | ||
| '/cancel', | ||
| ]; |
There was a problem hiding this comment.
This is the fix to not redirect to one of these routes in order to never be stuck in a logout loop
| }); | ||
|
|
||
| const hasStartedAuth = React.useRef(false); | ||
| const isAuthenticating = React.useRef(false); |
There was a problem hiding this comment.
Using references to avoid triggering the callback more than needed and end up with stale values
| profile: Profile | undefined | ||
| ): Promise<boolean> => { | ||
| if (!flags?.iam?.enabled) { | ||
| if (!flags?.iam?.enabled || !profile) { |
There was a problem hiding this comment.
not directly related to this PR. just a follow up to #13037
Cloud Manager UI test results🎉 862 passing tests on test run #13 ↗︎
|
jdamore-linode
left a comment
jdamore-linode
left a comment
There was a problem hiding this comment.
Thanks @abailly-akamai! I can't reproduce the login issue against a locally served copy of develop or this branch so I can't verify that the issue is fixed, but I'm not observing any regressions either. I'll be sure to cover this during release testing if we don't find a way to test this in the meantime
4 participants
Description 📝
This PR attempts to fix a couple issues with the oAuth callback.
Changes 🔄
Scope 🚢
Upon production release, changes in this PR will be visible to:
How to test 🧪
Reproduction steps
Some of this isn't consistently reproducible, happens on various network speed and/or reasons unbeknown to us mortals.
see ticket for details:
M3-9661However, on a slow connection this can easily be reproduced:
/logoutVerification steps
👉 try all sorts of logins & logouts in various environments at various network speeds
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 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
As an Author, before moving this PR from Draft to Open, I confirmed ✅