Skip to content

Commit b5ff29a

Browse files
EmyrkEmyrk
committed
chore: instrument external oauth2 requests
1 parent 97bd74b commit b5ff29a

File tree

9 files changed

+168
-40
lines changed

9 files changed

+168
-40
lines changed

‎cli/server.go

Lines changed: 15 additions & 9 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"
@@ -102,7 +103,7 @@ import (
102103
"github.com/coder/wgtunnel/tunnelsdk"
103104
)
104105

105-
func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*coderd.OIDCConfig, error) {
106+
func createOIDCConfig(ctx context.Context, instrument *promoauth.Factory, vals *codersdk.DeploymentValues) (*coderd.OIDCConfig, error) {
106107
if vals.OIDC.ClientID == "" {
107108
return nil, xerrors.Errorf("OIDC client ID must be set!")
108109
}
@@ -133,7 +134,7 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co
133134
Scopes: vals.OIDC.Scopes,
134135
}
135136

136-
var useCfg httpmw.OAuth2Config = oauthCfg
137+
var useCfg promoauth.OAuth2Config = oauthCfg
137138
if vals.OIDC.ClientKeyFile != "" {
138139
// PKI authentication is done in the params. If a
139140
// counter example is found, we can add a config option to
@@ -159,7 +160,7 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co
159160
}
160161

161162
return &coderd.OIDCConfig{
162-
OAuth2Config: useCfg,
163+
OAuth2Config: instrument.New("oidc-login", useCfg),
163164
Provider: oidcProvider,
164165
Verifier: oidcProvider.Verifier(&oidc.Config{
165166
ClientID: vals.OIDC.ClientID.String(),
@@ -523,8 +524,11 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
523524
return xerrors.Errorf("read external auth providers from env: %w", err)
524525
}
525526

527+
promRegistry := prometheus.NewRegistry()
528+
oauthIntrument := promoauth.NewFactory(promRegistry)
526529
vals.ExternalAuthConfigs.Value = append(vals.ExternalAuthConfigs.Value, extAuthEnv...)
527530
externalAuthConfigs, err := externalauth.ConvertConfig(
531+
oauthIntrument,
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
if vals.OAuth2.Github.ClientSecret != "" {
620-
options.GithubOAuth2Config, err = configureGithubOAuth2(vals.AccessURL.Value(),
624+
options.GithubOAuth2Config, err = configureGithubOAuth2(
625+
oauthIntrument,
626+
vals.AccessURL.Value(),
621627
vals.OAuth2.Github.ClientID.String(),
622628
vals.OAuth2.Github.ClientSecret.String(),
623629
vals.OAuth2.Github.AllowSignups.Value(),
@@ -636,7 +642,7 @@ 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

639-
oc, err := createOIDCConfig(ctx, vals)
645+
oc, err := createOIDCConfig(ctx, oauthIntrument, vals)
640646
if err != nil {
641647
return xerrors.Errorf("create oidc config: %w", err)
642648
}
@@ -1737,7 +1743,7 @@ func configureCAPool(tlsClientCAFile string, tlsConfig *tls.Config) error {
17371743
}
17381744

17391745
//nolint:revive // Ignore flag-parameter: parameter 'allowEveryone' seems to be a control flag, avoid control coupling (revive)
1740-
func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, allowSignups, allowEveryone bool, allowOrgs []string, rawTeams []string, enterpriseBaseURL string) (*coderd.GithubOAuth2Config, error) {
1746+
func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, clientID, clientSecret string, allowSignups, allowEveryone bool, allowOrgs []string, rawTeams []string, enterpriseBaseURL string) (*coderd.GithubOAuth2Config, error) {
17411747
redirectURL, err := accessURL.Parse("/api/v2/users/oauth2/github/callback")
17421748
if err != nil {
17431749
return nil, xerrors.Errorf("parse github oauth callback url: %w", err)
@@ -1790,7 +1796,7 @@ func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, al
17901796
}
17911797

17921798
return &coderd.GithubOAuth2Config{
1793-
OAuth2Config: &oauth2.Config{
1799+
OAuth2Config: instrument.New("github-login", &oauth2.Config{
17941800
ClientID: clientID,
17951801
ClientSecret: clientSecret,
17961802
Endpoint: endpoint,
@@ -1800,7 +1806,7 @@ func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, al
18001806
"read:org",
18011807
"user:email",
18021808
},
1803-
},
1809+
}),
18041810
AllowSignups: allowSignups,
18051811
AllowEveryone: allowEveryone,
18061812
AllowOrganizations: allowOrgs,

‎coderd/externalauth/externalauth.go

Lines changed: 5 additions & 10 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-
type OAuth2Config interface {
30-
AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string
31-
Exchange(ctx context.Context, code string, 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
type Config struct {
37-
OAuth2Config
32+
promoauth.OAuth2Config
3833
// ID is a unique identifier for the authenticator.
3934
ID string
4035
// Type is the type of provider.
@@ -401,7 +396,7 @@ func (c *DeviceAuth) formatDeviceCodeURL() (string, error) {
401396

402397
// ConvertConfig converts the SDK configuration entry format
403398
// to the parsed and ready-to-consume in coderd provider type.
404-
func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([]*Config, error) {
399+
func ConvertConfig(instrument *promoauth.Factory, entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([]*Config, error) {
405400
ids := map[string]struct{}{}
406401
configs := []*Config{}
407402
for _, entry := range entries {
@@ -453,7 +448,7 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([
453448
Scopes: entry.Scopes,
454449
}
455450

456-
var oauthConfig OAuth2Config = oc
451+
var oauthConfig promoauth.OAuth2Config = oc
457452
// Azure DevOps uses JWT token authentication!
458453
if entry.Type == string(codersdk.EnhancedExternalAuthProviderAzureDevops) {
459454
oauthConfig = &jwtConfig{oc}
@@ -463,7 +458,7 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([
463458
}
464459

465460
cfg := &Config{
466-
OAuth2Config: oauthConfig,
461+
OAuth2Config: instrument.New(entry.ID, oauthConfig),
467462
ID: entry.ID,
468463
Regex: regex,
469464
Type: entry.Type,

‎coderd/httpmw/apikey.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/coder/coder/v2/coderd/database/dbauthz"
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/coderd/rbac"
2627
"github.com/coder/coder/v2/codersdk"
2728
)
@@ -74,8 +75,8 @@ func UserAuthorization(r *http.Request) Authorization {
7475
// OAuth2Configs is a collection of configurations for OAuth-based authentication.
7576
// This should be extended to support other authentication types in the future.
7677
type OAuth2Configs struct {
77-
Github OAuth2Config
78-
OIDC OAuth2Config
78+
Github promoauth.OAuth2Config
79+
OIDC promoauth.OAuth2Config
7980
}
8081

8182
func (c *OAuth2Configs) IsZero() bool {
@@ -270,7 +271,7 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
270271
})
271272
}
272273

273-
var oauthConfig OAuth2Config
274+
var oauthConfig promoauth.OAuth2Config
274275
switch key.LoginType {
275276
case database.LoginTypeGithub:
276277
oauthConfig = cfg.OAuth2Configs.Github

‎coderd/httpmw/oauth2.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/coder/coder/v2/coderd/database"
1212
"github.com/coder/coder/v2/coderd/httpapi"
13+
"github.com/coder/coder/v2/coderd/promoauth"
1314
"github.com/coder/coder/v2/codersdk"
1415
"github.com/coder/coder/v2/cryptorand"
1516
)
@@ -22,14 +23,6 @@ type OAuth2State struct {
2223
StateString string
2324
}
2425

25-
// OAuth2Config exposes a subset of *oauth2.Config functions for easier testing.
26-
// *oauth2.Config should be used instead of implementing this in production.
27-
type OAuth2Config interface {
28-
AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string
29-
Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error)
30-
TokenSource(context.Context, *oauth2.Token) oauth2.TokenSource
31-
}
32-
3326
// OAuth2 returns the state from an oauth request.
3427
func OAuth2(r *http.Request) OAuth2State {
3528
oauth, ok := r.Context().Value(oauth2StateKey{}).(OAuth2State)
@@ -44,7 +37,7 @@ func OAuth2(r *http.Request) OAuth2State {
4437
// a "code" URL parameter will be redirected.
4538
// AuthURLOpts are passed to the AuthCodeURL function. If this is nil,
4639
// the default option oauth2.AccessTypeOffline will be used.
47-
func ExtractOAuth2(config OAuth2Config, client *http.Client, authURLOpts map[string]string) func(http.Handler) http.Handler {
40+
func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, authURLOpts map[string]string) func(http.Handler) http.Handler {
4841
opts := make([]oauth2.AuthCodeOption, 0, len(authURLOpts)+1)
4942
opts = append(opts, oauth2.AccessTypeOffline)
5043
for k, v := range authURLOpts {

‎coderd/oauthpki/oidcpki.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"golang.org/x/oauth2/jws"
2121
"golang.org/x/xerrors"
2222

23-
"github.com/coder/coder/v2/coderd/httpmw"
23+
"github.com/coder/coder/v2/coderd/promoauth"
2424
)
2525

2626
// Config uses jwt assertions over client_secret for oauth2 authentication of
@@ -33,7 +33,7 @@ import (
3333
//
3434
// https://datatracker.ietf.org/doc/html/rfc7523
3535
type Config struct {
36-
cfg httpmw.OAuth2Config
36+
cfg promoauth.OAuth2Config
3737

3838
// These values should match those provided in the oauth2.Config.
3939
// Because the inner config is an interface, we need to duplicate these
@@ -57,7 +57,7 @@ type ConfigParams struct {
5757
PemEncodedKey []byte
5858
PemEncodedCert []byte
5959

60-
Config httpmw.OAuth2Config
60+
Config promoauth.OAuth2Config
6161
}
6262

6363
// NewOauth2PKIConfig creates the oauth2 config for PKI based auth. It requires the certificate and it's private key.

‎coderd/promoauth/doc.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// Package promoauth is for instrumenting oauth2 flows with prometheus metrics.
2+
// Specifically, it is intended to count the number of external requests made
3+
// by the underlying oauth2 exchanges.
4+
package promoauth

‎coderd/promoauth/oauth2.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package promoauth
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"net/http"
7+
8+
"github.com/prometheus/client_golang/prometheus"
9+
"github.com/prometheus/client_golang/prometheus/promauto"
10+
"golang.org/x/oauth2"
11+
)
12+
13+
// OAuth2Config exposes a subset of *oauth2.Config functions for easier testing.
14+
// *oauth2.Config should be used instead of implementing this in production.
15+
type OAuth2Config interface {
16+
AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string
17+
Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error)
18+
TokenSource(context.Context, *oauth2.Token) oauth2.TokenSource
19+
}
20+
21+
var _ OAuth2Config = (*Config)(nil)
22+
23+
type Factory struct {
24+
metrics *metrics
25+
}
26+
27+
// metrics is the reusable metrics for all oauth2 providers.
28+
type metrics struct {
29+
externalRequestCount *prometheus.CounterVec
30+
}
31+
32+
func NewFactory(registry prometheus.Registerer) *Factory {
33+
factory := promauto.With(registry)
34+
35+
return &Factory{
36+
metrics: &metrics{
37+
externalRequestCount: factory.NewCounterVec(prometheus.CounterOpts{
38+
Namespace: "coderd",
39+
Subsystem: "oauth2",
40+
Name: "external_requests_total",
41+
Help: "The total number of api calls made to external oauth2 providers. 'status_code' will be 0 if the request failed with no response.",
42+
}, []string{
43+
"name",
44+
"status_code",
45+
"domain",
46+
}),
47+
},
48+
}
49+
}
50+
51+
func (f *Factory) New(name string, under OAuth2Config) *Config {
52+
return &Config{
53+
name: name,
54+
underlying: under,
55+
metrics: f.metrics,
56+
}
57+
}
58+
59+
type Config struct {
60+
// Name is a human friendly name to identify the oauth2 provider. This should be
61+
// deterministic from restart to restart, as it is going to be used as a label in
62+
// prometheus metrics.
63+
name string
64+
underlying OAuth2Config
65+
metrics *metrics
66+
}
67+
68+
func (c *Config) AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string {
69+
// No external requests are made when constructing the auth code url.
70+
return c.underlying.AuthCodeURL(state, opts...)
71+
}
72+
73+
func (c *Config) Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) {
74+
return c.underlying.Exchange(c.wrapClient(ctx), code, opts...)
75+
}
76+
77+
func (c *Config) TokenSource(ctx context.Context, token *oauth2.Token) oauth2.TokenSource {
78+
return c.underlying.TokenSource(c.wrapClient(ctx), token)
79+
}
80+
81+
// wrapClient is the only way we can accurately instrument the oauth2 client.
82+
// This is because method calls to the 'OAuth2Config' interface are not 1:1 with
83+
// network requests.
84+
//
85+
// For example, the 'TokenSource' method will return a token
86+
// source that will make a network request when the 'Token' method is called on
87+
// it if the token is expired.
88+
func (c *Config) wrapClient(ctx context.Context) context.Context {
89+
cli := http.DefaultClient
90+
91+
// Check if the context has an http client already.
92+
if hc, ok := ctx.Value(oauth2.HTTPClient).(*http.Client); ok {
93+
cli = hc
94+
}
95+
96+
// The new tripper will instrument every request made by the oauth2 client.
97+
cli.Transport = newInstrumentedTripper(c, cli.Transport)
98+
return context.WithValue(ctx, oauth2.HTTPClient, cli)
99+
}
100+
101+
type instrumentedTripper struct {
102+
c *Config
103+
underlying http.RoundTripper
104+
}
105+
106+
func newInstrumentedTripper(c *Config, under http.RoundTripper) *instrumentedTripper {
107+
if under == nil {
108+
under = http.DefaultTransport
109+
}
110+
return &instrumentedTripper{
111+
c: c,
112+
underlying: under,
113+
}
114+
}
115+
116+
func (i *instrumentedTripper) RoundTrip(r *http.Request) (*http.Response, error) {
117+
resp, err := i.underlying.RoundTrip(r)
118+
var statusCode int
119+
if resp != nil {
120+
statusCode = resp.StatusCode
121+
}
122+
i.c.metrics.externalRequestCount.With(prometheus.Labels{
123+
"name": i.c.name,
124+
"status_code": fmt.Sprintf("%d", statusCode),
125+
"domain": r.URL.Host,
126+
}).Inc()
127+
return resp, err
128+
}

0 commit comments

Comments
 (0)