diff --git a/Documentation/connectors/gitlab.md b/Documentation/connectors/gitlab.md
index 1ca93d5095510a864474f31e9b413e515eeb66cf..e22d0b46cfb5c59acb58543dbf8776a213b92f93 100644
--- a/Documentation/connectors/gitlab.md
+++ b/Documentation/connectors/gitlab.md
@@ -10,6 +10,8 @@ When a client redeems a refresh token through dex, dex will re-query GitLab to u
Register a new application via `User Settings -> Applications` ensuring the callback URL is `(dex issuer)/callback`. For example if dex is listening at the non-root path `https://auth.example.com/dex` the callback would be `https://auth.example.com/dex/callback`.
+The application requires the user to grant the `read_user` and `openid` scopes. The latter is required only if group membership is a desired claim.
+
The following is an example of a configuration for `examples/config-dev.yaml`:
```yaml
@@ -20,10 +22,10 @@ connectors:
# Required field for connector name.
name: GitLab
config:
- # optional, default = https://gitlab.com
+ # optional, default = https://gitlab.com
baseURL: https://gitlab.com
- # Credentials can be string literals or pulled from the environment.
- clientID: $GITLAB_APPLICATION_ID
+ # Credentials can be string literals or pulled from the environment.
+ clientID: $GITLAB_APPLICATION_ID
clientSecret: $GITLAB_CLIENT_SECRET
redirectURI: http://127.0.0.1:5556/dex/callback
```
diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go
index 5d1fc156f28e9fc1a709e7e9abab3a1918136563..41b0beb26154da99623c563a325d9a52801b3fcb 100644
--- a/connector/gitlab/gitlab.go
+++ b/connector/gitlab/gitlab.go
@@ -8,7 +8,6 @@ import (
"fmt"
"io/ioutil"
"net/http"
- "regexp"
"strconv"
"github.com/sirupsen/logrus"
@@ -18,14 +17,11 @@ import (
)
const (
- // https://docs.gitlab.com/ee/integration/oauth_provider.html#authorized-applications
+ // read operations of the /api/v4/user endpoint
scopeUser = "read_user"
- scopeAPI = "api"
-)
-
-var (
- reNext = regexp.MustCompile("<([^>]+)>; rel=\"next\"")
- reLast = regexp.MustCompile("<([^>]+)>; rel=\"last\"")
+ // used to retrieve groups from /oauth/userinfo
+ // https://docs.gitlab.com/ee/integration/openid_connect_provider.html
+ scopeOpenID = "openid"
)
// Config holds configuration options for gilab logins.
@@ -45,12 +41,6 @@ type gitlabUser struct {
IsAdmin bool
}
-type gitlabGroup struct {
- ID int
- Name string
- Path string
-}
-
// Open returns a strategy for logging in through GitLab.
func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) {
if c.BaseURL == "" {
@@ -87,7 +77,7 @@ type gitlabConnector struct {
func (c *gitlabConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config {
gitlabScopes := []string{scopeUser}
if scopes.Groups {
- gitlabScopes = []string{scopeAPI}
+ gitlabScopes = []string{scopeUser, scopeOpenID}
}
gitlabEndpoint := oauth2.Endpoint{AuthURL: c.baseURL + "/oauth/authorize", TokenURL: c.baseURL + "/oauth/token"}
@@ -239,64 +229,30 @@ func (c *gitlabConnector) user(ctx context.Context, client *http.Client) (gitlab
// 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) {
-
- apiURL := c.baseURL + "/api/v4/groups"
-
- groups := []string{}
- var gitlabGroups []gitlabGroup
- for {
- // 100 is the maximum number for per_page that allowed by gitlab
- req, err := http.NewRequest("GET", apiURL, nil)
- if err != nil {
- return nil, fmt.Errorf("gitlab: new req: %v", err)
- }
- req = req.WithContext(ctx)
- resp, err := client.Do(req)
- if err != nil {
- return nil, fmt.Errorf("gitlab: get groups: %v", err)
- }
- defer resp.Body.Close()
-
- if resp.StatusCode != http.StatusOK {
- body, err := ioutil.ReadAll(resp.Body)
- if err != nil {
- return nil, fmt.Errorf("gitlab: read body: %v", err)
- }
- return nil, fmt.Errorf("%s: %s", resp.Status, body)
- }
-
- if err := json.NewDecoder(resp.Body).Decode(&gitlabGroups); err != nil {
- return nil, fmt.Errorf("gitlab: unmarshal groups: %v", err)
- }
-
- for _, group := range gitlabGroups {
- groups = append(groups, group.Name)
- }
-
- link := resp.Header.Get("Link")
-
- apiURL = nextURL(apiURL, link)
- if apiURL == "" {
- break
- }
+ req, err := http.NewRequest("GET", c.baseURL+"/oauth/userinfo", nil)
+ if err != nil {
+ return nil, fmt.Errorf("gitlab: new req: %v", err)
}
- return groups, nil
-}
-
-func nextURL(url string, link string) string {
- if len(reLast.FindStringSubmatch(link)) > 1 {
- lastPageURL := reLast.FindStringSubmatch(link)[1]
+ req = req.WithContext(ctx)
+ resp, err := client.Do(req)
+ if err != nil {
+ return nil, fmt.Errorf("gitlab: get URL %v", err)
+ }
+ defer resp.Body.Close()
- if url == lastPageURL {
- return ""
+ if resp.StatusCode != http.StatusOK {
+ body, err := ioutil.ReadAll(resp.Body)
+ if err != nil {
+ return nil, fmt.Errorf("gitlab: read body: %v", err)
}
- } else {
- return ""
+ return nil, fmt.Errorf("%s: %s", resp.Status, body)
}
-
- if len(reNext.FindStringSubmatch(link)) > 1 {
- return reNext.FindStringSubmatch(link)[1]
+ u := struct {
+ Groups []string
+ }{}
+ if err := json.NewDecoder(resp.Body).Decode(&u); err != nil {
+ return nil, fmt.Errorf("failed to decode response: %v", err)
}
- return ""
+ return u.Groups, nil
}
diff --git a/connector/gitlab/gitlab_test.go b/connector/gitlab/gitlab_test.go
deleted file mode 100644
index ef669261e135c15c129ac280a9715e48749fc0d9..0000000000000000000000000000000000000000
--- a/connector/gitlab/gitlab_test.go
+++ /dev/null
@@ -1,30 +0,0 @@
-package gitlab
-
-import "testing"
-
-var nextURLTests = []struct {
- link string
- expected string
-}{
- {"<https://gitlab.com/api/v4/groups?page=2&per_page=20>; rel=\"next\", " +
- "<https://gitlab.com/api/v4/groups?page=1&per_page=20>; rel=\"prev\"; pet=\"cat\", " +
- "<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"last\"",
- "https://gitlab.com/api/v4/groups?page=2&per_page=20"},
- {"<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"next\", " +
- "<https://gitlab.com/api/v4/groups?page=2&per_page=20>; rel=\"prev\"; pet=\"dog\", " +
- "<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"last\"",
- "https://gitlab.com/api/v4/groups?page=3&per_page=20"},
- {"<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"prev\"; pet=\"bunny\", " +
- "<https://gitlab.com/api/v4/groups?page=3&per_page=20>; rel=\"last\"",
- ""},
-}
-
-func TestNextURL(t *testing.T) {
- apiURL := "https://gitlab.com/api/v4/groups"
- for _, tt := range nextURLTests {
- apiURL = nextURL(apiURL, tt.link)
- if apiURL != tt.expected {
- t.Errorf("Should have returned %s, got %s", tt.expected, apiURL)
- }
- }
-}