From e70c5a605a46f09970b8e07067fdcad32bd61305 Mon Sep 17 00:00:00 2001 From: Berke Kalkan <42390512+berkeka@users.noreply.github.com> Date: Mon, 8 Nov 2021 16:38:41 +0300 Subject: [PATCH] MM-32396 Create endpoint for listing all the roles (#16988) * Create an endpoint for listing roles * Add function to client model * Create tests for listing roles endpoint * Apply code review suggestions * Restore GetAllRoles app method * Use AppContext instead of App * Minor fix * Refactor according to new changes Co-authored-by: Mattermod --- api4/role.go | 22 ++++++++++++++++++++++ api4/role_local.go | 1 + api4/role_test.go | 22 ++++++++++++++++++++++ app/app_iface.go | 1 + app/opentracing/opentracing_layer.go | 22 ++++++++++++++++++++++ app/role.go | 14 ++++++++++++++ app/role_test.go | 14 ++++++++++++++ i18n/en.json | 4 ++++ model/client4.go | 14 ++++++++++++++ 9 files changed, 114 insertions(+) diff --git a/api4/role.go b/api4/role.go index d579ba8330..c004f22402 100644 --- a/api4/role.go +++ b/api4/role.go @@ -19,12 +19,34 @@ var notAllowedPermissions = []string{ } func (api *API) InitRole() { + api.BaseRoutes.Roles.Handle("", api.APISessionRequired(getAllRoles)).Methods("GET") api.BaseRoutes.Roles.Handle("/{role_id:[A-Za-z0-9]+}", api.APISessionRequiredTrustRequester(getRole)).Methods("GET") api.BaseRoutes.Roles.Handle("/name/{role_name:[a-z0-9_]+}", api.APISessionRequiredTrustRequester(getRoleByName)).Methods("GET") api.BaseRoutes.Roles.Handle("/names", api.APISessionRequiredTrustRequester(getRolesByNames)).Methods("POST") api.BaseRoutes.Roles.Handle("/{role_id:[A-Za-z0-9]+}/patch", api.APISessionRequired(patchRole)).Methods("PUT") } +func getAllRoles(c *Context, w http.ResponseWriter, r *http.Request) { + if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { + c.SetPermissionError(model.PermissionManageSystem) + return + } + + roles, err := c.App.GetAllRoles() + if err != nil { + c.Err = err + return + } + + js, jsonErr := json.Marshal(roles) + if jsonErr != nil { + c.Err = model.NewAppError("getAllRoles", "api.marshal_error", nil, jsonErr.Error(), http.StatusInternalServerError) + return + } + + w.Write(js) +} + func getRole(c *Context, w http.ResponseWriter, r *http.Request) { c.RequireRoleId() if c.Err != nil { diff --git a/api4/role_local.go b/api4/role_local.go index 9a2bdcb806..d23e1132bd 100644 --- a/api4/role_local.go +++ b/api4/role_local.go @@ -4,6 +4,7 @@ package api4 func (api *API) InitRoleLocal() { + api.BaseRoutes.Roles.Handle("", api.APILocal(getAllRoles)).Methods("GET") api.BaseRoutes.Roles.Handle("/{role_id:[A-Za-z0-9]+}", api.APILocal(getRole)).Methods("GET") api.BaseRoutes.Roles.Handle("/name/{role_name:[a-z0-9_]+}", api.APILocal(getRoleByName)).Methods("GET") api.BaseRoutes.Roles.Handle("/names", api.APILocal(getRolesByNames)).Methods("POST") diff --git a/api4/role_test.go b/api4/role_test.go index 7fb8930999..80cbcd5221 100644 --- a/api4/role_test.go +++ b/api4/role_test.go @@ -15,6 +15,28 @@ import ( "github.com/mattermost/mattermost-server/v6/model" ) +func TestGetAllRoles(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + roles, err := th.App.Srv().Store.Role().GetAll() + require.NoError(t, err) + + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + received, resp, err := client.GetAllRoles() + require.NoError(t, err) + CheckOKStatus(t, resp) + + assert.EqualValues(t, received, roles) + }) + + t.Run("NormalClient", func(t *testing.T) { + _, resp, err := th.Client.GetAllRoles() + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) +} + func TestGetRole(t *testing.T) { th := Setup(t) defer th.TearDown() diff --git a/app/app_iface.go b/app/app_iface.go index 4e9a267482..4e822e271d 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -543,6 +543,7 @@ type AppIface interface { GetAllPrivateTeams() ([]*model.Team, *model.AppError) GetAllPublicTeams() ([]*model.Team, *model.AppError) GetAllRemoteClusters(filter model.RemoteClusterQueryFilter) ([]*model.RemoteCluster, *model.AppError) + GetAllRoles() ([]*model.Role, *model.AppError) GetAllStatuses() map[string]*model.Status GetAllTeams() ([]*model.Team, *model.AppError) GetAllTeamsPage(offset int, limit int, opts *model.TeamSearch) ([]*model.Team, *model.AppError) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index d627da6ec5..c7024708b4 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -4369,6 +4369,28 @@ func (a *OpenTracingAppLayer) GetAllRemoteClusters(filter model.RemoteClusterQue return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) GetAllRoles() ([]*model.Role, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetAllRoles") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.GetAllRoles() + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) GetAllStatuses() map[string]*model.Status { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetAllStatuses") diff --git a/app/role.go b/app/role.go index 51633c1c80..846d9d83f7 100644 --- a/app/role.go +++ b/app/role.go @@ -37,6 +37,20 @@ func (a *App) GetRole(id string) (*model.Role, *model.AppError) { return role, nil } +func (a *App) GetAllRoles() ([]*model.Role, *model.AppError) { + roles, err := a.Srv().Store.Role().GetAll() + if err != nil { + return nil, model.NewAppError("GetAllRoles", "app.role.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + appErr := a.Srv().mergeChannelHigherScopedPermissions(roles) + if appErr != nil { + return nil, appErr + } + + return roles, nil +} + func (s *Server) GetRoleByName(ctx context.Context, name string) (*model.Role, *model.AppError) { role, nErr := s.Store.Role().GetByName(ctx, name) if nErr != nil { diff --git a/app/role_test.go b/app/role_test.go index 0a184be28e..5333081327 100644 --- a/app/role_test.go +++ b/app/role_test.go @@ -61,6 +61,20 @@ func TestGetRoleByID(t *testing.T) { }) } +func TestGetAllRoles(t *testing.T) { + testPermissionInheritance(t, func(t *testing.T, th *TestHelper, testData permissionInheritanceTestData) { + actualRoles, err := th.App.GetAllRoles() + require.Nil(t, err) + for _, actualRole := range actualRoles { + if actualRole.Id == testData.channelRole.Id { + require.NotNil(t, actualRole) + require.Equal(t, testData.channelRole.Id, actualRole.Id) + require.Equal(t, testData.shouldHavePermission, utils.StringInSlice(testData.permission.Id, actualRole.Permissions), "row: %+v", testData.truthTableRow) + } + } + }) +} + // testPermissionInheritance tests 48 combinations of scheme, permission, role data. func testPermissionInheritance(t *testing.T, testCallback func(t *testing.T, th *TestHelper, testData permissionInheritanceTestData)) { th := Setup(t).InitBasic() diff --git a/i18n/en.json b/i18n/en.json index ac63300375..9351d1a669 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -5915,6 +5915,10 @@ "id": "app.role.get.app_error", "translation": "Unable to get role." }, + { + "id": "app.role.get_all.app_error", + "translation": "Unable to get all the roles." + }, { "id": "app.role.get_by_name.app_error", "translation": "Unable to get role." diff --git a/model/client4.go b/model/client4.go index 2fa88cd679..51ad590736 100644 --- a/model/client4.go +++ b/model/client4.go @@ -6483,6 +6483,20 @@ func (c *Client4) DownloadJob(jobId string) ([]byte, *Response, error) { // Roles Section +// GetAllRoles returns a list of all the roles. +func (c *Client4) GetAllRoles() ([]*Role, *Response, error) { + r, err := c.DoAPIGet(c.rolesRoute(), "") + if err != nil { + return nil, BuildResponse(r), err + } + defer closeBody(r) + var list []*Role + if jsonErr := json.NewDecoder(r.Body).Decode(&list); jsonErr != nil { + return nil, nil, NewAppError("GetAllRoles", "api.unmarshal_error", nil, jsonErr.Error(), http.StatusInternalServerError) + } + return list, BuildResponse(r), nil +} + // GetRole gets a single role by ID. func (c *Client4) GetRole(id string) (*Role, *Response, error) { r, err := c.DoAPIGet(c.rolesRoute()+fmt.Sprintf("/%v", id), "")