Skip to content

Commit

Permalink
Fix console memory leak
Browse files Browse the repository at this point in the history
* Don't create a new HTTP client for every request to the API
* Only create the OIDC loginMethod once since it is stateful
  • Loading branch information
spadgett committed Mar 20, 2019
1 parent 4fbc382 commit 7a3d2a0
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 13 deletions.
34 changes: 22 additions & 12 deletions auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@ const (
var log = capnslog.NewPackageLogger("github.com/openshift/console", "auth")

type Authenticator struct {
tokenVerifier func(string) (*loginState, error)

authFunc func() (*oauth2.Config, loginMethod)

clientFunc func() *http.Client

// userFunc returns the User associated with the cookie of a request.
// This is not part of loginMethod to avoid creating an unnecessary
// HTTP client for every call.
userFunc func(*http.Request) (*User, error)

errorURL string
successURL string
cookiePath string
Expand All @@ -66,8 +69,6 @@ type loginMethod interface {
login(http.ResponseWriter, *oauth2.Token) (*loginState, error)
// logout deletes any cookies associated with the user.
logout(http.ResponseWriter, *http.Request)
// authenticate fetches the bearer token from the cookie of a request.
authenticate(*http.Request) (*User, error)
// getKubeAdminLogoutURL returns the logout URL for the special
// kube:admin user in OpenShift
getKubeAdminLogoutURL() string
Expand Down Expand Up @@ -154,6 +155,7 @@ func NewAuthenticator(ctx context.Context, c *Config) (*Authenticator, error) {
var authSourceFunc func() (oauth2.Endpoint, loginMethod, error)
switch c.AuthSource {
case AuthSourceOpenShift:
a.userFunc = getOpenShiftUser
authSourceFunc = func() (oauth2.Endpoint, loginMethod, error) {
// Use the k8s CA for OAuth metadata discovery.
// Don't include system roots when talking to the API server.
Expand All @@ -171,14 +173,22 @@ func NewAuthenticator(ctx context.Context, c *Config) (*Authenticator, error) {
})
}
default:
// OIDC auth source is stateful, so only create it once.
endpoint, oidcAuthSource, err := newOIDCAuth(ctx, &oidcConfig{
client: a.clientFunc(),
issuerURL: c.IssuerURL,
clientID: c.ClientID,
cookiePath: c.CookiePath,
secureCookies: c.SecureCookies,
})
a.userFunc = func(r *http.Request) (*User, error) {
if oidcAuthSource == nil {
return nil, fmt.Errorf("OIDC auth source is not intialized")
}
return oidcAuthSource.authenticate(r)
}
authSourceFunc = func() (oauth2.Endpoint, loginMethod, error) {
return newOIDCAuth(ctx, &oidcConfig{
client: a.clientFunc(),
issuerURL: c.IssuerURL,
clientID: c.ClientID,
cookiePath: c.CookiePath,
secureCookies: c.SecureCookies,
})
return endpoint, oidcAuthSource, err
}
}

Expand Down Expand Up @@ -273,7 +283,7 @@ type User struct {
}

func (a *Authenticator) Authenticate(r *http.Request) (*User, error) {
return a.getLoginMethod().authenticate(r)
return a.userFunc(r)
}

// LoginFunc redirects to the OIDC provider for user login.
Expand Down
2 changes: 1 addition & 1 deletion auth/auth_openshift.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func (o *openShiftAuth) logout(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNoContent)
}

func (o *openShiftAuth) authenticate(r *http.Request) (*User, error) {
func getOpenShiftUser(r *http.Request) (*User, error) {
// TODO: This doesn't do any validation of the cookie with the assumption that the
// API server will reject tokens it doesn't recognize. If we want to keep some backend
// state we should sign this cookie. If not there's not much we can do.
Expand Down

0 comments on commit 7a3d2a0

Please sign in to comment.