Просмотр исходного кода

fix: Dashboards now can have ACLs (#445)

jamesread 1 месяц назад
Родитель
Сommit
c72d180fae

+ 43 - 2
docs/modules/ROOT/pages/security/acl.adoc

@@ -74,7 +74,7 @@ defaultPermissions:
   logs: true
 ----
 
-In the example above, all users will start off with the permissions to only see action logs - but will not be able to view or execute actions. 
+In the example above, all users will start off with the permissions to only see action logs - but will not be able to view or execute actions.
 
 It is then possible to add an "admins" ACL on top of every action. In the example below, we define one extra ACL called "admins", which matches any users with the usergroup also called "admins". This ACL will then be applied to all actions, and will allow users in the "admins" usergroup to view and execute the action.
 
@@ -116,6 +116,48 @@ accessControlLists:
     addToEveryAction: true
 ```
 
+== ACLs and Dashboards
+
+Root dashboards can also list `acls`. This controls whether the **whole dashboard page** is visible (including `display` widgets and entity fieldsets), not just action buttons.
+
+* If a dashboard has **no** `acls` (or an empty list), it is unrestricted — anyone can see it in the side menu (subject to the usual “empty dashboard” hiding).
+* If a dashboard lists one or more `acls`, access uses the same allow-list rules as actions: a matching ACL that grants `view`, otherwise `defaultPermissions.view`.
+* `addToEveryAction` does **not** apply to dashboards. List the ACL on the dashboard explicitly when you want to restrict it.
+* Nested fieldsets and directories do not have their own `acls`; the root dashboard decision covers the whole page.
+
+Action `acls` still control individual buttons. Use dashboard `acls` when you need to hide a page that contains status or other non-action content from some users.
+
+[source,yaml]
+.`config.yaml`
+----
+defaultPermissions:
+  view: false
+  exec: false
+
+accessControlLists:
+  - name: admins
+    matchUsergroups:
+      - admins
+    permissions:
+      view: true
+      exec: true
+
+dashboards:
+  - title: Public tools
+    contents:
+      - title: Welcome
+        type: display
+
+  - title: Services
+    acls:
+      - admins
+    contents:
+      - title: 'Status: running'
+        type: display
+----
+
+In the example above, guests can open **Public tools**, but **Services** is hidden from the side menu and cannot be loaded by deep link.
+
 == ACL Matching - usernames and usergroups.
 
 You can match users based on their usergroup which is the most common, but it is also possible to match based on the user's username.
@@ -149,4 +191,3 @@ Now that you understand ACLs, here's how to implement them:
 * xref:security/local.adoc[Set up local users] - Create users for ACL matching
 * xref:security/oauth2.adoc[Configure OAuth2] - Set up OAuth2 for user groups
 * xref:security/design_choices.adoc[Security design recommendations] - Learn best practices for ACL design
-

+ 33 - 0
integration-tests/tests/dashboardAcls/config.yaml

@@ -0,0 +1,33 @@
+#
+# Integration Test Config: dashboardAcls
+#
+
+listenAddressSingleHTTPFrontend: 0.0.0.0:1337
+
+logLevel: "DEBUG"
+checkForUpdates: false
+
+defaultPermissions:
+  view: false
+  exec: false
+
+accessControlLists:
+  - name: admins
+    matchUsergroups:
+      - admins
+    permissions:
+      view: true
+      exec: true
+
+dashboards:
+  - title: Public tools
+    contents:
+      - title: Welcome guest
+        type: display
+
+  - title: Services
+    acls:
+      - admins
+    contents:
+      - title: 'Status: running'
+        type: display

+ 38 - 0
integration-tests/tests/dashboardAcls/dashboardAcls.mjs

@@ -0,0 +1,38 @@
+import { describe, it, before, after } from 'mocha'
+import { expect } from 'chai'
+import {
+  getRootAndWait,
+  openSidebar,
+  getNavigationLinks,
+  takeScreenshotOnFailure,
+} from '../../lib/elements.js'
+
+describe('config: dashboardAcls', function () {
+  before(async function () {
+    await runner.start('dashboardAcls')
+  })
+
+  after(async () => {
+    await runner.stop()
+  })
+
+  afterEach(function () {
+    takeScreenshotOnFailure(this.currentTest, webdriver)
+  })
+
+  it('hides ACL-restricted dashboards from guests in the side menu', async function () {
+    await getRootAndWait()
+    await openSidebar()
+
+    const navigationLinks = await getNavigationLinks()
+    expect(navigationLinks).to.not.be.empty
+
+    const linkTexts = []
+    for (const link of navigationLinks) {
+      linkTexts.push(await link.getText())
+    }
+
+    expect(linkTexts).to.include('Public tools')
+    expect(linkTexts).to.not.include('Services')
+  })
+})

+ 40 - 38
service/internal/acl/acl.go

@@ -21,38 +21,36 @@ func (p PermissionBits) Has(permission PermissionBits) bool {
 	return p&permission != 0
 }
 
-func logAclNotMatched(cfg *config.Config, aclFunction string, user *authpublic.AuthenticatedUser, action *config.Action, acl *config.AccessControlList) {
+func logAclNotMatched(cfg *config.Config, aclFunction string, user *authpublic.AuthenticatedUser, resourceTitle string, acl *config.AccessControlList) {
 	if cfg.LogDebugOptions.AclNotMatched {
 		log.WithFields(log.Fields{
-			"User":   user.Username,
-			"Action": action.Title,
-			"ACL":    acl.Name,
+			"User":     user.Username,
+			"Resource": resourceTitle,
+			"ACL":      acl.Name,
 		}).Debugf("%v - ACL Not Matched", aclFunction)
 	}
 }
 
-func logAclMatched(cfg *config.Config, aclFunction string, user *authpublic.AuthenticatedUser, action *config.Action, acl *config.AccessControlList) {
-	actionTitle := "N/A"
-
-	if action != nil {
-		actionTitle = action.Title
+func logAclMatched(cfg *config.Config, aclFunction string, user *authpublic.AuthenticatedUser, resourceTitle string, acl *config.AccessControlList) {
+	if resourceTitle == "" {
+		resourceTitle = "N/A"
 	}
 
 	if cfg.LogDebugOptions.AclMatched {
 		log.WithFields(log.Fields{
-			"User":   user.Username,
-			"Action": actionTitle,
-			"ACL":    acl.Name,
+			"User":     user.Username,
+			"Resource": resourceTitle,
+			"ACL":      acl.Name,
 		}).Debugf("%v - Matched ACL", aclFunction)
 	}
 }
 
-func logAclNoneMatched(cfg *config.Config, aclFunction string, user *authpublic.AuthenticatedUser, action *config.Action, defaultPermission bool) {
+func logAclNoneMatched(cfg *config.Config, aclFunction string, user *authpublic.AuthenticatedUser, resourceTitle string, defaultPermission bool) {
 	if cfg.LogDebugOptions.AclNoneMatched {
 		log.WithFields(log.Fields{
-			"User":    user.Username,
-			"Action":  action.Title,
-			"Default": defaultPermission,
+			"User":     user.Username,
+			"Resource": resourceTitle,
+			"Default":  defaultPermission,
 		}).Debugf("%v - No ACLs Matched, returning default permission", aclFunction)
 	}
 }
@@ -81,12 +79,12 @@ func permissionsConfigToBits(permissions config.PermissionsList) PermissionBits
 	return ret
 }
 
-func aclCheck(requiredPermission PermissionBits, defaultValue bool, cfg *config.Config, aclFunction string, user *authpublic.AuthenticatedUser, action *config.Action) bool {
-	relevantAcls := getRelevantAcls(cfg, action.Acls, user)
+func aclCheck(requiredPermission PermissionBits, defaultValue bool, cfg *config.Config, aclFunction string, user *authpublic.AuthenticatedUser, resourceTitle string, resourceAcls []string, includeAddToEvery bool) bool {
+	relevantAcls := getRelevantAcls(cfg, resourceAcls, user, includeAddToEvery)
 
 	if cfg.LogDebugOptions.AclCheckStarted {
 		log.WithFields(log.Fields{
-			"actionTitle":        action.Title,
+			"resourceTitle":      resourceTitle,
 			"username":           user.Username,
 			"usergroupLine":      user.UsergroupLine,
 			"relevantAcls":       len(relevantAcls),
@@ -98,27 +96,27 @@ func aclCheck(requiredPermission PermissionBits, defaultValue bool, cfg *config.
 		permissionBits := permissionsConfigToBits(acl.Permissions)
 
 		if permissionBits.Has(requiredPermission) {
-			logAclMatched(cfg, aclFunction, user, action, acl)
+			logAclMatched(cfg, aclFunction, user, resourceTitle, acl)
 
 			return true
-		} else {
-			logAclNotMatched(cfg, aclFunction, user, action, acl)
 		}
+
+		logAclNotMatched(cfg, aclFunction, user, resourceTitle, acl)
 	}
 
-	logAclNoneMatched(cfg, aclFunction, user, action, cfg.DefaultPermissions.Logs)
+	logAclNoneMatched(cfg, aclFunction, user, resourceTitle, defaultValue)
 
 	return defaultValue
 }
 
 // IsAllowedLogs checks if a AuthenticatedUser is allowed to view an action's logs
 func IsAllowedLogs(cfg *config.Config, user *authpublic.AuthenticatedUser, action *config.Action) bool {
-	return aclCheck(Logs, cfg.DefaultPermissions.Logs, cfg, "isAllowedLogs", user, action)
+	return aclCheck(Logs, cfg.DefaultPermissions.Logs, cfg, "isAllowedLogs", user, action.Title, action.Acls, true)
 }
 
 // IsAllowedExec checks if a AuthenticatedUser is allowed to execute an Action
 func IsAllowedExec(cfg *config.Config, user *authpublic.AuthenticatedUser, action *config.Action) bool {
-	return aclCheck(Exec, cfg.DefaultPermissions.Exec, cfg, "isAllowedExec", user, action)
+	return aclCheck(Exec, cfg.DefaultPermissions.Exec, cfg, "isAllowedExec", user, action.Title, action.Acls, true)
 }
 
 // IsAllowedView checks if a User is allowed to view an Action
@@ -127,36 +125,40 @@ func IsAllowedView(cfg *config.Config, user *authpublic.AuthenticatedUser, actio
 		return false
 	}
 
-	return aclCheck(View, cfg.DefaultPermissions.View, cfg, "isAllowedView", user, action)
+	return aclCheck(View, cfg.DefaultPermissions.View, cfg, "isAllowedView", user, action.Title, action.Acls, true)
 }
 
 func IsAllowedKill(cfg *config.Config, user *authpublic.AuthenticatedUser, action *config.Action) bool {
-	return aclCheck(Kill, cfg.DefaultPermissions.Kill, cfg, "isAllowedKill", user, action)
+	return aclCheck(Kill, cfg.DefaultPermissions.Kill, cfg, "isAllowedKill", user, action.Title, action.Acls, true)
 }
 
-func isACLRelevantToAction(actionAcls []string, acl *config.AccessControlList, user *authpublic.AuthenticatedUser) bool {
-	if !slices.Contains(user.Acls, acl.Name) {
-		// If the user does not have this ACL, then it is not relevant
-
-		return false
+// IsAllowedViewDashboard checks if a user may see a root dashboard.
+// Dashboards with no acls are unrestricted. AddToEveryAction does not apply.
+func IsAllowedViewDashboard(cfg *config.Config, user *authpublic.AuthenticatedUser, dashboard *config.DashboardComponent) bool {
+	if dashboard == nil || len(dashboard.Acls) == 0 {
+		return true
 	}
 
-	if acl.AddToEveryAction {
-		return true
+	return aclCheck(View, cfg.DefaultPermissions.View, cfg, "isAllowedViewDashboard", user, dashboard.Title, dashboard.Acls, false)
+}
+
+func isACLRelevant(resourceAcls []string, acl *config.AccessControlList, user *authpublic.AuthenticatedUser, includeAddToEvery bool) bool {
+	if !slices.Contains(user.Acls, acl.Name) {
+		return false
 	}
 
-	if slices.Contains(actionAcls, acl.Name) {
+	if includeAddToEvery && acl.AddToEveryAction {
 		return true
 	}
 
-	return false
+	return slices.Contains(resourceAcls, acl.Name)
 }
 
-func getRelevantAcls(cfg *config.Config, actionAcls []string, user *authpublic.AuthenticatedUser) []*config.AccessControlList {
+func getRelevantAcls(cfg *config.Config, resourceAcls []string, user *authpublic.AuthenticatedUser, includeAddToEvery bool) []*config.AccessControlList {
 	var ret []*config.AccessControlList
 
 	for _, acl := range cfg.AccessControlLists {
-		if isACLRelevantToAction(actionAcls, acl, user) {
+		if isACLRelevant(resourceAcls, acl, user, includeAddToEvery) {
 			ret = append(ret, acl)
 		}
 	}

+ 86 - 0
service/internal/acl/acl_dashboard_test.go

@@ -0,0 +1,86 @@
+package acl
+
+import (
+	"testing"
+
+	authpublic "github.com/OliveTin/OliveTin/internal/auth/authpublic"
+	config "github.com/OliveTin/OliveTin/internal/config"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsAllowedViewDashboardAbsentAclsUnrestricted(t *testing.T) {
+	cfg := config.DefaultConfig()
+	cfg.DefaultPermissions.View = false
+
+	dashboard := &config.DashboardComponent{
+		Title: "Public",
+		Contents: []*config.DashboardComponent{
+			{Title: "Status", Type: "display"},
+		},
+	}
+
+	guest := &authpublic.AuthenticatedUser{Username: "guest", Provider: "system"}
+	guest.BuildUserAcls(cfg)
+
+	assert.True(t, IsAllowedViewDashboard(cfg, guest, dashboard))
+	assert.True(t, IsAllowedViewDashboard(cfg, guest, nil))
+}
+
+func TestIsAllowedViewDashboardAllowDenyAndDefaultFallback(t *testing.T) {
+	cfg := config.DefaultConfig()
+	cfg.DefaultPermissions.View = false
+	cfg.AccessControlLists = []*config.AccessControlList{
+		{
+			Name:           "admins",
+			MatchUsernames: []string{"admin"},
+			Permissions:    config.PermissionsList{View: true, Exec: true},
+		},
+	}
+
+	dashboard := &config.DashboardComponent{
+		Title: "Services",
+		Acls:  []string{"admins"},
+		Contents: []*config.DashboardComponent{
+			{Title: "Status: running", Type: "display"},
+		},
+	}
+
+	guest := &authpublic.AuthenticatedUser{Username: "guest", Provider: "system"}
+	guest.BuildUserAcls(cfg)
+	admin := &authpublic.AuthenticatedUser{Username: "admin"}
+	admin.BuildUserAcls(cfg)
+
+	assert.False(t, IsAllowedViewDashboard(cfg, guest, dashboard))
+	assert.True(t, IsAllowedViewDashboard(cfg, admin, dashboard))
+
+	cfg.DefaultPermissions.View = true
+	assert.True(t, IsAllowedViewDashboard(cfg, guest, dashboard),
+		"when no relevant ACL matches, fall back to defaultPermissions.view")
+}
+
+func TestIsAllowedViewDashboardIgnoresAddToEveryAction(t *testing.T) {
+	cfg := config.DefaultConfig()
+	cfg.DefaultPermissions.View = false
+	cfg.AccessControlLists = []*config.AccessControlList{
+		{
+			Name:             "admins",
+			MatchUsernames:   []string{"admin"},
+			AddToEveryAction: true,
+			Permissions:      config.PermissionsList{View: true, Exec: true},
+		},
+	}
+
+	dashboard := &config.DashboardComponent{
+		Title: "Secret",
+		Acls:  []string{"other"},
+		Contents: []*config.DashboardComponent{
+			{Title: "Hidden status", Type: "display"},
+		},
+	}
+
+	admin := &authpublic.AuthenticatedUser{Username: "admin"}
+	admin.BuildUserAcls(cfg)
+
+	assert.False(t, IsAllowedViewDashboard(cfg, admin, dashboard),
+		"AddToEveryAction must not grant dashboard view without listing the ACL on the dashboard")
+}

+ 110 - 0
service/internal/api/dashboard_acl_test.go

@@ -0,0 +1,110 @@
+package api
+
+import (
+	"testing"
+
+	authpublic "github.com/OliveTin/OliveTin/internal/auth/authpublic"
+	config "github.com/OliveTin/OliveTin/internal/config"
+	"github.com/OliveTin/OliveTin/internal/executor"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func buildDashboardAclTestConfig() *config.Config {
+	cfg := config.DefaultConfig()
+	cfg.DefaultPermissions.View = false
+	cfg.DefaultPermissions.Exec = false
+	cfg.AccessControlLists = []*config.AccessControlList{
+		{
+			Name:           "admins",
+			MatchUsernames: []string{"admin"},
+			Permissions:    config.PermissionsList{View: true, Exec: true},
+		},
+	}
+	cfg.Dashboards = []*config.DashboardComponent{
+		{
+			Title: "Public tools",
+			Contents: []*config.DashboardComponent{
+				{Title: "Welcome", Type: "display"},
+			},
+		},
+		{
+			Title: "Services",
+			Acls:  []string{"admins"},
+			Contents: []*config.DashboardComponent{
+				{Title: "Status: running", Type: "display"},
+			},
+		},
+	}
+
+	return cfg
+}
+
+func TestDashboardAclsRootNavAndGetDashboard(t *testing.T) {
+	cfg := buildDashboardAclTestConfig()
+	ex := executor.DefaultExecutor(cfg)
+	api := newServer(ex)
+
+	guest := &authpublic.AuthenticatedUser{Username: "guest", Provider: "system"}
+	guest.BuildUserAcls(cfg)
+	admin := &authpublic.AuthenticatedUser{Username: "admin"}
+	admin.BuildUserAcls(cfg)
+
+	guestRoots := api.buildRootDashboards(guest, cfg.Dashboards)
+	assert.Contains(t, guestRoots, "Public tools")
+	assert.NotContains(t, guestRoots, "Services")
+
+	adminRoots := api.buildRootDashboards(admin, cfg.Dashboards)
+	assert.Contains(t, adminRoots, "Public tools")
+	assert.Contains(t, adminRoots, "Services")
+
+	guestRR := api.createDashboardRenderRequest(guest, "", "")
+	assert.Nil(t, renderDashboard(guestRR, "Services"),
+		"GetDashboard must not leak ACL-restricted dashboard content via deep link")
+
+	adminRR := api.createDashboardRenderRequest(admin, "", "")
+	db := renderDashboard(adminRR, "Services")
+	require.NotNil(t, db)
+	assert.Equal(t, "Services", db.Title)
+}
+
+func TestDashboardAclsNestedDirectoryDeepLink(t *testing.T) {
+	cfg := buildDashboardAclTestConfig()
+	cfg.Dashboards = []*config.DashboardComponent{
+		{
+			Title: "Public tools",
+			Contents: []*config.DashboardComponent{
+				{Title: "Welcome", Type: "display"},
+			},
+		},
+		{
+			Title: "Services",
+			Acls:  []string{"admins"},
+			Contents: []*config.DashboardComponent{
+				{
+					Title: "Infrastructure",
+					Contents: []*config.DashboardComponent{
+						{Title: "Status: running", Type: "display"},
+					},
+				},
+			},
+		},
+	}
+
+	ex := executor.DefaultExecutor(cfg)
+	api := newServer(ex)
+
+	guest := &authpublic.AuthenticatedUser{Username: "guest", Provider: "system"}
+	guest.BuildUserAcls(cfg)
+	admin := &authpublic.AuthenticatedUser{Username: "admin"}
+	admin.BuildUserAcls(cfg)
+
+	guestRR := api.createDashboardRenderRequest(guest, "", "")
+	assert.Nil(t, renderDashboard(guestRR, "Infrastructure"),
+		"nested directory under ACL-restricted root must not leak via deep link")
+
+	adminRR := api.createDashboardRenderRequest(admin, "", "")
+	db := renderDashboard(adminRR, "Infrastructure")
+	require.NotNil(t, db)
+	assert.Equal(t, "Infrastructure", db.Title)
+}

+ 13 - 1
service/internal/api/dashboards.go

@@ -52,6 +52,10 @@ func findDashboardByTitle(rr *DashboardRenderRequest, dashboardTitle string) *co
 }
 
 func renderDashboardIfValid(dashboard *config.DashboardComponent, rr *DashboardRenderRequest) *apiv1.Dashboard {
+	if !acl.IsAllowedViewDashboard(rr.cfg, rr.AuthenticatedUser, dashboard) {
+		return nil
+	}
+
 	if len(dashboard.Contents) == 0 {
 		logEmptyDashboard(dashboard.Title, rr.AuthenticatedUser.Username)
 		return nil
@@ -71,13 +75,21 @@ func renderDirectoryDashboard(rr *DashboardRenderRequest, dashboardTitle string)
 
 func findDirectoryComponent(rr *DashboardRenderRequest, title string) *config.DashboardComponent {
 	for _, dashboard := range rr.cfg.Dashboards {
-		if component := searchDirectoryInComponent(dashboard, title); component != nil {
+		if component := findDirectoryInRootIfAllowed(rr, dashboard, title); component != nil {
 			return component
 		}
 	}
 	return nil
 }
 
+func findDirectoryInRootIfAllowed(rr *DashboardRenderRequest, root *config.DashboardComponent, title string) *config.DashboardComponent {
+	if !acl.IsAllowedViewDashboard(rr.cfg, rr.AuthenticatedUser, root) {
+		return nil
+	}
+
+	return searchDirectoryInComponent(root, title)
+}
+
 func searchDirectoryInComponent(component *config.DashboardComponent, title string) *config.DashboardComponent {
 	if isMatchingDirectory(component, title) {
 		return component

+ 1 - 0
service/internal/config/config.go

@@ -289,6 +289,7 @@ type DashboardComponent struct {
 	Entity       string                `koanf:"entity"`
 	Icon         string                `koanf:"icon"`
 	CssClass     string                `koanf:"cssClass"`
+	Acls         []string              `koanf:"acls"`
 	InlineAction *Action               `koanf:"inlineAction"`
 	Contents     []*DashboardComponent `koanf:"contents"`
 }