DEVPROD-26290 Implement spawn host workflow with OAuth#1536
DEVPROD-26290 Implement spawn host workflow with OAuth#1536ZackarySantana merged 7 commits intomainfrom
Conversation
| fetchPolicy: "no-cache", | ||
| // We poll faster than normal because users expect to see | ||
| // faster feedback after performing the authentication flow. | ||
| pollInterval: skip ? 0 : FASTER_POLL_INTERVAL, |
There was a problem hiding this comment.
do we want network-only instead here or another cache strategy? i think with no-cache we're essentially replicating this interval call with each instance/usage of useUser as we can never use the cache and always reach out to the server which could cause some extra load on the server and some perceived slowness
that's fine if that's behavior we want, but just wanted to call out as it might cause some confusion down the line
There was a problem hiding this comment.
cause some extra load on the server and some perceived slowness
That's a fair point, I think the expected extra load on the server isn't as much as other pages like the waterfall- since it's part of a specific workflow (when the modal is open)- but having a responsive UX post-authentication is important for the overall smoothness of the flow.
I'm not sure what might be the best strategy here to reduce the user's waiting time post-authentication as much as possible and I'm open to suggestions!
There was a problem hiding this comment.
+1 to aaron, I'd suggest creating a separate query entirely from useUser so that you can control polling here without having wide ranging impacts!
There was a problem hiding this comment.
Okay, I've never written a new query but I think I did it correctly. Please lmk what yall think!
Also, I switched it to one hook + using an enum because I didn't like that there could exist the invalid state of both hooks/values returning true.
| return isTokenValid(tokenExpiresAt); | ||
| }; | ||
|
|
||
| export const useUserIsUndergoingAuthentication = (skip: boolean): boolean => { |
There was a problem hiding this comment.
we refetch on focus for useUserHasValidToken, that would mean this could be out of sync until the next poll. dont think it would break anything but might cause some ui inconsistency for FASTER_POLL_INTERVAL/20 seconds
| hasValidToken, | ||
| hostUptimeWarnings, | ||
| isMigration: false, | ||
| isUndergoingAuthentication: isUndergoingAuthentication, |
There was a problem hiding this comment.
this could just be isUndergoingAuthentication,
athammer
left a comment
athammer
left a comment
There was a problem hiding this comment.
few comments but nothing blocking. looks good!
There was a problem hiding this comment.
[nit] We don't really use this filename format, getFormSchema.test.tsx would be fine
| fetchPolicy: "no-cache", | ||
| // We poll faster than normal because users expect to see | ||
| // faster feedback after performing the authentication flow. | ||
| pollInterval: skip ? 0 : FASTER_POLL_INTERVAL, |
There was a problem hiding this comment.
+1 to aaron, I'd suggest creating a separate query entirely from useUser so that you can control polling here without having wide ranging impacts!
| @@ -0,0 +1,6 @@ | |||
| query UserTokenExchange { | |||
| user { | |||
There was a problem hiding this comment.
Could you also add userId to the query here? A type's ID is always required when it exists
There was a problem hiding this comment.
[nit] Can you name this useUserTokenExchange.ts? For custom hooks we tend to use that pattern
| const { schema, uiSchema } = getFormSchema({ | ||
| ...formSchemaInput, | ||
| availableRegions: selectedDistro?.availableRegions ?? [], | ||
| // No task loading on volume migration, so we don't need to check for a valid token. |
There was a problem hiding this comment.
[nit] Comment should be moved above tokenExchangeState I think
There was a problem hiding this comment.
Yeah, this was a remain from the old impl before enums
| const onFocus = () => void refetch(); | ||
| window.addEventListener("focus", onFocus); | ||
| return () => window.removeEventListener("focus", onFocus); |
There was a problem hiding this comment.
Didn't you remove the onFocus stuff in the previous PR 👀
There was a problem hiding this comment.
This onFocus stuff is making the UI load instantly when I return, so I figured it was pretty neat 🤷 . What do you think?
There was a problem hiding this comment.
Ah, could you use useLazyQuery to start polling on quick? I think that should be a pretty straightforward swap! It also supports polling.
There was a problem hiding this comment.
Actually, I guess the focus hook may still help with the most instantaneous refetch possible. I do like the idea of useLazyQuery but consider it optional!
| UserTokenExchangeQuery, | ||
| UserTokenExchangeQueryVariables | ||
| >(USER_TOKEN_EXCHANGE, { | ||
| skip, |
There was a problem hiding this comment.
Can you use the skipToken pattern please?
This reverts commit 0d98c3b.
3 participants
DEVPROD-26290
Description
(a 2nd attempt from #1505)
(more context here)
This PR implements the spawn host workflow using OAuth. The demo videos show it best, but essentially users will have to authenticate with Evergreen before using their spawn host, then we can properly set up the spawn host.
Screenshots
This is with jwt enabled (migration of CLI):
Screen.Recording.2026-04-07.at.6.38.47.PM.mov
Testing
Added some tests, deploy to staging.
Evergreen PR
#9915