MM-63728: simplify category store with graphql gone (#30848)

* move category permissions to api

In https://github.com/mattermost/mattermost/pull/21038, we changed the
behaviour of the channel category store to filter out deleted teams and
teams for which the user was not a member. This was necessary in part
due to querying multiple teams via GraphQL.

With GraphQL no longer supported, let's move the permissions to the
API instead and remove the `JOIN` to filter out teams in the store.

Note that we /don't/ prevent access to deleted teams. For better or
worse, deleted teams remain largely accessible via other API endpoints
anyway.

* remove ExcludeTeam / GraphQL support

As part of https://github.com/mattermost/mattermost/pull/20353, we added
`ExcludeTeam` and the associated logic to support a GraphQL API.

With GraphQL no longer supported, let's simplify this logic and remove
the filtering and associated complexity.

* Fix shadow variable declaration in channel_store_categories.go

Fixed golangci-lint error by reusing existing err variable rather than shadowing it.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix build issue

* Remove SidebarCategorySearchOpts and simplify API to use teamID string

Per code review feedback, this change removes the SidebarCategorySearchOpts
struct entirely since the Type field was never used in the store implementation.
All methods now accept a simple teamID string parameter instead of the struct,
which simplifies the API and makes the code clearer.

Changes:
- Remove SidebarCategorySearchOpts struct from store.go
- Update CreateInitialSidebarCategories and GetSidebarCategories signatures
- Update all implementations (sqlstore, retrylayer, timerlayer, mocks)
- Update all callers to pass teamID string directly
- Clean up unused imports

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Этот коммит содержится в:
Jesse Hallam
2025-06-19 21:48:12 -03:00
коммит произвёл GitHub
родитель b69412d23f
Коммит dcc72c4c61
12 изменённых файлов: 517 добавлений и 449 удалений

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

@@ -23,6 +23,11 @@ func getCategoriesForTeamForUser(c *Context, w http.ResponseWriter, r *http.Requ
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
categories, appErr := c.App.GetSidebarCategoriesForTeamForUser(c.AppContext, c.Params.UserId, c.Params.TeamId)
if appErr != nil {
c.Err = appErr
@@ -51,6 +56,11 @@ func createCategoryForTeamForUser(c *Context, w http.ResponseWriter, r *http.Req
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
auditRec := c.MakeAuditRecord("createCategoryForTeamForUser", audit.Fail)
defer c.LogAuditRec(auditRec)
@@ -96,6 +106,11 @@ func getCategoryOrderForTeamForUser(c *Context, w http.ResponseWriter, r *http.R
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
order, appErr := c.App.GetSidebarCategoryOrder(c.AppContext, c.Params.UserId, c.Params.TeamId)
if appErr != nil {
c.Err = appErr
@@ -119,6 +134,11 @@ func updateCategoryOrderForTeamForUser(c *Context, w http.ResponseWriter, r *htt
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
auditRec := c.MakeAuditRecord("updateCategoryOrderForTeamForUser", audit.Fail)
defer c.LogAuditRec(auditRec)
@@ -159,6 +179,11 @@ func getCategoryForTeamForUser(c *Context, w http.ResponseWriter, r *http.Reques
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
categories, appErr := c.App.GetSidebarCategory(c.AppContext, c.Params.CategoryId)
if appErr != nil {
c.Err = appErr
@@ -187,6 +212,11 @@ func updateCategoriesForTeamForUser(c *Context, w http.ResponseWriter, r *http.R
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
auditRec := c.MakeAuditRecord("updateCategoriesForTeamForUser", audit.Fail)
defer c.LogAuditRec(auditRec)
@@ -296,6 +326,11 @@ func updateCategoryForTeamForUser(c *Context, w http.ResponseWriter, r *http.Req
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
auditRec := c.MakeAuditRecord("updateCategoryForTeamForUser", audit.Fail)
defer c.LogAuditRec(auditRec)
@@ -342,6 +377,11 @@ func deleteCategoryForTeamForUser(c *Context, w http.ResponseWriter, r *http.Req
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
auditRec := c.MakeAuditRecord("deleteCategoryForTeamForUser", audit.Fail)
defer c.LogAuditRec(auditRec)

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

@@ -106,6 +106,49 @@ func TestCreateCategoryForTeamForUser(t *testing.T) {
})
})
t.Run("should return error when user tries to create a category for a team they're not a member of", func(t *testing.T) {
// Create a user
user, appErr := th.App.CreateUser(th.Context, &model.User{
Email: th.GenerateTestEmail(),
Username: "user_" + model.NewId(),
Password: "password",
})
require.Nil(t, appErr)
// Create a team and add the user to it
team, appErr := th.App.CreateTeam(th.Context, &model.Team{
DisplayName: "Team for testing",
Name: "test-team-" + model.NewId(),
Email: th.GenerateTestEmail(),
Type: model.TeamOpen,
})
require.Nil(t, appErr)
th.LinkUserToTeam(user, team)
// Create a client and log in
client := th.CreateClient()
_, _, err := client.Login(context.Background(), user.Email, "password")
require.NoError(t, err)
// Now remove the user from the team
appErr = th.App.RemoveUserFromTeam(th.Context, team.Id, user.Id, th.SystemAdminUser.Id)
require.Nil(t, appErr)
// Attempt to create a category for the team the user is no longer a member of
category := &model.SidebarCategoryWithChannels{
SidebarCategory: model.SidebarCategory{
UserId: user.Id,
TeamId: team.Id,
DisplayName: "test category",
},
}
_, resp, err := client.CreateSidebarCategoryForTeamForUser(context.Background(), user.Id, team.Id, category)
require.Error(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
})
t.Run("should publish expected WS payload", func(t *testing.T) {
userWSClient := th.CreateConnectedWebSocketClient(t)
@@ -450,6 +493,58 @@ func TestUpdateCategoryForTeamForUser(t *testing.T) {
closeBody(r)
})
})
t.Run("should return error when user tries to update a category for a team they're not a member of", func(t *testing.T) {
// Create a user
user, appErr := th.App.CreateUser(th.Context, &model.User{
Email: th.GenerateTestEmail(),
Username: "user_" + model.NewId(),
Password: "password",
})
require.Nil(t, appErr)
// Create a team and add the user to it
team, appErr := th.App.CreateTeam(th.Context, &model.Team{
DisplayName: "Team for testing",
Name: "test-team-" + model.NewId(),
Email: th.GenerateTestEmail(),
Type: model.TeamOpen,
})
require.Nil(t, appErr)
th.LinkUserToTeam(user, team)
// Create a client and log in
client := th.CreateClient()
_, _, err := client.Login(context.Background(), user.Email, "password")
require.NoError(t, err)
// Get categories to have valid category IDs
categories, _, err := client.GetSidebarCategoriesForTeamForUser(context.Background(), user.Id, team.Id, "")
require.NoError(t, err)
require.NotEmpty(t, categories.Categories)
// Store a category to use after team membership is revoked
categoryToUpdate := &model.SidebarCategoryWithChannels{
SidebarCategory: model.SidebarCategory{
Id: categories.Categories[0].Id,
UserId: user.Id,
TeamId: team.Id,
DisplayName: "Updated Category",
Type: categories.Categories[0].Type,
},
Channels: categories.Categories[0].Channels,
}
// Remove the user from the team
appErr = th.App.RemoveUserFromTeam(th.Context, team.Id, user.Id, th.SystemAdminUser.Id)
require.Nil(t, appErr)
// Attempt to update a category for the team after being removed
_, resp, err := client.UpdateSidebarCategoryForTeamForUser(context.Background(), user.Id, team.Id, categoryToUpdate.Id, categoryToUpdate)
require.Error(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
})
}
func TestUpdateCategoriesForTeamForUser(t *testing.T) {
@@ -548,6 +643,60 @@ func TestUpdateCategoriesForTeamForUser(t *testing.T) {
_, _, err = client.UpdateSidebarCategoryOrderForTeamForUser(context.Background(), user.Id, th.BasicTeam.Id, []string{categories.Order[1], categories.Order[0], "asd"})
require.Error(t, err)
})
t.Run("should return error when user tries to update categories for a team they're not a member of", func(t *testing.T) {
// Create a user
user, appErr := th.App.CreateUser(th.Context, &model.User{
Email: th.GenerateTestEmail(),
Username: "user_" + model.NewId(),
Password: "password",
})
require.Nil(t, appErr)
// Create a team and add the user to it
team, appErr := th.App.CreateTeam(th.Context, &model.Team{
DisplayName: "Team for testing",
Name: "test-team-" + model.NewId(),
Email: th.GenerateTestEmail(),
Type: model.TeamOpen,
})
require.Nil(t, appErr)
th.LinkUserToTeam(user, team)
// Create a client and log in
client := th.CreateClient()
_, _, err := client.Login(context.Background(), user.Email, "password")
require.NoError(t, err)
// Get categories to have valid category IDs
existingCategories, _, err := client.GetSidebarCategoriesForTeamForUser(context.Background(), user.Id, team.Id, "")
require.NoError(t, err)
require.NotEmpty(t, existingCategories.Categories)
// Prepare categories to update after team membership is revoked
categoriesToUpdate := []*model.SidebarCategoryWithChannels{
{
SidebarCategory: model.SidebarCategory{
Id: existingCategories.Categories[0].Id,
UserId: user.Id,
TeamId: team.Id,
DisplayName: "Updated Category",
Type: existingCategories.Categories[0].Type,
},
Channels: existingCategories.Categories[0].Channels,
},
}
// Remove the user from the team
appErr = th.App.RemoveUserFromTeam(th.Context, team.Id, user.Id, th.SystemAdminUser.Id)
require.Nil(t, appErr)
// Attempt to update categories for the team after being removed
_, resp, err := client.UpdateSidebarCategoriesForTeamForUser(context.Background(), user.Id, team.Id, categoriesToUpdate)
require.Error(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
})
}
func TestGetCategoriesForTeamForUser(t *testing.T) {
@@ -583,6 +732,34 @@ func TestGetCategoriesForTeamForUser(t *testing.T) {
require.Equal(t, http.StatusForbidden, resp.StatusCode)
})
t.Run("should return error for a team the user is not a member of", func(t *testing.T) {
// Create a new user and team
user, appErr := th.App.CreateUser(th.Context, &model.User{
Email: th.GenerateTestEmail(),
Username: "user_" + model.NewId(),
Password: "password",
})
require.Nil(t, appErr)
team, appErr := th.App.CreateTeam(th.Context, &model.Team{
DisplayName: "Team for testing",
Name: "test-team-" + model.NewId(),
Email: th.GenerateTestEmail(),
Type: model.TeamOpen,
})
require.Nil(t, appErr)
// Log in as the new user
client := th.CreateClient()
_, _, err := client.Login(context.Background(), user.Email, "password")
require.NoError(t, err)
// Attempt to get categories for a team the user is not a member of
_, resp, err := client.GetSidebarCategoriesForTeamForUser(context.Background(), user.Id, team.Id, "")
require.Error(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
})
t.Run("should return error with invalid user id", func(t *testing.T) {
_, resp, err := th.Client.GetSidebarCategoriesForTeamForUser(context.Background(), "invalid_user_id", th.BasicTeam.Id, "")
require.Error(t, err)
@@ -660,6 +837,45 @@ func TestGetCategoryOrderForTeamForUser(t *testing.T) {
require.Error(t, err)
require.Equal(t, http.StatusUnauthorized, resp.StatusCode)
})
t.Run("should return error when user tries to get category order for a team they're not a member of", func(t *testing.T) {
// Create a user
user, appErr := th.App.CreateUser(th.Context, &model.User{
Email: th.GenerateTestEmail(),
Username: "user_" + model.NewId(),
Password: "password",
})
require.Nil(t, appErr)
// Create a team and add the user to it
team, appErr := th.App.CreateTeam(th.Context, &model.Team{
DisplayName: "Team for testing",
Name: "test-team-" + model.NewId(),
Email: th.GenerateTestEmail(),
Type: model.TeamOpen,
})
require.Nil(t, appErr)
th.LinkUserToTeam(user, team)
// Create a client and log in
client := th.CreateClient()
_, _, err := client.Login(context.Background(), user.Email, "password")
require.NoError(t, err)
// Verify the user can access categories initially
_, _, err = client.GetSidebarCategoriesForTeamForUser(context.Background(), user.Id, team.Id, "")
require.NoError(t, err)
// Remove the user from the team
appErr = th.App.RemoveUserFromTeam(th.Context, team.Id, user.Id, th.SystemAdminUser.Id)
require.Nil(t, appErr)
// Attempt to get the category order for the team after being removed
_, resp, err := client.GetSidebarCategoryOrderForTeamForUser(context.Background(), user.Id, team.Id, "")
require.Error(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
})
}
func TestUpdateCategoryOrderForTeamForUser(t *testing.T) {
@@ -779,6 +995,46 @@ func TestUpdateCategoryOrderForTeamForUser(t *testing.T) {
closeBody(r)
})
})
t.Run("should return error when user tries to update category order for a team they're not a member of", func(t *testing.T) {
// Create a user
user, appErr := th.App.CreateUser(th.Context, &model.User{
Email: th.GenerateTestEmail(),
Username: "user_" + model.NewId(),
Password: "password",
})
require.Nil(t, appErr)
// Create a team and add the user to it
team, appErr := th.App.CreateTeam(th.Context, &model.Team{
DisplayName: "Team for testing",
Name: "test-team-" + model.NewId(),
Email: th.GenerateTestEmail(),
Type: model.TeamOpen,
})
require.Nil(t, appErr)
th.LinkUserToTeam(user, team)
// Create a client and log in
client := th.CreateClient()
_, _, err := client.Login(context.Background(), user.Email, "password")
require.NoError(t, err)
// Get categories to have a valid order
categories, _, err := client.GetSidebarCategoriesForTeamForUser(context.Background(), user.Id, team.Id, "")
require.NoError(t, err)
require.NotEmpty(t, categories.Order)
// Remove the user from the team
appErr = th.App.RemoveUserFromTeam(th.Context, team.Id, user.Id, th.SystemAdminUser.Id)
require.Nil(t, appErr)
// Attempt to update the category order for the team after being removed
_, resp, err := client.UpdateSidebarCategoryOrderForTeamForUser(context.Background(), user.Id, team.Id, categories.Order)
require.Error(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
})
}
func TestGetCategoryForTeamForUser(t *testing.T) {
@@ -878,6 +1134,49 @@ func TestGetCategoryForTeamForUser(t *testing.T) {
closeBody(r)
})
})
t.Run("should return error when user tries to get category for a team they're not a member of", func(t *testing.T) {
// Create a user
user, appErr := th.App.CreateUser(th.Context, &model.User{
Email: th.GenerateTestEmail(),
Username: "user_" + model.NewId(),
Password: "password",
})
require.Nil(t, appErr)
// Create a team and add the user to it
team, appErr := th.App.CreateTeam(th.Context, &model.Team{
DisplayName: "Team for testing",
Name: "test-team-" + model.NewId(),
Email: th.GenerateTestEmail(),
Type: model.TeamOpen,
})
require.Nil(t, appErr)
th.LinkUserToTeam(user, team)
// Create a client and log in
client := th.CreateClient()
_, _, err := client.Login(context.Background(), user.Email, "password")
require.NoError(t, err)
// Get categories to have valid category IDs
categories, _, err := client.GetSidebarCategoriesForTeamForUser(context.Background(), user.Id, team.Id, "")
require.NoError(t, err)
require.NotEmpty(t, categories.Categories)
// Store a category ID to use after team membership is revoked
categoryID := categories.Categories[0].Id
// Remove the user from the team
appErr = th.App.RemoveUserFromTeam(th.Context, team.Id, user.Id, th.SystemAdminUser.Id)
require.Nil(t, appErr)
// Attempt to get a category for the team after being removed
_, resp, err := client.GetSidebarCategoryForTeamForUser(context.Background(), user.Id, team.Id, categoryID, "")
require.Error(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
})
}
func TestValidateSidebarCategory(t *testing.T) {
@@ -1259,6 +1558,53 @@ func TestDeleteCategoryForTeamForUser(t *testing.T) {
}
}
})
t.Run("should return error when user tries to delete a category for a team they're not a member of", func(t *testing.T) {
// Create a user
user, appErr := th.App.CreateUser(th.Context, &model.User{
Email: th.GenerateTestEmail(),
Username: "user_" + model.NewId(),
Password: "password",
})
require.Nil(t, appErr)
// Create a team and add the user to it
team, appErr := th.App.CreateTeam(th.Context, &model.Team{
DisplayName: "Team for testing",
Name: "test-team-" + model.NewId(),
Email: th.GenerateTestEmail(),
Type: model.TeamOpen,
})
require.Nil(t, appErr)
th.LinkUserToTeam(user, team)
// Create a client and log in
client := th.CreateClient()
_, _, err := client.Login(context.Background(), user.Email, "password")
require.NoError(t, err)
// Create a custom category
customCategory, _, err := client.CreateSidebarCategoryForTeamForUser(context.Background(), user.Id, team.Id, &model.SidebarCategoryWithChannels{
SidebarCategory: model.SidebarCategory{
UserId: user.Id,
TeamId: team.Id,
DisplayName: "Custom Category",
Type: model.SidebarCategoryCustom,
},
})
require.NoError(t, err)
require.NotNil(t, customCategory)
// Remove the user from the team
appErr = th.App.RemoveUserFromTeam(th.Context, team.Id, user.Id, th.SystemAdminUser.Id)
require.Nil(t, appErr)
// Attempt to delete the category for the team after being removed
resp, err := client.DeleteSidebarCategoryForTeamForUser(context.Background(), user.Id, team.Id, customCategory.Id)
require.Error(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
})
}
func setupUserForSubtest(t *testing.T, th *TestHelper) (*model.User, *model.Client4) {