diff --git a/docs/openapi.yaml b/docs/openapi.yaml index e234ee9..3a740bc 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -3072,6 +3072,69 @@ paths: "204": { description: Deleted } "404": { $ref: "#/components/responses/NotFound" } + /api/v1/notes/{noteID}/shares: + parameters: + - name: noteID + in: path + required: true + schema: { type: string } + get: + tags: [notes] + summary: Who a note is shared with, and who it can be shared with. + description: > + Owner only. Returns the people the note is already shared with, each with + their role, and the candidates the owner may add — everyone on the instance + who is not already on the note. + responses: + "200": + description: OK + content: + application/json: + schema: + type: object + properties: + shares: + type: array + items: { $ref: "#/components/schemas/NoteShare" } + candidates: + type: array + items: { $ref: "#/components/schemas/Person" } + "404": { $ref: "#/components/responses/NotFound" } + post: + tags: [notes] + summary: Share a note with a person, or change the role they hold. + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [user_id] + properties: + user_id: { type: string } + role: { type: string, enum: [viewer, editor] } + responses: + "204": { description: Shared } + "404": { $ref: "#/components/responses/NotFound" } + "422": { $ref: "#/components/responses/Validation" } + + /api/v1/notes/{noteID}/shares/{userID}: + delete: + tags: [notes] + summary: Take a person's access to a note away again. + parameters: + - name: noteID + in: path + required: true + schema: { type: string } + - name: userID + in: path + required: true + schema: { type: string } + responses: + "204": { description: Unshared } + "404": { $ref: "#/components/responses/NotFound" } + /api/v1/notes/linking/{kind}/{targetID}: get: tags: [notes] @@ -3907,6 +3970,14 @@ components: pinned: { type: boolean } created_at: { type: string, format: date-time } updated_at: { type: string, format: date-time } + shared_with_me: + type: boolean + description: > + True when the reader reached this note through a direct share rather + than by owning it or its project. What puts it in the Delt med mig group. + owner: + allOf: [{ $ref: "#/components/schemas/Person" }] + description: Who the note belongs to, present on a note shared with the reader. links: type: array description: What the body points at, read out of the text on the way in. @@ -3923,6 +3994,21 @@ components: body: { type: string, description: "Markdown. Its first line becomes the title." } pinned: { type: boolean } + Person: + type: object + description: A user, as much of one as a name and a colour. + properties: + id: { type: string } + name: { type: string } + avatar_color: { type: string } + + NoteShare: + type: object + description: One person's direct grant on a note. + properties: + user: { $ref: "#/components/schemas/Person" } + role: { type: string, enum: [viewer, editor] } + Mailbox: type: object description: One connected mailbox. The password and any token are never returned. diff --git a/internal/httpapi/notes_handlers.go b/internal/httpapi/notes_handlers.go index 906898e..4c0d697 100644 --- a/internal/httpapi/notes_handlers.go +++ b/internal/httpapi/notes_handlers.go @@ -19,21 +19,38 @@ import ( // Notes. // -// Access follows the project, because that is where a note lives: a note in a -// shared project is readable by everybody who can read the project and writable -// by an editor. A note with no project is its author's alone — the same answer a -// task in the inbox gets, arrived at the same way rather than by a rule of its own. +// Access has three ways in, and a note may have any of them at once. A note with +// no project is its author's alone — the same answer a task in the inbox gets. A +// note in a project is readable by everyone who can read the project and writable +// by an editor. And a note can be shared with a person directly, which grants that +// person a role of its own. The effective role is the most any of the three allow. -// mayReadNote reports whether this person may see the note, and mayWriteNote -// whether they may change it. Split, because a viewer on a shared project can do -// the first and not the second. +// mayTouchNote reports whether this person may act on the note at the given role — +// RoleViewer to read, RoleEditor to change. Split by role because a viewer, whether +// on a shared project or a shared note, may do the first and not the second. func (s *Server) mayTouchNote(r *http.Request, n *store.Note, role store.Role) bool { user := userFrom(r.Context()) + // The owner and the project path, exactly as before. if n.ProjectID == "" { - return n.CreatedBy == user.ID + if n.CreatedBy == user.ID { + return true + } + } else if _, err := store.RequireProjectRole(r.Context(), s.db, n.ProjectID, user.ID, role); err == nil { + return true } - _, err := store.RequireProjectRole(r.Context(), s.db, n.ProjectID, user.ID, role) - return err == nil + // A direct share grants its role on top of whatever the above allow. + if shareRole, ok, err := s.db.NoteShareRole(r.Context(), n.ID, user.ID); err == nil && ok { + return shareRole.AtLeast(role) + } + return false +} + +// ownsNote reports whether this person is the note's owner — its created_by — which +// is who, and only who, may hand it to other people. An editor on a shared note can +// change its text; giving away access is the owner's alone, the same shape a +// project takes where members edit but only the owner invites. +func (s *Server) ownsNote(r *http.Request, n *store.Note) bool { + return n != nil && n.CreatedBy == userFrom(r.Context()).ID } func (s *Server) handleListNotes(w http.ResponseWriter, r *http.Request) { @@ -48,7 +65,7 @@ func (s *Server) handleListNotes(w http.ResponseWriter, r *http.Request) { s.internal(w, r, "search notes", err) return } - writeJSON(w, http.StatusOK, map[string]any{"notes": briefly(s.readable(r, found))}) + writeJSON(w, http.StatusOK, map[string]any{"notes": s.markShared(r, briefly(s.readable(r, found)))}) return } @@ -62,7 +79,7 @@ func (s *Server) handleListNotes(w http.ResponseWriter, r *http.Request) { s.internal(w, r, "notes in project", err) return } - writeJSON(w, http.StatusOK, map[string]any{"notes": briefly(found)}) + writeJSON(w, http.StatusOK, map[string]any{"notes": s.markShared(r, briefly(found))}) return } @@ -75,7 +92,7 @@ func (s *Server) handleListNotes(w http.ResponseWriter, r *http.Request) { s.internal(w, r, "list archived notes", err) return } - writeJSON(w, http.StatusOK, map[string]any{"notes": briefly(found)}) + writeJSON(w, http.StatusOK, map[string]any{"notes": s.markShared(r, briefly(found))}) return } @@ -87,7 +104,7 @@ func (s *Server) handleListNotes(w http.ResponseWriter, r *http.Request) { s.internal(w, r, "list notes", err) return } - writeJSON(w, http.StatusOK, map[string]any{"notes": briefly(found)}) + writeJSON(w, http.StatusOK, map[string]any{"notes": s.markShared(r, briefly(found))}) } // briefly replaces each note's body with the first of it. @@ -206,9 +223,16 @@ func (s *Server) handleUpdateNote(w http.ResponseWriter, r *http.Request) { return } - if body.ProjectID != nil { - // Moving it means being allowed on both sides: you cannot file somebody - // else's note into your project, nor yours into theirs. + if body.ProjectID != nil && *body.ProjectID != n.ProjectID { + // Filing a note is the owner's call, not an editor's. A shared editor may + // change the text; letting them also move the note would let them file + // somebody else's note into a project the owner is not in — and the owner, + // who is not a member there, would lose their own note. So the move is + // owner-only, and then only into a project the owner may write to. + if !s.ownsNote(r, n) { + writeError(w, http.StatusForbidden, CodeForbidden, "only the note's owner can move it") + return + } if *body.ProjectID != "" { if _, err := store.RequireProjectRole(r.Context(), s.db, *body.ProjectID, userFrom(r.Context()).ID, store.RoleEditor); err != nil { @@ -238,7 +262,16 @@ func (s *Server) handleDeleteNote(w http.ResponseWriter, r *http.Request) { s.internal(w, r, "delete note", err) return } - if n == nil || !s.mayTouchNote(r, n, store.RoleEditor) { + // Deleting is not editing. Someone the note was shared with — even as an editor + // — may change its text, but throwing away the owner's note is the owner's to + // do. A project editor keeps the reach they have always had over a project's + // notes; a direct share does not extend that far. + mayDelete := s.ownsNote(r, n) + if !mayDelete && n != nil && n.ProjectID != "" { + _, perr := store.RequireProjectRole(r.Context(), s.db, n.ProjectID, userFrom(r.Context()).ID, store.RoleEditor) + mayDelete = perr == nil + } + if n == nil || !mayDelete { writeError(w, http.StatusNotFound, CodeNotFound, "no such note") return } diff --git a/internal/httpapi/notes_share_handlers.go b/internal/httpapi/notes_share_handlers.go new file mode 100644 index 0000000..6796639 --- /dev/null +++ b/internal/httpapi/notes_share_handlers.go @@ -0,0 +1,181 @@ +package httpapi + +import ( + "net/http" + + "github.com/go-chi/chi/v5" + + "github.com/kristianwind/verdande/internal/store" +) + +// markShared fills in the two fields the list needs to show a note somebody else +// shared with the reader: that it is such a note, and whose it is. +// +// Done in one pass over the page rather than per note: one query asks which of +// these ids are shared with the reader, and one asks for the owners' names and +// colours. A note the reader owns is never "shared with me", however it was +// reached — the group is other people's notes, not one's own. +func (s *Server) markShared(r *http.Request, notes []store.Note) []store.Note { + me := userFrom(r.Context()).ID + if len(notes) == 0 { + return notes + } + + ids := make([]string, 0, len(notes)) + for _, n := range notes { + if n.CreatedBy != me { + ids = append(ids, n.ID) + } + } + shared, err := s.db.NotesSharedWith(r.Context(), me, ids) + if err != nil { + // The list is worth more than the chip: a note without its "shared" mark + // still reads and still opens. Log nothing here — the store already did. + return notes + } + + owners := map[string]store.Person{} + if len(shared) > 0 { + ownerIDs := make([]string, 0, len(shared)) + for _, n := range notes { + if shared[n.ID] && n.CreatedBy != "" { + ownerIDs = append(ownerIDs, n.CreatedBy) + } + } + owners, _ = s.db.PeopleByIDs(r.Context(), ownerIDs) + } + + for i := range notes { + if !shared[notes[i].ID] { + continue + } + notes[i].SharedWithMe = true + if p, ok := owners[notes[i].CreatedBy]; ok { + owner := p + notes[i].Owner = &owner + } + } + return notes +} + +// handleListNoteShares is who a note is shared with. Owner only: the list of people +// who can see your note is itself something only you should see. +func (s *Server) handleListNoteShares(w http.ResponseWriter, r *http.Request) { + n, err := s.db.Note(r.Context(), chi.URLParam(r, "noteID")) + if err != nil { + s.internal(w, r, "get note", err) + return + } + // 404, not 403: telling a stranger "not yours" still confirms the note exists. + if n == nil || !s.ownsNote(r, n) { + writeError(w, http.StatusNotFound, CodeNotFound, "no such note") + return + } + + shares, err := s.db.ListNoteShares(r.Context(), n.ID) + if err != nil { + s.internal(w, r, "list note shares", err) + return + } + out := make([]noteShareJSON, 0, len(shares)) + already := map[string]bool{} + for _, sh := range shares { + out = append(out, noteShareJSON{ + User: personJSON{ID: sh.User.ID, Name: sh.User.Name, AvatarColor: sh.User.AvatarColor}, + Role: string(sh.Role), + }) + already[sh.User.ID] = true + } + + // The picker is filled from the same response, so the panel opens complete in + // one round-trip. People already on the note are left out — they are in the list + // above it, not the "add someone" menu. + candidates, err := s.db.UsersForSharing(r.Context(), userFrom(r.Context()).ID) + if err != nil { + s.internal(w, r, "share candidates", err) + return + } + cand := make([]personJSON, 0, len(candidates)) + for _, p := range candidates { + if already[p.ID] { + continue + } + cand = append(cand, personJSON{ID: p.ID, Name: p.Name, AvatarColor: p.AvatarColor}) + } + + writeJSON(w, http.StatusOK, map[string]any{"shares": out, "candidates": cand}) +} + +type noteShareJSON struct { + User personJSON `json:"user"` + Role string `json:"role"` +} + +type shareNoteRequest struct { + UserID string `json:"user_id"` + Role string `json:"role"` +} + +// handleShareNote grants a person a role on a note, or changes the role they hold. +func (s *Server) handleShareNote(w http.ResponseWriter, r *http.Request) { + var req shareNoteRequest + if err := decodeJSON(w, r, &req); err != nil { + return + } + n, err := s.db.Note(r.Context(), chi.URLParam(r, "noteID")) + if err != nil { + s.internal(w, r, "get note", err) + return + } + if n == nil || !s.ownsNote(r, n) { + writeError(w, http.StatusNotFound, CodeNotFound, "no such note") + return + } + + role := store.Role(req.Role) + if role == "" { + role = store.RoleViewer + } + if role != store.RoleViewer && role != store.RoleEditor { + writeFieldErrors(w, map[string]string{"role": "must be viewer or editor"}) + return + } + if req.UserID == "" { + writeFieldErrors(w, map[string]string{"user_id": "required"}) + return + } + // The recipient must be a real account and not the sharer themselves; ShareNote + // refuses the owner, and a made-up id should not reach it. + if req.UserID == userFrom(r.Context()).ID { + writeFieldErrors(w, map[string]string{"user_id": "you already have this note"}) + return + } + if _, err := s.db.PersonByID(r.Context(), req.UserID); err != nil { + writeError(w, http.StatusNotFound, CodeNotFound, "no such person") + return + } + + if err := s.db.ShareNote(r.Context(), n.ID, req.UserID, role, userFrom(r.Context()).ID); err != nil { + s.storeError(w, r, "share note", err) + return + } + w.WriteHeader(http.StatusNoContent) +} + +// handleUnshareNote takes a person's access away again. +func (s *Server) handleUnshareNote(w http.ResponseWriter, r *http.Request) { + n, err := s.db.Note(r.Context(), chi.URLParam(r, "noteID")) + if err != nil { + s.internal(w, r, "get note", err) + return + } + if n == nil || !s.ownsNote(r, n) { + writeError(w, http.StatusNotFound, CodeNotFound, "no such note") + return + } + if err := s.db.UnshareNote(r.Context(), n.ID, chi.URLParam(r, "userID")); err != nil { + s.internal(w, r, "unshare note", err) + return + } + w.WriteHeader(http.StatusNoContent) +} diff --git a/internal/httpapi/notes_share_test.go b/internal/httpapi/notes_share_test.go new file mode 100644 index 0000000..9abff0b --- /dev/null +++ b/internal/httpapi/notes_share_test.go @@ -0,0 +1,199 @@ +package httpapi + +import ( + "net/http" + "testing" +) + +// createNote makes a note as the current client and returns its id. +func (ts *testServer) createNote(t *testing.T, body string) string { + t.Helper() + resp, out := ts.do(t, "POST", "/api/v1/notes", map[string]any{"body": body}) + if resp.StatusCode != http.StatusCreated { + t.Fatalf("create note: status %d", resp.StatusCode) + } + id, _ := out["id"].(string) + if id == "" { + t.Fatal("create note returned no id") + } + return id +} + +func userID(t *testing.T, ts *testServer, email string) string { + t.Helper() + u, err := ts.db.UserByEmail(t.Context(), email) + if err != nil { + t.Fatalf("look up %s: %v", email, err) + } + return u.ID +} + +// A note shared with a person shows up in their list, marked as theirs-from- +// somebody, and a viewer can read it but not change it. +func TestSharedNoteReachesTheOtherPerson(t *testing.T) { + owner := newTestServer(t) + owner.bootstrap(t) + other := owner.newUser(t, "sofie@example.dk", "Sofie") + sofie := userID(t, owner, "sofie@example.dk") + + noteID := owner.createNote(t, "# Ferieplan\nuge 29") + + // Before sharing, Sofie cannot see it at all. + if resp, _ := other.do(t, "GET", "/api/v1/notes/"+noteID, nil); resp.StatusCode != http.StatusNotFound { + t.Fatalf("an unshared note was reachable: %d", resp.StatusCode) + } + + // Share it as a viewer. + resp, _ := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", + map[string]string{"user_id": sofie, "role": "viewer"}) + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("share: status %d", resp.StatusCode) + } + + // Now it is in Sofie's list, flagged as shared with her and carrying its owner. + _, list := other.do(t, "GET", "/api/v1/notes", nil) + notes, _ := list["notes"].([]any) + var found map[string]any + for _, raw := range notes { + n := raw.(map[string]any) + if n["id"] == noteID { + found = n + } + } + if found == nil { + t.Fatal("the shared note is not in the recipient's list") + } + if found["shared_with_me"] != true { + t.Errorf("the note is not marked shared_with_me: %v", found) + } + ownerObj, _ := found["owner"].(map[string]any) + if ownerObj == nil || ownerObj["name"] != "Kristian" { + t.Errorf("the owner is not carried on the shared note: %v", found["owner"]) + } + + // A viewer reads it. + if resp, _ := other.do(t, "GET", "/api/v1/notes/"+noteID, nil); resp.StatusCode != http.StatusOK { + t.Errorf("a viewer could not read the shared note: %d", resp.StatusCode) + } + // But cannot change it. + if resp, _ := other.do(t, "PATCH", "/api/v1/notes/"+noteID, + map[string]any{"body": "hijacked"}); resp.StatusCode != http.StatusNotFound { + t.Errorf("a viewer changed a shared note: %d", resp.StatusCode) + } +} + +// An editor may change the text, but only the owner may delete the note or hand it +// to somebody else. +func TestSharedEditorCanWriteButNotGiveAway(t *testing.T) { + owner := newTestServer(t) + owner.bootstrap(t) + other := owner.newUser(t, "sofie@example.dk", "Sofie") + sofie := userID(t, owner, "sofie@example.dk") + + noteID := owner.createNote(t, "# Delt\noprindelig") + if resp, _ := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", + map[string]string{"user_id": sofie, "role": "editor"}); resp.StatusCode != http.StatusNoContent { + t.Fatalf("share as editor failed") + } + + // The editor writes. + if resp, _ := other.do(t, "PATCH", "/api/v1/notes/"+noteID, + map[string]any{"body": "# Delt\nrettet"}); resp.StatusCode != http.StatusOK { + t.Errorf("an editor could not change a shared note: %d", resp.StatusCode) + } + + // The editor cannot re-share it: giving access is the owner's alone. + if resp, _ := other.do(t, "GET", "/api/v1/notes/"+noteID+"/shares", nil); resp.StatusCode != http.StatusNotFound { + t.Errorf("an editor could read the share list: %d", resp.StatusCode) + } + // The editor cannot delete it. + if resp, _ := other.do(t, "DELETE", "/api/v1/notes/"+noteID, nil); resp.StatusCode == http.StatusNoContent { + t.Error("an editor deleted the owner's note") + } + // The note still exists for the owner. + if resp, _ := owner.do(t, "GET", "/api/v1/notes/"+noteID, nil); resp.StatusCode != http.StatusOK { + t.Errorf("the note is gone after the editor's delete attempt: %d", resp.StatusCode) + } +} + +// Unsharing takes access away again, and only the owner can do it. +func TestUnshareRemovesAccess(t *testing.T) { + owner := newTestServer(t) + owner.bootstrap(t) + other := owner.newUser(t, "sofie@example.dk", "Sofie") + sofie := userID(t, owner, "sofie@example.dk") + + noteID := owner.createNote(t, "# Hemmelig") + owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", + map[string]string{"user_id": sofie, "role": "viewer"}) + + // A non-owner cannot unshare. + if resp, _ := other.do(t, "DELETE", "/api/v1/notes/"+noteID+"/shares/"+sofie, nil); resp.StatusCode != http.StatusNotFound { + t.Errorf("a non-owner unshared a note: %d", resp.StatusCode) + } + + // The owner can. + if resp, _ := owner.do(t, "DELETE", "/api/v1/notes/"+noteID+"/shares/"+sofie, nil); resp.StatusCode != http.StatusNoContent { + t.Fatalf("owner unshare: %d", resp.StatusCode) + } + // And access is gone. + if resp, _ := other.do(t, "GET", "/api/v1/notes/"+noteID, nil); resp.StatusCode != http.StatusNotFound { + t.Errorf("the note was still reachable after unsharing: %d", resp.StatusCode) + } +} + +// The share list is owner-only and offers the instance's other people as +// candidates, minus those already on the note. +func TestShareListShowsCandidatesAndIsOwnerOnly(t *testing.T) { + owner := newTestServer(t) + owner.bootstrap(t) + owner.newUser(t, "sofie@example.dk", "Sofie") + owner.newUser(t, "anders@example.dk", "Anders") + sofie := userID(t, owner, "sofie@example.dk") + + noteID := owner.createNote(t, "# Plan") + owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", + map[string]string{"user_id": sofie, "role": "viewer"}) + + _, out := owner.do(t, "GET", "/api/v1/notes/"+noteID+"/shares", nil) + shares, _ := out["shares"].([]any) + if len(shares) != 1 { + t.Fatalf("shares = %d, want 1", len(shares)) + } + cands, _ := out["candidates"].([]any) + // Anders is a candidate; Sofie is already on the note and so is not. + names := map[string]bool{} + for _, c := range cands { + names[c.(map[string]any)["name"].(string)] = true + } + if !names["Anders"] { + t.Errorf("Anders is not offered as a candidate: %v", names) + } + if names["Sofie"] { + t.Error("somebody already on the note is offered as a candidate") + } + if names["Kristian"] { + t.Error("the owner is offered as a candidate to share with themselves") + } +} + +// Sharing refuses a made-up person and the owner themselves. +func TestShareRejectsUnknownAndSelf(t *testing.T) { + owner := newTestServer(t) + owner.bootstrap(t) + me := userID(t, owner, "kristian@example.dk") + noteID := owner.createNote(t, "# Note") + + if resp, _ := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", + map[string]string{"user_id": "does-not-exist", "role": "viewer"}); resp.StatusCode != http.StatusNotFound { + t.Errorf("sharing with an unknown id: %d, want 404", resp.StatusCode) + } + if resp, _ := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", + map[string]string{"user_id": me, "role": "viewer"}); resp.StatusCode != http.StatusUnprocessableEntity { + t.Errorf("sharing with oneself: %d, want 422", resp.StatusCode) + } + if resp, _ := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", + map[string]string{"user_id": me, "role": "owner"}); resp.StatusCode == http.StatusNoContent { + t.Error("a note was shared at the owner role") + } +} diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index b298302..9cc6726 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -348,6 +348,11 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server r.Post("/", s.handleCreateNote) r.Post("/import", s.handleImportNotes) r.Get("/linking/{kind}/{targetID}", s.handleNotesLinking) + // Sharing a note with a person directly — its own access list, beside + // the project path. Owner only; see notes_share_handlers.go. + r.Get("/{noteID}/shares", s.handleListNoteShares) + r.Post("/{noteID}/shares", s.handleShareNote) + r.Delete("/{noteID}/shares/{userID}", s.handleUnshareNote) r.Get("/{noteID}", s.handleGetNote) r.Patch("/{noteID}", s.handleUpdateNote) r.Delete("/{noteID}", s.handleDeleteNote) diff --git a/internal/store/migrations/0030_note_shares.sql b/internal/store/migrations/0030_note_shares.sql new file mode 100644 index 0000000..d201462 --- /dev/null +++ b/internal/store/migrations/0030_note_shares.sql @@ -0,0 +1,30 @@ +-- Sharing a note with a person, rather than only by filing it in a shared project. +-- +-- Until now a note was shared exactly one way: put it in a project, and everyone +-- who could read the project could read the note. That is the right tool when the +-- note belongs with a body of work, and the wrong one when it is just "let Sofie +-- see this" — it makes you stand up a project to hand over a single page. +-- +-- A note_shares row is that direct grant: this note, this person, this role. It +-- sits alongside the project path, not instead of it — a note can be loose, filed, +-- shared with people, or any combination, and access is the most that any of those +-- allow. Only the note's owner (its created_by) writes these rows. +-- +-- role is 'viewer' or 'editor', the same words a project membership uses, so there +-- is one idea of what a role means in this program and not two. +CREATE TABLE note_shares ( + note_id TEXT NOT NULL REFERENCES notes (id) ON DELETE CASCADE, + user_id TEXT NOT NULL REFERENCES users (id) ON DELETE CASCADE, + role TEXT NOT NULL DEFAULT 'viewer', + created_by TEXT REFERENCES users (id) ON DELETE SET NULL, + created_at INTEGER NOT NULL, + + -- One share per person per note: sharing again changes the role, it does not + -- add a second row that the first would then quietly disagree with. + PRIMARY KEY (note_id, user_id) +); + +-- "Which notes are shared with me" is asked on every visit to the notes page, to +-- build the Delt med mig group and to widen what the list may show. It is a lookup +-- by user_id, so that is what carries the index. +CREATE INDEX note_shares_by_user ON note_shares (user_id); diff --git a/internal/store/note_shares.go b/internal/store/note_shares.go new file mode 100644 index 0000000..c8e6468 --- /dev/null +++ b/internal/store/note_shares.go @@ -0,0 +1,133 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "time" +) + +// NoteShare is one person's direct grant on a note: who they are, and what they +// may do. The person is carried whole rather than by id, because every caller of +// this wants to show them — a name and an avatar colour, never a bare id. +type NoteShare struct { + User Person `json:"user"` + Role Role `json:"role"` +} + +// ShareNote grants a person a role on a note, or changes the role if they already +// had one. The owner is passed so the row records who did the sharing; the caller +// has already checked that they may. +// +// Sharing a note with its own owner is refused: the owner already has everything a +// share could give, and a row saying otherwise is a contradiction waiting to +// confuse the access check. +func (db *DB) ShareNote(ctx context.Context, noteID, userID string, role Role, byUserID string) error { + if !role.Valid() || role == RoleOwner { + return errors.New("store: a note can be shared as viewer or editor") + } + var createdBy sql.NullString + if err := db.QueryRowContext(ctx, + `SELECT COALESCE(created_by, '') FROM notes WHERE id = ? AND deleted_at IS NULL`, + noteID).Scan(&createdBy.String); errors.Is(err, sql.ErrNoRows) { + return ErrNotFound + } else if err != nil { + return err + } + if createdBy.String == userID { + return errors.New("store: a note is already the owner's") + } + _, err := db.ExecContext(ctx, ` + INSERT INTO note_shares (note_id, user_id, role, created_by, created_at) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT (note_id, user_id) DO UPDATE SET role = excluded.role`, + noteID, userID, role, byUserID, time.Now().Unix()) + return err +} + +// UnshareNote removes a person's grant. Removing one that is not there is not an +// error: the end state the caller asked for — this person cannot see this note — +// is the same either way. +func (db *DB) UnshareNote(ctx context.Context, noteID, userID string) error { + _, err := db.ExecContext(ctx, + `DELETE FROM note_shares WHERE note_id = ? AND user_id = ?`, noteID, userID) + return err +} + +// ListNoteShares is who a note is shared with, each with their role, ordered by +// name so the list under a note does not reshuffle every time it is opened. +func (db *DB) ListNoteShares(ctx context.Context, noteID string) ([]NoteShare, error) { + rows, err := db.QueryContext(ctx, ` + SELECT u.id, u.name, u.avatar_color, s.role + FROM note_shares s + JOIN users u ON u.id = s.user_id + WHERE s.note_id = ? + ORDER BY lower(u.name)`, noteID) + if err != nil { + return nil, err + } + defer rows.Close() + + var out []NoteShare + for rows.Next() { + var s NoteShare + if err := rows.Scan(&s.User.ID, &s.User.Name, &s.User.AvatarColor, &s.Role); err != nil { + return nil, err + } + out = append(out, s) + } + return out, rows.Err() +} + +// NoteShareRole is the role a person holds on a note through a direct share, if +// any. The bool is whether a share exists at all — distinct from a share whose +// role could not be read — so a caller does not have to treat "no share" as an +// error. +func (db *DB) NoteShareRole(ctx context.Context, noteID, userID string) (Role, bool, error) { + var role Role + err := db.QueryRowContext(ctx, + `SELECT role FROM note_shares WHERE note_id = ? AND user_id = ?`, noteID, userID).Scan(&role) + if errors.Is(err, sql.ErrNoRows) { + return "", false, nil + } + if err != nil { + return "", false, err + } + if !role.Valid() { + // A role the database holds but this build does not understand is no access, + // never a guess — the same rule project roles follow. + return "", false, nil + } + return role, true, nil +} + +// NotesSharedWith returns the ids, among those given, that are shared directly with +// the user. It answers the Delt med mig question in one query rather than one per +// note, and takes the ids it is scoping so it never has to load a person's whole +// world to mark a single page. +func (db *DB) NotesSharedWith(ctx context.Context, userID string, noteIDs []string) (map[string]bool, error) { + out := map[string]bool{} + if len(noteIDs) == 0 { + return out, nil + } + args := make([]any, 0, len(noteIDs)+1) + args = append(args, userID) + for _, id := range noteIDs { + args = append(args, id) + } + rows, err := db.QueryContext(ctx, + `SELECT note_id FROM note_shares WHERE user_id = ? AND note_id IN (`+placeholders(len(noteIDs))+`)`, + args...) + if err != nil { + return nil, err + } + defer rows.Close() + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, err + } + out[id] = true + } + return out, rows.Err() +} diff --git a/internal/store/notes.go b/internal/store/notes.go index 474a4ff..ffee571 100644 --- a/internal/store/notes.go +++ b/internal/store/notes.go @@ -37,6 +37,17 @@ type Note struct { // set by a caller: it is derived, and a caller that set it would be describing // a note whose text says something else. Links []NoteLink `json:"links,omitempty"` + + // SharedWithMe is true when the person reading the list reached this note + // through a direct share rather than by owning it or being in its project. It + // is what puts the note in the Delt med mig group and shows whose note it is. + // Derived per request, so it is never stored and never set by a caller. + SharedWithMe bool `json:"shared_with_me,omitempty"` + + // Owner is who the note belongs to — its created_by — as a name and a colour, + // for the chip on a note shared with the reader. Set only when it helps the + // list say something; nil the rest of the time. + Owner *Person `json:"owner,omitempty"` } // NoteLink is one reference from a note to something else. @@ -263,8 +274,9 @@ func (db *DB) allNotes(ctx context.Context, userID string, archived bool) ([]Not UNION SELECT project_id FROM project_members WHERE user_id = ? ) + OR notes.id IN (SELECT note_id FROM note_shares WHERE user_id = ?) ) - ORDER BY pinned DESC, updated_at DESC`, userID, userID, userID) + ORDER BY pinned DESC, updated_at DESC`, userID, userID, userID, userID) } // NotesLinking answers the backwards question: what points at this thing. diff --git a/internal/store/users.go b/internal/store/users.go index 67f6468..bde261e 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -205,9 +205,50 @@ func (db *DB) SetSidebarCollapsed(ctx context.Context, userID string, sections [ // Person is the public half of a user: what somebody else is allowed to see when // their name has to appear next to a task. No address, no hash, no timezone. type Person struct { - ID string - Name string - AvatarColor string + ID string `json:"id"` + Name string `json:"name"` + AvatarColor string `json:"avatar_color"` +} + +// PersonByID resolves one user to a name and colour, or ErrNotFound. Used to check +// that somebody a note is about to be shared with actually exists, without loading +// the whole account. +func (db *DB) PersonByID(ctx context.Context, id string) (Person, error) { + var p Person + err := db.QueryRowContext(ctx, + `SELECT id, name, avatar_color FROM users WHERE id = ?`, id).Scan(&p.ID, &p.Name, &p.AvatarColor) + if errors.Is(err, sql.ErrNoRows) { + return Person{}, ErrNotFound + } + return p, err +} + +// UsersForSharing is everyone on the instance except the caller — the people a note +// can be handed to. +// +// Deliberately wider than ListPeople, which is the project collaborators. The whole +// point of sharing a note directly is to do it without first standing up a shared +// project, so the picker cannot be limited to people you already share a project +// with. An instance's accounts are a closed, invited set to begin with — there is +// no open signup — so listing them to each other is not disclosure, it is the +// address book the feature needs. +func (db *DB) UsersForSharing(ctx context.Context, meID string) ([]Person, error) { + rows, err := db.QueryContext(ctx, + `SELECT id, name, avatar_color FROM users WHERE id <> ? ORDER BY lower(name)`, meID) + if err != nil { + return nil, err + } + defer rows.Close() + + people := []Person{} + for rows.Next() { + var p Person + if err := rows.Scan(&p.ID, &p.Name, &p.AvatarColor); err != nil { + return nil, err + } + people = append(people, p) + } + return people, rows.Err() } // PeopleByIDs looks up several users at once. diff --git a/web/src/lib/api.js b/web/src/lib/api.js index ca892e8..7a5a349 100644 --- a/web/src/lib/api.js +++ b/web/src/lib/api.js @@ -405,6 +405,12 @@ export const api = { deleteNote: (id) => del(`/notes/${id}`), notesLinking: (kind, targetId) => get(`/notes/linking/${kind}/${encodeURIComponent(targetId)}`), + // Sharing a note with people directly — its own access list, beside the project + // path. Owner only; the list call also returns who it can still be shared with. + noteShares: (id) => get(`/notes/${id}/shares`), + shareNote: (id, userId, role) => post(`/notes/${id}/shares`, { user_id: userId, role }), + unshareNote: (id, userId) => del(`/notes/${id}/shares/${userId}`), + // Mailboxes read over IMAP. Each belongs to the person who connected it, so // there is no instance-wide registration behind these the way Gmail has one. mailboxes: () => get('/mailboxes'), diff --git a/web/src/lib/locales/da.js b/web/src/lib/locales/da.js index 4930f1b..f1028ac 100644 --- a/web/src/lib/locales/da.js +++ b/web/src/lib/locales/da.js @@ -16,10 +16,23 @@ */ export const da = { // --- the shell --------------------------------------------------------------- - 'notes.shareWith': 'Del i', - 'notes.private': 'Kun mig', + 'notes.shareWith': 'Læg i projekt', + 'notes.private': 'Intet projekt', 'notes.shared': 'Noten er delt med projektets folk.', 'notes.unshared': 'Noten er din igen.', + // Deling med enkeltpersoner, ved siden af projektet. + 'notes.groupSharedWithMe': 'Delt med mig', + 'notes.sharedBy': 'Delt af {name}', + 'notes.sharePeople': 'Del med personer', + 'notes.sharePick': 'Vælg en person …', + 'notes.shareAdd': 'Del', + 'notes.shareRoleViewer': 'Kan læse', + 'notes.shareRoleEditor': 'Kan rette', + 'notes.shareRemove': 'Fjern adgang', + 'notes.shareNobody': 'Ikke delt med nogen endnu.', + 'notes.sharedToast': 'Delt med {name}.', + 'notes.unsharedPerson': '{name} kan ikke længere se noten.', + 'notes.shareOwnerOnly': 'Kun ejeren kan dele noten.', 'notes.save': 'Gem', 'notes.format': 'Formatér', 'notes.bold': 'Fed', diff --git a/web/src/lib/locales/en.js b/web/src/lib/locales/en.js index ce13913..d4bc226 100644 --- a/web/src/lib/locales/en.js +++ b/web/src/lib/locales/en.js @@ -12,10 +12,23 @@ */ export const en = { // --- the shell --------------------------------------------------------------- - 'notes.shareWith': 'Share in', - 'notes.private': 'Only me', + 'notes.shareWith': 'File in project', + 'notes.private': 'No project', 'notes.shared': "The note is shared with the project's people.", 'notes.unshared': 'The note is yours again.', + // Sharing with individual people, alongside the project. + 'notes.groupSharedWithMe': 'Shared with me', + 'notes.sharedBy': 'Shared by {name}', + 'notes.sharePeople': 'Share with people', + 'notes.sharePick': 'Choose someone …', + 'notes.shareAdd': 'Share', + 'notes.shareRoleViewer': 'Can view', + 'notes.shareRoleEditor': 'Can edit', + 'notes.shareRemove': 'Remove access', + 'notes.shareNobody': 'Not shared with anyone yet.', + 'notes.sharedToast': 'Shared with {name}.', + 'notes.unsharedPerson': '{name} can no longer see the note.', + 'notes.shareOwnerOnly': 'Only the owner can share the note.', 'notes.save': 'Save', 'notes.format': 'Format', 'notes.bold': 'Bold', diff --git a/web/src/routes/noter/+page.svelte b/web/src/routes/noter/+page.svelte index a2f1454..6e24c6f 100644 --- a/web/src/routes/noter/+page.svelte +++ b/web/src/routes/noter/+page.svelte @@ -146,14 +146,28 @@ if (query.trim()) return [{ key: 'fundet', label: '', notes: ordered, plain: true }]; const out = []; - const favourites = ordered.filter((n) => n.pinned); + + // Andres noter først, samlet. Som Favoritter-gruppen vises den kun, når der + // er noget i den — har man ingen delte noter, findes overskriften ikke og + // tager ingen plads. En delt note bor kun her, ikke også nede i en tidsbunke. + const sharedWithMe = ordered.filter((n) => n.shared_with_me); + if (sharedWithMe.length) { + out.push({ + key: 'delt-med-mig', + label: t('notes.groupSharedWithMe'), + notes: sharedWithMe, + people: true + }); + } + + const favourites = ordered.filter((n) => n.pinned && !n.shared_with_me); if (favourites.length) { out.push({ key: 'favoritter', label: t('notes.groupFavourites'), notes: favourites, star: true }); } let current = null; for (const note of ordered) { - if (note.pinned) continue; + if (note.pinned || note.shared_with_me) continue; const bucket = bucketOf(note); if (!current || current.key !== bucket.key) { current = { key: bucket.key, label: bucket.label, notes: [] }; @@ -170,6 +184,77 @@ // Hvilken række der er valgt. Se open(): editoren venter på hele noten. let selectedId = $state(null); + // Deling af den åbne note med enkeltpersoner. Kun ejeren ser panelet, så det + // hentes kun for en note, man selv har lavet. + let shares = $state([]); + let shareCandidates = $state([]); + let sharePick = $state(''); + let shareRole = $state('viewer'); + let shareBusy = $state(false); + const ownsSelected = $derived(!!selected && selected.created_by === app.user?.id); + + $effect(() => { + const id = selected?.id; + // Nulstil ved hvert skift, så den forrige notes folk ikke står et øjeblik + // under den nye. + shares = []; + shareCandidates = []; + sharePick = ''; + if (!id || selected.created_by !== app.user?.id) return; + let alive = true; + api + .noteShares(id) + .then((r) => { + if (!alive || selected?.id !== id) return; + shares = r.shares ?? []; + shareCandidates = r.candidates ?? []; + }) + .catch(() => {}); + return () => { + alive = false; + }; + }); + + const byName = (a, b) => (a.name ?? '').localeCompare(b.name ?? ''); + + async function addShare() { + if (!selected || !sharePick || shareBusy) return; + const person = shareCandidates.find((p) => p.id === sharePick); + shareBusy = true; + try { + await api.shareNote(selected.id, sharePick, shareRole); + shares = [...shares, { user: person, role: shareRole }].sort((a, b) => byName(a.user, b.user)); + shareCandidates = shareCandidates.filter((p) => p.id !== sharePick); + app.toast(t('notes.sharedToast', { name: person?.name ?? '' })); + sharePick = ''; + } catch (e) { + app.toast(humanMessage(e)); + } finally { + shareBusy = false; + } + } + + async function changeShareRole(userId, role) { + try { + await api.shareNote(selected.id, userId, role); + shares = shares.map((s) => (s.user.id === userId ? { ...s, role } : s)); + } catch (e) { + app.toast(humanMessage(e)); + } + } + + async function removeShare(userId) { + const person = shares.find((s) => s.user.id === userId)?.user; + try { + await api.unshareNote(selected.id, userId); + shares = shares.filter((s) => s.user.id !== userId); + if (person) shareCandidates = [...shareCandidates, person].sort(byName); + app.toast(t('notes.unsharedPerson', { name: person?.name ?? '' })); + } catch (e) { + app.toast(humanMessage(e)); + } + } + /** * Flere ad gangen. * @@ -650,6 +735,14 @@ @@ -699,6 +805,10 @@ Ved siden af hinanden tog de toogfyrre pixels af en liste, der er tre hundrede bred — og de bruges sjældent, mens titlen og uddraget læses hele tiden. --> + + {#if !note.shared_with_me}
- + + {#if ownsSelected} + + + +
+ {t('notes.sharePeople')} + {#if shares.length} + + {:else} + {t('notes.shareNobody')} + {/if} + {#if shareCandidates.length} +
+ + + +
+ {/if} +
+ {/if}
- + {#if ownsSelected} + + {/if}
{:else} @@ -1408,6 +1571,114 @@ padding: 2px var(--s1); } + /* The person icon on the Delt med mig heading, sized to the row of text it + sits in rather than to a fixed pixel. */ + h3.group .peoplemark { + flex: none; + width: 1em; + height: 1em; + color: var(--ink-faint); + } + h3.group .peoplemark svg { + width: 100%; + height: 100%; + display: block; + } + + /* The person chip: the project mark's shape in a person's own colour. No ▸ + before it — the coloured dot already says "belongs to", and two markers on + one short chip is noise. It sits next to the title rather than pushed to the + right, so in the Delt med mig group it reads as "Ferieplan — Sofie". */ + .filed.person { + margin-left: 0.5em; + max-width: 40%; + } + .filed.person::before { + content: none; + } + + /* Del med personer, under the note. */ + .people-share { + display: flex; + flex-direction: column; + gap: var(--s1); + width: 100%; + } + + .people-share .none { + font-size: var(--text-xs); + color: var(--ink-faint); + } + + .sharelist { + list-style: none; + margin: 0; + padding: 0; + display: flex; + flex-direction: column; + gap: 2px; + } + + .sharelist li { + display: flex; + align-items: center; + gap: var(--s1); + font-size: var(--text-xs); + } + + .sharelist .who { + display: flex; + align-items: center; + gap: 0.375em; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + .sharelist .avatar { + width: 8px; + height: 8px; + flex: none; + border-radius: var(--radius-full); + background: var(--who, var(--line-strong)); + } + + .sharelist select { + font-size: var(--text-xs); + padding: 1px var(--s1); + margin-left: auto; + } + + /* The × that takes access away. A word would crowd the row; the mark is enough + beside a name and a role. */ + .unshare { + flex: none; + width: 1.3em; + height: 1.3em; + line-height: 1; + border: none; + background: transparent; + color: var(--ink-faint); + cursor: pointer; + border-radius: var(--radius); + } + .unshare:hover { + color: var(--danger, var(--ink)); + background: var(--surface-raised); + } + + .addshare { + display: flex; + align-items: center; + gap: var(--s1); + flex-wrap: wrap; + } + .addshare select { + font-size: var(--text-xs); + padding: 2px var(--s1); + } + /* Real buttons. They were bare words, which on a Mac reads as a web page rather than as something you can press. */ .actions {