diff --git a/README.md b/README.md index a06b33e..5018dc8 100644 --- a/README.md +++ b/README.md @@ -143,13 +143,18 @@ the authoritative implementation: `COPY`/`CALL`/`EXPLAIN`/`VACUUM`, and nothing after a trailing `;`. 2. No `information_schema`/`pg_catalog`/`pg_*` introspection, `dblink`, `lo_*`, or `COPY` — closes standard sandbox-escape tricks. -3. **Project/integration-scoped queries must contain the literal - placeholder `{{project_id}}`** somewhere in a `WHERE`/`ON`/`HAVING` - clause. The guard substitutes it with `$1` and binds the caller's real +3. **Project/integration-scoped queries must use the literal placeholder + `{{project_id}}` exactly once, as a direct ` = {{project_id}}` + (or reversed) equality filter, with no bare `OR` anywhere in the + query.** The guard substitutes it with `$1` and binds the caller's real `project_id` — this is what actually prevents cross-project data - leakage, by forcing every project-scoped query to filter itself. - Admin-scope queries are intentionally cross-project and skip this - requirement (and reject the placeholder if present). + leakage, by forcing every project-scoped query to filter itself in a way + that can't be short-circuited (a duplicated/self-compared placeholder, a + comparison operator other than `=`, or a sibling `OR` can all make the + filter match regardless of project — see `validateProjectScopePlaceholder` + in `query_guard.go`). Need a multi-value filter? Use `IN (...)` instead + of `OR`. Admin-scope queries are intentionally cross-project and skip + this requirement (and reject the placeholder if present). 4. An explicit `LIMIT 500` is appended when the query doesn't already declare one. @@ -179,6 +184,78 @@ rather than risk a false negative. > textarea (frontend UX) — the two are independent of the rest of the > panel/view CRUD. +### Authorization + +Access to the dashboard *page* is gated separately from access to individual +*data*. Two custom permissions, declared in `plugin.json` and each available +at both project and global **permission** scope (the same key checked +against two different permission maps, mirroring `paca-plugin-time-logging`). +This is the only "scope" the permission system itself has — +`requirePermissions`/`customPermissions` entries are always either +`"project"` (checked against the caller's per-project permission map) or +`"global"` (checked against their global permission map). There is no third, +"admin" permission scope, even though the paragraphs below also need to talk +about `dashboard_views.scope`, an unrelated data-model column on this +plugin's own table that happens to have a value literally named `"admin"` — +every occurrence of "admin" below refers to that column's value or to the +`/admin/*`-routed pages, never to a permission scope: + +- **`dashboard.view`** — see the project's (or admin's) Dashboard page and + its panels. +- **`dashboard.manage`** — create, edit, delete, and rearrange panels, and + run query previews while authoring them. Implies `dashboard.view` for + loading purposes — a role with only `dashboard.manage` isn't blocked from + the page itself. + +Every project-*permission*-scope backend route requires `dashboard.view` at +minimum, declared directly in the manifest's `requirePermissions` — +including the routes shared with dashboards whose `dashboard_views.scope` +is `"integration"` (embedded in Backlog/Sprint/Timeline, see "Integration +view architecture" above), which the manifest can't distinguish from the +`dashboard_views.scope == "project"` singleton by path alone. There is no +"always open" carve-out for the embedded surface: `dashboard.view` gates +the whole feature uniformly, project page and embedded views alike. +`views.go`'s `loadViewWithPanels` and `getOrCreate*View` handlers re-check +this in-handler too, as defense-in-depth against the manifest ever being +misedited — not because the manifest leaves any gap today. + +On top of that `dashboard.view` floor, mutating a panel requires a second, +narrower check in `panels.go`'s `canManagePanel`, which resolves the +request's `dashboard_views.scope` value and picks the (project-permission- +scope) permission that matches: + +- row's `dashboard_views.scope` is `"project"`: requires `dashboard.manage`. +- row's `dashboard_views.scope` is `"integration"`: requires `views.write` + instead of `dashboard.manage` — an integration dashboard is embedded in, + and shares the write bar of, the view that hosts it. `views.write` is + exactly the permission that already lets an ordinary project Editor + create that hosting view in the first place; requiring anything higher + here would be a dead end (create the view, never populate it). +- row's `dashboard_views.scope` is `"admin"`: never reaches this function — + those routes are already fully gated at the manifest level by + `dashboard.manage` at **permission** scope `"global"`. + +This two-layer shape (`dashboard.view` as a uniform manifest-level floor, +a data-scope-specific manage permission checked in-handler on top) is +deliberate: `requirePermissions` ANDs every permission it lists, so putting +`dashboard.manage` directly in the manifest on a route shared with the +`"integration"` data scope would reject a `views.write`-holding Editor +before the in-handler check ever ran. + +`POST /dashboard/query/preview` (project permission scope) has no +`dashboard_views` row to resolve a data scope from — it validates a +not-yet-saved query directly against the caller's project, and is used by +both the project-dashboard editor and the integration-view editor with no +way to tell which from the request alone. Past the manifest's +`dashboard.view` floor, it requires +`dashboard.manage` in-handler (matching that permission's own description, +which promises coverage of "running query previews while authoring" +panels). The one remaining, accepted gap: a hand-crafted custom role +granted only `views.write` (able to populate an integration dashboard) +can't preview a query for it through this route — closing that fully would +need the frontend to pass a `viewId` so the backend can resolve scope, a +small API contract change tracked as a follow-up rather than done here. + ### MCP (`mcp/`) `mcp/src/index.ts` exposes the panel/view API to AI clients (Claude, GitHub @@ -309,20 +386,19 @@ bun run build ### `project.page` — `ProjectDashboardPage` Routed at `/projects/:projectId/plugins/com.paca.dashboard/dashboard` via -the `project` nav item declared in `plugin.json`. Fetches (get-or-creates) -the project's singleton dashboard view and renders the shared panel-grid -UI with edit permissions gated by the route's own `projects.write` -middleware (panel mutation routes require `projects.write`; read routes -require only `projects.read`). +the `project` nav item declared in `plugin.json`, requiring the +`dashboard.view` custom permission (see "Authorization" above). Fetches +(get-or-creates) the project's singleton dashboard view and renders the +shared panel-grid UI, with panel mutations requiring `dashboard.manage`. ### `admin.page` — `AdminDashboardPage` Routed at `/admin/plugins/com.paca.dashboard/dashboard` via the `admin` nav -item declared in `plugin.json`; gated by the built-in `users.write` global -permission (same gate as the host's other admin pages). Fetches -(get-or-creates) the instance-wide singleton dashboard view; its panel -queries are cross-project and do not require the `{{project_id}}` -placeholder. +item declared in `plugin.json`, requiring the global-scope `dashboard.view` +custom permission (panel mutations require `dashboard.manage`) — see +"Authorization" above. Fetches (get-or-creates) the instance-wide singleton +dashboard view; its panel queries are cross-project and do not require the +`{{project_id}}` placeholder. ### `view` — `DashboardIntegrationView` diff --git a/backend/panels.go b/backend/panels.go index fb8aa2d..b4f609e 100644 --- a/backend/panels.go +++ b/backend/panels.go @@ -131,6 +131,57 @@ func (p *dashboardPlugin) previewAdminQuery(req *plugin.Request, res *plugin.Res p.previewQuery(req, res, false) } +// canManagePanel reports whether the caller may create/edit/delete/reorder +// panels on a view whose dashboard_views.scope column holds viewScope +// ("project" | "integration" | "admin" — a data-model value describing +// which kind of dashboard the row is, NOT a permission scope; the +// permission system itself only ever has two scopes, "project" and +// "global", set on the requirePermissions/customPermissions entries in +// plugin.json). A single permission decides the answer here — not two — +// which is exactly why the manifest's own requirePermissions floor on +// these routes is deliberately just projects.read rather than +// projects.write: requirePermissions ANDs every listed permission, so if +// the manifest floor were projects.write, a role granted only +// dashboard.manage would 403 at the middleware layer before this in-handler +// check ever ran, silently defeating the point of a dedicated permission. +// +// - viewScope == "project": requires dashboard.manage (a project-scope +// permission, i.e. checked against the caller's per-project permission +// map). +// - viewScope == "integration": requires views.write instead of +// dashboard.manage — an integration dashboard is embedded in, and +// shares the write bar of, the view that hosts it. views.write is +// exactly the permission that already lets an ordinary Editor create +// that hosting view in the first place; requiring anything higher here +// would be a dead end (create the view, never populate it). Also a +// project-scope permission. +// - viewScope == "admin": never reaches this function — loadViewWithPanels +// only lets projectID == "" through the admin routes, which are already +// fully gated at the manifest level by dashboard.manage checked at +// permission scope "global" (not "admin" — there is no such permission +// scope). +func (p *dashboardPlugin) canManagePanel(viewScope string) bool { + if viewScope == "integration" { + return p.perm.Check("views.write") + } + return p.perm.Check("dashboard.manage") +} + +// requireManagePanel enforces canManagePanel for view, writing the shared +// 403 response and returning false when the caller may not manage it — the +// same check every panel-mutation handler below needs before touching +// view's panels. Admin-scope views skip the check entirely (loadViewWithPanels +// only lets projectID == "" through the admin routes, which are already +// fully gated at the manifest level), matching canManagePanel's own doc +// comment on why viewScope == "admin" never reaches canManagePanel itself. +func (p *dashboardPlugin) requireManagePanel(view *dashboardView, res *plugin.Response) bool { + if view.Scope != "admin" && !p.canManagePanel(view.Scope) { + res.Error(403, "you don't have permission to manage this dashboard") + return false + } + return true +} + // ── Shared implementations ─────────────────────────────────────────────────── // createPanelForView handles POST .../:viewId/panels for any scope. @@ -144,6 +195,9 @@ func (p *dashboardPlugin) createPanelForView(req *plugin.Request, res *plugin.Re if !loaded { return } + if !p.requireManagePanel(view, res) { + return + } b, err := plugin.JSONBody[panelBody](req) if err != nil { @@ -196,6 +250,9 @@ func (p *dashboardPlugin) updatePanelForView(req *plugin.Request, res *plugin.Re if !loaded { return } + if !p.requireManagePanel(view, res) { + return + } if !p.panelBelongsToView(panelID, viewID, res) { return } @@ -261,7 +318,11 @@ func (p *dashboardPlugin) deletePanelForView(req *plugin.Request, res *plugin.Re return } panelID := req.PathParam("panelId") - if _, ok2 := p.loadViewWithPanels(viewID, projectID, res); !ok2 { + view, loaded := p.loadViewWithPanels(viewID, projectID, res) + if !loaded { + return + } + if !p.requireManagePanel(view, res) { return } if !p.panelBelongsToView(panelID, viewID, res) { @@ -290,7 +351,11 @@ func (p *dashboardPlugin) updatePanelLayoutForView(req *plugin.Request, res *plu if !viewOK { return } - if _, ok2 := p.loadViewWithPanels(viewID, projectID, res); !ok2 { + view, loaded := p.loadViewWithPanels(viewID, projectID, res) + if !loaded { + return + } + if !p.requireManagePanel(view, res) { return } @@ -390,6 +455,17 @@ func (p *dashboardPlugin) runPanelQueryForView(req *plugin.Request, res *plugin. // equivalent) — validates and runs a not-yet-saved query so the panel // editor can show a live preview before the user hits Save. func (p *dashboardPlugin) previewQuery(req *plugin.Request, res *plugin.Response, requireProjectScope bool) { + // The admin path (requireProjectScope == false) is already fully gated + // at the manifest level (dashboard.manage, global scope) — no view to + // resolve a scope from here, so this only applies the project-scope + // check. dashboard.manage's own description promises it covers "running + // query previews while authoring" panels, so this must hold even though + // the manifest's own floor is the non-restrictive projects.read (same + // reasoning as canManagePanel). + if requireProjectScope && !p.canManagePanel("project") { + res.Error(403, "you don't have permission to manage this dashboard") + return + } type previewQueryBody struct { Query string `json:"query"` } diff --git a/backend/panels_cache_test.go b/backend/panels_cache_test.go index e4806dc..6424cb7 100644 --- a/backend/panels_cache_test.go +++ b/backend/panels_cache_test.go @@ -132,22 +132,36 @@ func TestUpdatePanel_InvalidatesCachedData(t *testing.T) { // Narrowing the panel's query must invalidate its cached result — // otherwise this would keep serving the old 2-row result for up to - // panelDataCacheTTL after the edit. The {{project_id}} placeholder is - // substituted for every occurrence of $1 (see query_guard.go), so - // repeating it as a second condition against `id` — which never equals - // the project_id string — deterministically narrows the seeded 2-row - // result to 0 without needing a second bindable parameter. - tc.Call("PATCH", "/dashboard/views/:viewId/panels/:panelId", + // panelDataCacheTTL after the edit. query_guard.go now requires the + // {{project_id}} placeholder to be used as a direct "project_id = + // {{project_id}}" equality filter and rejects comparing it against any + // other column (see validateProjectScopePlaceholder / + // placeholderComparisonRe) — so the previous version of this test, + // which compared it against tasks.id to force a deterministic 0-row + // result, is no longer a valid panel query. That's exactly the class of + // decoy filter the guard now closes (a placeholder-equality against an + // unrelated column doesn't actually scope the returned rows), so this + // test needs a different way to get a deterministic, different-from- + // before result. Querying dashboard_views instead of tasks does that + // while still using a single valid `project_id = {{project_id}}` + // filter: this project has exactly one dashboard_views row (the view + // fetched at the top of this test), versus tasks' two, and + // plugintest.InMemoryDB's minimal parser (col = $N chains only) has no + // trouble with it. + patchRes := tc.Call("PATCH", "/dashboard/views/:viewId/panels/:panelId", withPathParams(callerReq(), map[string]string{"viewId": view.ID, "panelId": panel.ID}). WithJSONBody(map[string]any{ "type": "table", "title": "My tasks", - "query": "SELECT id, title FROM tasks WHERE project_id = {{project_id}} AND id = {{project_id}}", + "query": "SELECT id, name FROM dashboard_views WHERE project_id = {{project_id}}", })) + if patchRes.StatusCode != 200 { + t.Fatalf("expected 200 updating the panel's query, got %d: %s", patchRes.StatusCode, patchRes.BodyString()) + } second := panelDataRows(t, tc.Call("POST", "/dashboard/views/:viewId/panels/:panelId/data", dataReq)) - if len(second) != 0 { - t.Fatalf("expected the updated query's 0-row result after cache invalidation, got %d: %+v", len(second), second) + if len(second) != 1 { + t.Fatalf("expected the updated query's 1-row result (this project's own dashboard_views row) after cache invalidation, got %d: %+v", len(second), second) } } diff --git a/backend/plugin.go b/backend/plugin.go index 53fb278..279c6b0 100644 --- a/backend/plugin.go +++ b/backend/plugin.go @@ -36,6 +36,7 @@ type dashboardPlugin struct { db *plugin.DB cache *plugin.Cache log *plugin.Logger + perm *plugin.Permissions } // Init registers all routes on the provided context. @@ -43,6 +44,7 @@ func (p *dashboardPlugin) Init(ctx *plugin.Context) error { p.db = ctx.DB() p.cache = ctx.Cache() p.log = ctx.Log() + p.perm = ctx.Permissions() // ── Project-scope dashboard (get-or-create singleton) ────────────────── ctx.Route("GET", "/dashboard/view", p.getOrCreateProjectView) diff --git a/backend/plugin_test.go b/backend/plugin_test.go index 3cfa84e..7109122 100644 --- a/backend/plugin_test.go +++ b/backend/plugin_test.go @@ -4,6 +4,7 @@ package main import ( "encoding/json" + "strings" "testing" plugin "github.com/Paca-AI/plugin-sdk-go" @@ -17,6 +18,11 @@ const testProjectID = "project-1" func setupPlugin(t *testing.T) *plugintest.Context { t.Helper() tc := plugintest.NewContext(t) + // Default test caller has full dashboard access; tests for the + // permission gate itself (TestLoadView_RequiresDashboardView etc.) + // explicitly revoke these to exercise the denied path. + tc.Permissions.Grant("dashboard.view") + tc.Permissions.Grant("dashboard.manage") tc.DB.SeedRows("dashboard_views", []string{"id", "project_id", "scope", "host_view_id", "name", "created_by", "created_at", "updated_at"}, @@ -255,6 +261,66 @@ func TestGetView_CrossProjectRejected(t *testing.T) { } } +// ── Dashboard permission gate (dashboard.view / dashboard.manage) ─────────── + +func TestGetOrCreateProjectView_RequiresDashboardView(t *testing.T) { + tc := setupPlugin(t) + tc.Permissions.Revoke("dashboard.view") + tc.Permissions.Revoke("dashboard.manage") + + res := tc.Call("GET", "/dashboard/view", callerReq()) + if res.StatusCode != 403 { + t.Fatalf("expected 403 for a caller with neither dashboard.view nor dashboard.manage, got %d: %s", res.StatusCode, res.BodyString()) + } +} + +func TestGetOrCreateProjectView_ManageAloneIsSufficientToView(t *testing.T) { + tc := setupPlugin(t) + tc.Permissions.Revoke("dashboard.view") + // dashboard.manage stays granted (setupPlugin's default) — manage alone + // must still be enough to load the view. + + res := tc.Call("GET", "/dashboard/view", callerReq()) + if res.StatusCode != 200 { + t.Fatalf("expected 200 for a caller with only dashboard.manage, got %d: %s", res.StatusCode, res.BodyString()) + } +} + +// TestGetOrCreateIntegrationView_RequiresDashboardView pins the current +// design: dashboard.view gates every dashboard surface uniformly, including +// integration-scope dashboards embedded in Backlog/Sprint/Timeline — not +// just the dedicated project-scope singleton page. Revoking dashboard.view +// from a role now hides the whole feature, embedded views included; there +// is no "always open" carve-out for the embedded surface. +func TestGetOrCreateIntegrationView_RequiresDashboardView(t *testing.T) { + tc := setupPlugin(t) + tc.Permissions.Revoke("dashboard.view") + tc.Permissions.Revoke("dashboard.manage") + + res := tc.Call("GET", "/dashboard/view/:hostViewId", + withPathParams(callerReq(), map[string]string{"hostViewId": testHostViewID})) + if res.StatusCode != 403 { + t.Fatalf("expected 403 for an integration-scope view without dashboard.view, got %d: %s", res.StatusCode, res.BodyString()) + } +} + +func TestCreatePanel_RequiresDashboardManage(t *testing.T) { + tc := setupPlugin(t) + view := decodeData[dashboardView](t, tc.Call("GET", "/dashboard/view", callerReq())) + tc.Permissions.Revoke("dashboard.manage") // dashboard.view stays granted + + res := tc.Call("POST", "/dashboard/views/:viewId/panels", + withPathParams(callerReq(), map[string]string{"viewId": view.ID}). + WithJSONBody(map[string]any{ + "type": "text", + "title": "Notes", + "content": "should be rejected before it's ever validated", + })) + if res.StatusCode != 403 { + t.Fatalf("expected 403 for a caller with dashboard.view but not dashboard.manage, got %d: %s", res.StatusCode, res.BodyString()) + } +} + // ── Panel CRUD ──────────────────────────────────────────────────────────────── func TestCreateTextPanel(t *testing.T) { @@ -501,6 +567,57 @@ func TestRunPanelQuery_ScopesToOwnProject(t *testing.T) { } } +// TestPreviewQuery_RequiresDashboardManage pins the fix for the +// query/preview route relying solely on the manifest's projects.read floor +// with no in-handler check at all — dashboard.manage's own description +// promises it covers "running query previews while authoring" panels, so a +// caller without it must be rejected here too, not just on save. +func TestPreviewQuery_RequiresDashboardManage(t *testing.T) { + tc := setupPlugin(t) + tc.Permissions.Revoke("dashboard.manage") + + res := tc.Call("POST", "/dashboard/query/preview", + callerReq().WithJSONBody(map[string]string{ + "query": "SELECT id FROM tasks WHERE project_id = {{project_id}}", + })) + if res.StatusCode != 403 { + t.Fatalf("expected 403 for a caller without dashboard.manage, got %d: %s", res.StatusCode, res.BodyString()) + } +} + +// TestCreatePanel_IntegrationScope_RequiresViewsWrite pins the fix for +// integration-scope panel mutations previously having no permission check +// at all in-handler (only the manifest's projects.write, which an ordinary +// Editor — who can create the hosting integration view via views.write — +// doesn't hold, making the view creatable but never populatable). Panel +// mutations on an integration view now require views.write specifically, +// not dashboard.manage — matching the permission that already gates +// creating the view itself. +func TestCreatePanel_IntegrationScope_RequiresViewsWrite(t *testing.T) { + tc := setupPlugin(t) + view := decodeData[dashboardView](t, tc.Call("GET", "/dashboard/view/:hostViewId", + withPathParams(callerReq(), map[string]string{"hostViewId": testHostViewID}))) + + // dashboard.manage alone must NOT be sufficient for an integration view. + tc.Permissions.Revoke("views.write") + denied := tc.Call("POST", "/dashboard/views/:viewId/panels", + withPathParams(callerReq(), map[string]string{"viewId": view.ID}). + WithJSONBody(map[string]any{"type": "text", "title": "Notes", "content": "x"})) + if denied.StatusCode != 403 { + t.Fatalf("expected 403 for an integration view without views.write, got %d: %s", denied.StatusCode, denied.BodyString()) + } + + // views.write alone (without dashboard.manage) must be sufficient. + tc.Permissions.Revoke("dashboard.manage") + tc.Permissions.Grant("views.write") + allowed := tc.Call("POST", "/dashboard/views/:viewId/panels", + withPathParams(callerReq(), map[string]string{"viewId": view.ID}). + WithJSONBody(map[string]any{"type": "text", "title": "Notes", "content": "x"})) + if allowed.StatusCode != 201 { + t.Fatalf("expected 201 for an integration view with views.write but not dashboard.manage, got %d: %s", allowed.StatusCode, allowed.BodyString()) + } +} + func TestPreviewQuery_RejectsForbiddenKeyword(t *testing.T) { tc := setupPlugin(t) @@ -588,6 +705,129 @@ func TestValidateQuery_RejectsDollarOneDirectUse(t *testing.T) { } } +// TestValidateQuery_RejectsDuplicatedPlaceholderTautology pins the fix for a +// real cross-project data leak: the old guard only checked that +// {{project_id}} appeared *somewhere* in the query, so a self-comparison +// like this one passed validation and became an always-true "$1 = $1" +// filter after substitution — returning every row in the table, across +// every project, regardless of the caller's own project_id. +func TestValidateQuery_RejectsDuplicatedPlaceholderTautology(t *testing.T) { + if _, err := validateQuery("SELECT id FROM tasks WHERE '{{project_id}}' = '{{project_id}}'", true); err == nil { + t.Fatal("expected error: duplicated placeholder forms an always-true tautology") + } +} + +// TestValidateQuery_RejectsOrShortCircuit pins the fix for the second +// concrete bypass the old "contains the token somewhere" check allowed: a +// correctly-used placeholder rendered moot by a sibling top-level OR, which +// would return rows regardless of project since the OR's other branch is +// always true. +func TestValidateQuery_RejectsOrShortCircuit(t *testing.T) { + if _, err := validateQuery("SELECT id FROM tasks WHERE 1=1 OR project_id = {{project_id}}", true); err == nil { + t.Fatal("expected error: a top-level OR can bypass the project filter") + } +} + +// TestValidateQuery_RejectsNonEqualityPlaceholderUse guards against a +// variant of the same class of bug: a comparison operator other than "=" +// (e.g. "!=") inverts the filter into "every other project" instead of +// "just mine". +func TestValidateQuery_RejectsNonEqualityPlaceholderUse(t *testing.T) { + if _, err := validateQuery("SELECT id FROM tasks WHERE project_id != {{project_id}}", true); err == nil { + t.Fatal("expected error: != inverts the project filter instead of scoping to it") + } +} + +// TestValidateQuery_AllowsOrInsideStringLiteral guards against a +// too-aggressive fix: "OR" appearing as ordinary text inside a quoted +// string literal (not the SQL keyword) must not trip the OR ban. +func TestValidateQuery_AllowsOrInsideStringLiteral(t *testing.T) { + safe, err := validateQuery( + "SELECT id FROM tasks WHERE project_id = {{project_id}} AND title = 'Manager or Director'", true) + if err != nil { + t.Fatalf("unexpected error for OR inside a string literal: %v", err) + } + if !strings.Contains(safe, "Manager or Director") { + t.Fatalf("expected the string literal to survive validation unchanged, got %q", safe) + } +} + +// TestValidateQuery_RejectsUnion pins the fix for a concrete cross-project +// leak: none of the placeholder/OR checks understand a second SELECT +// contributing rows via UNION, so a properly-scoped first branch used to be +// enough to pass validation while an unfiltered second branch returned +// every project's rows. +func TestValidateQuery_RejectsUnion(t *testing.T) { + if _, err := validateQuery( + "SELECT id, title FROM tasks WHERE project_id = {{project_id}} UNION SELECT id, title FROM tasks", true); err == nil { + t.Fatal("expected error: UNION can append unfiltered rows from a second SELECT") + } +} + +// TestValidateQuery_RejectsPlaceholderInsideSubquery pins the fix for a +// decoy filter: a placeholder-equality that satisfies a naive text search +// while sitting inside an unrelated nested subquery doesn't actually +// constrain which rows the outer query returns. +func TestValidateQuery_RejectsPlaceholderInsideSubquery(t *testing.T) { + if _, err := validateQuery( + "SELECT id, title FROM tasks WHERE (SELECT 1 FROM projects WHERE id = {{project_id}}) IS NOT NULL", true); err == nil { + t.Fatal("expected error: placeholder used inside a nested subquery doesn't filter the outer query's rows") + } +} + +// TestValidateQuery_RejectsPlaceholderAgainstWrongColumn pins the fix for a +// decoy cross join: comparing the placeholder against an unrelated column +// (here projects.id, a different table's primary key) leaves the actually +// -returned table's rows completely unfiltered. +func TestValidateQuery_RejectsPlaceholderAgainstWrongColumn(t *testing.T) { + if _, err := validateQuery( + "SELECT id, title FROM tasks, projects WHERE projects.id = {{project_id}}", true); err == nil { + t.Fatal("expected error: placeholder must filter a project_id column, not an unrelated column") + } +} + +// TestValidateQuery_RejectsOrHiddenByQuotedIdentifier pins the fix for a +// string-tracking desync: an apostrophe inside a double-quoted identifier +// (valid SQL, e.g. an alias) used to desync the old single-quote-only +// literal tracker, making everything after it invisible to the OR check. +func TestValidateQuery_RejectsOrHiddenByQuotedIdentifier(t *testing.T) { + if _, err := validateQuery( + `SELECT id, 1 AS "it's" FROM tasks WHERE project_id = {{project_id}} OR 1=1`, true); err == nil { + t.Fatal("expected error: a bare OR must still be caught after a double-quoted identifier containing an apostrophe") + } +} + +// TestValidateQuery_AllowsQualifiedProjectIdInJoin guards against a +// too-aggressive fix: an ordinary explicit JOIN, with the placeholder +// filtering an alias-qualified project_id column, is the documented +// supported shape and must keep working. +func TestValidateQuery_AllowsQualifiedProjectIdInJoin(t *testing.T) { + safe, err := validateQuery( + "SELECT t.id, u.username FROM tasks t JOIN users u ON u.id = t.assignee_id WHERE t.project_id = {{project_id}}", true) + if err != nil { + t.Fatalf("unexpected error for an alias-qualified project_id filter in a joined query: %v", err) + } + if !containsLimit(safe) { + t.Fatalf("expected LIMIT to be appended, got %q", safe) + } +} + +// TestValidateQuery_AllowsCteWithTopLevelFilter guards against a +// too-aggressive fix: a WITH/CTE query is a documented supported shape as +// long as the project_id filter is in the outer query's own WHERE, which is +// what every real usage pattern in this file does. +func TestValidateQuery_AllowsCteWithTopLevelFilter(t *testing.T) { + safe, err := validateQuery( + "WITH recent AS (SELECT id, title FROM tasks ORDER BY created_at DESC) "+ + "SELECT * FROM recent WHERE project_id = {{project_id}}", true) + if err != nil { + t.Fatalf("unexpected error for a CTE with the placeholder filter at the outer top level: %v", err) + } + if !containsLimit(safe) { + t.Fatalf("expected LIMIT to be appended, got %q", safe) + } +} + func containsLimit(sql string) bool { return limitRe.MatchString(sql) } diff --git a/backend/query_guard.go b/backend/query_guard.go index a8b11a6..2883267 100644 --- a/backend/query_guard.go +++ b/backend/query_guard.go @@ -16,14 +16,19 @@ // functions, dblink, lo_*, or COPY — closes the standard sandbox-escape // tricks for a restricted SQL surface. // 3. Project-scoped queries (project + integration dashboard scopes) must -// contain the literal placeholder token `{{project_id}}` somewhere in -// a WHERE/ON/HAVING clause; the guard substitutes it with `$1` and the -// caller's real project_id is bound as that parameter. A query with no -// `{{project_id}}` placeholder is rejected outright for these scopes — -// this is what actually prevents cross-project data leakage, since it -// forces every project-scoped query to filter by the caller's own -// project. Admin-scope queries are intentionally cross-project and skip -// this requirement. +// use the literal placeholder token `{{project_id}}` exactly once, as a +// direct ` = {{project_id}}` (or reversed) equality filter, with +// no bare OR anywhere in the query; the guard then substitutes it with +// `$1` and the caller's real project_id is bound as that parameter (see +// validateProjectScopePlaceholder). A query with no `{{project_id}}` +// placeholder, or one that only *contains* the token without using it as +// a genuine per-row filter (duplicated into a self-comparison, compared +// with anything but `=`, or otherwise made bypassable via OR), is +// rejected outright for these scopes — this is what actually prevents +// cross-project data leakage, since it forces every project-scoped query +// to filter by the caller's own project in a way that can't be +// short-circuited. Admin-scope queries are intentionally cross-project +// and skip this requirement. // 4. An explicit LIMIT is appended (LIMIT 500) when the query doesn't // already declare one, bounding worst-case result size / query cost. // @@ -52,8 +57,13 @@ import ( ) // forbiddenKeywords are rejected anywhere in the query (case-insensitive, -// word-bounded) — mutating statements, introspection, and known -// sandbox-escape surfaces. +// word-bounded) — mutating statements, introspection, known +// sandbox-escape surfaces, and compound-SELECT operators. The latter close +// a real bypass: none of the placeholder/OR checks below understand +// multiple SELECTs contributing rows to one result, so +// "... WHERE project_id={{project_id}} UNION SELECT ... FROM tasks" (no +// filter on the second branch) satisfied every other check and returned +// every project's rows. var forbiddenKeywords = []string{ "insert", "update", "delete", "drop", "alter", "create", "grant", "revoke", "truncate", "copy", "call", "execute", "explain", "vacuum", @@ -61,6 +71,7 @@ var forbiddenKeywords = []string{ "pg_sleep", "pg_read_file", "pg_read_binary_file", "pg_ls_dir", "dblink", "lo_import", "lo_export", "information_schema", "pg_catalog", "pg_shadow", "pg_authid", "current_setting", "set_config", + "union", "intersect", "except", } var identWordRe = regexp.MustCompile(`[A-Za-z_][A-Za-z0-9_]*`) @@ -81,6 +92,216 @@ func invalidQuery(format string, args ...any) error { return &queryValidationError{msg: fmt.Sprintf(format, args...)} } +// placeholderComparisonRe matches "project_id = {{project_id}}" or +// "{{project_id}} = project_id" (optionally table/alias-qualified, e.g. +// "t.project_id"), run against the *redacted* query text (see +// redactForStructuralChecks) rather than the raw query. Two things narrow +// this beyond a bare "some column somewhere": +// +// - The column name must literally be project_id, not any identifier. +// Every real use in this file's own docs/tests filters on project_id; +// requiring the name closes a decoy like +// "FROM tasks, projects WHERE projects.id = {{project_id}}" — a +// cross join whose only placeholder use filters an unrelated table's +// primary key, leaving the actually-returned tasks rows unfiltered. +// - Running against the redacted text means a match can't be hiding +// inside a string literal or a nested subquery — see +// redactForStructuralChecks's doc comment for why the latter matters. +var placeholderComparisonRe = regexp.MustCompile( + `(?i)(?:[A-Za-z_][A-Za-z0-9_]*\.)?project_id\s*=\s*` + regexp.QuoteMeta(placeholderToken) + + `|` + regexp.QuoteMeta(placeholderToken) + `\s*=\s*(?:[A-Za-z_][A-Za-z0-9_]*\.)?project_id`, +) + +// validateProjectScopePlaceholder enforces that {{project_id}} appears +// exactly once and is used as a direct, unconditional equality filter +// against a real column — closing two concrete bypasses that a bare +// "contains the placeholder somewhere" check allows through: +// +// - A duplicated/self-compared placeholder, e.g. +// "WHERE '{{project_id}}' = '{{project_id}}'", which becomes an +// always-true tautology once both sides are substituted with the same +// bound value — the old check only verified the token appeared +// *somewhere*, so this passed. +// - The placeholder appearing correctly, but alongside a bare OR that can +// make the whole filter match regardless of project, e.g. +// "WHERE 1=1 OR project_id = {{project_id}}". Reasoning about which +// OR/parenthesis nestings are "safe" would require a real SQL parser +// (a parenthesized OR can still be just as unsafe as a bare one, e.g. +// "WHERE (other_col = 5 OR project_id = {{project_id}})" — the parens +// don't stop it from being the entire filter), so OR is rejected +// outright for project/integration-scoped queries rather than +// attempting to classify individual uses as safe. Use IN (...) for a +// multi-value filter instead. +func validateProjectScopePlaceholder(query string) error { + count := strings.Count(query, placeholderToken) + if count == 0 { + return invalidQuery( + "query must scope itself to the current project using the %s placeholder somewhere in a WHERE/ON/HAVING clause (e.g. \"WHERE project_id = %s\")", + placeholderToken, placeholderToken, + ) + } + if count > 1 { + return invalidQuery("the %s placeholder must appear exactly once", placeholderToken) + } + + redacted := redactForStructuralChecks(query) + + if hasBareOr(redacted) { + return invalidQuery("OR is not allowed in panel queries, since it could bypass the project filter — use IN (...) for a multi-value filter instead") + } + if !placeholderComparisonRe.MatchString(redacted) { + return invalidQuery( + "the %s placeholder must be used as a direct equality filter against a project_id column, at the query's own top level (not inside a nested subquery), e.g. \"WHERE project_id = %s\" — it cannot be compared with anything other than \"=\", against a different column, duplicated, or embedded inside a string literal or larger expression", + placeholderToken, placeholderToken, + ) + } + return nil +} + +// redactForStructuralChecks returns a same-length copy of query with the +// content of every single-quoted string literal, double-quoted identifier, +// and nested SELECT/WITH subquery (anything inside parentheses that open +// with SELECT or WITH, at any depth) blanked out to spaces. hasBareOr and +// placeholderComparisonRe both run against this redacted form instead of +// the raw query, for two independent reasons: +// +// - String-literal content can contain a stray quote, paren, or +// keyword-looking text that isn't SQL syntax at all. +// - A nested subquery can contain a placeholder-equality that satisfies +// placeholderComparisonRe by pure text matching without actually +// constraining which rows the *outer* query returns — e.g. +// "SELECT id,title FROM tasks WHERE (SELECT 1 FROM projects WHERE +// id={{project_id}}) IS NOT NULL" filters on an unrelated existence +// check while every row of tasks (every project's) comes back +// unfiltered. Blanking nested-subquery spans means a placeholder +// hiding in one no longer counts as a valid top-level filter, so this +// query now correctly falls through to "no placeholder used as a +// direct equality filter" and is rejected. Legitimate queries are +// expected to place the filter in their own top-level WHERE (every +// example in this file's docs/tests already does), so this doesn't +// restrict the documented usage pattern. +func redactForStructuralChecks(query string) string { + runes := []rune(query) + out := make([]rune, len(runes)) + copy(out, runes) + + inSingle, inDouble := false, false + var subqueryParen []bool // one entry per currently-open paren; true if it opens a nested SELECT/WITH + + insideSubquery := func() bool { + for _, sq := range subqueryParen { + if sq { + return true + } + } + return false + } + + for i := 0; i < len(runes); i++ { + c := runes[i] + + if inSingle { + if c == '\'' { + if i+1 < len(runes) && runes[i+1] == '\'' { + out[i], out[i+1] = ' ', ' ' + i++ + continue + } + inSingle = false + continue + } + out[i] = ' ' + continue + } + if inDouble { + if c == '"' { + if i+1 < len(runes) && runes[i+1] == '"' { + out[i], out[i+1] = ' ', ' ' + i++ + continue + } + inDouble = false + continue + } + out[i] = ' ' + continue + } + + switch c { + case '\'': + inSingle = true + continue + case '"': + inDouble = true + continue + case '(': + subqueryParen = append(subqueryParen, startsWithSelectOrWith(runes[i+1:])) + continue + case ')': + if len(subqueryParen) > 0 { + subqueryParen = subqueryParen[:len(subqueryParen)-1] + } + continue + } + + if insideSubquery() { + out[i] = ' ' + } + } + return string(out) +} + +// startsWithSelectOrWith reports whether after, everything immediately +// following an opening paren, begins (modulo leading whitespace) with the +// keyword SELECT or WITH — i.e. whether that paren opens a nested +// subquery/CTE rather than a plain grouping expression like +// "(project_id = {{project_id}})" or "(a = 1 AND b = 2)". +func startsWithSelectOrWith(after []rune) bool { + i := 0 + for i < len(after) && (after[i] == ' ' || after[i] == '\t' || after[i] == '\n' || after[i] == '\r') { + i++ + } + return isWordBoundaryKeyword(after, i, "select") || isWordBoundaryKeyword(after, i, "with") +} + +// hasBareOr reports whether query contains the keyword OR outside of a +// string literal (single- or double-quoted — see redactForStructuralChecks, +// which callers are expected to have already applied) and not as part of a +// longer identifier (e.g. "corporation", "order"). +func hasBareOr(query string) bool { + runes := []rune(query) + for i := 0; i < len(runes); i++ { + if isWordBoundaryKeyword(runes, i, "or") { + return true + } + } + return false +} + +// isWordBoundaryKeyword reports whether the case-insensitive keyword kw +// occurs at position i in runes, bounded by non-identifier characters (or +// the string's edges) on both sides. +func isWordBoundaryKeyword(runes []rune, i int, kw string) bool { + n := len(kw) + if i+n > len(runes) { + return false + } + if !strings.EqualFold(string(runes[i:i+n]), kw) { + return false + } + if i > 0 && isIdentRune(runes[i-1]) { + return false + } + if i+n < len(runes) && isIdentRune(runes[i+n]) { + return false + } + return true +} + +func isIdentRune(r rune) bool { + return r == '_' || (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') +} + // validateQuery checks a raw panel query string against the safety model // described above. requireProjectScope is true for 'project' and // 'integration' scope panels (false for 'admin' scope, which is @@ -121,11 +342,8 @@ func validateQuery(raw string, requireProjectScope bool) (string, error) { } if requireProjectScope { - if !strings.Contains(trimmed, placeholderToken) { - return "", invalidQuery( - "query must scope itself to the current project using the %s placeholder somewhere in a WHERE/ON/HAVING clause (e.g. \"WHERE project_id = %s\")", - placeholderToken, placeholderToken, - ) + if err := validateProjectScopePlaceholder(trimmed); err != nil { + return "", err } // $1 is reserved for the injected project_id; reject any other // use of $1 so we don't silently override a user's own parameter. diff --git a/backend/views.go b/backend/views.go index 9c0ec4d..7acae32 100644 --- a/backend/views.go +++ b/backend/views.go @@ -23,6 +23,15 @@ import ( // /projects/:projectId/dashboard/view via the manifest). Returns the // project's single dashboard, creating an empty one on first visit. func (p *dashboardPlugin) getOrCreateProjectView(req *plugin.Request, res *plugin.Response) { + // Defense-in-depth: this route is already gated at the manifest level + // (requirePermissions dashboard.view), but every other project-scope + // entry point re-checks in-handler too (see loadViewWithPanels below), + // so this one shouldn't be the sole exception if the manifest is ever + // misedited. + if !p.canViewDashboard() { + res.Error(403, "you don't have permission to view this dashboard") + return + } projectID := req.Caller.ProjectID view, err := p.fetchOrCreateSingletonView(projectID, "project", "", "Dashboard", req.Caller.CallerID) if err != nil { @@ -58,6 +67,14 @@ func (p *dashboardPlugin) getOrCreateAdminView(req *plugin.Request, res *plugin. // visit — same get-or-create-singleton shape as the project/admin scopes, // just keyed by hostViewId instead of projectID/"". func (p *dashboardPlugin) getOrCreateIntegrationView(req *plugin.Request, res *plugin.Response) { + // Defense-in-depth, mirroring getOrCreateProjectView above: this route + // is already gated at the manifest level (requirePermissions + // dashboard.view) so this one shouldn't be the sole exception if the + // manifest is ever misedited. + if !p.canViewDashboard() { + res.Error(403, "you don't have permission to view this dashboard") + return + } projectID := req.Caller.ProjectID hostViewID := req.PathParam("hostViewId") if hostViewID == "" { @@ -253,6 +270,24 @@ func (p *dashboardPlugin) loadViewWithPanels(viewID, projectID string, res *plug return nil, false } + // Permission check: views whose dashboard_views.scope is "project" or + // "integration" require dashboard.view/dashboard.manage — a + // project-scope permission pair, checked against the caller's + // per-project permission map. The manifest already enforces this + // uniformly for every route that reaches here (see plugin.json); this + // in-handler check is defense-in-depth so it isn't the sole + // enforcement if the manifest is ever misedited. Rows with + // dashboard_views.scope == "admin" are excluded from this check: they + // only ever reach here via the admin routes (projectID == ""), which + // are already fully gated at the manifest level by dashboard.view/ + // dashboard.manage checked at permission scope "global" — not "admin"; + // the permission system has no such scope, dashboard_views.scope is an + // unrelated data-model value that happens to share the word "admin". + if v.Scope != "admin" && !p.canViewDashboard() { + res.Error(403, "you don't have permission to view this dashboard") + return nil, false + } + panels, err := p.fetchPanelsForView(v.ID) if err != nil { p.log.Error("loadViewWithPanels panels: " + err.Error()) @@ -263,6 +298,16 @@ func (p *dashboardPlugin) loadViewWithPanels(viewID, projectID string, res *plug return &v, true } +// canViewDashboard reports whether the caller may view a project-scope +// dashboard — dashboard.view, or dashboard.manage as a superset (a caller +// who can manage a dashboard can always view it too). Single source of +// truth for that view-or-manage relationship, mirroring canManagePanel's +// shape in panels.go, so the three call sites above stay in sync rather +// than each hand-rolling the same two-permission check. +func (p *dashboardPlugin) canViewDashboard() bool { + return p.perm.Check("dashboard.view") || p.perm.Check("dashboard.manage") +} + func viewFromRow(cols []string, row []any) dashboardView { sc := newRowScanner(cols, row) return dashboardView{ diff --git a/plugin.json b/plugin.json index 1eb0283..3ee185e 100644 --- a/plugin.json +++ b/plugin.json @@ -2,9 +2,35 @@ "id": "com.paca.dashboard", "displayName": "Dashboard", "description": "Customizable panel-based dashboard builder (charts, tables, text) across the project dashboard page, the admin dashboard page, and a single dashboard per Integration-page view (backlog/sprint/timeline).", - "version": "0.3.4", + "version": "0.4.0", "minCoreVersion": "v0.13.3", "permissions": ["db.read", "db.write", "events.subscribe", "cache"], + "customPermissions": [ + { + "key": "dashboard.view", + "label": "View dashboard", + "description": "View the project's \"Dashboard\" page and its panels. Does not affect the per-view dashboards embedded in Backlog/Sprint/Timeline pages, which any project member can already see.", + "scope": "project" + }, + { + "key": "dashboard.manage", + "label": "Manage dashboard", + "description": "Create, edit, delete, and rearrange panels on the project's \"Dashboard\" page, and run query previews while authoring them.", + "scope": "project" + }, + { + "key": "dashboard.view", + "label": "View dashboard", + "description": "View the admin-sidebar \"Dashboard\" page — the instance-wide, cross-project dashboard.", + "scope": "global" + }, + { + "key": "dashboard.manage", + "label": "Manage dashboard", + "description": "Create, edit, delete, and rearrange panels on the admin-sidebar \"Dashboard\" page, and run query previews while authoring them.", + "scope": "global" + } + ], "backend": { "eventSubscriptions": [], "routes": [ @@ -14,7 +40,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.read"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -23,7 +49,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.read"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -32,7 +58,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.read"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -41,7 +67,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -50,7 +76,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -59,7 +85,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -68,7 +94,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -77,7 +103,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.read"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -86,7 +112,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -95,7 +121,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.view"] } ] }, { @@ -104,7 +130,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] }, { @@ -113,7 +139,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] }, { @@ -122,7 +148,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] }, { @@ -131,7 +157,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] }, { @@ -140,7 +166,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.view"] } ] }, { @@ -149,7 +175,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] } ] @@ -181,7 +207,8 @@ "label": "Dashboard", "icon": "LayoutDashboard", "component": "ProjectDashboardPage", - "order": 5 + "order": 5, + "requiredPermission": "dashboard.view" }, { "scope": "admin", @@ -189,7 +216,8 @@ "label": "Dashboard", "icon": "LayoutDashboard", "component": "AdminDashboardPage", - "order": 5 + "order": 5, + "requiredPermission": "dashboard.view" } ] },