diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go index e604dadf..05919973 100644 --- a/connector/openshift/openshift.go +++ b/connector/openshift/openshift.go @@ -28,14 +28,13 @@ const ( // Config holds configuration options for OpenShift login type Config struct { - Issuer string `json:"issuer"` - ClientID string `json:"clientID"` - ClientSecret string `json:"clientSecret"` - RedirectURI string `json:"redirectURI"` - Groups []string `json:"groups"` - InsecureCA bool `json:"insecureCA"` - RootCA string `json:"rootCA"` - IncludeSystemRootCAs bool `json:"includeSystemRootCAs"` + Issuer string `json:"issuer"` + ClientID string `json:"clientID"` + ClientSecret string `json:"clientSecret"` + RedirectURI string `json:"redirectURI"` + Groups []string `json:"groups"` + InsecureCA bool `json:"insecureCA"` + RootCA string `json:"rootCA"` } var ( @@ -44,18 +43,17 @@ var ( ) 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 - includeSystemRootCAs bool - 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 } type user struct { @@ -69,27 +67,34 @@ type user struct { // 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) { + httpClient, err := newHTTPClient(c.InsecureCA, c.RootCA) + if err != nil { + return nil, fmt.Errorf("failed to create HTTP client: %w", err) + } + + return c.OpenWithHTTPClient(id, logger, 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) (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{ - apiURL: c.Issuer, - cancel: cancel, - clientID: c.ClientID, - clientSecret: c.ClientSecret, - insecureCA: c.InsecureCA, - logger: logger, - redirectURI: c.RedirectURI, - rootCA: c.RootCA, - includeSystemRootCAs: c.IncludeSystemRootCAs, - groups: c.Groups, - } - - if openshiftConnector.httpClient, err = newHTTPClient(c.InsecureCA, c.RootCA, c.IncludeSystemRootCAs); err != nil { - cancel() - return nil, fmt.Errorf("failed to create HTTP client: %v", err) + apiURL: c.Issuer, + cancel: cancel, + clientID: c.ClientID, + clientSecret: c.ClientSecret, + insecureCA: c.InsecureCA, + logger: logger, + redirectURI: c.RedirectURI, + rootCA: c.RootCA, + groups: c.Groups, + httpClient: httpClient, } var metadata struct { @@ -100,14 +105,14 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e resp, err := openshiftConnector.httpClient.Do(req.WithContext(ctx)) if err != nil { cancel() - return nil, fmt.Errorf("failed to query OpenShift endpoint %v", err) + return nil, fmt.Errorf("failed to query OpenShift endpoint %w", err) } defer resp.Body.Close() if err := json.NewDecoder(resp.Body).Decode(&metadata); err != nil { cancel() - return nil, fmt.Errorf("discovery through endpoint %s failed to decode body: %v", + return nil, fmt.Errorf("discovery through endpoint %s failed to decode body: %w", wellKnownURL, err) } @@ -131,7 +136,8 @@ func (c *openshiftConnector) Close() error { // LoginURL returns the URL to redirect the user to login with. func (c *openshiftConnector) LoginURL(scopes connector.Scopes, callbackURL, state string) (string, error) { if c.redirectURI != callbackURL { - return "", fmt.Errorf("expected callback URL %q did not match the URL in the config %q", callbackURL, c.redirectURI) + return "", fmt.Errorf("expected callback URL %q did not match the URL in the config %q", + callbackURL, c.redirectURI) } return c.oauth2Config.AuthCodeURL(state), nil } @@ -149,7 +155,8 @@ func (e *oauth2Error) Error() string { } // 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) { +func (c *openshiftConnector) HandleCallback(s connector.Scopes, + r *http.Request) (identity connector.Identity, err error) { q := r.URL.Query() if errType := q.Get("error"); errType != "" { return identity, &oauth2Error{errType, q.Get("error_description")} @@ -168,7 +175,8 @@ func (c *openshiftConnector) HandleCallback(s connector.Scopes, r *http.Request) return c.identity(ctx, s, token) } -func (c *openshiftConnector) Refresh(ctx context.Context, s connector.Scopes, oldID connector.Identity) (connector.Identity, error) { +func (c *openshiftConnector) Refresh(ctx context.Context, s connector.Scopes, + oldID connector.Identity) (connector.Identity, error) { var token oauth2.Token err := json.Unmarshal(oldID.ConnectorData, &token) if err != nil { @@ -180,7 +188,8 @@ func (c *openshiftConnector) Refresh(ctx context.Context, s connector.Scopes, ol return c.identity(ctx, s, &token) } -func (c *openshiftConnector) identity(ctx context.Context, s connector.Scopes, token *oauth2.Token) (identity connector.Identity, err error) { +func (c *openshiftConnector) identity(ctx context.Context, s connector.Scopes, + token *oauth2.Token) (identity connector.Identity, err error) { client := c.oauth2Config.Client(ctx, token) user, err := c.user(ctx, client) if err != nil { @@ -251,21 +260,12 @@ func validateAllowedGroups(userGroups, allowedGroups []string) bool { } // newHTTPClient returns a new HTTP client -func newHTTPClient(insecureCA bool, rootCA string, includeSystemRootCAs bool) (*http.Client, error) { +func newHTTPClient(insecureCA bool, rootCA string) (*http.Client, error) { tlsConfig := tls.Config{} - if insecureCA { tlsConfig = tls.Config{InsecureSkipVerify: true} } else if rootCA != "" { - if !includeSystemRootCAs { - tlsConfig = tls.Config{RootCAs: x509.NewCertPool()} - } else { - systemCAs, err := x509.SystemCertPool() - if err != nil { - return nil, fmt.Errorf("failed to read host CA: %w", err) - } - tlsConfig = tls.Config{RootCAs: systemCAs} - } + tlsConfig = tls.Config{RootCAs: x509.NewCertPool()} rootCABytes, err := os.ReadFile(rootCA) if err != nil { return nil, fmt.Errorf("failed to read root-ca: %w", err) diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index d119fe80..6280b831 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -70,7 +70,7 @@ func TestGetUser(t *testing.T) { _, err = http.NewRequest("GET", hostURL.String(), nil) expectNil(t, err) - h, err := newHTTPClient(true, "", false) + h, err := newHTTPClient(true, "") expectNil(t, err) @@ -128,7 +128,7 @@ func TestVerifyGroup(t *testing.T) { _, err = http.NewRequest("GET", hostURL.String(), nil) expectNil(t, err) - h, err := newHTTPClient(true, "", false) + h, err := newHTTPClient(true, "") expectNil(t, err) @@ -164,7 +164,7 @@ func TestCallbackIdentity(t *testing.T) { req, err := http.NewRequest("GET", hostURL.String(), nil) expectNil(t, err) - h, err := newHTTPClient(true, "", false) + h, err := newHTTPClient(true, "") expectNil(t, err) @@ -198,7 +198,7 @@ func TestRefreshIdentity(t *testing.T) { }) defer s.Close() - h, err := newHTTPClient(true, "", false) + h, err := newHTTPClient(true, "") expectNil(t, err) oc := openshiftConnector{apiURL: s.URL, httpClient: h, oauth2Config: &oauth2.Config{ @@ -237,7 +237,7 @@ func TestRefreshIdentityFailure(t *testing.T) { }) defer s.Close() - h, err := newHTTPClient(true, "", false) + h, err := newHTTPClient(true, "") expectNil(t, err) oc := openshiftConnector{apiURL: s.URL, httpClient: h, oauth2Config: &oauth2.Config{