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
```
Этот коммит содержится в:
Agniva De Sarker
2022-08-08 20:02:49 +05:30
коммит произвёл GitHub
родитель f0e4794cda
Коммит 97b3ffd7ea
10 изменённых файлов: 78 добавлений и 70 удалений

Просмотреть файл

@@ -1820,7 +1820,7 @@ func updateChannelScheme(c *Context, w http.ResponseWriter, r *http.Request) {
auditRec.AddEventParameter("scheme_id", *schemeID) auditRec.AddEventParameter("scheme_id", *schemeID)
if c.App.Channels().License() == nil { 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 return
} }
@@ -1916,7 +1916,7 @@ func channelMembersMinusGroupMembers(c *Context, w http.ResponseWriter, r *http.
func channelMemberCountsByGroup(c *Context, w http.ResponseWriter, r *http.Request) { func channelMemberCountsByGroup(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil { 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 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) { func getChannelModerations(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil { 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 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) { func patchChannelModerations(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil { 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 return
} }

Просмотреть файл

@@ -3840,7 +3840,7 @@ func TestUpdateChannelScheme(t *testing.T) {
th.App.Srv().SetLicense(nil) th.App.Srv().SetLicense(nil)
resp, err = th.SystemAdminClient.UpdateChannelScheme(channel.Id, channelScheme.Id) resp, err = th.SystemAdminClient.UpdateChannelScheme(channel.Id, channelScheme.Id)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, resp) CheckForbiddenStatus(t, resp)
th.App.Srv().SetLicense(model.NewTestLicense("")) th.App.Srv().SetLicense(model.NewTestLicense(""))
// Test an invalid scheme scope. // Test an invalid scheme scope.

Просмотреть файл

@@ -73,7 +73,7 @@ func handleNotifyAdminToUpgrade(c *Context, w http.ResponseWriter, r *http.Reque
func getSubscription(c *Context, w http.ResponseWriter, r *http.Request) { func getSubscription(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func getCloudProducts(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func getCloudLimits(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func getCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func updateCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func updateCloudCustomerAddress(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func createCustomerPayment(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func confirmCustomerPayment(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func getInvoicesForSubscription(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func getSubscriptionInvoicePDF(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 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) { func handleCWSWebhook(c *Context, w http.ResponseWriter, r *http.Request) {
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.Cloud { 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 return
} }

Просмотреть файл

@@ -30,7 +30,7 @@ func Test_getCloudLimits(t *testing.T) {
limits, r, err := th.Client.GetProductLimits() limits, r, err := th.Client.GetProductLimits()
require.Error(t, err) require.Error(t, err)
require.Nil(t, limits) 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) { 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() limits, r, err := th.Client.GetProductLimits()
require.Error(t, err) require.Error(t, err)
require.Nil(t, limits) 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) { t.Run("error fetching limits returns internal server error", func(t *testing.T) {

Просмотреть файл

@@ -103,7 +103,7 @@ func uploadFileStream(c *Context, w http.ResponseWriter, r *http.Request) {
if !*c.App.Config().FileSettings.EnableFileAttachments { if !*c.App.Config().FileSettings.EnableFileAttachments {
c.Err = model.NewAppError("uploadFileStream", c.Err = model.NewAppError("uploadFileStream",
"api.file.attachments.disabled.app_error", "api.file.attachments.disabled.app_error",
nil, "", http.StatusNotImplemented) nil, "", http.StatusForbidden)
return return
} }
@@ -545,7 +545,7 @@ func getFileLink(c *Context, w http.ResponseWriter, r *http.Request) {
} }
if !*c.App.Config().FileSettings.EnablePublicLink { 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 return
} }
@@ -643,7 +643,7 @@ func getPublicFile(c *Context, w http.ResponseWriter, r *http.Request) {
} }
if !*c.App.Config().FileSettings.EnablePublicLink { 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 return
} }

Просмотреть файл

@@ -512,7 +512,7 @@ func TestUploadFiles(t *testing.T) {
client: th.SystemAdminClient, client: th.SystemAdminClient,
names: []string{"test.png"}, names: []string{"test.png"},
skipSuccessValidation: true, skipSuccessValidation: true,
checkResponse: CheckNotImplementedStatus, checkResponse: CheckForbiddenStatus,
setupConfig: func(a *app.App) func(a *app.App) { setupConfig: func(a *app.App) func(a *app.App) {
enableFileAttachments := *a.Config().FileSettings.EnableFileAttachments enableFileAttachments := *a.Config().FileSettings.EnableFileAttachments
a.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = false }) 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 }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = false })
_, resp, err = client.GetFileLink(fileId) _, resp, err = client.GetFileLink(fileId)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, resp) CheckForbiddenStatus(t, resp)
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true })
link, _, err := client.GetFileLink(fileId) link, _, err := client.GetFileLink(fileId)
@@ -1090,7 +1090,7 @@ func TestGetPublicFile(t *testing.T) {
resp, err = http.Get(link) resp, err = http.Get(link)
require.NoError(t, err) 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 // test after the salt has changed
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnablePublicLink = true })

Просмотреть файл

@@ -137,7 +137,7 @@ func createGroup(c *Context, w http.ResponseWriter, r *http.Request) {
} }
if group.Source != model.GroupSourceCustom { 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 return
} }
@@ -153,12 +153,12 @@ func createGroup(c *Context, w http.ResponseWriter, r *http.Request) {
} }
if !group.AllowReference { 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 return
} }
if group.GetRemoteId() != "" { 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 return
} }
@@ -234,13 +234,13 @@ func patchGroup(c *Context, w http.ResponseWriter, r *http.Request) {
groupPatch.Name = &tmp groupPatch.Name = &tmp
} else { } else {
if *groupPatch.Name == model.UserNotifyAll || *groupPatch.Name == model.ChannelMentionsNotifyProp || *groupPatch.Name == model.UserNotifyHere { 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 return
} }
//check if a user already has this group name //check if a user already has this group name
user, _ := c.App.GetUserByUsername(*groupPatch.Name) user, _ := c.App.GetUserByUsername(*groupPatch.Name)
if user != nil { 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 return
} }
//check if a mentionable group already has this name //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) existingGroup, _ := c.App.GetGroupByName(*groupPatch.Name, searchOpts)
if existingGroup != nil { 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 return
} }
} }
@@ -326,7 +326,7 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) {
auditRec.AddEventParameter("patch", patch) auditRec.AddEventParameter("patch", patch)
if !*c.App.Channels().License().Features.LDAPGroups { 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 return
} }
@@ -385,7 +385,7 @@ func getGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) {
syncableType := c.Params.SyncableType syncableType := c.Params.SyncableType
if !*c.App.Channels().License().Features.LDAPGroups { 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 return
} }
@@ -422,7 +422,7 @@ func getGroupSyncables(c *Context, w http.ResponseWriter, r *http.Request) {
syncableType := c.Params.SyncableType syncableType := c.Params.SyncableType
if !*c.App.Channels().License().Features.LDAPGroups { 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 return
} }
@@ -487,7 +487,7 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) {
if !*c.App.Channels().License().Features.LDAPGroups { if !*c.App.Channels().License().Features.LDAPGroups {
c.Err = model.NewAppError("Api4.patchGroupSyncable", "api.ldap_groups.license_error", nil, "", c.Err = model.NewAppError("Api4.patchGroupSyncable", "api.ldap_groups.license_error", nil, "",
http.StatusNotImplemented) http.StatusForbidden)
return return
} }
@@ -552,7 +552,7 @@ func unlinkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) {
auditRec.AddEventParameter("syncable_type", syncableType) auditRec.AddEventParameter("syncable_type", syncableType)
if !*c.App.Channels().License().Features.LDAPGroups { 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 return
} }
@@ -655,7 +655,7 @@ func getGroupStats(c *Context, w http.ResponseWriter, r *http.Request) {
} }
if !*c.App.Channels().License().Features.LDAPGroups { 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 return
} }
@@ -695,7 +695,7 @@ func getGroupsByUserId(c *Context, w http.ResponseWriter, r *http.Request) {
} }
if !*c.App.Channels().License().Features.LDAPGroups { 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 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 { 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 return
} }
@@ -778,7 +778,7 @@ func getGroupsByTeam(c *Context, w http.ResponseWriter, r *http.Request) {
return return
} }
if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.LDAPGroups { 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 return
} }
@@ -820,7 +820,7 @@ func getGroupsAssociatedToChannelsByTeam(c *Context, w http.ResponseWriter, r *h
} }
if !*c.App.Channels().License().Features.LDAPGroups { 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 return
} }
@@ -970,7 +970,7 @@ func deleteGroup(c *Context, w http.ResponseWriter, r *http.Request) {
} }
if group.Source != model.GroupSourceCustom { 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 return
} }
@@ -1013,7 +1013,7 @@ func addGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) {
} }
if group.Source != model.GroupSourceCustom { 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 return
} }
@@ -1066,7 +1066,7 @@ func deleteGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) {
} }
if group.Source != model.GroupSourceCustom { 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 return
} }
@@ -1117,19 +1117,19 @@ func licensedAndConfiguredForGroupBySource(app app.AppIface, source model.GroupS
lic := app.Srv().License() lic := app.Srv().License()
if lic == nil { 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 { 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 { 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) { 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 return nil

Просмотреть файл

@@ -98,7 +98,7 @@ func TestCreateGroup(t *testing.T) {
_, response, err := th.SystemAdminClient.CreateGroup(gbroken) _, response, err := th.SystemAdminClient.CreateGroup(gbroken)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, response) CheckBadRequestStatus(t, response)
validGroup := &model.Group{ validGroup := &model.Group{
DisplayName: "dn_" + model.NewId(), DisplayName: "dn_" + model.NewId(),
@@ -137,7 +137,7 @@ func TestCreateGroup(t *testing.T) {
} }
_, response, err = th.SystemAdminClient.CreateGroup(unReferenceableCustomGroup) _, response, err = th.SystemAdminClient.CreateGroup(unReferenceableCustomGroup)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, response) CheckBadRequestStatus(t, response)
unReferenceableCustomGroup.AllowReference = true unReferenceableCustomGroup.AllowReference = true
_, response, err = th.SystemAdminClient.CreateGroup(unReferenceableCustomGroup) _, response, err = th.SystemAdminClient.CreateGroup(unReferenceableCustomGroup)
require.NoError(t, err) require.NoError(t, err)
@@ -152,7 +152,7 @@ func TestCreateGroup(t *testing.T) {
} }
_, response, err = th.SystemAdminClient.CreateGroup(customGroupWithRemoteID) _, response, err = th.SystemAdminClient.CreateGroup(customGroupWithRemoteID)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, response) CheckBadRequestStatus(t, response)
th.SystemAdminClient.Logout() th.SystemAdminClient.Logout()
_, response, err = th.SystemAdminClient.CreateGroup(g) _, response, err = th.SystemAdminClient.CreateGroup(g)
@@ -178,16 +178,16 @@ func TestDeleteGroup(t *testing.T) {
_, response, err := th.Client.DeleteGroup(g.Id) _, response, err := th.Client.DeleteGroup(g.Id)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, response) CheckBadRequestStatus(t, response)
th.AddPermissionToRole(model.PermissionDeleteCustomGroup.Id, model.SystemUserRoleId) th.AddPermissionToRole(model.PermissionDeleteCustomGroup.Id, model.SystemUserRoleId)
_, response, err = th.Client.DeleteGroup(g.Id) _, response, err = th.Client.DeleteGroup(g.Id)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, response) CheckBadRequestStatus(t, response)
_, response, err = th.Client.DeleteGroup(g.Id) _, response, err = th.Client.DeleteGroup(g.Id)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, response) CheckBadRequestStatus(t, response)
_, response, err = th.Client.DeleteGroup("wertyuijhbgvfcde") _, response, err = th.Client.DeleteGroup("wertyuijhbgvfcde")
require.Error(t, err) require.Error(t, err)
@@ -939,7 +939,11 @@ func TestGetGroupsByChannel(t *testing.T) {
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
_, _, response, err := client.GetGroupsByChannel(th.BasicChannel.Id, opts) _, _, response, err := client.GetGroupsByChannel(th.BasicChannel.Id, opts)
require.Error(t, err) 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")) 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) { th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
_, _, response, err := client.GetGroupsByTeam(th.BasicTeam.Id, opts) _, _, response, err := client.GetGroupsByTeam(th.BasicTeam.Id, opts)
require.Error(t, err) 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")) th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
@@ -1248,7 +1256,7 @@ func TestGetGroups(t *testing.T) {
opts.Source = model.GroupSourceCustom opts.Source = model.GroupSourceCustom
_, response, err := th.Client.GetGroups(opts) _, response, err := th.Client.GetGroups(opts)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, response) CheckBadRequestStatus(t, response)
// Specify ldap groups source when custom groups feature is disabled // Specify ldap groups source when custom groups feature is disabled
opts.Source = model.GroupSourceLdap opts.Source = model.GroupSourceLdap
@@ -1527,7 +1535,7 @@ func TestAddMembersToGroup(t *testing.T) {
_, response, upsertErr = th.SystemAdminClient.UpsertGroupMembers(ldapGroup.Id, members) _, response, upsertErr = th.SystemAdminClient.UpsertGroupMembers(ldapGroup.Id, members)
require.Error(t, upsertErr) require.Error(t, upsertErr)
CheckNotImplementedStatus(t, response) CheckBadRequestStatus(t, response)
} }
func TestDeleteMembersFromGroup(t *testing.T) { func TestDeleteMembersFromGroup(t *testing.T) {
@@ -1605,5 +1613,5 @@ func TestDeleteMembersFromGroup(t *testing.T) {
_, response, deleteErr = th.SystemAdminClient.DeleteGroupMembers(ldapGroup.Id, members) _, response, deleteErr = th.SystemAdminClient.DeleteGroupMembers(ldapGroup.Id, members)
require.Error(t, deleteErr) require.Error(t, deleteErr)
CheckNotImplementedStatus(t, response) CheckBadRequestStatus(t, response)
} }

Просмотреть файл

@@ -1188,7 +1188,7 @@ func TestSearchUsers(t *testing.T) {
t.Run("Requires ldap license when searching in group", func(t *testing.T) { t.Run("Requires ldap license when searching in group", func(t *testing.T) {
_, resp, err = th.SystemAdminClient.SearchUsers(search) _, resp, err = th.SystemAdminClient.SearchUsers(search)
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, resp) CheckForbiddenStatus(t, resp)
}) })
th.App.Srv().SetLicense(model.NewTestLicense("ldap")) 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) { t.Run("Requires ldap license", func(t *testing.T) {
_, response, err := th.SystemAdminClient.GetUsersInGroup(group.Id, 0, 60, "") _, response, err := th.SystemAdminClient.GetUsersInGroup(group.Id, 0, 60, "")
require.Error(t, err) require.Error(t, err)
CheckNotImplementedStatus(t, response) CheckForbiddenStatus(t, response)
}) })
th.App.Srv().SetLicense(model.NewTestLicense("ldap")) th.App.Srv().SetLicense(model.NewTestLicense("ldap"))

Просмотреть файл

@@ -39,11 +39,11 @@ const (
func (a *App) CreateEmoji(sessionUserId string, emoji *model.Emoji, multiPartImageData *multipart.Form) (*model.Emoji, *model.AppError) { func (a *App) CreateEmoji(sessionUserId string, emoji *model.Emoji, multiPartImageData *multipart.Form) (*model.Emoji, *model.AppError) {
if !*a.Config().ServiceSettings.EnableCustomEmoji { 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 == "" { 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 // 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 { func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *model.AppError {
if !*a.Config().ServiceSettings.EnableCustomEmoji { 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 == "" { 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() 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) { func (a *App) GetEmoji(emojiId string) (*model.Emoji, *model.AppError) {
if !*a.Config().ServiceSettings.EnableCustomEmoji { 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 == "" { 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) 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) { func (a *App) GetEmojiByName(emojiName string) (*model.Emoji, *model.AppError) {
if !*a.Config().ServiceSettings.EnableCustomEmoji { 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 == "" { 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) 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) { func (a *App) GetMultipleEmojiByName(names []string) ([]*model.Emoji, *model.AppError) {
if !*a.Config().ServiceSettings.EnableCustomEmoji { 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) 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) { func (a *App) SearchEmoji(name string, prefixOnly bool, limit int) ([]*model.Emoji, *model.AppError) {
if !*a.Config().ServiceSettings.EnableCustomEmoji { 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) list, err := a.Srv().Store.Emoji().Search(name, prefixOnly, limit)