added full access row to operator modal, plus a test for the last-manager guard
The "*" wildcard was never in assignablePermissions, so an operator holding it (the one the first-run wizard creates) showed every box unticked while having every right, and there was no way to take it away. Its own row fixes both; the grid is disabled while it is on, since normalisePermissions collapses "*" plus anything back to "*". That made guardManagerRemoval reachable from the UI for the first time, so it now has an integration test covering the wildcard match and the enabled filter in its SQL.
This commit is contained in:
parent
c04a8ddc6a
commit
7ad68c3983
4 changed files with 136 additions and 5 deletions
101
cmd/telesrv-admin/adminusers_guard_integration_test.go
Normal file
101
cmd/telesrv-admin/adminusers_guard_integration_test.go
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strconv"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// guardManagerRemoval is the only thing between an operator and a console
|
||||
// nobody can administer, and it decides from a COUNT over admin_console_users
|
||||
// whose predicate treats '*' as holding every right. Neither that wildcard
|
||||
// matching nor the enabled filter can be proven anywhere but against the real
|
||||
// table, so this is an integration test, gated on TELESRV_TEST_POSTGRES_DSN
|
||||
// like the rest of the package.
|
||||
//
|
||||
// The count spans every enabled manager except the row being edited, so unlike
|
||||
// the other integration tests here this one cannot keep to its own fixtures
|
||||
// with a unique suffix -- a manager left behind by an earlier run would make
|
||||
// the "nobody else" cases silently pass for the wrong reason. It empties
|
||||
// admin_console_users instead; verificationReadStore refuses a DSN whose
|
||||
// database name does not contain "test", which is what makes that safe, and no
|
||||
// other test in the repo touches this table.
|
||||
func TestGuardManagerRemovalIntegration(t *testing.T) {
|
||||
store, pool := verificationReadStore(t)
|
||||
srv := &server{read: store}
|
||||
ctx := context.Background()
|
||||
|
||||
resetTable := func() {
|
||||
t.Helper()
|
||||
if _, err := pool.Exec(ctx, `TRUNCATE admin_console_users RESTART IDENTITY`); err != nil {
|
||||
t.Fatalf("truncate admin_console_users: %v", err)
|
||||
}
|
||||
}
|
||||
resetTable()
|
||||
t.Cleanup(resetTable)
|
||||
|
||||
unique := time.Now().UnixNano() & 0x7fffffff
|
||||
insertOperator := func(permissions []string, enabled bool) int64 {
|
||||
t.Helper()
|
||||
unique++
|
||||
var id int64
|
||||
if err := pool.QueryRow(ctx, `
|
||||
INSERT INTO admin_console_users (username, password_hash, permissions, enabled)
|
||||
VALUES ($1, 'not-a-real-hash', $2, $3)
|
||||
RETURNING id`, "guardop"+strconv.FormatInt(unique, 10), permissions, enabled).Scan(&id); err != nil {
|
||||
t.Fatalf("insert operator: %v", err)
|
||||
}
|
||||
return id
|
||||
}
|
||||
|
||||
t.Run("sole wildcard holder cannot drop the wildcard", func(t *testing.T) {
|
||||
resetTable()
|
||||
id := insertOperator([]string{permissionAll}, true)
|
||||
err := srv.guardManagerRemoval(ctx, id, []string{permissionAccountsRead}, true)
|
||||
if !errors.Is(err, errLastManagerStanding) {
|
||||
t.Fatalf("err = %v, want errLastManagerStanding", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("sole manager cannot disable itself", func(t *testing.T) {
|
||||
resetTable()
|
||||
id := insertOperator([]string{permissionAll}, true)
|
||||
err := srv.guardManagerRemoval(ctx, id, []string{permissionAll}, false)
|
||||
if !errors.Is(err, errLastManagerStanding) {
|
||||
t.Fatalf("err = %v, want errLastManagerStanding", err)
|
||||
}
|
||||
})
|
||||
|
||||
// Narrowing the wildcard down to the managing right itself is the supported
|
||||
// way out of full access, so it must not trip the guard.
|
||||
t.Run("sole manager may trade the wildcard for admins.manage", func(t *testing.T) {
|
||||
resetTable()
|
||||
id := insertOperator([]string{permissionAll}, true)
|
||||
if err := srv.guardManagerRemoval(ctx, id, []string{permissionAdminsManage}, true); err != nil {
|
||||
t.Fatalf("err = %v, want nil", err)
|
||||
}
|
||||
})
|
||||
|
||||
// The case the SQL's '*' arm exists for: the remaining manager holds the
|
||||
// wildcard rather than a literal admins.manage, and must still be counted.
|
||||
t.Run("another enabled wildcard holder counts as a manager", func(t *testing.T) {
|
||||
resetTable()
|
||||
id := insertOperator([]string{permissionAll}, true)
|
||||
insertOperator([]string{permissionAll}, true)
|
||||
if err := srv.guardManagerRemoval(ctx, id, []string{permissionAccountsRead}, true); err != nil {
|
||||
t.Fatalf("err = %v, want nil", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("a disabled second manager does not count", func(t *testing.T) {
|
||||
resetTable()
|
||||
id := insertOperator([]string{permissionAll}, true)
|
||||
insertOperator([]string{permissionAdminsManage}, false)
|
||||
err := srv.guardManagerRemoval(ctx, id, []string{permissionAccountsRead}, true)
|
||||
if !errors.Is(err, errLastManagerStanding) {
|
||||
t.Fatalf("err = %v, want errLastManagerStanding", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
File diff suppressed because one or more lines are too long
2
cmd/telesrv-admin/web/dist/index.html
vendored
2
cmd/telesrv-admin/web/dist/index.html
vendored
|
|
@ -23,7 +23,7 @@
|
|||
})();
|
||||
</script>
|
||||
|
||||
<script type="module" crossorigin src="/assets/index-3atviP2p.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-DTpNyCcP.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-P_k7ini0.css">
|
||||
</head>
|
||||
<body>
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import { createPortal } from "react-dom";
|
|||
import { api, errorMessage } from "../api";
|
||||
import { ActionButton } from "../components/ActionButton";
|
||||
import { Alert, EmptyRow, LoadingRow, PageFrame, QueryPanel, SectionHead } from "../components/ui";
|
||||
import { groupPermissions, permissionHint, permissionTitle } from "../permissions";
|
||||
import { groupPermissions, permissionAll, permissionHint, permissionTitle } from "../permissions";
|
||||
import type { AdminConsoleUser, AdminConsoleSystemOperator } from "../types";
|
||||
|
||||
// The operator-accounts screen. The table only reports; every change happens in
|
||||
|
|
@ -165,19 +165,42 @@ function PermissionChips({ permissions }: { permissions: string[] }) {
|
|||
//
|
||||
// The raw permission string stays as each row's tooltip, so the screen never
|
||||
// hides what is actually being stored.
|
||||
//
|
||||
// "*" gets its own row rather than a box in the grid below, because
|
||||
// assignablePermissions() deliberately leaves it out of the assignable list
|
||||
// (cmd/telesrv-admin/security.go) -- without this row an operator holding the
|
||||
// wildcard, like the one the first-run wizard creates, renders as every box
|
||||
// unticked while Has() answers true for everything, and there is no way to take
|
||||
// it away again. While it is on the grid is disabled: normalisePermissions
|
||||
// collapses "*" plus anything back to just "*", so ticking a box there would be
|
||||
// a no-op the screen would otherwise show as a change.
|
||||
function PermissionPicker({
|
||||
available,
|
||||
selected,
|
||||
onToggle,
|
||||
onToggleGroup
|
||||
onToggleGroup,
|
||||
onToggleAll
|
||||
}: {
|
||||
available: string[];
|
||||
selected: string[];
|
||||
onToggle: (permission: string, on: boolean) => void;
|
||||
onToggleGroup: (permissions: string[], on: boolean) => void;
|
||||
onToggleAll: (on: boolean) => void;
|
||||
}) {
|
||||
const full = selected.includes(permissionAll);
|
||||
return (
|
||||
<div className="permission-groups">
|
||||
<section className="permission-group">
|
||||
<div className="permission-grid">
|
||||
<label className="permission-item" title={permissionAll}>
|
||||
<input type="checkbox" checked={full} onChange={(event) => onToggleAll(event.target.checked)} />
|
||||
<span className="permission-copy">
|
||||
<strong>{"Full access"}</strong>
|
||||
<small>{"Every right below, including ones added in future updates. Turn off to pick rights individually."}</small>
|
||||
</span>
|
||||
</label>
|
||||
</div>
|
||||
</section>
|
||||
{groupPermissions(available).map((group) => {
|
||||
const all = group.permissions.every((p) => selected.includes(p));
|
||||
return (
|
||||
|
|
@ -190,6 +213,7 @@ function PermissionPicker({
|
|||
<button
|
||||
className="btn compact"
|
||||
type="button"
|
||||
disabled={full}
|
||||
onClick={() => onToggleGroup(group.permissions, !all)}
|
||||
>
|
||||
{all ? "Clear" : "Select all"}
|
||||
|
|
@ -200,7 +224,8 @@ function PermissionPicker({
|
|||
<label className="permission-item" key={permission} title={permission}>
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={selected.includes(permission)}
|
||||
checked={full || selected.includes(permission)}
|
||||
disabled={full}
|
||||
onChange={(event) => onToggle(permission, event.target.checked)}
|
||||
/>
|
||||
<span className="permission-copy">
|
||||
|
|
@ -301,6 +326,11 @@ function OperatorModal({
|
|||
: current.filter((p) => !group.includes(p))
|
||||
)
|
||||
}
|
||||
onToggleAll={(on) =>
|
||||
setPermissions((current) =>
|
||||
on ? [permissionAll] : current.filter((p) => p !== permissionAll)
|
||||
)
|
||||
}
|
||||
/>
|
||||
|
||||
<label className="permission-item standalone">
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue