From 1983c2520702b5672de44551b1b9ae2c6554cf71 Mon Sep 17 00:00:00 2001 From: Daniel Haus Date: Fri, 26 Nov 2021 21:07:03 +0100 Subject: [PATCH 1/2] Introduce CheckEndpointConnector, implement for OpenShift connector. Signed-off-by: Daniel Haus --- connector/connector.go | 8 ++++++++ connector/openshift/openshift.go | 25 +++++++++++++++++++++++-- connector/openshift/openshift_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/connector/connector.go b/connector/connector.go index aab994b468..409262bec5 100644 --- a/connector/connector.go +++ b/connector/connector.go @@ -98,3 +98,11 @@ type RefreshConnector interface { // changes since the token was last refreshed. Refresh(ctx context.Context, s Scopes, identity Identity) (Identity, error) } + +// CheckEndpointConnector is a connector that can test its endpoints for availability. +type CheckEndpointConnector interface { + // CheckEndpoint is called when a client wants to check the availability of + // endpoints used by the connector. If no error is returned, the connector + // can be assumed to be working. + CheckEndpoint(ctx context.Context) error +} diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go index 81d2b35633..6be45119c8 100644 --- a/connector/openshift/openshift.go +++ b/connector/openshift/openshift.go @@ -38,8 +38,9 @@ type Config struct { } var ( - _ connector.CallbackConnector = (*openshiftConnector)(nil) - _ connector.RefreshConnector = (*openshiftConnector)(nil) + _ connector.CallbackConnector = (*openshiftConnector)(nil) + _ connector.RefreshConnector = (*openshiftConnector)(nil) + _ connector.CheckEndpointConnector = (*openshiftConnector)(nil) ) type openshiftConnector struct { @@ -126,6 +127,7 @@ func (c *Config) OpenWithHTTPClient(id string, logger log.Logger, Scopes: []string{"user:info"}, RedirectURL: c.RedirectURI, } + return &openshiftConnector, nil } @@ -257,6 +259,25 @@ func (c *openshiftConnector) user(ctx context.Context, client *http.Client) (u u return u, err } +func (c *openshiftConnector) CheckEndpoint(ctx context.Context) error { + return c.checkOAuth2EndpointsAvailability(ctx) +} + +func (c *openshiftConnector) checkOAuth2EndpointsAvailability(ctx context.Context) error { + for _, endpoint := range []string{c.oauth2Config.Endpoint.AuthURL, c.oauth2Config.Endpoint.TokenURL} { + req, err := http.NewRequest(http.MethodHead, endpoint, nil) + if err != nil { + return err + } + resp, err := c.httpClient.Do(req.WithContext(ctx)) + if err != nil { + return err + } + resp.Body.Close() + } + return nil +} + func validateAllowedGroups(userGroups, allowedGroups []string) bool { matchingGroups := groups.Filter(userGroups, allowedGroups) diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index 6280b831de..b39cfa26a2 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -257,6 +257,30 @@ func TestRefreshIdentityFailure(t *testing.T) { expectEquals(t, connector.Identity{}, identity) } +func TestCheckEndpoint(t *testing.T) { + s := newTestServer(map[string]interface{}{ + "/oauth/authorize": nil, + "/oauth/token": nil, + }) + defer s.Close() + + h, err := newHTTPClient(true, "") + expectNil(t, err) + + oc := openshiftConnector{apiURL: s.URL, httpClient: h, oauth2Config: &oauth2.Config{ + Endpoint: oauth2.Endpoint{ + AuthURL: s.URL + "/oauth/authorize", TokenURL: s.URL + "/oauth/token", + }, + }} + err = oc.CheckEndpoint(context.Background()) + expectNil(t, err) + + s.Close() + + err = oc.CheckEndpoint(context.Background()) + expectNotNil(t, err) +} + func newTestServer(responses map[string]interface{}) *httptest.Server { var s *httptest.Server s = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From eea20e11da13ce916c6f772b4a90acaa59fff0d9 Mon Sep 17 00:00:00 2001 From: Daniel Haus Date: Mon, 30 May 2022 03:27:34 +0200 Subject: [PATCH 2/2] Use Options to conditionally enable endpoints health check. Refactor HTTP client injection to use options as well. Signed-off-by: Daniel Haus --- connector/openshift/openshift.go | 84 ++++++++++++++++++--------- connector/openshift/openshift_test.go | 4 +- 2 files changed, 57 insertions(+), 31 deletions(-) diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go index 6be45119c8..0383d09fc2 100644 --- a/connector/openshift/openshift.go +++ b/connector/openshift/openshift.go @@ -26,7 +26,7 @@ const ( usersURLPath = "/apis/user.openshift.io/v1/users/~" ) -// Config holds configuration options for OpenShift login +// Config holds configuration options for OpenShift login. type Config struct { Issuer string `json:"issuer"` ClientID string `json:"clientID"` @@ -37,24 +37,27 @@ type Config struct { RootCA string `json:"rootCA"` } +// Option allows overriding default values of the openshift connector. +type Option func(*openshiftConnector) + var ( - _ connector.CallbackConnector = (*openshiftConnector)(nil) - _ connector.RefreshConnector = (*openshiftConnector)(nil) - _ connector.CheckEndpointConnector = (*openshiftConnector)(nil) + _ connector.CallbackConnector = (*openshiftConnector)(nil) + _ connector.RefreshConnector = (*openshiftConnector)(nil) ) type openshiftConnector struct { - apiURL string - redirectURI string - clientID string - clientSecret string - cancel context.CancelFunc - logger log.Logger - httpClient *http.Client - oauth2Config *oauth2.Config - insecureCA bool - rootCA string - groups []string + apiURL string + redirectURI string + clientID string + clientSecret string + cancel context.CancelFunc + logger log.Logger + httpClient *http.Client + oauth2Config *oauth2.Config + insecureCA bool + rootCA string + groups []string + endpointsHealthCheck bool } type user struct { @@ -65,6 +68,23 @@ type user struct { Groups []string `json:"groups" protobuf:"bytes,4,rep,name=groups"` } +// WithHTTPClient will inject a http.Client for the OpenShift connector. This can be used to +// add custom client specific settings. +func WithHTTPClient(httpClient *http.Client) Option { + return func(o *openshiftConnector) { + o.httpClient = httpClient + } +} + +// WithEndpointHealthChecks will enable health checks for OAuth endpoints during creation of the +// OpenShift connector. If the endpoints are not reachable at the time of creation, the connector +// will not be created and a respective error returned. +func WithEndpointHealthChecks() Option { + return func(o *openshiftConnector) { + o.endpointsHealthCheck = true + } +} + // Open returns a connector which can be used to login users through an upstream // OpenShift OAuth2 provider. func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, err error) { @@ -73,20 +93,21 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e return nil, fmt.Errorf("failed to create HTTP client: %w", err) } - return c.OpenWithHTTPClient(id, logger, httpClient) + return c.OpenWithOptions(id, logger, WithHTTPClient(httpClient)) } -// OpenWithHTTPClient returns a connector which can be used to login users through an upstream -// OpenShift OAuth2 provider. It provides the ability to inject a http.Client. -func (c *Config) OpenWithHTTPClient(id string, logger log.Logger, - httpClient *http.Client, +// OpenWithOptions returns a connector which can be used to login users through an upstream +// OpenShift OAuth2 provider. It provides the ability to override or inject values based on +// the given options. +func (c *Config) OpenWithOptions(id string, logger log.Logger, + options ...Option, ) (conn connector.Connector, err error) { ctx, cancel := context.WithCancel(context.Background()) wellKnownURL := strings.TrimSuffix(c.Issuer, "/") + wellKnownURLPath req, err := http.NewRequest(http.MethodGet, wellKnownURL, nil) - openshiftConnector := openshiftConnector{ + openshiftConnector := &openshiftConnector{ apiURL: c.Issuer, cancel: cancel, clientID: c.ClientID, @@ -96,7 +117,10 @@ func (c *Config) OpenWithHTTPClient(id string, logger log.Logger, redirectURI: c.RedirectURI, rootCA: c.RootCA, groups: c.Groups, - httpClient: httpClient, + } + + for _, opt := range options { + opt(openshiftConnector) } var metadata struct { @@ -128,6 +152,12 @@ func (c *Config) OpenWithHTTPClient(id string, logger log.Logger, RedirectURL: c.RedirectURI, } + if openshiftConnector.endpointsHealthCheck { + if err := openshiftConnector.oauth2EndpointsHealthCheck(ctx); err != nil { + return nil, fmt.Errorf("failed healthcheck for openshift oauth endpoints %w", err) + } + } + return &openshiftConnector, nil } @@ -157,7 +187,7 @@ func (e *oauth2Error) Error() string { return e.error + ": " + e.errorDescription } -// HandleCallback parses the request and returns the user's identity +// HandleCallback parses the request and returns the user's identity. func (c *openshiftConnector) HandleCallback(s connector.Scopes, r *http.Request, ) (identity connector.Identity, err error) { @@ -229,7 +259,7 @@ func (c *openshiftConnector) identity(ctx context.Context, s connector.Scopes, return identity, nil } -// user function returns the OpenShift user associated with the authenticated user +// user function returns the OpenShift user associated with the authenticated user. func (c *openshiftConnector) user(ctx context.Context, client *http.Client) (u user, err error) { url := c.apiURL + usersURLPath @@ -259,11 +289,7 @@ func (c *openshiftConnector) user(ctx context.Context, client *http.Client) (u u return u, err } -func (c *openshiftConnector) CheckEndpoint(ctx context.Context) error { - return c.checkOAuth2EndpointsAvailability(ctx) -} - -func (c *openshiftConnector) checkOAuth2EndpointsAvailability(ctx context.Context) error { +func (c *openshiftConnector) oauth2EndpointsHealthCheck(ctx context.Context) error { for _, endpoint := range []string{c.oauth2Config.Endpoint.AuthURL, c.oauth2Config.Endpoint.TokenURL} { req, err := http.NewRequest(http.MethodHead, endpoint, nil) if err != nil { diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index b39cfa26a2..1618c55f29 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -272,12 +272,12 @@ func TestCheckEndpoint(t *testing.T) { AuthURL: s.URL + "/oauth/authorize", TokenURL: s.URL + "/oauth/token", }, }} - err = oc.CheckEndpoint(context.Background()) + err = oc.oauth2EndpointsHealthCheck(context.Background()) expectNil(t, err) s.Close() - err = oc.CheckEndpoint(context.Background()) + err = oc.oauth2EndpointsHealthCheck(context.Background()) expectNotNil(t, err) }