Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd5557fc

Browse files
authored
fix: implement device auth rate limit handling (#17079)
The [OAuth2specification](https://datatracker.ietf.org/doc/html/rfc8628) describeshow clients in the device flow should handle retrying requests when theyare rate limited.We didn't respect it, which sometimes prevented users from logging in orsetting up external auth. They'd see a `slow_down` error in the UI andwould be unable to complete the authentication flow. This PR implementsrate limit handling according to the spec.
1 parent7b65422 commitd5557fc

File tree

4 files changed

+135
-22
lines changed

4 files changed

+135
-22
lines changed
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import{AxiosError,typeAxiosResponse}from"axios";
2+
import{newRetryDelay}from"./GitDeviceAuth";
3+
4+
test("device auth retry delay",async()=>{
5+
constslowDownError=newAxiosError(
6+
"slow_down",
7+
"500",
8+
undefined,
9+
undefined,
10+
{
11+
data:{
12+
detail:"slow_down",
13+
},
14+
}asAxiosResponse,
15+
);
16+
constretryDelay=newRetryDelay(undefined);
17+
18+
// If no initial interval is provided, the default must be 5 seconds.
19+
expect(retryDelay(0,undefined)).toBe(5000);
20+
// If the error is a slow down error, the interval should increase by 5 seconds
21+
// for this and all subsequent requests, and by 5 seconds extra delay for this
22+
// request.
23+
expect(retryDelay(1,slowDownError)).toBe(15000);
24+
expect(retryDelay(1,slowDownError)).toBe(15000);
25+
expect(retryDelay(2,undefined)).toBe(10000);
26+
27+
// Like previous request.
28+
expect(retryDelay(3,slowDownError)).toBe(20000);
29+
expect(retryDelay(3,undefined)).toBe(15000);
30+
// If the error is not a slow down error, the interval should not increase.
31+
expect(retryDelay(4,newAxiosError("other","500"))).toBe(15000);
32+
33+
// If the initial interval is provided, it should be used.
34+
constretryDelayWithInitialInterval=newRetryDelay(1);
35+
expect(retryDelayWithInitialInterval(0,undefined)).toBe(1000);
36+
expect(retryDelayWithInitialInterval(1,slowDownError)).toBe(11000);
37+
expect(retryDelayWithInitialInterval(2,undefined)).toBe(6000);
38+
});

‎site/src/components/GitDeviceAuth/GitDeviceAuth.tsx

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import CircularProgress from "@mui/material/CircularProgress";
55
importLinkfrom"@mui/material/Link";
66
importtype{ApiErrorResponse}from"api/errors";
77
importtype{ExternalAuthDevice}from"api/typesGenerated";
8+
import{isAxiosError}from"axios";
89
import{Alert,AlertDetail}from"components/Alert/Alert";
910
import{CopyButton}from"components/CopyButton/CopyButton";
1011
importtype{FC}from"react";
@@ -14,6 +15,59 @@ interface GitDeviceAuthProps {
1415
deviceExchangeError?:ApiErrorResponse;
1516
}
1617

18+
constDeviceExchangeError={
19+
AuthorizationPending:"authorization_pending",
20+
SlowDown:"slow_down",
21+
ExpiredToken:"expired_token",
22+
AccessDenied:"access_denied",
23+
}asconst;
24+
25+
exportconstisExchangeErrorRetryable=(_:number,error:unknown)=>{
26+
if(!isAxiosError(error)){
27+
returnfalse;
28+
}
29+
constdetail=error.response?.data?.detail;
30+
return(
31+
detail===DeviceExchangeError.AuthorizationPending||
32+
detail===DeviceExchangeError.SlowDown
33+
);
34+
};
35+
36+
/**
37+
* The OAuth2 specification (https://datatracker.ietf.org/doc/html/rfc8628)
38+
* describes how the client should handle retries. This function returns a
39+
* closure that implements the retry logic described in the specification.
40+
* The closure should be memoized because it stores state.
41+
*/
42+
exportconstnewRetryDelay=(initialInterval:number|undefined)=>{
43+
// "If no value is provided, clients MUST use 5 as the default."
44+
// https://datatracker.ietf.org/doc/html/rfc8628#section-3.2
45+
letinterval=initialInterval??5;
46+
letlastFailureCountHandled=0;
47+
return(failureCount:number,error:unknown)=>{
48+
constisSlowDown=
49+
isAxiosError(error)&&
50+
error.response?.data.detail===DeviceExchangeError.SlowDown;
51+
// We check the failure count to ensure we increase the interval
52+
// at most once per failure.
53+
if(isSlowDown&&lastFailureCountHandled<failureCount){
54+
lastFailureCountHandled=failureCount;
55+
// https://datatracker.ietf.org/doc/html/rfc8628#section-3.5
56+
// "the interval MUST be increased by 5 seconds for this and all subsequent requests"
57+
interval+=5;
58+
}
59+
letextraDelay=0;
60+
if(isSlowDown){
61+
// I found GitHub is very strict about their rate limits, and they'll block
62+
// even if the request is 500ms earlier than they expect. This may happen due to
63+
// e.g. network latency, so it's best to cool down for longer if GitHub just
64+
// rejected our request.
65+
extraDelay=5;
66+
}
67+
return(interval+extraDelay)*1000;
68+
};
69+
};
70+
1771
exportconstGitDeviceAuth:FC<GitDeviceAuthProps>=({
1872
externalAuthDevice,
1973
deviceExchangeError,
@@ -27,16 +81,26 @@ export const GitDeviceAuth: FC<GitDeviceAuthProps> = ({
2781
if(deviceExchangeError){
2882
// See https://datatracker.ietf.org/doc/html/rfc8628#section-3.5
2983
switch(deviceExchangeError.detail){
30-
case"authorization_pending":
84+
caseDeviceExchangeError.AuthorizationPending:
85+
break;
86+
caseDeviceExchangeError.SlowDown:
87+
status=(
88+
<div>
89+
{status}
90+
<Alertseverity="warning">
91+
Rate limit reached. Waiting a few seconds before retrying...
92+
</Alert>
93+
</div>
94+
);
3195
break;
32-
case"expired_token":
96+
caseDeviceExchangeError.ExpiredToken:
3397
status=(
3498
<Alertseverity="error">
3599
The one-time code has expired. Refresh to get a new one!
36100
</Alert>
37101
);
38102
break;
39-
case"access_denied":
103+
caseDeviceExchangeError.AccessDenied:
40104
status=(
41105
<Alertseverity="error">Access to the Git provider was denied.</Alert>
42106
);

‎site/src/pages/ExternalAuthPage/ExternalAuthPage.tsx

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,15 @@ import {
66
externalAuthProvider,
77
}from"api/queries/externalAuth";
88
import{isAxiosError}from"axios";
9+
import{
10+
isExchangeErrorRetryable,
11+
newRetryDelay,
12+
}from"components/GitDeviceAuth/GitDeviceAuth";
913
import{SignInLayout}from"components/SignInLayout/SignInLayout";
1014
import{Welcome}from"components/Welcome/Welcome";
1115
import{useAuthenticated}from"contexts/auth/RequireAuth";
1216
importtype{FC}from"react";
17+
import{useMemo}from"react";
1318
import{useQuery,useQueryClient}from"react-query";
1419
import{useParams,useSearchParams}from"react-router-dom";
1520
importExternalAuthPageViewfrom"./ExternalAuthPageView";
@@ -32,17 +37,22 @@ const ExternalAuthPage: FC = () => {
3237
Boolean(externalAuthProviderQuery.data?.device),
3338
refetchOnMount:false,
3439
});
40+
constretryDelay=useMemo(
41+
()=>newRetryDelay(externalAuthDeviceQuery.data?.interval),
42+
[externalAuthDeviceQuery.data],
43+
);
3544
constexchangeExternalAuthDeviceQuery=useQuery({
3645
...exchangeExternalAuthDevice(
3746
provider,
3847
externalAuthDeviceQuery.data?.device_code??"",
3948
queryClient,
4049
),
4150
enabled:Boolean(externalAuthDeviceQuery.data),
42-
retry:true,
43-
retryDelay:(externalAuthDeviceQuery.data?.interval||5)*1000,
44-
refetchOnWindowFocus:(query)=>
45-
query.state.status==="success" ?false :"always",
51+
retry:isExchangeErrorRetryable,
52+
retryDelay,
53+
// We don't want to refetch the query outside of the standard retry
54+
// logic, because the device auth flow is very strict about rate limits.
55+
refetchOnWindowFocus:false,
4656
});
4757

4858
if(externalAuthProviderQuery.isLoading||!externalAuthProviderQuery.data){

‎site/src/pages/LoginOAuthDevicePage/LoginOAuthDevicePage.tsx

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,18 @@ import {
44
getGitHubDeviceFlowCallback,
55
}from"api/queries/oauth2";
66
import{isAxiosError}from"axios";
7+
import{
8+
isExchangeErrorRetryable,
9+
newRetryDelay,
10+
}from"components/GitDeviceAuth/GitDeviceAuth";
711
import{SignInLayout}from"components/SignInLayout/SignInLayout";
812
import{Welcome}from"components/Welcome/Welcome";
9-
import{useEffect}from"react";
13+
import{useEffect,useMemo}from"react";
1014
importtype{FC}from"react";
1115
import{useQuery}from"react-query";
1216
import{useSearchParams}from"react-router-dom";
1317
importLoginOAuthDevicePageViewfrom"./LoginOAuthDevicePageView";
1418

15-
constisErrorRetryable=(error:unknown)=>{
16-
if(!isAxiosError(error)){
17-
returnfalse;
18-
}
19-
returnerror.response?.data?.detail==="authorization_pending";
20-
};
21-
2219
// The page is hardcoded to only use GitHub,
2320
// as that's the only OAuth2 login provider in our backend
2421
// that currently supports the device flow.
@@ -38,19 +35,23 @@ const LoginOAuthDevicePage: FC = () => {
3835
...getGitHubDevice(),
3936
refetchOnMount:false,
4037
});
38+
39+
constretryDelay=useMemo(
40+
()=>newRetryDelay(externalAuthDeviceQuery.data?.interval),
41+
[externalAuthDeviceQuery.data],
42+
);
43+
4144
constexchangeExternalAuthDeviceQuery=useQuery({
4245
...getGitHubDeviceFlowCallback(
4346
externalAuthDeviceQuery.data?.device_code??"",
4447
state,
4548
),
4649
enabled:Boolean(externalAuthDeviceQuery.data),
47-
retry:(_,error)=>isErrorRetryable(error),
48-
retryDelay:(externalAuthDeviceQuery.data?.interval||5)*1000,
49-
refetchOnWindowFocus:(query)=>
50-
query.state.status==="success"||
51-
(query.state.error!=null&&!isErrorRetryable(query.state.error))
52-
?false
53-
:"always",
50+
retry:isExchangeErrorRetryable,
51+
retryDelay,
52+
// We don't want to refetch the query outside of the standard retry
53+
// logic, because the device auth flow is very strict about rate limits.
54+
refetchOnWindowFocus:false,
5455
});
5556

5657
useEffect(()=>{

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp