diff --git a/cmd/bdrive/web.go b/cmd/bdrive/web.go index bf20215..996e896 100644 --- a/cmd/bdrive/web.go +++ b/cmd/bdrive/web.go @@ -319,7 +319,7 @@ credentials); otherwise it is relayed through this server.`, if err != nil { return fmt.Errorf("open org registry: %w", err) } - srv.Orgs = orgs + srv.Dir = webapp.LocalDirectory{OrgDB: orgs} // Invite links can bootstrap an account on an invite-only hub. auth.InviteValid = orgs.ValidInvite // A hub that predates organizations: sweep its projects into diff --git a/internal/syncer/org_authz_test.go b/internal/syncer/org_authz_test.go index d4e5892..39a5396 100644 --- a/internal/syncer/org_authz_test.go +++ b/internal/syncer/org_authz_test.go @@ -38,7 +38,7 @@ func orgHub(t *testing.T, storage remote.Backend) (ts *httptest.Server, aliceTok srv := &webapp.Server{ Root: storage, Projects: db, Refresh: 0, Upload: webapp.UploadConfig{Enabled: true}, - Auth: auth, Orgs: orgs, + Auth: auth, Dir: webapp.LocalDirectory{OrgDB: orgs}, } ts = httptest.NewServer(srv.Handler()) t.Cleanup(ts.Close) diff --git a/internal/webapp/admin.go b/internal/webapp/admin.go index 33146f9..dc8e9dd 100644 --- a/internal/webapp/admin.go +++ b/internal/webapp/admin.go @@ -14,14 +14,14 @@ import ( // projectOwner returns true when the request's account owns the project's org. func (s *Server) projectOwner(r *http.Request, projectID string) bool { - if s.Orgs == nil || s.Auth == nil { + if s.Dir == nil || s.Auth == nil { return true } org := s.orgOf(projectID) if org == "" { return true } - return s.Orgs.Role(org, s.requestUser(r).Email) == RoleOwner + return s.Dir.Role(org, s.requestUser(r).Email) == RoleOwner } // handleProjectRename renames a project. Owner of its org only. @@ -80,12 +80,12 @@ func (s *Server) handleProjectDelete(w http.ResponseWriter, r *http.Request) { // so an owner can audit "what have we made public?" in one place. Any org // member may view; only owners revoke (via the existing per-share endpoint). func (s *Server) handleOrgShares(w http.ResponseWriter, r *http.Request) { - if s.Shares == nil || s.Orgs == nil { + if s.Shares == nil || s.Dir == nil { http.Error(w, "sharing is not enabled on this server", http.StatusNotFound) return } orgID := r.PathValue("org") - if s.Orgs.Role(orgID, s.requestUser(r).Email) == "" { + if s.Dir.Role(orgID, s.requestUser(r).Email) == "" { http.Error(w, "you are not a member of this organization", http.StatusForbidden) return } @@ -103,11 +103,19 @@ func (s *Server) handleOrgShares(w http.ResponseWriter, r *http.Request) { writeJSON(w, map[string]any{"shares": out}) } -// builtinAuth returns the concrete OSS auth provider, or nil for a swapped -// provider that doesn't support approval. -func (s *Server) builtinAuth() *BuiltinAuth { - a, _ := s.Auth.(*BuiltinAuth) - return a +// approver returns the auth provider's account-administration half, if it has +// one. A provider whose accounts live in an external identity system does not: +// there is no local approval queue to show and no local policy to flip. +func (s *Server) approver(w http.ResponseWriter) (AccountApprover, bool) { + a, ok := s.Auth.(AccountApprover) + if !ok { + // 503, not an empty list: "no queue here" and "queue is empty" are + // different answers, and only one of them is true. + http.Error(w, "accounts on this hub are administered in its identity provider", + http.StatusServiceUnavailable) + return nil, false + } + return a, true } // handleAdminPending lists accounts awaiting approval. Hub admins only. @@ -116,9 +124,8 @@ func (s *Server) handleAdminPending(w http.ResponseWriter, r *http.Request) { http.Error(w, "hub admins only", http.StatusForbidden) return } - a := s.builtinAuth() - if a == nil { - writeJSON(w, map[string]any{"pending": []any{}}) + a, ok := s.approver(w) + if !ok { return } writeJSON(w, map[string]any{"pending": a.PendingUsers()}) @@ -133,9 +140,8 @@ func (s *Server) handleAdminPolicy(w http.ResponseWriter, r *http.Request) { http.Error(w, "hub admins only", http.StatusForbidden) return } - a := s.builtinAuth() - if a == nil { - http.Error(w, "policy is not supported by this auth provider", http.StatusNotFound) + a, ok := s.approver(w) + if !ok { return } if r.Method == http.MethodPost { @@ -149,7 +155,7 @@ func (s *Server) handleAdminPolicy(w http.ResponseWriter, r *http.Request) { } // Email verification is only a real gate with a mailer; refuse to turn // it on without SMTP rather than silently logging links. - if req.RequireVerification && a.Mail == nil { + if req.RequireVerification && !a.Policy().Mailer { http.Error(w, "email verification needs SMTP configured on the server", http.StatusBadRequest) return } @@ -158,18 +164,7 @@ func (s *Server) handleAdminPolicy(w http.ResponseWriter, r *http.Request) { return } } - admins := make([]string, 0, len(a.Admins)) - for e := range a.Admins { - admins = append(admins, e) - } - writeJSON(w, map[string]any{ - "require_verification": a.RequireVerification, - "require_approval": a.RequireApproval, - "allow_signup": a.AllowSignup, - "allowed_domains": a.AllowedDomains, // read-only (server config) - "admins": admins, // read-only (server config) - "mailer": a.Mail != nil, - }) + writeJSON(w, a.Policy()) } // handleAdminApprove activates a pending account. Hub admins only. @@ -178,9 +173,8 @@ func (s *Server) handleAdminApprove(w http.ResponseWriter, r *http.Request) { http.Error(w, "hub admins only", http.StatusForbidden) return } - a := s.builtinAuth() - if a == nil { - http.Error(w, "approval is not supported by this auth provider", http.StatusNotFound) + a, ok := s.approver(w) + if !ok { return } if err := a.Approve(r.PathValue("id")); err != nil { @@ -196,9 +190,8 @@ func (s *Server) handleAdminDeny(w http.ResponseWriter, r *http.Request) { http.Error(w, "hub admins only", http.StatusForbidden) return } - a := s.builtinAuth() - if a == nil { - http.Error(w, "approval is not supported by this auth provider", http.StatusNotFound) + a, ok := s.approver(w) + if !ok { return } if err := a.Deny(r.PathValue("id")); err != nil { diff --git a/internal/webapp/auth.go b/internal/webapp/auth.go index 5d0eab1..26931a4 100644 --- a/internal/webapp/auth.go +++ b/internal/webapp/auth.go @@ -31,6 +31,40 @@ type AuthProvider interface { // Register mounts the provider's own pages and endpoints (/auth/*, // /api/auth/*) on the server mux. Register(mux *http.ServeMux) + // Accounts lists every account the provider knows, oldest first. Startup + // tasks (the org migration) need it, and both implementations already had + // it — declaring it here stops callers reaching for a concrete type. + Accounts() []User +} + +// AccountApprover is the optional half of account administration: signup +// policy and the approval queue behind /api/admin/*. A provider whose accounts +// live in an external identity system does not implement it, and those routes +// say so (503) rather than pretending the queue is empty. +type AccountApprover interface { + PendingUsers() []User + Approve(id string) error + Deny(id string) error + SetPolicy(requireVerification, requireApproval bool) error + // Policy reports the signup gates as configured. The provider assembles + // it, so the hub never reaches into provider fields to render the page. + Policy() SignupPolicy +} + +// Brander is the optional hub-name half: a provider that renders its own +// sign-in pages knows what to call this hub. +type Brander interface{ Branding() string } + +// SignupPolicy is what /api/admin/policy reports: which gates are on, and +// which of them are server-config owned (read-only to a browser session, so +// that no one can widen access by clicking). +type SignupPolicy struct { + RequireVerification bool `json:"require_verification"` + RequireApproval bool `json:"require_approval"` + AllowSignup bool `json:"allow_signup"` + AllowedDomains []string `json:"allowed_domains"` // read-only + Admins []string `json:"admins"` // read-only + Mailer bool `json:"mailer"` // SMTP configured? } // authGate wraps the API with authentication when a provider is configured. diff --git a/internal/webapp/authlocal.go b/internal/webapp/authlocal.go index ccfddf4..b55bce9 100644 --- a/internal/webapp/authlocal.go +++ b/internal/webapp/authlocal.go @@ -400,6 +400,29 @@ func (a *BuiltinAuth) grantDevice(id, userID string) bool { return true } +// Branding is the hub name this provider renders on its own pages. +func (a *BuiltinAuth) Branding() string { return a.Brand } + +// Policy reports this provider's signup gates (webapp.AccountApprover). The +// provider assembles it so the hub never reaches into these fields itself. +func (a *BuiltinAuth) Policy() SignupPolicy { + a.mu.Lock() + defer a.mu.Unlock() + admins := make([]string, 0, len(a.Admins)) + for e := range a.Admins { + admins = append(admins, e) + } + sort.Strings(admins) + return SignupPolicy{ + RequireVerification: a.RequireVerification, + RequireApproval: a.RequireApproval, + AllowSignup: a.AllowSignup, + AllowedDomains: a.AllowedDomains, + Admins: admins, + Mailer: a.Mail != nil, + } +} + // PendingUsers lists accounts awaiting admin approval, oldest first. func (a *BuiltinAuth) PendingUsers() []User { a.mu.Lock() @@ -606,8 +629,34 @@ font-family:ui-monospace,Menlo,monospace} } func field(label, name, typ, value string) string { - return fmt.Sprintf(``, - html.EscapeString(label), name, typ, html.EscapeString(value)) + return fmt.Sprintf(``, + name, html.EscapeString(label), name, name, typ, html.EscapeString(value), autocompleteFor(name, typ)) +} + +// autocompleteFor names a field's purpose so browsers and password managers +// fill it (WCAG 1.3.5). A password field's purpose depends on the page, not +// its name: offering a saved credential on a signup form is worse than +// offering nothing, so those callers use newPasswordField. +func autocompleteFor(name, typ string) string { + switch name { + case "email": + return "email" + case "name": + return "name" + case "code": + return "one-time-code" + } + if typ == "password" { + return "current-password" + } + return "on" +} + +// newPasswordField is field() for a password being CREATED (signup, reset), +// so a manager generates one instead of filling an existing login. +func newPasswordField(label, name string) string { + return fmt.Sprintf(``, + name, html.EscapeString(label), name, name) } func (a *BuiltinAuth) pageLogin(w http.ResponseWriter, r *http.Request) { @@ -715,7 +764,7 @@ func (a *BuiltinAuth) pageSignup(w http.ResponseWriter, r *http.Request) { field("Name", "name", "text", r.FormValue("name")), field("Email", "email", "email", r.FormValue("email")), domainNote, - field("Password (min 8 chars)", "password", "password", ""), + newPasswordField("Password (min 8 chars)", "password"), errMsg, url.QueryEscape(next))) } @@ -863,7 +912,7 @@ func (a *BuiltinAuth) pageResetConfirm(w http.ResponseWriter, r *http.Request) { func resetForm(token, msg string) string { return fmt.Sprintf(`
%s%s
`, - html.EscapeString(token), field("New password (min 8 chars)", "password", "password", ""), msg) + html.EscapeString(token), newPasswordField("New password (min 8 chars)", "password"), msg) } func requestBaseURL(r *http.Request) string { diff --git a/internal/webapp/directory.go b/internal/webapp/directory.go new file mode 100644 index 0000000..00d0298 --- /dev/null +++ b/internal/webapp/directory.go @@ -0,0 +1,64 @@ +package webapp + +import ( + "errors" + "time" +) + +// Directory is where a hub's organizations live. The built-in one is this +// package's OrgDB (LocalDirectory); a deployment whose users, orgs and +// memberships are owned by an external identity system supplies its own, +// alongside its AuthProvider. +// +// Two rules shape this interface: +// +// Reads are on the request path. Role is called for every project request, +// including the /store/* sync endpoints a device hits every few seconds with +// a device token that carries no identity claims. An implementation backed by +// a remote system therefore has to answer Role from a local cache, and that +// cache is the implementation's business — the hub does not keep one, does not +// refresh one, and must never be written to from the side. (It used to be: +// the hub owned the mirror and the auth provider poked at it, which is how a +// hub-invented org that the identity system had never heard of could exist.) +// +// Writes are optional. A directory that does not own its data returns +// ErrManagedElsewhere and the handler answers 409 with ManageURL, so the hub +// never needs to know WHY it cannot write — only where the user should go. +type Directory interface { + // ---- reads (request path) ---- + Role(orgID, email string) string + Get(orgID string) (Org, bool) + OrgsFor(email string) []Org + ListInvites(orgID string) []OrgInvite + ValidInvite(token string) bool + // ManageURL is where this org is administered: a path within this hub + // when it owns its orgs, an external page when it does not. The client + // follows it and never has to know which kind of hub it is talking to. + ManageURL(orgID string) string + + // ---- writes (ErrManagedElsewhere when the directory is read-only) ---- + Create(name, ownerEmail string) (Org, error) + Rename(orgID, name string) error + AddMember(orgID, email, role string) error + SetRole(orgID, email, role string) error + RemoveMember(orgID, email string) error + CreateInvite(orgID, creator string, ttl time.Duration) (OrgInvite, error) + RevokeInvite(token string) bool + Redeem(token string) (OrgInvite, bool) + RecordInviteUse(token string) +} + +// ErrManagedElsewhere is returned by a directory that does not own its +// organizations. Handlers turn it into 409 plus the org's ManageURL — the +// request was well-formed, it is the state of the world that makes it wrong. +var ErrManagedElsewhere = errors.New("this organization is managed outside this hub") + +// LocalDirectory is the built-in directory: organizations owned by this hub, +// stored in its own metadata store. This is what every self-hosted install +// runs, and its behavior is exactly OrgDB's — the type exists to add the one +// thing an org store has no opinion about, which is where to send a browser +// to administer an org. +type LocalDirectory struct{ *OrgDB } + +// ManageURL is the hub's own org page (a route in the frontend). +func (LocalDirectory) ManageURL(orgID string) string { return "/orgs/" + orgID } diff --git a/internal/webapp/directory_test.go b/internal/webapp/directory_test.go new file mode 100644 index 0000000..bc22e5f --- /dev/null +++ b/internal/webapp/directory_test.go @@ -0,0 +1,146 @@ +package webapp + +import ( + "errors" + "path/filepath" + "testing" + "time" +) + +// Every Directory implementation has to behave the same way from the hub's +// side, whether it owns its organizations or mirrors someone else's. This is +// the contract, run against each implementation — the same shape as +// db_conformance_test does for MetaStore backends. +// +// A read-only directory (one whose orgs live in an external identity system) +// is a first-class case: it answers reads normally and refuses writes with +// ErrManagedElsewhere, which is what lets the hub answer 409 without knowing +// anything about the system on the other side. + +// readOnlyDir wraps a directory and refuses every write, the way an +// implementation backed by an external identity system does. +type readOnlyDir struct{ Directory } + +func (readOnlyDir) Create(string, string) (Org, error) { return Org{}, ErrManagedElsewhere } +func (readOnlyDir) Rename(string, string) error { return ErrManagedElsewhere } +func (readOnlyDir) AddMember(string, string, string) error { return ErrManagedElsewhere } +func (readOnlyDir) SetRole(string, string, string) error { return ErrManagedElsewhere } +func (readOnlyDir) RemoveMember(string, string) error { return ErrManagedElsewhere } +func (readOnlyDir) CreateInvite(string, string, time.Duration) (OrgInvite, error) { + return OrgInvite{}, ErrManagedElsewhere +} +func (readOnlyDir) ManageURL(orgID string) string { return "https://elsewhere.example/" + orgID } + +func TestDirectoryConformance(t *testing.T) { + t.Run("LocalDirectory", func(t *testing.T) { + db, err := OpenOrgDB(filepath.Join(t.TempDir(), "orgs.json")) + if err != nil { + t.Fatal(err) + } + dirReads(t, LocalDirectory{OrgDB: db}, true) + }) + + t.Run("read-only directory", func(t *testing.T) { + db, err := OpenOrgDB(filepath.Join(t.TempDir(), "orgs.json")) + if err != nil { + t.Fatal(err) + } + // Seed through the underlying store: an external directory's contents + // arrive by mirroring, not through the hub's write path. + o, err := db.Create("acme", "alice@x.io") + if err != nil { + t.Fatal(err) + } + dir := readOnlyDir{Directory: LocalDirectory{OrgDB: db}} + dirReads(t, dir, false) + + // Reads still work on the seeded org... + if got := dir.Role(o.ID, "alice@x.io"); got != RoleOwner { + t.Errorf("Role = %q, want owner", got) + } + // ...and every write is refused the same recognizable way. + for name, err := range map[string]error{ + "Rename": dir.Rename(o.ID, "nope"), + "AddMember": dir.AddMember(o.ID, "bob@x.io", RoleMember), + "SetRole": dir.SetRole(o.ID, "alice@x.io", RoleMember), + "RemoveMember": dir.RemoveMember(o.ID, "alice@x.io"), + } { + if !errors.Is(err, ErrManagedElsewhere) { + t.Errorf("%s err = %v, want ErrManagedElsewhere", name, err) + } + } + if _, err := dir.Create("other", "alice@x.io"); !errors.Is(err, ErrManagedElsewhere) { + t.Errorf("Create err = %v, want ErrManagedElsewhere", err) + } + if _, err := dir.CreateInvite(o.ID, "alice@x.io", 0); !errors.Is(err, ErrManagedElsewhere) { + t.Errorf("CreateInvite err = %v, want ErrManagedElsewhere", err) + } + }) +} + +// dirReads exercises the read surface every implementation must answer, and +// the writes only an owning directory supports. +func dirReads(t *testing.T, dir Directory, writable bool) { + t.Helper() + + if writable { + o, err := dir.Create("acme", "alice@x.io") + if err != nil { + t.Fatal(err) + } + if got, ok := dir.Get(o.ID); !ok || got.Name != "acme" { + t.Fatalf("Get = %+v ok=%v", got, ok) + } + if got := dir.Role(o.ID, "ALICE@x.io"); got != RoleOwner { + t.Errorf("Role is not email-normalized: %q", got) + } + if err := dir.AddMember(o.ID, "bob@x.io", RoleMember); err != nil { + t.Fatal(err) + } + // The invariants live in the implementation, not in the caller. + if err := dir.RemoveMember(o.ID, "alice@x.io"); err == nil { + t.Error("removing the last owner must be refused") + } + if err := dir.SetRole(o.ID, "alice@x.io", RoleMember); err == nil { + t.Error("demoting the last owner must be refused") + } + inv, err := dir.CreateInvite(o.ID, "alice@x.io", time.Hour) + if err != nil { + t.Fatal(err) + } + if !dir.ValidInvite(inv.Token) { + t.Error("fresh invite is not valid") + } + if got := dir.ListInvites(o.ID); len(got) != 1 { + t.Errorf("ListInvites = %d, want 1", len(got)) + } + if _, ok := dir.Redeem(inv.Token); !ok { + t.Error("Redeem failed") + } + dir.RecordInviteUse(inv.Token) + if !dir.RevokeInvite(inv.Token) { + t.Error("RevokeInvite failed") + } + if dir.ValidInvite(inv.Token) { + t.Error("revoked invite is still valid") + } + if len(dir.OrgsFor("bob@x.io")) != 1 { + t.Error("OrgsFor missed the org bob belongs to") + } + } + + // Unknown ids are answered, never panicked on — Role in particular runs on + // every project request including device sync. + if got := dir.Role("o-nope", "nobody@x.io"); got != "" { + t.Errorf("Role of a non-member = %q, want \"\"", got) + } + if _, ok := dir.Get("o-nope"); ok { + t.Error("Get of an unknown org reported ok") + } + if dir.ValidInvite("not-a-token") { + t.Error("bogus invite reported valid") + } + if dir.ManageURL("o-1234") == "" { + t.Error("ManageURL must always give the client somewhere to go") + } +} diff --git a/internal/webapp/e2e_serve_test.go b/internal/webapp/e2e_serve_test.go index f16053b..7cea327 100644 --- a/internal/webapp/e2e_serve_test.go +++ b/internal/webapp/e2e_serve_test.go @@ -99,7 +99,7 @@ func TestE2EServe(t *testing.T) { if err := db.SetOrg(p.ID, org.ID); err != nil { t.Fatal(err) } - srv.Orgs = orgs + srv.Dir = LocalDirectory{OrgDB: orgs} auth.InviteValid = orgs.ValidInvite shares, err := OpenShareDB(filepath.Join(state, "shares.json")) diff --git a/internal/webapp/frontend/e2e/admin.spec.ts b/internal/webapp/frontend/e2e/admin.spec.ts index 33eedc2..3b46819 100644 --- a/internal/webapp/frontend/e2e/admin.spec.ts +++ b/internal/webapp/frontend/e2e/admin.spec.ts @@ -16,7 +16,9 @@ test("org admin: members with roles, self marked, rename round-trip", async ({ p await login(page); await openOrgSettings(page); await expect(page.locator("#org-title")).toHaveText("default"); - await expect(page.locator("#crumb")).toHaveText("default"); + // The crumb names the surface; repeating the org name under the

that + // already says it told the reader nothing. + await expect(page.locator("#crumb")).toHaveText("Organization"); await expect(page.locator(".admin-item", { hasText: ADMIN })).toContainText("(you)"); const memberRow = page.locator(".admin-item", { hasText: MEMBER }); await expect(memberRow.locator("select")).toHaveValue("member"); @@ -35,6 +37,22 @@ test("org admin: members with roles, self marked, rename round-trip", async ({ p await page.keyboard.press("Escape"); }); +// The org page owns a URL like every other page: the account menu is a plain +// link to it, so a deep link and a reload both land on the same view. +test("org admin: is a real route, not panel state", async ({ page }) => { + await login(page); + await openOrgSettings(page); + await expect(page).toHaveURL(/\/orgs\/[^/]+$/); + const url = page.url(); + + await page.reload(); + await expect(page.locator("#org-title")).toHaveText("default"); + + await page.goto("/"); + await page.goto(url); + await expect(page.locator("#org-title")).toHaveText("default"); +}); + test("org admin: member role change round-trip", async ({ page }) => { await login(page); await openOrgSettings(page); @@ -96,7 +114,12 @@ test("org admin: project rename and delete", async ({ page }) => { test("member sees the org panel read-only", async ({ page }) => { await login(page, MEMBER); await openOrgSettings(page); - await expect(page.locator("#org-title")).toContainText("member"); + // The role reads as a chip beside the name, not as part of it, and the + // short page explains itself rather than looking truncated. + // The chip is a sibling of the

, not a child: inside it, the + // accessible name of the heading came out as "defaultMember". + await expect(page.locator(".role-chip")).toHaveText("Member"); + await expect(page.locator(".admin-sub")).toContainText("Only owners"); await expect(page.locator("#org-rename")).toHaveCount(0); await expect(page.locator(".admin-item select")).toHaveCount(0); await expect(page.locator(".admin-item .ai-tag").first()).toBeVisible(); // role tags @@ -140,7 +163,10 @@ test("members table sorts by email", async ({ page }) => { const emails = page.locator(".admin-table .admin-item .ai-main"); await expect(emails.first()).toBeVisible(); const before = await emails.allTextContents(); - await page.click('.admin-table th:has-text("Member")'); + // Sorting is a button inside the header cell so it is reachable by + // keyboard; the header also announces the direction via aria-sort. + await page.click('.admin-table th:has-text("Member") .th-sort'); + await expect(page.locator('.admin-table th:has-text("Member")')).toHaveAttribute("aria-sort", /ascending|descending/); const after = await emails.allTextContents(); expect([...before].reverse()).toEqual(after); }); diff --git a/internal/webapp/frontend/shots/01-owner-org-desktop.png b/internal/webapp/frontend/shots/01-owner-org-desktop.png new file mode 100644 index 0000000..505b40a Binary files /dev/null and b/internal/webapp/frontend/shots/01-owner-org-desktop.png differ diff --git a/internal/webapp/frontend/shots/02-owner-org-unknown.png b/internal/webapp/frontend/shots/02-owner-org-unknown.png new file mode 100644 index 0000000..e7fd064 Binary files /dev/null and b/internal/webapp/frontend/shots/02-owner-org-unknown.png differ diff --git a/internal/webapp/frontend/shots/03-focus-primary-btn.png b/internal/webapp/frontend/shots/03-focus-primary-btn.png new file mode 100644 index 0000000..67cc8af Binary files /dev/null and b/internal/webapp/frontend/shots/03-focus-primary-btn.png differ diff --git a/internal/webapp/frontend/shots/03b-unfocused-primary-btn.png b/internal/webapp/frontend/shots/03b-unfocused-primary-btn.png new file mode 100644 index 0000000..9727f8f Binary files /dev/null and b/internal/webapp/frontend/shots/03b-unfocused-primary-btn.png differ diff --git a/internal/webapp/frontend/shots/03c-focus-primary-PROPOSED-FIX.png b/internal/webapp/frontend/shots/03c-focus-primary-PROPOSED-FIX.png new file mode 100644 index 0000000..8c46cba Binary files /dev/null and b/internal/webapp/frontend/shots/03c-focus-primary-PROPOSED-FIX.png differ diff --git a/internal/webapp/frontend/shots/10-mobile-org-owner.png b/internal/webapp/frontend/shots/10-mobile-org-owner.png new file mode 100644 index 0000000..b99d8b8 Binary files /dev/null and b/internal/webapp/frontend/shots/10-mobile-org-owner.png differ diff --git a/internal/webapp/frontend/shots/20-desktop-account-menu.png b/internal/webapp/frontend/shots/20-desktop-account-menu.png new file mode 100644 index 0000000..f060932 Binary files /dev/null and b/internal/webapp/frontend/shots/20-desktop-account-menu.png differ diff --git a/internal/webapp/frontend/shots/20-mobile-account-menu.png b/internal/webapp/frontend/shots/20-mobile-account-menu.png new file mode 100644 index 0000000..2988d7d Binary files /dev/null and b/internal/webapp/frontend/shots/20-mobile-account-menu.png differ diff --git a/internal/webapp/frontend/shots/21-desktop-after-org-tap.png b/internal/webapp/frontend/shots/21-desktop-after-org-tap.png new file mode 100644 index 0000000..1ce9fce Binary files /dev/null and b/internal/webapp/frontend/shots/21-desktop-after-org-tap.png differ diff --git a/internal/webapp/frontend/shots/21-mobile-after-org-tap.png b/internal/webapp/frontend/shots/21-mobile-after-org-tap.png new file mode 100644 index 0000000..5185286 Binary files /dev/null and b/internal/webapp/frontend/shots/21-mobile-after-org-tap.png differ diff --git a/internal/webapp/frontend/shots/30-desktop-member-org.png b/internal/webapp/frontend/shots/30-desktop-member-org.png new file mode 100644 index 0000000..877213c Binary files /dev/null and b/internal/webapp/frontend/shots/30-desktop-member-org.png differ diff --git a/internal/webapp/frontend/shots/30-mobile-member-org.png b/internal/webapp/frontend/shots/30-mobile-member-org.png new file mode 100644 index 0000000..c81b2cf Binary files /dev/null and b/internal/webapp/frontend/shots/30-mobile-member-org.png differ diff --git a/internal/webapp/frontend/shots/31-desktop-member-foreign-org.png b/internal/webapp/frontend/shots/31-desktop-member-foreign-org.png new file mode 100644 index 0000000..a6a57c0 Binary files /dev/null and b/internal/webapp/frontend/shots/31-desktop-member-foreign-org.png differ diff --git a/internal/webapp/frontend/shots/31-mobile-member-foreign-org.png b/internal/webapp/frontend/shots/31-mobile-member-foreign-org.png new file mode 100644 index 0000000..92e5cb3 Binary files /dev/null and b/internal/webapp/frontend/shots/31-mobile-member-foreign-org.png differ diff --git a/internal/webapp/frontend/shots/40-invite-created-toast.png b/internal/webapp/frontend/shots/40-invite-created-toast.png new file mode 100644 index 0000000..d54ffb1 Binary files /dev/null and b/internal/webapp/frontend/shots/40-invite-created-toast.png differ diff --git a/internal/webapp/frontend/shots/41-invite-list.png b/internal/webapp/frontend/shots/41-invite-list.png new file mode 100644 index 0000000..51eec41 Binary files /dev/null and b/internal/webapp/frontend/shots/41-invite-list.png differ diff --git a/internal/webapp/frontend/shots/42-revoke-dialog.png b/internal/webapp/frontend/shots/42-revoke-dialog.png new file mode 100644 index 0000000..eee0683 Binary files /dev/null and b/internal/webapp/frontend/shots/42-revoke-dialog.png differ diff --git a/internal/webapp/frontend/shots/43-rename-validation.png b/internal/webapp/frontend/shots/43-rename-validation.png new file mode 100644 index 0000000..588ce42 Binary files /dev/null and b/internal/webapp/frontend/shots/43-rename-validation.png differ diff --git a/internal/webapp/frontend/shots/50-external-account-menu.png b/internal/webapp/frontend/shots/50-external-account-menu.png new file mode 100644 index 0000000..2081a4c Binary files /dev/null and b/internal/webapp/frontend/shots/50-external-account-menu.png differ diff --git a/internal/webapp/frontend/shots/60-shares-table.png b/internal/webapp/frontend/shots/60-shares-table.png new file mode 100644 index 0000000..c9bc8b0 Binary files /dev/null and b/internal/webapp/frontend/shots/60-shares-table.png differ diff --git a/internal/webapp/frontend/shots/70-org-640.png b/internal/webapp/frontend/shots/70-org-640.png new file mode 100644 index 0000000..6c67c0e Binary files /dev/null and b/internal/webapp/frontend/shots/70-org-640.png differ diff --git a/internal/webapp/frontend/shots/70-org-768.png b/internal/webapp/frontend/shots/70-org-768.png new file mode 100644 index 0000000..13f6469 Binary files /dev/null and b/internal/webapp/frontend/shots/70-org-768.png differ diff --git a/internal/webapp/frontend/shots/70-org-834.png b/internal/webapp/frontend/shots/70-org-834.png new file mode 100644 index 0000000..b0e5b70 Binary files /dev/null and b/internal/webapp/frontend/shots/70-org-834.png differ diff --git a/internal/webapp/frontend/shots/80-409-toast.png b/internal/webapp/frontend/shots/80-409-toast.png new file mode 100644 index 0000000..1124df1 Binary files /dev/null and b/internal/webapp/frontend/shots/80-409-toast.png differ diff --git a/internal/webapp/frontend/shots/90-mobile-shares.png b/internal/webapp/frontend/shots/90-mobile-shares.png new file mode 100644 index 0000000..cd8e940 Binary files /dev/null and b/internal/webapp/frontend/shots/90-mobile-shares.png differ diff --git a/internal/webapp/frontend/src/api/http.ts b/internal/webapp/frontend/src/api/http.ts index 2d858c6..c57c633 100644 --- a/internal/webapp/frontend/src/api/http.ts +++ b/internal/webapp/frontend/src/api/http.ts @@ -9,10 +9,40 @@ function toLogin(): never { throw new Error("signing in…"); } +// Server messages are written for operators and CLIs — lowercase, unpunctuated, +// occasionally naming internals ("forbidden: seat limit reached for plan free"). +// They surface verbatim in toasts, so the ones we can predict get product copy +// and everything else falls back to the server's own words, which is still +// better than a generic apology when the cause is specific. +function errorFor(status: number, body: string): string { + const raw = body.trim(); + switch (status) { + case 403: + if (raw.includes("seat")) return "This plan is out of seats. Upgrade to add more people."; + if (raw.includes("owner")) return "Only owners can do that."; + return "You don't have access to that."; + case 409: + // The 409 body carries the URL that owns the thing, which is the whole + // point of the message — keep it, but capitalize like the rest. + return raw ? raw[0].toUpperCase() + raw.slice(1) : "That is managed outside this hub."; + case 404: + return "That is gone — it may have been removed already."; + case 429: + return "Too many requests. Give it a moment."; + default: + if (status >= 500) return "The server had a problem. Try again."; + return raw ? raw[0].toUpperCase() + raw.slice(1) : "Something went wrong."; + } +} + +async function fail(r: Response): Promise { + throw new Error(errorFor(r.status, await r.text())); +} + export async function getJSON(url: string): Promise { const r = await fetch(url); if (r.status === 401) toLogin(); - if (!r.ok) throw new Error(await r.text()); + if (!r.ok) await fail(r); return r.json(); } @@ -24,7 +54,7 @@ export async function api(method: string, url: string, body?: unkno opt.body = JSON.stringify(body); } const r = await fetch(url, opt); - if (!r.ok) throw new Error(await r.text()); + if (!r.ok) await fail(r); return r.status === 204 ? ({} as T) : r.json(); } @@ -35,6 +65,6 @@ export async function postJSON(url: string, body?: unknown): Promise { body: JSON.stringify(body || {}), }); if (r.status === 401) toLogin(); - if (!r.ok) throw new Error(await r.text()); + if (!r.ok) await fail(r); return r.json(); } diff --git a/internal/webapp/frontend/src/api/types.ts b/internal/webapp/frontend/src/api/types.ts index 290b36a..5e69a28 100644 --- a/internal/webapp/frontend/src/api/types.ts +++ b/internal/webapp/frontend/src/api/types.ts @@ -49,6 +49,10 @@ export interface Org { role: string; // the signed-in account's role in this org members: OrgMember[]; created?: string; + // Where this org is administered: the hub's own page on a self-hosted + // install, an external directory's page on a managed one. Follow it; do + // not branch on it. + manage_url: string; } export interface OrgList { diff --git a/internal/webapp/frontend/src/apps/HubApp.tsx b/internal/webapp/frontend/src/apps/HubApp.tsx index 1bb9d4e..ad6b01b 100644 --- a/internal/webapp/frontend/src/apps/HubApp.tsx +++ b/internal/webapp/frontend/src/apps/HubApp.tsx @@ -3,7 +3,7 @@ import { postJSON } from "../api/http"; import type { InviteAccepted, Project, ProjectCreated, ServerConfig } from "../api/types"; import { useOrgs, usePending, useProjects, useHubRefresh } from "../hooks/useHub"; import { parseRoute, urlForView } from "../router"; -import { navigate, Redirect, useLocationPath } from "../nav"; +import { linkProps, navigate, Redirect, useLocationPath } from "../nav"; import { AppShell, Page, Topbar, VaultHeader, closeSidebarOnMobile } from "../components/shell"; import { OrgAdmin } from "../components/OrgAdmin"; import { HubSettings } from "../components/HubSettings"; @@ -21,9 +21,10 @@ export default function HubApp({ config }: { config: ServerConfig }) { // Org just joined via an invite this page-load: prefer its projects over // whatever happens to be first in the list. const [joinedOrgId, setJoinedOrgId] = useState(null); - // Admin panels replace the content pane without touching the URL (they - // were never routes in the classic app); any navigation closes them. - const [panel, setPanel] = useState(null); + // The hub-admin panel still replaces the content pane without touching the + // URL (the last of the classic app's URL-less surfaces); any navigation + // closes it. Org administration is a real route — see /orgs/ below. + const [panel, setPanel] = useState(null); useEffect(() => setPanel(null), [pathname]); const joinToken = useMemo(() => { @@ -81,6 +82,7 @@ export default function HubApp({ config }: { config: ServerConfig }) { { - setPanel({ kind: "org", orgId: o.id }); - closeSidebarOnMobile(); - }} /> ) : undefined; @@ -140,23 +138,38 @@ export default function HubApp({ config }: { config: ServerConfig }) { ); } - const panelOrg = panel?.kind === "org" ? orgs.find((o) => o.id === panel.orgId) : null; - const activePanel = - panel?.kind === "hub" - ? { crumb: "Signup & access", body: } - : panelOrg - ? { - crumb: panelOrg.name, - body: ( - - ), - } - : null; + const activePanel = panel?.kind === "hub" ? { crumb: "Signup & access", body: } : null; + + const routeOrg = route.org ? orgs.find((o) => o.id === route.org) : null; + // A stale link, a revoked membership, or a typo: say so. Rendering the + // project view at /orgs/ told the user nothing and survived a reload. + const orgMissing = route.org && !routeOrg; + const orgPage = orgMissing + ? { + crumb: "Organization", + body: ( +
+

Organization not found

+

This organization doesn't exist, or you're no longer a member.

+

+ Back to {current.name} +

+
+ ), + } + : routeOrg + ? { + crumb: "Organization", + body: ( + + ), + } + : null; const routePage = route.view === "settings" @@ -172,8 +185,10 @@ export default function HubApp({ config }: { config: ServerConfig }) { : null; // Landing ("/") and unknown project ids both resolve to a real project - // URL; replace so back/forward never bounces through the redirect. - if (route.project !== current.id) { + // URL; replace so back/forward never bounces through the redirect. The + // org route is not project-scoped, so it is exempt — it borrows whichever + // project the sidebar is showing. + if (!route.org && route.project !== current.id) { return ; } @@ -235,7 +250,7 @@ export default function HubApp({ config }: { config: ServerConfig }) { ), orgBar: accountBar, }} - panel={activePanel || routePage} + panel={activePanel || orgPage || routePage} onClosePanel={() => setPanel(null)} /> ); diff --git a/internal/webapp/frontend/src/components/AccountBar.tsx b/internal/webapp/frontend/src/components/AccountBar.tsx index 535a26c..189fcb4 100644 --- a/internal/webapp/frontend/src/components/AccountBar.tsx +++ b/internal/webapp/frontend/src/components/AccountBar.tsx @@ -1,4 +1,6 @@ +import { useState } from "react"; import type { Org } from "../api/types"; +import { linkProps } from "../nav"; import { Icon } from "./shell"; import { projColor } from "./ProjectNav"; import { @@ -13,23 +15,35 @@ import { // opens a menu with the workspace (org) and account actions — settings, // hub administration for admins, and sign-out. Radix owns open/dismiss // behavior (Escape, outside click, focus). +// +// The org entry is a plain link to org.manage_url: this hub's own org page +// when it owns its orgs, the identity provider's page when it does not. The +// server decides; nothing here branches on the answer. export function AccountBar({ me, org, admin, - onOrgSettings, + orgActive, }: { me: { email: string; name: string }; org: Org | null; admin?: { pending: number; onClick: () => void }; // hub admins only - onOrgSettings: (org: Org) => void; + orgActive?: boolean; // the org page is the open surface }) { const display = me.name || me.email; + // The menu's open state is ours because the org entry is a link: linkProps + // calls preventDefault to route internally, and Radix composes its own + // select handler with checkForDefaultPrevented, so that handler never runs + // and the menu stays open on top of the page it just opened. Closing it + // here works for both destinations without giving up a real + // (middle-click, copy link address). + const [menuOpen, setMenuOpen] = useState(false); + const orgLink = org ? linkProps(org.manage_url) : null; return (