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

Commit439b041

Browse files
authored
feat: add best effort attempt to revoke oauth access token in external auth provider (#19775)
Solves#15575Adds OAuth access token revocation when unlinking external authprovider. Due to revocation not being consistently implemented byproviders this is only best effort attempt. Unsuccessful revocationwon't influence link removal.
1 parent0601cc8 commit439b041

File tree

24 files changed

+629
-69
lines changed

24 files changed

+629
-69
lines changed

‎cli/server.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2692,6 +2692,8 @@ func parseExternalAuthProvidersFromEnv(prefix string, environ []string) ([]coder
26922692
provider.AuthURL=v.Value
26932693
case"TOKEN_URL":
26942694
provider.TokenURL=v.Value
2695+
case"REVOKE_URL":
2696+
provider.RevokeURL=v.Value
26952697
case"VALIDATE_URL":
26962698
provider.ValidateURL=v.Value
26972699
case"REGEX":

‎cli/server_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ func TestReadExternalAuthProvidersFromEnv(t *testing.T) {
7777
"CODER_EXTERNAL_AUTH_1_CLIENT_SECRET=hunter12",
7878
"CODER_EXTERNAL_AUTH_1_TOKEN_URL=google.com",
7979
"CODER_EXTERNAL_AUTH_1_VALIDATE_URL=bing.com",
80+
"CODER_EXTERNAL_AUTH_1_REVOKE_URL=revoke.url",
8081
"CODER_EXTERNAL_AUTH_1_SCOPES=repo:read repo:write",
8182
"CODER_EXTERNAL_AUTH_1_NO_REFRESH=true",
8283
"CODER_EXTERNAL_AUTH_1_DISPLAY_NAME=Google",
@@ -88,13 +89,15 @@ func TestReadExternalAuthProvidersFromEnv(t *testing.T) {
8889
// Validate the first provider.
8990
assert.Equal(t,"1",providers[0].ID)
9091
assert.Equal(t,"gitlab",providers[0].Type)
92+
assert.Equal(t,"",providers[0].RevokeURL)
9193

9294
// Validate the second provider.
9395
assert.Equal(t,"2",providers[1].ID)
9496
assert.Equal(t,"sid",providers[1].ClientID)
9597
assert.Equal(t,"hunter12",providers[1].ClientSecret)
9698
assert.Equal(t,"google.com",providers[1].TokenURL)
9799
assert.Equal(t,"bing.com",providers[1].ValidateURL)
100+
assert.Equal(t,"revoke.url",providers[1].RevokeURL)
98101
assert.Equal(t, []string{"repo:read","repo:write"},providers[1].Scopes)
99102
assert.Equal(t,true,providers[1].NoRefresh)
100103
assert.Equal(t,"Google",providers[1].DisplayName)

‎coderd/apidoc/docs.go‎

Lines changed: 25 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: 23 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: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ import (
4646
"github.com/coder/coder/v2/testutil"
4747
)
4848

49+
typeHookRevokeTokenFnfunc() (httpStatusint,errerror)
50+
4951
typetokenstruct {
5052
issued time.Time
5153
emailstring
@@ -196,9 +198,11 @@ type FakeIDP struct {
196198
// hookValidRedirectURL can be used to reject a redirect url from the
197199
// IDP -> Application. Almost all IDPs have the concept of
198200
// "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
201+
hookValidRedirectURLfunc(redirectURLstring)error
202+
hookUserInfofunc(emailstring) (jwt.MapClaims,error)
203+
hookRevokeTokenHookRevokeTokenFn
204+
revokeTokenGitHubFormatbool// GitHub doesn't follow token revocation RFC spec
205+
hookAccessTokenJWTfunc(emailstring,exp time.Time) jwt.MapClaims
202206
// defaultIDClaims is if a new client connects and we didn't preset
203207
// some claims.
204208
defaultIDClaims jwt.MapClaims
@@ -327,6 +331,19 @@ func WithStaticUserInfo(info jwt.MapClaims) func(*FakeIDP) {
327331
}
328332
}
329333

334+
funcWithRevokeTokenRFC(revokeFuncHookRevokeTokenFn)func(*FakeIDP) {
335+
returnfunc(f*FakeIDP) {
336+
f.hookRevokeToken=revokeFunc
337+
}
338+
}
339+
340+
funcWithRevokeTokenGitHub(revokeFuncHookRevokeTokenFn)func(*FakeIDP) {
341+
returnfunc(f*FakeIDP) {
342+
f.hookRevokeToken=revokeFunc
343+
f.revokeTokenGitHubFormat=true
344+
}
345+
}
346+
330347
funcWithDefaultIDClaims(claims jwt.MapClaims)func(*FakeIDP) {
331348
returnfunc(f*FakeIDP) {
332349
f.defaultIDClaims=claims
@@ -358,6 +375,7 @@ type With429Arguments struct {
358375
AuthorizePathbool
359376
KeysPathbool
360377
UserInfoPathbool
378+
RevokePathbool
361379
DeviceAuthbool
362380
DeviceVerifybool
363381
}
@@ -387,6 +405,10 @@ func With429(params With429Arguments) func(*FakeIDP) {
387405
http.Error(rw,"429, being manually blocked (userinfo)",http.StatusTooManyRequests)
388406
return
389407
}
408+
ifparams.RevokePath&&strings.Contains(r.URL.Path,revokeTokenPath) {
409+
http.Error(rw,"429, being manually blocked (revoke)",http.StatusTooManyRequests)
410+
return
411+
}
390412
ifparams.DeviceAuth&&strings.Contains(r.URL.Path,deviceAuth) {
391413
http.Error(rw,"429, being manually blocked (device-auth)",http.StatusTooManyRequests)
392414
return
@@ -408,8 +430,10 @@ const (
408430
authorizePath="/oauth2/authorize"
409431
keysPath="/oauth2/keys"
410432
userInfoPath="/oauth2/userinfo"
411-
deviceAuth="/login/device/code"
412-
deviceVerify="/login/device"
433+
// nolint:gosec // It also thinks this is a secret lol
434+
revokeTokenPath="/oauth2/revoke"
435+
deviceAuth="/login/device/code"
436+
deviceVerify="/login/device"
413437
)
414438

415439
funcNewFakeIDP(t testing.TB,opts...FakeIDPOpt)*FakeIDP {
@@ -486,6 +510,7 @@ func (f *FakeIDP) updateIssuerURL(t testing.TB, issuer string) {
486510
TokenURL:u.ResolveReference(&url.URL{Path:tokenPath}).String(),
487511
JWKSURL:u.ResolveReference(&url.URL{Path:keysPath}).String(),
488512
UserInfoURL:u.ResolveReference(&url.URL{Path:userInfoPath}).String(),
513+
RevokeURL:u.ResolveReference(&url.URL{Path:revokeTokenPath}).String(),
489514
DeviceCodeURL:u.ResolveReference(&url.URL{Path:deviceAuth}).String(),
490515
Algorithms: []string{
491516
"RS256",
@@ -756,6 +781,7 @@ type ProviderJSON struct {
756781
TokenURLstring`json:"token_endpoint"`
757782
JWKSURLstring`json:"jwks_uri"`
758783
UserInfoURLstring`json:"userinfo_endpoint"`
784+
RevokeURLstring`json:"revocation_endpoint"`
759785
DeviceCodeURLstring`json:"device_authorization_endpoint"`
760786
Algorithms []string`json:"id_token_signing_alg_values_supported"`
761787
// This is custom
@@ -1146,6 +1172,29 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler {
11461172
_=json.NewEncoder(rw).Encode(claims)
11471173
}))
11481174

1175+
mux.Handle(revokeTokenPath,http.HandlerFunc(func(rw http.ResponseWriter,r*http.Request) {
1176+
iff.revokeTokenGitHubFormat {
1177+
u,p,ok:=r.BasicAuth()
1178+
if!ok||!(u==f.clientID&&p==f.clientSecret) {
1179+
httpError(rw,http.StatusForbidden,xerrors.Errorf("basic auth failed"))
1180+
return
1181+
}
1182+
}else {
1183+
_,ok:=validateMW(rw,r)
1184+
if!ok {
1185+
httpError(rw,http.StatusForbidden,xerrors.Errorf("token validation failed"))
1186+
return
1187+
}
1188+
}
1189+
1190+
code,err:=f.hookRevokeToken()
1191+
iferr!=nil {
1192+
httpError(rw,code,xerrors.Errorf("hook err: %w",err))
1193+
return
1194+
}
1195+
httpapi.Write(r.Context(),rw,code,"")
1196+
}))
1197+
11491198
// There is almost no difference between this and /userinfo.
11501199
// The main tweak is that this route is "mounted" vs "handle" because "/userinfo"
11511200
// should be strict, and this one needs to handle sub routes.
@@ -1474,12 +1523,16 @@ func (f *FakeIDP) ExternalAuthConfig(t testing.TB, id string, custom *ExternalAu
14741523
DisplayName:id,
14751524
InstrumentedOAuth2Config:oauthCfg,
14761525
ID:id,
1526+
ClientID:f.clientID,
1527+
ClientSecret:f.clientSecret,
14771528
// No defaults for these fields by omitting the type
14781529
Type:"",
14791530
DisplayIcon:f.WellknownConfig().UserInfoURL,
14801531
// Omit the /user for the validate so we can easily append to it when modifying
14811532
// the cfg for advanced tests.
1482-
ValidateURL:f.locked.IssuerURL().ResolveReference(&url.URL{Path:"/external-auth-validate/"}).String(),
1533+
ValidateURL:f.locked.IssuerURL().ResolveReference(&url.URL{Path:"/external-auth-validate/"}).String(),
1534+
RevokeURL:f.locked.IssuerURL().ResolveReference(&url.URL{Path:revokeTokenPath}).String(),
1535+
RevokeTimeout:1*time.Second,
14831536
DeviceAuth:&externalauth.DeviceAuth{
14841537
Config:oauthCfg,
14851538
ClientID:f.clientID,

‎coderd/externalauth.go‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,20 +85,37 @@ func (api *API) externalAuthByID(w http.ResponseWriter, r *http.Request) {
8585
// @ID delete-external-auth-user-link-by-id
8686
// @Security CoderSessionToken
8787
// @Tags Git
88-
// @Success 200
88+
// @Produce json
8989
// @Param externalauth path string true "Git Provider ID" format(string)
90+
// @Success 200 {object} codersdk.DeleteExternalAuthByIDResponse
9091
// @Router /external-auth/{externalauth} [delete]
9192
func (api*API)deleteExternalAuthByID(w http.ResponseWriter,r*http.Request) {
9293
config:=httpmw.ExternalAuthParam(r)
9394
apiKey:=httpmw.APIKey(r)
9495
ctx:=r.Context()
9596

96-
err:=api.Database.DeleteExternalAuthLink(ctx, database.DeleteExternalAuthLinkParams{
97+
link,err:=api.Database.GetExternalAuthLink(ctx, database.GetExternalAuthLinkParams{
9798
ProviderID:config.ID,
9899
UserID:apiKey.UserID,
99100
})
100101
iferr!=nil {
101-
if!errors.Is(err,sql.ErrNoRows) {
102+
iferrors.Is(err,sql.ErrNoRows) {
103+
httpapi.ResourceNotFound(w)
104+
return
105+
}
106+
httpapi.Write(ctx,w,http.StatusInternalServerError, codersdk.Response{
107+
Message:"Failed to get external auth link during deletion.",
108+
Detail:err.Error(),
109+
})
110+
return
111+
}
112+
113+
err=api.Database.DeleteExternalAuthLink(ctx, database.DeleteExternalAuthLinkParams{
114+
ProviderID:config.ID,
115+
UserID:apiKey.UserID,
116+
})
117+
iferr!=nil {
118+
iferrors.Is(err,sql.ErrNoRows) {
102119
httpapi.ResourceNotFound(w)
103120
return
104121
}
@@ -109,7 +126,13 @@ func (api *API) deleteExternalAuthByID(w http.ResponseWriter, r *http.Request) {
109126
return
110127
}
111128

112-
httpapi.Write(ctx,w,http.StatusOK,"OK")
129+
ok,err:=config.RevokeToken(ctx,link)
130+
resp:= codersdk.DeleteExternalAuthByIDResponse{TokenRevoked:ok}
131+
132+
iferr!=nil {
133+
resp.TokenRevocationError=err.Error()
134+
}
135+
httpapi.Write(ctx,w,http.StatusOK,resp)
113136
}
114137

115138
// @Summary Post external auth device by ID
@@ -394,13 +417,14 @@ func ExternalAuthConfigs(auths []*externalauth.Config) []codersdk.ExternalAuthLi
394417

395418
funcExternalAuthConfig(cfg*externalauth.Config) codersdk.ExternalAuthLinkProvider {
396419
return codersdk.ExternalAuthLinkProvider{
397-
ID:cfg.ID,
398-
Type:cfg.Type,
399-
Device:cfg.DeviceAuth!=nil,
400-
DisplayName:cfg.DisplayName,
401-
DisplayIcon:cfg.DisplayIcon,
402-
AllowRefresh:!cfg.NoRefresh,
403-
AllowValidate:cfg.ValidateURL!="",
420+
ID:cfg.ID,
421+
Type:cfg.Type,
422+
Device:cfg.DeviceAuth!=nil,
423+
DisplayName:cfg.DisplayName,
424+
DisplayIcon:cfg.DisplayIcon,
425+
AllowRefresh:!cfg.NoRefresh,
426+
AllowValidate:cfg.ValidateURL!="",
427+
SupportsRevocation:cfg.RevokeURL!="",
404428
}
405429
}
406430

0 commit comments

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