correct ListRoles to ensure it always lists roles from the database

Signed-off-by: Mike Mason <[email protected]>
infratographer · Jan 18, 2024 · f2d105f · f2d105f
1 parent 1c56883
commit f2d105f
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 16 deletions.
diff --git a/internal/query/relations.go b/internal/query/relations.go
@@ -844,6 +844,11 @@ func (e *engine) ListRelationshipsTo(ctx context.Context, resource types.Resourc
 
 // ListRoles returns all roles bound to a given resource.
 func (e *engine) ListRoles(ctx context.Context, resource types.Resource) ([]types.Role, error) {
+	dbRoles, err := e.store.ListResourceRoles(ctx, resource.ID)
+	if err != nil {
+		return nil, err
+	}
+
 	resType := e.namespace + "/" + resource.Type
 	roleType := e.namespace + "/role"
 
@@ -858,32 +863,33 @@ func (e *engine) ListRoles(ctx context.Context, resource types.Resource) ([]type
 		},
 	}
 
-	dbRoles, err := e.store.ListResourceRoles(ctx, resource.ID)
-	if err != nil {
-		return nil, err
-	}
-
 	relationships, err := e.readRelationships(ctx, filter)
 	if err != nil {
 		return nil, err
 	}
 
-	out := relationshipsToRoles(relationships)
+	spicedbRoles := relationshipsToRoles(relationships)
+
+	rolesByID := make(map[gidx.PrefixedID]types.Role, len(spicedbRoles))
 
-	rolesByID := make(map[gidx.PrefixedID]storage.Role, len(dbRoles))
-	for _, role := range dbRoles {
+	for _, role := range spicedbRoles {
 		rolesByID[role.ID] = role
 	}
 
-	for i, role := range out {
-		if dbRole, ok := rolesByID[role.ID]; ok {
-			role.Name = dbRole.Name
-			role.CreatedBy = dbRole.CreatedBy
-			role.UpdatedBy = dbRole.UpdatedBy
-			role.CreatedAt = dbRole.CreatedAt
-			role.UpdatedAt = dbRole.UpdatedAt
+	out := make([]types.Role, len(dbRoles))
 
-			out[i] = role
+	for i, dbRole := range dbRoles {
+		spicedbRole := rolesByID[dbRole.ID]
+
+		out[i] = types.Role{
+			ID:         dbRole.ID,
+			Name:       dbRole.Name,
+			Actions:    spicedbRole.Actions,
+			ResourceID: dbRole.ResourceID,
+			CreatedBy:  dbRole.CreatedBy,
+			UpdatedBy:  dbRole.UpdatedBy,
+			CreatedAt:  dbRole.CreatedAt,
+			UpdatedAt:  dbRole.UpdatedAt,
 		}
 	}
 

diff --git a/internal/query/relations_test.go b/internal/query/relations_test.go
@@ -289,6 +289,91 @@ func TestRoleUpdate(t *testing.T) {
 	testingx.RunTests(ctx, t, testCases, testFn)
 }
 
+func TestListRoles(t *testing.T) {
+	namespace := "testroles"
+	ctx := context.Background()
+	e := testEngine(ctx, t, namespace)
+
+	actorRes, err := e.NewResourceFromID(gidx.MustNewID("idntusr"))
+	require.NoError(t, err)
+
+	type (
+		tenCtxKey  struct{}
+		roleCtxKey struct{}
+	)
+
+	var (
+		tenCtx  tenCtxKey
+		roleCtx roleCtxKey
+	)
+
+	testCases := []testingx.TestCase[any, []types.Role]{
+		{
+			Name: "RoleFoundWithActions",
+			SetupFn: func(ctx context.Context, t *testing.T) context.Context {
+				tenID, err := gidx.NewID("tnntten")
+				require.NoError(t, err)
+
+				tenRes, err := e.NewResourceFromID(tenID)
+				require.NoError(t, err)
+
+				role, err := e.CreateRole(ctx, actorRes, tenRes, t.Name(), []string{"loadbalancer_get"})
+				require.NoError(t, err)
+				require.NotEmpty(t, role.ID)
+
+				ctx = context.WithValue(ctx, tenCtx, tenRes)
+				ctx = context.WithValue(ctx, roleCtx, role)
+
+				return ctx
+			},
+			CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.Role]) {
+				assert.NoError(t, res.Err)
+				require.NotEmpty(t, res.Success)
+				assert.Equal(t, ctx.Value(roleCtx), res.Success[0])
+				assert.NotEmpty(t, res.Success[0].Actions)
+			},
+		},
+		{
+			Name: "RoleFoundWithoutActions",
+			SetupFn: func(ctx context.Context, t *testing.T) context.Context {
+				tenID, err := gidx.NewID("tnntten")
+				require.NoError(t, err)
+
+				tenRes, err := e.NewResourceFromID(tenID)
+				require.NoError(t, err)
+
+				role, err := e.CreateRole(ctx, actorRes, tenRes, t.Name(), nil)
+				require.NoError(t, err)
+				require.NotEmpty(t, role.ID)
+
+				ctx = context.WithValue(ctx, tenCtx, tenRes)
+				ctx = context.WithValue(ctx, roleCtx, role)
+
+				return ctx
+			},
+			CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.Role]) {
+				assert.NoError(t, res.Err)
+				require.NotEmpty(t, res.Success)
+				assert.Equal(t, ctx.Value(roleCtx), res.Success[0])
+				assert.Empty(t, res.Success[0].Actions)
+			},
+		},
+	}
+
+	testFn := func(ctx context.Context, _ any) testingx.TestResult[[]types.Role] {
+		tenRes := ctx.Value(tenCtx).(types.Resource)
+
+		roles, err := e.ListRoles(ctx, tenRes)
+
+		return testingx.TestResult[[]types.Role]{
+			Success: roles,
+			Err:     err,
+		}
+	}
+
+	testingx.RunTests(ctx, t, testCases, testFn)
+}
+
 func TestRoleDelete(t *testing.T) {
 	namespace := "testroles"
 	ctx := context.Background()