From 97b3ffd7ead0166de3a4817ea28dd973c4f9e6f7 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Mon, 8 Aug 2022 20:02:49 +0530 Subject: [PATCH] MM-43202: Change some 501 response codes to fix SLO violations (#20767) There were lots of cases where we were using the code 501 in an incorrect manner, and since 5xx codes are considered as SLO violations, these were flagged by our monitoring incorrectly. These were the guidelines to change the codes: 1. Any case of missing license or config not correctly enabled is set as Forbidden. The logic is that the user is correctly authenticated with a session, but does not have enough privileges to have the higher license, or hasn't changed the config correctly. 2. Any case of incorrectly formed request is obviously a 400. https://mattermost.atlassian.net/browse/MM-43202 ```release-note NONE ``` --- api4/channel.go | 8 ++++---- api4/channel_test.go | 2 +- api4/cloud.go | 22 ++++++++++----------- api4/cloud_test.go | 4 ++-- api4/file.go | 6 +++--- api4/file_test.go | 6 +++--- api4/group.go | 46 ++++++++++++++++++++++---------------------- api4/group_test.go | 30 ++++++++++++++++++----------- api4/user_test.go | 4 ++-- app/emoji.go | 20 +++++++++---------- 10 files changed, 78 insertions(+), 70 deletions(-) diff --git a/api4/channel.go b/api4/channel.go index 954ec5632e..40372dc068 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -1820,7 +1820,7 @@ func updateChannelScheme(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventParameter("scheme_id", *schemeID) if c.App.Channels().License() == nil { - c.Err = model.NewAppError("Api4.UpdateChannelScheme", "api.channel.update_channel_scheme.license.error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.UpdateChannelScheme", "api.channel.update_channel_scheme.license.error", nil, "", http.StatusForbidden) return } @@ -1916,7 +1916,7 @@ func channelMembersMinusGroupMembers(c *Context, w http.ResponseWriter, r *http. func channelMemberCountsByGroup(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil { - c.Err = model.NewAppError("Api4.channelMemberCountsByGroup", "api.channel.channel_member_counts_by_group.license.error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.channelMemberCountsByGroup", "api.channel.channel_member_counts_by_group.license.error", nil, "", http.StatusForbidden) return } @@ -1949,7 +1949,7 @@ func channelMemberCountsByGroup(c *Context, w http.ResponseWriter, r *http.Reque func getChannelModerations(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil { - c.Err = model.NewAppError("Api4.GetChannelModerations", "api.channel.get_channel_moderations.license.error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.GetChannelModerations", "api.channel.get_channel_moderations.license.error", nil, "", http.StatusForbidden) return } @@ -1986,7 +1986,7 @@ func getChannelModerations(c *Context, w http.ResponseWriter, r *http.Request) { func patchChannelModerations(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil { - c.Err = model.NewAppError("Api4.patchChannelModerations", "api.channel.patch_channel_moderations.license.error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.patchChannelModerations", "api.channel.patch_channel_moderations.license.error", nil, "", http.StatusForbidden) return } diff --git a/api4/channel_test.go b/api4/channel_test.go index 216e54f082..e45312b71a 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -3840,7 +3840,7 @@ func TestUpdateChannelScheme(t *testing.T) { th.App.Srv().SetLicense(nil) resp, err = th.SystemAdminClient.UpdateChannelScheme(channel.Id, channelScheme.Id) require.Error(t, err) - CheckNotImplementedStatus(t, resp) + CheckForbiddenStatus(t, resp) th.App.Srv().SetLicense(model.NewTestLicense("")) // Test an invalid scheme scope. diff --git a/api4/cloud.go b/api4/cloud.go index 825db6d5bb..c9675bb118 100644 --- a/api4/cloud.go +++ b/api4/cloud.go @@ -73,7 +73,7 @@ func handleNotifyAdminToUpgrade(c *Context, w http.ResponseWriter, r *http.Reque func getSubscription(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.getSubscription", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getSubscription", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -296,7 +296,7 @@ func validateWorkspaceBusinessEmail(c *Context, w http.ResponseWriter, r *http.R func getCloudProducts(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.getCloudProducts", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getCloudProducts", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -338,7 +338,7 @@ func getCloudProducts(c *Context, w http.ResponseWriter, r *http.Request) { func getCloudLimits(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.getCloudLimits", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getCloudLimits", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -359,7 +359,7 @@ func getCloudLimits(c *Context, w http.ResponseWriter, r *http.Request) { func getCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.getCloudCustomer", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getCloudCustomer", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -385,7 +385,7 @@ func getCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) { func updateCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.updateCloudCustomer", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.updateCloudCustomer", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -423,7 +423,7 @@ func updateCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) { func updateCloudCustomerAddress(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.updateCloudCustomerAddress", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.updateCloudCustomerAddress", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -461,7 +461,7 @@ func updateCloudCustomerAddress(c *Context, w http.ResponseWriter, r *http.Reque func createCustomerPayment(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.createCustomerPayment", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.createCustomerPayment", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -492,7 +492,7 @@ func createCustomerPayment(c *Context, w http.ResponseWriter, r *http.Request) { func confirmCustomerPayment(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.confirmCustomerPayment", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.confirmCustomerPayment", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -529,7 +529,7 @@ func confirmCustomerPayment(c *Context, w http.ResponseWriter, r *http.Request) func getInvoicesForSubscription(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.getInvoicesForSubscription", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getInvoicesForSubscription", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -555,7 +555,7 @@ func getInvoicesForSubscription(c *Context, w http.ResponseWriter, r *http.Reque func getSubscriptionInvoicePDF(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.getSubscriptionInvoicePDF", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getSubscriptionInvoicePDF", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -590,7 +590,7 @@ func getSubscriptionInvoicePDF(c *Context, w http.ResponseWriter, r *http.Reques func handleCWSWebhook(c *Context, w http.ResponseWriter, r *http.Request) { if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { - c.Err = model.NewAppError("Api4.handleCWSWebhook", "api.cloud.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.handleCWSWebhook", "api.cloud.license_error", nil, "", http.StatusForbidden) return } diff --git a/api4/cloud_test.go b/api4/cloud_test.go index a689c68a8e..1964c6981d 100644 --- a/api4/cloud_test.go +++ b/api4/cloud_test.go @@ -30,7 +30,7 @@ func Test_getCloudLimits(t *testing.T) { limits, r, err := th.Client.GetProductLimits() require.Error(t, err) require.Nil(t, limits) - require.Equal(t, http.StatusNotImplemented, r.StatusCode, "Expected 501 Not Implemented") + require.Equal(t, http.StatusForbidden, r.StatusCode, "Expected 403 forbidden") }) t.Run("non cloud license returns not implemented", func(t *testing.T) { @@ -44,7 +44,7 @@ func Test_getCloudLimits(t *testing.T) { limits, r, err := th.Client.GetProductLimits() require.Error(t, err) require.Nil(t, limits) - require.Equal(t, http.StatusNotImplemented, r.StatusCode, "Expected 501 Not Implemented") + require.Equal(t, http.StatusForbidden, r.StatusCode, "Expected 403 forbidden") }) t.Run("error fetching limits returns internal server error", func(t *testing.T) { diff --git a/api4/file.go b/api4/file.go index 1eadda69b8..c1cf36d4a5 100644 --- a/api4/file.go +++ b/api4/file.go @@ -103,7 +103,7 @@ func uploadFileStream(c *Context, w http.ResponseWriter, r *http.Request) { if !*c.App.Config().FileSettings.EnableFileAttachments { c.Err = model.NewAppError("uploadFileStream", "api.file.attachments.disabled.app_error", - nil, "", http.StatusNotImplemented) + nil, "", http.StatusForbidden) return } @@ -545,7 +545,7 @@ func getFileLink(c *Context, w http.ResponseWriter, r *http.Request) { } if !*c.App.Config().FileSettings.EnablePublicLink { - c.Err = model.NewAppError("getPublicLink", "api.file.get_public_link.disabled.app_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("getPublicLink", "api.file.get_public_link.disabled.app_error", nil, "", http.StatusForbidden) return } @@ -643,7 +643,7 @@ func getPublicFile(c *Context, w http.ResponseWriter, r *http.Request) { } if !*c.App.Config().FileSettings.EnablePublicLink { - c.Err = model.NewAppError("getPublicFile", "api.file.get_public_link.disabled.app_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("getPublicFile", "api.file.get_public_link.disabled.app_error", nil, "", http.StatusForbidden) return } diff --git a/api4/file_test.go b/api4/file_test.go index dafb76110e..cc9b055985 100644 --- a/api4/file_test.go +++ b/api4/file_test.go @@ -512,7 +512,7 @@ func TestUploadFiles(t *testing.T) { client: th.SystemAdminClient, names: []string{"test.png"}, skipSuccessValidation: true, - checkResponse: CheckNotImplementedStatus, + checkResponse: CheckForbiddenStatus, setupConfig: func(a *app.App) func(a *app.App) { enableFileAttachments := *a.Config().FileSettings.EnableFileAttachments a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = false }) @@ -919,7 +919,7 @@ func TestGetFileLink(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = false }) _, resp, err = client.GetFileLink(fileId) require.Error(t, err) - CheckNotImplementedStatus(t, resp) + CheckForbiddenStatus(t, resp) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) link, _, err := client.GetFileLink(fileId) @@ -1090,7 +1090,7 @@ func TestGetPublicFile(t *testing.T) { resp, err = http.Get(link) require.NoError(t, err) - require.Equal(t, http.StatusNotImplemented, resp.StatusCode, "should've failed to get image with disabled public link") + require.Equal(t, http.StatusForbidden, resp.StatusCode, "should've failed to get image with disabled public link") // test after the salt has changed th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) diff --git a/api4/group.go b/api4/group.go index 9d13c18325..fa7dd38e0c 100644 --- a/api4/group.go +++ b/api4/group.go @@ -137,7 +137,7 @@ func createGroup(c *Context, w http.ResponseWriter, r *http.Request) { } if group.Source != model.GroupSourceCustom { - c.Err = model.NewAppError("createGroup", "app.group.crud_permission", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("createGroup", "app.group.crud_permission", nil, "", http.StatusBadRequest) return } @@ -153,12 +153,12 @@ func createGroup(c *Context, w http.ResponseWriter, r *http.Request) { } if !group.AllowReference { - c.Err = model.NewAppError("createGroup", "api.custom_groups.must_be_referenceable", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("createGroup", "api.custom_groups.must_be_referenceable", nil, "", http.StatusBadRequest) return } if group.GetRemoteId() != "" { - c.Err = model.NewAppError("createGroup", "api.custom_groups.no_remote_id", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("createGroup", "api.custom_groups.no_remote_id", nil, "", http.StatusBadRequest) return } @@ -234,13 +234,13 @@ func patchGroup(c *Context, w http.ResponseWriter, r *http.Request) { groupPatch.Name = &tmp } else { if *groupPatch.Name == model.UserNotifyAll || *groupPatch.Name == model.ChannelMentionsNotifyProp || *groupPatch.Name == model.UserNotifyHere { - c.Err = model.NewAppError("Api4.patchGroup", "api.ldap_groups.existing_reserved_name_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.patchGroup", "api.ldap_groups.existing_reserved_name_error", nil, "", http.StatusBadRequest) return } //check if a user already has this group name user, _ := c.App.GetUserByUsername(*groupPatch.Name) if user != nil { - c.Err = model.NewAppError("Api4.patchGroup", "api.ldap_groups.existing_user_name_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.patchGroup", "api.ldap_groups.existing_user_name_error", nil, "", http.StatusBadRequest) return } //check if a mentionable group already has this name @@ -249,7 +249,7 @@ func patchGroup(c *Context, w http.ResponseWriter, r *http.Request) { } existingGroup, _ := c.App.GetGroupByName(*groupPatch.Name, searchOpts) if existingGroup != nil { - c.Err = model.NewAppError("Api4.patchGroup", "api.ldap_groups.existing_group_name_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.patchGroup", "api.ldap_groups.existing_group_name_error", nil, "", http.StatusBadRequest) return } } @@ -326,7 +326,7 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventParameter("patch", patch) if !*c.App.Channels().License().Features.LDAPGroups { - c.Err = model.NewAppError("Api4.createGroupSyncable", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.createGroupSyncable", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) return } @@ -385,7 +385,7 @@ func getGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { syncableType := c.Params.SyncableType if !*c.App.Channels().License().Features.LDAPGroups { - c.Err = model.NewAppError("Api4.getGroupSyncable", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getGroupSyncable", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) return } @@ -422,7 +422,7 @@ func getGroupSyncables(c *Context, w http.ResponseWriter, r *http.Request) { syncableType := c.Params.SyncableType if !*c.App.Channels().License().Features.LDAPGroups { - c.Err = model.NewAppError("Api4.getGroupSyncables", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getGroupSyncables", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) return } @@ -487,7 +487,7 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { if !*c.App.Channels().License().Features.LDAPGroups { c.Err = model.NewAppError("Api4.patchGroupSyncable", "api.ldap_groups.license_error", nil, "", - http.StatusNotImplemented) + http.StatusForbidden) return } @@ -552,7 +552,7 @@ func unlinkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventParameter("syncable_type", syncableType) if !*c.App.Channels().License().Features.LDAPGroups { - c.Err = model.NewAppError("Api4.unlinkGroupSyncable", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.unlinkGroupSyncable", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) return } @@ -655,7 +655,7 @@ func getGroupStats(c *Context, w http.ResponseWriter, r *http.Request) { } if !*c.App.Channels().License().Features.LDAPGroups { - c.Err = model.NewAppError("Api4.getGroupStats", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getGroupStats", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) return } @@ -695,7 +695,7 @@ func getGroupsByUserId(c *Context, w http.ResponseWriter, r *http.Request) { } if !*c.App.Channels().License().Features.LDAPGroups { - c.Err = model.NewAppError("Api4.getGroupsByUserId", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getGroupsByUserId", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) return } @@ -721,7 +721,7 @@ func getGroupsByChannel(c *Context, w http.ResponseWriter, r *http.Request) { } if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.LDAPGroups { - c.Err = model.NewAppError("Api4.getGroupsByChannel", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getGroupsByChannel", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) return } @@ -778,7 +778,7 @@ func getGroupsByTeam(c *Context, w http.ResponseWriter, r *http.Request) { return } if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.LDAPGroups { - c.Err = model.NewAppError("Api4.getGroupsByTeam", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getGroupsByTeam", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) return } @@ -820,7 +820,7 @@ func getGroupsAssociatedToChannelsByTeam(c *Context, w http.ResponseWriter, r *h } if !*c.App.Channels().License().Features.LDAPGroups { - c.Err = model.NewAppError("Api4.getGroupsAssociatedToChannelsByTeam", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.getGroupsAssociatedToChannelsByTeam", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) return } @@ -970,7 +970,7 @@ func deleteGroup(c *Context, w http.ResponseWriter, r *http.Request) { } if group.Source != model.GroupSourceCustom { - c.Err = model.NewAppError("Api4.deleteGroup", "app.group.crud_permission", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.deleteGroup", "app.group.crud_permission", nil, "", http.StatusBadRequest) return } @@ -1013,7 +1013,7 @@ func addGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) { } if group.Source != model.GroupSourceCustom { - c.Err = model.NewAppError("Api4.deleteGroup", "app.group.crud_permission", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.deleteGroup", "app.group.crud_permission", nil, "", http.StatusBadRequest) return } @@ -1066,7 +1066,7 @@ func deleteGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) { } if group.Source != model.GroupSourceCustom { - c.Err = model.NewAppError("Api4.deleteGroup", "app.group.crud_permission", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.deleteGroup", "app.group.crud_permission", nil, "", http.StatusBadRequest) return } @@ -1117,19 +1117,19 @@ func licensedAndConfiguredForGroupBySource(app app.AppIface, source model.GroupS lic := app.Srv().License() if lic == nil { - return model.NewAppError("", "api.license_error", nil, "", http.StatusNotImplemented) + return model.NewAppError("", "api.license_error", nil, "", http.StatusForbidden) } if source == model.GroupSourceLdap && !*lic.Features.LDAPGroups { - return model.NewAppError("", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) + return model.NewAppError("", "api.ldap_groups.license_error", nil, "", http.StatusForbidden) } if source == model.GroupSourceCustom && lic.SkuShortName != model.LicenseShortSkuProfessional && lic.SkuShortName != model.LicenseShortSkuEnterprise { - return model.NewAppError("", "api.custom_groups.license_error", nil, "", http.StatusNotImplemented) + return model.NewAppError("", "api.custom_groups.license_error", nil, "", http.StatusBadRequest) } if source == model.GroupSourceCustom && (!app.Config().FeatureFlags.CustomGroups || !*app.Config().ServiceSettings.EnableCustomGroups) { - return model.NewAppError("", "api.custom_groups.feature_disabled", nil, "", http.StatusNotImplemented) + return model.NewAppError("", "api.custom_groups.feature_disabled", nil, "", http.StatusBadRequest) } return nil diff --git a/api4/group_test.go b/api4/group_test.go index 14688fc474..2c0f359caa 100644 --- a/api4/group_test.go +++ b/api4/group_test.go @@ -98,7 +98,7 @@ func TestCreateGroup(t *testing.T) { _, response, err := th.SystemAdminClient.CreateGroup(gbroken) require.Error(t, err) - CheckNotImplementedStatus(t, response) + CheckBadRequestStatus(t, response) validGroup := &model.Group{ DisplayName: "dn_" + model.NewId(), @@ -137,7 +137,7 @@ func TestCreateGroup(t *testing.T) { } _, response, err = th.SystemAdminClient.CreateGroup(unReferenceableCustomGroup) require.Error(t, err) - CheckNotImplementedStatus(t, response) + CheckBadRequestStatus(t, response) unReferenceableCustomGroup.AllowReference = true _, response, err = th.SystemAdminClient.CreateGroup(unReferenceableCustomGroup) require.NoError(t, err) @@ -152,7 +152,7 @@ func TestCreateGroup(t *testing.T) { } _, response, err = th.SystemAdminClient.CreateGroup(customGroupWithRemoteID) require.Error(t, err) - CheckNotImplementedStatus(t, response) + CheckBadRequestStatus(t, response) th.SystemAdminClient.Logout() _, response, err = th.SystemAdminClient.CreateGroup(g) @@ -178,16 +178,16 @@ func TestDeleteGroup(t *testing.T) { _, response, err := th.Client.DeleteGroup(g.Id) require.Error(t, err) - CheckNotImplementedStatus(t, response) + CheckBadRequestStatus(t, response) th.AddPermissionToRole(model.PermissionDeleteCustomGroup.Id, model.SystemUserRoleId) _, response, err = th.Client.DeleteGroup(g.Id) require.Error(t, err) - CheckNotImplementedStatus(t, response) + CheckBadRequestStatus(t, response) _, response, err = th.Client.DeleteGroup(g.Id) require.Error(t, err) - CheckNotImplementedStatus(t, response) + CheckBadRequestStatus(t, response) _, response, err = th.Client.DeleteGroup("wertyuijhbgvfcde") require.Error(t, err) @@ -939,7 +939,11 @@ func TestGetGroupsByChannel(t *testing.T) { th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { _, _, response, err := client.GetGroupsByChannel(th.BasicChannel.Id, opts) require.Error(t, err) - CheckNotImplementedStatus(t, response) + if client == th.SystemAdminClient { + CheckNotImplementedStatus(t, response) + } else { + CheckForbiddenStatus(t, response) + } }) th.App.Srv().SetLicense(model.NewTestLicense("ldap")) @@ -1098,7 +1102,11 @@ func TestGetGroupsByTeam(t *testing.T) { th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { _, _, response, err := client.GetGroupsByTeam(th.BasicTeam.Id, opts) require.Error(t, err) - CheckNotImplementedStatus(t, response) + if client == th.SystemAdminClient { + CheckNotImplementedStatus(t, response) + } else { + CheckForbiddenStatus(t, response) + } }) th.App.Srv().SetLicense(model.NewTestLicense("ldap")) @@ -1248,7 +1256,7 @@ func TestGetGroups(t *testing.T) { opts.Source = model.GroupSourceCustom _, response, err := th.Client.GetGroups(opts) require.Error(t, err) - CheckNotImplementedStatus(t, response) + CheckBadRequestStatus(t, response) // Specify ldap groups source when custom groups feature is disabled opts.Source = model.GroupSourceLdap @@ -1527,7 +1535,7 @@ func TestAddMembersToGroup(t *testing.T) { _, response, upsertErr = th.SystemAdminClient.UpsertGroupMembers(ldapGroup.Id, members) require.Error(t, upsertErr) - CheckNotImplementedStatus(t, response) + CheckBadRequestStatus(t, response) } func TestDeleteMembersFromGroup(t *testing.T) { @@ -1605,5 +1613,5 @@ func TestDeleteMembersFromGroup(t *testing.T) { _, response, deleteErr = th.SystemAdminClient.DeleteGroupMembers(ldapGroup.Id, members) require.Error(t, deleteErr) - CheckNotImplementedStatus(t, response) + CheckBadRequestStatus(t, response) } diff --git a/api4/user_test.go b/api4/user_test.go index 733f6e0e27..91b2df4703 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -1188,7 +1188,7 @@ func TestSearchUsers(t *testing.T) { t.Run("Requires ldap license when searching in group", func(t *testing.T) { _, resp, err = th.SystemAdminClient.SearchUsers(search) require.Error(t, err) - CheckNotImplementedStatus(t, resp) + CheckForbiddenStatus(t, resp) }) th.App.Srv().SetLicense(model.NewTestLicense("ldap")) @@ -2719,7 +2719,7 @@ func TestGetUsersInGroup(t *testing.T) { t.Run("Requires ldap license", func(t *testing.T) { _, response, err := th.SystemAdminClient.GetUsersInGroup(group.Id, 0, 60, "") require.Error(t, err) - CheckNotImplementedStatus(t, response) + CheckForbiddenStatus(t, response) }) th.App.Srv().SetLicense(model.NewTestLicense("ldap")) diff --git a/app/emoji.go b/app/emoji.go index 496868dff3..3de13ab795 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -39,11 +39,11 @@ const ( func (a *App) CreateEmoji(sessionUserId string, emoji *model.Emoji, multiPartImageData *multipart.Form) (*model.Emoji, *model.AppError) { if !*a.Config().ServiceSettings.EnableCustomEmoji { - return nil, model.NewAppError("UploadEmojiImage", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented) + return nil, model.NewAppError("UploadEmojiImage", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden) } if *a.Config().FileSettings.DriverName == "" { - return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) + return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusForbidden) } // wipe the emoji id so that existing emojis can't get overwritten @@ -100,11 +100,11 @@ func (a *App) GetEmojiList(page, perPage int, sort string) ([]*model.Emoji, *mod func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *model.AppError { if !*a.Config().ServiceSettings.EnableCustomEmoji { - return model.NewAppError("UploadEmojiImage", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented) + return model.NewAppError("UploadEmojiImage", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden) } if *a.Config().FileSettings.DriverName == "" { - return model.NewAppError("UploadEmojiImage", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) + return model.NewAppError("UploadEmojiImage", "api.emoji.storage.app_error", nil, "", http.StatusForbidden) } file, err := imageData.Open() @@ -185,11 +185,11 @@ func (a *App) DeleteEmoji(emoji *model.Emoji) *model.AppError { func (a *App) GetEmoji(emojiId string) (*model.Emoji, *model.AppError) { if !*a.Config().ServiceSettings.EnableCustomEmoji { - return nil, model.NewAppError("GetEmoji", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented) + return nil, model.NewAppError("GetEmoji", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden) } if *a.Config().FileSettings.DriverName == "" { - return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) + return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusForbidden) } emoji, err := a.Srv().Store.Emoji().Get(context.Background(), emojiId, true) @@ -208,11 +208,11 @@ func (a *App) GetEmoji(emojiId string) (*model.Emoji, *model.AppError) { func (a *App) GetEmojiByName(emojiName string) (*model.Emoji, *model.AppError) { if !*a.Config().ServiceSettings.EnableCustomEmoji { - return nil, model.NewAppError("GetEmojiByName", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented) + return nil, model.NewAppError("GetEmojiByName", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden) } if *a.Config().FileSettings.DriverName == "" { - return nil, model.NewAppError("GetEmojiByName", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) + return nil, model.NewAppError("GetEmojiByName", "api.emoji.storage.app_error", nil, "", http.StatusForbidden) } emoji, err := a.Srv().Store.Emoji().GetByName(context.Background(), emojiName, true) @@ -231,7 +231,7 @@ func (a *App) GetEmojiByName(emojiName string) (*model.Emoji, *model.AppError) { func (a *App) GetMultipleEmojiByName(names []string) ([]*model.Emoji, *model.AppError) { if !*a.Config().ServiceSettings.EnableCustomEmoji { - return nil, model.NewAppError("GetMultipleEmojiByName", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented) + return nil, model.NewAppError("GetMultipleEmojiByName", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden) } emoji, err := a.Srv().Store.Emoji().GetMultipleByName(names) @@ -269,7 +269,7 @@ func (a *App) GetEmojiImage(emojiId string) ([]byte, string, *model.AppError) { func (a *App) SearchEmoji(name string, prefixOnly bool, limit int) ([]*model.Emoji, *model.AppError) { if !*a.Config().ServiceSettings.EnableCustomEmoji { - return nil, model.NewAppError("SearchEmoji", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented) + return nil, model.NewAppError("SearchEmoji", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden) } list, err := a.Srv().Store.Emoji().Search(name, prefixOnly, limit)