From c39c05e93ccd9397b72fd07186233ef8b13fedbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20V=C3=A9lez=20Vidal?= Date: Thu, 20 Oct 2022 16:01:15 +0200 Subject: [PATCH] Replace 500 errors with more meaningful error codes (#21439) * Replace 500 errors with more meaningful error codes * replace forbidden with badrequest for cloud customer fetch, rename parameter name --- api4/cloud.go | 16 ++++++------ api4/cloud_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++++++ model/client4.go | 4 +-- 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/api4/cloud.go b/api4/cloud.go index e86a9b0aca..a88ac919b3 100644 --- a/api4/cloud.go +++ b/api4/cloud.go @@ -158,14 +158,14 @@ func requestCloudTrial(c *Context, w http.ResponseWriter, r *http.Request) { // check if the email needs to be set bodyBytes, err := io.ReadAll(r.Body) if err != nil { - c.Err = model.NewAppError("Api4.requestCloudTrial", "api.cloud.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + c.Err = model.NewAppError("Api4.requestCloudTrial", "api.cloud.app_error", nil, "", http.StatusBadRequest).Wrap(err) return } // this value will not be empty when both emails (user admin and CWS customer) are not business email and - // we need to request a new email from the user via the request business email modal + // a new business email was provided via the request business email modal var startTrialRequest *model.StartCloudTrialRequest if err = json.Unmarshal(bodyBytes, &startTrialRequest); err != nil { - c.Err = model.NewAppError("Api4.requestCloudTrial", "api.cloud.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + c.Err = model.NewAppError("Api4.requestCloudTrial", "api.cloud.app_error", nil, "", http.StatusBadRequest).Wrap(err) return } @@ -199,20 +199,20 @@ func validateBusinessEmail(c *Context, w http.ResponseWriter, r *http.Request) { user, appErr := c.App.GetUser(c.AppContext.Session().UserId) if appErr != nil { - c.Err = model.NewAppError("Api4.validateBusinessEmail", "api.cloud.request_error", nil, "", http.StatusInternalServerError).Wrap(appErr) + c.Err = model.NewAppError("Api4.validateBusinessEmail", "api.cloud.request_error", nil, "", http.StatusForbidden).Wrap(appErr) return } bodyBytes, err := io.ReadAll(r.Body) if err != nil { - c.Err = model.NewAppError("Api4.requestCloudTrial", "api.cloud.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + c.Err = model.NewAppError("Api4.requestCloudTrial", "api.cloud.app_error", nil, "", http.StatusBadRequest).Wrap(err) return } var emailToValidate *model.ValidateBusinessEmailRequest err = json.Unmarshal(bodyBytes, &emailToValidate) if err != nil { - c.Err = model.NewAppError("Api4.requestCloudTrial", "api.cloud.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + c.Err = model.NewAppError("Api4.requestCloudTrial", "api.cloud.app_error", nil, "", http.StatusBadRequest).Wrap(err) return } @@ -244,14 +244,14 @@ func validateWorkspaceBusinessEmail(c *Context, w http.ResponseWriter, r *http.R user, userErr := c.App.GetUser(c.AppContext.Session().UserId) if userErr != nil { - c.Err = model.NewAppError("Api4.validateWorkspaceBusinessEmail", "api.cloud.request_error", nil, userErr.Error(), http.StatusInternalServerError) + c.Err = userErr return } // get the cloud customer email to validate if is a valid business email cloudCustomer, err := c.App.Cloud().GetCloudCustomer(user.Id) if err != nil { - c.Err = model.NewAppError("Api4.validateWorkspaceBusinessEmail", "api.cloud.request_error", nil, err.Error(), http.StatusInternalServerError) + c.Err = model.NewAppError("Api4.validateWorkspaceBusinessEmail", "api.cloud.request_error", nil, err.Error(), http.StatusBadRequest) return } emailErr := c.App.Cloud().ValidateBusinessEmail(user.Id, cloudCustomer.Email) diff --git a/api4/cloud_test.go b/api4/cloud_test.go index 999f1077e4..2092342c89 100644 --- a/api4/cloud_test.go +++ b/api4/cloud_test.go @@ -296,6 +296,20 @@ func Test_requestTrial(t *testing.T) { require.Equal(t, subscriptionChanged, subscription) require.Equal(t, http.StatusOK, r.StatusCode, "Status OK") }) + + t.Run("Empty body returns bad request", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) + + th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + + r, err := th.SystemAdminClient.DoAPIPutBytes("/cloud/request-trial", nil) + require.Error(t, err) + closeBody(r) + require.Equal(t, http.StatusBadRequest, r.StatusCode, "Status Bad Request") + }) } func Test_validateBusinessEmail(t *testing.T) { @@ -373,6 +387,20 @@ func Test_validateBusinessEmail(t *testing.T) { require.NoError(t, err) require.Equal(t, http.StatusOK, res.StatusCode, "200") }) + + t.Run("Empty body returns bad request", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) + + th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + + r, err := th.SystemAdminClient.DoAPIPostBytes("/cloud/validate-business-email", nil) + require.Error(t, err) + closeBody(r) + require.Equal(t, http.StatusBadRequest, r.StatusCode, "Status Bad Request") + }) } func Test_validateWorkspaceBusinessEmail(t *testing.T) { @@ -442,6 +470,39 @@ func Test_validateWorkspaceBusinessEmail(t *testing.T) { _, err := th.SystemAdminClient.ValidateWorkspaceBusinessEmail() require.NoError(t, err) }) + + t.Run("Error while grabbing the cloud customer returns bad request", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) + + th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + + cloud := mocks.CloudInterface{} + + cloudCustomerInfo := model.CloudCustomerInfo{ + Email: "badrequest@gmail.com", + } + + // return an error while getting the cloud customer so we validate the forbidden error return + cloud.Mock.On("GetCloudCustomer", th.SystemAdminUser.Id).Return(nil, errors.New("error while gettings the cloud customer")) + + // required cloud mocks so the request doesn't fail + cloud.Mock.On("ValidateBusinessEmail", th.SystemAdminUser.Id, cloudCustomerInfo.Email).Return(errors.New("invalid email")) + cloud.Mock.On("ValidateBusinessEmail", th.SystemAdminUser.Id, th.SystemAdminUser.Email).Return(nil) + + cloudImpl := th.App.Srv().Cloud + defer func() { + th.App.Srv().Cloud = cloudImpl + }() + th.App.Srv().Cloud = &cloud + + r, err := th.SystemAdminClient.DoAPIPostBytes("/cloud/validate-workspace-business-email", nil) + require.Error(t, err) + closeBody(r) + require.Equal(t, http.StatusBadRequest, r.StatusCode, "Status Bad Request") + }) } func TestGetCloudProducts(t *testing.T) { diff --git a/model/client4.go b/model/client4.go index cfe0f9244e..b569a8fa86 100644 --- a/model/client4.go +++ b/model/client4.go @@ -8004,8 +8004,8 @@ func (c *Client4) ConfirmCustomerPayment(confirmRequest *ConfirmPaymentMethodReq return BuildResponse(r), nil } -func (c *Client4) RequestCloudTrial(email *StartCloudTrialRequest) (*Subscription, *Response, error) { - payload, err := json.Marshal(email) +func (c *Client4) RequestCloudTrial(cloudTrialRequest *StartCloudTrialRequest) (*Subscription, *Response, error) { + payload, err := json.Marshal(cloudTrialRequest) if err != nil { return nil, nil, NewAppError("RequestCloudTrial", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) }