From be40fa8f20890b6d0422abe369539b0181a3c2b3 Mon Sep 17 00:00:00 2001 From: kylecarbs Date: Fri, 8 Jul 2022 02:25:19 +0000 Subject: [PATCH] feat: Add allowlist of GitHub teams for OAuth Fixes #2848. --- cli/cliflag/cliflag.go | 2 +- cli/server.go | 17 ++++++++++-- coderd/userauth.go | 46 +++++++++++++++++++++++++++++++ coderd/userauth_test.go | 61 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 3 deletions(-) diff --git a/cli/cliflag/cliflag.go b/cli/cliflag/cliflag.go index 358696637cfd8..107b4515b55f9 100644 --- a/cli/cliflag/cliflag.go +++ b/cli/cliflag/cliflag.go @@ -47,7 +47,7 @@ func StringArrayVarP(flagset *pflag.FlagSet, ptr *[]string, name string, shortha def = strings.Split(val, ",") } } - flagset.StringArrayVarP(ptr, name, shorthand, def, usage) + flagset.StringArrayVarP(ptr, name, shorthand, def, fmtUsage(usage, env)) } // Uint8VarP sets a uint8 flag on the given flag set. diff --git a/cli/server.go b/cli/server.go index 7dc27a340c5cf..29fedbae3469d 100644 --- a/cli/server.go +++ b/cli/server.go @@ -82,6 +82,7 @@ func server() *cobra.Command { oauth2GithubClientID string oauth2GithubClientSecret string oauth2GithubAllowedOrganizations []string + oauth2GithubAllowedTeams []string oauth2GithubAllowSignups bool telemetryEnable bool telemetryURL string @@ -264,7 +265,7 @@ func server() *cobra.Command { } if oauth2GithubClientSecret != "" { - options.GithubOAuth2Config, err = configureGithubOAuth2(accessURLParsed, oauth2GithubClientID, oauth2GithubClientSecret, oauth2GithubAllowSignups, oauth2GithubAllowedOrganizations) + options.GithubOAuth2Config, err = configureGithubOAuth2(accessURLParsed, oauth2GithubClientID, oauth2GithubClientSecret, oauth2GithubAllowSignups, oauth2GithubAllowedOrganizations, oauth2GithubAllowedTeams) if err != nil { return xerrors.Errorf("configure github oauth2: %w", err) } @@ -535,6 +536,8 @@ func server() *cobra.Command { "Specifies a client secret to use for oauth2 with GitHub.") cliflag.StringArrayVarP(root.Flags(), &oauth2GithubAllowedOrganizations, "oauth2-github-allowed-orgs", "", "CODER_OAUTH2_GITHUB_ALLOWED_ORGS", nil, "Specifies organizations the user must be a member of to authenticate with GitHub.") + cliflag.StringArrayVarP(root.Flags(), &oauth2GithubAllowedTeams, "oauth2-github-allowed-teams", "", "CODER_OAUTH2_GITHUB_ALLOWED_TEAMS", nil, + "Specifies teams inside organizations the user must be a member of to authenticate with GitHub. Formatted as: /.") cliflag.BoolVarP(root.Flags(), &oauth2GithubAllowSignups, "oauth2-github-allow-signups", "", "CODER_OAUTH2_GITHUB_ALLOW_SIGNUPS", false, "Specifies whether new users can sign up with GitHub.") cliflag.BoolVarP(root.Flags(), &telemetryEnable, "telemetry", "", "CODER_TELEMETRY", true, "Specifies whether telemetry is enabled or not. Coder collects anonymized usage data to help improve our product.") @@ -719,7 +722,7 @@ func configureTLS(listener net.Listener, tlsMinVersion, tlsClientAuth, tlsCertFi return tls.NewListener(listener, tlsConfig), nil } -func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, allowSignups bool, allowOrgs []string) (*coderd.GithubOAuth2Config, error) { +func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, allowSignups bool, allowOrgs []string, allowTeams []string) (*coderd.GithubOAuth2Config, error) { redirectURL, err := accessURL.Parse("/api/v2/users/oauth2/github/callback") if err != nil { return nil, xerrors.Errorf("parse github oauth callback url: %w", err) @@ -738,6 +741,7 @@ func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, al }, AllowSignups: allowSignups, AllowOrganizations: allowOrgs, + AllowTeams: allowTeams, AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) { user, _, err := github.NewClient(client).Users.Get(ctx, "") return user, err @@ -749,9 +753,18 @@ func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, al ListOrganizationMemberships: func(ctx context.Context, client *http.Client) ([]*github.Membership, error) { memberships, _, err := github.NewClient(client).Organizations.ListOrgMemberships(ctx, &github.ListOrgMembershipsOptions{ State: "active", + ListOptions: github.ListOptions{ + PerPage: 100, + }, }) return memberships, err }, + ListTeams: func(ctx context.Context, client *http.Client, org string) ([]*github.Team, error) { + teams, _, err := github.NewClient(client).Teams.ListTeams(ctx, org, &github.ListOptions{ + PerPage: 100, + }) + return teams, err + }, }, nil } diff --git a/coderd/userauth.go b/coderd/userauth.go index 117071e511fbb..ecb314eba8604 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/google/go-github/v43/github" "github.com/google/uuid" @@ -23,9 +24,11 @@ type GithubOAuth2Config struct { AuthenticatedUser func(ctx context.Context, client *http.Client) (*github.User, error) ListEmails func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) ListOrganizationMemberships func(ctx context.Context, client *http.Client) ([]*github.Membership, error) + ListTeams func(ctx context.Context, client *http.Client, org string) ([]*github.Team, error) AllowSignups bool AllowOrganizations []string + AllowTeams []string } func (api *API) userAuthMethods(rw http.ResponseWriter, _ *http.Request) { @@ -64,6 +67,49 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { return } + // The default if no teams are specified is to allow all. + if len(api.GithubOAuth2Config.AllowTeams) > 0 { + teams, err := api.GithubOAuth2Config.ListTeams(r.Context(), oauthClient, *selectedMembership.Organization.Login) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "Failed to fetch teams from GitHub.", + Detail: err.Error(), + }) + return + } + + var allowedTeam *github.Team + for _, team := range teams { + for _, organizationAndTeam := range api.GithubOAuth2Config.AllowTeams { + parts := strings.SplitN(organizationAndTeam, "/", 2) + if len(parts) != 2 { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "Team allowlist isn't formatted correctly.", + Detail: fmt.Sprintf("Got %s, wanted /", organizationAndTeam), + }) + return + } + if parts[0] != *selectedMembership.Organization.Login { + // This needs to continue because multiple organizations + // could exist in the allow/team listings. + continue + } + if parts[1] != *team.Slug { + continue + } + allowedTeam = team + break + } + } + + if allowedTeam == nil { + httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ + Message: fmt.Sprintf("You aren't a member of an authorized team in the %s Github organization!", *selectedMembership.Organization.Login), + }) + return + } + } + emails, err := api.GithubOAuth2Config.ListEmails(r.Context(), oauthClient) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index 19d8b0e4b759e..e92de271fc466 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -73,6 +73,30 @@ func TestUserOAuth2Github(t *testing.T) { resp := oauth2Callback(t, client) require.Equal(t, http.StatusUnauthorized, resp.StatusCode) }) + t.Run("NotInAllowedTeam", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + GithubOAuth2Config: &coderd.GithubOAuth2Config{ + AllowOrganizations: []string{"coder"}, + AllowTeams: []string{"another/something", "coder/frontend"}, + OAuth2Config: &oauth2Config{}, + ListOrganizationMemberships: func(ctx context.Context, client *http.Client) ([]*github.Membership, error) { + return []*github.Membership{{ + Organization: &github.Organization{ + Login: github.String("coder"), + }, + }}, nil + }, + ListTeams: func(ctx context.Context, client *http.Client, org string) ([]*github.Team, error) { + return []*github.Team{{ + Slug: github.String("nope"), + }}, nil + }, + }, + }) + resp := oauth2Callback(t, client) + require.Equal(t, http.StatusUnauthorized, resp.StatusCode) + }) t.Run("UnverifiedEmail", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{ @@ -184,6 +208,43 @@ func TestUserOAuth2Github(t *testing.T) { resp := oauth2Callback(t, client) require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) }) + t.Run("SignupAllowedTeam", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + GithubOAuth2Config: &coderd.GithubOAuth2Config{ + AllowSignups: true, + AllowOrganizations: []string{"coder"}, + AllowTeams: []string{"coder/frontend"}, + OAuth2Config: &oauth2Config{}, + ListOrganizationMemberships: func(ctx context.Context, client *http.Client) ([]*github.Membership, error) { + return []*github.Membership{{ + Organization: &github.Organization{ + Login: github.String("coder"), + }, + }}, nil + }, + ListTeams: func(ctx context.Context, client *http.Client, org string) ([]*github.Team, error) { + return []*github.Team{{ + Slug: github.String("frontend"), + }}, nil + }, + AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) { + return &github.User{ + Login: github.String("kyle"), + }, nil + }, + ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { + return []*github.UserEmail{{ + Email: github.String("kyle@coder.com"), + Verified: github.Bool(true), + Primary: github.Bool(true), + }}, nil + }, + }, + }) + resp := oauth2Callback(t, client) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + }) } func oauth2Callback(t *testing.T, client *codersdk.Client) *http.Response {