From a08a5811d4bd87b1f83b19b893fbd213f50168ba Mon Sep 17 00:00:00 2001 From: Nandor Kracser Date: Thu, 25 Apr 2019 12:47:01 +0200 Subject: [PATCH 1/2] gitlab: support for group whitelist --- Documentation/connectors/gitlab.md | 5 +++ connector/gitlab/gitlab.go | 54 +++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/Documentation/connectors/gitlab.md b/Documentation/connectors/gitlab.md index e22d0b46..f85e24c7 100644 --- 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 ``` diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go index 58ecba17..fd932142 100644 --- 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,7 +70,7 @@ var ( type gitlabConnector struct { baseURL string redirectURI string - org string + groups []string clientID string clientSecret string logger log.Logger @@ -142,7 +144,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 +187,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 +226,11 @@ func (c *gitlabConnector) user(ctx context.Context, client *http.Client) (gitlab return u, nil } -// groups queries the GitLab API for group membership. +// 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) @@ -256,3 +258,37 @@ func (c *gitlabConnector) groups(ctx context.Context, client *http.Client) ([]st 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 +} From 7b416b5a8e5c364986a5f6ed1d310c740d5f4359 Mon Sep 17 00:00:00 2001 From: Nandor Kracser Date: Wed, 1 May 2019 20:13:44 +0200 Subject: [PATCH 2/2] gitlab: add tests --- connector/gitlab/gitlab.go | 13 +- connector/gitlab/gitlab_test.go | 220 ++++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 3 deletions(-) create mode 100644 connector/gitlab/gitlab_test.go diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go index fd932142..2c031712 100644 --- a/connector/gitlab/gitlab.go +++ b/connector/gitlab/gitlab.go @@ -74,6 +74,7 @@ type gitlabConnector struct { clientID string clientSecret string logger log.Logger + httpClient *http.Client } func (c *gitlabConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config { @@ -118,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 { @@ -226,6 +231,10 @@ func (c *gitlabConnector) user(ctx context.Context, client *http.Client) (gitlab return u, nil } +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, @@ -249,9 +258,7 @@ func (c *gitlabConnector) userGroups(ctx context.Context, client *http.Client) ( } 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) } diff --git a/connector/gitlab/gitlab_test.go b/connector/gitlab/gitlab_test.go new file mode 100644 index 00000000..4be5e0f2 --- /dev/null +++ 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) + } +}