From 49178bf480158923ce2bcc511de001b9bb6b4bd1 Mon Sep 17 00:00:00 2001
From: Martin Kraft <martinkraft@gmail.com>
Date: Tue, 27 Apr 2021 12:36:05 -0400
Subject: [PATCH] MM-24455: Fix role responses. (#17385)

Automatic Merge
---
 app/app_iface.go                     |  1 -
 app/opentracing/opentracing_layer.go | 22 ----------------------
 app/permissions_migrations.go        |  2 +-
 app/role.go                          | 12 ++++--------
 app/role_test.go                     | 10 ++++++++++
 i18n/en.json                         |  4 ----
 6 files changed, 15 insertions(+), 36 deletions(-)

diff --git a/app/app_iface.go b/app/app_iface.go
index 8cc62b92593..d1c6f9145ac 100644
--- a/app/app_iface.go
+++ b/app/app_iface.go
@@ -541,7 +541,6 @@ 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 e09bf10d33a..ffcc729a7b5 100644
--- a/app/opentracing/opentracing_layer.go
+++ b/app/opentracing/opentracing_layer.go
@@ -4412,28 +4412,6 @@ 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/permissions_migrations.go b/app/permissions_migrations.go
index 36c8860e8f5..d73d352fc48 100644
--- a/app/permissions_migrations.go
+++ b/app/permissions_migrations.go
@@ -924,7 +924,7 @@ func (a *App) DoPermissionsMigrations() error {
 		{Key: model.MIGRATION_KEY_ADD_REPORTING_SUBSECTION_PERMISSIONS, Migration: a.getAddReportingSubsectionPermissions},
 	}
 
-	roles, err := a.GetAllRoles()
+	roles, err := a.srv.Store.Role().GetAll()
 	if err != nil {
 		return err
 	}
diff --git a/app/role.go b/app/role.go
index da1269b3795..d6640150d6e 100644
--- a/app/role.go
+++ b/app/role.go
@@ -27,16 +27,12 @@ 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([]*model.Role{role})
+	if appErr != nil {
+		return nil, appErr
 	}
 
-	return roles, nil
+	return role, nil
 }
 
 func (s *Server) GetRoleByName(ctx context.Context, name string) (*model.Role, *model.AppError) {
diff --git a/app/role_test.go b/app/role_test.go
index 23f339a0728..b3cae19515f 100644
--- a/app/role_test.go
+++ b/app/role_test.go
@@ -51,6 +51,16 @@ func TestGetRoleByName(t *testing.T) {
 	})
 }
 
+func TestGetRoleByID(t *testing.T) {
+	testPermissionInheritance(t, func(t *testing.T, th *TestHelper, testData permissionInheritanceTestData) {
+		actualRole, err := th.App.GetRole(testData.channelRole.Id)
+		require.Nil(t, err)
+		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 2b17c3bfbaf..53b4dc46244 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -5938,10 +5938,6 @@
     "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."