Consolidate share access management

internal/core/endpoints_repeater_test.go

@@ -181,13 +181,13 @@ func TestRepeaterSharePosts(t *testing.T) {
 	}
 	tid := strconv.FormatInt(target.ID, 10)
 
-	cmds := post(t, ts, h.app, share+"/"+tid+"/commands", url.Values{}, sess) // empty = no commands
-	cmds.Body.Close()
-	assertRedirect(t, cmds, share, "set share commands")
-
-	stew := post(t, ts, h.app, share+"/"+tid+"/steward", url.Values{"steward": {"1"}}, sess)
-	stew.Body.Close()
-	assertRedirect(t, stew, share, "set steward")
+	// "Manage access" save: steward + command grants in one POST.
+	acc := post(t, ts, h.app, share+"/"+tid+"/access", url.Values{"steward": {"1"}}, sess)
+	acc.Body.Close()
+	assertRedirect(t, acc, share, "save person access")
+	if steward, _ := st.IsSteward(ctx, rep.ID, target.ID); !steward {
+		t.Fatal("save person access with steward=1 did not make them a steward")
+	}
 
 	un := post(t, ts, h.app, "/repeaters/"+pid+"/unshare", url.Values{"user_id": {tid}}, sess)
 	un.Body.Close()
@@ -197,6 +197,88 @@ func TestRepeaterSharePosts(t *testing.T) {
 	// TestRepeaterOrgLimitsPosts (there is no standalone participation endpoint).
 }
 
+// TestSharePageRenders is a full-page render check for the share page (the e2e
+// entry point): it must 200 with both the org and person "Manage access" buttons.
+func TestSharePageRenders(t *testing.T) {
+	t.Parallel()
+	st, ctx, ts, h := splitServer(t)
+	owner, sess := appLogin(t, ts, st, ctx, h.app, "sharepageowner")
+	rep := newOwnedRepeater(t, st, ctx, owner.ID, "SP Rep")
+	if _, err := st.CreateOrg(ctx, "SP Org", owner.ID); err != nil { // owner is a member → org row renders
+		t.Fatal(err)
+	}
+	sharee, err := st.CreateUser(ctx, "spsharee", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := st.AddShare(ctx, rep.ID, sharee.ID); err != nil {
+		t.Fatal(err)
+	}
+
+	resp := do(t, ts, h.app, "/repeaters/"+rep.PublicID+"/share", sess)
+	body := readBody(t, resp)
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("share page status = %d, want 200:\n%s", resp.StatusCode, body)
+	}
+	if !strings.Contains(body, `data-testid="manage-access"`) {
+		t.Fatal("share page missing the org Manage access button")
+	}
+	if !strings.Contains(body, `data-testid="manage-person"`) {
+		t.Fatal("share page missing the person Manage access button")
+	}
+}
+
+// TestPersonAccessModal covers the per-person "manage access" modal: the GET
+// fragment renders the steward toggle + command grid, and the POST saves the
+// steward flag and command grants together.
+func TestPersonAccessModal(t *testing.T) {
+	t.Parallel()
+	st, ctx, ts, h := splitServer(t)
+	owner, sess := appLogin(t, ts, st, ctx, h.app, "paccessowner")
+	rep := newOwnedRepeater(t, st, ctx, owner.ID, "PA Rep")
+	target, err := st.CreateUser(ctx, "paccessee", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := st.AddShare(ctx, rep.ID, target.ID); err != nil {
+		t.Fatal(err)
+	}
+	base := "/repeaters/" + rep.PublicID + "/share/" + strconv.FormatInt(target.ID, 10) + "/access"
+
+	// GET renders the modal fragment (no page chrome): steward switch + cmd boxes.
+	frag := readBody(t, do(t, ts, h.app, base, sess))
+	if !strings.Contains(frag, "Manage access") || !strings.Contains(frag, `name="steward"`) || !strings.Contains(frag, `name="cmd"`) {
+		t.Fatalf("person-access fragment missing expected content:\n%s", frag)
+	}
+	if strings.Contains(frag, "back-link") {
+		t.Fatal("person-access fragment should be modal chrome, not a full page")
+	}
+	// Footer Save/Revoke reference their separate forms via form= (scrollable body).
+	if !strings.Contains(frag, `form="person-access-form"`) || !strings.Contains(frag, `form="person-revoke-form"`) {
+		t.Fatal("person-access footer buttons should reference their forms via form=")
+	}
+
+	// Save: not a steward, grant exactly one command.
+	catalog, err := st.ListCommands(ctx)
+	if err != nil || len(catalog) == 0 {
+		t.Fatalf("list commands: %v (n=%d)", err, len(catalog))
+	}
+	grant := catalog[0].ID
+	save := post(t, ts, h.app, base, url.Values{"cmd": {strconv.FormatInt(grant, 10)}}, sess)
+	save.Body.Close()
+	assertRedirect(t, save, "/repeaters/"+rep.PublicID+"/share", "save person access")
+	if steward, _ := st.IsSteward(ctx, rep.ID, target.ID); steward {
+		t.Fatal("saving without the steward flag left them a steward")
+	}
+	ids, err := st.ListShareCommandIDs(ctx, rep.ID, target.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(ids) != 1 || ids[0] != grant {
+		t.Fatalf("granted commands = %v, want [%d]", ids, grant)
+	}
+}
+
 // 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

internal/core/ignored_errors_test.go

@@ -13,10 +13,10 @@ import (
 // call fails. A swallowed error would render 200 with an empty form; the fix
 // returns 500.
 
-// TestShareCommandsPageReadErrorFailsClosed: if loading a share's granted command
-// ids fails, the grants page must 500 rather than render every box unchecked
+// TestPersonAccessReadErrorFailsClosed: if loading a share's granted command ids
+// fails, the manage-access modal must 500 rather than render every box unchecked
 // (which a subsequent Save would persist, wiping the target's real grants).
-func TestShareCommandsPageReadErrorFailsClosed(t *testing.T) {
+func TestPersonAccessReadErrorFailsClosed(t *testing.T) {
 	t.Parallel()
 	st, ctx, ts, h := splitServer(t)
 	owner, sess := appLogin(t, ts, st, ctx, h.app, "grantowner")
@@ -35,7 +35,7 @@ func TestShareCommandsPageReadErrorFailsClosed(t *testing.T) {
 		t.Fatalf("drop share_commands: %v", err)
 	}
 
-	path := "/repeaters/" + rep.PublicID + "/share/" + strconv.FormatInt(target.ID, 10) + "/commands"
+	path := "/repeaters/" + rep.PublicID + "/share/" + strconv.FormatInt(target.ID, 10) + "/access"
 	resp := do(t, ts, h.app, path, sess)
 	resp.Body.Close()
 	if resp.StatusCode != http.StatusInternalServerError {

internal/core/shares.go

@@ -111,28 +111,6 @@ func (s *Handlers) handleDeleteInvite(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, sharePath(repeaterParam(r)), http.StatusSeeOther) //nolint:gosec // G710: local path or config-pinned origin
 }
 
-// handleSetShareSteward flags or unflags a shared user as a steward (owner only).
-func (s *Handlers) handleSetShareSteward(w http.ResponseWriter, r *http.Request) {
-	id, ok := s.requireOwned(w, r)
-	if !ok {
-		return
-	}
-	targetID, err := strconv.ParseInt(chi.URLParam(r, "userID"), 10, 64)
-	if err != nil {
-		shareErr(w, r, "Invalid user.")
-		return
-	}
-	if shared, err := s.Store.IsShared(r.Context(), id, targetID); err != nil || !shared {
-		s.NotFound(w, r)
-		return
-	}
-	if err := s.Store.SetShareSteward(r.Context(), id, targetID, r.FormValue("steward") == "1"); err != nil {
-		shareErr(w, r, "Could not update steward.")
-		return
-	}
-	http.Redirect(w, r, sharePath(repeaterParam(r)), http.StatusSeeOther) //nolint:gosec // G710: local path or config-pinned origin
-}
-
 // handleUnshare revokes a user's access (owner only).
 func (s *Handlers) handleUnshare(w http.ResponseWriter, r *http.Request) {
 	id, ok := s.requireOwned(w, r)
@@ -259,8 +237,10 @@ func groupCommands(catalog []*store.Command, checked map[int64]bool) []commandGr
 	})
 }
 
-// pageShareCommands lets a repeater owner choose which commands a shared user may run.
-func (s *Handlers) pageShareCommands(w http.ResponseWriter, r *http.Request) {
+// pagePersonAccess renders the "manage access" modal fragment for one shared
+// person: a steward toggle plus the per-command grid the owner grants from.
+// Loaded via htmx into the share page's shared #person-modal.
+func (s *Handlers) pagePersonAccess(w http.ResponseWriter, r *http.Request) {
 	rep, id, ok := s.requireRepeaterOwned(w, r)
 	if !ok {
 		return
@@ -274,16 +254,16 @@ func (s *Handlers) pageShareCommands(w http.ResponseWriter, r *http.Request) {
 		s.NotFound(w, r)
 		return
 	}
-	// A steward already has every command; per-command limits don't apply to them.
-	if steward, err := s.Store.IsSteward(r.Context(), id, targetID); err == nil && steward {
-		http.Redirect(w, r, sharePath(repeaterParam(r)), http.StatusSeeOther) //nolint:gosec // G710: local path or config-pinned origin
-		return
-	}
 	target, err := s.Store.GetUserByID(r.Context(), targetID)
 	if err != nil {
 		s.NotFound(w, r)
 		return
 	}
+	steward, err := s.Store.IsSteward(r.Context(), id, targetID)
+	if err != nil {
+		s.ServerError(w, r, "could not load access", err)
+		return
+	}
 	catalog, err := s.Store.ListCommands(r.Context())
 	if err != nil {
 		s.ServerError(w, r, "could not load commands", err)
@@ -300,15 +280,20 @@ func (s *Handlers) pageShareCommands(w http.ResponseWriter, r *http.Request) {
 	for _, cid := range ids {
 		checked[cid] = true
 	}
-	s.Render(w, r, "share_commands.html", map[string]any{
+	s.Render(w, r, "person_access.html", map[string]any{
 		"Repeater": rep,
 		"Target":   target,
+		"Steward":  steward,
 		"Groups":   groupCommands(catalog, checked),
+		"Layout":   "person-access-modal",
 	})
 }
 
-// handleSetShareCommands saves the chosen command set for a shared user.
-func (s *Handlers) handleSetShareCommands(w http.ResponseWriter, r *http.Request) {
+// handleSavePersonAccess applies the person "manage access" modal: it sets the
+// steward flag (the toggle) and the per-command grants together. A steward runs
+// every command regardless, but we still persist the grant selection so it applies
+// if steward is later turned off.
+func (s *Handlers) handleSavePersonAccess(w http.ResponseWriter, r *http.Request) {
 	id, ok := s.requireOwned(w, r)
 	if !ok {
 		return
@@ -326,8 +311,11 @@ func (s *Handlers) handleSetShareCommands(w http.ResponseWriter, r *http.Request
 		http.Error(w, "bad form", http.StatusBadRequest)
 		return
 	}
-	cmdIDs := parseCommandIDs(r.Form["cmd"])
-	if err := s.Store.SetShareCommands(r.Context(), id, targetID, cmdIDs); err != nil {
+	if err := s.Store.SetShareSteward(r.Context(), id, targetID, r.FormValue("steward") == "1"); err != nil {
+		s.ServerError(w, r, "could not update steward", err)
+		return
+	}
+	if err := s.Store.SetShareCommands(r.Context(), id, targetID, parseCommandIDs(r.Form["cmd"])); err != nil {
 		s.ServerError(w, r, "could not save commands", err)
 		return
 	}

internal/core/templates/person_access.html

@@ -0,0 +1,40 @@
+{{/* person-access-modal is the htmx fragment swapped into the share page's shared
+     #person-modal. It controls one shared person's steward flag and per-command
+     grants; one Save applies both, and Revoke removes their access. The form lives
+     inside .modal-body so the body scrolls with header/footer pinned; footer
+     buttons reference their form via form=. No client JS. */}}
+{{define "person-access-modal"}}
+<div class="modal-header">
+  <h5 class="modal-title">Manage access — {{.Target.Name}} <span class="text-secondary">@{{.Target.Username}}</span></h5>
+  <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
+</div>
+<div class="modal-body">
+  <form method="post" action="/repeaters/{{.Repeater.PublicID}}/share/{{.Target.ID}}/access" id="person-access-form">
+    <div class="hr-text hr-text-left mt-0 mb-3">Steward</div>
+    <label class="form-check form-switch">
+      <input class="form-check-input" type="checkbox" name="steward" value="1" {{if .Steward}}checked{{end}}>
+      <span class="form-check-label"><strong>Steward — full access</strong></span>
+    </label>
+    <p class="form-hint">
+      A steward can run <strong>every</strong> command (including risky ones), just like you, and is listed
+      as a backup maintainer on this repeater's public page.
+    </p>
+
+    <div class="hr-text hr-text-left mt-5 mb-3">Permissions</div>
+    <p class="text-secondary">
+      Choose which commands {{.Target.Name}} may send. These apply when steward is off — a steward already
+      has every command. Risky commands can take over or brick the node; grant them only with care.
+    </p>
+    {{template "command-grid" .}}
+  </form>
+  <form method="post" action="/repeaters/{{.Repeater.PublicID}}/unshare" id="person-revoke-form">
+    <input type="hidden" name="user_id" value="{{.Target.ID}}">
+  </form>
+</div>
+<div class="modal-footer">
+  <button type="submit" form="person-revoke-form" class="btn btn-link link-danger me-auto"
+          data-confirm="Revoke {{.Target.Name}}'s access to this repeater?">Revoke access</button>
+  <button type="button" class="btn btn-link link-secondary" data-bs-dismiss="modal">Cancel</button>
+  <button type="submit" form="person-access-form" class="btn btn-primary">Save</button>
+</div>
+{{end}}

internal/core/templates/share.html

@@ -67,19 +67,14 @@
     {{if .Shares}}
     <div class="list-group list-group-flush">
       {{range .Shares}}
-      <div class="list-group-item d-flex align-items-center flex-wrap gap-2 px-0">
+      <div class="list-group-item d-flex align-items-center gap-2 flex-wrap px-0" data-testid="share-person-row">
         <span class="fw-bold">{{.Name}}</span>
         <span class="text-secondary">@{{.Username}}</span>
         {{if .Steward}}<span class="badge bg-azure-lt" title="Co-operator: can run every command, including risky ones">Steward · full access</span>{{end}}
-        <form method="post" action="/repeaters/{{$.Repeater.PublicID}}/share/{{.UserID}}/steward" class="m-0 ms-auto">
-          <input type="hidden" name="steward" value="{{if .Steward}}0{{else}}1{{end}}">
-          <button type="submit" class="btn btn-sm"{{if not .Steward}} data-confirm="Make this person a steward? They will be able to run every command on this repeater, including risky ones."{{end}}>{{if .Steward}}Remove as steward{{else}}Make steward{{end}}</button>
-        </form>
-        {{if not .Steward}}<a class="btn btn-sm" href="/repeaters/{{$.Repeater.PublicID}}/share/{{.UserID}}/commands">Edit commands</a>{{end}}
-        <form method="post" action="/repeaters/{{$.Repeater.PublicID}}/unshare" class="m-0">
-          <input type="hidden" name="user_id" value="{{.UserID}}">
-          <button type="submit" class="btn btn-sm btn-ghost-danger">Revoke</button>
-        </form>
+        <button type="button" class="btn btn-sm ms-auto" data-bs-toggle="modal" data-bs-target="#person-modal"
+                data-testid="manage-person"
+                hx-get="/repeaters/{{$.Repeater.PublicID}}/share/{{.UserID}}/access"
+                hx-target="#person-modal-content" hx-swap="innerHTML">Manage access</button>
       </div>
       {{end}}
     </div>
@@ -117,6 +112,11 @@
     <div class="modal-content" id="invite-modal-content"></div>
   </div>
 </div>
+<div class="modal modal-blur fade" id="person-modal" tabindex="-1" aria-hidden="true">
+  <div class="modal-dialog modal-dialog-centered modal-dialog-scrollable modal-lg" role="document">
+    <div class="modal-content" id="person-modal-content"></div>
+  </div>
+</div>
 {{template "limits-modal-shell" .}}
 
 <a class="back-link mt-3" href="/">{{template "icon-arrow-left" "me-1"}}Back to dashboard</a>

internal/core/templates/share_commands.html

@@ -1,27 +0,0 @@
-{{define "title"}}Commands for {{.Target.Name}} · MeshTender{{end}}
-{{define "header"}}
-<div class="row g-2 align-items-center">
-  <div class="col">
-    <div class="page-pretitle">Sharing</div>
-    <h2 class="page-title">Commands for {{.Target.Name}}</h2>
-  </div>
-</div>
-{{end}}
-{{define "content"}}
-<div class="card">
-  <div class="card-body">
-    <p class="text-secondary mb-0">
-      Choose which commands <strong>{{.Target.Name}}</strong> (@{{.Target.Username}}) may send to
-      “{{.Repeater.Name}}”. Risky commands can take over or brick the node — grant them only with care.
-    </p>
-  </div>
-</div>
-
-<form method="post" action="/repeaters/{{.Repeater.PublicID}}/share/{{.Target.ID}}/commands" id="cmdform-perms">
-  {{template "command-grid" .}}
-  <div class="btn-list mt-3">
-    <button type="submit" class="btn btn-primary">Save commands</button>
-    <a class="btn" href="/repeaters/{{.Repeater.PublicID}}/share">Cancel</a>
-  </div>
-</form>
-{{end}}

internal/core/web.go

@@ -204,9 +204,8 @@ func (s *Handlers) appRouter() chi.Router {
 			r.Post("/repeaters/{id}/share/link", s.handleCreateLink)
 			r.Post("/repeaters/{id}/share/link/delete", s.handleDeleteInvite)
 			r.Post("/repeaters/{id}/unshare", s.handleUnshare)
-			r.Get("/repeaters/{id}/share/{userID}/commands", s.pageShareCommands)
-			r.Post("/repeaters/{id}/share/{userID}/commands", s.handleSetShareCommands)
-			r.Post("/repeaters/{id}/share/{userID}/steward", s.handleSetShareSteward)
+			r.Get("/repeaters/{id}/share/{userID}/access", s.pagePersonAccess)
+			r.Post("/repeaters/{id}/share/{userID}/access", s.handleSavePersonAccess)
 			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)

internal/e2e/share_person_test.go

@@ -0,0 +1,60 @@
+//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"
+)
+
+// TestE2EPersonAccessModal drives the People-with-access "Manage access" button:
+// it opens the shared modal (Bootstrap under the strict CSP) and htmx loads the
+// steward toggle + command grid. Confirms the steward switch and grid render and
+// the page runs clean under the CSP.
+func TestE2EPersonAccessModal(t *testing.T) {
+	srv := newE2EServer(t)
+	owner, cookie := srv.login(t, "e2eperson")
+	rep, err := srv.store.CreateRepeater(srv.ctx, &store.Repeater{
+		OwnerID: owner.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)
+	}
+	// A person with accepted access shows up in the People-with-access list.
+	sharee, err := srv.store.CreateUser(srv.ctx, "e2esharee", "")
+	if err != nil {
+		t.Fatalf("create sharee: %v", err)
+	}
+	if _, err := srv.store.AddShare(srv.ctx, rep.ID, sharee.ID); err != nil {
+		t.Fatalf("add share: %v", err)
+	}
+
+	bctx, cancel, watch := startBrowser(t)
+	defer cancel()
+
+	shareURL := srv.appURL + "/repeaters/" + rep.PublicID + "/share"
+	var hasSteward bool
+	if err := chromedp.Run(bctx,
+		network.Enable(),
+		cdplog.Enable(),
+		setSessionCookie(cookie),
+		chromedp.Navigate(shareURL),
+		chromedp.WaitVisible(`[data-testid="manage-person"]`, chromedp.ByQuery),
+		chromedp.Click(`[data-testid="manage-person"]`, chromedp.ByQuery),
+		chromedp.WaitVisible(`#person-modal-content [data-check-scope]`, chromedp.ByQuery),
+		chromedp.Evaluate(`!!document.querySelector('#person-modal-content input[name=steward]')`, &hasSteward),
+	); err != nil {
+		t.Fatalf("browser run against %s: %v", shareURL, err)
+	}
+	if !hasSteward {
+		t.Fatal("person-access modal missing the steward switch")
+	}
+	watch.assertClean(t)
+}