From 4de49983fb682e15460d9cd1670e37a15f7782ab Mon Sep 17 00:00:00 2001 From: Alexander Overvoorde Date: Tue, 6 Aug 2019 13:20:54 +0200 Subject: [PATCH] Rework GitLab provider (#231) * Initial version of OIDC based GitLab provider * Add support for email domain check to GitLab provider * Add gitlab.com as default issuer for GitLab provider * Update documentation for GitLab provider * Update unit tests for new GitLab provider implementation * Update CHANGELOG for GitLab provider * Rename GitLab test access token as response to linter --- CHANGELOG.md | 6 + docs/2_auth.md | 10 +- docs/configuration/configuration.md | 1 + main.go | 1 + options.go | 24 +++ providers/gitlab.go | 266 ++++++++++++++++++++++++---- providers/gitlab_test.go | 172 +++++++++++------- 7 files changed, 374 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f979719..8606b6b 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 7a9bebd..e1a5ecd 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="/oauth/authorize" - -redeem-url="/oauth/token" - -validate-url="/api/v4/user" + -oidc-issuer-url="" ### LinkedIn Auth Provider diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index f02d2ec..c9a70b1 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 97e2357..6db6025 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 7dcb12b..6969faa 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 663ebd4..c32ebe8 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.RedeemURL == nil || p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{ - Scheme: "https", - Host: "gitlab.com", - Path: "/oauth/token", - } - } - if p.ValidateURL == nil || p.ValidateURL.String() == "" { - p.ValidateURL = &url.URL{ - Scheme: "https", - Host: "gitlab.com", - Path: "/api/v4/user", - } - } + if p.Scope == "" { - p.Scope = "read_user" + 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 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 + } + } + + 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 + } + } + + 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") + } + + // 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) + } - req, err := http.NewRequest("GET", - p.ValidateURL.String()+"?access_token="+s.AccessToken, nil) - if err != nil { - logger.Printf("failed building request %s", err) - return "", err + // Check if email is verified + if !p.AllowUnverifiedEmail && !userInfo.EmailVerified { + return "", fmt.Errorf("user email is not verified") } - json, err := requests.Request(req) + + // Check if email has valid domain + err = p.verifyEmailDomain(userInfo) if err != nil { - logger.Printf("failed making request %s", err) - return "", err + return "", fmt.Errorf("email domain check failed: %v", err) } - return json.Get("email").String() + + // Check group membership + err = p.verifyGroupMembership(userInfo) + if err != nil { + return "", fmt.Errorf("group membership check failed: %v", err) + } + + 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 { + return "", fmt.Errorf("failed to retrieve user info: %v", err) + } + + return userInfo.Username, nil } diff --git a/providers/gitlab_test.go b/providers/gitlab_test.go index 112eb89..f75c4bf 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 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 TestGitLabProviderGetEmailAddress(t *testing.T) { - b := testGitLabBackend("{\"email\": \"michael.bland@gsa.gov\"}") +func TestGitLabProviderBadToken(t *testing.T) { + b := testGitLabBackend() defer b.Close() bURL, _ := url.Parse(b.URL) p := testGitLabProvider(bURL.Host) - session := &sessions.SessionState{AccessToken: "imaginary_access_token"} + session := &sessions.SessionState{AccessToken: "unexpected_gitlab_access_token"} + _, err := p.GetEmailAddress(session) + assert.NotEqual(t, nil, err) +} + +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 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: "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"} - email, err := p.GetEmailAddress(session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) + session := &sessions.SessionState{AccessToken: "gitlab_access_token"} + username, err := p.GetUserName(session) + assert.Equal(t, nil, err) + assert.Equal(t, "FooBar", username) } -func TestGitLabProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) { - b := testGitLabBackend("{\"foo\": \"bar\"}") +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: "imaginary_access_token"} + session := &sessions.SessionState{AccessToken: "gitlab_access_token"} email, err := p.GetEmailAddress(session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) + 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) +} + +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: "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) }