Merge branch 'main' into githubteams

kylecarbs · kylecarbs · commit ce18015b0b32 · 2022-07-09T00:47:48.000Z
diff --git a/Makefile b/Makefile
@@ -175,7 +175,7 @@ test-postgres: test-clean test-postgres-docker
 	DB=ci DB_FROM=$(shell go run scripts/migrate-ci/main.go) gotestsum --junitfile="gotests.xml" --packages="./..." -- \
 		-covermode=atomic -coverprofile="gotests.coverage" -timeout=30m \
 		-coverpkg=./...,github.com/coder/coder/codersdk \
-		-count=2 -race -failfast
+		-count=1 -race -failfast
 .PHONY: test-postgres
 
 test-postgres-docker:
diff --git a/cli/server.go b/cli/server.go
@@ -722,11 +722,22 @@ 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, allowTeams []string) (*coderd.GithubOAuth2Config, error) {
+func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, allowSignups bool, allowOrgs []string, rawTeams []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)
 	}
+	allowTeams := make([]coderd.GithubOAuth2Team, 0, len(rawTeams))
+	for _, rawTeam := range rawTeams {
+		parts := strings.SplitN(rawTeam, "/", 2)
+		if len(parts) != 2 {
+			return nil, xerrors.Errorf("github team allowlist is formatted incorrectly. got %s; wanted <organization>/<team>", rawTeam)
+		}
+		allowTeams = append(allowTeams, coderd.GithubOAuth2Team{
+			Organization: parts[0],
+			Slug:         parts[1],
+		})
+	}
 	return &coderd.GithubOAuth2Config{
 		OAuth2Config: &oauth2.Config{
 			ClientID:     clientID,
diff --git a/coderd/database/db_test.go b/coderd/database/db_test.go
@@ -14,6 +14,9 @@ import (
 
 func TestNestedInTx(t *testing.T) {
 	t.Parallel()
+	if testing.Short() {
+		t.SkipNow()
+	}
 
 	uid := uuid.New()
 	sqlDB := testSQLDB(t)
diff --git a/coderd/userauth.go b/coderd/userauth.go
@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"strings"
 
 	"github.com/google/go-github/v43/github"
 	"github.com/google/uuid"
@@ -18,6 +17,12 @@ import (
 	"github.com/coder/coder/codersdk"
 )
 
+// GithubOAuth2Team represents a team scoped to an organization.
+type GithubOAuth2Team struct {
+	Organization string
+	Slug         string
+}
+
 // GithubOAuth2Provider exposes required functions for the Github authentication flow.
 type GithubOAuth2Config struct {
 	httpmw.OAuth2Config
@@ -28,7 +33,7 @@ type GithubOAuth2Config struct {
 
 	AllowSignups       bool
 	AllowOrganizations []string
-	AllowTeams         []string
+	AllowTeams         []GithubOAuth2Team
 }
 
 func (api *API) userAuthMethods(rw http.ResponseWriter, _ *http.Request) {
@@ -80,21 +85,13 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
 
 		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 <organization>/<team>", organizationAndTeam),
-					})
-					return
-				}
-				if parts[0] != *selectedMembership.Organization.Login {
+			for _, allowTeam := range api.GithubOAuth2Config.AllowTeams {
+				if allowTeam.Organization != *selectedMembership.Organization.Login {
 					// This needs to continue because multiple organizations
 					// could exist in the allow/team listings.
 					continue
 				}
-				if parts[1] != *team.Slug {
+				if allowTeam.Slug != *team.Slug {
 					continue
 				}
 				allowedTeam = team
diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go
@@ -78,7 +78,7 @@ func TestUserOAuth2Github(t *testing.T) {
 		client := coderdtest.New(t, &coderdtest.Options{
 			GithubOAuth2Config: &coderd.GithubOAuth2Config{
 				AllowOrganizations: []string{"coder"},
-				AllowTeams:         []string{"another/something", "coder/frontend"},
+				AllowTeams:         []coderd.GithubOAuth2Team{{"another", "something"}, {"coder", "frontend"}},
 				OAuth2Config:       &oauth2Config{},
 				ListOrganizationMemberships: func(ctx context.Context, client *http.Client) ([]*github.Membership, error) {
 					return []*github.Membership{{
@@ -214,7 +214,7 @@ func TestUserOAuth2Github(t *testing.T) {
 			GithubOAuth2Config: &coderd.GithubOAuth2Config{
 				AllowSignups:       true,
 				AllowOrganizations: []string{"coder"},
-				AllowTeams:         []string{"coder/frontend"},
+				AllowTeams:         []coderd.GithubOAuth2Team{{"coder", "frontend"}},
 				OAuth2Config:       &oauth2Config{},
 				ListOrganizationMemberships: func(ctx context.Context, client *http.Client) ([]*github.Membership, error) {
 					return []*github.Membership{{
diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go
@@ -343,6 +343,9 @@ func TestWorkspaceByOwnerAndName(t *testing.T) {
 // to run various filters against for testing.
 func TestWorkspaceFilter(t *testing.T) {
 	t.Parallel()
+	// Manual tests still occur below, so this is safe to disable.
+	t.Skip("This test is slow and flaky. See: https://github.com/coder/coder/issues/2854")
+	// nolint:unused
 	type coderUser struct {
 		*codersdk.Client
 		User codersdk.User
