diff --git a/internal/core/endpoints_repeater_test.go b/internal/core/endpoints_repeater_test.go index 6d94a01..d6e257e 100644 --- a/internal/core/endpoints_repeater_test.go +++ b/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 diff --git a/internal/core/ignored_errors_test.go b/internal/core/ignored_errors_test.go index 5aeb2a3..6557e0b 100644 --- a/internal/core/ignored_errors_test.go +++ b/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 { diff --git a/internal/core/shares.go b/internal/core/shares.go index 5619d7f..9d91737 100644 --- a/internal/core/shares.go +++ b/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 } diff --git a/internal/core/templates/person_access.html b/internal/core/templates/person_access.html new file mode 100644 index 0000000..ec005eb --- /dev/null +++ b/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"}} +
+
Manage access — {{.Target.Name}} @{{.Target.Username}}
+ +
+
+
+
Steward
+ +

+ A steward can run every command (including risky ones), just like you, and is listed + as a backup maintainer on this repeater's public page. +

+ +
Permissions
+

+ 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. +

+ {{template "command-grid" .}} +
+
+ +
+
+
+ + + +
+{{end}} diff --git a/internal/core/templates/share.html b/internal/core/templates/share.html index b29c8bc..227af48 100644 --- a/internal/core/templates/share.html +++ b/internal/core/templates/share.html @@ -67,19 +67,14 @@ {{if .Shares}}
{{range .Shares}} -
+
{{.Name}} @{{.Username}} {{if .Steward}}Steward · full access{{end}} -
- - -
- {{if not .Steward}}Edit commands{{end}} -
- - -
+
{{end}}
@@ -117,6 +112,11 @@
+ {{template "limits-modal-shell" .}} {{template "icon-arrow-left" "me-1"}}Back to dashboard diff --git a/internal/core/templates/share_commands.html b/internal/core/templates/share_commands.html deleted file mode 100644 index e51d8ae..0000000 --- a/internal/core/templates/share_commands.html +++ /dev/null @@ -1,27 +0,0 @@ -{{define "title"}}Commands for {{.Target.Name}} · MeshTender{{end}} -{{define "header"}} -
-
-
Sharing
-

Commands for {{.Target.Name}}

-
-
-{{end}} -{{define "content"}} -
-
-

- Choose which commands {{.Target.Name}} (@{{.Target.Username}}) may send to - “{{.Repeater.Name}}”. Risky commands can take over or brick the node — grant them only with care. -

-
-
- -
- {{template "command-grid" .}} -
- - Cancel -
-
-{{end}} diff --git a/internal/core/web.go b/internal/core/web.go index 9a9b949..eb9c00e 100644 --- a/internal/core/web.go +++ b/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) diff --git a/internal/e2e/share_person_test.go b/internal/e2e/share_person_test.go new file mode 100644 index 0000000..9e39a12 --- /dev/null +++ b/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) +}