From 6f8dd5e9157214074478dfde98efd68afa23a631 Mon Sep 17 00:00:00 2001 From: Onkar Bhat Date: Wed, 16 Sep 2020 11:38:05 -0700 Subject: [PATCH] Add the ability to configure the client id and client secret using environment variables for the OpenShift connector. Signed-off-by: Onkar Bhat --- connector/openshift/openshift.go | 55 ++++++++++--- connector/openshift/openshift_test.go | 106 ++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 11 deletions(-) diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go index 0666935620..087f404db6 100644 --- a/connector/openshift/openshift.go +++ b/connector/openshift/openshift.go @@ -9,6 +9,7 @@ import ( "io/ioutil" "net" "net/http" + "os" "strings" "time" @@ -23,13 +24,15 @@ import ( // 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"` + Issuer string `json:"issuer"` + ClientID string `json:"clientID"` + ClientIDFromEnv string `json:"clientIDFromEnv"` + ClientSecret string `json:"clientSecret"` + ClientSecretFromEnv string `json:"clientSecretFromEnv"` + RedirectURI string `json:"redirectURI"` + Groups []string `json:"groups"` + InsecureCA bool `json:"insecureCA"` + RootCA string `json:"rootCA"` } var ( @@ -66,11 +69,41 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e wellKnownURL := strings.TrimSuffix(c.Issuer, "/") + "/.well-known/oauth-authorization-server" req, err := http.NewRequest(http.MethodGet, wellKnownURL, nil) + if c.ClientIDFromEnv == "" && c.ClientID == "" { + return nil, fmt.Errorf("invalid config: clientID or clientIDEnv are required for the OpenShift connector") + } + clientID := c.ClientID + if c.ClientIDFromEnv != "" { + if c.ClientID != "" { + return nil, fmt.Errorf("invalid config: clientID and clientIDEnv are exclusive for the OpenShift connector") + } + var ok bool + clientID, ok = os.LookupEnv(c.ClientIDFromEnv) + if !ok || clientID == "" { + return nil, fmt.Errorf("invalid config: environment variable for the client ID was not set to a valid value") + } + } + + if c.ClientSecretFromEnv == "" && c.ClientSecret == "" { + return nil, fmt.Errorf("invalid config: clientSecret or clientSecretEnv are required for the OpenShift connector") + } + clientSecret := c.ClientSecret + if c.ClientSecretFromEnv != "" { + if c.ClientSecret != "" { + return nil, fmt.Errorf("invalid config: clientSecret and clientSecretEnv are exclusive for the OpenShift connector") + } + var ok bool + clientSecret, ok = os.LookupEnv(c.ClientSecretFromEnv) + if !ok || clientSecret == "" { + return nil, fmt.Errorf("invalid config: environment variable for the client secret was not set to a valid value") + } + } + openshiftConnector := openshiftConnector{ apiURL: c.Issuer, cancel: cancel, - clientID: c.ClientID, - clientSecret: c.ClientSecret, + clientID: clientID, + clientSecret: clientSecret, insecureCA: c.InsecureCA, logger: logger, redirectURI: c.RedirectURI, @@ -104,8 +137,8 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e } openshiftConnector.oauth2Config = &oauth2.Config{ - ClientID: c.ClientID, - ClientSecret: c.ClientSecret, + ClientID: clientID, + ClientSecret: clientSecret, Endpoint: oauth2.Endpoint{ AuthURL: metadata.Auth, TokenURL: metadata.Token, }, diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index d4efa14d84..5279abd6d8 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "os" "reflect" "testing" @@ -52,6 +53,111 @@ func TestOpen(t *testing.T) { expectEquals(t, oc.oauth2Config.Endpoint.TokenURL, fmt.Sprintf("%s/oauth/token", s.URL)) } +func TestOpenWithEnvsSuccess(t *testing.T) { + s := newTestServer(map[string]interface{}{}) + defer s.Close() + + hostURL, err := url.Parse(s.URL) + expectNil(t, err) + + _, err = http.NewRequest("GET", hostURL.String(), nil) + expectNil(t, err) + + os.Setenv("TEST_CLIENT_ID", "testClientId") + os.Setenv("TEST_CLIENT_SECRET", "testClientSecret") + + c := Config{ + Issuer: s.URL, + ClientIDFromEnv: "TEST_CLIENT_ID", + ClientSecretFromEnv: "TEST_CLIENT_SECRET", + RedirectURI: "https://localhost/callback", + InsecureCA: true, + } + + logger := logrus.New() + + oconfig, err := c.Open("id", logger) + + oc, ok := oconfig.(*openshiftConnector) + + expectNil(t, err) + expectEquals(t, ok, true) + expectEquals(t, oc.apiURL, s.URL) + expectEquals(t, oc.clientID, "testClientId") + expectEquals(t, oc.clientSecret, "testClientSecret") + expectEquals(t, oc.redirectURI, "https://localhost/callback") + expectEquals(t, oc.oauth2Config.Endpoint.AuthURL, fmt.Sprintf("%s/oauth/authorize", s.URL)) + expectEquals(t, oc.oauth2Config.Endpoint.TokenURL, fmt.Sprintf("%s/oauth/token", s.URL)) +} + +func TestOpenFailuresForEnvCases(t *testing.T) { + s := newTestServer(map[string]interface{}{}) + defer s.Close() + + hostURL, err := url.Parse(s.URL) + expectNil(t, err) + + _, err = http.NewRequest("GET", hostURL.String(), nil) + expectNil(t, err) + + tests := []struct { + clientID string + clientIDFromEnv string + clientSecret string + clientSecretFromEnv string + expectedError error + }{ + { + clientID: "", + clientIDFromEnv: "", + clientSecret: "testClientSecret", + clientSecretFromEnv: "", + expectedError: fmt.Errorf("invalid config: clientID or clientIDEnv are required for the OpenShift connector"), + }, + { + clientID: "clientID", + clientIDFromEnv: "TEST_CLIENT_ID", + clientSecret: "", + clientSecretFromEnv: "TEST_CLIENT_SECRET", + expectedError: fmt.Errorf("invalid config: clientID and clientIDEnv are exclusive for the OpenShift connector"), + }, + { + clientID: "clientID", + clientIDFromEnv: "", + clientSecret: "", + clientSecretFromEnv: "", + expectedError: fmt.Errorf("invalid config: clientSecret or clientSecretEnv are required for the OpenShift connector"), + }, + { + clientID: "", + clientIDFromEnv: "TEST_CLIENT_ID", + clientSecret: "clientSecret", + clientSecretFromEnv: "TEST_CLIENT_SECRET", + expectedError: fmt.Errorf("invalid config: clientSecret and clientSecretEnv are exclusive for the OpenShift connector"), + }, + } + + for _, tc := range tests { + c := Config{ + Issuer: s.URL, + ClientID: tc.clientID, + ClientIDFromEnv: tc.clientIDFromEnv, + ClientSecret: tc.clientSecret, + ClientSecretFromEnv: tc.clientSecretFromEnv, + RedirectURI: "https://localhost/callback", + InsecureCA: true, + } + + logger := logrus.New() + + oconfig, err := c.Open("id", logger) + expectEquals(t, err, tc.expectedError) + + _, ok := oconfig.(*openshiftConnector) + expectEquals(t, ok, false) + } +} + func TestGetUser(t *testing.T) { s := newTestServer(map[string]interface{}{ "/apis/user.openshift.io/v1/users/~": user{