Преглед изворни кода

security: Actions that people didnt have permission to view were being returned (#921)

jamesread пре 3 месеци
родитељ
комит
71bb999950

+ 67 - 0
service/internal/api/api_test.go

@@ -432,6 +432,73 @@ func TestViewPermissionAllowedSeesAction(t *testing.T) {
 	assert.Equal(t, "secret_action", resp.Action.BindingId)
 }
 
+// TestViewPermissionExcludedFromCustomDashboard (issue #921) asserts that when a custom dashboard
+// lists an action by title, users without view permission do not see that action (title or icon).
+func TestViewPermissionExcludedFromCustomDashboard(t *testing.T) {
+	cfg, lowUser, _ := buildViewPermissionTestConfig(t)
+	cfg.Dashboards = []*config.DashboardComponent{
+		{
+			Title: "Custom",
+			Contents: []*config.DashboardComponent{
+				{Title: "Secret Action"},
+			},
+		},
+	}
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+
+	rr := &DashboardRenderRequest{
+		AuthenticatedUser: lowUser,
+		cfg:               cfg,
+		ex:                ex,
+	}
+	dashboard := findDashboardByTitle(rr, "Custom")
+	require.NotNil(t, dashboard)
+	db := buildDashboardFromConfig(dashboard, rr)
+	require.NotNil(t, db)
+
+	bindingIdsInDashboard := bindingIdsInDashboardContents(db.Contents)
+	assert.NotContains(t, bindingIdsInDashboard, "secret_action",
+		"user with view:false must not see action on custom dashboard; got bindingIds: %v", bindingIdsInDashboard)
+}
+
+// TestViewPermissionExcludedFromEntityDashboard (GHSA: view permission) asserts that when a dashboard
+// has an entity fieldset listing an action, users without view permission do not see that action.
+func TestViewPermissionExcludedFromEntityDashboard(t *testing.T) {
+	entities.ClearEntitiesOfType("vp_entity_test")
+	defer entities.ClearEntitiesOfType("vp_entity_test")
+	entities.AddEntity("vp_entity_test", "1", map[string]any{"title": "Test Entity"})
+
+	cfg, lowUser, _ := buildViewPermissionTestConfig(t)
+	cfg.Dashboards = []*config.DashboardComponent{
+		{
+			Title: "WithEntity",
+			Contents: []*config.DashboardComponent{
+				{
+					Title: "Servers", Type: "fieldset", Entity: "vp_entity_test",
+					Contents: []*config.DashboardComponent{{Title: "Secret Action"}},
+				},
+			},
+		},
+	}
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+
+	rr := &DashboardRenderRequest{
+		AuthenticatedUser: lowUser,
+		cfg:               cfg,
+		ex:                ex,
+	}
+	dashboard := findDashboardByTitle(rr, "WithEntity")
+	require.NotNil(t, dashboard)
+	db := buildDashboardFromConfig(dashboard, rr)
+	require.NotNil(t, db)
+
+	bindingIdsInDashboard := bindingIdsInDashboardContents(db.Contents)
+	assert.NotContains(t, bindingIdsInDashboard, "secret_action",
+		"user with view:false must not see action in entity fieldset; got bindingIds: %v", bindingIdsInDashboard)
+}
+
 func bindingIdsInDashboardContents(contents []*apiv1.DashboardComponent) []string {
 	var ids []string
 	for _, c := range contents {

+ 5 - 3
service/internal/api/dashboard_entities.go

@@ -82,8 +82,6 @@ func isLinkType(itemType string) bool {
 }
 
 func cloneLinkItem(subitem *config.DashboardComponent, ent *entities.Entity, clone *apiv1.DashboardComponent, rr *DashboardRenderRequest) *apiv1.DashboardComponent {
-	clone.Type = "link"
-	clone.Title = tpl.ParseTemplateOfActionBeforeExec(subitem.Title, ent)
 	// Prefer an entity-specific action when available, but fall back to a
 	// non-entity-scoped action with the same title. This allows inline actions
 	// defined inside entity dashboards to work without requiring an explicit
@@ -92,7 +90,11 @@ func cloneLinkItem(subitem *config.DashboardComponent, ent *entities.Entity, clo
 	if action == nil {
 		action = rr.findAction(subitem.Title)
 	}
-
+	if action == nil {
+		return nil
+	}
+	clone.Type = "link"
+	clone.Title = tpl.ParseTemplateOfActionBeforeExec(subitem.Title, ent)
 	clone.Action = action
 	return clone
 }

+ 17 - 2
service/internal/api/dashboards.go

@@ -277,16 +277,31 @@ func createRootFieldset() *apiv1.DashboardComponent {
 	}
 }
 
+func appendComponentIfNotNil(components *[]*apiv1.DashboardComponent, comp *apiv1.DashboardComponent) {
+	if comp != nil {
+		*components = append(*components, comp)
+	}
+}
+
+func getDashboardComponentOrNil(subitem *config.DashboardComponent, rr *DashboardRenderRequest, entity *entities.Entity) *apiv1.DashboardComponent {
+	if len(subitem.Contents) == 0 && rr.findActionForEntity(subitem.Title, entity) == nil {
+		if !isAllowedType(subitem.Type) {
+			return nil
+		}
+	}
+	return buildDashboardComponentSimpleWithEntity(subitem, rr, entity)
+}
+
 func processDashboardSubitemWithEntity(subitem *config.DashboardComponent, rr *DashboardRenderRequest, ret *[]*apiv1.DashboardComponent, rootFieldset *apiv1.DashboardComponent, entity *entities.Entity) {
 	if subitem.Type != "fieldset" {
-		rootFieldset.Contents = append(rootFieldset.Contents, buildDashboardComponentSimpleWithEntity(subitem, rr, entity))
+		appendComponentIfNotNil(&rootFieldset.Contents, getDashboardComponentOrNil(subitem, rr, entity))
 		return
 	}
 
 	if subitem.Entity != "" {
 		*ret = append(*ret, buildEntityFieldsets(subitem.Entity, subitem, rr)...)
 	} else {
-		*ret = append(*ret, buildDashboardComponentSimpleWithEntity(subitem, rr, entity))
+		appendComponentIfNotNil(ret, getDashboardComponentOrNil(subitem, rr, entity))
 	}
 }