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

Commit50b78e3

Browse files
authored
chore: instrument external oauth2 requests (#11519)
* chore: instrument external oauth2 requestsExternal requests made by oauth2 configs are now instrumented into prometheus metrics.
1 parentaa7fe07 commit50b78e3

File tree

17 files changed

+425
-98
lines changed

17 files changed

+425
-98
lines changed

‎cli/create_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -767,11 +767,11 @@ func TestCreateWithGitAuth(t *testing.T) {
767767

768768
client:=coderdtest.New(t,&coderdtest.Options{
769769
ExternalAuthConfigs: []*externalauth.Config{{
770-
OAuth2Config:&testutil.OAuth2Config{},
771-
ID:"github",
772-
Regex:regexp.MustCompile(`github\.com`),
773-
Type:codersdk.EnhancedExternalAuthProviderGitHub.String(),
774-
DisplayName:"GitHub",
770+
InstrumentedOAuth2Config:&testutil.OAuth2Config{},
771+
ID:"github",
772+
Regex:regexp.MustCompile(`github\.com`),
773+
Type:codersdk.EnhancedExternalAuthProviderGitHub.String(),
774+
DisplayName:"GitHub",
775775
}},
776776
IncludeProvisionerDaemon:true,
777777
})

‎cli/server.go‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ import (
8080
"github.com/coder/coder/v2/coderd/oauthpki"
8181
"github.com/coder/coder/v2/coderd/prometheusmetrics"
8282
"github.com/coder/coder/v2/coderd/prometheusmetrics/insights"
83+
"github.com/coder/coder/v2/coderd/promoauth"
8384
"github.com/coder/coder/v2/coderd/schedule"
8485
"github.com/coder/coder/v2/coderd/telemetry"
8586
"github.com/coder/coder/v2/coderd/tracing"
@@ -133,7 +134,7 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co
133134
Scopes:vals.OIDC.Scopes,
134135
}
135136

136-
varuseCfghttpmw.OAuth2Config=oauthCfg
137+
varuseCfgpromoauth.OAuth2Config=oauthCfg
137138
ifvals.OIDC.ClientKeyFile!="" {
138139
// PKI authentication is done in the params. If a
139140
// counter example is found, we can add a config option to
@@ -523,8 +524,11 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
523524
returnxerrors.Errorf("read external auth providers from env: %w",err)
524525
}
525526

527+
promRegistry:=prometheus.NewRegistry()
528+
oauthInstrument:=promoauth.NewFactory(promRegistry)
526529
vals.ExternalAuthConfigs.Value=append(vals.ExternalAuthConfigs.Value,extAuthEnv...)
527530
externalAuthConfigs,err:=externalauth.ConvertConfig(
531+
oauthInstrument,
528532
vals.ExternalAuthConfigs.Value,
529533
vals.AccessURL.Value(),
530534
)
@@ -571,7 +575,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
571575
// the DeploymentValues instead, this just serves to indicate the source of each
572576
// option. This is just defensive to prevent accidentally leaking.
573577
DeploymentOptions:codersdk.DeploymentOptionsWithoutSecrets(opts),
574-
PrometheusRegistry:prometheus.NewRegistry(),
578+
PrometheusRegistry:promRegistry,
575579
APIRateLimit:int(vals.RateLimit.API.Value()),
576580
LoginRateLimit:loginRateLimit,
577581
FilesRateLimit:filesRateLimit,
@@ -617,7 +621,9 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
617621
}
618622

619623
ifvals.OAuth2.Github.ClientSecret!="" {
620-
options.GithubOAuth2Config,err=configureGithubOAuth2(vals.AccessURL.Value(),
624+
options.GithubOAuth2Config,err=configureGithubOAuth2(
625+
oauthInstrument,
626+
vals.AccessURL.Value(),
621627
vals.OAuth2.Github.ClientID.String(),
622628
vals.OAuth2.Github.ClientSecret.String(),
623629
vals.OAuth2.Github.AllowSignups.Value(),
@@ -636,6 +642,12 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
636642
logger.Warn(ctx,"coder will not check email_verified for OIDC logins")
637643
}
638644

645+
// This OIDC config is **not** being instrumented with the
646+
// oauth2 instrument wrapper. If we implement the missing
647+
// oidc methods, then we can instrument it.
648+
// Missing:
649+
//- Userinfo
650+
//- Verify
639651
oc,err:=createOIDCConfig(ctx,vals)
640652
iferr!=nil {
641653
returnxerrors.Errorf("create oidc config: %w",err)
@@ -1737,7 +1749,7 @@ func configureCAPool(tlsClientCAFile string, tlsConfig *tls.Config) error {
17371749
}
17381750

17391751
//nolint:revive // Ignore flag-parameter: parameter 'allowEveryone' seems to be a control flag, avoid control coupling (revive)
1740-
funcconfigureGithubOAuth2(accessURL*url.URL,clientID,clientSecretstring,allowSignups,allowEveryonebool,allowOrgs []string,rawTeams []string,enterpriseBaseURLstring) (*coderd.GithubOAuth2Config,error) {
1752+
funcconfigureGithubOAuth2(instrument*promoauth.Factory,accessURL*url.URL,clientID,clientSecretstring,allowSignups,allowEveryonebool,allowOrgs []string,rawTeams []string,enterpriseBaseURLstring) (*coderd.GithubOAuth2Config,error) {
17411753
redirectURL,err:=accessURL.Parse("/api/v2/users/oauth2/github/callback")
17421754
iferr!=nil {
17431755
returnnil,xerrors.Errorf("parse github oauth callback url: %w",err)
@@ -1790,7 +1802,7 @@ func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, al
17901802
}
17911803

17921804
return&coderd.GithubOAuth2Config{
1793-
OAuth2Config:&oauth2.Config{
1805+
OAuth2Config:instrument.New("github-login",&oauth2.Config{
17941806
ClientID:clientID,
17951807
ClientSecret:clientSecret,
17961808
Endpoint:endpoint,
@@ -1800,7 +1812,7 @@ func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, al
18001812
"read:org",
18011813
"user:email",
18021814
},
1803-
},
1815+
}),
18041816
AllowSignups:allowSignups,
18051817
AllowEveryone:allowEveryone,
18061818
AllowOrganizations:allowOrgs,

‎coderd/coderdtest/oidctest/idp.go‎

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/go-jose/go-jose/v3"
2525
"github.com/golang-jwt/jwt/v4"
2626
"github.com/google/uuid"
27+
"github.com/prometheus/client_golang/prometheus"
2728
"github.com/stretchr/testify/assert"
2829
"github.com/stretchr/testify/require"
2930
"golang.org/x/oauth2"
@@ -33,6 +34,7 @@ import (
3334
"cdr.dev/slog/sloggers/slogtest"
3435
"github.com/coder/coder/v2/coderd"
3536
"github.com/coder/coder/v2/coderd/externalauth"
37+
"github.com/coder/coder/v2/coderd/promoauth"
3638
"github.com/coder/coder/v2/coderd/util/syncmap"
3739
"github.com/coder/coder/v2/codersdk"
3840
)
@@ -223,6 +225,10 @@ func (f *FakeIDP) WellknownConfig() ProviderJSON {
223225
returnf.provider
224226
}
225227

228+
func (f*FakeIDP)IssuerURL()*url.URL {
229+
returnf.issuerURL
230+
}
231+
226232
func (f*FakeIDP)updateIssuerURL(t testing.TB,issuerstring) {
227233
t.Helper()
228234

@@ -397,6 +403,44 @@ func (f *FakeIDP) ExternalLogin(t testing.TB, client *codersdk.Client, opts ...f
397403
_=res.Body.Close()
398404
}
399405

406+
// CreateAuthCode emulates a user clicking "allow" on the IDP page. When doing
407+
// unit tests, it's easier to skip this step sometimes. It does make an actual
408+
// request to the IDP, so it should be equivalent to doing this "manually" with
409+
// actual requests.
410+
func (f*FakeIDP)CreateAuthCode(t testing.TB,statestring,opts...func(r*http.Request))string {
411+
// We need to store some claims, because this is also an OIDC provider, and
412+
// it expects some claims to be present.
413+
f.stateToIDTokenClaims.Store(state, jwt.MapClaims{})
414+
415+
u:=f.cfg.AuthCodeURL(state)
416+
r,err:=http.NewRequestWithContext(context.Background(),http.MethodPost,u,nil)
417+
require.NoError(t,err,"failed to create auth request")
418+
419+
for_,opt:=rangeopts {
420+
opt(r)
421+
}
422+
423+
rw:=httptest.NewRecorder()
424+
f.handler.ServeHTTP(rw,r)
425+
resp:=rw.Result()
426+
deferresp.Body.Close()
427+
428+
require.Equal(t,http.StatusTemporaryRedirect,resp.StatusCode,"expected redirect")
429+
to:=resp.Header.Get("Location")
430+
require.NotEmpty(t,to,"expected redirect location")
431+
432+
toURL,err:=url.Parse(to)
433+
require.NoError(t,err,"failed to parse redirect location")
434+
435+
code:=toURL.Query().Get("code")
436+
require.NotEmpty(t,code,"expected code in redirect location")
437+
438+
newState:=toURL.Query().Get("state")
439+
require.Equal(t,state,newState,"expected state to match")
440+
441+
returncode
442+
}
443+
400444
// OIDCCallback will emulate the IDP redirecting back to the Coder callback.
401445
// This is helpful if no Coderd exists because the IDP needs to redirect to
402446
// something.
@@ -901,9 +945,10 @@ func (f *FakeIDP) ExternalAuthConfig(t testing.TB, id string, custom *ExternalAu
901945
handle(email,rw,r)
902946
}
903947
}
948+
instrumentF:=promoauth.NewFactory(prometheus.NewRegistry())
904949
cfg:=&externalauth.Config{
905-
OAuth2Config:f.OIDCConfig(t,nil),
906-
ID:id,
950+
InstrumentedOAuth2Config:instrumentF.New(f.clientID,f.OIDCConfig(t,nil)),
951+
ID:id,
907952
// No defaults for these fields by omitting the type
908953
Type:"",
909954
DisplayIcon:f.WellknownConfig().UserInfoURL,
@@ -920,10 +965,10 @@ func (f *FakeIDP) ExternalAuthConfig(t testing.TB, id string, custom *ExternalAu
920965
// OIDCConfig returns the OIDC config to use for Coderd.
921966
func (f*FakeIDP)OIDCConfig(t testing.TB,scopes []string,opts...func(cfg*coderd.OIDCConfig))*coderd.OIDCConfig {
922967
t.Helper()
968+
923969
iflen(scopes)==0 {
924970
scopes= []string{"openid","email","profile"}
925971
}
926-
927972
oauthCfg:=&oauth2.Config{
928973
ClientID:f.clientID,
929974
ClientSecret:f.clientSecret,
@@ -966,7 +1011,6 @@ func (f *FakeIDP) OIDCConfig(t testing.TB, scopes []string, opts ...func(cfg *co
9661011
}
9671012

9681013
f.cfg=oauthCfg
969-
9701014
returncfg
9711015
}
9721016

‎coderd/externalauth/externalauth.go‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,14 @@ import (
2222
"github.com/coder/coder/v2/coderd/database"
2323
"github.com/coder/coder/v2/coderd/database/dbtime"
2424
"github.com/coder/coder/v2/coderd/httpapi"
25+
"github.com/coder/coder/v2/coderd/promoauth"
2526
"github.com/coder/coder/v2/codersdk"
2627
"github.com/coder/retry"
2728
)
2829

29-
typeOAuth2Configinterface {
30-
AuthCodeURL(statestring,opts...oauth2.AuthCodeOption)string
31-
Exchange(ctx context.Context,codestring,opts...oauth2.AuthCodeOption) (*oauth2.Token,error)
32-
TokenSource(context.Context,*oauth2.Token) oauth2.TokenSource
33-
}
34-
3530
// Config is used for authentication for Git operations.
3631
typeConfigstruct {
37-
OAuth2Config
32+
promoauth.InstrumentedOAuth2Config
3833
// ID is a unique identifier for the authenticator.
3934
IDstring
4035
// Type is the type of provider.
@@ -192,12 +187,8 @@ func (c *Config) ValidateToken(ctx context.Context, token string) (bool, *coders
192187
returnfalse,nil,err
193188
}
194189

195-
cli:=http.DefaultClient
196-
ifv,ok:=ctx.Value(oauth2.HTTPClient).(*http.Client);ok {
197-
cli=v
198-
}
199190
req.Header.Set("Authorization",fmt.Sprintf("Bearer %s",token))
200-
res,err:=cli.Do(req)
191+
res,err:=c.InstrumentedOAuth2Config.Do(ctx,promoauth.SourceValidateToken,req)
201192
iferr!=nil {
202193
returnfalse,nil,err
203194
}
@@ -247,7 +238,7 @@ func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk
247238
returnnil,false,err
248239
}
249240
req.Header.Set("Authorization",fmt.Sprintf("Bearer %s",token))
250-
res,err:=http.DefaultClient.Do(req)
241+
res,err:=c.InstrumentedOAuth2Config.Do(ctx,promoauth.SourceAppInstallations,req)
251242
iferr!=nil {
252243
returnnil,false,err
253244
}
@@ -287,6 +278,8 @@ func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk
287278
}
288279

289280
typeDeviceAuthstruct {
281+
// Config is provided for the http client method.
282+
Config promoauth.InstrumentedOAuth2Config
290283
ClientIDstring
291284
TokenURLstring
292285
Scopes []string
@@ -307,8 +300,17 @@ func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*codersdk.ExternalAut
307300
iferr!=nil {
308301
returnnil,err
309302
}
303+
304+
do:=http.DefaultClient.Do
305+
ifc.Config!=nil {
306+
// The cfg can be nil in unit tests.
307+
do=func(req*http.Request) (*http.Response,error) {
308+
returnc.Config.Do(ctx,promoauth.SourceAuthorizeDevice,req)
309+
}
310+
}
311+
312+
resp,err:=do(req)
310313
req.Header.Set("Accept","application/json")
311-
resp,err:=http.DefaultClient.Do(req)
312314
iferr!=nil {
313315
returnnil,err
314316
}
@@ -401,7 +403,7 @@ func (c *DeviceAuth) formatDeviceCodeURL() (string, error) {
401403

402404
// ConvertConfig converts the SDK configuration entry format
403405
// to the parsed and ready-to-consume in coderd provider type.
404-
funcConvertConfig(entries []codersdk.ExternalAuthConfig,accessURL*url.URL) ([]*Config,error) {
406+
funcConvertConfig(instrument*promoauth.Factory,entries []codersdk.ExternalAuthConfig,accessURL*url.URL) ([]*Config,error) {
405407
ids:=map[string]struct{}{}
406408
configs:= []*Config{}
407409
for_,entry:=rangeentries {
@@ -453,7 +455,7 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([
453455
Scopes:entry.Scopes,
454456
}
455457

456-
varoauthConfigOAuth2Config=oc
458+
varoauthConfigpromoauth.OAuth2Config=oc
457459
// Azure DevOps uses JWT token authentication!
458460
ifentry.Type==string(codersdk.EnhancedExternalAuthProviderAzureDevops) {
459461
oauthConfig=&jwtConfig{oc}
@@ -463,24 +465,25 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([
463465
}
464466

465467
cfg:=&Config{
466-
OAuth2Config:oauthConfig,
467-
ID:entry.ID,
468-
Regex:regex,
469-
Type:entry.Type,
470-
NoRefresh:entry.NoRefresh,
471-
ValidateURL:entry.ValidateURL,
472-
AppInstallationsURL:entry.AppInstallationsURL,
473-
AppInstallURL:entry.AppInstallURL,
474-
DisplayName:entry.DisplayName,
475-
DisplayIcon:entry.DisplayIcon,
476-
ExtraTokenKeys:entry.ExtraTokenKeys,
468+
InstrumentedOAuth2Config:instrument.New(entry.ID,oauthConfig),
469+
ID:entry.ID,
470+
Regex:regex,
471+
Type:entry.Type,
472+
NoRefresh:entry.NoRefresh,
473+
ValidateURL:entry.ValidateURL,
474+
AppInstallationsURL:entry.AppInstallationsURL,
475+
AppInstallURL:entry.AppInstallURL,
476+
DisplayName:entry.DisplayName,
477+
DisplayIcon:entry.DisplayIcon,
478+
ExtraTokenKeys:entry.ExtraTokenKeys,
477479
}
478480

479481
ifentry.DeviceFlow {
480482
ifentry.DeviceCodeURL=="" {
481483
returnnil,xerrors.Errorf("external auth provider %q: device auth url must be provided",entry.ID)
482484
}
483485
cfg.DeviceAuth=&DeviceAuth{
486+
Config:cfg,
484487
ClientID:entry.ClientID,
485488
TokenURL:oc.Endpoint.TokenURL,
486489
Scopes:entry.Scopes,

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp