Check team allowlist in command runner, rather than event controller

server/controllers/events/events_controller.go

@@ -62,7 +62,6 @@ type VCSEventsController struct {
 	// request validation is done.
 	GitlabWebhookSecret  []byte
 	RepoAllowlistChecker *events.RepoAllowlistChecker
-	TeamAllowlistChecker *events.TeamAllowlistChecker
 	// SilenceAllowlistErrors controls whether we write an error comment on
 	// pull requests from non-allowlisted repos.
 	SilenceAllowlistErrors bool
@@ -437,20 +436,6 @@ func (e *VCSEventsController) handleCommentEvent(w http.ResponseWriter, baseRepo
 		return
 	}
 
-	// Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands
-	if !e.TestingMode {
-		ok, err := e.checkUserPermissions(baseRepo, user, parseResult.Command)
-		if err != nil {
-			e.Logger.Err("unable to comment on pull request: %s", err)
-			return
-		}
-		if !ok {
-			e.commentUserDoesNotHavePermissions(baseRepo, pullNum, user, parseResult.Command)
-			e.respond(w, logging.Warn, http.StatusForbidden, "User @%s does not have permissions to execute '%s' command", user.Username, parseResult.Command.Name.String())
-			return
-		}
-	}
-
 	e.Logger.Debug("executing command")
 	fmt.Fprintln(w, "Processing...")
 	if !e.TestingMode {
@@ -568,25 +553,3 @@ func (e *VCSEventsController) commentNotAllowlisted(baseRepo models.Repo, pullNu
 		e.Logger.Err("unable to comment on pull request: %s", err)
 	}
 }
-
-// commentUserDoesNotHavePermissions comments on the pull request that the user
-// is not allowed to execute the command.
-func (e *VCSEventsController) commentUserDoesNotHavePermissions(baseRepo models.Repo, pullNum int, user models.User, cmd *events.CommentCommand) {
-	errMsg := fmt.Sprintf("```\nError: User @%s does not have permissions to execute '%s' command.\n```", user.Username, cmd.Name)
-	if err := e.VCSClient.CreateComment(baseRepo, pullNum, errMsg, ""); err != nil {
-		e.Logger.Err("unable to comment on pull request: %s", err)
-	}
-}
-
-// checkUserPermissions checks if the user has permissions to execute the command
-func (e *VCSEventsController) checkUserPermissions(repo models.Repo, user models.User, cmd *events.CommentCommand) (bool, error) {
-	teams, err := e.VCSClient.GetTeamNamesForUser(repo, user)
-	if err != nil {
-		return false, err
-	}
-	ok := e.TeamAllowlistChecker.IsCommandAllowedForAnyTeam(teams, cmd.Name.String())
-	if !ok {
-		return false, nil
-	}
-	return true, nil
-}

server/events/command_runner.go

@@ -116,6 +116,7 @@ type DefaultCommandRunner struct {
 	Drainer                       *Drainer
 	PreWorkflowHooksCommandRunner PreWorkflowHooksCommandRunner
 	PullStatusFetcher             PullStatusFetcher
+	TeamAllowlistChecker          *TeamAllowlistChecker
 }
 
 // RunAutoplanCommand runs plan and policy_checks when a pull request is opened or updated.
@@ -162,6 +163,32 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo
 	autoPlanRunner.Run(ctx, nil)
 }
 
+// commentUserDoesNotHavePermissions comments on the pull request that the user
+// is not allowed to execute the command.
+func (c *DefaultCommandRunner) commentUserDoesNotHavePermissions(baseRepo models.Repo, pullNum int, user models.User, cmd *CommentCommand) {
+	errMsg := fmt.Sprintf("```\nError: User @%s does not have permissions to execute '%s' command.\n```", user.Username, cmd.Name.String())
+	if err := c.VCSClient.CreateComment(baseRepo, pullNum, errMsg, ""); err != nil {
+		c.Logger.Err("unable to comment on pull request: %s", err)
+	}
+}
+
+// checkUserPermissions checks if the user has permissions to execute the command
+func (c *DefaultCommandRunner) checkUserPermissions(repo models.Repo, user models.User, cmd *CommentCommand) (bool, error) {
+	if c.TeamAllowlistChecker == nil || len(c.TeamAllowlistChecker.rules) == 0 {
+		// allowlist restriction is not enabled
+		return true, nil
+	}
+	teams, err := c.VCSClient.GetTeamNamesForUser(repo, user)
+	if err != nil {
+		return false, err
+	}
+	ok := c.TeamAllowlistChecker.IsCommandAllowedForAnyTeam(teams, cmd.Name.String())
+	if !ok {
+		return false, nil
+	}
+	return true, nil
+}
+
 // RunCommentCommand executes the command.
 // We take in a pointer for maybeHeadRepo because for some events there isn't
 // enough data to construct the Repo model and callers might want to wait until
@@ -179,6 +206,17 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead
 	log := c.buildLogger(baseRepo.FullName, pullNum)
 	defer c.logPanics(baseRepo, pullNum, log)
 
+	// Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands
+	ok, err := c.checkUserPermissions(baseRepo, user, cmd)
+	if err != nil {
+		c.Logger.Err("Unable to check user permissions: %s", err)
+		return
+	}
+	if !ok {
+		c.commentUserDoesNotHavePermissions(baseRepo, pullNum, user, cmd)
+		return
+	}
+
 	headRepo, pull, err := c.ensureValidRepoMetadata(baseRepo, maybeHeadRepo, maybePull, user, pullNum, log)
 	if err != nil {
 		return

server/server.go

@@ -596,6 +596,11 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 		models.VersionCommand:         versionCommandRunner,
 	}
 
+	githubTeamAllowlistChecker, err := events.NewTeamAllowlistChecker(userConfig.GithubTeamAllowlist)
+	if err != nil {
+		return nil, err
+	}
+
 	commandRunner := &events.DefaultCommandRunner{
 		VCSClient:                     vcsClient,
 		GithubPullGetter:              githubClient,
@@ -613,15 +618,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 		Drainer:                       drainer,
 		PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner,
 		PullStatusFetcher:             boltdb,
+		TeamAllowlistChecker:          githubTeamAllowlistChecker,
 	}
 	repoAllowlist, err := events.NewRepoAllowlistChecker(userConfig.RepoAllowlist)
 	if err != nil {
 		return nil, err
 	}
-	githubTeamAllowlistChecker, err := events.NewTeamAllowlistChecker(userConfig.GithubTeamAllowlist)
-	if err != nil {
-		return nil, err
-	}
 	locksController := &controllers.LocksController{
 		AtlantisVersion:    config.AtlantisVersion,
 		AtlantisURL:        parsedURL,
@@ -647,7 +649,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 		GitlabRequestParserValidator:    &events_controllers.DefaultGitlabRequestParserValidator{},
 		GitlabWebhookSecret:             []byte(userConfig.GitlabWebhookSecret),
 		RepoAllowlistChecker:            repoAllowlist,
-		TeamAllowlistChecker:            githubTeamAllowlistChecker,
 		SilenceAllowlistErrors:          userConfig.SilenceAllowlistErrors,
 		SupportedVCSHosts:               supportedVCSHosts,
 		VCSClient:                       vcsClient,