Skip to content

chore: instrument external oauth2 requests #11519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Jan 10, 2024
Merged

chore: instrument external oauth2 requests #11519

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
b7f13fa
chore: instrument external oauth2 requests
Emyrk Jan 9, 2024
fd1e012
Add a unit test
Emyrk Jan 9, 2024
a6de1e3
remove internet based request
Emyrk Jan 9, 2024
005883a
use ctx
Emyrk Jan 9, 2024
2f77f99
typo
Emyrk Jan 9, 2024
3377a9b
Work on instrumenting related oauth2 calls
Emyrk Jan 9, 2024
07fd10d
remove print
Emyrk Jan 9, 2024
117a405
typos
Emyrk Jan 9, 2024
73abae6
Fixup some unit tests
Emyrk Jan 9, 2024
e5e190d
fixup test cases
Emyrk Jan 9, 2024
d4b36d3
remove oidc instrument
Emyrk Jan 9, 2024
8964297
typos
Emyrk Jan 9, 2024
2ba7a5c
more test coverage
Emyrk Jan 9, 2024
8963aaa
left debug
Emyrk Jan 9, 2024
bfa427f
Add comments
Emyrk Jan 9, 2024
9d1c76c
use consts for source labels
Emyrk Jan 10, 2024
c149f8f
Spell out config
Emyrk Jan 10, 2024
30c459f
fix compile
Emyrk Jan 10, 2024
cd98806
Use req with context
Emyrk Jan 10, 2024
85e2d91
no panic
Emyrk Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
use consts for source labels
  • Loading branch information
@Emyrk
Emyrk committed Jan 10, 2024
commit 9d1c76c96c6ff19905efb9b69366c29048dff944
6 changes: 3 additions & 3 deletions coderd/externalauth/externalauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func (c *Config) ValidateToken(ctx context.Context, token string) (bool, *coders
}

req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
res, err := c.InstrumentedOAuth2Config.Do(ctx, "ValidateToken", req)
res, err := c.InstrumentedOAuth2Config.Do(ctx, promoauth.SourceValidateToken, req)
if err != nil {
return false, nil, err
}
Expand Down Expand Up @@ -238,7 +238,7 @@ func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk
return nil, false, err
}
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
res, err := c.InstrumentedOAuth2Config.Do(ctx, "AppInstallations", req)
res, err := c.InstrumentedOAuth2Config.Do(ctx, promoauth.SourceAppInstallations, req)
if err != nil {
return nil, false, err
}
Expand Down Expand Up @@ -305,7 +305,7 @@ func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*codersdk.ExternalAut
if c.Cfg != nil {
// The cfg can be nil in unit tests.
do = func(req *http.Request) (*http.Response, error) {
return c.Cfg.Do(ctx, "AuthorizeDevice", req)
return c.Cfg.Do(ctx, promoauth.SourceAuthorizeDevice, req)
}
}

Expand Down
28 changes: 19 additions & 9 deletions coderd/promoauth/oauth2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ import (
"golang.org/x/oauth2"
)

type Oauth2Source string

const (
SourceValidateToken Oauth2Source = "ValidateToken"
SourceExchange Oauth2Source = "Exchange"
SourceTokenSource Oauth2Source = "TokenSource"
SourceAppInstallations Oauth2Source = "AppInstallations"
SourceAuthorizeDevice Oauth2Source = "AuthorizeDevice"
)

// OAuth2Config exposes a subset of *oauth2.Config functions for easier testing.
// *oauth2.Config should be used instead of implementing this in production.
type OAuth2Config interface {
Expand All @@ -27,7 +37,7 @@ type InstrumentedOAuth2Config interface {

// Do is provided as a convenience method to make a request with the oauth2 client.
// It mirrors `http.Client.Do`.
Do(ctx context.Context, source string, req *http.Request) (*http.Response, error)
Do(ctx context.Context, source Oauth2Source, req *http.Request) (*http.Response, error)
}

var _ OAuth2Config = (*Config)(nil)
Expand Down Expand Up @@ -79,7 +89,7 @@ type Config struct {
metrics *metrics
}

func (c *Config) Do(ctx context.Context, source string, req *http.Request) (*http.Response, error) {
func (c *Config) Do(ctx context.Context, source Oauth2Source, req *http.Request) (*http.Response, error) {
cli := c.oauthHTTPClient(ctx, source)
return cli.Do(req)
}
Expand All @@ -90,11 +100,11 @@ func (c *Config) AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string
}

func (c *Config) Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) {
return c.underlying.Exchange(c.wrapClient(ctx, "Exchange"), code, opts...)
return c.underlying.Exchange(c.wrapClient(ctx, SourceExchange), code, opts...)
}

func (c *Config) TokenSource(ctx context.Context, token *oauth2.Token) oauth2.TokenSource {
return c.underlying.TokenSource(c.wrapClient(ctx, "TokenSource"), token)
return c.underlying.TokenSource(c.wrapClient(ctx, SourceTokenSource), token)
}

// wrapClient is the only way we can accurately instrument the oauth2 client.
Expand All @@ -104,12 +114,12 @@ func (c *Config) TokenSource(ctx context.Context, token *oauth2.Token) oauth2.To
// For example, the 'TokenSource' method will return a token
// source that will make a network request when the 'Token' method is called on
// it if the token is expired.
func (c *Config) wrapClient(ctx context.Context, source string) context.Context {
func (c *Config) wrapClient(ctx context.Context, source Oauth2Source) context.Context {
return context.WithValue(ctx, oauth2.HTTPClient, c.oauthHTTPClient(ctx, source))
}

// oauthHTTPClient returns an http client that will instrument every request made.
func (c *Config) oauthHTTPClient(ctx context.Context, source string) *http.Client {
func (c *Config) oauthHTTPClient(ctx context.Context, source Oauth2Source) *http.Client {
cli := &http.Client{}

// Check if the context has a http client already.
Expand All @@ -124,13 +134,13 @@ func (c *Config) oauthHTTPClient(ctx context.Context, source string) *http.Clien

type instrumentedTripper struct {
c *Config
source string
source Oauth2Source
underlying http.RoundTripper
}

// newInstrumentedTripper intercepts a http request, and increments the
// externalRequestCount metric.
func newInstrumentedTripper(c *Config, source string, under http.RoundTripper) *instrumentedTripper {
func newInstrumentedTripper(c *Config, source Oauth2Source, under http.RoundTripper) *instrumentedTripper {
if under == nil {
under = http.DefaultTransport
}
Expand All @@ -156,7 +166,7 @@ func (i *instrumentedTripper) RoundTrip(r *http.Request) (*http.Response, error)
}
i.c.metrics.externalRequestCount.With(prometheus.Labels{
"name": i.c.name,
"source": i.source,
"source": string(i.source),
"status_code": fmt.Sprintf("%d", statusCode),
}).Inc()
return resp, err
Expand Down