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

Commitd66e6e7

Browse files
authored
fix: always attempt external auth refresh when fetching (#11762) (#11830)
* fix: always attempt external auth refresh when fetching* refactor validate to check expiry when considering "valid"
1 parenteeef56a commitd66e6e7

File tree

6 files changed

+129
-80
lines changed

6 files changed

+129
-80
lines changed

‎coderd/database/modelmethods.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"time"
77

88
"golang.org/x/exp/maps"
9+
"golang.org/x/oauth2"
910

1011
"github.com/coder/coder/v2/coderd/database/dbtime"
1112
"github.com/coder/coder/v2/coderd/rbac"
@@ -268,6 +269,14 @@ func (u ExternalAuthLink) RBACObject() rbac.Object {
268269
returnrbac.ResourceUserData.WithID(u.UserID).WithOwner(u.UserID.String())
269270
}
270271

272+
func (uExternalAuthLink)OAuthToken()*oauth2.Token {
273+
return&oauth2.Token{
274+
AccessToken:u.OAuthAccessToken,
275+
RefreshToken:u.OAuthRefreshToken,
276+
Expiry:u.OAuthExpiry,
277+
}
278+
}
279+
271280
func (uUserLink)RBACObject() rbac.Object {
272281
// I assume UserData is ok?
273282
returnrbac.ResourceUserData.WithOwner(u.UserID.String()).WithID(u.UserID)

‎coderd/externalauth.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (api *API) externalAuthByID(w http.ResponseWriter, r *http.Request) {
5757
}
5858
vareg errgroup.Group
5959
eg.Go(func() (errerror) {
60-
res.Authenticated,res.User,err=config.ValidateToken(ctx,link.OAuthAccessToken)
60+
res.Authenticated,res.User,err=config.ValidateToken(ctx,link.OAuthToken())
6161
returnerr
6262
})
6363
eg.Go(func() (errerror) {

‎coderd/externalauth/externalauth.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ func (c *Config) RefreshToken(ctx context.Context, db database.Store, externalAu
138138
retryCtx,retryCtxCancel:=context.WithTimeout(ctx,time.Second)
139139
deferretryCtxCancel()
140140
validate:
141-
valid,_,err:=c.ValidateToken(ctx,token.AccessToken)
141+
valid,_,err:=c.ValidateToken(ctx,token)
142142
iferr!=nil {
143143
returnexternalAuthLink,false,xerrors.Errorf("validate external auth token: %w",err)
144144
}
@@ -179,7 +179,14 @@ validate:
179179

180180
// ValidateToken ensures the Git token provided is valid!
181181
// The user is optionally returned if the provider supports it.
182-
func (c*Config)ValidateToken(ctx context.Context,tokenstring) (bool,*codersdk.ExternalAuthUser,error) {
182+
func (c*Config)ValidateToken(ctx context.Context,link*oauth2.Token) (bool,*codersdk.ExternalAuthUser,error) {
183+
iflink==nil {
184+
returnfalse,nil,xerrors.New("validate external auth token: token is nil")
185+
}
186+
if!link.Expiry.IsZero()&&link.Expiry.Before(dbtime.Now()) {
187+
returnfalse,nil,nil
188+
}
189+
183190
ifc.ValidateURL=="" {
184191
// Default that the token is valid if no validation URL is provided.
185192
returntrue,nil,nil
@@ -189,7 +196,7 @@ func (c *Config) ValidateToken(ctx context.Context, token string) (bool, *coders
189196
returnfalse,nil,err
190197
}
191198

192-
req.Header.Set("Authorization",fmt.Sprintf("Bearer %s",token))
199+
req.Header.Set("Authorization",fmt.Sprintf("Bearer %s",link.AccessToken))
193200
res,err:=c.InstrumentedOAuth2Config.Do(ctx,promoauth.SourceValidateToken,req)
194201
iferr!=nil {
195202
returnfalse,nil,err
@@ -396,10 +403,15 @@ func (c *DeviceAuth) ExchangeDeviceCode(ctx context.Context, deviceCode string)
396403
ifbody.Error!="" {
397404
returnnil,xerrors.New(body.Error)
398405
}
406+
// If expiresIn is 0, then the token never expires.
407+
expires:=dbtime.Now().Add(time.Duration(body.ExpiresIn)*time.Second)
408+
ifbody.ExpiresIn==0 {
409+
expires= time.Time{}
410+
}
399411
return&oauth2.Token{
400412
AccessToken:body.AccessToken,
401413
RefreshToken:body.RefreshToken,
402-
Expiry:dbtime.Now().Add(time.Duration(body.ExpiresIn)*time.Second),
414+
Expiry:expires,
403415
},nil
404416
}
405417

‎coderd/promoauth/oauth2_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func TestInstrument(t *testing.T) {
7575
require.Equal(t,count("TokenSource"),1)
7676

7777
// Try a validate
78-
valid,_,err:=cfg.ValidateToken(ctx,refreshed.AccessToken)
78+
valid,_,err:=cfg.ValidateToken(ctx,refreshed)
7979
require.NoError(t,err)
8080
require.True(t,valid)
8181
require.Equal(t,count("ValidateToken"),1)

‎coderd/workspaceagents.go

Lines changed: 99 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2037,78 +2037,26 @@ func (api *API) workspaceAgentsExternalAuth(rw http.ResponseWriter, r *http.Requ
20372037
return
20382038
}
20392039

2040-
iflisten {
2041-
// Since we're ticking frequently and this sign-in operation is rare,
2042-
// we are OK with polling to avoid the complexity of pubsub.
2043-
ticker,done:=api.NewTicker(time.Second)
2044-
deferdone()
2045-
varpreviousToken database.ExternalAuthLink
2046-
for {
2047-
select {
2048-
case<-ctx.Done():
2049-
return
2050-
case<-ticker:
2051-
}
2052-
externalAuthLink,err:=api.Database.GetExternalAuthLink(ctx, database.GetExternalAuthLinkParams{
2053-
ProviderID:externalAuthConfig.ID,
2054-
UserID:workspace.OwnerID,
2055-
})
2056-
iferr!=nil {
2057-
iferrors.Is(err,sql.ErrNoRows) {
2058-
continue
2059-
}
2060-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2061-
Message:"Failed to get external auth link.",
2062-
Detail:err.Error(),
2063-
})
2064-
return
2065-
}
2066-
2067-
// Expiry may be unset if the application doesn't configure tokens
2068-
// to expire.
2069-
// See
2070-
// https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/generating-a-user-access-token-for-a-github-app.
2071-
ifexternalAuthLink.OAuthExpiry.Before(dbtime.Now())&&!externalAuthLink.OAuthExpiry.IsZero() {
2072-
continue
2073-
}
2074-
2075-
// Only attempt to revalidate an oauth token if it has actually changed.
2076-
// No point in trying to validate the same token over and over again.
2077-
ifpreviousToken.OAuthAccessToken==externalAuthLink.OAuthAccessToken&&
2078-
previousToken.OAuthRefreshToken==externalAuthLink.OAuthRefreshToken&&
2079-
previousToken.OAuthExpiry==externalAuthLink.OAuthExpiry {
2080-
continue
2081-
}
2082-
2083-
valid,_,err:=externalAuthConfig.ValidateToken(ctx,externalAuthLink.OAuthAccessToken)
2084-
iferr!=nil {
2085-
api.Logger.Warn(ctx,"failed to validate external auth token",
2086-
slog.F("workspace_owner_id",workspace.OwnerID.String()),
2087-
slog.F("validate_url",externalAuthConfig.ValidateURL),
2088-
slog.Error(err),
2089-
)
2090-
}
2091-
previousToken=externalAuthLink
2092-
if!valid {
2093-
continue
2094-
}
2095-
resp,err:=createExternalAuthResponse(externalAuthConfig.Type,externalAuthLink.OAuthAccessToken,externalAuthLink.OAuthExtra)
2096-
iferr!=nil {
2097-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2098-
Message:"Failed to create external auth response.",
2099-
Detail:err.Error(),
2100-
})
2101-
return
2102-
}
2103-
httpapi.Write(ctx,rw,http.StatusOK,resp)
2040+
varpreviousToken*database.ExternalAuthLink
2041+
// handleRetrying will attempt to continually check for a new token
2042+
// if listen is true. This is useful if an error is encountered in the
2043+
// original single flow.
2044+
//
2045+
// By default, if no errors are encountered, then the single flow response
2046+
// is returned.
2047+
handleRetrying:=func(codeint,responseany) {
2048+
if!listen {
2049+
httpapi.Write(ctx,rw,code,response)
21042050
return
21052051
}
2052+
2053+
api.workspaceAgentsExternalAuthListen(ctx,rw,previousToken,externalAuthConfig,workspace)
21062054
}
21072055

21082056
// This is the URL that will redirect the user with a state token.
21092057
redirectURL,err:=api.AccessURL.Parse(fmt.Sprintf("/external-auth/%s",externalAuthConfig.ID))
21102058
iferr!=nil {
2111-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2059+
handleRetrying(http.StatusInternalServerError, codersdk.Response{
21122060
Message:"Failed to parse access URL.",
21132061
Detail:err.Error(),
21142062
})
@@ -2121,36 +2069,40 @@ func (api *API) workspaceAgentsExternalAuth(rw http.ResponseWriter, r *http.Requ
21212069
})
21222070
iferr!=nil {
21232071
if!errors.Is(err,sql.ErrNoRows) {
2124-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2072+
handleRetrying(http.StatusInternalServerError, codersdk.Response{
21252073
Message:"Failed to get external auth link.",
21262074
Detail:err.Error(),
21272075
})
21282076
return
21292077
}
21302078

2131-
httpapi.Write(ctx,rw,http.StatusOK, agentsdk.ExternalAuthResponse{
2079+
handleRetrying(http.StatusOK, agentsdk.ExternalAuthResponse{
21322080
URL:redirectURL.String(),
21332081
})
21342082
return
21352083
}
21362084

2137-
externalAuthLink,updated,err:=externalAuthConfig.RefreshToken(ctx,api.Database,externalAuthLink)
2085+
externalAuthLink,valid,err:=externalAuthConfig.RefreshToken(ctx,api.Database,externalAuthLink)
21382086
iferr!=nil {
2139-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2087+
handleRetrying(http.StatusInternalServerError, codersdk.Response{
21402088
Message:"Failed to refresh external auth token.",
21412089
Detail:err.Error(),
21422090
})
21432091
return
21442092
}
2145-
if!updated {
2146-
httpapi.Write(ctx,rw,http.StatusOK, agentsdk.ExternalAuthResponse{
2093+
if!valid {
2094+
// Set the previous token so the retry logic will skip validating the
2095+
// same token again. This should only be set if the token is invalid and there
2096+
// was no error. If it is invalid because of an error, then we should recheck.
2097+
previousToken=&externalAuthLink
2098+
handleRetrying(http.StatusOK, agentsdk.ExternalAuthResponse{
21472099
URL:redirectURL.String(),
21482100
})
21492101
return
21502102
}
21512103
resp,err:=createExternalAuthResponse(externalAuthConfig.Type,externalAuthLink.OAuthAccessToken,externalAuthLink.OAuthExtra)
21522104
iferr!=nil {
2153-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2105+
handleRetrying(http.StatusInternalServerError, codersdk.Response{
21542106
Message:"Failed to create external auth response.",
21552107
Detail:err.Error(),
21562108
})
@@ -2159,6 +2111,81 @@ func (api *API) workspaceAgentsExternalAuth(rw http.ResponseWriter, r *http.Requ
21592111
httpapi.Write(ctx,rw,http.StatusOK,resp)
21602112
}
21612113

2114+
func (api*API)workspaceAgentsExternalAuthListen(ctx context.Context,rw http.ResponseWriter,previous*database.ExternalAuthLink,externalAuthConfig*externalauth.Config,workspace database.Workspace) {
2115+
// Since we're ticking frequently and this sign-in operation is rare,
2116+
// we are OK with polling to avoid the complexity of pubsub.
2117+
ticker,done:=api.NewTicker(time.Second)
2118+
deferdone()
2119+
// If we have a previous token that is invalid, we should not check this again.
2120+
// This serves to prevent doing excessive unauthorized requests to the external
2121+
// auth provider. For github, this limit is 60 per hour, so saving a call
2122+
// per invalid token can be significant.
2123+
varpreviousToken database.ExternalAuthLink
2124+
ifprevious!=nil {
2125+
previousToken=*previous
2126+
}
2127+
for {
2128+
select {
2129+
case<-ctx.Done():
2130+
return
2131+
case<-ticker:
2132+
}
2133+
externalAuthLink,err:=api.Database.GetExternalAuthLink(ctx, database.GetExternalAuthLinkParams{
2134+
ProviderID:externalAuthConfig.ID,
2135+
UserID:workspace.OwnerID,
2136+
})
2137+
iferr!=nil {
2138+
iferrors.Is(err,sql.ErrNoRows) {
2139+
continue
2140+
}
2141+
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2142+
Message:"Failed to get external auth link.",
2143+
Detail:err.Error(),
2144+
})
2145+
return
2146+
}
2147+
2148+
// Expiry may be unset if the application doesn't configure tokens
2149+
// to expire.
2150+
// See
2151+
// https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/generating-a-user-access-token-for-a-github-app.
2152+
ifexternalAuthLink.OAuthExpiry.Before(dbtime.Now())&&!externalAuthLink.OAuthExpiry.IsZero() {
2153+
continue
2154+
}
2155+
2156+
// Only attempt to revalidate an oauth token if it has actually changed.
2157+
// No point in trying to validate the same token over and over again.
2158+
ifpreviousToken.OAuthAccessToken==externalAuthLink.OAuthAccessToken&&
2159+
previousToken.OAuthRefreshToken==externalAuthLink.OAuthRefreshToken&&
2160+
previousToken.OAuthExpiry==externalAuthLink.OAuthExpiry {
2161+
continue
2162+
}
2163+
2164+
valid,_,err:=externalAuthConfig.ValidateToken(ctx,externalAuthLink.OAuthToken())
2165+
iferr!=nil {
2166+
api.Logger.Warn(ctx,"failed to validate external auth token",
2167+
slog.F("workspace_owner_id",workspace.OwnerID.String()),
2168+
slog.F("validate_url",externalAuthConfig.ValidateURL),
2169+
slog.Error(err),
2170+
)
2171+
}
2172+
previousToken=externalAuthLink
2173+
if!valid {
2174+
continue
2175+
}
2176+
resp,err:=createExternalAuthResponse(externalAuthConfig.Type,externalAuthLink.OAuthAccessToken,externalAuthLink.OAuthExtra)
2177+
iferr!=nil {
2178+
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2179+
Message:"Failed to create external auth response.",
2180+
Detail:err.Error(),
2181+
})
2182+
return
2183+
}
2184+
httpapi.Write(ctx,rw,http.StatusOK,resp)
2185+
return
2186+
}
2187+
}
2188+
21622189
// createExternalAuthResponse creates an ExternalAuthResponse based on the
21632190
// provider type. This is to support legacy `/workspaceagents/me/gitauth`
21642191
// which uses `Username` and `Password`.

‎coderd/workspaceagents_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,7 +1577,7 @@ func TestWorkspaceAgentExternalAuthListen(t *testing.T) {
15771577
},
15781578
ExternalAuthConfigs: []*externalauth.Config{
15791579
fake.ExternalAuthConfig(t,providerID,nil,func(cfg*externalauth.Config) {
1580-
cfg.Type=codersdk.EnhancedExternalAuthProviderGitHub.String()
1580+
cfg.Type=codersdk.EnhancedExternalAuthProviderGitLab.String()
15811581
}),
15821582
},
15831583
})
@@ -1623,7 +1623,8 @@ func TestWorkspaceAgentExternalAuthListen(t *testing.T) {
16231623
ticks<-time.Now()
16241624
}
16251625
cancel()
1626-
// We expect only 1
1626+
// We expect only 1. One from the initial "Refresh" attempt, and the
1627+
// other should be skipped.
16271628
// In a failed test, you will likely see 9, as the last one
16281629
// gets canceled.
16291630
require.Equal(t,1,validateCalls,"validate calls duplicated on same token")

0 commit comments

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