From 7cf685140bd096cbb33e00365bdb964c21422edc Mon Sep 17 00:00:00 2001 From: John Clayton Date: Mon, 11 May 2020 11:02:40 -0600 Subject: [PATCH] Restrict access using Github collaborators (#497) * Allow access based on Github repository --- CHANGELOG.md | 1 + contrib/oauth2-proxy_autocomplete.sh | 2 +- docs/2_auth.md | 15 ++- docs/configuration/configuration.md | 2 + main.go | 2 + options.go | 3 + providers/github.go | 99 ++++++++++++++++- providers/github_test.go | 153 +++++++++++++++++++++++++-- 8 files changed, 263 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf78b3fd..3511bb61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ - [#483](https://github.com/oauth2-proxy/oauth2-proxy/pull/483) Warn users when session cookies are split (@JoelSpeed) - [#488](https://github.com/oauth2-proxy/oauth2-proxy/pull/488) Set-Basic-Auth should default to false (@JoelSpeed) - [#494](https://github.com/oauth2-proxy/oauth2-proxy/pull/494) Upstream websockets TLS certificate validation now depends on ssl-upstream-insecure-skip-verify (@yaroslavros) +- [#497](https://github.com/oauth2-proxy/oauth2-proxy/pull/497) Restrict access using Github collaborators (@jsclayton) # v5.1.1 diff --git a/contrib/oauth2-proxy_autocomplete.sh b/contrib/oauth2-proxy_autocomplete.sh index 422d2b98..bea798b9 100644 --- a/contrib/oauth2-proxy_autocomplete.sh +++ b/contrib/oauth2-proxy_autocomplete.sh @@ -20,7 +20,7 @@ _oauth2_proxy() { COMPREPLY=( $(compgen -W "google azure facebook github keycloak gitlab linkedin login.gov digitalocean" -- ${cur}) ) return 0 ;; - -@(http-address|https-address|redirect-url|upstream|basic-auth-password|skip-auth-regex|flush-interval|extra-jwt-issuers|email-domain|whitelist-domain|keycloak-group|azure-tenant|bitbucket-team|bitbucket-repository|github-org|github-team|gitlab-group|google-group|google-admin-email|google-service-account-json|client-id|client_secret|banner|footer|proxy-prefix|ping-path|cookie-name|cookie-secret|cookie-domain|cookie-path|cookie-expire|cookie-refresh|cookie-samesite|redist-sentinel-master-name|redist-sentinel-connection-urls|redist-cluster-connection-urls|logging-max-size|logging-max-age|logging-max-backups|standard-logging-format|request-logging-format|exclude-logging-paths|auth-logging-format|oidc-issuer-url|oidc-jwks-url|login-url|redeem-url|profile-url|resource|validate-url|scope|approval-prompt|signature-key|acr-values|jwt-key|pubjwk-url)) + -@(http-address|https-address|redirect-url|upstream|basic-auth-password|skip-auth-regex|flush-interval|extra-jwt-issuers|email-domain|whitelist-domain|keycloak-group|azure-tenant|bitbucket-team|bitbucket-repository|github-org|github-team|github-repo|github-token|gitlab-group|google-group|google-admin-email|google-service-account-json|client-id|client_secret|banner|footer|proxy-prefix|ping-path|cookie-name|cookie-secret|cookie-domain|cookie-path|cookie-expire|cookie-refresh|cookie-samesite|redist-sentinel-master-name|redist-sentinel-connection-urls|redist-cluster-connection-urls|logging-max-size|logging-max-age|logging-max-backups|standard-logging-format|request-logging-format|exclude-logging-paths|auth-logging-format|oidc-issuer-url|oidc-jwks-url|login-url|redeem-url|profile-url|resource|validate-url|scope|approval-prompt|signature-key|acr-values|jwt-key|pubjwk-url)) return 0 ;; esac diff --git a/docs/2_auth.md b/docs/2_auth.md index db6f724c..6a595646 100644 --- a/docs/2_auth.md +++ b/docs/2_auth.md @@ -101,11 +101,24 @@ Note: When using the Azure Auth provider with nginx and the cookie session store 1. Create a new project: https://github.com/settings/developers 2. Under `Authorization callback URL` enter the correct url ie `https://internal.yourcompany.com/oauth2/callback` -The GitHub auth provider supports two additional parameters to restrict authentication to Organization or Team level access. Restricting by org and team is normally accompanied with `--email-domain=*` +The GitHub auth provider supports two additional ways to restrict authentication to either organization and optional team level access, or to collaborators of a repository. Restricting by these options is normally accompanied with `--email-domain=*` + +To restrict by organization only, include the following flag: -github-org="": restrict logins to members of this organisation + +To restrict within an organization to specific teams, include the following flag in addition to `-github-org`: + -github-team="": restrict logins to members of any of these teams (slug), separated by a comma +If you would rather restrict access to collaborators of a repository, those users must either have push access to a public repository or any access to a private repository: + + -github-repo="": restrict logins to collaborators of this repository formatted as orgname/repo + +If you'd like to allow access to users with **read only** access to a **public** repository you will need to provide a [token](https://github.com/settings/tokens) for a user that has write access to the repository. The token must be created with at least the `public_repo` scope: + + -github-token="": the token to use when verifying repository collaborators + If you are using GitHub enterprise, make sure you set the following to the appropriate url: -login-url="http(s):///login/oauth/authorize" diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 7dd1473f..90d26e6c 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -54,6 +54,8 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example | `--gcp-healthchecks` | bool | 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 | 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 | | +| `--github-repo` | string | restrict logins to collaborators of this repository formatted as `orgname/repo` | | +| `--github-token` | string | the token to use when verifying repository collaborators (must have push access to the repository) | | | `--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` | string | restrict logins to members of this google group (may be given multiple times). | | diff --git a/main.go b/main.go index 473c1e60..049b6ae8 100644 --- a/main.go +++ b/main.go @@ -58,6 +58,8 @@ func main() { flagSet.String("bitbucket-repository", "", "restrict logins to user with access to this repository") flagSet.String("github-org", "", "restrict logins to members of this organisation") flagSet.String("github-team", "", "restrict logins to members of this team") + flagSet.String("github-repo", "", "restrict logins to collaborators of this repository") + flagSet.String("github-token", "", "the token to use when verifying repository collaborators (must have push access to the repository)") flagSet.String("gitlab-group", "", "restrict logins to members of this group") flagSet.StringSlice("google-group", []string{}, "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") diff --git a/options.go b/options.go index b59c2b14..80b1ff57 100644 --- a/options.go +++ b/options.go @@ -54,6 +54,8 @@ 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"` + GitHubRepo string `flag:"github-repo" cfg:"github_repo" env:"OAUTH2_PROXY_GITHUB_REPO"` + GitHubToken string `flag:"github-token" cfg:"github_token" env:"OAUTH2_PROXY_GITHUB_TOKEN"` 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"` @@ -483,6 +485,7 @@ func parseProviderInfo(o *Options, msgs []string) []string { p.Configure(o.AzureTenant) case *providers.GitHubProvider: p.SetOrgTeam(o.GitHubOrg, o.GitHubTeam) + p.SetRepo(o.GitHubRepo, o.GitHubToken) case *providers.KeycloakProvider: p.SetGroup(o.KeycloakGroup) case *providers.GoogleProvider: diff --git a/providers/github.go b/providers/github.go index 153373cb..1dc1e5d3 100644 --- a/providers/github.go +++ b/providers/github.go @@ -19,8 +19,10 @@ import ( // GitHubProvider represents an GitHub based Identity Provider type GitHubProvider struct { *ProviderData - Org string - Team string + Org string + Team string + Repo string + Token string } var _ Provider = (*GitHubProvider)(nil) @@ -72,6 +74,12 @@ func (p *GitHubProvider) SetOrgTeam(org, team string) { } } +// SetRepo configures the target repository and optional token to use +func (p *GitHubProvider) SetRepo(repo, token string) { + p.Repo = repo + p.Token = token +} + func (p *GitHubProvider) hasOrg(ctx context.Context, accessToken string) (bool, error) { // https://developer.github.com/v3/orgs/#list-your-organizations @@ -263,6 +271,82 @@ func (p *GitHubProvider) hasOrgAndTeam(ctx context.Context, accessToken string) return false, nil } +func (p *GitHubProvider) hasRepo(ctx context.Context, accessToken string) (bool, error) { + // https://developer.github.com/v3/repos/#get-a-repository + + type permissions struct { + Pull bool `json:"pull"` + Push bool `json:"push"` + } + + type repository struct { + Permissions permissions `json:"permissions"` + Private bool `json:"private"` + } + + endpoint := &url.URL{ + Scheme: p.ValidateURL.Scheme, + Host: p.ValidateURL.Host, + Path: path.Join(p.ValidateURL.Path, "/repo/", p.Repo), + } + + req, _ := http.NewRequestWithContext(ctx, "GET", endpoint.String(), nil) + req.Header = getGitHubHeader(accessToken) + 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.String(), body) + } + + var repo repository + if err := json.Unmarshal(body, &repo); err != nil { + return false, err + } + + // Every user can implicitly pull from a public repo, so only grant access + // if they have push access or the repo is private and they can pull + return repo.Permissions.Push || (repo.Private && repo.Permissions.Pull), nil +} + +func (p *GitHubProvider) isCollaborator(ctx context.Context, username, accessToken string) (bool, error) { + //https://developer.github.com/v3/repos/collaborators/#check-if-a-user-is-a-collaborator + + endpoint := &url.URL{ + Scheme: p.ValidateURL.Scheme, + Host: p.ValidateURL.Host, + Path: path.Join(p.ValidateURL.Path, "/repos/", p.Repo, "/collaborators/", username), + } + req, _ := http.NewRequestWithContext(ctx, "GET", endpoint.String(), nil) + req.Header = getGitHubHeader(accessToken) + 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 != 204 { + return false, fmt.Errorf("got %d from %q %s", + resp.StatusCode, endpoint.String(), body) + } + + logger.Printf("got %d from %q %s", resp.StatusCode, endpoint.String(), body) + + return true, nil +} + // GetEmailAddress returns the Account email address func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) { @@ -283,6 +367,10 @@ func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.Sessio return "", err } } + } else if p.Repo != "" && p.Token == "" { // If we have a token we'll do the collaborator check in GetUserName + if ok, err := p.hasRepo(ctx, s.AccessToken); err != nil || !ok { + return "", err + } } endpoint := &url.URL{ @@ -367,6 +455,13 @@ func (p *GitHubProvider) GetUserName(ctx context.Context, s *sessions.SessionSta return "", fmt.Errorf("%s unmarshaling %s", err, body) } + // Now that we have the username we can check collaborator status + if p.Org == "" && p.Repo != "" && p.Token != "" { + if ok, err := p.isCollaborator(ctx, user.Login, p.Token); err != nil || !ok { + return "", err + } + } + return user.Login, nil } diff --git a/providers/github_test.go b/providers/github_test.go index 7a1c4723..ddf0ccc7 100644 --- a/providers/github_test.go +++ b/providers/github_test.go @@ -29,8 +29,10 @@ func testGitHubProvider(hostname string) *GitHubProvider { return p } -func testGitHubBackend(payload []string) *httptest.Server { +func testGitHubBackend(payloads map[string][]string) *httptest.Server { pathToQueryMap := map[string][]string{ + "/repo/oauth2-proxy/oauth2-proxy": {""}, + "/repos/oauth2-proxy/oauth2-proxy/collaborators/mbland": {""}, "/user": {""}, "/user/emails": {""}, "/user/orgs": {"page=1&per_page=100", "page=2&per_page=100", "page=3&per_page=100"}, @@ -47,10 +49,16 @@ func testGitHubBackend(payload []string) *httptest.Server { index = i } } + payload := []string{} + if ok && validQuery { + payload, ok = payloads[r.URL.Path] + } if !ok { w.WriteHeader(404) } else if !validQuery { w.WriteHeader(404) + } else if payload[index] == "" { + w.WriteHeader(204) } else { w.WriteHeader(200) w.Write([]byte(payload[index])) @@ -99,7 +107,9 @@ func TestGitHubProviderOverrides(t *testing.T) { } func TestGitHubProviderGetEmailAddress(t *testing.T) { - b := testGitHubBackend([]string{`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}) + b := testGitHubBackend(map[string][]string{ + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + }) defer b.Close() bURL, _ := url.Parse(b.URL) @@ -112,7 +122,9 @@ func TestGitHubProviderGetEmailAddress(t *testing.T) { } func TestGitHubProviderGetEmailAddressNotVerified(t *testing.T) { - b := testGitHubBackend([]string{`[ {"email": "michael.bland@gsa.gov", "verified": false, "primary": true} ]`}) + b := testGitHubBackend(map[string][]string{ + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + }) defer b.Close() bURL, _ := url.Parse(b.URL) @@ -125,10 +137,13 @@ func TestGitHubProviderGetEmailAddressNotVerified(t *testing.T) { } func TestGitHubProviderGetEmailAddressWithOrg(t *testing.T) { - b := testGitHubBackend([]string{ - `[ {"email": "michael.bland@gsa.gov", "primary": true, "verified": true, "login":"testorg"} ]`, - `[ {"email": "michael.bland1@gsa.gov", "primary": true, "verified": true, "login":"testorg1"} ]`, - `[ ]`, + b := testGitHubBackend(map[string][]string{ + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + "/user/orgs": { + `[ {"login":"testorg"} ]`, + `[ {"login":"testorg1"} ]`, + `[ ]`, + }, }) defer b.Close() @@ -142,10 +157,93 @@ func TestGitHubProviderGetEmailAddressWithOrg(t *testing.T) { assert.Equal(t, "michael.bland@gsa.gov", email) } +func TestGitHubProviderGetEmailAddressWithWriteAccessToPublicRepo(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": true}, "private": false}`}, + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetRepo("oauth2-proxy/oauth2-proxy", "") + + session := CreateAuthorizedSession() + email, err := p.GetEmailAddress(context.Background(), session) + assert.Equal(t, nil, err) + assert.Equal(t, "michael.bland@gsa.gov", email) +} + +func TestGitHubProviderGetEmailAddressWithReadOnlyAccessToPrivateRepo(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": false}, "private": true}`}, + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetRepo("oauth2-proxy/oauth2-proxy", "") + + session := CreateAuthorizedSession() + email, err := p.GetEmailAddress(context.Background(), session) + assert.Equal(t, nil, err) + assert.Equal(t, "michael.bland@gsa.gov", email) +} + +func TestGitHubProviderGetEmailAddressWithWriteAccessToPrivateRepo(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": true}, "private": true}`}, + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetRepo("oauth2-proxy/oauth2-proxy", "") + + session := CreateAuthorizedSession() + email, err := p.GetEmailAddress(context.Background(), session) + assert.Equal(t, nil, err) + assert.Equal(t, "michael.bland@gsa.gov", email) +} + +func TestGitHubProviderGetEmailAddressWithNoAccessToPrivateRepo(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/repo/oauth2-proxy/oauth2-proxy": {}, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetRepo("oauth2-proxy/oauth2-proxy", "") + + session := CreateAuthorizedSession() + email, err := p.GetEmailAddress(context.Background(), session) + assert.NotEqual(t, nil, err) + assert.Equal(t, "", email) +} + +func TestGitHubProviderGetEmailAddressWithToken(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetRepo("oauth2-proxy/oauth2-proxy", "token") + + session := CreateAuthorizedSession() + email, err := p.GetEmailAddress(context.Background(), session) + assert.Equal(t, nil, err) + assert.Equal(t, "michael.bland@gsa.gov", 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 TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) { - b := testGitHubBackend([]string{"unused payload"}) + b := testGitHubBackend(map[string][]string{}) defer b.Close() bURL, _ := url.Parse(b.URL) @@ -161,7 +259,9 @@ func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) { } func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) { - b := testGitHubBackend([]string{"{\"foo\": \"bar\"}"}) + b := testGitHubBackend(map[string][]string{ + "/user/emails": {`{"foo": "bar"}`}, + }) defer b.Close() bURL, _ := url.Parse(b.URL) @@ -174,7 +274,9 @@ func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) { } func TestGitHubProviderGetUserName(t *testing.T) { - b := testGitHubBackend([]string{`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}) + b := testGitHubBackend(map[string][]string{ + "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, + }) defer b.Close() bURL, _ := url.Parse(b.URL) @@ -185,3 +287,34 @@ func TestGitHubProviderGetUserName(t *testing.T) { assert.Equal(t, nil, err) assert.Equal(t, "mbland", email) } + +func TestGitHubProviderGetUserNameWithRepoAndToken(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, + "/repos/oauth2-proxy/oauth2-proxy/collaborators/mbland": {""}, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetRepo("oauth2-proxy/oauth2-proxy", "token") + + session := CreateAuthorizedSession() + email, err := p.GetUserName(context.Background(), session) + assert.Equal(t, nil, err) + assert.Equal(t, "mbland", email) +} + +func TestGitHubProviderGetUserNameWithRepoAndTokenWithoutPushAccess(t *testing.T) { + b := testGitHubBackend(map[string][]string{}) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetRepo("oauth2-proxy/oauth2-proxy", "token") + + session := CreateAuthorizedSession() + email, err := p.GetUserName(context.Background(), session) + assert.NotEqual(t, nil, err) + assert.Equal(t, "", email) +}