MM-45713 - change 500 error to json object (#20682)

* MM-45713 - change 500 error to json object

* validate possible encoding errors and follow standards

* replace normal debugging string with true string

* use bool type instead of string

Co-authored-by: Pablo Velez Vidal <pablo.velez@mattermost.com>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

api4/cloud.go

@@ -14,6 +14,7 @@ import (
 	"github.com/mattermost/mattermost-server/v6/audit"
 	"github.com/mattermost/mattermost-server/v6/model"
 	"github.com/mattermost/mattermost-server/v6/plugin"
+	"github.com/mattermost/mattermost-server/v6/shared/mlog"
 )
 
 func (api *API) InitCloud() {
@@ -232,12 +233,19 @@ func validateBusinessEmail(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	errValidatingEmail := c.App.Cloud().ValidateBusinessEmail(user.Id, emailToValidate.Email)
+	emailErr := c.App.Cloud().ValidateBusinessEmail(user.Id, emailToValidate.Email)
-	if errValidatingEmail != nil {
+	if emailErr != nil {
-		c.Err = model.NewAppError("Api4.valiateBusinessEmail", "api.cloud.request_error", nil, errValidatingEmail.Error(), http.StatusInternalServerError)
+		c.Err = model.NewAppError("Api4.validateBusinessEmail", "api.cloud.request_error", nil, emailErr.Error(), http.StatusForbidden)
+		emailResp := model.ValidateBusinessEmailResponse{IsValid: false}
+		if err := json.NewEncoder(w).Encode(emailResp); err != nil {
+			mlog.Warn("Error while writing response", mlog.Err(err))
+		}
 		return
 	}
-	ReturnStatusOK(w)
+	emailResp := model.ValidateBusinessEmailResponse{IsValid: true}
+	if err := json.NewEncoder(w).Encode(emailResp); err != nil {
+		mlog.Warn("Error while writing response", mlog.Err(err))
+	}
 }
 
 func validateWorkspaceBusinessEmail(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -263,20 +271,27 @@ func validateWorkspaceBusinessEmail(c *Context, w http.ResponseWriter, r *http.R
 		c.Err = model.NewAppError("Api4.validateWorkspaceBusinessEmail", "api.cloud.request_error", nil, err.Error(), http.StatusInternalServerError)
 		return
 	}
-	errValidatingSystemEmail := c.App.Cloud().ValidateBusinessEmail(user.Id, cloudCustomer.Email)
+	emailErr := c.App.Cloud().ValidateBusinessEmail(user.Id, cloudCustomer.Email)
 
 	// if the current workspace email is not a valid business email
-	if errValidatingSystemEmail != nil {
+	if emailErr != nil {
 		// grab the current admin email and validate it
 		errValidatingAdminEmail := c.App.Cloud().ValidateBusinessEmail(user.Id, user.Email)
 		if errValidatingAdminEmail != nil {
-			c.Err = model.NewAppError("Api4.validateWorkspaceBusinessEmail", "api.cloud.request_error", nil, errValidatingAdminEmail.Error(), http.StatusInternalServerError)
+			c.Err = model.NewAppError("Api4.validateWorkspaceBusinessEmail", "api.cloud.request_error", nil, errValidatingAdminEmail.Error(), http.StatusForbidden)
+			emailResp := model.ValidateBusinessEmailResponse{IsValid: false}
+			if err := json.NewEncoder(w).Encode(emailResp); err != nil {
+				mlog.Warn("Error while writing response", mlog.Err(err))
+			}
 			return
 		}
 	}
 
 	// if any of the emails is valid, return ok
-	ReturnStatusOK(w)
+	emailResp := model.ValidateBusinessEmailResponse{IsValid: true}
+	if err := json.NewEncoder(w).Encode(emailResp); err != nil {
+		mlog.Warn("Error while writing response", mlog.Err(err))
+	}
 }
 
 func getCloudProducts(c *Context, w http.ResponseWriter, r *http.Request) {

api4/cloud_test.go

@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"net/http/httptest"
 	"os"
 	"testing"
 	"time"
@@ -401,21 +400,19 @@ func TestNotifyAdminToUpgrade(t *testing.T) {
 	})
 }
 func Test_validateBusinessEmail(t *testing.T) {
-	t.Run("Initial request has invalid email", func(t *testing.T) {
+	t.Run("Returns forbidden for non admin executors", func(t *testing.T) {
 		th := Setup(t).InitBasic()
 		defer th.TearDown()
 
 		th.Client.Login(th.BasicUser.Email, th.BasicUser.Password)
 
-		validateBusinessEmail := model.ValidateBusinessEmailRequest{Email: ""}
+		invalidEmail := model.ValidateBusinessEmailRequest{Email: "invalid@gmail.com"}
 
 		th.App.Srv().SetLicense(model.NewTestLicense("cloud"))
 
 		cloud := mocks.CloudInterface{}
 
-		resp := httptest.NewRecorder()
+		cloud.Mock.On("ValidateBusinessEmail", th.SystemAdminUser.Id, invalidEmail.Email).Return(errors.New("invalid email"))
-
-		cloud.Mock.On("ValidateBusinessEmail", mock.Anything).Return(resp, nil)
 
 		cloudImpl := th.App.Srv().Cloud
 		defer func() {
@@ -423,8 +420,59 @@ func Test_validateBusinessEmail(t *testing.T) {
 		}()
 		th.App.Srv().Cloud = &cloud
 
-		_, err := th.Client.ValidateBusinessEmail(&validateBusinessEmail)
+		res, err := th.Client.ValidateBusinessEmail(&invalidEmail)
 		require.Error(t, err)
+		require.Equal(t, http.StatusForbidden, res.StatusCode, "403")
+	})
+
+	t.Run("Returns forbidden for invalid business email", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		th.Client.Login(th.BasicUser.Email, th.BasicUser.Password)
+
+		validBusinessEmail := model.ValidateBusinessEmailRequest{Email: "invalid@slacker.com"}
+
+		th.App.Srv().SetLicense(model.NewTestLicense("cloud"))
+
+		cloud := mocks.CloudInterface{}
+
+		cloud.Mock.On("ValidateBusinessEmail", th.SystemAdminUser.Id, validBusinessEmail.Email).Return(errors.New("invalid email"))
+
+		cloudImpl := th.App.Srv().Cloud
+		defer func() {
+			th.App.Srv().Cloud = cloudImpl
+		}()
+		th.App.Srv().Cloud = &cloud
+
+		res, err := th.SystemAdminClient.ValidateBusinessEmail(&validBusinessEmail)
+		require.Error(t, err)
+		require.Equal(t, http.StatusForbidden, res.StatusCode, "403")
+	})
+
+	t.Run("Validate business email for admin", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		th.Client.Login(th.BasicUser.Email, th.BasicUser.Password)
+
+		validBusinessEmail := model.ValidateBusinessEmailRequest{Email: "valid@mattermost.com"}
+
+		th.App.Srv().SetLicense(model.NewTestLicense("cloud"))
+
+		cloud := mocks.CloudInterface{}
+
+		cloud.Mock.On("ValidateBusinessEmail", th.SystemAdminUser.Id, validBusinessEmail.Email).Return(nil)
+
+		cloudImpl := th.App.Srv().Cloud
+		defer func() {
+			th.App.Srv().Cloud = cloudImpl
+		}()
+		th.App.Srv().Cloud = &cloud
+
+		res, err := th.SystemAdminClient.ValidateBusinessEmail(&validBusinessEmail)
+		require.NoError(t, err)
+		require.Equal(t, http.StatusOK, res.StatusCode, "200")
 	})
 }
 

model/cloud.go

@@ -105,6 +105,10 @@ type ValidateBusinessEmailRequest struct {
 	Email string `json:"email"`
 }
 
+type ValidateBusinessEmailResponse struct {
+	IsValid bool `json:"is_valid"`
+}
+
 // CloudCustomerInfo represents editable info of a customer.
 type CloudCustomerInfo struct {
 	Name             string `json:"name"`