diff --git a/README.md b/README.md index f09ceaf..5d6d81f 100644 --- a/README.md +++ b/README.md @@ -37,8 +37,10 @@ mail-to-task address, and an MCP endpoint so Claude can work your task list dire views, and foldable groups over the projects in the sidebar. - **Drag where it means something.** Reorder a list or a board, file a project under a group, drop a task on another project, or drop it on another day. -- **Sharing.** Internal users are created by an admin; anyone else joins through an - invite link sent to their email. Owner, editor and viewer roles. +- **Sharing.** Owner, editor and viewer roles. There is no open registration: + everybody arrives through an invite link, either to a project or — from + **Settings → Brugere** — to the instance itself. Nobody's password is ever chosen + by somebody else. - **Live sync.** A change made by somebody else appears without a refresh. - **Recurring tasks** as RRULE (RFC 5545), reminders, comments and attachments. - **Standards, not homegrown formats.** iCalendar and CalDAV, ICS feeds, OAuth2 with @@ -69,6 +71,7 @@ Apple Reminders, forward mail into it, and add it to Claude as a connector. | Task detail: description, sub-tasks, comments, files, reminders | Done | | Settings: account, notifications, integrations, AI, tokens, data | Done | | Device list: where you are signed in, and signing one out | Done, tested | +| User administration: invite to the instance, promote, remove | Done, tested | | Invite and password-reset links that open the right page | Done, smoke-tested | | Drag to reorder, in both the list and the board | Done | | Drag a task to another project or another day | Done, smoke-tested | diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 44205e8..b464b73 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -856,6 +856,140 @@ paths: "204": { description: Deleted. } "404": { $ref: "#/components/responses/NotFound" } + /api/v1/users: + get: + tags: [auth] + summary: Everybody on the instance, and the invites not yet accepted. + description: | + Administrators only. Each user carries what deleting them would destroy, + so the confirmation can name real numbers without a second request in the + middle of a decision. + + Session only: an API token cannot enumerate the instance's membership. + responses: + "200": + description: OK + content: + application/json: + schema: + type: object + properties: + users: + type: array + items: { $ref: "#/components/schemas/AdminUser" } + invites: + type: array + items: { $ref: "#/components/schemas/PendingInvite" } + "403": { $ref: "#/components/responses/Forbidden" } + post: + tags: [auth] + summary: Invite somebody to the instance. + description: | + There is no open registration and no account created with a password + somebody else chose: this issues an invite with no project attached, and + the link ends with a password only its owner has seen. The same link and + the same `/auth/signup` as a project invite — an invite whose project is + empty simply grants no membership when it is accepted. + + No name is asked for. The signup form asks the person themselves, and a + name an administrator guesses is a name somebody has to correct. + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [email] + properties: + email: { type: string, format: email } + responses: + "201": + description: Created + content: + application/json: + schema: + type: object + properties: + link: + type: string + description: | + Returned so the interface can offer to copy it. On an + instance with no mail server that is the only way the invite + reaches anybody. + emailed: { type: boolean } + "403": { $ref: "#/components/responses/Forbidden" } + "409": + description: That address already has an account. + content: + application/json: + schema: { $ref: "#/components/schemas/Error" } + "422": { $ref: "#/components/responses/Validation" } + + /api/v1/users/{userID}: + parameters: + - { name: userID, in: path, required: true, schema: { type: string } } + patch: + tags: [auth] + summary: Make somebody an administrator, or stop them being one. + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + is_admin: { type: boolean } + responses: + "204": { description: Changed. } + "403": { $ref: "#/components/responses/Forbidden" } + "404": { $ref: "#/components/responses/NotFound" } + "409": + description: | + The last administrator cannot be demoted — `code` is `last_admin`. + An instance with no administrator has no way back: there is no + console, and setup refuses to run once an account exists. + content: + application/json: + schema: { $ref: "#/components/schemas/Error" } + delete: + tags: [auth] + summary: Remove an account and everything the foreign keys take with it. + description: | + The only hard delete in this API. Everything else is soft-deleted with a + trash behind it; this is not recoverable. + + What goes is wider than it looks: their projects, every task in those + projects, **and every task they wrote anywhere** — `tasks.created_by` + cascades, so retiring a colleague's account removes their contributions + from shared projects too. Tasks merely assigned to them are unassigned + and survive. + + Refused for your own account, and for the last administrator. + responses: + "204": { description: Deleted. } + "403": { $ref: "#/components/responses/Forbidden" } + "404": { $ref: "#/components/responses/NotFound" } + "409": + description: Your own account, or the last administrator. + content: + application/json: + schema: { $ref: "#/components/schemas/Error" } + + /api/v1/invites/{inviteID}: + parameters: + - { name: inviteID, in: path, required: true, schema: { type: string } } + delete: + tags: [auth] + summary: Withdraw an invite that has not been accepted. + description: | + Only the token's hash is stored, so a link sent to the wrong address + cannot be looked up and resent — withdrawing it is the only way to stop + it working before it expires. + responses: + "204": { description: Withdrawn. } + "403": { $ref: "#/components/responses/Forbidden" } + "404": { $ref: "#/components/responses/NotFound" } + /api/v1/project-groups: get: tags: [projects] @@ -2270,6 +2404,48 @@ components: type: array items: { type: string } + AdminUser: + type: object + properties: + id: { type: string } + email: { type: string, format: email } + name: { type: string } + avatar_color: { type: string } + is_admin: { type: boolean } + self: + type: boolean + description: The caller, so the interface can grey out what nobody should do to their own account here. + created_at: { type: string, format: date-time } + last_seen_at: + type: string + format: date-time + description: | + Absent when they have never signed in — which is what an invite that + was accepted and then forgotten looks like. Absent rather than zero, + because 1970 in an interface reads as a bug. + project_count: + type: integer + description: Projects they own, excluding the Inbox every account has. + task_count: + type: integer + description: | + Everything a delete would destroy: tasks in projects they own, plus + every task they wrote anywhere. Not the same as "their tasks", and + the difference is the whole point of showing it. + + PendingInvite: + type: object + properties: + id: { type: string } + email: { type: string, format: email } + project_name: + type: string + description: Absent for an invite to the instance itself rather than to a project. + role: { type: string, enum: [owner, editor, viewer] } + invited_by: { type: string } + created_at: { type: string, format: date-time } + expires_at: { type: string, format: date-time } + Session: type: object properties: diff --git a/internal/httpapi/errors.go b/internal/httpapi/errors.go index e174502..7295314 100644 --- a/internal/httpapi/errors.go +++ b/internal/httpapi/errors.go @@ -47,8 +47,16 @@ const ( CodeAINotConfigured = "ai_not_configured" CodeTOTPNotEnabled = "totp_not_enabled" CodeInboxProtected = "inbox_protected" + // And a fifth, for the same reason: refusing to remove the last administrator + // is not "that already exists" either, and it is the one refusal on the user + // page that a person needs explained rather than merely reported. + CodeLastAdmin = "last_admin" ) +// errLastAdmin is returned to the caller that already wrote the response, so it +// stops rather than carrying on. It is never rendered anywhere. +var errLastAdmin = errors.New("httpapi: refused to remove the last administrator") + func writeError(w http.ResponseWriter, status int, code, message string) { writeJSON(w, status, APIError{Code: code, Message: message}) } diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index aaea91a..f8d1bf2 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -297,6 +297,19 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server }) }) + // The instance's membership. Administrators only, and behind + // requireSession as well as requireAdmin: an API token belongs to a + // person, and a leaked one must not be able to mint an account or + // delete somebody else's. + r.Group(func(r chi.Router) { + r.Use(s.requireSession, s.requireAdmin) + r.Get("/users", s.handleListUsers) + r.Post("/users", s.handleCreateUser) + r.Patch("/users/{userID}", s.handleUpdateUser) + r.Delete("/users/{userID}", s.handleDeleteUser) + r.Delete("/invites/{inviteID}", s.handleDeleteInvite) + }) + // Groups are a person's own headings over their projects, so there is // no project permission to check — every query here is scoped to the // caller. diff --git a/internal/httpapi/users_handlers.go b/internal/httpapi/users_handlers.go new file mode 100644 index 0000000..627a0d7 --- /dev/null +++ b/internal/httpapi/users_handlers.go @@ -0,0 +1,237 @@ +package httpapi + +import ( + "net/http" + "strings" + "time" + + "github.com/go-chi/chi/v5" + + "github.com/kristianwind/verdande/internal/store" +) + +// Who is on the instance, and how somebody else gets on it. +// +// There is no open registration and there never was: an account comes into +// existence in exactly two ways — the first admin at setup, and somebody accepting +// an invite. This surface is the second one made reachable without first sharing a +// project, which is what "create a user" turns out to mean here. +// +// The whole file is administrators only. + +type adminUserJSON struct { + ID string `json:"id"` + Email string `json:"email"` + Name string `json:"name"` + AvatarColor string `json:"avatar_color"` + IsAdmin bool `json:"is_admin"` + // Self marks the caller, so the interface can grey out the two things nobody + // should do to their own account from here. + Self bool `json:"self"` + CreatedAt string `json:"created_at"` + LastSeenAt string `json:"last_seen_at,omitempty"` + // ProjectCount and TaskCount are what a delete would destroy. Sent with the + // list rather than fetched when the button is pressed, so the confirmation can + // name real numbers without a round trip in the middle of a decision. + ProjectCount int `json:"project_count"` + TaskCount int `json:"task_count"` +} + +type pendingInviteJSON struct { + ID string `json:"id"` + Email string `json:"email"` + // ProjectName is empty for an invite to the instance itself, which is what + // this page issues. The two read very differently to whoever is looking. + ProjectName string `json:"project_name,omitempty"` + Role string `json:"role"` + InvitedBy string `json:"invited_by,omitempty"` + CreatedAt string `json:"created_at"` + ExpiresAt string `json:"expires_at"` +} + +func (s *Server) handleListUsers(w http.ResponseWriter, r *http.Request) { + me := userFrom(r.Context()) + + users, err := s.db.ListUsers(r.Context()) + if err != nil { + s.internal(w, "list users", err) + return + } + invites, err := s.db.ListPendingInvites(r.Context()) + if err != nil { + s.internal(w, "list pending invites", err) + return + } + + out := make([]adminUserJSON, 0, len(users)) + for _, u := range users { + j := adminUserJSON{ + ID: u.ID, Email: u.Email, Name: u.Name, AvatarColor: u.AvatarColor, + IsAdmin: u.IsAdmin, Self: u.ID == me.ID, + CreatedAt: u.CreatedAt.Format(time.RFC3339), + ProjectCount: u.ProjectCount, TaskCount: u.TaskCount, + } + // Zero means never signed in, which is not a time and must not be sent as + // one — 1970 in an interface reads as a bug rather than as "never". + if !u.LastSeenAt.IsZero() { + j.LastSeenAt = u.LastSeenAt.Format(time.RFC3339) + } + out = append(out, j) + } + + pending := make([]pendingInviteJSON, 0, len(invites)) + for _, i := range invites { + pending = append(pending, pendingInviteJSON{ + ID: i.ID, Email: i.Email, ProjectName: i.ProjectName, Role: string(i.Role), + InvitedBy: i.InvitedBy, + CreatedAt: i.CreatedAt.Format(time.RFC3339), + ExpiresAt: i.ExpiresAt.Format(time.RFC3339), + }) + } + + writeJSON(w, http.StatusOK, map[string]any{"users": out, "invites": pending}) +} + +type createUserRequest struct { + Email string `json:"email"` +} + +// handleCreateUser invites somebody to the instance. +// +// An invite rather than an account with a password the administrator chooses. A +// password typed here would be known to two people before it was ever used, and +// would have to travel to the second one by some channel nobody has thought about. +// The link does the same job and ends with a password only its owner has seen. +// +// No name is asked for, because the signup form asks the person themselves — a +// name an administrator guesses is a name somebody has to correct. +func (s *Server) handleCreateUser(w http.ResponseWriter, r *http.Request) { + var req createUserRequest + if err := decodeJSON(w, r, &req); err != nil { + return + } + inviter := userFrom(r.Context()) + + email := store.NormalizeEmail(req.Email) + if email == "" || !strings.Contains(email, "@") { + writeFieldErrors(w, map[string]string{"email": "must be an email address"}) + return + } + if _, err := s.db.UserByEmail(r.Context(), email); err == nil { + writeError(w, http.StatusConflict, CodeConflict, "that email address already has an account") + return + } + + // RoleEditor is carried on the row because the column requires a role, and it + // is never read: AcceptInvite returns early when there is no project to grant + // anything on. + token, _, err := s.db.CreateInvite(r.Context(), email, "", store.RoleEditor, + inviter.ID, s.cfg.InviteTTL) + if err != nil { + s.internal(w, "create instance invite", err) + return + } + link := s.cfg.BaseURL + "/invite?token=" + token + + emailed := false + if s.mail.Configured() { + if err := s.mail.SendInvite(r.Context(), email, inviter.Name, "", link, s.cfg.InviteTTL); err != nil { + s.log.Error("send instance invite", "err", err, "to", email) + } else { + emailed = true + } + } + writeJSON(w, http.StatusCreated, inviteResponse{Link: link, Emailed: emailed}) +} + +type updateUserRequest struct { + IsAdmin *bool `json:"is_admin"` +} + +// handleUpdateUser promotes or demotes an administrator. +func (s *Server) handleUpdateUser(w http.ResponseWriter, r *http.Request) { + var req updateUserRequest + if err := decodeJSON(w, r, &req); err != nil { + return + } + userID := chi.URLParam(r, "userID") + me := userFrom(r.Context()) + + if req.IsAdmin == nil { + w.WriteHeader(http.StatusNoContent) + return + } + // Standing down as the last administrator locks everybody out of this page, + // and there is no console and no second setup run to undo it with. + if !*req.IsAdmin { + if err := s.refuseIfLastAdmin(w, r, userID, "the last administrator cannot be demoted"); err != nil { + return + } + } + + if err := s.db.SetUserAdmin(r.Context(), userID, *req.IsAdmin); err != nil { + s.storeError(w, "set user admin", err) + return + } + s.log.Info("admin changed", "by", me.ID, "user", userID, "is_admin", *req.IsAdmin) + w.WriteHeader(http.StatusNoContent) +} + +// handleDeleteUser removes an account and everything the foreign keys take with +// it. See store.DeleteUser for what that is; it is more than it looks. +func (s *Server) handleDeleteUser(w http.ResponseWriter, r *http.Request) { + userID := chi.URLParam(r, "userID") + me := userFrom(r.Context()) + + // Not yourself. Deleting the account you are signed in as would take your own + // projects while you were looking at them, and end the session mid-request. + // Somebody who really means it can be demoted first and removed by another + // administrator. + if userID == me.ID { + writeError(w, http.StatusConflict, CodeConflict, + "you cannot delete the account you are signed in as") + return + } + if err := s.refuseIfLastAdmin(w, r, userID, "the last administrator cannot be deleted"); err != nil { + return + } + + if err := s.db.DeleteUser(r.Context(), userID); err != nil { + s.storeError(w, "delete user", err) + return + } + s.log.Info("user deleted", "by", me.ID, "user", userID) + w.WriteHeader(http.StatusNoContent) +} + +func (s *Server) handleDeleteInvite(w http.ResponseWriter, r *http.Request) { + if err := s.db.DeleteInvite(r.Context(), chi.URLParam(r, "inviteID")); err != nil { + s.storeError(w, "delete invite", err) + return + } + w.WriteHeader(http.StatusNoContent) +} + +// refuseIfLastAdmin writes a 409 and returns an error when the named user is the +// only administrator left. The two callers both need it, and getting it wrong in +// one of them is an instance nobody can administer again. +func (s *Server) refuseIfLastAdmin(w http.ResponseWriter, r *http.Request, userID, message string) error { + target, err := s.db.UserByID(r.Context(), userID) + if err != nil { + s.storeError(w, "get user", err) + return err + } + if !target.IsAdmin { + return nil + } + admins, err := s.db.CountAdmins(r.Context()) + if err != nil { + s.internal(w, "count admins", err) + return err + } + if admins <= 1 { + writeError(w, http.StatusConflict, CodeLastAdmin, message) + return errLastAdmin + } + return nil +} diff --git a/internal/httpapi/users_test.go b/internal/httpapi/users_test.go new file mode 100644 index 0000000..517408c --- /dev/null +++ b/internal/httpapi/users_test.go @@ -0,0 +1,279 @@ +package httpapi + +import ( + "net/http" + "testing" +) + +// The instance's membership is an administrator's list, and creating an account +// means issuing an invite — there is no other door, and no password chosen by +// somebody other than its owner. +func TestAnAdminCanInviteSomebodyToTheInstance(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + resp, body := ts.do(t, "POST", "/api/v1/users", map[string]any{"email": "ny@example.dk"}) + if resp.StatusCode != http.StatusCreated { + t.Fatalf("invite: %d %v", resp.StatusCode, body) + } + link, _ := body["link"].(string) + if link == "" { + t.Fatal("no link came back — with no mail server it is the only way the invite reaches anybody") + } + if body["emailed"] != false { + t.Errorf("emailed = %v, want false with no mail server configured", body["emailed"]) + } + + // It shows as pending, with no project attached: this is an invite to the + // instance, not to somewhere in it. + _, listed := ts.do(t, "GET", "/api/v1/users", nil) + invites, _ := listed["invites"].([]any) + if len(invites) != 1 { + t.Fatalf("want one pending invite, got %v", invites) + } + invite := invites[0].(map[string]any) + if invite["email"] != "ny@example.dk" { + t.Errorf("email = %v", invite["email"]) + } + if name, ok := invite["project_name"]; ok && name != "" { + t.Errorf("project_name = %v, want none for an instance invite", name) + } + + // And the token in the link creates the account, through the same signup the + // project invites use. + token := link[len(link)-len(link)+indexAfter(link, "token="):] + fresh := newSignedOut(t, ts) + resp, _ = fresh.do(t, "POST", "/api/v1/auth/signup", map[string]any{ + "token": token, "name": "Ny Bruger", "password": "et langt kodeord", + }) + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { + t.Fatalf("signup with the instance invite: status %d", resp.StatusCode) + } + + // They are on the instance, and they are not an administrator. + _, after := ts.do(t, "GET", "/api/v1/users", nil) + users, _ := after["users"].([]any) + if len(users) != 2 { + t.Fatalf("want two accounts, got %v", users) + } + for _, raw := range users { + u := raw.(map[string]any) + if u["email"] == "ny@example.dk" && u["is_admin"] != false { + t.Errorf("a newly invited account is an administrator: %v", u) + } + } + // The invite is used up rather than left live. + if pending, _ := after["invites"].([]any); len(pending) != 0 { + t.Errorf("the invite is still pending after being accepted: %v", pending) + } +} + +// The two refusals that make this page safe to have at all. +func TestTheLastAdministratorCannotBeRemoved(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + me, err := ts.db.UserByEmail(t.Context(), "kristian@example.dk") + if err != nil { + t.Fatal(err) + } + + // Demoting yourself as the only administrator locks everybody out: there is no + // console, and setup refuses to run once an account exists. + resp, body := ts.do(t, "PATCH", "/api/v1/users/"+me.ID, map[string]any{"is_admin": false}) + if resp.StatusCode != http.StatusConflict { + t.Fatalf("demote the last admin: status %d, want 409", resp.StatusCode) + } + if body["code"] != "last_admin" { + t.Errorf("code = %v, want last_admin — the interface has to explain this one", body["code"]) + } + + // And deleting the account you are signed in as is refused before that check + // is even reached. + resp, _ = ts.do(t, "DELETE", "/api/v1/users/"+me.ID, nil) + if resp.StatusCode != http.StatusConflict { + t.Errorf("delete your own account: status %d, want 409", resp.StatusCode) + } + + // Still an administrator, and still there. + _, listed := ts.do(t, "GET", "/api/v1/users", nil) + first := listed["users"].([]any)[0].(map[string]any) + if first["is_admin"] != true || first["self"] != true { + t.Errorf("the only administrator is no longer one: %v", first) + } +} + +// With a second administrator the refusal lifts — it is about the last one, not +// about administrators. +func TestASecondAdministratorCanBeDemoted(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + ts.newUser(t, "anden@example.dk", "Anden") + + other, err := ts.db.UserByEmail(t.Context(), "anden@example.dk") + if err != nil { + t.Fatal(err) + } + + if resp, _ := ts.do(t, "PATCH", "/api/v1/users/"+other.ID, + map[string]any{"is_admin": true}); resp.StatusCode != http.StatusNoContent { + t.Fatalf("promote: status %d", resp.StatusCode) + } + if resp, _ := ts.do(t, "PATCH", "/api/v1/users/"+other.ID, + map[string]any{"is_admin": false}); resp.StatusCode != http.StatusNoContent { + t.Errorf("demote a second administrator: status %d, want 204", resp.StatusCode) + } +} + +// Deleting an account takes more than its own projects, and the list says so +// before anybody presses the button. +func TestTheDeleteCountsSayWhatWouldGo(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + other := ts.newUser(t, "anden@example.dk", "Anden") + + otherUser, err := ts.db.UserByEmail(t.Context(), "anden@example.dk") + if err != nil { + t.Fatal(err) + } + + // One project of their own, with a task in it. + _, theirs := other.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Deres"}) + other.do(t, "POST", "/api/v1/tasks", map[string]any{ + "content": "deres egen", "project_id": theirs["id"].(string), + }) + + // And a task they wrote in a project somebody else owns. tasks.created_by + // cascades, so this one goes too — and a count that missed it would understate + // the damage in exactly the case that matters. + _, mine := ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Fælles"}) + sharedID := mine["id"].(string) + ts.do(t, "POST", "/api/v1/projects/"+sharedID+"/invites", map[string]any{ + "email": "anden@example.dk", "role": "editor", + }) + other.do(t, "POST", "/api/v1/tasks", map[string]any{ + "content": "skrevet i et delt projekt", "project_id": sharedID, + }) + + _, listed := ts.do(t, "GET", "/api/v1/users", nil) + var found map[string]any + for _, raw := range listed["users"].([]any) { + if u := raw.(map[string]any); u["id"] == otherUser.ID { + found = u + } + } + if found == nil { + t.Fatal("the second account is not in the list") + } + if found["project_count"] != float64(1) { + t.Errorf("project_count = %v, want 1 — the Inbox must not be counted", found["project_count"]) + } + if found["task_count"] != float64(2) { + t.Errorf("task_count = %v, want 2 — the task in the shared project cascades too", + found["task_count"]) + } + + // And the delete really does take it. + if resp, _ := ts.do(t, "DELETE", "/api/v1/users/"+otherUser.ID, nil); resp.StatusCode != http.StatusNoContent { + t.Fatalf("delete: status %d", resp.StatusCode) + } + _, tasks := ts.do(t, "GET", "/api/v1/tasks?project_id="+sharedID, nil) + if got, _ := tasks["tasks"].([]any); len(got) != 0 { + t.Errorf("a task written by the deleted account survived in a shared project: %v", got) + } +} + +// Everything here is administrators only, and not reachable with an API token +// even by an administrator: a leaked token must not be able to mint an account. +func TestTheUserPageIsAdminsOnlyAndSessionsOnly(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + other := ts.newUser(t, "anden@example.dk", "Anden") + + for _, c := range []struct { + what, method, path string + body any + }{ + {"list", "GET", "/api/v1/users", nil}, + {"invite", "POST", "/api/v1/users", map[string]any{"email": "x@y.dk"}}, + {"promote", "PATCH", "/api/v1/users/whoever", map[string]any{"is_admin": true}}, + {"delete", "DELETE", "/api/v1/users/whoever", nil}, + {"revoke", "DELETE", "/api/v1/invites/whichever", nil}, + } { + if resp, _ := other.do(t, c.method, c.path, c.body); resp.StatusCode != http.StatusForbidden { + t.Errorf("%s as an ordinary user: status %d, want 403", c.what, resp.StatusCode) + } + } + + admin, err := ts.db.UserByEmail(t.Context(), "kristian@example.dk") + if err != nil { + t.Fatal(err) + } + token := ts.apiToken(t, admin.ID) + req, _ := http.NewRequest("GET", ts.URL+"/api/v1/users", nil) + req.Header.Set("Authorization", "Bearer "+token) + resp, err := (&http.Client{}).Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusForbidden { + t.Errorf("an administrator's API token reached the user list: status %d", resp.StatusCode) + } +} + +// A pending invite can be withdrawn, which is the only way to stop a link that +// went to the wrong address: the token is stored as a hash and cannot be looked up. +func TestAPendingInviteCanBeWithdrawn(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + _, created := ts.do(t, "POST", "/api/v1/users", map[string]any{"email": "forkert@example.dk"}) + link := created["link"].(string) + + _, listed := ts.do(t, "GET", "/api/v1/users", nil) + inviteID := listed["invites"].([]any)[0].(map[string]any)["id"].(string) + + if resp, _ := ts.do(t, "DELETE", "/api/v1/invites/"+inviteID, nil); resp.StatusCode != http.StatusNoContent { + t.Fatalf("withdraw: status %d", resp.StatusCode) + } + + // The link no longer works. + token := link[indexAfter(link, "token="):] + fresh := newSignedOut(t, ts) + resp, _ := fresh.do(t, "POST", "/api/v1/auth/signup", map[string]any{ + "token": token, "name": "Forkert", "password": "et langt kodeord", + }) + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("a withdrawn invite still signs somebody up: status %d", resp.StatusCode) + } +} + +// An address that already has an account is a conflict rather than a second +// invite: the signup would fail at the end, after somebody had chosen a password. +func TestInvitingAnAddressThatAlreadyHasAnAccount(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + resp, body := ts.do(t, "POST", "/api/v1/users", + map[string]any{"email": "kristian@example.dk"}) + if resp.StatusCode != http.StatusConflict { + t.Fatalf("status %d, want 409 — body %v", resp.StatusCode, body) + } +} + +// newSignedOut is a client against the same server with an empty cookie jar. +func newSignedOut(t *testing.T, ts *testServer) *testServer { + t.Helper() + return &testServer{Server: ts.Server, db: ts.db, client: newJarClient(t)} +} + +// indexAfter returns the offset just past `needle` in `s`. +func indexAfter(s, needle string) int { + for i := 0; i+len(needle) <= len(s); i++ { + if s[i:i+len(needle)] == needle { + return i + len(needle) + } + } + return 0 +} diff --git a/internal/store/invites.go b/internal/store/invites.go index 78ff4f5..05ea3f9 100644 --- a/internal/store/invites.go +++ b/internal/store/invites.go @@ -136,6 +136,70 @@ func (db *DB) InviteByToken(ctx context.Context, token string) (*Invite, error) return &inv, nil } +// PendingInvite is an invite that has been sent and not yet used, as the +// administrator's list shows it. The token is not here and cannot be: only its +// hash is stored, so a link that has gone astray is revoked rather than resent. +type PendingInvite struct { + ID string + Email string + ProjectID string + ProjectName string + Role Role + InvitedBy string + CreatedAt time.Time + ExpiresAt time.Time +} + +// ListPendingInvites returns invites that are neither accepted nor expired. +// +// Expired ones are left out rather than shown greyed: an invite past its date is +// not a thing anybody can act on, and a list of them would grow forever. The +// project name is joined in because an invite with no project means something +// different — an invitation to the instance itself — and the interface has to be +// able to say which it is. +func (db *DB) ListPendingInvites(ctx context.Context) ([]PendingInvite, error) { + rows, err := db.QueryContext(ctx, ` + SELECT i.id, i.email, COALESCE(i.project_id, ''), COALESCE(p.name, ''), i.role, + COALESCE(u.name, ''), i.created_at, i.expires_at + FROM invites i + LEFT JOIN projects p ON p.id = i.project_id + LEFT JOIN users u ON u.id = i.created_by + WHERE i.accepted_at IS NULL AND i.expires_at > ? + ORDER BY i.created_at DESC`, time.Now().Unix()) + if err != nil { + return nil, err + } + defer rows.Close() + + out := []PendingInvite{} + for rows.Next() { + var p PendingInvite + var created, expires int64 + if err := rows.Scan(&p.ID, &p.Email, &p.ProjectID, &p.ProjectName, &p.Role, + &p.InvitedBy, &created, &expires); err != nil { + return nil, err + } + p.CreatedAt = time.Unix(created, 0).UTC() + p.ExpiresAt = time.Unix(expires, 0).UTC() + out = append(out, p) + } + return out, rows.Err() +} + +// DeleteInvite revokes an invite, which is what makes a link sent to the wrong +// address recoverable: the token cannot be looked up, so withdrawing it is the +// only way to stop it working before it expires. +func (db *DB) DeleteInvite(ctx context.Context, inviteID string) error { + res, err := db.ExecContext(ctx, `DELETE FROM invites WHERE id = ?`, inviteID) + if err != nil { + return err + } + if n, _ := res.RowsAffected(); n == 0 { + return ErrNotFound + } + return nil +} + // AcceptInvite marks the invite used and grants the membership it promised, in one // transaction. Splitting the two would allow a signup to fail partway and leave a // live invite behind, which is a link that still works after it has been used. diff --git a/internal/store/users.go b/internal/store/users.go index b099dc4..0340ac6 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -163,6 +163,117 @@ func (db *DB) PeopleByIDs(ctx context.Context, ids []string) (map[string]Person, return people, rows.Err() } +// UserSummary is a user as the administrator's list shows them: the account, plus +// the two numbers that say what deleting it would take with it and the one that +// says whether it is in use at all. +type UserSummary struct { + User + // LastSeenAt is the most recent request from any of their sessions, zero if + // they have never signed in — which is what an invite that was accepted and + // then forgotten looks like. + LastSeenAt time.Time + // ProjectCount excludes the Inbox: every account has one, so counting it would + // make "1 project" mean "nothing". + ProjectCount int + // TaskCount is everything a delete would destroy, which is not the same as + // "their tasks": it is the tasks in projects they own *plus* every task they + // wrote anywhere, because tasks.created_by cascades. A number that counted + // only the first would understate the damage in exactly the case that matters + // — a colleague leaving a shared project. + TaskCount int +} + +// ListUsers returns every account on the instance. Administrators only — this is +// the whole membership of the server, which is not a list anybody else is owed. +func (db *DB) ListUsers(ctx context.Context) ([]UserSummary, error) { + rows, err := db.QueryContext(ctx, ` + SELECT u.id, u.email, u.name, u.avatar_color, u.is_admin, u.created_at, + COALESCE((SELECT max(s.last_seen_at) FROM sessions s WHERE s.user_id = u.id), 0), + (SELECT count(*) FROM projects p + WHERE p.owner_id = u.id AND p.is_inbox = 0 AND p.deleted_at IS NULL), + (SELECT count(*) FROM tasks t JOIN projects p ON p.id = t.project_id + WHERE t.deleted_at IS NULL + AND (p.owner_id = u.id OR t.created_by = u.id)) + FROM users u + ORDER BY u.is_admin DESC, u.name`) + if err != nil { + return nil, err + } + defer rows.Close() + + out := []UserSummary{} + for rows.Next() { + var s UserSummary + var isAdmin int + var created, lastSeen int64 + if err := rows.Scan(&s.ID, &s.Email, &s.Name, &s.AvatarColor, &isAdmin, &created, + &lastSeen, &s.ProjectCount, &s.TaskCount); err != nil { + return nil, err + } + s.IsAdmin = isAdmin == 1 + s.CreatedAt = time.Unix(created, 0).UTC() + if lastSeen > 0 { + s.LastSeenAt = time.Unix(lastSeen, 0).UTC() + } + out = append(out, s) + } + return out, rows.Err() +} + +// CountAdmins is what stops the last administrator from being deleted or demoted. +// An instance with no administrator has no way back: there is no console, and the +// setup route refuses to run once an account exists. +func (db *DB) CountAdmins(ctx context.Context) (int, error) { + var n int + err := db.QueryRowContext(ctx, `SELECT count(*) FROM users WHERE is_admin = 1`).Scan(&n) + return n, err +} + +func (db *DB) SetUserAdmin(ctx context.Context, userID string, admin bool) error { + res, err := db.ExecContext(ctx, + `UPDATE users SET is_admin = ?, updated_at = ? WHERE id = ?`, + boolToInt(admin), time.Now().Unix(), userID) + if err != nil { + return err + } + if n, _ := res.RowsAffected(); n == 0 { + return ErrNotFound + } + return nil +} + +// DeleteUser removes an account and everything that hangs off it. +// +// A hard delete, and the only one in this database. Every other delete here is a +// `deleted_at` with a trash behind it; this one relies on the foreign keys and +// there is no way back — which is why the handler sends the counts to the +// interface first. +// +// What goes is wider than it looks. `projects.owner_id` cascades, so their +// projects go and every task in them goes too. But `tasks.created_by` cascades as +// well, so **every task they ever wrote goes, including in projects somebody else +// owns**. Retiring a colleague's account therefore removes their contributions +// from shared projects rather than leaving them behind unattributed. +// +// That is the schema as it stands rather than a decision made here. Changing it — +// `created_by` as ON DELETE SET NULL, so the work stays and loses its author — +// means rebuilding the tasks table, which carries the FTS triggers with it. Worth +// doing; not worth doing quietly as part of adding a screen. +// +// What survives: tasks merely *assigned* to them, which are unassigned by +// `assignee_id ON DELETE SET NULL`, and completions, by the same rule on +// `completed_by`. +func (db *DB) DeleteUser(ctx context.Context, userID string) error { + res, err := db.ExecContext(ctx, `DELETE FROM users WHERE id = ?`, userID) + if err != nil { + return err + } + if n, _ := res.RowsAffected(); n == 0 { + return ErrNotFound + } + return nil +} + // UserCount is what decides whether the instance still needs its first admin. func (db *DB) UserCount(ctx context.Context) (int, error) { var n int diff --git a/web/e2e/smoke.spec.js b/web/e2e/smoke.spec.js index c1eaa48..a3b90cc 100644 --- a/web/e2e/smoke.spec.js +++ b/web/e2e/smoke.spec.js @@ -153,6 +153,9 @@ test('hver fane under indstillinger renderer', async ({ page }) => { 'Integrationer', 'AI', 'API-tokens', + // Only an administrator sees this one, and the account these tests run as + // is the first account, which is one. + 'Brugere', 'Data og skabeloner' ]) { await tabs.getByRole('link', { name: label, exact: true }).click(); @@ -533,6 +536,72 @@ test('enhedslisten viser denne enhed, og en anden kan logges ud', async ({ brows expect(trouble).toEqual([]); }); +/** + * An administrator can put somebody on the instance without sharing a project + * with them first. + * + * This is the answer to "how do I create a user": there is no open registration + * and no account with a password somebody else chose, so adding a person means + * issuing an invite that carries no project. The whole loop is worth driving — + * the link, a second browser, the signup, and the new account showing up in the + * list as somebody who is *not* an administrator. + */ +test('en administrator kan invitere en bruger til instansen', async ({ browser, page }) => { + const trouble = watchForTrouble(page); + await page.goto('/indstillinger/brugere'); + + await page.getByLabel('E-mailadresse').fill('anders@example.dk'); + await page.getByRole('button', { name: 'Send invitation' }).click(); + + const link = await page.locator('.link-out').textContent(); + expect(link).toContain('/invite?token='); + await expect(page.getByText('anders@example.dk')).toBeVisible(); + await expect(page.getByText('til instansen')).toBeVisible(); + + const context = await browser.newContext({ storageState: { cookies: [], origins: [] } }); + const invitee = await context.newPage(); + await invitee.goto(link); + await invitee.getByLabel('Navn').fill('Anders'); + await invitee.getByLabel(/Adgangskode/).fill('et langt kodeord til test'); + await invitee.getByRole('button', { name: 'Opret konto' }).click(); + + // They are in — with an Inbox of their own, and no project shared with them, + // which is the difference between this and a project invite. + await expect(invitee.getByRole('navigation', { name: 'Hovedmenu' })).toBeVisible(); + await expect(invitee.getByRole('heading', { name: 'I dag' })).toBeVisible(); + await context.close(); + + await page.reload(); + const konti = page.locator('section.panel').filter({ hasText: 'Konti' }); + const row = konti.locator('li').filter({ hasText: 'Anders' }); + await expect(row).toBeVisible(); + // Invited, not promoted. Asserted through the button that offers to promote + // them — which only reads this way for somebody who is not one. Matching the + // word "administrator" in the row would match that very button. + await expect(row.getByRole('button', { name: 'Gør til administrator' })).toBeVisible(); + // And the invite is spent rather than left live. + await expect(page.getByText('Afventer svar')).toBeHidden(); + + expect(trouble).toEqual([]); +}); + +/** + * The two refusals that keep an instance administrable. Driven here as well as in + * the API tests because the interface has to *say* why, and a 409 nobody explains + * is a button that appears broken. + */ +test('den sidste administrator kan hverken fjernes eller slette sig selv', async ({ page }) => { + await page.goto('/indstillinger/brugere'); + + const konti = page.locator('section.panel').filter({ hasText: 'Konti' }); + const mine = konti.locator('li').filter({ hasText: 'dig' }); + await expect(mine).toBeVisible(); + + // No buttons on your own row: the server refuses both, and a button whose only + // purpose is to be refused wastes a click to teach a rule. + await expect(mine.getByRole('button')).toHaveCount(0); +}); + /** * The MCP connector address must not fall through to the app shell. * diff --git a/web/src/lib/api.js b/web/src/lib/api.js index cd78a2d..4c0f1d2 100644 --- a/web/src/lib/api.js +++ b/web/src/lib/api.js @@ -46,7 +46,9 @@ const MESSAGES = { 'Gmail er ikke sat op på denne server. Administratoren skal registrere en OAuth-klient hos Google og sætte VERDANDE_GMAIL_CLIENT_ID og _SECRET.', ai_not_configured: 'Der er ikke valgt en AI-udbyder. Sæt den op under Indstillinger → AI.', totp_not_enabled: 'To-faktor er ikke slået til.', - inbox_protected: 'Indbakken kan ikke slettes.' + inbox_protected: 'Indbakken kan ikke slettes.', + last_admin: + 'Det er den sidste administrator. Gør en anden til administrator først — ellers er der ingen, der kan komme ind på denne side igen.' }; /** @@ -170,6 +172,13 @@ export const api = { listSessions: () => get('/auth/sessions'), endSession: (id) => del(`/auth/sessions/${id}`), + // --- users, administrators only + listUsers: () => get('/users'), + inviteUser: (email) => post('/users', { email }), + setUserAdmin: (id, isAdmin) => patch(`/users/${id}`, { is_admin: isAdmin }), + deleteUser: (id) => del(`/users/${id}`), + revokeInvite: (id) => del(`/invites/${id}`), + recoveryCodes: () => get('/auth/recovery-codes'), regenerateRecoveryCodes: (password) => post('/auth/recovery-codes', { password }), diff --git a/web/src/routes/indstillinger/+layout.svelte b/web/src/routes/indstillinger/+layout.svelte index 9f35d65..b0ccaf1 100644 --- a/web/src/routes/indstillinger/+layout.svelte +++ b/web/src/routes/indstillinger/+layout.svelte @@ -11,17 +11,23 @@ * is six places for them to drift apart. */ import { page } from '$app/stores'; + import { app } from '$lib/stores.svelte.js'; let { children } = $props(); - const SECTIONS = [ + // Brugere is only shown to administrators — the API refuses everybody else, and + // a tab that answers 403 is a promise the interface cannot keep. Hidden rather + // than disabled: this is not a feature to upsell, it is one that does not + // apply to you. + let sections = $derived([ { href: '/indstillinger', label: 'Konto' }, { href: '/indstillinger/notifikationer', label: 'Notifikationer' }, { href: '/indstillinger/integrationer', label: 'Integrationer' }, { href: '/indstillinger/ai', label: 'AI' }, { href: '/indstillinger/tokens', label: 'API-tokens' }, + ...(app.user?.is_admin ? [{ href: '/indstillinger/brugere', label: 'Brugere' }] : []), { href: '/indstillinger/data', label: 'Data og skabeloner' } - ]; + ]); let current = $derived($page.url.pathname.replace(/\/$/, '') || '/indstillinger'); @@ -32,7 +38,7 @@ diff --git a/web/src/routes/indstillinger/brugere/+page.svelte b/web/src/routes/indstillinger/brugere/+page.svelte new file mode 100644 index 0000000..5bc7b4f --- /dev/null +++ b/web/src/routes/indstillinger/brugere/+page.svelte @@ -0,0 +1,316 @@ + + +
+
+

Inviter en bruger

+

+ Der er ingen åben registrering. Du sender et link, og personen vælger selv navn + og adgangskode — så er der ikke en adgangskode, to mennesker kender. +

+
+ +
+
+ + + {#if errors.email}

Skal være en e-mailadresse.

{/if} +
+
+ +
+
+ + {#if link} +
+

Invitationen er oprettet. Send linket til personen:

+ {link} +
+ {/if} +
+ +{#if invites.length} +
+
+

Afventer svar

+

+ Invitationer, der er sendt og ikke brugt endnu. Kun linkets hash er gemt, så + det kan ikke vises igen — er det gået til den forkerte, så træk det tilbage. +

+
+ + +
+{/if} + +
+
+

Konti

+

Alle på denne instans.

+
+ + +
+ +