diff --git a/internal/core/endpoints_repeater_test.go b/internal/core/endpoints_repeater_test.go index a1df3d8..5597a13 100644 --- a/internal/core/endpoints_repeater_test.go +++ b/internal/core/endpoints_repeater_test.go @@ -193,16 +193,116 @@ func TestRepeaterSharePosts(t *testing.T) { un.Body.Close() assertRedirect(t, un, share, "unshare") - // #87 participation: exclude this repeater from an org the owner belongs to. + // #87 participation: the per-org "Shared" switch. Unchecked submits no + // "include" field (opt out); checked submits include=1 (participate). org, err := st.CreateOrg(ctx, "Participation Org", owner.ID) if err != nil { t.Fatal(err) } + excluded := func() bool { + orgs, err := st.ListRepeaterOrgMemberships(ctx, rep.ID) + if err != nil { + t.Fatalf("list memberships: %v", err) + } + for _, o := range orgs { + if o.OrgID == org.ID { + return o.Excluded + } + } + t.Fatalf("org %d not in memberships", org.ID) + return false + } // The {orgID} route param is the org slug, not the numeric id. - part := post(t, ts, h.app, "/repeaters/"+pid+"/orgs/"+org.Slug+"/participation", - url.Values{"action": {"exclude"}}, sess) - part.Body.Close() - assertRedirect(t, part, share, "org participation") + partURL := "/repeaters/" + pid + "/orgs/" + org.Slug + "/participation" + out := post(t, ts, h.app, partURL, url.Values{}, sess) // switch off → opt out + out.Body.Close() + assertRedirect(t, out, share, "org opt out") + if !excluded() { + t.Fatal("switch off did not opt the repeater out") + } + in := post(t, ts, h.app, partURL, url.Values{"include": {"1"}}, sess) // switch on + in.Body.Close() + assertRedirect(t, in, share, "org opt in") + if excluded() { + t.Fatal("switch on did not re-include the repeater") + } +} + +// TestRepeaterOrgLimitsPosts covers the per-(repeater, org) command-limits modal: +// the GET fragment renders the editor, and the POST restricts / collapses back to +// permissive. This is the share-page home for limits after they moved off the +// org-wide page and became per repeater. +func TestRepeaterOrgLimitsPosts(t *testing.T) { + t.Parallel() + st, ctx, ts, h := splitServer(t) + owner, sess := appLogin(t, ts, st, ctx, h.app, "limitowner") + rep := newOwnedRepeater(t, st, ctx, owner.ID, "Limited Rep") + org, err := st.CreateOrg(ctx, "Limits Org", owner.ID) // owner is an admin member + if err != nil { + t.Fatal(err) + } + base := "/repeaters/" + rep.PublicID + "/orgs/" + org.Slug + "/limits" + + // The ceiling: commands an org may ever run. Restrict to the first one. + catalog, err := st.ListCommands(ctx) + if err != nil { + t.Fatal(err) + } + var ceiling []int64 + for _, c := range catalog { + if c.OrgMemberAllowed || c.OrgAdminAllowed { + ceiling = append(ceiling, c.ID) + } + } + if len(ceiling) < 2 { + t.Fatalf("need >=2 ceiling commands, got %d", len(ceiling)) + } + + // GET renders the modal fragment (no page chrome), with the org name and cmd boxes. + frag := readBody(t, do(t, ts, h.app, base, sess)) + if !strings.Contains(frag, "Command limits") || !strings.Contains(frag, `name="cmd"`) { + t.Fatalf("limits fragment missing expected content:\n%s", frag) + } + if strings.Contains(frag, "back-link") { + t.Fatal("limits fragment should be modal chrome, not a full page") + } + + optIn := func() []int64 { + ids, err := st.RepeaterOrgOptInCommandIDs(ctx, org.ID, rep.ID) + if err != nil { + t.Fatalf("opt-in ids: %v", err) + } + return ids + } + + // Restrict to exactly the first ceiling command. + save := post(t, ts, h.app, base, url.Values{"cmd": {strconv.FormatInt(ceiling[0], 10)}}, sess) + save.Body.Close() + assertRedirect(t, save, "/repeaters/"+rep.PublicID+"/share", "save limits") + if got := optIn(); len(got) != 1 || got[0] != ceiling[0] { + t.Fatalf("opt-in = %v, want [%d]", got, ceiling[0]) + } + + // Selecting the full ceiling collapses back to permissive (no rows stored). + full := url.Values{} + for _, id := range ceiling { + full.Add("cmd", strconv.FormatInt(id, 10)) + } + fullResp := post(t, ts, h.app, base, full, sess) + fullResp.Body.Close() + assertRedirect(t, fullResp, "/repeaters/"+rep.PublicID+"/share", "save full ceiling") + if got := optIn(); len(got) != 0 { + t.Fatalf("full selection should store nothing (permissive), got %v", got) + } + + // Restrict again, then "Remove restriction" clears it. + post(t, ts, h.app, base, url.Values{"cmd": {strconv.FormatInt(ceiling[0], 10)}}, sess).Body.Close() + clr := post(t, ts, h.app, base, url.Values{"clear": {"1"}}, sess) + clr.Body.Close() + assertRedirect(t, clr, "/repeaters/"+rep.PublicID+"/share", "clear limits") + if got := optIn(); len(got) != 0 { + t.Fatalf("clear should remove all rows, got %v", got) + } } // #88 POST /invite/{token}/accept — a second user redeems a share link. diff --git a/internal/core/ignored_errors_test.go b/internal/core/ignored_errors_test.go index ca04ac2..5aeb2a3 100644 --- a/internal/core/ignored_errors_test.go +++ b/internal/core/ignored_errors_test.go @@ -42,3 +42,29 @@ func TestShareCommandsPageReadErrorFailsClosed(t *testing.T) { t.Fatalf("status = %d, want 500 (a swallowed error would render 200 with an empty, data-wiping form)", resp.StatusCode) } } + +// TestRepeaterOrgLimitsReadErrorFailsClosed: if loading a repeater's per-org opt-in +// list fails, the limits modal must 500 rather than render as "permissive" (all +// checked), which a Save would persist as clearing the real restriction. +func TestRepeaterOrgLimitsReadErrorFailsClosed(t *testing.T) { + t.Parallel() + st, ctx, ts, h := splitServer(t) + owner, sess := appLogin(t, ts, st, ctx, h.app, "limitowner") + rep := newOwnedRepeater(t, st, ctx, owner.ID, "Rep") + org, err := st.CreateOrg(ctx, "Org", owner.ID) // creator is an admin member + if err != nil { + t.Fatal(err) + } + + // Make RepeaterOrgOptInCommandIDs fail; the handler's earlier queries (org, + // membership, catalog ceiling) use other tables. + if _, err := st.Pool().Exec(ctx, `DROP TABLE org_repeater_command_optin`); err != nil { + t.Fatalf("drop org_repeater_command_optin: %v", err) + } + + resp := do(t, ts, h.app, "/repeaters/"+rep.PublicID+"/orgs/"+org.Slug+"/limits", sess) + resp.Body.Close() + if resp.StatusCode != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500 (a swallowed error would render 200 as permissive, wiping the restriction on save)", resp.StatusCode) + } +} diff --git a/internal/core/org_participation.go b/internal/core/org_participation.go index a6e29b2..c93526d 100644 --- a/internal/core/org_participation.go +++ b/internal/core/org_participation.go @@ -2,6 +2,7 @@ package core import ( "net/http" + "strconv" "github.com/go-chi/chi/v5" @@ -33,16 +34,127 @@ func (s *Handlers) repeaterOrgContext(w http.ResponseWriter, r *http.Request) (* // handleSetRepeaterOrg opts a repeater into or out of an org (owner only). A // repeater participates in every org its owner belongs to by default; this writes -// or clears the opt-out. +// or clears the opt-out. Driven by the share page's per-org "Shared" switch: a +// checked switch submits include=1 (participate), unchecked submits nothing (opt +// out). func (s *Handlers) handleSetRepeaterOrg(w http.ResponseWriter, r *http.Request) { rep, orgID, ok := s.repeaterOrgContext(w, r) if !ok { return } - exclude := r.FormValue("action") == "exclude" + exclude := r.FormValue("include") != "1" if err := s.Store.SetRepeaterOrgExcluded(r.Context(), orgID, rep.ID, exclude); err != nil { s.ServerError(w, r, "could not update participation", err) return } http.Redirect(w, r, sharePath(rep.PublicID), http.StatusSeeOther) } + +// pageRepeaterOrgLimits renders the per-org command-limits modal fragment for one +// repeater: which of the commands the org may run are allowed to run on this box. +// No opt-in rows = permissive (every ceiling command checked). Editable regardless +// of participation, so an owner can pre-set limits before opting an org back in. +func (s *Handlers) pageRepeaterOrgLimits(w http.ResponseWriter, r *http.Request) { + rep, orgID, ok := s.repeaterOrgContext(w, r) + if !ok { + return + } + org, err := s.Store.GetOrg(r.Context(), orgID) + if err != nil { + s.NotFound(w, r) + return + } + ceiling, err := s.orgCeilingCommands(r) + if err != nil { + s.ServerError(w, r, "could not load commands", err) + return + } + // Must not swallow this error: an empty list reads as "permissive" (everything + // checked), and saving that would clear a real restriction. + optIn, err := s.Store.RepeaterOrgOptInCommandIDs(r.Context(), orgID, rep.ID) + if err != nil { + s.ServerError(w, r, "could not load commands", err) + return + } + restricted := len(optIn) > 0 + checked := make(map[int64]bool, len(ceiling)) + if restricted { + for _, cid := range optIn { + checked[cid] = true + } + } else { + for _, c := range ceiling { + checked[c.ID] = true + } + } + s.Render(w, r, "share_org_limits.html", map[string]any{ + "Repeater": rep, + "Org": org, + "Groups": groupCommands(ceiling, checked), + "Restricted": restricted, + "ShowAccess": true, + "Layout": "org-limits-modal", + }) +} + +// handleSaveRepeaterOrgLimits saves the per-(repeater, org) command opt-in list. A +// full selection (or the "Remove restriction" button) clears it back to permissive +// so we don't persist a redundant full allowlist. +func (s *Handlers) handleSaveRepeaterOrgLimits(w http.ResponseWriter, r *http.Request) { + rep, orgID, ok := s.repeaterOrgContext(w, r) + if !ok { + return + } + if err := r.ParseForm(); err != nil { + http.Error(w, "bad form", http.StatusBadRequest) + return + } + var chosen []int64 + // "Remove restriction" clears the list regardless of checkboxes. + if r.FormValue("clear") == "" { + ceiling, err := s.orgCeilingCommands(r) + if err != nil { + s.ServerError(w, r, "could not load commands", err) + return + } + chosen = parseCommandIDs(r.Form["cmd"]) + // Selecting the full ceiling is equivalent to permissive — store nothing. + if len(chosen) >= len(ceiling) { + chosen = nil + } + } + if err := s.Store.SetRepeaterOrgOptIn(r.Context(), orgID, rep.ID, chosen); err != nil { + s.ServerError(w, r, "could not save limits", err) + return + } + http.Redirect(w, r, sharePath(rep.PublicID), http.StatusSeeOther) +} + +// orgCeilingCommands returns the catalog commands an org is ever permitted to run +// (member or admin tier) — the universe the per-repeater opt-in editor restricts +// within. +func (s *Handlers) orgCeilingCommands(r *http.Request) ([]*store.Command, error) { + catalog, err := s.Store.ListCommands(r.Context()) + if err != nil { + return nil, err + } + var out []*store.Command + for _, c := range catalog { + if c.OrgMemberAllowed || c.OrgAdminAllowed { + out = append(out, c) + } + } + return out, nil +} + +// parseCommandIDs parses form values into catalog command ids, skipping any that +// aren't valid integers. +func parseCommandIDs(values []string) []int64 { + var ids []int64 + for _, v := range values { + if cid, err := strconv.ParseInt(v, 10, 64); err == nil { + ids = append(ids, cid) + } + } + return ids +} diff --git a/internal/core/shares.go b/internal/core/shares.go index da48978..129066b 100644 --- a/internal/core/shares.go +++ b/internal/core/shares.go @@ -295,12 +295,7 @@ func (s *Handlers) handleSetShareCommands(w http.ResponseWriter, r *http.Request http.Error(w, "bad form", http.StatusBadRequest) return } - var cmdIDs []int64 - for _, v := range r.Form["cmd"] { - if cid, err := strconv.ParseInt(v, 10, 64); err == nil { - cmdIDs = append(cmdIDs, cid) - } - } + cmdIDs := parseCommandIDs(r.Form["cmd"]) if err := s.Store.SetShareCommands(r.Context(), id, targetID, cmdIDs); err != nil { s.ServerError(w, r, "could not save commands", err) return diff --git a/internal/core/templates/share.html b/internal/core/templates/share.html index 30f7a38..e79097d 100644 --- a/internal/core/templates/share.html +++ b/internal/core/templates/share.html @@ -13,45 +13,9 @@ -
-
-

Organizations

-

- This repeater is shared with every organization you belong to, so their admins and members can run - the commands those organizations are permitted to run on it. Opt it out of any organization here. To - limit which commands an organization may run across all your shared repeaters, use - Actions → Limit commands on that organization's page. -

- - {{if .Orgs}} -
- {{range .Orgs}} -
- {{.OrgName}} - {{if .Excluded}} - Opted out - {{else}} - Shared - {{end}} -
- {{if .Excluded}} - - - {{else}} - - - {{end}} -
-
- {{end}} -
- {{else}} -

Join an organization to share this repeater with it.

- {{end}} -
-
- -
+
+
+

People

@@ -125,6 +89,61 @@ {{end}}

+
+ +
+
+
+

Organizations

+

+ This repeater is shared with every organization you belong to, so their admins and members can run the + commands those organizations are permitted to run on it. Turn an organization off to opt out, or edit + its command limits for this repeater. +

+ + {{if .Orgs}} +
+ {{range .Orgs}} +
+
+ {{.OrgName}} + {{if .Excluded}} + Opted out + {{else if .Restricted}} + Limited commands + {{else}} + All commands + {{end}} +
+ +
+ +
+
+
+
+ {{end}} +
+ {{else}} +

Join an organization to share this repeater with it.

+ {{end}} +
+
+
+
+ + + {{template "icon-arrow-left" "me-1"}}Back to dashboard {{end}} diff --git a/internal/core/templates/share_commands.html b/internal/core/templates/share_commands.html index 6950165..e51d8ae 100644 --- a/internal/core/templates/share_commands.html +++ b/internal/core/templates/share_commands.html @@ -18,36 +18,10 @@
- {{range .Groups}} -
-

{{.Name}}

-
- {{range .Commands}} - - {{end}} -
-
- {{end}} + {{template "command-grid" .}}
Cancel
- {{end}} diff --git a/internal/core/templates/share_org_limits.html b/internal/core/templates/share_org_limits.html new file mode 100644 index 0000000..d1d0028 --- /dev/null +++ b/internal/core/templates/share_org_limits.html @@ -0,0 +1,29 @@ +{{/* org-limits-modal is the htmx fragment swapped into the share page's shared + #limits-modal. The form posts normally (full navigation back to the share + page), which also closes the modal — no client JS needed. */}} +{{define "org-limits-modal"}} +
+
Command limits — {{.Org.Name}}
+ +
+
+
+

+ Restrict which of the commands {{.Org.Name}} is permitted to run actually run on + “{{.Repeater.Name}}”. + {{if .Restricted}}Only the checked commands are allowed.{{else}}No restriction is set — every command + the organization is permitted to run can run. Uncheck commands to restrict them.{{end}} + Commands badged Admins are available only to organization + admins; Members to everyone. +

+ {{template "command-grid" .}} +
+
+ {{if .Restricted}} + + {{end}} + + +
+
+{{end}} diff --git a/internal/core/web.go b/internal/core/web.go index 9c206f7..01e19ba 100644 --- a/internal/core/web.go +++ b/internal/core/web.go @@ -207,6 +207,8 @@ func (s *Handlers) appRouter() chi.Router { r.Post("/repeaters/{id}/share/{userID}/commands", s.handleSetShareCommands) r.Post("/repeaters/{id}/share/{userID}/steward", s.handleSetShareSteward) r.Post("/repeaters/{id}/orgs/{orgID}/participation", s.handleSetRepeaterOrg) + r.Get("/repeaters/{id}/orgs/{orgID}/limits", s.pageRepeaterOrgLimits) + r.Post("/repeaters/{id}/orgs/{orgID}/limits", s.handleSaveRepeaterOrgLimits) r.Post("/invite/{token}/accept", s.handleAcceptInvite) r.Get("/orgs/new", s.pageNewOrg) diff --git a/internal/e2e/share_limits_test.go b/internal/e2e/share_limits_test.go new file mode 100644 index 0000000..99b2fbc --- /dev/null +++ b/internal/e2e/share_limits_test.go @@ -0,0 +1,91 @@ +//go:build browser + +package e2e + +import ( + "strings" + "testing" + + cdplog "github.com/chromedp/cdproto/log" + "github.com/chromedp/cdproto/network" + "github.com/chromedp/chromedp" + + "github.com/jleight/meshtender/internal/store" +) + +// TestE2EShareOrgLimitsModal drives the share page's per-org "Edit limits" modal: +// the button opens the shared modal (Bootstrap data-bs-toggle under the strict +// CSP) and htmx loads the command-grid fragment into it. It then exercises the +// per-section Select none control — which comes from the delegated ui.js handler, +// proving it works on htmx-injected content (a modal fragment can't run inline +// script under the CSP). Also asserts the page runs clean under the CSP. +func TestE2EShareOrgLimitsModal(t *testing.T) { + srv := newE2EServer(t) + user, cookie := srv.login(t, "e2elimit") + + // CreateOrg makes the creator an admin member; the owner's repeater then + // participates in it automatically, so the org row (with Edit limits) renders. + if _, err := srv.store.CreateOrg(srv.ctx, "Limits Org", user.ID); err != nil { + t.Fatalf("create org: %v", err) + } + rep, err := srv.store.CreateRepeater(srv.ctx, &store.Repeater{ + OwnerID: user.ID, Name: "Rep", PublicKeyHex: strings.Repeat("a", 64), + RadioFreqHz: 1, RadioBwHz: 1, RadioSF: 11, RadioCR: 5, + }) + if err != nil { + t.Fatalf("create repeater: %v", err) + } + + bctx, cancel, watch := startBrowser(t) + defer cancel() + + shareURL := srv.appURL + "/repeaters/" + rep.PublicID + "/share" + + // Count checkboxes per section and how many are checked, scoped to the modal. + const countChecked = `(function () { + var scopes = document.querySelectorAll('#limits-modal-content [data-check-scope]'); + return Array.prototype.map.call(scopes, function (s) { + var boxes = s.querySelectorAll('input[type=checkbox]'); + var checked = 0; + Array.prototype.forEach.call(boxes, function (b) { if (b.checked) checked++; }); + return [boxes.length, checked]; + }); + })()` + + var initial, afterNone [][]int + if err := chromedp.Run(bctx, + network.Enable(), + cdplog.Enable(), + setSessionCookie(cookie), + chromedp.Navigate(shareURL), + // Open the per-org limits modal; htmx loads the command grid into it. + chromedp.WaitVisible(`[data-testid="edit-limits"]`, chromedp.ByQuery), + chromedp.Click(`[data-testid="edit-limits"]`, chromedp.ByQuery), + chromedp.WaitVisible(`#limits-modal-content [data-check-scope]`, chromedp.ByQuery), + chromedp.Evaluate(countChecked, &initial), + // Default is permissive: everything checked. Uncheck only the first section. + chromedp.Click(`#limits-modal-content [data-check-scope]:first-of-type [data-check-none]`, chromedp.ByQuery), + chromedp.Evaluate(countChecked, &afterNone), + ); err != nil { + t.Fatalf("browser run against %s: %v", shareURL, err) + } + + if len(initial) < 2 { + t.Fatalf("expected >=2 command sections in the modal, got %d", len(initial)) + } + for i, s := range initial { + if s[0] == 0 || s[1] != s[0] { + t.Fatalf("section %d: expected all %d boxes checked initially, got %d", i, s[0], s[1]) + } + } + // Select none scoped to section 0: only section 0 clears. + if afterNone[0][1] != 0 { + t.Fatalf("section 0: Select none left %d boxes checked, want 0", afterNone[0][1]) + } + for i := 1; i < len(afterNone); i++ { + if afterNone[i][1] != afterNone[i][0] { + t.Fatalf("section %d: unscoped Select none changed it (%d/%d)", i, afterNone[i][1], afterNone[i][0]) + } + } + watch.assertClean(t) +} diff --git a/internal/store/org_repeaters.go b/internal/store/org_repeaters.go index 57f4cd8..7b9056a 100644 --- a/internal/store/org_repeaters.go +++ b/internal/store/org_repeaters.go @@ -19,13 +19,17 @@ type RepeaterOrg struct { } // RepeaterOrgMembership is an org the repeater's owner belongs to, with whether -// the owner has opted this repeater out of it — drives the per-org include/exclude -// toggles on the owner's repeater/share pages. +// the owner has opted this repeater out of it and whether a per-repeater command +// restriction is in effect for it — drives the per-org toggles and the +// "limited commands" indicator on the owner's share page. type RepeaterOrgMembership struct { OrgID int64 OrgSlug string OrgName string Excluded bool + // Restricted is true when this repeater has a per-org command opt-in list for + // this org (>=1 row); false means permissive (the org's full ceiling applies). + Restricted bool } // SetRepeaterOrgExcluded opts a repeater out of (excluded=true) or back into @@ -243,7 +247,9 @@ func (s *Store) ListRepeaterOrgMemberships(ctx context.Context, repeaterID int64 rows, err := s.pool.Query(ctx, ` SELECT o.id, o.slug, o.name, EXISTS (SELECT 1 FROM org_repeater_excludes e - WHERE e.org_id = o.id AND e.repeater_id = r.id) + WHERE e.org_id = o.id AND e.repeater_id = r.id), + EXISTS (SELECT 1 FROM org_repeater_command_optin oc + WHERE oc.org_id = o.id AND oc.repeater_id = r.id) FROM repeaters r JOIN org_members om ON om.user_id = r.owner_id JOIN organizations o ON o.id = om.org_id @@ -254,7 +260,7 @@ func (s *Store) ListRepeaterOrgMemberships(ctx context.Context, repeaterID int64 } return collectRows(rows, func(r pgx.Row) (RepeaterOrgMembership, error) { var m RepeaterOrgMembership - err := r.Scan(&m.OrgID, &m.OrgSlug, &m.OrgName, &m.Excluded) + err := r.Scan(&m.OrgID, &m.OrgSlug, &m.OrgName, &m.Excluded, &m.Restricted) return m, err }) } diff --git a/internal/web/env.go b/internal/web/env.go index cfee3d2..9c400a1 100644 --- a/internal/web/env.go +++ b/internal/web/env.go @@ -26,7 +26,7 @@ import ( "github.com/jleight/meshtender/internal/store" ) -//go:embed templates/base.html templates/icons.html templates/org_tabs.html templates/repeater_tabs.html templates/org_public.html templates/org_config.html templates/org_repeaters.html templates/error.html +//go:embed templates/base.html templates/icons.html templates/org_tabs.html templates/repeater_tabs.html templates/command_grid.html templates/org_public.html templates/org_config.html templates/org_repeaters.html templates/error.html var sharedTemplatesFS embed.FS // sharedPages are full content pages (not just layout partials) that more than @@ -229,7 +229,7 @@ func TimeElement(t time.Time, kind string) template.HTML { } func NewRenderer(cfg *config.Config, surfaceTemplates fs.FS) (*Renderer, error) { - base, err := template.New("").Funcs(templateFuncs).ParseFS(sharedTemplatesFS, "templates/base.html", "templates/icons.html", "templates/org_tabs.html", "templates/repeater_tabs.html") + base, err := template.New("").Funcs(templateFuncs).ParseFS(sharedTemplatesFS, "templates/base.html", "templates/icons.html", "templates/org_tabs.html", "templates/repeater_tabs.html", "templates/command_grid.html") if err != nil { return nil, err } diff --git a/internal/web/static/ui.js b/internal/web/static/ui.js index 9e17ea3..e723613 100644 --- a/internal/web/static/ui.js +++ b/internal/web/static/ui.js @@ -16,6 +16,9 @@ // [data-check-none] every enabled checkbox within its scope. The // scope is the closest [data-check-scope] // ancestor, or the whole document if none. +// [data-risky] (checkbox) — confirm before enabling; unchecks on cancel. +// Delegated so it works in htmx-swapped content +// (a modal fragment can't run inline script). // // It also localizes timestamps: any