Repository: pusher/oauth2_proxy
Pull request #231 (2019-08-01 15:40:51)

Rework GitLab provider
##  Description

Similar to #137, I've implemented a group membership and email domain check for the GitLab provider. The difference is that I've reimplemented the provider to use [OIDC](https://docs.gitlab.com/ce/integration/openid_connect_provider.html) so that group memberships [can be retrieved](https://docs.gitlab.com/ce/integration/openid_connect_provider.html#shared-information) without requiring the whole `api` scope. Instead it only requires the `openid`, `profile` and `email` scopes (which were already used by default).

It does change how the GitLab provider is used, as it now requires specifying an OIDC issuer URL to reference a custom GitLab instance. This is mentioned in the updated documentation.

Example parameters:

```
--provider=gitlab
--oidc-issuer-url=https://gitlab.mycompany.com
--gitlab-group=infra
```

## Motivation and Context

It implements features that people have asked for the GitLab provider to support (#228).

## How Has This Been Tested?

I've tested the provider with both gitlab.com and a self-hosted GitLab instance, along with a new suite of unit tests.

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My change requires a change to the documentation or CHANGELOG.
- [x] I have updated the documentation/CHANGELOG accordingly.
- [x] I have created a feature (non-master) branch for my PR.


sc250024: @Overv Thanks so much for the PR! I just did some testing with Kubernetes using a Docker image I built locally from commit https://github.com/pusher/oauth2_proxy/pull/231/commits/b530f18827d93eb138791eefb9cda208f3af4e53, (find it here: `sc250024/oauth2_proxy:b530f18`)and it worked like a charm.

I've made some comments regarding the documentation, but here was my setup in case anyone wants to replicate.

<details>
<summary>values.yaml</summary>
<p>

```yaml
config:
  clientID: <my-client-id>
  clientSecret: <my-client-secret>
  configFile: |
    pass_access_token = true
    pass_authorization_header = true
    pass_basic_auth = false
    pass_user_headers = true
    set_authorization_header = true
  cookieSecret: <my-cookie-secret>
extraArgs:
  cookie-domain: .mydomain.com
  email-domain: '*'
  gitlab-group: mygitlabgroup
  oidc-issuer-url: https://gitlab.com
  provider: gitlab
  upstream: file:///dev/null
  whitelist-domain: .mydomain.com
image:
  pullPolicy: Always
  repository: sc250024/oauth2_proxy
  tag: b530f18
ingress:
  annotations:
    external-dns.alpha.kubernetes.io/hostname: oauth2.mydomain.com
    kubernetes.io/ingress.class: nginx-ingress
    nginx.ingress.kubernetes.io/ingress.class: nginx-ingress
  enabled: true
  hosts:
  - oauth2.mydomain.com
  tls:
  - hosts:
    - oauth2.mydomain.com
    secretName: star-mydomain-com-tls
replicaCount: 2
service:
  port: 8080
``` 

</p>
</details>

<details>
<summary>Helm command</summary>
<p>

```shell
$ helm-wrapper upgrade \
--force \
--install \
--values values.yaml \
--version 0.13.0 \
gitlab-oauth2-test \
stable/oauth2-proxy
``` 

</p>
</details>

## Successful deny for non-group member

<img width="1065" alt="successful-deny" src="https://user-images.githubusercontent.com/8792243/62401917-88afa100-b585-11e9-8c2a-7549b2197c9d.png">

sc250024: @JoelSpeed Possible to get a quick review on this? It looks all good. Thank you!
steakunderscore: @sc250024 @Overv Any ideas why the tests are failing? I just restarted the travis build and it failed with the same error
sc250024: Hmmm, I'm actually getting other errors locally for the Azure provider?

```shell
$ ./configure && make test
.env
Checking for make... found
Checking for awk... found
Checking for go... found
Checking go version... 1.12.7
Checking $GOPATH... valid - /Users/localuser/go
Checking for golangci-lint... found

Environment configuration written to .env
GO111MODULE=on "/Users/localuser/go/bin/golangci-lint" run
providers/azure_test.go:116:47: string `Bearer imaginary_access_token` has 3 occurrences, make it a constant (goconst)
			} else if r.Header.Get("Authorization") != "Bearer imaginary_access_token" {
			                                           ^
make: *** [lint] Error 1
```
steakunderscore: > Tests ran against Overv/oauth2_proxy <= His fork with new commits (FAIL)
Although the linter has a valid point, instead I would change the test by replacing `imaginary_access_token` with `a_gitlab_access_token` or some thing like that.
sc250024: @Overv Do you have time to update the tests?
Overv: @sc250024 Yeah, looking into it right now.
Overv: I don't think Mr Travis likes me very much.
sc250024: @Overv Maybe this? https://github.com/golangci/golangci-lint/issues/395
Overv: @sc250024 Nah, it looks like the linter just times out after the default runtime of 1 minute due to the long dependency installation. Someone will have to fix the CI configuration to either install the dependencies in advance (which is possible since the change to Go modules) or increase the linting deadline.
sc250024: @Overv Gotcha.

@steakunderscore I saw you spun up this PR => https://github.com/pusher/oauth2_proxy/pull/233. Can @Overv 's suggestion be included?
steakunderscore: Just updated my PR #233. Once that's merged I'll kick off a rebuild of this branch.
steakunderscore: Actually you'll need to rebase/merge master, which will cause this branch to rebuild
sc250024: Thanks guys for being quick this! Appreciate your work.
syscll: @Overv if you have time to fix the changelog conflicts I'm happy to merge.
Overv: @syscll I've rebased and fixed the conflict.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f979719b..8606b6b4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Breaking Changes
 
+- [#231](https://github.com/pusher/oauth2_proxy/pull/231) Rework GitLab provider (@Overv)
+  - This PR changes the configuration options for the GitLab provider to use
+  a self-hosted instance. You now need to specify a `-oidc-issuer-url` rather than
+  explicit `-login-url`, `-redeem-url` and `-validate-url` parameters.
+
 - [#186](https://github.com/pusher/oauth2_proxy/pull/186) Make config consistent
   - This PR changes configuration options so that all flags have a config counterpart
   of the same name but with underscores (`_`) in place of hyphens (`-`).
@@ -32,6 +37,7 @@
 ## Changes since v3.2.0
 
 - [#224](https://github.com/pusher/oauth2_proxy/pull/224) Check Google group membership using hasMember to support nested groups and external users (@jpalpant)
+- [#231](https://github.com/pusher/oauth2_proxy/pull/231) Add optional group membership and email domain checks to the GitLab provider (@Overv)
 - [#178](https://github.com/pusher/outh2_proxy/pull/178) Add Silence Ping Logging and Exclude Logging Paths flags (@kskewes)
 - [#209](https://github.com/pusher/outh2_proxy/pull/209) Improve docker build caching of layers (@dekimsey)
 - [#186](https://github.com/pusher/oauth2_proxy/pull/186) Make config consistent (@JoelSpeed)
diff --git a/docs/2_auth.md b/docs/2_auth.md
index 7a9bebd3..e1a5ecd5 100644
--- a/docs/2_auth.md
+++ b/docs/2_auth.md
@@ -103,13 +103,15 @@ If you are using GitHub enterprise, make sure you set the following to the appro
 
 ### GitLab Auth Provider
 
-Whether you are using GitLab.com or self-hosting GitLab, follow [these steps to add an application](http://doc.gitlab.com/ce/integration/oauth_provider.html)
+Whether you are using GitLab.com or self-hosting GitLab, follow [these steps to add an application](http://doc.gitlab.com/ce/integration/oauth_provider.html). Make sure to enable at least the `openid`, `profile` and `email` scopes.
+
+Restricting by group membership is possible with the following option:
+
+    -gitlab-group="": restrict logins to members of any of these groups (slug), separated by a comma
 
 If you are using self-hosted GitLab, make sure you set the following to the appropriate URL:
 
-    -login-url="<your gitlab url>/oauth/authorize"
-    -redeem-url="<your gitlab url>/oauth/token"
-    -validate-url="<your gitlab url>/api/v4/user"
+    -oidc-issuer-url="<your gitlab url>"
 
 ### LinkedIn Auth Provider
 
diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md
index f02d2ece..c9a70b14 100644
--- a/docs/configuration/configuration.md
+++ b/docs/configuration/configuration.md
@@ -49,6 +49,7 @@ Usage of oauth2_proxy:
   -gcp-healthchecks: will enable /liveness_check, /readiness_check, and / (with the proper user-agent) endpoints that will make it work well with GCP App Engine and GKE Ingresses (default false)
   -github-org string: restrict logins to members of this organisation
   -github-team string: restrict logins to members of any of these teams (slug), separated by a comma
+  -gitlab-group string: restrict logins to members of any of these groups (slug), separated by a comma
   -google-admin-email string: the google admin to impersonate for api calls
   -google-group value: restrict logins to members of this google group (may be given multiple times).
   -google-service-account-json string: the path to the service account json credentials
diff --git a/main.go b/main.go
index 97e2357d..6db6025b 100644
--- a/main.go
+++ b/main.go
@@ -57,6 +57,7 @@ func main() {
 	flagSet.String("azure-tenant", "common", "go to a tenant-specific or common (tenant-independent) endpoint.")
 	flagSet.String("github-org", "", "restrict logins to members of this organisation")
 	flagSet.String("github-team", "", "restrict logins to members of this team")
+	flagSet.String("gitlab-group", "", "restrict logins to members of this group")
 	flagSet.Var(&googleGroups, "google-group", "restrict logins to members of this google group (may be given multiple times).")
 	flagSet.String("google-admin-email", "", "the google admin to impersonate for api calls")
 	flagSet.String("google-service-account-json", "", "the path to the service account json credentials")
diff --git a/options.go b/options.go
index 7dcb12bb..6969faa1 100644
--- a/options.go
+++ b/options.go
@@ -46,6 +46,7 @@ type Options struct {
 	WhitelistDomains         []string `flag:"whitelist-domain" cfg:"whitelist_domains" env:"OAUTH2_PROXY_WHITELIST_DOMAINS"`
 	GitHubOrg                string   `flag:"github-org" cfg:"github_org" env:"OAUTH2_PROXY_GITHUB_ORG"`
 	GitHubTeam               string   `flag:"github-team" cfg:"github_team" env:"OAUTH2_PROXY_GITHUB_TEAM"`
+	GitLabGroup              string   `flag:"gitlab-group" cfg:"gitlab_group" env:"OAUTH2_PROXY_GITLAB_GROUP"`
 	GoogleGroups             []string `flag:"google-group" cfg:"google_group" env:"OAUTH2_PROXY_GOOGLE_GROUPS"`
 	GoogleAdminEmail         string   `flag:"google-admin-email" cfg:"google_admin_email" env:"OAUTH2_PROXY_GOOGLE_ADMIN_EMAIL"`
 	GoogleServiceAccountJSON string   `flag:"google-service-account-json" cfg:"google_service_account_json" env:"OAUTH2_PROXY_GOOGLE_SERVICE_ACCOUNT_JSON"`
@@ -410,6 +411,29 @@ func parseProviderInfo(o *Options, msgs []string) []string {
 		} else {
 			p.Verifier = o.oidcVerifier
 		}
+	case *providers.GitLabProvider:
+		p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail
+		p.Group = o.GitLabGroup
+		p.EmailDomains = o.EmailDomains
+
+		if o.oidcVerifier != nil {
+			p.Verifier = o.oidcVerifier
+		} else {
+			// Initialize with default verifier for gitlab.com
+			ctx := context.Background()
+
+			provider, err := oidc.NewProvider(ctx, "https://gitlab.com")
+			if err != nil {
+				msgs = append(msgs, "failed to initialize oidc provider for gitlab.com")
+			} else {
+				p.Verifier = provider.Verifier(&oidc.Config{
+					ClientID: o.ClientID,
+				})
+
+				p.LoginURL, msgs = parseURL(provider.Endpoint().AuthURL, "login", msgs)
+				p.RedeemURL, msgs = parseURL(provider.Endpoint().TokenURL, "redeem", msgs)
+			}
+		}
 	case *providers.LoginGovProvider:
 		p.AcrValues = o.AcrValues
 		p.PubJWKURL, msgs = parseURL(o.PubJWKURL, "pubjwk", msgs)
diff --git a/providers/gitlab.go b/providers/gitlab.go
index 663ebd45..c32ebe85 100644
--- a/providers/gitlab.go
+++ b/providers/gitlab.go
@@ -1,62 +1,258 @@
 package providers
 
 import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
 	"net/http"
-	"net/url"
+	"strings"
+	"time"
 
+	oidc "github.com/coreos/go-oidc"
 	"github.com/pusher/oauth2_proxy/pkg/apis/sessions"
-	"github.com/pusher/oauth2_proxy/pkg/logger"
-	"github.com/pusher/oauth2_proxy/pkg/requests"
+	"golang.org/x/oauth2"
 )
 
-// GitLabProvider represents an GitLab based Identity Provider
+// GitLabProvider represents a GitLab based Identity Provider
 type GitLabProvider struct {
 	*ProviderData
+
+	Group        string
+	EmailDomains []string
+
+	Verifier             *oidc.IDTokenVerifier
+	AllowUnverifiedEmail bool
 }
 
 // NewGitLabProvider initiates a new GitLabProvider
 func NewGitLabProvider(p *ProviderData) *GitLabProvider {
 	p.ProviderName = "GitLab"
-	if p.LoginURL == nil || p.LoginURL.String() == "" {
-		p.LoginURL = &url.URL{
-			Scheme: "https",
-			Host:   "gitlab.com",
-			Path:   "/oauth/authorize",
-		}
+
+	if p.Scope == "" {
+		p.Scope = "openid email"
+	}
+
+	return &GitLabProvider{ProviderData: p}
+}
+
+// Redeem exchanges the OAuth2 authentication token for an ID token
+func (p *GitLabProvider) Redeem(redirectURL, code string) (s *sessions.SessionState, err error) {
+	ctx := context.Background()
+	c := oauth2.Config{
+		ClientID:     p.ClientID,
+		ClientSecret: p.ClientSecret,
+		Endpoint: oauth2.Endpoint{
+			TokenURL: p.RedeemURL.String(),
+		},
+		RedirectURL: redirectURL,
+	}
+	token, err := c.Exchange(ctx, code)
+	if err != nil {
+		return nil, fmt.Errorf("token exchange: %v", err)
+	}
+	s, err = p.createSessionState(ctx, token)
+	if err != nil {
+		return nil, fmt.Errorf("unable to update session: %v", err)
+	}
+	return
+}
+
+// RefreshSessionIfNeeded checks if the session has expired and uses the
+// RefreshToken to fetch a new ID token if required
+func (p *GitLabProvider) RefreshSessionIfNeeded(s *sessions.SessionState) (bool, error) {
+	if s == nil || s.ExpiresOn.After(time.Now()) || s.RefreshToken == "" {
+		return false, nil
+	}
+
+	origExpiration := s.ExpiresOn
+
+	err := p.redeemRefreshToken(s)
+	if err != nil {
+		return false, fmt.Errorf("unable to redeem refresh token: %v", err)
+	}
+
+	fmt.Printf("refreshed id token %s (expired on %s)\n", s, origExpiration)
+	return true, nil
+}
+
+func (p *GitLabProvider) redeemRefreshToken(s *sessions.SessionState) (err error) {
+	c := oauth2.Config{
+		ClientID:     p.ClientID,
+		ClientSecret: p.ClientSecret,
+		Endpoint: oauth2.Endpoint{
+			TokenURL: p.RedeemURL.String(),
+		},
+	}
+	ctx := context.Background()
+	t := &oauth2.Token{
+		RefreshToken: s.RefreshToken,
+		Expiry:       time.Now().Add(-time.Hour),
+	}
+	token, err := c.TokenSource(ctx, t).Token()
+	if err != nil {
+		return fmt.Errorf("failed to get token: %v", err)
+	}
+	newSession, err := p.createSessionState(ctx, token)
+	if err != nil {
+		return fmt.Errorf("unable to update session: %v", err)
+	}
+	s.AccessToken = newSession.AccessToken
+	s.IDToken = newSession.IDToken
+	s.RefreshToken = newSession.RefreshToken
+	s.CreatedAt = newSession.CreatedAt
+	s.ExpiresOn = newSession.ExpiresOn
+	s.Email = newSession.Email
+	return
+}
+
+type gitlabUserInfo struct {
+	Username      string   `json:"nickname"`
+	Email         string   `json:"email"`
+	EmailVerified bool     `json:"email_verified"`
+	Groups        []string `json:"groups"`
+}
+
+func (p *GitLabProvider) getUserInfo(s *sessions.SessionState) (*gitlabUserInfo, error) {
+	// Retrieve user info JSON
+	// https://docs.gitlab.com/ee/integration/openid_connect_provider.html#shared-information
+
+	// Build user info url from login url of GitLab instance
+	userInfoURL := *p.LoginURL
+	userInfoURL.Path = "/oauth/userinfo"
+
+	req, err := http.NewRequest("GET", userInfoURL.String(), nil)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create user info request: %v", err)
+	}
+	req.Header.Set("Authorization", "Bearer "+s.AccessToken)
+
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("failed to perform user info request: %v", err)
+	}
+	var body []byte
+	body, err = ioutil.ReadAll(resp.Body)
+	resp.Body.Close()
+	if err != nil {
+		return nil, fmt.Errorf("failed to read user info response: %v", err)
 	}
-	if p.RedeemURL == nil || p.RedeemURL.String() == "" {
-		p.RedeemURL = &url.URL{
-			Scheme: "https",
-			Host:   "gitlab.com",
-			Path:   "/oauth/token",
+
+	if resp.StatusCode != 200 {
+		return nil, fmt.Errorf("got %d during user info request: %s", resp.StatusCode, body)
+	}
+
+	var userInfo gitlabUserInfo
+	err = json.Unmarshal(body, &userInfo)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse user info: %v", err)
+	}
+
+	return &userInfo, nil
+}
+
+func (p *GitLabProvider) verifyGroupMembership(userInfo *gitlabUserInfo) error {
+	if p.Group == "" {
+		return nil
+	}
+
+	// Collect user group memberships
+	membershipSet := make(map[string]bool)
+	for _, group := range userInfo.Groups {
+		membershipSet[group] = true
+	}
+
+	// Find a valid group that they are a member of
+	validGroups := strings.Split(p.Group, " ")
+	for _, validGroup := range validGroups {
+		if _, ok := membershipSet[validGroup]; ok {
+			return nil
 		}
 	}
-	if p.ValidateURL == nil || p.ValidateURL.String() == "" {
-		p.ValidateURL = &url.URL{
-			Scheme: "https",
-			Host:   "gitlab.com",
-			Path:   "/api/v4/user",
+
+	return fmt.Errorf("user is not a member of '%s'", p.Group)
+}
+
+func (p *GitLabProvider) verifyEmailDomain(userInfo *gitlabUserInfo) error {
+	if len(p.EmailDomains) == 0 || p.EmailDomains[0] == "*" {
+		return nil
+	}
+
+	for _, domain := range p.EmailDomains {
+		if strings.HasSuffix(userInfo.Email, domain) {
+			return nil
 		}
 	}
-	if p.Scope == "" {
-		p.Scope = "read_user"
+
+	return fmt.Errorf("user email is not one of the valid domains '%v'", p.EmailDomains)
+}
+
+func (p *GitLabProvider) createSessionState(ctx context.Context, token *oauth2.Token) (*sessions.SessionState, error) {
+	rawIDToken, ok := token.Extra("id_token").(string)
+	if !ok {
+		return nil, fmt.Errorf("token response did not contain an id_token")
 	}
-	return &GitLabProvider{ProviderData: p}
+
+	// Parse and verify ID Token payload.
+	idToken, err := p.Verifier.Verify(ctx, rawIDToken)
+	if err != nil {
+		return nil, fmt.Errorf("could not verify id_token: %v", err)
+	}
+
+	return &sessions.SessionState{
+		AccessToken:  token.AccessToken,
+		IDToken:      rawIDToken,
+		RefreshToken: token.RefreshToken,
+		CreatedAt:    time.Now(),
+		ExpiresOn:    idToken.Expiry,
+	}, nil
+}
+
+// ValidateSessionState checks that the session's IDToken is still valid
+func (p *GitLabProvider) ValidateSessionState(s *sessions.SessionState) bool {
+	ctx := context.Background()
+	_, err := p.Verifier.Verify(ctx, s.IDToken)
+	if err != nil {
+		return false
+	}
+
+	return true
 }
 
 // GetEmailAddress returns the Account email address
 func (p *GitLabProvider) GetEmailAddress(s *sessions.SessionState) (string, error) {
+	// Retrieve user info
+	userInfo, err := p.getUserInfo(s)
+	if err != nil {
+		return "", fmt.Errorf("failed to retrieve user info: %v", err)
+	}
+
+	// Check if email is verified
+	if !p.AllowUnverifiedEmail && !userInfo.EmailVerified {
+		return "", fmt.Errorf("user email is not verified")
+	}
+
+	// Check if email has valid domain
+	err = p.verifyEmailDomain(userInfo)
+	if err != nil {
+		return "", fmt.Errorf("email domain check failed: %v", err)
+	}
 
-	req, err := http.NewRequest("GET",
-		p.ValidateURL.String()+"?access_token="+s.AccessToken, nil)
+	// Check group membership
+	err = p.verifyGroupMembership(userInfo)
 	if err != nil {
-		logger.Printf("failed building request %s", err)
-		return "", err
+		return "", fmt.Errorf("group membership check failed: %v", err)
 	}
-	json, err := requests.Request(req)
+
+	return userInfo.Email, nil
+}
+
+// GetUserName returns the Account user name
+func (p *GitLabProvider) GetUserName(s *sessions.SessionState) (string, error) {
+	userInfo, err := p.getUserInfo(s)
 	if err != nil {
-		logger.Printf("failed making request %s", err)
-		return "", err
+		return "", fmt.Errorf("failed to retrieve user info: %v", err)
 	}
-	return json.Get("email").String()
+
+	return userInfo.Username, nil
 }


Test patch:
diff --git a/providers/gitlab_test.go b/providers/gitlab_test.go
index 112eb892..f75c4bfd 100644
--- a/providers/gitlab_test.go
+++ b/providers/gitlab_test.go
@@ -25,104 +25,142 @@ func testGitLabProvider(hostname string) *GitLabProvider {
 		updateURL(p.Data().ProfileURL, hostname)
 		updateURL(p.Data().ValidateURL, hostname)
 	}
+
 	return p
 }
 
-func testGitLabBackend(payload string) *httptest.Server {
-	path := "/api/v4/user"
-	query := "access_token=imaginary_access_token"
+func testGitLabBackend() *httptest.Server {
+	userInfo := `
+		{
+			"nickname": "FooBar",
+			"email": "foo@bar.com",
+			"email_verified": false,
+			"groups": ["foo", "bar"]
+		}
+	`
+	authHeader := "Bearer gitlab_access_token"
 
 	return httptest.NewServer(http.HandlerFunc(
 		func(w http.ResponseWriter, r *http.Request) {
-			if r.URL.Path != path || r.URL.RawQuery != query {
-				w.WriteHeader(404)
+			if r.URL.Path == "/oauth/userinfo" {
+				if r.Header["Authorization"][0] == authHeader {
+					w.WriteHeader(200)
+					w.Write([]byte(userInfo))
+				} else {
+					w.WriteHeader(401)
+				}
 			} else {
-				w.WriteHeader(200)
-				w.Write([]byte(payload))
+				w.WriteHeader(404)
 			}
 		}))
 }
 
-func TestGitLabProviderDefaults(t *testing.T) {
-	p := testGitLabProvider("")
-	assert.NotEqual(t, nil, p)
-	assert.Equal(t, "GitLab", p.Data().ProviderName)
-	assert.Equal(t, "https://gitlab.com/oauth/authorize",
-		p.Data().LoginURL.String())
-	assert.Equal(t, "https://gitlab.com/oauth/token",
-		p.Data().RedeemURL.String())
-	assert.Equal(t, "https://gitlab.com/api/v4/user",
-		p.Data().ValidateURL.String())
-	assert.Equal(t, "read_user", p.Data().Scope)
+func TestGitLabProviderBadToken(t *testing.T) {
+	b := testGitLabBackend()
+	defer b.Close()
+
+	bURL, _ := url.Parse(b.URL)
+	p := testGitLabProvider(bURL.Host)
+
+	session := &sessions.SessionState{AccessToken: "unexpected_gitlab_access_token"}
+	_, err := p.GetEmailAddress(session)
+	assert.NotEqual(t, nil, err)
 }
 
-func TestGitLabProviderOverrides(t *testing.T) {
-	p := NewGitLabProvider(
-		&ProviderData{
-			LoginURL: &url.URL{
-				Scheme: "https",
-				Host:   "example.com",
-				Path:   "/oauth/auth"},
-			RedeemURL: &url.URL{
-				Scheme: "https",
-				Host:   "example.com",
-				Path:   "/oauth/token"},
-			ValidateURL: &url.URL{
-				Scheme: "https",
-				Host:   "example.com",
-				Path:   "/api/v4/user"},
-			Scope: "profile"})
-	assert.NotEqual(t, nil, p)
-	assert.Equal(t, "GitLab", p.Data().ProviderName)
-	assert.Equal(t, "https://example.com/oauth/auth",
-		p.Data().LoginURL.String())
-	assert.Equal(t, "https://example.com/oauth/token",
-		p.Data().RedeemURL.String())
-	assert.Equal(t, "https://example.com/api/v4/user",
-		p.Data().ValidateURL.String())
-	assert.Equal(t, "profile", p.Data().Scope)
+func TestGitLabProviderUnverifiedEmailDenied(t *testing.T) {
+	b := testGitLabBackend()
+	defer b.Close()
+
+	bURL, _ := url.Parse(b.URL)
+	p := testGitLabProvider(bURL.Host)
+
+	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
+	_, err := p.GetEmailAddress(session)
+	assert.NotEqual(t, nil, err)
 }
 
-func TestGitLabProviderGetEmailAddress(t *testing.T) {
-	b := testGitLabBackend("{\"email\": \"michael.bland@gsa.gov\"}")
+func TestGitLabProviderUnverifiedEmailAllowed(t *testing.T) {
+	b := testGitLabBackend()
 	defer b.Close()
 
 	bURL, _ := url.Parse(b.URL)
 	p := testGitLabProvider(bURL.Host)
+	p.AllowUnverifiedEmail = true
 
-	session := &sessions.SessionState{AccessToken: "imaginary_access_token"}
+	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
 	email, err := p.GetEmailAddress(session)
 	assert.Equal(t, nil, err)
-	assert.Equal(t, "michael.bland@gsa.gov", email)
+	assert.Equal(t, "foo@bar.com", email)
 }
 
-// Note that trying to trigger the "failed building request" case is not
-// practical, since the only way it can fail is if the URL fails to parse.
-func TestGitLabProviderGetEmailAddressFailedRequest(t *testing.T) {
-	b := testGitLabBackend("unused payload")
+func TestGitLabProviderUsername(t *testing.T) {
+	b := testGitLabBackend()
 	defer b.Close()
 
 	bURL, _ := url.Parse(b.URL)
 	p := testGitLabProvider(bURL.Host)
+	p.AllowUnverifiedEmail = true
 
-	// We'll trigger a request failure by using an unexpected access
-	// token. Alternatively, we could allow the parsing of the payload as
-	// JSON to fail.
-	session := &sessions.SessionState{AccessToken: "unexpected_access_token"}
+	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
+	username, err := p.GetUserName(session)
+	assert.Equal(t, nil, err)
+	assert.Equal(t, "FooBar", username)
+}
+
+func TestGitLabProviderGroupMembershipValid(t *testing.T) {
+	b := testGitLabBackend()
+	defer b.Close()
+
+	bURL, _ := url.Parse(b.URL)
+	p := testGitLabProvider(bURL.Host)
+	p.AllowUnverifiedEmail = true
+	p.Group = "foo"
+
+	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
 	email, err := p.GetEmailAddress(session)
+	assert.Equal(t, nil, err)
+	assert.Equal(t, "foo@bar.com", email)
+}
+
+func TestGitLabProviderGroupMembershipMissing(t *testing.T) {
+	b := testGitLabBackend()
+	defer b.Close()
+
+	bURL, _ := url.Parse(b.URL)
+	p := testGitLabProvider(bURL.Host)
+	p.AllowUnverifiedEmail = true
+	p.Group = "baz"
+
+	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
+	_, err := p.GetEmailAddress(session)
 	assert.NotEqual(t, nil, err)
-	assert.Equal(t, "", email)
 }
 
-func TestGitLabProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {
-	b := testGitLabBackend("{\"foo\": \"bar\"}")
+func TestGitLabProviderEmailDomainValid(t *testing.T) {
+	b := testGitLabBackend()
 	defer b.Close()
 
 	bURL, _ := url.Parse(b.URL)
 	p := testGitLabProvider(bURL.Host)
+	p.AllowUnverifiedEmail = true
+	p.EmailDomains = []string{"bar.com"}
 
-	session := &sessions.SessionState{AccessToken: "imaginary_access_token"}
+	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
 	email, err := p.GetEmailAddress(session)
+	assert.Equal(t, nil, err)
+	assert.Equal(t, "foo@bar.com", email)
+}
+
+func TestGitLabProviderEmailDomainInvalid(t *testing.T) {
+	b := testGitLabBackend()
+	defer b.Close()
+
+	bURL, _ := url.Parse(b.URL)
+	p := testGitLabProvider(bURL.Host)
+	p.AllowUnverifiedEmail = true
+	p.EmailDomains = []string{"baz.com"}
+
+	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
+	_, err := p.GetEmailAddress(session)
 	assert.NotEqual(t, nil, err)
-	assert.Equal(t, "", email)
 }