From d8d43bb51b7cc5c29966f43324a10fe286274629 Mon Sep 17 00:00:00 2001 From: Yoshiki Nakagawa Date: Tue, 2 Jun 2020 04:02:07 +0900 Subject: [PATCH] Support new option "github-user" (#421) * feat(github): support new option "github-user" * feat(github): rename github-user to github-users * feat(github): update docs for github-users option * feat(github): remove unneeded code * feat(github): remove logging * feat(github-user): use github-user as flagset options * feat(github-user): remove optionns.go * feat(github-user): add github-user flagset * feat(github): improve readability in the docs * feat(github-user): refactored SetUsers method * Update flag description Co-authored-by: Joel Speed --- CHANGELOG.md | 1 + contrib/oauth2-proxy_autocomplete.sh | 2 +- docs/2_auth.md | 6 ++ docs/configuration/configuration.md | 1 + pkg/apis/options/options.go | 2 + pkg/validation/options.go | 1 + providers/github.go | 100 +++++++++++++++++++++++---- providers/github_test.go | 75 ++++++++++++++++++++ 8 files changed, 173 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd63a9f7..6f856ac5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ - [#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) - [#414](https://github.com/oauth2-proxy/oauth2-proxy/pull/414) Always encrypt sessions regardless of config (@ti-mo) +- [#421](https://github.com/oauth2-proxy/oauth2-proxy/pull/421) Allow logins by usernames even if they do not belong to the specified org and team or collaborators (@yyoshiki41) # v5.1.1 diff --git a/contrib/oauth2-proxy_autocomplete.sh b/contrib/oauth2-proxy_autocomplete.sh index e1a51d8e..f7d6ec68 100644 --- a/contrib/oauth2-proxy_autocomplete.sh +++ b/contrib/oauth2-proxy_autocomplete.sh @@ -24,7 +24,7 @@ _oauth2_proxy() { COMPREPLY=( $(compgen -W 'X-Real-IP X-Forwarded-For X-ProxyUser-IP' -- ${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|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)) + --@(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|github-user|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 851bde5e..46d33c3a 100644 --- a/docs/2_auth.md +++ b/docs/2_auth.md @@ -103,6 +103,8 @@ Note: When using the Azure Auth provider with nginx and the cookie session store 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=*` +NOTE: When `--github-user` is set, the specified users are allowed to login even if they do not belong to the specified org and team or collaborators. + To restrict by organization only, include the following flag: -github-org="": restrict logins to members of this organisation @@ -119,6 +121,10 @@ If you'd like to allow access to users with **read only** access to a **public** -github-token="": the token to use when verifying repository collaborators +To allow a user to login with their username even if they do not belong to the specified org and team or collaborators, separated by a comma + + -github-user="": allow logins by username, separated by a comma + 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 69b3cfa0..1896e0e8 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -56,6 +56,7 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example | `--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) | | +| `--github-user` | string \| list | To allow users to login by username even if they do not belong to the specified org and team or collaborators | | | `--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/pkg/apis/options/options.go b/pkg/apis/options/options.go index abd96495..9edd1489 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -48,6 +48,7 @@ type Options struct { GitHubTeam string `flag:"github-team" cfg:"github_team"` GitHubRepo string `flag:"github-repo" cfg:"github_repo"` GitHubToken string `flag:"github-token" cfg:"github_token"` + GitHubUsers []string `flag:"github-user" cfg:"github_users"` GitLabGroup string `flag:"gitlab-group" cfg:"gitlab_group"` GoogleGroups []string `flag:"google-group" cfg:"google_group"` GoogleAdminEmail string `flag:"google-admin-email" cfg:"google_admin_email"` @@ -228,6 +229,7 @@ func NewFlagSet() *pflag.FlagSet { 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.StringSlice("github-user", []string{}, "allow users with these usernames to login even if they do not belong to the specified org and team or collaborators (may be given multiple times)") 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/pkg/validation/options.go b/pkg/validation/options.go index 5a99e4ff..2e028677 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -309,6 +309,7 @@ func parseProviderInfo(o *options.Options, msgs []string) []string { case *providers.GitHubProvider: p.SetOrgTeam(o.GitHubOrg, o.GitHubTeam) p.SetRepo(o.GitHubRepo, o.GitHubToken) + p.SetUsers(o.GitHubUsers) case *providers.KeycloakProvider: p.SetGroup(o.KeycloakGroup) case *providers.GoogleProvider: diff --git a/providers/github.go b/providers/github.go index 1dc1e5d3..6d3f8b02 100644 --- a/providers/github.go +++ b/providers/github.go @@ -3,6 +3,7 @@ package providers import ( "context" "encoding/json" + "errors" "fmt" "io/ioutil" "net/http" @@ -23,6 +24,7 @@ type GitHubProvider struct { Team string Repo string Token string + Users []string } var _ Provider = (*GitHubProvider)(nil) @@ -80,6 +82,11 @@ func (p *GitHubProvider) SetRepo(repo, token string) { p.Token = token } +// SetUsers configures allowed usernames +func (p *GitHubProvider) SetUsers(users []string) { + p.Users = users +} + func (p *GitHubProvider) hasOrg(ctx context.Context, accessToken string) (bool, error) { // https://developer.github.com/v3/orgs/#list-your-organizations @@ -317,6 +324,46 @@ func (p *GitHubProvider) hasRepo(ctx context.Context, accessToken string) (bool, return repo.Permissions.Push || (repo.Private && repo.Permissions.Pull), nil } +func (p *GitHubProvider) hasUser(ctx context.Context, accessToken string) (bool, error) { + // https://developer.github.com/v3/users/#get-the-authenticated-user + + var user struct { + Login string `json:"login"` + Email string `json:"email"` + } + + endpoint := &url.URL{ + Scheme: p.ValidateURL.Scheme, + Host: p.ValidateURL.Host, + Path: path.Join(p.ValidateURL.Path, "/user"), + } + req, _ := http.NewRequestWithContext(ctx, "GET", endpoint.String(), nil) + req.Header = getGitHubHeader(accessToken) + resp, err := http.DefaultClient.Do(req) + if err != nil { + return false, err + } + defer resp.Body.Close() + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return false, err + } + if resp.StatusCode != 200 { + return false, fmt.Errorf("got %d from %q %s", + resp.StatusCode, stripToken(endpoint.String()), body) + } + + if err := json.Unmarshal(body, &user); err != nil { + return false, err + } + + if p.isVerifiedUser(user.Login) { + return true, nil + } + return false, 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 @@ -356,21 +403,36 @@ func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.Sessio Verified bool `json:"verified"` } - // if we require an Org or Team, check that first - if p.Org != "" { - if p.Team != "" { - if ok, err := p.hasOrgAndTeam(ctx, s.AccessToken); err != nil || !ok { - return "", err - } - } else { - if ok, err := p.hasOrg(ctx, s.AccessToken); err != nil || !ok { - 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 { + // If usernames are set, check that first + verifiedUser := false + if len(p.Users) > 0 { + var err error + verifiedUser, err = p.hasUser(ctx, s.AccessToken) + if err != nil { return "", err } + // org and repository options are not configured + if !verifiedUser && p.Org == "" && p.Repo == "" { + return "", errors.New("missing github user") + } + } + // If a user is verified by username options, skip the following restrictions + if !verifiedUser { + if p.Org != "" { + if p.Team != "" { + if ok, err := p.hasOrgAndTeam(ctx, s.AccessToken); err != nil || !ok { + return "", err + } + } else { + if ok, err := p.hasOrg(ctx, s.AccessToken); err != nil || !ok { + 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{ @@ -456,7 +518,7 @@ func (p *GitHubProvider) GetUserName(ctx context.Context, s *sessions.SessionSta } // Now that we have the username we can check collaborator status - if p.Org == "" && p.Repo != "" && p.Token != "" { + if !p.isVerifiedUser(user.Login) && p.Org == "" && p.Repo != "" && p.Token != "" { if ok, err := p.isCollaborator(ctx, user.Login, p.Token); err != nil || !ok { return "", err } @@ -469,3 +531,13 @@ func (p *GitHubProvider) GetUserName(ctx context.Context, s *sessions.SessionSta func (p *GitHubProvider) ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool { return validateToken(ctx, p, s.AccessToken, getGitHubHeader(s.AccessToken)) } + +// isVerifiedUser +func (p *GitHubProvider) isVerifiedUser(username string) bool { + for _, u := range p.Users { + if username == u { + return true + } + } + return false +} diff --git a/providers/github_test.go b/providers/github_test.go index ddf0ccc7..46e6f1fc 100644 --- a/providers/github_test.go +++ b/providers/github_test.go @@ -318,3 +318,78 @@ func TestGitHubProviderGetUserNameWithRepoAndTokenWithoutPushAccess(t *testing.T assert.NotEqual(t, nil, err) assert.Equal(t, "", email) } + +func TestGitHubProviderGetEmailAddressWithUsername(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetUsers([]string{"mbland", "octocat"}) + + session := CreateAuthorizedSession() + email, err := p.GetEmailAddress(context.Background(), session) + assert.Equal(t, nil, err) + assert.Equal(t, "michael.bland@gsa.gov", email) +} + +func TestGitHubProviderGetEmailAddressWithNotAllowedUsername(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetUsers([]string{"octocat"}) + + session := CreateAuthorizedSession() + email, err := p.GetEmailAddress(context.Background(), session) + assert.NotEqual(t, nil, err) + assert.Equal(t, "", email) +} + +func TestGitHubProviderGetEmailAddressWithUsernameAndNotBelongToOrg(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + "/user/orgs": { + `[ {"login":"testorg"} ]`, + `[ ]`, + }, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetOrgTeam("not_belong_to", "") + p.SetUsers([]string{"mbland"}) + + session := CreateAuthorizedSession() + email, err := p.GetEmailAddress(context.Background(), session) + assert.Equal(t, nil, err) + assert.Equal(t, "michael.bland@gsa.gov", email) +} + +func TestGitHubProviderGetEmailAddressWithUsernameAndNoAccessToPrivateRepo(t *testing.T) { + b := testGitHubBackend(map[string][]string{ + "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + "/repo/oauth2-proxy/oauth2-proxy": {}, + }) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + p.SetRepo("oauth2-proxy/oauth2-proxy", "") + p.SetUsers([]string{"mbland"}) + + session := CreateAuthorizedSession() + email, err := p.GetEmailAddress(context.Background(), session) + assert.Equal(t, nil, err) + assert.Equal(t, "michael.bland@gsa.gov", email) +}