diff --git a/site/src/api/queries/externalAuth.ts b/site/src/api/queries/externalAuth.ts index b0cd753cda..310df8ac95 100644 --- a/site/src/api/queries/externalAuth.ts +++ b/site/src/api/queries/externalAuth.ts @@ -27,7 +27,6 @@ export const externalAuthDevice = (providerId: string) => { export const exchangeExternalAuthDevice = ( providerId: string, deviceCode: string, - queryClient: QueryClient, ) => { return { queryFn: () => @@ -35,12 +34,6 @@ export const exchangeExternalAuthDevice = ( device_code: deviceCode, }), queryKey: ["external-auth", providerId, "device", deviceCode], - onSuccess: async () => { - // Force a refresh of the Git auth status. - await queryClient.invalidateQueries({ - queryKey: ["external-auth", providerId], - }); - }, }; }; diff --git a/site/src/pages/ExternalAuthPage/ExternalAuthPage.test.tsx b/site/src/pages/ExternalAuthPage/ExternalAuthPage.test.tsx new file mode 100644 index 0000000000..4144e147ea --- /dev/null +++ b/site/src/pages/ExternalAuthPage/ExternalAuthPage.test.tsx @@ -0,0 +1,81 @@ +import { screen, waitFor } from "@testing-library/react"; +import { HttpResponse, http } from "msw"; +import type { ExternalAuth, ExternalAuthDevice } from "#/api/typesGenerated"; +import { renderWithAuth } from "#/testHelpers/renderHelpers"; +import { server } from "#/testHelpers/server"; +import ExternalAuthPage from "./ExternalAuthPage"; + +const provider = "github"; + +const deviceResponse: ExternalAuthDevice = { + device_code: "device-code", + user_code: "1234-5678", + verification_uri: "https://github.com/login/device", + expires_in: 900, + interval: 0, +}; + +const baseProvider: ExternalAuth = { + authenticated: false, + device: true, + display_name: "GitHub", + supports_revocation: false, + user: null, + app_installable: false, + installations: [], + app_install_url: "", +}; + +const renderPage = () => + renderWithAuth(, { + route: `/external-auth/${provider}`, + path: "/external-auth/:provider", + }); + +describe("ExternalAuthPage", () => { + // Regression test for the device flow: after a successful device-code + // exchange the provider query must be invalidated so the page refetches and + // flips to the authenticated view. Prior to the fix the dropped react-query + // `onSuccess` left the UI stuck on the "Checking for authentication..." + // polling screen until a manual refresh. + it("refreshes the provider state after a successful device exchange", async () => { + let exchanged = false; + let providerRequests = 0; + + server.use( + http.get(`/api/v2/external-auth/${provider}`, () => { + providerRequests++; + return HttpResponse.json({ + ...baseProvider, + // The exchange marks the account authenticated. The page only + // observes this after it invalidates and refetches the query. + authenticated: exchanged, + }); + }), + http.get(`/api/v2/external-auth/${provider}/device`, () => + HttpResponse.json(deviceResponse), + ), + http.post(`/api/v2/external-auth/${provider}/device`, () => { + exchanged = true; + return new HttpResponse(null, { status: 204 }); + }), + ); + + renderPage(); + + // The polling screen renders first while the exchange is pending. + await screen.findByText("Authenticate with GitHub"); + + // Once the exchange succeeds, the provider query is invalidated and + // refetched, so the authenticated view appears without a manual refresh. + await waitFor(() => { + expect( + screen.getByText("You've authenticated with GitHub!"), + ).toBeInTheDocument(); + }); + + // The provider endpoint is hit more than once: the initial load plus the + // refetch triggered by the post-exchange invalidation. + expect(providerRequests).toBeGreaterThan(1); + }); +}); diff --git a/site/src/pages/ExternalAuthPage/ExternalAuthPage.tsx b/site/src/pages/ExternalAuthPage/ExternalAuthPage.tsx index f66a09c2da..2873063abe 100644 --- a/site/src/pages/ExternalAuthPage/ExternalAuthPage.tsx +++ b/site/src/pages/ExternalAuthPage/ExternalAuthPage.tsx @@ -1,6 +1,6 @@ import { isAxiosError } from "axios"; import type { FC } from "react"; -import { useMemo } from "react"; +import { useEffect, useMemo } from "react"; import { useQuery, useQueryClient } from "react-query"; import { useParams, useSearchParams } from "react-router"; import type { ApiErrorResponse } from "#/api/errors"; @@ -24,7 +24,10 @@ const ExternalAuthPage: FC = () => { const [searchParams] = useSearchParams(); const { permissions } = useAuthenticated(); const queryClient = useQueryClient(); - const externalAuthProviderOpts = externalAuthProvider(provider); + const externalAuthProviderOpts = useMemo( + () => externalAuthProvider(provider), + [provider], + ); const externalAuthProviderQuery = useQuery({ ...externalAuthProviderOpts, refetchOnWindowFocus: true, @@ -45,7 +48,6 @@ const ExternalAuthPage: FC = () => { ...exchangeExternalAuthDevice( provider, externalAuthDeviceQuery.data?.device_code ?? "", - queryClient, ), enabled: Boolean(externalAuthDeviceQuery.data), retry: isExchangeErrorRetryable, @@ -55,6 +57,23 @@ const ExternalAuthPage: FC = () => { refetchOnWindowFocus: false, }); + // Flip the UI out of polling once the exchange succeeds. Replaces the + // `onSuccess` that react-query v5 dropped from `useQuery`. `exact` avoids + // re-POSTing the one-time device code. + useEffect(() => { + if (!exchangeExternalAuthDeviceQuery.isSuccess) { + return; + } + queryClient.invalidateQueries({ + queryKey: externalAuthProviderOpts.queryKey, + exact: true, + }); + }, [ + exchangeExternalAuthDeviceQuery.isSuccess, + externalAuthProviderOpts.queryKey, + queryClient, + ]); + if (externalAuthProviderQuery.isLoading || !externalAuthProviderQuery.data) { return null; }