Skip to content

fix: implement device auth rate limit handling #17079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions site/src/components/GitDeviceAuth/GitDeviceAuth.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { AxiosError, type AxiosResponse } from "axios";
import { newRetryDelay } from "./GitDeviceAuth";

test("device auth retry delay", async () => {
const slowDownError = new AxiosError(
"slow_down",
"500",
undefined,
undefined,
{
data: {
detail: "slow_down",
},
} as AxiosResponse,
);
const retryDelay = newRetryDelay(undefined);

// If no initial interval is provided, the default must be 5 seconds.
expect(retryDelay(0, undefined)).toBe(5000);
// If the error is a slow down error, the interval should increase by 5 seconds
// for this and all subsequent requests, and by 5 seconds extra delay for this
// request.
expect(retryDelay(1, slowDownError)).toBe(15000);
expect(retryDelay(1, slowDownError)).toBe(15000);
expect(retryDelay(2, undefined)).toBe(10000);

// Like previous request.
expect(retryDelay(3, slowDownError)).toBe(20000);
expect(retryDelay(3, undefined)).toBe(15000);
// If the error is not a slow down error, the interval should not increase.
expect(retryDelay(4, new AxiosError("other", "500"))).toBe(15000);

// If the initial interval is provided, it should be used.
const retryDelayWithInitialInterval = newRetryDelay(1);
expect(retryDelayWithInitialInterval(0, undefined)).toBe(1000);
expect(retryDelayWithInitialInterval(1, slowDownError)).toBe(11000);
expect(retryDelayWithInitialInterval(2, undefined)).toBe(6000);
});
70 changes: 67 additions & 3 deletions site/src/components/GitDeviceAuth/GitDeviceAuth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import CircularProgress from "@mui/material/CircularProgress";
import Link from "@mui/material/Link";
import type { ApiErrorResponse } from "api/errors";
import type { ExternalAuthDevice } from "api/typesGenerated";
import { isAxiosError } from "axios";
import { Alert, AlertDetail } from "components/Alert/Alert";
import { CopyButton } from "components/CopyButton/CopyButton";
import type { FC } from "react";
Expand All @@ -14,6 +15,59 @@ interface GitDeviceAuthProps {
deviceExchangeError?: ApiErrorResponse;
}

const DeviceExchangeError = {
AuthorizationPending: "authorization_pending",
SlowDown: "slow_down",
ExpiredToken: "expired_token",
AccessDenied: "access_denied",
} as const;

export const isExchangeErrorRetryable = (_: number, error: unknown) => {
if (!isAxiosError(error)) {
return false;
}
const detail = error.response?.data?.detail;
return (
detail === DeviceExchangeError.AuthorizationPending ||
detail === DeviceExchangeError.SlowDown
);
};

/**
* The OAuth2 specification (https://datatracker.ietf.org/doc/html/rfc8628)
* describes how the client should handle retries. This function returns a
* closure that implements the retry logic described in the specification.
* The closure should be memoized because it stores state.
*/
export const newRetryDelay = (initialInterval: number | undefined) => {
// "If no value is provided, clients MUST use 5 as the default."
// https://datatracker.ietf.org/doc/html/rfc8628#section-3.2
let interval = initialInterval ?? 5;
let lastFailureCountHandled = 0;
return (failureCount: number, error: unknown) => {
const isSlowDown =
isAxiosError(error) &&
error.response?.data.detail === DeviceExchangeError.SlowDown;
// We check the failure count to ensure we increase the interval
// at most once per failure.
if (isSlowDown && lastFailureCountHandled < failureCount) {
lastFailureCountHandled = failureCount;
// https://datatracker.ietf.org/doc/html/rfc8628#section-3.5
// "the interval MUST be increased by 5 seconds for this and all subsequent requests"
interval += 5;
}
let extraDelay = 0;
if (isSlowDown) {
// I found GitHub is very strict about their rate limits, and they'll block
// even if the request is 500ms earlier than they expect. This may happen due to
// e.g. network latency, so it's best to cool down for longer if GitHub just
// rejected our request.
extraDelay = 5;
}
return (interval + extraDelay) * 1000;
};
};

export const GitDeviceAuth: FC<GitDeviceAuthProps> = ({
externalAuthDevice,
deviceExchangeError,
Expand All @@ -27,16 +81,26 @@ export const GitDeviceAuth: FC<GitDeviceAuthProps> = ({
if (deviceExchangeError) {
// See https://datatracker.ietf.org/doc/html/rfc8628#section-3.5
switch (deviceExchangeError.detail) {
case "authorization_pending":
case DeviceExchangeError.AuthorizationPending:
break;
case DeviceExchangeError.SlowDown:
status = (
<div>
{status}
<Alert severity="warning">
Rate limit reached. Waiting a few seconds before retrying...
</Alert>
</div>
);
break;
case "expired_token":
case DeviceExchangeError.ExpiredToken:
status = (
<Alert severity="error">
The one-time code has expired. Refresh to get a new one!
</Alert>
);
break;
case "access_denied":
case DeviceExchangeError.AccessDenied:
status = (
<Alert severity="error">Access to the Git provider was denied.</Alert>
);
Expand Down
18 changes: 14 additions & 4 deletions site/src/pages/ExternalAuthPage/ExternalAuthPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@ import {
externalAuthProvider,
} from "api/queries/externalAuth";
import { isAxiosError } from "axios";
import {
isExchangeErrorRetryable,
newRetryDelay,
} from "components/GitDeviceAuth/GitDeviceAuth";
import { SignInLayout } from "components/SignInLayout/SignInLayout";
import { Welcome } from "components/Welcome/Welcome";
import { useAuthenticated } from "contexts/auth/RequireAuth";
import type { FC } from "react";
import { useMemo } from "react";
import { useQuery, useQueryClient } from "react-query";
import { useParams, useSearchParams } from "react-router-dom";
import ExternalAuthPageView from "./ExternalAuthPageView";
Expand All @@ -32,17 +37,22 @@ const ExternalAuthPage: FC = () => {
Boolean(externalAuthProviderQuery.data?.device),
refetchOnMount: false,
});
const retryDelay = useMemo(
() => newRetryDelay(externalAuthDeviceQuery.data?.interval),
[externalAuthDeviceQuery.data],
);
const exchangeExternalAuthDeviceQuery = useQuery({
...exchangeExternalAuthDevice(
provider,
externalAuthDeviceQuery.data?.device_code ?? "",
queryClient,
),
enabled: Boolean(externalAuthDeviceQuery.data),
retry: true,
retryDelay: (externalAuthDeviceQuery.data?.interval || 5) * 1000,
refetchOnWindowFocus: (query) =>
query.state.status === "success" ? false : "always",
retry: isExchangeErrorRetryable,
retryDelay,
// We don't want to refetch the query outside of the standard retry
// logic, because the device auth flow is very strict about rate limits.
refetchOnWindowFocus: false,
});

if (externalAuthProviderQuery.isLoading || !externalAuthProviderQuery.data) {
Expand Down
31 changes: 16 additions & 15 deletions site/src/pages/LoginOAuthDevicePage/LoginOAuthDevicePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,18 @@ import {
getGitHubDeviceFlowCallback,
} from "api/queries/oauth2";
import { isAxiosError } from "axios";
import {
isExchangeErrorRetryable,
newRetryDelay,
} from "components/GitDeviceAuth/GitDeviceAuth";
import { SignInLayout } from "components/SignInLayout/SignInLayout";
import { Welcome } from "components/Welcome/Welcome";
import { useEffect } from "react";
import { useEffect, useMemo } from "react";
import type { FC } from "react";
import { useQuery } from "react-query";
import { useSearchParams } from "react-router-dom";
import LoginOAuthDevicePageView from "./LoginOAuthDevicePageView";

const isErrorRetryable = (error: unknown) => {
if (!isAxiosError(error)) {
return false;
}
return error.response?.data?.detail === "authorization_pending";
};

// The page is hardcoded to only use GitHub,
// as that's the only OAuth2 login provider in our backend
// that currently supports the device flow.
Expand All @@ -38,19 +35,23 @@ const LoginOAuthDevicePage: FC = () => {
...getGitHubDevice(),
refetchOnMount: false,
});

const retryDelay = useMemo(
() => newRetryDelay(externalAuthDeviceQuery.data?.interval),
[externalAuthDeviceQuery.data],
);

const exchangeExternalAuthDeviceQuery = useQuery({
...getGitHubDeviceFlowCallback(
externalAuthDeviceQuery.data?.device_code ?? "",
state,
),
enabled: Boolean(externalAuthDeviceQuery.data),
retry: (_, error) => isErrorRetryable(error),
retryDelay: (externalAuthDeviceQuery.data?.interval || 5) * 1000,
refetchOnWindowFocus: (query) =>
query.state.status === "success" ||
(query.state.error != null && !isErrorRetryable(query.state.error))
? false
: "always",
retry: isExchangeErrorRetryable,
retryDelay,
// We don't want to refetch the query outside of the standard retry
// logic, because the device auth flow is very strict about rate limits.
refetchOnWindowFocus: false,
});

useEffect(() => {
Expand Down
Loading