Skip to content
This repository has been archived by the owner on Jan 24, 2019. It is now read-only.

Support for group restriction for Gitlab provider #312

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ If you are using GitHub enterprise, make sure you set the following to the appro

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)

The GitLab auth provider supports one additional parameters to restrict authentication to a specific group. Restricting by group is normally accompanied with `--email-domain=*`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: "one additional parameters" or "one additional parameters"


-gitlab-group="": restrict logins to members of this group

If you are using self-hosted GitLab, make sure you set the following to the appropriate URL:

-login-url="<your gitlab url>/oauth/authorize"
Expand Down
1 change: 1 addition & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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")
Expand Down
3 changes: 3 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type Options struct {
EmailDomains []string `flag:"email-domain" cfg:"email_domains"`
GitHubOrg string `flag:"github-org" cfg:"github_org"`
GitHubTeam string `flag:"github-team" cfg:"github_team"`
GitLabGroup string `flag:"gitlab-group" cfg:"gitlab_group"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if multiple groups could be specified, just like multiple GoogleGroups can be specified.

GoogleGroups []string `flag:"google-group" cfg:"google_group"`
GoogleAdminEmail string `flag:"google-admin-email" cfg:"google_admin_email"`
GoogleServiceAccountJSON string `flag:"google-service-account-json" cfg:"google_service_account_json"`
Expand Down Expand Up @@ -229,6 +230,8 @@ func parseProviderInfo(o *Options, msgs []string) []string {
p.Configure(o.AzureTenant)
case *providers.GitHubProvider:
p.SetOrgTeam(o.GitHubOrg, o.GitHubTeam)
case *providers.GitLabProvider:
p.SetGroup(o.GitLabGroup)
case *providers.GoogleProvider:
if o.GoogleServiceAccountJSON != "" {
file, err := os.Open(o.GoogleServiceAccountJSON)
Expand Down
53 changes: 53 additions & 0 deletions providers/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ import (
"net/url"

"github.com/bitly/oauth2_proxy/api"
"fmt"
"io/ioutil"
"encoding/json"
)

type GitLabProvider struct {
*ProviderData
Group string
}

func NewGitLabProvider(p *ProviderData) *GitLabProvider {
Expand Down Expand Up @@ -41,8 +45,57 @@ func NewGitLabProvider(p *ProviderData) *GitLabProvider {
return &GitLabProvider{ProviderData: p}
}

func (p *GitLabProvider) SetGroup(group string) {
p.Group = group
}

func (p *GitLabProvider) hasGroup(accessToken string) (bool, error) {

var groups []struct {
Group string `json:"name"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a serious security issue since the name of a group is only unique within it's namespace. Anyone who can create groups (usually anyone) could dupe this by creating a group with an arbitrary name and then a subgroup with the desired name.

Consider this scenario:

[
    {
        "id": 1,
        "web_url": "https://gitlab.com/groups/admins",
        "name": "admins",
        "path": "admins",
        "description": "",
        "visibility": "private",
        "lfs_enabled": true,
        "avatar_url": null,
        "request_access_enabled": false,
        "full_name": "admins",
        "full_path": "admins",
        "parent_id": null
    },
    {
        "id": 2,
        "web_url": "https://gitlab.com/groups/myfunnyproject",
        "name": "myfunnyproject",
        "path": "myfunnyproject",
        "description": "",
        "visibility": "private",
        "lfs_enabled": true,
        "avatar_url": null,
        "request_access_enabled": false,
        "full_name": "myfunnyproject",
        "full_path": "myfunnyproject",
        "parent_id": null
    },
    {
        "id": 3,
        "web_url": "https://gitlab.com/groups/myfunnyproject/admins",
        "name": "admins",
        "path": "admins",
        "description": "",
        "visibility": "private",
        "lfs_enabled": true,
        "avatar_url": null,
        "request_access_enabled": false,
        "full_name": "myfunnyproject / myfunnyproject",
        "full_path": "myfunnyproject/myfunnyproject",
        "parent_id": 2
    }
]

There are three groups: admins, myfunnyproject and a subgroup of myfunnyproject also named admins. In the current state this would allow both members of admins and members of myfunnyproject/admins to access the site.

I suggest using full_path instead of name.

}

endpoint := p.ValidateURL.Scheme + "://" + p.ValidateURL.Host + "/api/v3/groups?access_token="+accessToken
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: spaces around +: "+accessToken

req, _ := http.NewRequest("GET", endpoint, nil)
resp, err := http.DefaultClient.Do(req)
if err != nil {
return false, err
}

body, err := ioutil.ReadAll(resp.Body)
resp.Body.Close()
if err != nil {
return false, err
}
if resp.StatusCode != 200 {
return false, fmt.Errorf("got %d from %q %s", resp.StatusCode, endpoint, body)
}

if err := json.Unmarshal(body, &groups); err != nil {
return false, err
}

for _, group := range groups {
if( p.Group == group.Group) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: no round brackets required here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably best to just ask to run "gofmt"

// Found the group
return true, nil
}
}

log.Printf("Group %s not found in %s", p.Group, groups)
return false, nil
}


func (p *GitLabProvider) GetEmailAddress(s *SessionState) (string, error) {

// if we require a group, check that first
if p.Group != "" {
if ok, err := p.hasGroup(s.AccessToken); err != nil || !ok {
return "", err
}
}

req, err := http.NewRequest("GET",
p.ValidateURL.String()+"?access_token="+s.AccessToken, nil)
if err != nil {
Expand Down