Merge pull request #1436 from bonifaido/gitlab-groups

gitlab: support for group whitelist, add tests
2019-05-08 09:57:51 +02:00 · 2019-05-08 09:57:51 +02:00 · d8f9634afc
commit d8f9634afc
parent 60f47c4228 7b416b5a8e
3 changed files with 280 additions and 12 deletions
--- a/Documentation/connectors/gitlab.md
+++ b/Documentation/connectors/gitlab.md
@ -28,4 +28,9 @@ connectors:
      clientID: $GITLAB_APPLICATION_ID
      clientSecret: $GITLAB_CLIENT_SECRET
      redirectURI: http://127.0.0.1:5556/dex/callback
+      # Optional groups whitelist, communicated through the "groups" scope.
+      # If `groups` is omitted, all of the user's GitLab groups are returned when the groups scope is present.
+      # If `groups` is provided, this acts as a whitelist - only the user's GitLab groups that are in the configured `groups` below will go into the groups claim.  Conversely, if the user is not in any of the configured `groups`, the user will not be authenticated.
+      groups:
+      - my-group
 ```
--- a/connector/gitlab/gitlab.go
+++ b/connector/gitlab/gitlab.go
@ -26,10 +26,11 @@ const (

 // Config holds configuration options for gilab logins.
 type Config struct {
-	BaseURL      string `json:"baseURL"`
-	ClientID     string `json:"clientID"`
-	ClientSecret string `json:"clientSecret"`
-	RedirectURI  string `json:"redirectURI"`
+	BaseURL      string   `json:"baseURL"`
+	ClientID     string   `json:"clientID"`
+	ClientSecret string   `json:"clientSecret"`
+	RedirectURI  string   `json:"redirectURI"`
+	Groups       []string `json:"groups"`
 }

 type gitlabUser struct {
@ -52,6 +53,7 @@ func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error)
 		clientID:     c.ClientID,
 		clientSecret: c.ClientSecret,
 		logger:       logger,
+		groups:       c.Groups,
 	}, nil
 }

@ -68,10 +70,11 @@ var (
 type gitlabConnector struct {
 	baseURL      string
 	redirectURI  string
-	org          string
+	groups       []string
 	clientID     string
 	clientSecret string
 	logger       log.Logger
+	httpClient   *http.Client
 }

 func (c *gitlabConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config {
@ -116,7 +119,11 @@ func (c *gitlabConnector) HandleCallback(s connector.Scopes, r *http.Request) (i
 	}

 	oauth2Config := c.oauth2Config(s)
+
 	ctx := r.Context()
+	if c.httpClient != nil {
+		ctx = context.WithValue(r.Context(), oauth2.HTTPClient, c.httpClient)
+	}

 	token, err := oauth2Config.Exchange(ctx, q.Get("code"))
 	if err != nil {
@ -142,7 +149,7 @@ func (c *gitlabConnector) HandleCallback(s connector.Scopes, r *http.Request) (i
 	}

 	if s.Groups {
-		groups, err := c.groups(ctx, client)
+		groups, err := c.getGroups(ctx, client, s.Groups, user.Username)
 		if err != nil {
 			return identity, fmt.Errorf("gitlab: get groups: %v", err)
 		}
@ -185,7 +192,7 @@ func (c *gitlabConnector) Refresh(ctx context.Context, s connector.Scopes, ident
 	ident.Email = user.Email

 	if s.Groups {
-		groups, err := c.groups(ctx, client)
+		groups, err := c.getGroups(ctx, client, s.Groups, user.Username)
 		if err != nil {
 			return ident, fmt.Errorf("gitlab: get groups: %v", err)
 		}
@ -224,11 +231,15 @@ func (c *gitlabConnector) user(ctx context.Context, client *http.Client) (gitlab
 	return u, nil
 }

-// groups queries the GitLab API for group membership.
+type userInfo struct {
+	Groups []string
+}
+
+// userGroups queries the GitLab API for group membership.
 //
 // The HTTP passed client is expected to be constructed by the golang.org/x/oauth2 package,
 // which inserts a bearer token as part of the request.
-func (c *gitlabConnector) groups(ctx context.Context, client *http.Client) ([]string, error) {
+func (c *gitlabConnector) userGroups(ctx context.Context, client *http.Client) ([]string, error) {
 	req, err := http.NewRequest("GET", c.baseURL+"/oauth/userinfo", nil)
 	if err != nil {
 		return nil, fmt.Errorf("gitlab: new req: %v", err)
@ -247,12 +258,44 @@ func (c *gitlabConnector) groups(ctx context.Context, client *http.Client) ([]st
 		}
 		return nil, fmt.Errorf("%s: %s", resp.Status, body)
 	}
-	u := struct {
-		Groups []string
-	}{}
+	var u userInfo
 	if err := json.NewDecoder(resp.Body).Decode(&u); err != nil {
 		return nil, fmt.Errorf("failed to decode response: %v", err)
 	}

 	return u.Groups, nil
 }
+
+func (c *gitlabConnector) getGroups(ctx context.Context, client *http.Client, groupScope bool, userLogin string) ([]string, error) {
+	gitlabGroups, err := c.userGroups(ctx, client)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(c.groups) > 0 {
+		filteredGroups := filterGroups(gitlabGroups, c.groups)
+		if len(filteredGroups) == 0 {
+			return nil, fmt.Errorf("gitlab: user %q is not in any of the required groups", userLogin)
+		}
+		return filteredGroups, nil
+	} else if groupScope {
+		return gitlabGroups, nil
+	}
+
+	return nil, nil
+}
+
+// Filter the users' group memberships by 'groups' from config.
+func filterGroups(userGroups, configGroups []string) []string {
+	groups := []string{}
+	groupFilter := make(map[string]struct{})
+	for _, group := range configGroups {
+		groupFilter[group] = struct{}{}
+	}
+	for _, group := range userGroups {
+		if _, ok := groupFilter[group]; ok {
+			groups = append(groups, group)
+		}
+	}
+	return groups
+}
--- a/connector/gitlab/gitlab_test.go
+++ b/connector/gitlab/gitlab_test.go
@ -0,0 +1,220 @@
+package gitlab
+
+import (
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"reflect"
+	"testing"
+
+	"github.com/dexidp/dex/connector"
+)
+
+func TestUserGroups(t *testing.T) {
+	s := newTestServer(map[string]interface{}{
+		"/oauth/userinfo": userInfo{
+			Groups: []string{"team-1", "team-2"},
+		},
+	})
+	defer s.Close()
+
+	c := gitlabConnector{baseURL: s.URL}
+	groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs")
+
+	expectNil(t, err)
+	expectEquals(t, groups, []string{
+		"team-1",
+		"team-2",
+	})
+}
+
+func TestUserGroupsWithFiltering(t *testing.T) {
+	s := newTestServer(map[string]interface{}{
+		"/oauth/userinfo": userInfo{
+			Groups: []string{"team-1", "team-2"},
+		},
+	})
+	defer s.Close()
+
+	c := gitlabConnector{baseURL: s.URL, groups: []string{"team-1"}}
+	groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs")
+
+	expectNil(t, err)
+	expectEquals(t, groups, []string{
+		"team-1",
+	})
+}
+
+func TestUserGroupsWithoutOrgs(t *testing.T) {
+	s := newTestServer(map[string]interface{}{
+		"/oauth/userinfo": userInfo{
+			Groups: []string{},
+		},
+	})
+	defer s.Close()
+
+	c := gitlabConnector{baseURL: s.URL}
+	groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs")
+
+	expectNil(t, err)
+	expectEquals(t, len(groups), 0)
+}
+
+// tests that the email is used as their username when they have no username set
+func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
+
+	s := newTestServer(map[string]interface{}{
+		"/api/v4/user": gitlabUser{Email: "some@email.com", ID: 12345678},
+		"/oauth/token": map[string]interface{}{
+			"access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9",
+			"expires_in":   "30",
+		},
+		"/oauth/userinfo": userInfo{
+			Groups: []string{"team-1"},
+		},
+	})
+	defer s.Close()
+
+	hostURL, err := url.Parse(s.URL)
+	expectNil(t, err)
+
+	req, err := http.NewRequest("GET", hostURL.String(), nil)
+	expectNil(t, err)
+
+	c := gitlabConnector{baseURL: s.URL, httpClient: newClient()}
+	identity, err := c.HandleCallback(connector.Scopes{Groups: false}, req)
+
+	expectNil(t, err)
+	expectEquals(t, identity.Username, "some@email.com")
+	expectEquals(t, identity.UserID, "12345678")
+	expectEquals(t, 0, len(identity.Groups))
+
+	c = gitlabConnector{baseURL: s.URL, httpClient: newClient()}
+	identity, err = c.HandleCallback(connector.Scopes{Groups: true}, req)
+
+	expectNil(t, err)
+	expectEquals(t, identity.Username, "some@email.com")
+	expectEquals(t, identity.UserID, "12345678")
+	expectEquals(t, identity.Groups, []string{"team-1"})
+}
+
+func TestLoginUsedAsIDWhenConfigured(t *testing.T) {
+
+	s := newTestServer(map[string]interface{}{
+		"/api/v4/user": gitlabUser{Email: "some@email.com", ID: 12345678, Name: "Joe Bloggs"},
+		"/oauth/token": map[string]interface{}{
+			"access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9",
+			"expires_in":   "30",
+		},
+		"/oauth/userinfo": userInfo{
+			Groups: []string{"team-1"},
+		},
+	})
+	defer s.Close()
+
+	hostURL, err := url.Parse(s.URL)
+	expectNil(t, err)
+
+	req, err := http.NewRequest("GET", hostURL.String(), nil)
+	expectNil(t, err)
+
+	c := gitlabConnector{baseURL: s.URL, httpClient: newClient()}
+	identity, err := c.HandleCallback(connector.Scopes{Groups: true}, req)
+
+	expectNil(t, err)
+	expectEquals(t, identity.UserID, "12345678")
+	expectEquals(t, identity.Username, "Joe Bloggs")
+}
+
+func TestLoginWithTeamWhitelisted(t *testing.T) {
+
+	s := newTestServer(map[string]interface{}{
+		"/api/v4/user": gitlabUser{Email: "some@email.com", ID: 12345678, Name: "Joe Bloggs"},
+		"/oauth/token": map[string]interface{}{
+			"access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9",
+			"expires_in":   "30",
+		},
+		"/oauth/userinfo": userInfo{
+			Groups: []string{"team-1"},
+		},
+	})
+	defer s.Close()
+
+	hostURL, err := url.Parse(s.URL)
+	expectNil(t, err)
+
+	req, err := http.NewRequest("GET", hostURL.String(), nil)
+	expectNil(t, err)
+
+	c := gitlabConnector{baseURL: s.URL, httpClient: newClient(), groups: []string{"team-1"}}
+	identity, err := c.HandleCallback(connector.Scopes{Groups: true}, req)
+
+	expectNil(t, err)
+	expectEquals(t, identity.UserID, "12345678")
+	expectEquals(t, identity.Username, "Joe Bloggs")
+}
+
+func TestLoginWithTeamNonWhitelisted(t *testing.T) {
+
+	s := newTestServer(map[string]interface{}{
+		"/api/v4/user": gitlabUser{Email: "some@email.com", ID: 12345678, Name: "Joe Bloggs", Username: "joebloggs"},
+		"/oauth/token": map[string]interface{}{
+			"access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9",
+			"expires_in":   "30",
+		},
+		"/oauth/userinfo": userInfo{
+			Groups: []string{"team-1"},
+		},
+	})
+	defer s.Close()
+
+	hostURL, err := url.Parse(s.URL)
+	expectNil(t, err)
+
+	req, err := http.NewRequest("GET", hostURL.String(), nil)
+	expectNil(t, err)
+
+	c := gitlabConnector{baseURL: s.URL, httpClient: newClient(), groups: []string{"team-2"}}
+	_, err = c.HandleCallback(connector.Scopes{Groups: true}, req)
+
+	expectNotNil(t, err, "HandleCallback error")
+	expectEquals(t, err.Error(), "gitlab: get groups: gitlab: user \"joebloggs\" is not in any of the required groups")
+}
+
+func newTestServer(responses map[string]interface{}) *httptest.Server {
+	var s *httptest.Server
+	s = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		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)
+	}
+}