[MI-3629] Added cluster mutexes and solved the problem of race conditions for whitelist

server/api.go

@@ -258,7 +258,7 @@ func (a *API) needsConnect(w http.ResponseWriter, r *http.Request) {
 			if a.p.getConfiguration().EnabledTeams == "" {
 				response["needsConnect"] = true
 			} else {
-				enabledTeams := strings.Fields(a.p.getConfiguration().EnabledTeams)
+				enabledTeams := strings.Split(a.p.getConfiguration().EnabledTeams, ",")
 
 				teams, _ := a.p.API.GetTeamsForUser(userID)
 				for _, enabledTeam := range enabledTeams {
@@ -399,7 +399,45 @@ func (a *API) oauthRedirectHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err = a.p.store.StoreUserInWhitelist(mmUserID); err != nil {
+	tx, err := a.p.store.BeginTx()
+	if err != nil {
+		a.p.API.LogError("Unable to begin transaction", "error", err.Error())
+		http.Error(w, "Something went wrong", http.StatusInternalServerError)
+		return
+	}
+
+	if err = a.p.store.LockWhitelist(tx); err != nil {
+		a.p.API.LogError("Unable to lock whitelist", "error", err.Error())
+		http.Error(w, "Something went wrong", http.StatusInternalServerError)
+		return
+	}
+
+	defer func() {
+		if err = a.p.store.UnlockWhitelist(tx); err != nil {
+			a.p.API.LogError("Unable to unlock whitelist", "error", err.Error())
+		}
+
+		if err = a.p.store.CommitTx(tx); err != nil {
+			a.p.API.LogError("Unable to commit transaction", "error", err.Error())
+		}
+	}()
+
+	whitelistSize, err := a.p.store.GetSizeOfWhitelist(tx)
+	if err != nil {
+		a.p.API.LogError("Unable to get whitelist size", "error", err.Error())
+		http.Error(w, "Something went wrong", http.StatusInternalServerError)
+		return
+	}
+
+	if whitelistSize >= a.p.getConfiguration().ConnectedUsersAllowed {
+		if err = a.p.store.SetUserInfo(mmUserID, msteamsUser.ID, nil); err != nil {
+			a.p.API.LogError("Unable to delete the OAuth token for user", "UserID", mmUserID, "Error", err.Error())
+		}
+		http.Error(w, "You cannot connect your account because the maximum limit of users allowed to connect has been reached. Please contact your system administrator.", http.StatusBadRequest)
+		return
+	}
+
+	if err := a.p.store.StoreUserInWhitelist(tx, mmUserID); err != nil {
 		if !strings.Contains(err.Error(), "Duplicate entry") {
 			a.p.API.LogError("Unable to store the user in whitelist", "UserID", mmUserID, "Error", err.Error())
 			if err = a.p.store.SetUserInfo(mmUserID, msteamsUser.ID, nil); err != nil {

server/command.go

@@ -439,7 +439,7 @@ func (p *Plugin) executeConnectCommand(args *model.CommandArgs) (*model.CommandR
 	}
 
 	if !presentInWhitelist {
-		whitelistSize, err := p.store.GetSizeOfWhitelist()
+		whitelistSize, err := p.store.GetSizeOfWhitelist(nil)
 		if err != nil {
 			p.API.LogError("Error in getting the size of whitelist", "Error", err.Error())
 			return p.cmdError(args.UserId, args.ChannelId, genericErrorMessage)
@@ -484,7 +484,7 @@ func (p *Plugin) executeConnectBotCommand(args *model.CommandArgs) (*model.Comma
 	}
 
 	if !presentInWhitelist {
-		whitelistSize, err := p.store.GetSizeOfWhitelist()
+		whitelistSize, err := p.store.GetSizeOfWhitelist(nil)
 		if err != nil {
 			p.API.LogError("Error in getting the size of whitelist", "Error", err.Error())
 			return p.cmdError(args.UserId, args.ChannelId, genericErrorMessage)

server/command_test.go

@@ -855,7 +855,7 @@ func TestExecuteConnectCommand(t *testing.T) {
 			setupStore: func(s *mockStore.Store) {
 				s.On("GetTokenForMattermostUser", testutils.GetUserID()).Return(nil, nil).Once()
 				s.On("IsUserPresentInWhitelist", testutils.GetUserID()).Return(false, nil).Once()
-				s.On("GetSizeOfWhitelist").Return(0, errors.New("unable to get size of whitelist")).Once()
+				s.On("GetSizeOfWhitelist", (*sql.Tx)(nil)).Return(0, errors.New("unable to get size of whitelist")).Once()
 			},
 		},
 		{
@@ -870,7 +870,7 @@ func TestExecuteConnectCommand(t *testing.T) {
 			setupStore: func(s *mockStore.Store) {
 				s.On("GetTokenForMattermostUser", testutils.GetUserID()).Return(nil, nil).Once()
 				s.On("IsUserPresentInWhitelist", testutils.GetUserID()).Return(false, nil).Once()
-				s.On("GetSizeOfWhitelist").Return(0, nil).Once()
+				s.On("GetSizeOfWhitelist", (*sql.Tx)(nil)).Return(0, nil).Once()
 			},
 		},
 		{
@@ -985,7 +985,7 @@ func TestExecuteConnectBotCommand(t *testing.T) {
 			setupStore: func(s *mockStore.Store) {
 				s.On("GetTokenForMattermostUser", p.userID).Return(nil, nil).Once()
 				s.On("IsUserPresentInWhitelist", p.userID).Return(false, nil).Once()
-				s.On("GetSizeOfWhitelist").Return(0, errors.New("unable to get size of whitelist")).Once()
+				s.On("GetSizeOfWhitelist", (*sql.Tx)(nil)).Return(0, errors.New("unable to get size of whitelist")).Once()
 			},
 		},
 		{
@@ -1001,7 +1001,7 @@ func TestExecuteConnectBotCommand(t *testing.T) {
 			setupStore: func(s *mockStore.Store) {
 				s.On("GetTokenForMattermostUser", p.userID).Return(nil, nil).Once()
 				s.On("IsUserPresentInWhitelist", p.userID).Return(false, nil).Once()
-				s.On("GetSizeOfWhitelist").Return(0, nil).Once()
+				s.On("GetSizeOfWhitelist", (*sql.Tx)(nil)).Return(0, nil).Once()
 			},
 		},
 		{

server/configuration.go

@@ -3,6 +3,7 @@ package main
 import (
 	"encoding/json"
 	"reflect"
+	"strings"
 
 	"github.com/pkg/errors"
 )
@@ -37,6 +38,53 @@ type configuration struct {
 	ConnectedUsersAllowed      int    `json:"connectedUsersAllowed"`
 }
 
+func (c *configuration) ProcessConfiguration() {
+	c.TenantID = strings.TrimSpace(c.TenantID)
+	c.ClientID = strings.TrimSpace(c.ClientID)
+	c.ClientSecret = strings.TrimSpace(c.ClientSecret)
+	c.EncryptionKey = strings.TrimSpace(c.EncryptionKey)
+	c.WebhookSecret = strings.TrimSpace(c.WebhookSecret)
+	c.EnabledTeams = strings.TrimSpace(c.EnabledTeams)
+}
+
+func (p *Plugin) validateConfiguration(configuration *configuration) error {
+	configuration.ProcessConfiguration()
+	if configuration.TenantID == "" {
+		return errors.New("tenant ID should not be empty")
+	}
+	if configuration.ClientID == "" {
+		return errors.New("client ID should not be empty")
+	}
+	if configuration.ClientSecret == "" {
+		return errors.New("client secret should not be empty")
+	}
+	if configuration.EncryptionKey == "" {
+		return errors.New("encryption key should not be empty")
+	}
+	if configuration.WebhookSecret == "" {
+		return errors.New("webhook secret should not be empty")
+	}
+	if configuration.MaxSizeForCompleteDownload < 0 {
+		return errors.New("max size for complete single download should not be negative")
+	}
+	if configuration.BufferSizeForFileStreaming <= 0 {
+		return errors.New("buffer size for file streaming should be greater than zero")
+	}
+
+	if p.store != nil {
+		whitelistSize, err := p.store.GetSizeOfWhitelist(nil)
+		if err != nil {
+			return errors.New("failed to get the size of whitelist from the DB")
+		}
+
+		if configuration.ConnectedUsersAllowed < whitelistSize {
+			return errors.New("failed to save configuration, no. of connected users allowed should be greater than or equal to the current size of the whitelist")
+		}
+	}
+
+	return nil
+}
+
 // Clone shallow copies the configuration. Your implementation may require a deep copy if
 // your configuration has reference types.
 func (c *configuration) Clone() *configuration {
@@ -108,15 +156,8 @@ func (p *Plugin) OnConfigurationChange() error {
 		return errors.Wrap(err, "failed to load plugin configuration")
 	}
 
-	if p.store != nil {
-		whitelistSize, err := p.store.GetSizeOfWhitelist()
-		if err != nil {
-			return errors.New("failed to get the size of whitelist from the DB")
-		}
-
-		if configuration.ConnectedUsersAllowed < whitelistSize {
-			return errors.New("failed to save configuration, no. of connected users allowed should be greater than the current size of the whitelist")
-		}
+	if err := p.validateConfiguration(configuration); err != nil {
+		return err
 	}
 
 	p.setConfiguration(configuration)

server/plugin.go

@@ -344,13 +344,8 @@ func (p *Plugin) OnActivate() error {
 		}
 	}
 
-	whitelistSize, err := p.store.GetSizeOfWhitelist()
-	if err != nil {
-		return errors.New("failed to get the size of whitelist from the DB")
-	}
-
-	if p.getConfiguration().ConnectedUsersAllowed < whitelistSize {
-		return errors.New("failed to save configuration, no. of connected users allowed should be greater than the current size of the whitelist")
+	if err := p.validateConfiguration(p.getConfiguration()); err != nil {
+		return err
 	}
 
 	go func() {