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

Commit8b82266

Browse files
committed
feat: add best effort attempt to revoke oauth access token in provider
1 parente7da80d commit8b82266

File tree

18 files changed

+363
-59
lines changed

18 files changed

+363
-59
lines changed

‎.vscode/settings.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,6 @@
6060
"typos.config":".github/workflows/typos.toml",
6161
"[markdown]": {
6262
"editor.defaultFormatter":"DavidAnson.vscode-markdownlint"
63-
}
63+
},
64+
"codeQL.githubDatabase.update":"never"
6465
}

‎coderd/apidoc/docs.go‎

Lines changed: 16 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json‎

Lines changed: 16 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/coderdtest/oidctest/idp.go‎

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,11 @@ type FakeIDP struct {
196196
// hookValidRedirectURL can be used to reject a redirect url from the
197197
// IDP -> Application. Almost all IDPs have the concept of
198198
// "Authorized Redirect URLs". This can be used to emulate that.
199-
hookValidRedirectURLfunc(redirectURLstring)error
200-
hookUserInfofunc(emailstring) (jwt.MapClaims,error)
201-
hookAccessTokenJWTfunc(emailstring,exp time.Time) jwt.MapClaims
199+
hookValidRedirectURLfunc(redirectURLstring)error
200+
hookUserInfofunc(emailstring) (jwt.MapClaims,error)
201+
hookRevokeTokenfunc() (int,error)
202+
revokeTokenGitHubFormatbool// GitHub doesn't follow token revocation RFC spec
203+
hookAccessTokenJWTfunc(emailstring,exp time.Time) jwt.MapClaims
202204
// defaultIDClaims is if a new client connects and we didn't preset
203205
// some claims.
204206
defaultIDClaims jwt.MapClaims
@@ -327,6 +329,19 @@ func WithStaticUserInfo(info jwt.MapClaims) func(*FakeIDP) {
327329
}
328330
}
329331

332+
funcWithRevokeTokenRFC(revokeFuncfunc() (int,error))func(*FakeIDP) {
333+
returnfunc(f*FakeIDP) {
334+
f.hookRevokeToken=revokeFunc
335+
}
336+
}
337+
338+
funcWithRevokeTokenGitHub(revokeFuncfunc() (int,error))func(*FakeIDP) {
339+
returnfunc(f*FakeIDP) {
340+
f.hookRevokeToken=revokeFunc
341+
f.revokeTokenGitHubFormat=true
342+
}
343+
}
344+
330345
funcWithDefaultIDClaims(claims jwt.MapClaims)func(*FakeIDP) {
331346
returnfunc(f*FakeIDP) {
332347
f.defaultIDClaims=claims
@@ -358,6 +373,7 @@ type With429Arguments struct {
358373
AuthorizePathbool
359374
KeysPathbool
360375
UserInfoPathbool
376+
RevokePathbool
361377
DeviceAuthbool
362378
DeviceVerifybool
363379
}
@@ -387,6 +403,10 @@ func With429(params With429Arguments) func(*FakeIDP) {
387403
http.Error(rw,"429, being manually blocked (userinfo)",http.StatusTooManyRequests)
388404
return
389405
}
406+
ifparams.RevokePath&&strings.Contains(r.URL.Path,revokeTokenPath) {
407+
http.Error(rw,"429, being manually blocked (revoke)",http.StatusTooManyRequests)
408+
return
409+
}
390410
ifparams.DeviceAuth&&strings.Contains(r.URL.Path,deviceAuth) {
391411
http.Error(rw,"429, being manually blocked (device-auth)",http.StatusTooManyRequests)
392412
return
@@ -408,8 +428,10 @@ const (
408428
authorizePath="/oauth2/authorize"
409429
keysPath="/oauth2/keys"
410430
userInfoPath="/oauth2/userinfo"
411-
deviceAuth="/login/device/code"
412-
deviceVerify="/login/device"
431+
// nolint:gosec // It also thinks this is a secret lol
432+
revokeTokenPath="/oauth2/revoke"
433+
deviceAuth="/login/device/code"
434+
deviceVerify="/login/device"
413435
)
414436

415437
funcNewFakeIDP(t testing.TB,opts...FakeIDPOpt)*FakeIDP {
@@ -486,6 +508,7 @@ func (f *FakeIDP) updateIssuerURL(t testing.TB, issuer string) {
486508
TokenURL:u.ResolveReference(&url.URL{Path:tokenPath}).String(),
487509
JWKSURL:u.ResolveReference(&url.URL{Path:keysPath}).String(),
488510
UserInfoURL:u.ResolveReference(&url.URL{Path:userInfoPath}).String(),
511+
RevokeURL:u.ResolveReference(&url.URL{Path:revokeTokenPath}).String(),
489512
DeviceCodeURL:u.ResolveReference(&url.URL{Path:deviceAuth}).String(),
490513
Algorithms: []string{
491514
"RS256",
@@ -756,6 +779,7 @@ type ProviderJSON struct {
756779
TokenURLstring`json:"token_endpoint"`
757780
JWKSURLstring`json:"jwks_uri"`
758781
UserInfoURLstring`json:"userinfo_endpoint"`
782+
RevokeURLstring`json:"revocation_endpoint"`
759783
DeviceCodeURLstring`json:"device_authorization_endpoint"`
760784
Algorithms []string`json:"id_token_signing_alg_values_supported"`
761785
// This is custom
@@ -1146,6 +1170,29 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler {
11461170
_=json.NewEncoder(rw).Encode(claims)
11471171
}))
11481172

1173+
mux.Handle(revokeTokenPath,http.HandlerFunc(func(rw http.ResponseWriter,r*http.Request) {
1174+
iff.revokeTokenGitHubFormat {
1175+
u,p,ok:=r.BasicAuth()
1176+
if!ok||!(u==f.clientID&&p==f.clientSecret) {
1177+
httpError(rw,http.StatusForbidden,xerrors.Errorf("basic auth failed"))
1178+
return
1179+
}
1180+
}else {
1181+
_,ok:=validateMW(rw,r)
1182+
if!ok {
1183+
httpError(rw,http.StatusForbidden,xerrors.Errorf("validation failed"))
1184+
return
1185+
}
1186+
}
1187+
1188+
code,err:=f.hookRevokeToken()
1189+
iferr!=nil {
1190+
httpError(rw,code,xerrors.Errorf("hook err: %w",err))
1191+
return
1192+
}
1193+
httpapi.Write(r.Context(),rw,code,"")
1194+
}))
1195+
11491196
// There is almost no difference between this and /userinfo.
11501197
// The main tweak is that this route is "mounted" vs "handle" because "/userinfo"
11511198
// should be strict, and this one needs to handle sub routes.
@@ -1474,12 +1521,16 @@ func (f *FakeIDP) ExternalAuthConfig(t testing.TB, id string, custom *ExternalAu
14741521
DisplayName:id,
14751522
InstrumentedOAuth2Config:oauthCfg,
14761523
ID:id,
1524+
ClientID:f.clientID,
1525+
ClientSecret:f.clientSecret,
14771526
// No defaults for these fields by omitting the type
14781527
Type:"",
14791528
DisplayIcon:f.WellknownConfig().UserInfoURL,
14801529
// Omit the /user for the validate so we can easily append to it when modifying
14811530
// the cfg for advanced tests.
1482-
ValidateURL:f.locked.IssuerURL().ResolveReference(&url.URL{Path:"/external-auth-validate/"}).String(),
1531+
ValidateURL:f.locked.IssuerURL().ResolveReference(&url.URL{Path:"/external-auth-validate/"}).String(),
1532+
RevokeURL:f.locked.IssuerURL().ResolveReference(&url.URL{Path:revokeTokenPath}).String(),
1533+
RevokeTimeout:1*time.Second,
14831534
DeviceAuth:&externalauth.DeviceAuth{
14841535
Config:oauthCfg,
14851536
ClientID:f.clientID,

‎coderd/externalauth.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func (api *API) externalAuthByID(w http.ResponseWriter, r *http.Request) {
8787
// @Tags Git
8888
// @Produce json
8989
// @Param externalauth path string true "Git Provider ID" format(string)
90-
// @Success 200
90+
// @Success 200 {object} codersdk.DeleteExternalAuthByIDResponse
9191
// @Router /external-auth/{externalauth} [delete]
9292
func (api*API)deleteExternalAuthByID(w http.ResponseWriter,r*http.Request) {
9393
config:=httpmw.ExternalAuthParam(r)
@@ -98,7 +98,6 @@ func (api *API) deleteExternalAuthByID(w http.ResponseWriter, r *http.Request) {
9898
ProviderID:config.ID,
9999
UserID:apiKey.UserID,
100100
})
101-
102101
iferr!=nil {
103102
iferrors.Is(err,sql.ErrNoRows) {
104103
httpapi.ResourceNotFound(w)

‎coderd/externalauth/externalauth.go‎

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ const (
3434
// database for a failed refresh token. In rare cases, the error could be a large
3535
// HTML payload.
3636
failureReasonLimit=400
37+
38+
// tokenRevocationTimeout timeout for requests to external oauth provider.
39+
tokenRevocationTimeout=10*time.Second
3740
)
3841

3942
// Config is used for authentication for Git operations.
@@ -44,7 +47,7 @@ type Config struct {
4447
// Type is the type of provider.
4548
Typestring
4649

47-
ClientIdstring
50+
ClientIDstring
4851
ClientSecretstring
4952
// DeviceAuth is set if the provider uses the device flow.
5053
DeviceAuth*DeviceAuth
@@ -72,7 +75,8 @@ type Config struct {
7275
// not be validated before being returned.
7376
ValidateURLstring
7477

75-
RevokeURLstring
78+
RevokeURLstring
79+
RevokeTimeout time.Duration
7680

7781
// Regex is a Regexp matched against URLs for
7882
// a Git clone. e.g. "Username for 'https://github.com':"
@@ -395,13 +399,9 @@ func (c *Config) RevokeToken(ctx context.Context, link database.ExternalAuthLink
395399
returnfalse,nil
396400
}
397401

398-
varerrerror
399-
varreq*http.Request
400-
ifc.Type==codersdk.EnhancedExternalAuthProviderGitHub.String() {
401-
req,err=c.tokenRevocationRequestGitHub(ctx,link)
402-
}else {
403-
req,err=c.tokenRevocationRequestRFC7009(ctx,link)
404-
}
402+
reqCtx,cancel:=context.WithTimeout(ctx,c.RevokeTimeout)
403+
defercancel()
404+
req,err:=c.tokenRevocationRequest(reqCtx,link)
405405
iferr!=nil {
406406
returnfalse,err
407407
}
@@ -411,31 +411,42 @@ func (c *Config) RevokeToken(ctx context.Context, link database.ExternalAuthLink
411411
returnfalse,err
412412
}
413413
deferres.Body.Close()
414+
// RFC spec returns 200, GitHub 204
415+
returnres.StatusCode==http.StatusOK||res.StatusCode==http.StatusNoContent,nil
416+
}
414417

415-
returnres.StatusCode==http.StatusOK,nil
418+
func (c*Config)tokenRevocationRequest(ctx context.Context,link database.ExternalAuthLink) (*http.Request,error) {
419+
ifc.Type==codersdk.EnhancedExternalAuthProviderGitHub.String() {
420+
returnc.tokenRevocationRequestGitHub(ctx,link)
421+
}
422+
returnc.tokenRevocationRequestRFC7009(ctx,link)
416423
}
417424

418425
func (c*Config)tokenRevocationRequestRFC7009(ctx context.Context,link database.ExternalAuthLink) (*http.Request,error) {
419426
p:= url.Values{}
420-
p.Add("client_id",c.ClientId)
427+
p.Add("client_id",c.ClientID)
421428
p.Add("client_secret",c.ClientSecret)
422429
p.Add("token_type_hint","refresh_token")
423430
p.Add("token",link.OAuthRefreshToken)
424-
body:=p.Encode()
425-
returnhttp.NewRequestWithContext(ctx,http.MethodPost,c.RevokeURL,strings.NewReader(body))
431+
req,err:=http.NewRequestWithContext(ctx,http.MethodPost,c.RevokeURL,strings.NewReader(p.Encode()))
432+
iferr!=nil {
433+
returnnil,err
434+
}
435+
req.Header.Set("Authorization",fmt.Sprintf("Bearer %s",link.OAuthAccessToken))
436+
returnreq,nil
426437
}
427438

428439
func (c*Config)tokenRevocationRequestGitHub(ctx context.Context,link database.ExternalAuthLink) (*http.Request,error) {
429-
// GitHub doesn't follow RFC spec, GitHub specific request is needed
440+
// GitHub doesn't follow RFC spec
430441
// https://docs.github.com/en/rest/apps/oauth-applications?apiVersion=2022-11-28#delete-an-app-authorization
431-
body:=fmt.Sprintf("{\"access_token\":\"%s\"}",link.OAuthAccessToken)
442+
body:=fmt.Sprintf("{\"access_token\":%q}",link.OAuthAccessToken)
432443
req,err:=http.NewRequestWithContext(ctx,http.MethodDelete,c.RevokeURL,strings.NewReader(body))
433444
iferr!=nil {
434445
returnnil,err
435446
}
436447
req.Header.Add("Accept","application/vnd.github+json")
437448
req.Header.Add("X-GitHub-Api-Version","2022-11-28")
438-
req.SetBasicAuth(c.ClientId,c.ClientSecret)
449+
req.SetBasicAuth(c.ClientID,c.ClientSecret)
439450
returnreq,nil
440451
}
441452

@@ -665,13 +676,14 @@ func ConvertConfig(instrument *promoauth.Factory, entries []codersdk.ExternalAut
665676
cfg:=&Config{
666677
InstrumentedOAuth2Config:instrumented,
667678
ID:entry.ID,
668-
ClientId:entry.ClientID,
679+
ClientID:entry.ClientID,
669680
ClientSecret:entry.ClientSecret,
670681
Regex:regex,
671682
Type:entry.Type,
672683
NoRefresh:entry.NoRefresh,
673684
ValidateURL:entry.ValidateURL,
674685
RevokeURL:entry.RevokeURL,
686+
RevokeTimeout:tokenRevocationTimeout,
675687
AppInstallationsURL:entry.AppInstallationsURL,
676688
AppInstallURL:entry.AppInstallURL,
677689
DisplayName:entry.DisplayName,

0 commit comments

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