From 92e63771ac69563603ffbd68a0c8f82fcbb4f1ed Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Tue, 10 Dec 2019 10:51:09 -0300 Subject: [PATCH 01/10] Added OpenShift connector --- Documentation/connectors/openshift.md | 50 +++++ connector/openshift/openshift.go | 252 ++++++++++++++++++++++++++ connector/openshift/openshift_test.go | 223 +++++++++++++++++++++++ server/server.go | 2 + 4 files changed, 527 insertions(+) create mode 100644 Documentation/connectors/openshift.md create mode 100644 connector/openshift/openshift.go create mode 100644 connector/openshift/openshift_test.go diff --git a/Documentation/connectors/openshift.md b/Documentation/connectors/openshift.md new file mode 100644 index 00000000..a33ca772 --- /dev/null +++ b/Documentation/connectors/openshift.md @@ -0,0 +1,50 @@ +# Authentication using OpenShift + +## Overview + +Dex can make use of users and groups defined within OpenShift by querying the platform provided OAuth server. + +## Configuration + +Create a new OAuth Client by following the steps described in the documentation for [Registering Additional OAuth Clients[(https://docs.openshift.com/container-platform/latest/authentication/configuring-internal-oauth.html#oauth-register-additional-client_configuring-internal-oauth) + +This involves creating a resource similar the following + +```yaml +kind: OAuthClient +apiVersion: oauth.openshift.io/v1 +metadata: + name: dex +# The value that should be utilized as the `client_secret` +secret: "" +# List of valid addresses for the callback. Ensure one of the values that are provided is `(dex issuer)/callback` +redirectURIs: + - "https:////callback" +grantMethod: prompt +``` + +The following is an example of a configuration for `examples/config-dev.yaml`: + +```yaml +connectors: + - type: openshift + # Required field for connector id. + id: openshift + # Required field for connector name. + name: OppenShift + config: + # OpenShift API + baseURL: https://api.mycluster.example.com:6443 + # Credentials can be string literals or pulled from the environment. + clientID: $OPENSHIFT_OAUTH_CLIENT_ID + clientSecret: $OPENSHIFT_OAUTH_CLIENT_SECRET + redirectURI: http://127.0.0.1:5556/dex/ + # Optional: Specify whether to communicate to OpenShift without validating SSL ceertificates + insecureCA: false + # Optional: The location of file containing SSL certificates to commmunicate to OpenShift + rootCA: /etc/ssl/openshift.pem + # Optional list of required groups a user mmust be a member of + groups: + - users + +``` \ No newline at end of file diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go new file mode 100644 index 00000000..69c88741 --- /dev/null +++ b/connector/openshift/openshift.go @@ -0,0 +1,252 @@ +package openshift + +import ( + "context" + "crypto/tls" + "crypto/x509" + "encoding/json" + "fmt" + "io/ioutil" + "net" + "net/http" + "strings" + "time" + + "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/groups" + "github.com/dexidp/dex/pkg/log" + "github.com/dexidp/dex/storage/kubernetes/k8sapi" + "golang.org/x/oauth2" +) + +// 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"` +} + +var ( + _ connector.CallbackConnector = (*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 +} + +type user struct { + k8sapi.TypeMeta `json:",inline"` + k8sapi.ObjectMeta `json:"metadata,omitempty"` + Identities []string `json:"identities" protobuf:"bytes,3,rep,name=identities"` + FullName string `json:"fullName,omitempty" protobuf:"bytes,2,opt,name=fullName"` + Groups []string `json:"groups" protobuf:"bytes,4,rep,name=groups"` +} + +// 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) { + + ctx, cancel := context.WithCancel(context.Background()) + + wellKnownURL := strings.TrimSuffix(c.Issuer, "/") + "/.well-known/oauth-authorization-server" + 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, + groups: c.Groups, + } + + if openshiftConnector.httpClient, err = newHTTPClient(c.InsecureCA, c.RootCA); err != nil { + cancel() + return nil, fmt.Errorf("failed to create HTTP client: %v", err) + } + + var metadata struct { + Auth string `json:"authorization_endpoint"` + Token string `json:"token_endpoint"` + } + + resp, err := openshiftConnector.httpClient.Do(req.WithContext(ctx)) + + if err != nil { + cancel() + return nil, fmt.Errorf("Failed to query OpenShift Endpoint %v", 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", + wellKnownURL, err) + } + + openshiftConnector.oauth2Config = &oauth2.Config{ + ClientID: c.ClientID, + ClientSecret: c.ClientSecret, + Endpoint: oauth2.Endpoint{ + AuthURL: metadata.Auth, TokenURL: metadata.Token, + }, + Scopes: []string{"user:info", "user:check-access", "user:full"}, + RedirectURL: c.RedirectURI, + } + return &openshiftConnector, nil +} + +func (c *openshiftConnector) Close() error { + c.cancel() + return nil +} + +// 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 c.oauth2Config.AuthCodeURL(state), nil +} + +type oauth2Error struct { + error string + errorDescription string +} + +func (e *oauth2Error) Error() string { + if e.errorDescription == "" { + return e.error + } + return e.error + ": " + e.errorDescription +} + +// 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) { + q := r.URL.Query() + if errType := q.Get("error"); errType != "" { + return identity, &oauth2Error{errType, q.Get("error_description")} + } + + ctx := r.Context() + if c.httpClient != nil { + ctx = context.WithValue(r.Context(), oauth2.HTTPClient, c.httpClient) + } + + token, err := c.oauth2Config.Exchange(ctx, q.Get("code")) + if err != nil { + return identity, fmt.Errorf("oidc: failed to get token: %v", err) + } + + client := c.oauth2Config.Client(ctx, token) + + user, err := c.user(ctx, client) + + if err != nil { + return identity, fmt.Errorf("openshift: get user: %v", err) + } + + validGroups := validateRequiredGroups(user.Groups, c.groups) + + if !validGroups { + return identity, fmt.Errorf("openshift: user %q is not in any of the required teams", user.Name) + } + + identity = connector.Identity{ + UserID: user.UID, + Username: user.Name, + PreferredUsername: user.Name, + Groups: user.Groups, + } + + return identity, nil +} + +// 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 + "/apis/user.openshift.io/v1/users/~" + + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return u, fmt.Errorf("new req: %v", err) + } + + resp, err := client.Do(req.WithContext(ctx)) + if err != nil { + return u, fmt.Errorf("get URL %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return u, fmt.Errorf("read body: %v", err) + } + return u, fmt.Errorf("%s: %s", resp.Status, body) + } + + if err := json.NewDecoder(resp.Body).Decode(&u); err != nil { + return u, fmt.Errorf("JSON decode: %v", err) + } + + return u, err +} + +func validateRequiredGroups(userGroups, requiredGroups []string) bool { + + matchingGroups := groups.Filter(userGroups, requiredGroups) + + return len(requiredGroups) == len(matchingGroups) +} + +// newHTTPClient returns a new HTTP client +func newHTTPClient(insecureCA bool, rootCA string) (*http.Client, error) { + tlsConfig := tls.Config{} + + if insecureCA { + tlsConfig = tls.Config{InsecureSkipVerify: true} + } else if rootCA != "" { + tlsConfig := tls.Config{RootCAs: x509.NewCertPool()} + rootCABytes, err := ioutil.ReadFile(rootCA) + if err != nil { + return nil, fmt.Errorf("failed to read root-ca: %v", err) + } + if !tlsConfig.RootCAs.AppendCertsFromPEM(rootCABytes) { + return nil, fmt.Errorf("no certs found in root CA file %q", rootCA) + } + } + + return &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tlsConfig, + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + DualStack: true, + }).DialContext, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + }, + }, nil +} diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go new file mode 100644 index 00000000..474686e8 --- /dev/null +++ b/connector/openshift/openshift_test.go @@ -0,0 +1,223 @@ +package openshift + +import ( + "context" + "crypto/tls" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "reflect" + "testing" + + "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/storage/kubernetes/k8sapi" + "golang.org/x/oauth2" + + "github.com/sirupsen/logrus" +) + +func TestOpen(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) + + c := Config{ + Issuer: s.URL, + ClientID: "testClientId", + ClientSecret: "testClientSecret", + 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 TestGetUser(t *testing.T) { + + s := newTestServer(map[string]interface{}{ + "/apis/user.openshift.io/v1/users/~": user{ + ObjectMeta: k8sapi.ObjectMeta{ + Name: "jdoe", + }, + FullName: "John Doe", + Groups: []string{"users"}, + }, + }) + defer s.Close() + + hostURL, err := url.Parse(s.URL) + expectNil(t, err) + + _, err = http.NewRequest("GET", hostURL.String(), nil) + expectNil(t, err) + + h, err := newHTTPClient(true, "") + + expectNil(t, err) + + oc := openshiftConnector{apiURL: s.URL, httpClient: h} + u, err := oc.user(context.Background(), h) + + expectNil(t, err) + expectEquals(t, u.Name, "jdoe") + expectEquals(t, u.FullName, "John Doe") + expectEquals(t, len(u.Groups), 1) + +} + +func TestVerifyGroupFn(t *testing.T) { + + requiredGroups := []string{"users"} + groupMembership := []string{"users","org1"} + + validGroupMembership := validateRequiredGroups(groupMembership, requiredGroups) + + expectEquals(t, validGroupMembership, true) + +} + +func TestVerifyGroup(t *testing.T) { + + s := newTestServer(map[string]interface{}{ + "/apis/user.openshift.io/v1/users/~": user{ + ObjectMeta: k8sapi.ObjectMeta{ + Name: "jdoe", + }, + FullName: "John Doe", + Groups: []string{"users"}, + }, + }) + defer s.Close() + + hostURL, err := url.Parse(s.URL) + expectNil(t, err) + + _, err = http.NewRequest("GET", hostURL.String(), nil) + expectNil(t, err) + + h, err := newHTTPClient(true, "") + + expectNil(t, err) + + oc := openshiftConnector{apiURL: s.URL, httpClient: h} + u, err := oc.user(context.Background(), h) + + expectNil(t, err) + expectEquals(t, u.Name, "jdoe") + expectEquals(t, u.FullName, "John Doe") + expectEquals(t, len(u.Groups), 1) + +} + +func TestCallbackIdentity(t *testing.T) { + + s := newTestServer(map[string]interface{}{ + "/apis/user.openshift.io/v1/users/~": user{ + ObjectMeta: k8sapi.ObjectMeta{ + Name: "jdoe", + UID: "12345", + }, + FullName: "John Doe", + Groups: []string{"users"}, + }, + "/oauth/token": map[string]interface{}{ + "access_token": "oRzxVjCnohYRHEYEhZshkmakKmoyVoTjfUGC", + "expires_in": "30", + }, + }) + defer s.Close() + + hostURL, err := url.Parse(s.URL) + expectNil(t, err) + + req, err := http.NewRequest("GET", hostURL.String(), nil) + expectNil(t, err) + + h, err := newHTTPClient(true, "") + + expectNil(t, err) + + oc := openshiftConnector{apiURL: s.URL, httpClient: h, oauth2Config: &oauth2.Config{ + Endpoint: oauth2.Endpoint{ + AuthURL: fmt.Sprintf("%s/oauth/authorize", s.URL), + TokenURL: fmt.Sprintf("%s/oauth/token", s.URL), + }, + }} + identity, err := oc.HandleCallback(connector.Scopes{Groups: true}, req) + + expectNil(t, err) + expectEquals(t, identity.UserID, "12345") + expectEquals(t, identity.Username, "jdoe") + expectEquals(t, identity.PreferredUsername, "jdoe") + expectEquals(t, len(identity.Groups), 1) + expectEquals(t, identity.Groups[0], "users") +} + +func newTestServer(responses map[string]interface{}) *httptest.Server { + + var s *httptest.Server + s = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + + responses["/.well-known/oauth-authorization-server"] = map[string]interface{}{ + "issuer": fmt.Sprintf("%s", s.URL), + "authorization_endpoint": fmt.Sprintf("%s/oauth/authorize", s.URL), + "token_endpoint": fmt.Sprintf("%s/oauth/token", s.URL), + "scopes_supported": []string{"user:full", "user:info", "user:check-access", "user:list-scoped-projects", "user:list-projects"}, + "response_types_supported": []string{"token", "code"}, + "grant_types_supported": []string{"authorization_code", "implicit"}, + "code_challenge_methods_supported": []string{"plain", "S256"}, + } + + response := responses[r.RequestURI] + w.Header().Add("Content-Type", "application/json") + json.NewEncoder(w).Encode(response) + })) + + return s +} + +func newClient() *http.Client { + tr := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + return &http.Client{Transport: tr} +} + +func expectNil(t *testing.T, a interface{}) { + if a != nil { + t.Errorf("Expected %+v to equal nil", a) + } +} + +func expectNotNil(t *testing.T, a interface{}, msg string) { + if a == nil { + t.Errorf("Expected %+v to not to be nil", msg) + } +} + +func expectEquals(t *testing.T, a interface{}, b interface{}) { + if !reflect.DeepEqual(a, b) { + t.Errorf("Expected %+v to equal %+v", a, b) + } +} diff --git a/server/server.go b/server/server.go index 4dc7337d..27d93064 100644 --- a/server/server.go +++ b/server/server.go @@ -32,6 +32,7 @@ import ( "github.com/dexidp/dex/connector/microsoft" "github.com/dexidp/dex/connector/mock" "github.com/dexidp/dex/connector/oidc" + "github.com/dexidp/dex/connector/openshift" "github.com/dexidp/dex/connector/saml" "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" @@ -461,6 +462,7 @@ var ConnectorsConfig = map[string]func() ConnectorConfig{ "linkedin": func() ConnectorConfig { return new(linkedin.Config) }, "microsoft": func() ConnectorConfig { return new(microsoft.Config) }, "bitbucket-cloud": func() ConnectorConfig { return new(bitbucketcloud.Config) }, + "openshift": func() ConnectorConfig { return new(openshift.Config) }, // Keep around for backwards compatibility. "samlExperimental": func() ConnectorConfig { return new(saml.Config) }, } From 48954ca716565751a89940aaa3193ee51f10b716 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sat, 21 Dec 2019 11:48:08 -0500 Subject: [PATCH 02/10] Corrected test formatting --- connector/openshift/openshift_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index 474686e8..a4a5c27a 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -89,7 +89,7 @@ func TestGetUser(t *testing.T) { func TestVerifyGroupFn(t *testing.T) { requiredGroups := []string{"users"} - groupMembership := []string{"users","org1"} + groupMembership := []string{"users", "org1"} validGroupMembership := validateRequiredGroups(groupMembership, requiredGroups) From 5881a2cfcad79a7d741f72cacad830b90679cb22 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 22 Dec 2019 00:02:08 -0500 Subject: [PATCH 03/10] Test cleanup --- connector/openshift/openshift_test.go | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index a4a5c27a..a44e5f21 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -2,7 +2,6 @@ package openshift import ( "context" - "crypto/tls" "encoding/json" "fmt" "net/http" @@ -19,7 +18,6 @@ import ( ) func TestOpen(t *testing.T) { - s := newTestServer(map[string]interface{}{}) defer s.Close() @@ -180,7 +178,7 @@ func newTestServer(responses map[string]interface{}) *httptest.Server { s = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { responses["/.well-known/oauth-authorization-server"] = map[string]interface{}{ - "issuer": fmt.Sprintf("%s", s.URL), + "issuer": s.URL, "authorization_endpoint": fmt.Sprintf("%s/oauth/authorize", s.URL), "token_endpoint": fmt.Sprintf("%s/oauth/token", s.URL), "scopes_supported": []string{"user:full", "user:info", "user:check-access", "user:list-scoped-projects", "user:list-projects"}, @@ -197,25 +195,12 @@ func newTestServer(responses map[string]interface{}) *httptest.Server { return s } -func newClient() *http.Client { - tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - } - return &http.Client{Transport: tr} -} - func expectNil(t *testing.T, a interface{}) { if a != nil { t.Errorf("Expected %+v to equal nil", a) } } -func expectNotNil(t *testing.T, a interface{}, msg string) { - if a == nil { - t.Errorf("Expected %+v to not to be nil", msg) - } -} - func expectEquals(t *testing.T, a interface{}, b interface{}) { if !reflect.DeepEqual(a, b) { t.Errorf("Expected %+v to equal %+v", a, b) From db7711d72ac90b77d509d230930656e794f5ff57 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 22 Dec 2019 00:32:05 -0500 Subject: [PATCH 04/10] Test cleanup --- connector/openshift/openshift_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index a44e5f21..5d489259 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -52,7 +52,6 @@ func TestOpen(t *testing.T) { } func TestGetUser(t *testing.T) { - s := newTestServer(map[string]interface{}{ "/apis/user.openshift.io/v1/users/~": user{ ObjectMeta: k8sapi.ObjectMeta{ @@ -85,7 +84,6 @@ func TestGetUser(t *testing.T) { } func TestVerifyGroupFn(t *testing.T) { - requiredGroups := []string{"users"} groupMembership := []string{"users", "org1"} @@ -96,7 +94,6 @@ func TestVerifyGroupFn(t *testing.T) { } func TestVerifyGroup(t *testing.T) { - s := newTestServer(map[string]interface{}{ "/apis/user.openshift.io/v1/users/~": user{ ObjectMeta: k8sapi.ObjectMeta{ @@ -129,7 +126,6 @@ func TestVerifyGroup(t *testing.T) { } func TestCallbackIdentity(t *testing.T) { - s := newTestServer(map[string]interface{}{ "/apis/user.openshift.io/v1/users/~": user{ ObjectMeta: k8sapi.ObjectMeta{ From 02c8f85e4d941b76257c299cc809482082d1f951 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 22 Dec 2019 01:20:46 -0500 Subject: [PATCH 05/10] Resolved newline issues --- connector/openshift/openshift.go | 4 +--- connector/openshift/openshift_test.go | 4 ---- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go index 69c88741..a19ae531 100644 --- a/connector/openshift/openshift.go +++ b/connector/openshift/openshift.go @@ -59,7 +59,6 @@ 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) { - ctx, cancel := context.WithCancel(context.Background()) wellKnownURL := strings.TrimSuffix(c.Issuer, "/") + "/.well-known/oauth-authorization-server" @@ -91,7 +90,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e if err != nil { cancel() - return nil, fmt.Errorf("Failed to query OpenShift Endpoint %v", err) + return nil, fmt.Errorf("failed to query OpenShift endpoint %v", err) } defer resp.Body.Close() @@ -211,7 +210,6 @@ func (c *openshiftConnector) user(ctx context.Context, client *http.Client) (u u } func validateRequiredGroups(userGroups, requiredGroups []string) bool { - matchingGroups := groups.Filter(userGroups, requiredGroups) return len(requiredGroups) == len(matchingGroups) diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index 5d489259..407531e7 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -80,7 +80,6 @@ func TestGetUser(t *testing.T) { expectEquals(t, u.Name, "jdoe") expectEquals(t, u.FullName, "John Doe") expectEquals(t, len(u.Groups), 1) - } func TestVerifyGroupFn(t *testing.T) { @@ -90,7 +89,6 @@ func TestVerifyGroupFn(t *testing.T) { validGroupMembership := validateRequiredGroups(groupMembership, requiredGroups) expectEquals(t, validGroupMembership, true) - } func TestVerifyGroup(t *testing.T) { @@ -122,7 +120,6 @@ func TestVerifyGroup(t *testing.T) { expectEquals(t, u.Name, "jdoe") expectEquals(t, u.FullName, "John Doe") expectEquals(t, len(u.Groups), 1) - } func TestCallbackIdentity(t *testing.T) { @@ -169,7 +166,6 @@ func TestCallbackIdentity(t *testing.T) { } 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 7e89d8ca2498f1ee7f180ec47023f2b438dfec8e Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 22 Dec 2019 01:46:35 -0500 Subject: [PATCH 06/10] Resolved newline issues --- connector/openshift/openshift_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index 407531e7..89910942 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -12,9 +12,8 @@ import ( "github.com/dexidp/dex/connector" "github.com/dexidp/dex/storage/kubernetes/k8sapi" - "golang.org/x/oauth2" - "github.com/sirupsen/logrus" + "golang.org/x/oauth2" ) func TestOpen(t *testing.T) { @@ -168,7 +167,6 @@ func TestCallbackIdentity(t *testing.T) { func newTestServer(responses map[string]interface{}) *httptest.Server { var s *httptest.Server s = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - responses["/.well-known/oauth-authorization-server"] = map[string]interface{}{ "issuer": s.URL, "authorization_endpoint": fmt.Sprintf("%s/oauth/authorize", s.URL), From 075ab0938ebe89f511e32838a4a2679f2e92e0bc Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 22 Dec 2019 02:53:10 -0500 Subject: [PATCH 07/10] Fixed formatting --- connector/openshift/openshift.go | 2 ++ connector/openshift/openshift_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go index a19ae531..f3eb53dc 100644 --- a/connector/openshift/openshift.go +++ b/connector/openshift/openshift.go @@ -15,7 +15,9 @@ import ( "github.com/dexidp/dex/connector" "github.com/dexidp/dex/pkg/groups" "github.com/dexidp/dex/pkg/log" + "github.com/dexidp/dex/storage/kubernetes/k8sapi" + "golang.org/x/oauth2" ) diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index 89910942..2ed50150 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -11,7 +11,9 @@ import ( "testing" "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/storage/kubernetes/k8sapi" + "github.com/sirupsen/logrus" "golang.org/x/oauth2" ) From 5afa02644a616bf9bc0d62ec972087c5547a62f2 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 22 Dec 2019 13:39:40 -0500 Subject: [PATCH 08/10] Added OpenShift documentation to README --- Documentation/connectors/openshift.md | 8 ++++---- README.md | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Documentation/connectors/openshift.md b/Documentation/connectors/openshift.md index a33ca772..c9650846 100644 --- a/Documentation/connectors/openshift.md +++ b/Documentation/connectors/openshift.md @@ -6,7 +6,7 @@ Dex can make use of users and groups defined within OpenShift by querying the pl ## Configuration -Create a new OAuth Client by following the steps described in the documentation for [Registering Additional OAuth Clients[(https://docs.openshift.com/container-platform/latest/authentication/configuring-internal-oauth.html#oauth-register-additional-client_configuring-internal-oauth) +Create a new OAuth Client by following the steps described in the documentation for [Registering Additional OAuth Clients](https://docs.openshift.com/container-platform/latest/authentication/configuring-internal-oauth.html#oauth-register-additional-client_configuring-internal-oauth) This involves creating a resource similar the following @@ -31,10 +31,10 @@ connectors: # Required field for connector id. id: openshift # Required field for connector name. - name: OppenShift + name: OpenShift config: # OpenShift API - baseURL: https://api.mycluster.example.com:6443 + issuer: https://api.mycluster.example.com:6443 # Credentials can be string literals or pulled from the environment. clientID: $OPENSHIFT_OAUTH_CLIENT_ID clientSecret: $OPENSHIFT_OAUTH_CLIENT_SECRET @@ -47,4 +47,4 @@ connectors: groups: - users -``` \ No newline at end of file +``` diff --git a/README.md b/README.md index 7247270a..e1d34604 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,7 @@ Dex implements the following connectors: | [Microsoft](Documentation/connectors/microsoft.md) | yes | yes | no | beta | | | [AuthProxy](Documentation/connectors/authproxy.md) | no | no | no | alpha | Authentication proxies such as Apache2 mod_auth, etc. | | [Bitbucket Cloud](Documentation/connectors/bitbucketcloud.md) | yes | yes | no | alpha | | +| [OpenShift](Documentation/connectors/openshift.md) | no | yes | no | stable | | Stable, beta, and alpha are defined as: From 296659cb504d365fa47c4104c0d229b5008ea570 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Thu, 26 Dec 2019 03:14:20 -0600 Subject: [PATCH 09/10] Reduced OpenShift scopes and enhanced documentation --- Documentation/connectors/openshift.md | 35 ++++++++++++++++++++++++--- connector/openshift/openshift.go | 4 +-- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/Documentation/connectors/openshift.md b/Documentation/connectors/openshift.md index c9650846..bd3df40d 100644 --- a/Documentation/connectors/openshift.md +++ b/Documentation/connectors/openshift.md @@ -6,9 +6,37 @@ Dex can make use of users and groups defined within OpenShift by querying the pl ## Configuration -Create a new OAuth Client by following the steps described in the documentation for [Registering Additional OAuth Clients](https://docs.openshift.com/container-platform/latest/authentication/configuring-internal-oauth.html#oauth-register-additional-client_configuring-internal-oauth) -This involves creating a resource similar the following +### Creating an OAuth Client + +Two forms of OAuth Clients can be utilized: + +* [Using a Service Account as an OAuth Client](https://docs.openshift.com/container-platform/latest/authentication/using-service-accounts-as-oauth-client.html) (Recommended) +* [Registering An Additional OAuth Client](https://docs.openshift.com/container-platform/latest/authentication/configuring-internal-oauth.html#oauth-register-additional-client_configuring-internal-oauth) + +#### Using a Service Account as an OAuth Client + +OpenShift Service Accounts can be used as a constrained form of OAuth client. Making use of a Service Account to represent an OAuth Client is the recommended option as it does not require elevated privileged within the OpenShift cluster. Create a new Service Account or make use of an existing Service Account. + +Patch the Service Account to add an annotation for location of the Redirect URI + +``` +oc patch serviceaccount --type='json' -p='[{"op": "add", "path": "/metadata/annotations/serviceaccounts.openshift.io~1oauth-redirecturi.dex", "value":"https:////callback"}]' +``` + +The Client ID for a Service Account representing an OAuth Client takes the form ` + +The Client Secret for a Service Account representing an OAuth Client is the long lived OAuth Token that is configued for the Service Account. Execute the following command to retrieve the OAuth Token. + +``` +oc serviceaccounts get-token +``` + +#### Registering An Additional OAuth Client + +Instead of using a constrained form of Service Account to represent an OAuth Client, an additional OAuthClient resource can be created. + +Create a new OAuthClient resource similar to the following: ```yaml kind: OAuthClient @@ -23,6 +51,8 @@ redirectURIs: grantMethod: prompt ``` +### Dex Configuration + The following is an example of a configuration for `examples/config-dev.yaml`: ```yaml @@ -46,5 +76,4 @@ connectors: # Optional list of required groups a user mmust be a member of groups: - users - ``` diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go index f3eb53dc..e1974694 100644 --- a/connector/openshift/openshift.go +++ b/connector/openshift/openshift.go @@ -109,7 +109,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e Endpoint: oauth2.Endpoint{ AuthURL: metadata.Auth, TokenURL: metadata.Token, }, - Scopes: []string{"user:info", "user:check-access", "user:full"}, + Scopes: []string{"user:info"}, RedirectURL: c.RedirectURI, } return &openshiftConnector, nil @@ -168,7 +168,7 @@ func (c *openshiftConnector) HandleCallback(s connector.Scopes, r *http.Request) validGroups := validateRequiredGroups(user.Groups, c.groups) if !validGroups { - return identity, fmt.Errorf("openshift: user %q is not in any of the required teams", user.Name) + return identity, fmt.Errorf("openshift: user %q is not in any of the required groups", user.Name) } identity = connector.Identity{ From d31f6eabd467dd2b9ddb1aae184b7c67f44f5dd2 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Thu, 26 Dec 2019 20:32:12 -0600 Subject: [PATCH 10/10] Corrected logic in group verification --- connector/openshift/openshift.go | 14 ++++++++------ connector/openshift/openshift_test.go | 24 +++++++++++++++++++++--- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go index e1974694..6ac5d044 100644 --- a/connector/openshift/openshift.go +++ b/connector/openshift/openshift.go @@ -165,10 +165,12 @@ func (c *openshiftConnector) HandleCallback(s connector.Scopes, r *http.Request) return identity, fmt.Errorf("openshift: get user: %v", err) } - validGroups := validateRequiredGroups(user.Groups, c.groups) + if len(c.groups) > 0 { + validGroups := validateAllowedGroups(user.Groups, c.groups) - if !validGroups { - return identity, fmt.Errorf("openshift: user %q is not in any of the required groups", user.Name) + if !validGroups { + return identity, fmt.Errorf("openshift: user %q is not in any of the required groups", user.Name) + } } identity = connector.Identity{ @@ -211,10 +213,10 @@ func (c *openshiftConnector) user(ctx context.Context, client *http.Client) (u u return u, err } -func validateRequiredGroups(userGroups, requiredGroups []string) bool { - matchingGroups := groups.Filter(userGroups, requiredGroups) +func validateAllowedGroups(userGroups, allowedGroups []string) bool { + matchingGroups := groups.Filter(userGroups, allowedGroups) - return len(requiredGroups) == len(matchingGroups) + return len(matchingGroups) != 0 } // newHTTPClient returns a new HTTP client diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index 2ed50150..316af60a 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -83,11 +83,29 @@ func TestGetUser(t *testing.T) { expectEquals(t, len(u.Groups), 1) } -func TestVerifyGroupFn(t *testing.T) { - requiredGroups := []string{"users"} +func TestVerifySingleGroupFn(t *testing.T) { + allowedGroups := []string{"users"} groupMembership := []string{"users", "org1"} - validGroupMembership := validateRequiredGroups(groupMembership, requiredGroups) + validGroupMembership := validateAllowedGroups(groupMembership, allowedGroups) + + expectEquals(t, validGroupMembership, true) +} + +func TestVerifySingleGroupFailureFn(t *testing.T) { + allowedGroups := []string{"admins"} + groupMembership := []string{"users"} + + validGroupMembership := validateAllowedGroups(groupMembership, allowedGroups) + + expectEquals(t, validGroupMembership, false) +} + +func TestVerifyMultipleGroupFn(t *testing.T) { + allowedGroups := []string{"users", "admins"} + groupMembership := []string{"users", "org1"} + + validGroupMembership := validateAllowedGroups(groupMembership, allowedGroups) expectEquals(t, validGroupMembership, true) }