Merge branch 'master' of github.com:mattermost/mattermost-plugin-github into cyrusjc/adding-mentions-to-sidebar

Author: abbas-dependable-naqvi

CODEOWNERS

@@ -1 +1 @@
-* @wiggin77
+* @mattermost/deployment-engineering

Makefile

@@ -246,7 +246,11 @@ ifneq ($(HAS_WEBAPP),)
 	mkdir -p dist/$(PLUGIN_ID)/webapp
 	cp -r webapp/dist dist/$(PLUGIN_ID)/webapp/
 endif
+ifeq ($(shell uname),Darwin)
+	cd dist && tar --disable-copyfile -cvzf $(BUNDLE_NAME) $(PLUGIN_ID)
+else
 	cd dist && tar -cvzf $(BUNDLE_NAME) $(PLUGIN_ID)
+endif
 
 	@echo plugin built at: dist/$(BUNDLE_NAME)
 

server/mocks/mock_KvStore.go

@@ -37,7 +37,8 @@ const (
 	requestTimeout       = 30 * time.Second
 	oauthCompleteTimeout = 2 * time.Minute
 
-	channelIDParam = "channelId"
+	channelIDParam    = "channelId"
+	organisationParam = "organization"
 )
 
 type OAuthState struct {
@@ -1443,59 +1444,99 @@ func (p *Plugin) getReposByOrg(c *UserContext, w http.ResponseWriter, r *http.Re
 
 	opt := github.ListOptions{PerPage: 50}
 
-	org := r.URL.Query().Get("organization")
+	orgString := r.URL.Query().Get(organisationParam)
 
-	if org == "" {
+	if orgString == "" {
 		c.Log.Warnf("Organization query param is empty")
-		p.writeAPIError(w, &APIErrorResponse{Message: "Organization query is empty, must include organization name ", StatusCode: http.StatusBadRequest})
+		p.writeAPIError(w, &APIErrorResponse{Message: "Organization query parameter is empty, must include organization name ", StatusCode: http.StatusBadRequest})
+		return
+	}
+
+	channelIDString := r.URL.Query().Get(channelIDParam)
+	if channelIDString == "" {
+		c.Log.Warnf("Channel ID query param is empty")
+		p.writeAPIError(w, &APIErrorResponse{Message: "ChannelId query parameter is empty, must include Channel ID ", StatusCode: http.StatusBadRequest})
 		return
 	}
 
+	orgList := strings.Split(orgString, ",")
 	var allRepos []*github.Repository
-	var err error
-	var statusCode int
 
-	// If an organization is the username of an authenticated user then return repos where the authenticated user is the owner
-	if org == c.GHInfo.GitHubUsername {
-		allRepos, err = p.getRepositoryList(c.Ctx, c.GHInfo, "", githubClient, opt)
-		if err != nil {
-			c.Log.WithError(err).Errorf("Failed to list repositories")
-			p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: http.StatusInternalServerError})
-			return
+	for _, org := range orgList {
+		org = strings.TrimSpace(org)
+		if org == "" {
+			continue
 		}
-	} else {
-		allRepos, statusCode, err = p.getRepositoryListByOrg(c.Ctx, c.GHInfo, org, githubClient, opt)
-		if err != nil {
-			if statusCode == http.StatusNotFound {
-				allRepos, err = p.getRepositoryList(c.Ctx, c.GHInfo, org, githubClient, opt)
-				if err != nil {
-					c.Log.WithError(err).Errorf("Failed to list repositories")
-					p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: http.StatusInternalServerError})
-					return
+
+		var repos []*github.Repository
+		var err error
+		var statusCode int
+
+		// If an organization is the username of an authenticated user then return repos where the authenticated user is the owner
+		if org == c.GHInfo.GitHubUsername {
+			repos, err = p.getRepositoryList(c.Ctx, c.GHInfo, "", githubClient, opt)
+			if err != nil {
+				c.Log.WithError(err).Errorf("Failed to list repositories for user %s", org)
+				continue
+			}
+		} else {
+			repos, statusCode, err = p.getRepositoryListByOrg(c.Ctx, c.GHInfo, org, githubClient, opt)
+			if err != nil {
+				if statusCode == http.StatusNotFound {
+					repos, err = p.getRepositoryList(c.Ctx, c.GHInfo, org, githubClient, opt)
+					if err != nil {
+						c.Log.WithError(err).Errorf("Failed to list repositories for org/user %s", org)
+						continue
+					}
+				} else {
+					c.Log.WithError(err).Warnf("Failed to list repositories for org %s", org)
+					continue
 				}
-			} else {
-				c.Log.WithError(err).Warnf("Failed to list repositories")
-				p.writeAPIError(w, &APIErrorResponse{Message: "Failed to fetch repositories", StatusCode: statusCode})
-				return
 			}
 		}
+
+		allRepos = append(allRepos, repos...)
 	}
-	// Only send repositories which are part of the requested organization
-	type RepositoryResponse struct {
-		Name        string          `json:"name,omitempty"`
-		FullName    string          `json:"full_name,omitempty"`
-		Permissions map[string]bool `json:"permissions,omitempty"`
-	}
 
-	resp := make([]*RepositoryResponse, len(allRepos))
+	// Only send repositories which are part of the requested organization(s)
+
+	repoResp := make([]RepoResponse, len(allRepos))
 	for i, r := range allRepos {
-		resp[i] = &RepositoryResponse{
+		repoResp[i] = RepoResponse{
 			Name:        r.GetName(),
 			FullName:    r.GetFullName(),
 			Permissions: r.GetPermissions(),
 		}
 	}
 
+	resp := RepositoryResponse{
+		Repos: repoResp,
+	}
+
+	// Add default repo if available
+	defaultRepo, dErr := p.GetDefaultRepo(c.GHInfo.UserID, channelIDString)
+	if dErr != nil {
+		c.Log.WithError(dErr).Warnf("Failed to get the default repo for the channel. UserID: %s. ChannelID: %s", c.GHInfo.UserID, channelIDString)
+	}
+
+	if defaultRepo != "" {
+		config := p.getConfiguration()
+		baseURL := config.getBaseURL()
+		owner, repo := parseOwnerAndRepo(defaultRepo, baseURL)
+		defaultRepository, err := getRepository(c.Ctx, owner, repo, githubClient)
+		if err != nil {
+			c.Log.WithError(err).Warnf("Failed to get the default repo %s/%s", owner, repo)
+		}
+
+		if defaultRepository != nil {
+			resp.DefaultRepo = RepoResponse{
+				Name:        defaultRepository.GetName(),
+				FullName:    defaultRepository.GetFullName(),
+				Permissions: defaultRepository.Permissions,
+			}
+		}
+	}
+
 	p.writeJSON(w, resp)
 }
 

server/plugin/api.go

@@ -6,6 +6,7 @@ package plugin
 import (
 	"context"
 	"fmt"
+	"net/http"
 	"strings"
 	"unicode"
 
@@ -41,6 +42,8 @@ const (
 	PerPageValue = 50
 )
 
+var ErrNotFound = errors.New("github user not found")
+
 const DefaultRepoKey string = "%s_%s-default-repo"
 
 var validFeatures = map[string]bool{
@@ -187,6 +190,36 @@ func contains(s []string, e string) bool {
 	return false
 }
 
+func (p *Plugin) isValidGitHubUsername(username string, userInfo *GitHubUserInfo) (bool, error) {
+	githubClient := p.githubConnectUser(context.Background(), userInfo)
+
+	if cErr := p.useGitHubClient(userInfo, func(userInfo *GitHubUserInfo, token *oauth2.Token) error {
+		ghUser, _, err := githubClient.Users.Get(context.Background(), username)
+		if err != nil {
+			if gErr, ok := err.(*github.ErrorResponse); ok && gErr.Response.StatusCode == http.StatusNotFound {
+				return ErrNotFound
+			}
+
+			return err
+		}
+
+		if ghUser == nil {
+			return ErrNotFound
+		}
+
+		return nil
+	}); cErr != nil {
+		if errors.Is(cErr, ErrNotFound) {
+			return false, nil
+		}
+
+		p.client.Log.Warn("Failed to fetch user", "error", cErr.Error())
+		return false, errors.New("Failed to fetch user")
+	}
+
+	return true, nil
+}
+
 func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo *GitHubUserInfo) string {
 	mutedUsernames, err := p.getMutedUsernames(userInfo)
 	if err != nil {
@@ -198,7 +231,12 @@ func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo *
 		return username + " is already muted"
 	}
 
-	if strings.Contains(username, ",") {
+	isValidUsername, err := p.isValidGitHubUsername(username, userInfo)
+	if err != nil {
+		return "Error occurred validating username"
+	}
+
+	if strings.Contains(username, ",") || !isValidUsername {
 		return "Invalid username provided"
 	}
 
@@ -226,7 +264,10 @@ func (p *Plugin) handleUnmute(_ *model.CommandArgs, username string, userInfo *G
 	}
 
 	userToMute := []string{username}
-	newMutedList := arrayDifference(mutedUsernames, userToMute)
+	newMutedList, removed := arrayDifference(mutedUsernames, userToMute)
+	if !removed {
+		return username + " is not muted"
+	}
 
 	_, err = p.store.Set(userInfo.UserID+"-muted-users", []byte(strings.Join(newMutedList, ",")))
 	if err != nil {
@@ -237,7 +278,17 @@ func (p *Plugin) handleUnmute(_ *model.CommandArgs, username string, userInfo *G
 }
 
 func (p *Plugin) handleUnmuteAll(_ *model.CommandArgs, userInfo *GitHubUserInfo) string {
-	_, err := p.store.Set(userInfo.UserID+"-muted-users", []byte(""))
+	mutedUsernames, err := p.getMutedUsernames(userInfo)
+	if err != nil {
+		p.client.Log.Error("error occurred getting muted users.", "UserID", userInfo.UserID, "Error", err)
+		return "An error occurred getting muted users. Please try again later"
+	}
+
+	if len(mutedUsernames) == 0 {
+		return "You have no muted users"
+	}
+
+	_, err = p.store.Set(userInfo.UserID+"-muted-users", []byte(""))
 	if err != nil {
 		return "Error occurred unmuting users"
 	}
@@ -273,18 +324,22 @@ func (p *Plugin) handleMuteCommand(_ *plugin.Context, args *model.CommandArgs, p
 }
 
 // Returns the elements in a, that are not in b
-func arrayDifference(a, b []string) []string {
+func arrayDifference(a, b []string) ([]string, bool) {
 	mb := make(map[string]struct{}, len(b))
 	for _, x := range b {
 		mb[x] = struct{}{}
 	}
+
 	var diff []string
+	removed := false
 	for _, x := range a {
 		if _, found := mb[x]; !found {
 			diff = append(diff, x)
+		} else {
+			removed = true
 		}
 	}
-	return diff
+	return diff, removed
 }
 
 func (p *Plugin) handleSubscribe(c *plugin.Context, args *model.CommandArgs, parameters []string, userInfo *GitHubUserInfo) string {
@@ -599,8 +654,12 @@ func (p *Plugin) handleUnsubscribe(_ *plugin.Context, args *model.CommandArgs, p
 
 	owner = strings.ToLower(owner)
 	repo = strings.ToLower(repo)
-	if err := p.Unsubscribe(args.ChannelId, repo, owner); err != nil {
-		p.client.Log.Warn("Failed to unsubscribe", "repo", repo, "error", err.Error())
+	if sErr := p.Unsubscribe(args.ChannelId, repo, owner); sErr != nil {
+		if sErr.Code == SubscriptionNotFound {
+			return sErr.Error.Error()
+		}
+
+		p.client.Log.Warn("Failed to unsubscribe", "repo", repo, "error", sErr.Error.Error())
 		return "Encountered an error trying to unsubscribe. Please try again."
 	}
 
@@ -798,8 +857,8 @@ func (p *Plugin) handleSetDefaultRepo(args *model.CommandArgs, parameters []stri
 	owner = strings.ToLower(owner)
 	repo = strings.ToLower(repo)
 
-	if config.GitHubOrg != "" && strings.ToLower(config.GitHubOrg) != owner {
-		return fmt.Sprintf("Repository is not part of the locked Github organization. Locked Github organization: %s", config.GitHubOrg)
+	if config.GitHubOrg != "" && !p.isOrgInLockedOrgs(config.GitHubOrg, owner) {
+		return fmt.Sprintf("Repository is not part of the locked Github organization. Locked Github organizations: %s", config.GitHubOrg)
 	}
 
 	ctx := context.Background()
@@ -866,6 +925,21 @@ func (p *Plugin) handleUnSetDefaultRepo(args *model.CommandArgs, userInfo *GitHu
 	return "The default repository has been unset successfully"
 }
 
+func (p *Plugin) isOrgInLockedOrgs(configuredOrgs, owner string) bool {
+	if configuredOrgs == "" {
+		return true
+	}
+
+	orgs := strings.Split(configuredOrgs, ",")
+	for _, org := range orgs {
+		if strings.EqualFold(strings.TrimSpace(org), strings.TrimSpace(owner)) {
+			return true
+		}
+	}
+
+	return false
+}
+
 func (p *Plugin) handleSetup(_ *plugin.Context, args *model.CommandArgs, parameters []string) string {
 	userID := args.UserId
 	isSysAdmin, err := p.isAuthorizedSysAdmin(userID)
@@ -944,6 +1018,14 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo
 		return &model.CommandResponse{}, nil
 	}
 
+	if action == "help" {
+		message := p.handleHelp(c, args, parameters, nil)
+		if message != "" {
+			p.postCommandResponse(args, message)
+		}
+		return &model.CommandResponse{}, nil
+	}
+
 	config := p.getConfiguration()
 
 	if validationErr := config.IsValid(); validationErr != nil {
@@ -1032,6 +1114,9 @@ func getAutocompleteData(config *Configuration) *model.AutocompleteData {
 		about := command.BuildInfoAutocomplete("about")
 		github.AddCommand(about)
 
+		help := model.NewAutocompleteData("help", "", "Display Slash Command help text")
+		github.AddCommand(help)
+
 		return github
 	}