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

Commit5b0d243

Browse files
committed
fix: always attempt external auth refresh when fetching (#11762)
* fix: always attempt external auth refresh when fetching* refactor validate to check expiry when considering "valid"
1 parent73a6899 commit5b0d243

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
@@ -2031,78 +2031,26 @@ func (api *API) workspaceAgentsExternalAuth(rw http.ResponseWriter, r *http.Requ
20312031
return
20322032
}
20332033

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

21022050
// This is the URL that will redirect the user with a state token.
21032051
redirectURL,err:=api.AccessURL.Parse(fmt.Sprintf("/external-auth/%s",externalAuthConfig.ID))
21042052
iferr!=nil {
2105-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2053+
handleRetrying(http.StatusInternalServerError, codersdk.Response{
21062054
Message:"Failed to parse access URL.",
21072055
Detail:err.Error(),
21082056
})
@@ -2115,36 +2063,40 @@ func (api *API) workspaceAgentsExternalAuth(rw http.ResponseWriter, r *http.Requ
21152063
})
21162064
iferr!=nil {
21172065
if!errors.Is(err,sql.ErrNoRows) {
2118-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2066+
handleRetrying(http.StatusInternalServerError, codersdk.Response{
21192067
Message:"Failed to get external auth link.",
21202068
Detail:err.Error(),
21212069
})
21222070
return
21232071
}
21242072

2125-
httpapi.Write(ctx,rw,http.StatusOK, agentsdk.ExternalAuthResponse{
2073+
handleRetrying(http.StatusOK, agentsdk.ExternalAuthResponse{
21262074
URL:redirectURL.String(),
21272075
})
21282076
return
21292077
}
21302078

2131-
externalAuthLink,updated,err:=externalAuthConfig.RefreshToken(ctx,api.Database,externalAuthLink)
2079+
externalAuthLink,valid,err:=externalAuthConfig.RefreshToken(ctx,api.Database,externalAuthLink)
21322080
iferr!=nil {
2133-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2081+
handleRetrying(http.StatusInternalServerError, codersdk.Response{
21342082
Message:"Failed to refresh external auth token.",
21352083
Detail:err.Error(),
21362084
})
21372085
return
21382086
}
2139-
if!updated {
2140-
httpapi.Write(ctx,rw,http.StatusOK, agentsdk.ExternalAuthResponse{
2087+
if!valid {
2088+
// Set the previous token so the retry logic will skip validating the
2089+
// same token again. This should only be set if the token is invalid and there
2090+
// was no error. If it is invalid because of an error, then we should recheck.
2091+
previousToken=&externalAuthLink
2092+
handleRetrying(http.StatusOK, agentsdk.ExternalAuthResponse{
21412093
URL:redirectURL.String(),
21422094
})
21432095
return
21442096
}
21452097
resp,err:=createExternalAuthResponse(externalAuthConfig.Type,externalAuthLink.OAuthAccessToken,externalAuthLink.OAuthExtra)
21462098
iferr!=nil {
2147-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
2099+
handleRetrying(http.StatusInternalServerError, codersdk.Response{
21482100
Message:"Failed to create external auth response.",
21492101
Detail:err.Error(),
21502102
})
@@ -2153,6 +2105,81 @@ func (api *API) workspaceAgentsExternalAuth(rw http.ResponseWriter, r *http.Requ
21532105
httpapi.Write(ctx,rw,http.StatusOK,resp)
21542106
}
21552107

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