Sharing backend: shares CRUD + ACL enforcement everywhere (#16) #35

Merged
steve merged 2 commits from phase-6-sharing-api into main 2026-07-19 03:49:55 +00:00
11 changed files with 733 additions and 24 deletions
+6
View File
@@ -84,6 +84,12 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
gardens.GET("/:id/full", h.getGardenFull) // one-shot editor load
gardens.POST("/:id/objects", h.createObject)
// Sharing (owner-managed; a recipient may remove their own share).
gardens.GET("/:id/shares", h.listShares)
gardens.POST("/:id/shares", h.addShare)
gardens.PATCH("/:id/shares/:userId", h.updateShare)
gardens.DELETE("/:id/shares/:userId", h.removeShare)
// Objects are addressed by their own id; the service resolves the owning
// garden for the permission check.
objects := v1.Group("/objects", h.requireAuth())
+6
View File
@@ -30,6 +30,12 @@ func writeServiceError(c *gin.Context, err error) {
writeAPIError(c, http.StatusConflict, "VERSION_CONFLICT", "the resource was modified; refetch and retry")
case errors.Is(err, domain.ErrPlantInUse):
writeAPIError(c, http.StatusConflict, "PLANT_IN_USE", "this plant is used by plantings and can't be deleted")
case errors.Is(err, domain.ErrShareUserNotFound):
writeAPIError(c, http.StatusNotFound, "SHARE_USER_NOT_FOUND", "no account with that email")
case errors.Is(err, domain.ErrCannotShareWithSelf):
writeAPIError(c, http.StatusBadRequest, "CANNOT_SHARE_WITH_SELF", "you can't share a garden with yourself")
case errors.Is(err, domain.ErrShareExists):
writeAPIError(c, http.StatusConflict, "SHARE_EXISTS", "this garden is already shared with that user")
case errors.Is(err, domain.ErrInvalidCredentials):
writeAPIError(c, http.StatusUnauthorized, "INVALID_CREDENTIALS", "invalid email or password")
case errors.Is(err, domain.ErrEmailTaken):
+88
View File
@@ -0,0 +1,88 @@
package api
import (
"net/http"
"github.com/gin-gonic/gin"
)
// shareCreateRequest is the body for POST /gardens/:id/shares: the recipient's
// exact account email and the role to grant (viewer|editor).
type shareCreateRequest struct {
Email string `json:"email" binding:"required"`
Role string `json:"role" binding:"required"`
}
// shareUpdateRequest is the body for PATCH /gardens/:id/shares/:userId.
type shareUpdateRequest struct {
Role string `json:"role" binding:"required"`
}
func (h *handlers) listShares(c *gin.Context) {
gardenID, ok := parseIDParam(c, "id")
if !ok {
return
}
shares, err := h.svc.ListShares(c.Request.Context(), mustActor(c).ID, gardenID)
if err != nil {
writeServiceError(c, err)
return
}
c.JSON(http.StatusOK, shares)
}
func (h *handlers) addShare(c *gin.Context) {
gardenID, ok := parseIDParam(c, "id")
if !ok {
return
}
var req shareCreateRequest
if err := c.ShouldBindJSON(&req); err != nil {
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "an email and a role are required")
return
}
sh, err := h.svc.AddShare(c.Request.Context(), mustActor(c).ID, gardenID, req.Email, req.Role)
if err != nil {
writeServiceError(c, err)
return
}
c.JSON(http.StatusCreated, sh)
}
func (h *handlers) updateShare(c *gin.Context) {
gardenID, ok := parseIDParam(c, "id")
if !ok {
return
}
userID, ok := parseIDParam(c, "userId")
if !ok {
return
}
var req shareUpdateRequest
if err := c.ShouldBindJSON(&req); err != nil {
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "a role is required")
return
}
sh, err := h.svc.UpdateShareRole(c.Request.Context(), mustActor(c).ID, gardenID, userID, req.Role)
if err != nil {
writeServiceError(c, err)
return
}
c.JSON(http.StatusOK, sh)
}
func (h *handlers) removeShare(c *gin.Context) {
gardenID, ok := parseIDParam(c, "id")
if !ok {
return
}
userID, ok := parseIDParam(c, "userId")
if !ok {
return
}
if err := h.svc.RemoveShare(c.Request.Context(), mustActor(c).ID, gardenID, userID); err != nil {
writeServiceError(c, err)
return
}
c.Status(http.StatusNoContent)
}
+91
View File
@@ -0,0 +1,91 @@
package api
import (
"encoding/json"
"net/http"
"strconv"
"testing"
)
func sharesPath(gid int64) string { return "/api/v1/gardens/" + strconv.FormatInt(gid, 10) + "/shares" }
func sharePath(gid, userID int64) string {
return sharesPath(gid) + "/" + strconv.FormatInt(userID, 10)
}
func TestSharingFlowAPI(t *testing.T) {
r := authEngine(t, localCfg())
a := registerAndCookie(t, r, "[email protected]")
b := registerAndCookie(t, r, "[email protected]")
gid := createGardenAPI(t, r, a, "A's yard")
// A bed to mutate later.
w := doJSON(t, r, http.MethodPost, objectsPath(gid),
map[string]any{"kind": "bed", "xCm": 500, "yCm": 500, "widthCm": 200, "heightCm": 200}, a)
oid := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
// Unknown email → 404 SHARE_USER_NOT_FOUND.
w = doJSON(t, r, http.MethodPost, sharesPath(gid), map[string]any{"email": "[email protected]", "role": "viewer"}, a)
if w.Code != http.StatusNotFound {
t.Fatalf("share unknown email = %d, want 404 (body %s)", w.Code, w.Body.String())
}
// Share with B as viewer → 201.
w = doJSON(t, r, http.MethodPost, sharesPath(gid), map[string]any{"email": "[email protected]", "role": "viewer"}, a)
if w.Code != http.StatusCreated {
t.Fatalf("share viewer = %d, body %s", w.Code, w.Body.String())
}
bID := int64(decodeMap(t, w.Body.Bytes())["userId"].(float64))
// B lists → the garden with myRole viewer.
w = doJSON(t, r, http.MethodGet, "/api/v1/gardens", nil, b)
var list []map[string]any
if err := json.Unmarshal(w.Body.Bytes(), &list); err != nil {
t.Fatalf("decode b list: %v", err)
}
if len(list) != 1 || list[0]["myRole"] != "viewer" {
t.Fatalf("b list = %+v, want one garden with myRole viewer", list)
}
// B reads /full (200) but can't mutate (403) or manage shares (403).
if w := doJSON(t, r, http.MethodGet, fullPath(gid), nil, b); w.Code != http.StatusOK {
t.Errorf("b read full = %d, want 200", w.Code)
}
if w := doJSON(t, r, http.MethodPatch, objectPath(oid), map[string]any{"xCm": 100, "version": 1}, b); w.Code != http.StatusForbidden {
t.Errorf("viewer mutate object = %d, want 403", w.Code)
}
if w := doJSON(t, r, http.MethodGet, sharesPath(gid), nil, b); w.Code != http.StatusForbidden {
t.Errorf("viewer list shares = %d, want 403", w.Code)
}
// A upgrades B to editor.
if w := doJSON(t, r, http.MethodPatch, sharePath(gid, bID), map[string]any{"role": "editor"}, a); w.Code != http.StatusOK {
t.Fatalf("upgrade role = %d, body %s", w.Code, w.Body.String())
}
// B can now move the object, but still can't rename the garden (owner-only).
if w := doJSON(t, r, http.MethodPatch, objectPath(oid), map[string]any{"xCm": 100, "version": 1}, b); w.Code != http.StatusOK {
t.Errorf("editor move object = %d, want 200 (body %s)", w.Code, w.Body.String())
}
if w := doJSON(t, r, http.MethodPatch, gardenPath(gid),
map[string]any{"name": "hijack", "widthCm": 100, "heightCm": 100, "unitPref": "metric", "version": 1}, b); w.Code != http.StatusForbidden {
t.Errorf("editor rename garden = %d, want 403", w.Code)
}
// A removes the share → B loses access.
if w := doJSON(t, r, http.MethodDelete, sharePath(gid, bID), nil, a); w.Code != http.StatusNoContent {
t.Fatalf("remove share = %d", w.Code)
}
if w := doJSON(t, r, http.MethodGet, fullPath(gid), nil, b); w.Code != http.StatusNotFound {
t.Errorf("b read after unshare = %d, want 404", w.Code)
}
if w := doJSON(t, r, http.MethodGet, "/api/v1/gardens", nil, b); w.Body.String() != "[]" {
t.Errorf("b list after unshare = %s, want []", w.Body.String())
}
}
func TestSharesRequireAuth(t *testing.T) {
r := authEngine(t, localCfg())
if w := doJSON(t, r, http.MethodGet, "/api/v1/gardens/1/shares", nil, nil); w.Code != http.StatusUnauthorized {
t.Errorf("unauthenticated list shares = %d, want 401", w.Code)
}
}
+26
View File
@@ -21,6 +21,17 @@ var (
// reference it (the FK is ON DELETE RESTRICT). Mapped to 409.
ErrPlantInUse = errors.New("plant is referenced by plantings")
// ErrShareUserNotFound means no account exists for the email a share invite
// targeted (v1 shares an existing user only — no invitation emails). Mapped to
// 404 with a clear "no account with that email" message.
ErrShareUserNotFound = errors.New("no account with that email")
// ErrCannotShareWithSelf means the owner tried to share a garden with their
// own account. Mapped to 400.
ErrCannotShareWithSelf = errors.New("cannot share a garden with yourself")
// ErrShareExists means the garden is already shared with that user. Mapped to
// 409 (change the existing share's role instead).
ErrShareExists = errors.New("garden already shared with that user")
// ErrInvalidInput means the caller supplied structurally invalid data (empty
// required field, malformed value). Mapped to 400.
ErrInvalidInput = errors.New("invalid input")
@@ -60,6 +71,9 @@ const (
RoleViewer = "viewer"
RoleEditor = "editor"
// RoleOwner is not a garden_shares row (ownership is implicit via
// gardens.owner_id); it's the value Garden.MyRole carries for an owner.
RoleOwner = "owner"
KindBed = "bed"
KindGrowBag = "grow_bag"
@@ -116,6 +130,10 @@ type Garden struct {
Version int64 `json:"version"`
CreatedAt string `json:"createdAt"`
UpdatedAt string `json:"updatedAt"`
// MyRole is the requesting actor's effective role on this garden ("owner",
// "editor", or "viewer"). Computed by the service, never persisted.
MyRole string `json:"myRole,omitempty"`
}
// GardenShare grants a non-owner user viewer or editor access to a garden.
@@ -130,6 +148,14 @@ type GardenShare struct {
UpdatedAt string `json:"updatedAt"`
}
// ShareWithUser is a garden share plus the target user's identity, for the
// sharing UI's list (which shows who a garden is shared with).
type ShareWithUser struct {
GardenShare
Email string `json:"email"`
DisplayName string `json:"displayName"`
}
// GardenObject is any placeable object in a garden (bed, container, tree, path…).
// Positioned by center point + rotation about center, in garden space.
type GardenObject struct {
+57 -14
View File
@@ -32,6 +32,20 @@ const (
roleOwner
)
// String renders a role for the API's Garden.MyRole ("owner"/"editor"/"viewer").
func (r gardenRole) String() string {
switch r {
case roleOwner:
return domain.RoleOwner
Outdated
Review

🟡 "owner" role string duplicated between service String() and store SQL literal; no domain.RoleOwner constant

maintainability · flagged by 2 models

  • internal/service/gardens.go:39"owner" role string is hardcoded in gardenRole.String() while the store SQL at internal/store/gardens.go:78 emits 'owner' AS my_role as a literal. There is no domain.RoleOwner constant (only RoleViewer/RoleEditor). If either side drifts, owned-garden MyRole silently mismatches. Fix: add domain.RoleOwner = "owner" and reference it in both places. - internal/store/shares.go:61ListSharesForGarden inlines the share-column scan ins…

🪰 Gadfly · advisory

🟡 **"owner" role string duplicated between service String() and store SQL literal; no domain.RoleOwner constant** _maintainability · flagged by 2 models_ - **`internal/service/gardens.go:39`** — `"owner"` role string is hardcoded in `gardenRole.String()` while the store SQL at `internal/store/gardens.go:78` emits `'owner' AS my_role` as a literal. There is no `domain.RoleOwner` constant (only `RoleViewer`/`RoleEditor`). If either side drifts, owned-garden `MyRole` silently mismatches. Fix: add `domain.RoleOwner = "owner"` and reference it in both places. - **`internal/store/shares.go:61`** — `ListSharesForGarden` inlines the share-column scan ins… <sub>🪰 Gadfly · advisory</sub>
case roleEditor:
return domain.RoleEditor
case roleViewer:
return domain.RoleViewer
default:
return ""
}
}
// GardenInput is the mutable field set for creating or updating a garden.
type GardenInput struct {
Name string
@@ -54,23 +68,41 @@ func (s *Service) requireGardenRole(ctx context.Context, actorID, gardenID int64
if err != nil {
return nil, err // ErrNotFound or a real error
}
role := effectiveGardenRole(actorID, g)
role, err := s.effectiveGardenRole(ctx, actorID, g)
if err != nil {
return nil, err
}
if role == roleNone {
return nil, domain.ErrNotFound
}
if role < min {
return nil, domain.ErrForbidden
}
g.MyRole = role.String() // so every read-through carries the actor's role
return g, nil
}
// effectiveGardenRole is the actor's role on a garden. Owner is implicit via
// gardens.owner_id; share-based viewer/editor roles are added in #16.
func effectiveGardenRole(actorID int64, g *domain.Garden) gardenRole {
// effectiveGardenRole is the actor's role on a garden: owner (implicit via
// gardens.owner_id), else a viewer/editor grant from garden_shares, else none.
func (s *Service) effectiveGardenRole(ctx context.Context, actorID int64, g *domain.Garden) (gardenRole, error) {
if g.OwnerID == actorID {
return roleOwner
return roleOwner, nil
}
role, found, err := s.store.GetShareRole(ctx, g.ID, actorID)
if err != nil {
return roleNone, err
}
if !found {
return roleNone, nil
}
switch role {
case domain.RoleEditor:
return roleEditor, nil
case domain.RoleViewer:
return roleViewer, nil
default:
return roleNone, nil
}
return roleNone
}
// CreateGarden creates a garden owned by the actor. Missing dimensions default
@@ -81,7 +113,12 @@ func (s *Service) CreateGarden(ctx context.Context, actorID int64, in GardenInpu
return nil, err
}
g.OwnerID = actorID
return s.store.CreateGarden(ctx, g)
created, err := s.store.CreateGarden(ctx, g)
if err != nil {
return nil, err
}
created.MyRole = roleOwner.String() // the creator owns it
return created, nil
}
// GetGarden returns a garden the actor may at least view.
@@ -89,16 +126,18 @@ func (s *Service) GetGarden(ctx context.Context, actorID, gardenID int64) (*doma
return s.requireGardenRole(ctx, actorID, gardenID, roleViewer)
}
// ListGardens returns the gardens the actor can see. Owned-only until #16.
// ListGardens returns the gardens the actor can see: owned plus shared-with-them,
// each row carrying the actor's my_role.
func (s *Service) ListGardens(ctx context.Context, actorID int64) ([]domain.Garden, error) {
return s.store.ListGardensForOwner(ctx, actorID)
return s.store.ListGardensForActor(ctx, actorID)
}
// UpdateGarden applies a version-guarded update; the actor must be at least an
// editor. On a version mismatch it returns (current garden, ErrVersionConflict)
// so the handler can return the fresh row for the client to rebase.
// UpdateGarden applies a version-guarded update to a garden's metadata; only the
// OWNER may edit metadata (editors edit contents, not the garden itself). On a
// version mismatch it returns (current garden, ErrVersionConflict) so the handler
// can return the fresh row for the client to rebase.
func (s *Service) UpdateGarden(ctx context.Context, actorID, gardenID int64, in GardenInput, version int64) (*domain.Garden, error) {
if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleEditor); err != nil {
if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleOwner); err != nil {
return nil, err
}
g, err := gardenFromInput(in, false) // no defaults: an update states every field
@@ -107,7 +146,11 @@ func (s *Service) UpdateGarden(ctx context.Context, actorID, gardenID int64, in
}
g.ID = gardenID
g.Version = version
return s.store.UpdateGarden(ctx, g)
updated, err := s.store.UpdateGarden(ctx, g)
if updated != nil {
updated.MyRole = roleOwner.String() // only the owner reaches here
Review

🟡 UpdateGarden hardcodes roleOwner.String() instead of reusing requireGardenRole result

maintainability · flagged by 1 model

  • internal/service/gardens.go:151UpdateGarden hardcodes updated.MyRole = roleOwner.String() instead of reusing the role already computed and stamped by requireGardenRole. The returned garden from requireGardenRole is discarded (if _, err := ...), so the permission result is recalculated by hand. This is brittle: if the model ever allows non-owners through (co-owners, admin override), the response will falsely claim "owner". Capture the garden from requireGardenRole and use its…

🪰 Gadfly · advisory

🟡 **UpdateGarden hardcodes roleOwner.String() instead of reusing requireGardenRole result** _maintainability · flagged by 1 model_ - `internal/service/gardens.go:151` — `UpdateGarden` hardcodes `updated.MyRole = roleOwner.String()` instead of reusing the role already computed and stamped by `requireGardenRole`. The returned garden from `requireGardenRole` is discarded (`if _, err := ...`), so the permission result is recalculated by hand. This is brittle: if the model ever allows non-owners through (co-owners, admin override), the response will falsely claim `"owner"`. Capture the garden from `requireGardenRole` and use its… <sub>🪰 Gadfly · advisory</sub>
}
return updated, err
}
// DeleteGarden removes a garden; only the owner may.
+12 -2
View File
@@ -121,9 +121,19 @@ func (s *Service) UpdatePlanting(ctx context.Context, actorID, plantingID int64,
return nil, err
}
originalPlantID := pl.PlantID
applyPlantingPatch(pl, patch)
// The plant may have changed; the (possibly new) plant must be visible.
plant, err := s.visiblePlant(ctx, actorID, pl.PlantID)
// Fetch the plop's plant for the derived count. Only when the actor is
// actually CHANGING the plant (new id ≠ old) is the new plant gated on
// visibility — an existing plop may reference a plant the actor can't see
// (e.g. a shared editor in the owner's garden using the owner's private
Outdated
Review

🟠 Plant-visibility re-check triggers on patch field presence, not actual value change, defeating the shared-editor fix for a no-op plantId resend

error-handling, maintainability · flagged by 1 model

  • internal/service/plantings.go:129-133 — The "only re-check plant visibility when the plant is changed" guard checks whether patch.PlantID is present, not whether the value actually differs from the planting's current PlantID: go if patch.PlantID != nil { if _, err := s.visiblePlant(ctx, actorID, pl.PlantID); err != nil { return nil, err } } If a caller sends plantId in the PATCH body equal to the plop's existing plant (e.g. a user re-opens a plant picker and re-selects…

🪰 Gadfly · advisory

🟠 **Plant-visibility re-check triggers on patch field presence, not actual value change, defeating the shared-editor fix for a no-op plantId resend** _error-handling, maintainability · flagged by 1 model_ - **`internal/service/plantings.go:129-133`** — The "only re-check plant visibility when the plant is *changed*" guard checks whether `patch.PlantID` is *present*, not whether the value actually *differs* from the planting's current `PlantID`: ```go if patch.PlantID != nil { if _, err := s.visiblePlant(ctx, actorID, pl.PlantID); err != nil { return nil, err } } ``` If a caller sends `plantId` in the PATCH body equal to the plop's existing plant (e.g. a user re-opens a plant picker and re-selects… <sub>🪰 Gadfly · advisory</sub>
// plant), and a no-op plantId resend must not break editing it.
var plant *domain.Plant
if pl.PlantID != originalPlantID {
plant, err = s.visiblePlant(ctx, actorID, pl.PlantID)
} else {
Outdated
Review

🟡 Redundant duplicate GetPlant query in UpdatePlanting when the plant is changed

error-handling, performance · flagged by 2 models

  • internal/service/plantings.go:134GetPlant after a plant-id change can return ErrNotFound and surface as a raw 404 where ErrInvalidInput (400) was intended. When patch.PlantID != nil, the code first calls s.visiblePlant(...) which maps a missing/invisible plant to ErrInvalidInput (line 130, confirmed at plantings.go:176-187). But visiblePlant already fetches the plant and returns it — that value is discarded (if _, err := ...), and then line 134 calls `s.store.GetPlan…

🪰 Gadfly · advisory

🟡 **Redundant duplicate GetPlant query in UpdatePlanting when the plant is changed** _error-handling, performance · flagged by 2 models_ - `internal/service/plantings.go:134` — **`GetPlant` after a plant-id change can return `ErrNotFound` and surface as a raw 404 where `ErrInvalidInput` (400) was intended.** When `patch.PlantID != nil`, the code first calls `s.visiblePlant(...)` which maps a missing/invisible plant to `ErrInvalidInput` (line 130, confirmed at `plantings.go:176-187`). But `visiblePlant` already fetches the plant and returns it — that value is discarded (`if _, err := ...`), and then line 134 calls `s.store.GetPlan… <sub>🪰 Gadfly · advisory</sub>
plant, err = s.store.GetPlant(ctx, pl.PlantID)
}
if err != nil {
return nil, err
}
+79
View File
@@ -0,0 +1,79 @@
package service
import (
"context"
"errors"
"strings"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
)
// isShareRole reports whether role is a grantable share role (owner is implicit,
// never a share row).
func isShareRole(role string) bool {
return role == domain.RoleViewer || role == domain.RoleEditor
}
// ListShares returns a garden's shares (each with the recipient's identity).
// Owner only — sharing is managed by the owner alone.
func (s *Service) ListShares(ctx context.Context, actorID, gardenID int64) ([]domain.ShareWithUser, error) {
if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleOwner); err != nil {
return nil, err
}
return s.store.ListSharesForGarden(ctx, gardenID)
}
// AddShare grants a user viewer/editor access to a garden, targeting them by the
// exact email of an existing account (v1 has no invitation emails). Owner only.
// Unknown email → ErrShareUserNotFound; the owner's own email →
// ErrCannotShareWithSelf; an already-shared user → ErrShareExists.
func (s *Service) AddShare(ctx context.Context, actorID, gardenID int64, email, role string) (*domain.GardenShare, error) {
Review

🟡 AddShare exposes an authenticated user-enumeration oracle (unknown email → 404, known → 201/409)

security · flagged by 1 model

  • internal/service/shares.go:30 (AddShare) — authenticated user-enumeration oracle. Any authenticated owner of a garden can probe whether an arbitrary email is a registered pansy account: calling POST /gardens/:id/shares with {"email": "<guess>", "role": "viewer"} returns 404 SHARE_USER_NOT_FOUND for an unknown email (mapping at internal/api/errors.go:33-34) vs. 201 (or 409 SHARE_EXISTS per errors.go:37-38) for a known one. The owner can then DELETE the share to clean up.…

🪰 Gadfly · advisory

🟡 **AddShare exposes an authenticated user-enumeration oracle (unknown email → 404, known → 201/409)** _security · flagged by 1 model_ - `internal/service/shares.go:30` (`AddShare`) — **authenticated user-enumeration oracle.** Any authenticated owner of a garden can probe whether an arbitrary email is a registered pansy account: calling `POST /gardens/:id/shares` with `{"email": "<guess>", "role": "viewer"}` returns `404 SHARE_USER_NOT_FOUND` for an unknown email (mapping at `internal/api/errors.go:33-34`) vs. `201` (or `409 SHARE_EXISTS` per `errors.go:37-38`) for a known one. The owner can then `DELETE` the share to clean up.… <sub>🪰 Gadfly · advisory</sub>
if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleOwner); err != nil {
return nil, err
}
if !isShareRole(role) {
return nil, domain.ErrInvalidInput
}
target, err := s.store.GetUserByEmail(ctx, strings.TrimSpace(email))
if errors.Is(err, domain.ErrNotFound) {
return nil, domain.ErrShareUserNotFound
Review

🟡 AddShare 404 for unknown email enables authenticated account enumeration

security · flagged by 1 model

  • internal/service/shares.go:39 (AddShare) — authenticated account enumeration. GetUserByEmail returning domain.ErrNotFound is converted to ErrShareUserNotFound, which writeServiceError maps to a distinct 404 SHARE_USER_NOT_FOUND with message "no account with that email"; a valid target yields 409/201. Since any authenticated user can create a garden (becoming its owner) and then call AddShare on it, any logged-in user can probe arbitrary email addresses to learn whether th…

🪰 Gadfly · advisory

🟡 **AddShare 404 for unknown email enables authenticated account enumeration** _security · flagged by 1 model_ - `internal/service/shares.go:39` (`AddShare`) — **authenticated account enumeration.** `GetUserByEmail` returning `domain.ErrNotFound` is converted to `ErrShareUserNotFound`, which `writeServiceError` maps to a distinct 404 `SHARE_USER_NOT_FOUND` with message "no account with that email"; a valid target yields 409/201. Since any authenticated user can create a garden (becoming its owner) and then call `AddShare` on it, *any* logged-in user can probe arbitrary email addresses to learn whether th… <sub>🪰 Gadfly · advisory</sub>
}
if err != nil {
return nil, err
}
if target.ID == actorID {
return nil, domain.ErrCannotShareWithSelf
}
return s.store.CreateShare(ctx, &domain.GardenShare{
GardenID: gardenID, UserID: target.ID, Role: role, CreatedBy: actorID,
})
}
// UpdateShareRole changes an existing share's role. Owner only.
func (s *Service) UpdateShareRole(ctx context.Context, actorID, gardenID, targetUserID int64, role string) (*domain.GardenShare, error) {
if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleOwner); err != nil {
return nil, err
}
if !isShareRole(role) {
return nil, domain.ErrInvalidInput
}
return s.store.UpdateShareRole(ctx, gardenID, targetUserID, role)
}
// RemoveShare revokes a share. The garden owner may remove anyone; a recipient
// may remove themselves ("leave garden"). It routes through requireGardenRole
// (roleViewer) so a non-participant gets the standard masked ErrNotFound, then
// applies the owner-or-self rule on top (a participant removing someone else's
Outdated
Review

🔴 RemoveShare bypasses requireGardenRole authorization choke point

maintainability, security · flagged by 5 models

  • RemoveShare bypasses the requireGardenRole authorization choke point, allowing an actor with no role on a garden to reach store.DeleteShare. internal/service/shares.go:66 opens the garden with a raw store.GetGarden instead of funneling through requireGardenRole — the comment on requireGardenRole calls it “THE place authorization for a garden is decided.” A stranger can call RemoveShare with targetUserID == actorID; the ad-hoc check `g.OwnerID != actorID && actorID != targ…

🪰 Gadfly · advisory

🔴 **RemoveShare bypasses requireGardenRole authorization choke point** _maintainability, security · flagged by 5 models_ - **`RemoveShare` bypasses the `requireGardenRole` authorization choke point**, allowing an actor with no role on a garden to reach `store.DeleteShare`. `internal/service/shares.go:66` opens the garden with a raw `store.GetGarden` instead of funneling through `requireGardenRole` — the comment on `requireGardenRole` calls it “THE place authorization for a garden is decided.” A stranger can call `RemoveShare` with `targetUserID == actorID`; the ad-hoc check `g.OwnerID != actorID && actorID != targ… <sub>🪰 Gadfly · advisory</sub>
// share is ErrForbidden — they can already see the garden).
func (s *Service) RemoveShare(ctx context.Context, actorID, gardenID, targetUserID int64) error {
g, err := s.requireGardenRole(ctx, actorID, gardenID, roleViewer)
if err != nil {
return err // ErrNotFound for a non-participant
Outdated
Review

🟡 Owner removing a non-existent share returns ErrNotFound (404) instead of 204; behavior is defensible but undocumented beyond a comment

error-handling, maintainability, security · flagged by 2 models

  • internal/service/shares.go:71Owner removing a non-existent share / self-leave of a non-share returns ErrNotFound instead of 204. In RemoveShare, the guard is g.OwnerID != actorID && actorID != targetUserID (confirmed). The GetGarden lookup at line 67 masks the garden from strangers (good). But the owner path and the self-leave path both fall through to s.store.DeleteShare (line 76), which returns domain.ErrNotFound when 0 rows are affected (confirmed at `internal/store/sha…

🪰 Gadfly · advisory

🟡 **Owner removing a non-existent share returns ErrNotFound (404) instead of 204; behavior is defensible but undocumented beyond a comment** _error-handling, maintainability, security · flagged by 2 models_ - `internal/service/shares.go:71` — **Owner removing a non-existent share / self-leave of a non-share returns `ErrNotFound` instead of 204.** In `RemoveShare`, the guard is `g.OwnerID != actorID && actorID != targetUserID` (confirmed). The `GetGarden` lookup at line 67 masks the garden from strangers (good). But the owner path and the self-leave path both fall through to `s.store.DeleteShare` (line 76), which returns `domain.ErrNotFound` when 0 rows are affected (confirmed at `internal/store/sha… <sub>🪰 Gadfly · advisory</sub>
}
if g.OwnerID != actorID && actorID != targetUserID {
return domain.ErrForbidden
}
// Owner path removes any share; self-leave removes the actor's own (a missing
// row is ErrNotFound either way).
return s.store.DeleteShare(ctx, gardenID, targetUserID)
}
+205
View File
@@ -0,0 +1,205 @@
package service
import (
"context"
"errors"
"testing"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
)
// wantErr asserts err matches want (nil = expected success, else errors.Is).
func wantErr(t *testing.T, name string, err, want error) {
Review

wantErr helper is local to shares_test.go but generally useful; other tests re-implement the same assertion inline

maintainability · flagged by 1 model

  • internal/service/gardens.go:39"owner" role string is hardcoded in gardenRole.String() while the store SQL at internal/store/gardens.go:78 emits 'owner' AS my_role as a literal. There is no domain.RoleOwner constant (only RoleViewer/RoleEditor). If either side drifts, owned-garden MyRole silently mismatches. Fix: add domain.RoleOwner = "owner" and reference it in both places. - internal/store/shares.go:61ListSharesForGarden inlines the share-column scan ins…

🪰 Gadfly · advisory

⚪ **wantErr helper is local to shares_test.go but generally useful; other tests re-implement the same assertion inline** _maintainability · flagged by 1 model_ - **`internal/service/gardens.go:39`** — `"owner"` role string is hardcoded in `gardenRole.String()` while the store SQL at `internal/store/gardens.go:78` emits `'owner' AS my_role` as a literal. There is no `domain.RoleOwner` constant (only `RoleViewer`/`RoleEditor`). If either side drifts, owned-garden `MyRole` silently mismatches. Fix: add `domain.RoleOwner = "owner"` and reference it in both places. - **`internal/store/shares.go:61`** — `ListSharesForGarden` inlines the share-column scan ins… <sub>🪰 Gadfly · advisory</sub>
t.Helper()
if want == nil {
if err != nil {
t.Errorf("%s: got %v, want success", name, err)
}
return
}
if !errors.Is(err, want) {
t.Errorf("%s: got %v, want %v", name, err, want)
}
}
// TestGardenACLMatrix is the authoritative {owner, editor, viewer, stranger} ×
// {read, mutate object, mutate planting, edit garden, delete garden, manage
// shares} table. Read the columns as: nil = allowed, ErrForbidden = visible but
// insufficient, ErrNotFound = existence masked.
func TestGardenACLMatrix(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
editor := seedUser(t, s, "[email protected]")
viewer := seedUser(t, s, "[email protected]")
stranger := seedUser(t, s, "[email protected]")
g := seedGarden(t, s, owner)
bed := seedBed(t, s, owner, g.ID)
plant := seedOwnPlant(t, s, owner, 10)
plop, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 10})
if err != nil {
t.Fatalf("seed plop: %v", err)
}
if _, err := s.AddShare(ctx, owner, g.ID, "[email protected]", domain.RoleEditor); err != nil {
t.Fatalf("share editor: %v", err)
}
if _, err := s.AddShare(ctx, owner, g.ID, "[email protected]", domain.RoleViewer); err != nil {
t.Fatalf("share viewer: %v", err)
}
// read /full — viewer and up; stranger masked.
for _, tc := range []struct {
actor int64
want error
}{{owner, nil}, {editor, nil}, {viewer, nil}, {stranger, domain.ErrNotFound}} {
_, err := s.GardenFull(ctx, tc.actor, g.ID)
wantErr(t, "readFull", err, tc.want)
}
// mutate object — editor and up. Failure actors fail the role check before any
// version check, so a dummy version is fine; success actors need the live one.
x := 300.0
_, ev := s.UpdateObject(ctx, viewer, bed.ID, ObjectPatch{XCM: &x}, 1)
wantErr(t, "mutObject/viewer", ev, domain.ErrForbidden)
_, es := s.UpdateObject(ctx, stranger, bed.ID, ObjectPatch{XCM: &x}, 1)
wantErr(t, "mutObject/stranger", es, domain.ErrNotFound)
cur, _ := s.store.GetObject(ctx, bed.ID)
_, ee := s.UpdateObject(ctx, editor, bed.ID, ObjectPatch{XCM: &x}, cur.Version)
wantErr(t, "mutObject/editor", ee, nil)
cur, _ = s.store.GetObject(ctx, bed.ID)
_, eo := s.UpdateObject(ctx, owner, bed.ID, ObjectPatch{YCM: &x}, cur.Version)
wantErr(t, "mutObject/owner", eo, nil)
// mutate planting — editor and up.
nr := 12.0
_, pv := s.UpdatePlanting(ctx, viewer, plop.ID, PlantingPatch{RadiusCM: &nr}, 1)
wantErr(t, "mutPlop/viewer", pv, domain.ErrForbidden)
_, ps := s.UpdatePlanting(ctx, stranger, plop.ID, PlantingPatch{RadiusCM: &nr}, 1)
wantErr(t, "mutPlop/stranger", ps, domain.ErrNotFound)
curp, _ := s.store.GetPlanting(ctx, plop.ID)
_, pe := s.UpdatePlanting(ctx, editor, plop.ID, PlantingPatch{RadiusCM: &nr}, curp.Version)
wantErr(t, "mutPlop/editor", pe, nil)
// edit garden meta — OWNER only (editors edit contents, not the garden).
edit := func(actor, version int64) error {
_, err := s.UpdateGarden(ctx, actor, g.ID, GardenInput{Name: "Renamed", WidthCM: 1000, HeightCM: 1000, UnitPref: "metric"}, version)
return err
}
wantErr(t, "editGarden/editor", edit(editor, 1), domain.ErrForbidden)
wantErr(t, "editGarden/viewer", edit(viewer, 1), domain.ErrForbidden)
wantErr(t, "editGarden/stranger", edit(stranger, 1), domain.ErrNotFound)
curg, _ := s.GetGarden(ctx, owner, g.ID)
wantErr(t, "editGarden/owner", edit(owner, curg.Version), nil)
// manage shares (list) — OWNER only.
for _, tc := range []struct {
actor int64
want error
}{{editor, domain.ErrForbidden}, {viewer, domain.ErrForbidden}, {stranger, domain.ErrNotFound}, {owner, nil}} {
_, err := s.ListShares(ctx, tc.actor, g.ID)
wantErr(t, "listShares", err, tc.want)
}
// delete garden — OWNER only. Non-owners are refused (garden survives).
wantErr(t, "delGarden/editor", s.DeleteGarden(ctx, editor, g.ID), domain.ErrForbidden)
wantErr(t, "delGarden/viewer", s.DeleteGarden(ctx, viewer, g.ID), domain.ErrForbidden)
wantErr(t, "delGarden/stranger", s.DeleteGarden(ctx, stranger, g.ID), domain.ErrNotFound)
wantErr(t, "delGarden/owner", s.DeleteGarden(ctx, owner, g.ID), nil)
}
func TestListGardensIncludesSharedWithRole(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
other := seedUser(t, s, "[email protected]")
g := seedGarden(t, s, owner)
// Owner sees it as "owner".
own, _ := s.ListGardens(ctx, owner)
if len(own) != 1 || own[0].MyRole != domain.RoleOwner {
t.Fatalf("owner list = %+v, want one garden with myRole owner", own)
}
// other sees nothing yet.
if list, _ := s.ListGardens(ctx, other); len(list) != 0 {
t.Fatalf("other list = %d, want 0 before sharing", len(list))
}
// Share as viewer → other sees it with myRole viewer.
if _, err := s.AddShare(ctx, owner, g.ID, "[email protected]", domain.RoleViewer); err != nil {
t.Fatalf("share: %v", err)
}
list, _ := s.ListGardens(ctx, other)
if len(list) != 1 || list[0].MyRole != domain.RoleViewer || list[0].ID != g.ID {
t.Fatalf("other list after share = %+v, want the garden with myRole viewer", list)
}
}
func TestAddShareErrors(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
seedUser(t, s, "[email protected]")
g := seedGarden(t, s, owner)
// Unknown email → ErrShareUserNotFound.
if _, err := s.AddShare(ctx, owner, g.ID, "[email protected]", domain.RoleViewer); !errors.Is(err, domain.ErrShareUserNotFound) {
t.Errorf("unknown email err = %v, want ErrShareUserNotFound", err)
}
// Self → ErrCannotShareWithSelf.
if _, err := s.AddShare(ctx, owner, g.ID, "[email protected]", domain.RoleViewer); !errors.Is(err, domain.ErrCannotShareWithSelf) {
t.Errorf("self-share err = %v, want ErrCannotShareWithSelf", err)
}
// Bad role → ErrInvalidInput.
if _, err := s.AddShare(ctx, owner, g.ID, "[email protected]", "admin"); !errors.Is(err, domain.ErrInvalidInput) {
t.Errorf("bad role err = %v, want ErrInvalidInput", err)
}
// First share ok; a second to the same user → ErrShareExists.
if _, err := s.AddShare(ctx, owner, g.ID, "[email protected]", domain.RoleViewer); err != nil {
t.Fatalf("first share: %v", err)
}
if _, err := s.AddShare(ctx, owner, g.ID, "[email protected]", domain.RoleEditor); !errors.Is(err, domain.ErrShareExists) {
t.Errorf("double-share err = %v, want ErrShareExists", err)
}
}
func TestUpdateAndRemoveShare(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
friend := seedUser(t, s, "[email protected]")
g := seedGarden(t, s, owner)
if _, err := s.AddShare(ctx, owner, g.ID, "[email protected]", domain.RoleViewer); err != nil {
t.Fatalf("share: %v", err)
}
// Upgrade to editor.
up, err := s.UpdateShareRole(ctx, owner, g.ID, friend, domain.RoleEditor)
if err != nil || up.Role != domain.RoleEditor {
t.Fatalf("upgrade role = %+v, %v", up, err)
}
// The recipient can now mutate but still can't manage shares.
bed := seedBed(t, s, owner, g.ID)
nx := 400.0
cur, _ := s.store.GetObject(ctx, bed.ID)
if _, err := s.UpdateObject(ctx, friend, bed.ID, ObjectPatch{XCM: &nx}, cur.Version); err != nil {
t.Errorf("editor should move an object: %v", err)
}
if _, err := s.ListShares(ctx, friend, g.ID); !errors.Is(err, domain.ErrForbidden) {
t.Errorf("editor listing shares = %v, want ErrForbidden", err)
}
// A recipient can remove themselves ("leave garden").
if err := s.RemoveShare(ctx, friend, g.ID, friend); err != nil {
t.Errorf("self-leave: %v", err)
}
// After leaving, the garden is invisible again.
if _, err := s.GardenFull(ctx, friend, g.ID); !errors.Is(err, domain.ErrNotFound) {
t.Errorf("after leaving, read = %v, want ErrNotFound", err)
}
// A non-participant can't remove someone else's share.
stranger := seedUser(t, s, "[email protected]")
if err := s.RemoveShare(ctx, stranger, g.ID, owner); !errors.Is(err, domain.ErrNotFound) {
t.Errorf("stranger remove = %v, want ErrNotFound", err)
}
}
+29 -8
View File
@@ -23,7 +23,20 @@ func scanGarden(s scanner) (*domain.Garden, error) {
return &g, nil
}
// maxGardensListed caps ListGardensForOwner as a defensive backstop against a
// scanGardenWithRole reads a garden row plus a trailing my_role column into the
// computed Garden.MyRole field (used by the actor-scoped list).
func scanGardenWithRole(s scanner) (*domain.Garden, error) {
var g domain.Garden
if err := s.Scan(
&g.ID, &g.OwnerID, &g.Name, &g.WidthCM, &g.HeightCM,
&g.UnitPref, &g.Notes, &g.Version, &g.CreatedAt, &g.UpdatedAt, &g.MyRole,
); err != nil {
return nil, err
}
return &g, nil
}
// maxGardensListed caps the gardens list as a defensive backstop against a
// pathologically large result. At household scale a user has a handful of
// gardens, so this is never hit; genuine pagination is post-v1 if ever needed.
const maxGardensListed = 1000
@@ -56,13 +69,21 @@ func (d *DB) GetGarden(ctx context.Context, id int64) (*domain.Garden, error) {
return g, nil
}
// ListGardensForOwner returns the gardens owned by ownerID, newest first. The
// slice is always non-nil (an empty list, not null). Shared-with-me gardens are
// added in #16.
func (d *DB) ListGardensForOwner(ctx context.Context, ownerID int64) ([]domain.Garden, error) {
// ListGardensForActor returns the gardens an actor can see — those they own
// (my_role "owner") plus those shared with them (my_role = the share's role) —
// newest first. Always a non-nil slice.
func (d *DB) ListGardensForActor(ctx context.Context, actorID int64) ([]domain.Garden, error) {
rows, err := d.sql.QueryContext(ctx,
`SELECT `+gardenColumns+` FROM gardens WHERE owner_id = ? ORDER BY created_at DESC, id DESC LIMIT ?`,
ownerID, maxGardensListed,
`SELECT `+gardenColumns+`, my_role FROM (
SELECT `+gardenColumns+`, 'owner' AS my_role FROM gardens WHERE owner_id = ?
Review

🟡 SQL hardcodes 'owner' role string duplicating service-layer gardenRole.String()

maintainability · flagged by 1 model

  • internal/store/gardens.go:78 — The ListGardensForActor query hardcodes the string literal 'owner' for the owner role ('owner' AS my_role), while the service layer derives the same value via roleOwner.String(). This duplicates the role-string definition across store and service layers; if the API contract changes, both places must be updated or they drift apart.

🪰 Gadfly · advisory

🟡 **SQL hardcodes 'owner' role string duplicating service-layer gardenRole.String()** _maintainability · flagged by 1 model_ - `internal/store/gardens.go:78` — The `ListGardensForActor` query hardcodes the string literal `'owner'` for the owner role (`'owner' AS my_role`), while the service layer derives the same value via `roleOwner.String()`. This duplicates the role-string definition across store and service layers; if the API contract changes, both places must be updated or they drift apart. <sub>🪰 Gadfly · advisory</sub>
UNION ALL
SELECT `+qualifyColumns("g", gardenColumns)+`, s.role AS my_role
FROM gardens g JOIN garden_shares s ON s.garden_id = g.id
WHERE s.user_id = ?
)
ORDER BY created_at DESC, id DESC
LIMIT ?`,
actorID, actorID, maxGardensListed,
)
if err != nil {
return nil, fmt.Errorf("store: list gardens: %w", err)
@@ -71,7 +92,7 @@ func (d *DB) ListGardensForOwner(ctx context.Context, ownerID int64) ([]domain.G
gardens := []domain.Garden{}
for rows.Next() {
g, err := scanGarden(rows)
g, err := scanGardenWithRole(rows)
if err != nil {
return nil, fmt.Errorf("store: scan garden: %w", err)
}
+134
View File
@@ -0,0 +1,134 @@
package store
import (
"context"
"database/sql"
"errors"
"fmt"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
)
// shareColumns lists garden_shares columns in the order scanShare expects.
const shareColumns = `id, garden_id, user_id, role, created_by, version, created_at, updated_at`
func scanShare(s scanner) (*domain.GardenShare, error) {
var sh domain.GardenShare
if err := s.Scan(
&sh.ID, &sh.GardenID, &sh.UserID, &sh.Role, &sh.CreatedBy,
&sh.Version, &sh.CreatedAt, &sh.UpdatedAt,
); err != nil {
return nil, err
}
return &sh, nil
}
// GetShareRole returns the role a user holds on a garden via a share row, and
// whether such a row exists. Owner access is NOT a share row (it's implicit via
// gardens.owner_id) — this only reports viewer/editor grants. It is the lookup
// requireGardenRole uses to consult shares.
func (d *DB) GetShareRole(ctx context.Context, gardenID, userID int64) (role string, found bool, err error) {
err = d.sql.QueryRowContext(ctx,
`SELECT role FROM garden_shares WHERE garden_id = ? AND user_id = ?`, gardenID, userID,
).Scan(&role)
if errors.Is(err, sql.ErrNoRows) {
return "", false, nil
}
if err != nil {
return "", false, fmt.Errorf("store: get share role: %w", err)
}
return role, true, nil
}
// maxSharesListed bounds a garden's share list defensively (a garden is shared
// with a handful of people at household scale; this is never hit).
const maxSharesListed = 1000
Outdated
Review

🟠 ListSharesForGarden is unbounded — missing LIMIT clause

performance · flagged by 1 model

  • internal/store/shares.go:46ListSharesForGarden queries garden_shares joined with users but has no LIMIT clause, making the result set unbounded. ListGardensForActor (in the same PR area) explicitly caps at maxGardensListed = 1000 as a "defensive backstop against a pathologically large result"; the shares list should have the same defense. Even at normal sharing scales this is unlikely to be hit, but the endpoint should not be the only unbounded list query in the surface. *…

🪰 Gadfly · advisory

🟠 **ListSharesForGarden is unbounded — missing LIMIT clause** _performance · flagged by 1 model_ - **`internal/store/shares.go:46`** — `ListSharesForGarden` queries `garden_shares` joined with `users` but has no `LIMIT` clause, making the result set unbounded. `ListGardensForActor` (in the same PR area) explicitly caps at `maxGardensListed = 1000` as a "defensive backstop against a pathologically large result"; the shares list should have the same defense. Even at normal sharing scales this is unlikely to be hit, but the endpoint should not be the only unbounded list query in the surface. *… <sub>🪰 Gadfly · advisory</sub>
// ListSharesForGarden returns a garden's shares joined with each recipient's
// email and display name, for the sharing UI. Always a non-nil slice.
func (d *DB) ListSharesForGarden(ctx context.Context, gardenID int64) ([]domain.ShareWithUser, error) {
rows, err := d.sql.QueryContext(ctx,
`SELECT `+qualifyColumns("s", shareColumns)+`, u.email, u.display_name
FROM garden_shares s JOIN users u ON u.id = s.user_id
WHERE s.garden_id = ?
ORDER BY u.display_name COLLATE NOCASE, s.id
LIMIT ?`,
gardenID, maxSharesListed,
)
if err != nil {
return nil, fmt.Errorf("store: list shares: %w", err)
}
defer rows.Close()
Review

🟡 ListSharesForGarden inlines the share-column scan, duplicating shareColumns order instead of extending scanShare

maintainability · flagged by 1 model

  • internal/service/gardens.go:39"owner" role string is hardcoded in gardenRole.String() while the store SQL at internal/store/gardens.go:78 emits 'owner' AS my_role as a literal. There is no domain.RoleOwner constant (only RoleViewer/RoleEditor). If either side drifts, owned-garden MyRole silently mismatches. Fix: add domain.RoleOwner = "owner" and reference it in both places. - internal/store/shares.go:61ListSharesForGarden inlines the share-column scan ins…

🪰 Gadfly · advisory

🟡 **ListSharesForGarden inlines the share-column scan, duplicating shareColumns order instead of extending scanShare** _maintainability · flagged by 1 model_ - **`internal/service/gardens.go:39`** — `"owner"` role string is hardcoded in `gardenRole.String()` while the store SQL at `internal/store/gardens.go:78` emits `'owner' AS my_role` as a literal. There is no `domain.RoleOwner` constant (only `RoleViewer`/`RoleEditor`). If either side drifts, owned-garden `MyRole` silently mismatches. Fix: add `domain.RoleOwner = "owner"` and reference it in both places. - **`internal/store/shares.go:61`** — `ListSharesForGarden` inlines the share-column scan ins… <sub>🪰 Gadfly · advisory</sub>
shares := []domain.ShareWithUser{}
for rows.Next() {
var sh domain.ShareWithUser
if err := rows.Scan(
&sh.ID, &sh.GardenID, &sh.UserID, &sh.Role, &sh.CreatedBy,
&sh.Version, &sh.CreatedAt, &sh.UpdatedAt, &sh.Email, &sh.DisplayName,
); err != nil {
return nil, fmt.Errorf("store: scan share: %w", err)
}
shares = append(shares, sh)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("store: iterate shares: %w", err)
}
return shares, nil
}
// CreateShare inserts a share (fields validated by the service) and returns the
// stored row. A duplicate (garden_id, user_id) trips the UNIQUE index and maps to
// domain.ErrShareExists.
func (d *DB) CreateShare(ctx context.Context, sh *domain.GardenShare) (*domain.GardenShare, error) {
created, err := scanShare(d.sql.QueryRowContext(ctx,
`INSERT INTO garden_shares (garden_id, user_id, role, created_by)
VALUES (?, ?, ?, ?)
RETURNING `+shareColumns,
sh.GardenID, sh.UserID, sh.Role, sh.CreatedBy,
))
if err != nil {
if isUniqueViolation(err) {
return nil, domain.ErrShareExists
}
return nil, fmt.Errorf("store: insert share: %w", err)
}
return created, nil
}
// UpdateShareRole changes a share's role, returning the updated row or
// domain.ErrNotFound if no such share exists.
func (d *DB) UpdateShareRole(ctx context.Context, gardenID, userID int64, role string) (*domain.GardenShare, error) {
updated, err := scanShare(d.sql.QueryRowContext(ctx,
`UPDATE garden_shares
SET role = ?, version = version + 1,
updated_at = strftime('%Y-%m-%dT%H:%M:%SZ', 'now')
WHERE garden_id = ? AND user_id = ?
RETURNING `+shareColumns,
role, gardenID, userID,
))
if errors.Is(err, sql.ErrNoRows) {
return nil, domain.ErrNotFound
}
if err != nil {
return nil, fmt.Errorf("store: update share role: %w", err)
}
return updated, nil
}
// DeleteShare removes a share. Returns domain.ErrNotFound if no row was deleted.
func (d *DB) DeleteShare(ctx context.Context, gardenID, userID int64) error {
res, err := d.sql.ExecContext(ctx,
`DELETE FROM garden_shares WHERE garden_id = ? AND user_id = ?`, gardenID, userID)
if err != nil {
return fmt.Errorf("store: delete share: %w", err)
}
n, err := res.RowsAffected()
if err != nil {
return fmt.Errorf("store: share delete rows: %w", err)
}
if n == 0 {
return domain.ErrNotFound
}
return nil
}