Sharing backend: shares CRUD + ACL enforcement everywhere (#16) #35
@@ -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.GET("/:id/full", h.getGardenFull) // one-shot editor load
|
||||||
gardens.POST("/:id/objects", h.createObject)
|
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
|
// Objects are addressed by their own id; the service resolves the owning
|
||||||
// garden for the permission check.
|
// garden for the permission check.
|
||||||
objects := v1.Group("/objects", h.requireAuth())
|
objects := v1.Group("/objects", h.requireAuth())
|
||||||
|
|||||||
@@ -30,6 +30,12 @@ func writeServiceError(c *gin.Context, err error) {
|
|||||||
writeAPIError(c, http.StatusConflict, "VERSION_CONFLICT", "the resource was modified; refetch and retry")
|
writeAPIError(c, http.StatusConflict, "VERSION_CONFLICT", "the resource was modified; refetch and retry")
|
||||||
case errors.Is(err, domain.ErrPlantInUse):
|
case errors.Is(err, domain.ErrPlantInUse):
|
||||||
writeAPIError(c, http.StatusConflict, "PLANT_IN_USE", "this plant is used by plantings and can't be deleted")
|
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):
|
case errors.Is(err, domain.ErrInvalidCredentials):
|
||||||
writeAPIError(c, http.StatusUnauthorized, "INVALID_CREDENTIALS", "invalid email or password")
|
writeAPIError(c, http.StatusUnauthorized, "INVALID_CREDENTIALS", "invalid email or password")
|
||||||
case errors.Is(err, domain.ErrEmailTaken):
|
case errors.Is(err, domain.ErrEmailTaken):
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -21,6 +21,17 @@ var (
|
|||||||
// reference it (the FK is ON DELETE RESTRICT). Mapped to 409.
|
// reference it (the FK is ON DELETE RESTRICT). Mapped to 409.
|
||||||
ErrPlantInUse = errors.New("plant is referenced by plantings")
|
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
|
// ErrInvalidInput means the caller supplied structurally invalid data (empty
|
||||||
// required field, malformed value). Mapped to 400.
|
// required field, malformed value). Mapped to 400.
|
||||||
ErrInvalidInput = errors.New("invalid input")
|
ErrInvalidInput = errors.New("invalid input")
|
||||||
@@ -60,6 +71,9 @@ const (
|
|||||||
|
|
||||||
RoleViewer = "viewer"
|
RoleViewer = "viewer"
|
||||||
RoleEditor = "editor"
|
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"
|
KindBed = "bed"
|
||||||
KindGrowBag = "grow_bag"
|
KindGrowBag = "grow_bag"
|
||||||
@@ -116,6 +130,10 @@ type Garden struct {
|
|||||||
Version int64 `json:"version"`
|
Version int64 `json:"version"`
|
||||||
CreatedAt string `json:"createdAt"`
|
CreatedAt string `json:"createdAt"`
|
||||||
UpdatedAt string `json:"updatedAt"`
|
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.
|
// GardenShare grants a non-owner user viewer or editor access to a garden.
|
||||||
@@ -130,6 +148,14 @@ type GardenShare struct {
|
|||||||
UpdatedAt string `json:"updatedAt"`
|
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…).
|
// GardenObject is any placeable object in a garden (bed, container, tree, path…).
|
||||||
// Positioned by center point + rotation about center, in garden space.
|
// Positioned by center point + rotation about center, in garden space.
|
||||||
type GardenObject struct {
|
type GardenObject struct {
|
||||||
|
|||||||
+57
-14
@@ -32,6 +32,20 @@ const (
|
|||||||
roleOwner
|
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
|
||||||
|
case roleEditor:
|
||||||
|
return domain.RoleEditor
|
||||||
|
case roleViewer:
|
||||||
|
return domain.RoleViewer
|
||||||
|
default:
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// GardenInput is the mutable field set for creating or updating a garden.
|
// GardenInput is the mutable field set for creating or updating a garden.
|
||||||
type GardenInput struct {
|
type GardenInput struct {
|
||||||
Name string
|
Name string
|
||||||
@@ -54,23 +68,41 @@ func (s *Service) requireGardenRole(ctx context.Context, actorID, gardenID int64
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err // ErrNotFound or a real error
|
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 {
|
if role == roleNone {
|
||||||
return nil, domain.ErrNotFound
|
return nil, domain.ErrNotFound
|
||||||
}
|
}
|
||||||
if role < min {
|
if role < min {
|
||||||
return nil, domain.ErrForbidden
|
return nil, domain.ErrForbidden
|
||||||
}
|
}
|
||||||
|
g.MyRole = role.String() // so every read-through carries the actor's role
|
||||||
return g, nil
|
return g, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// effectiveGardenRole is the actor's role on a garden. Owner is implicit via
|
// effectiveGardenRole is the actor's role on a garden: owner (implicit via
|
||||||
// gardens.owner_id; share-based viewer/editor roles are added in #16.
|
// gardens.owner_id), else a viewer/editor grant from garden_shares, else none.
|
||||||
func effectiveGardenRole(actorID int64, g *domain.Garden) gardenRole {
|
func (s *Service) effectiveGardenRole(ctx context.Context, actorID int64, g *domain.Garden) (gardenRole, error) {
|
||||||
if g.OwnerID == actorID {
|
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
|
// 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
|
return nil, err
|
||||||
}
|
}
|
||||||
g.OwnerID = actorID
|
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.
|
// 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)
|
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) {
|
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
|
// UpdateGarden applies a version-guarded update to a garden's metadata; only the
|
||||||
// editor. On a version mismatch it returns (current garden, ErrVersionConflict)
|
// OWNER may edit metadata (editors edit contents, not the garden itself). On a
|
||||||
// so the handler can return the fresh row for the client to rebase.
|
// 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) {
|
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
|
return nil, err
|
||||||
}
|
}
|
||||||
g, err := gardenFromInput(in, false) // no defaults: an update states every field
|
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.ID = gardenID
|
||||||
g.Version = version
|
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
|
||||||
|
|
|||||||
|
}
|
||||||
|
return updated, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// DeleteGarden removes a garden; only the owner may.
|
// DeleteGarden removes a garden; only the owner may.
|
||||||
|
|||||||
@@ -121,9 +121,19 @@ func (s *Service) UpdatePlanting(ctx context.Context, actorID, plantingID int64,
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
originalPlantID := pl.PlantID
|
||||||
applyPlantingPatch(pl, patch)
|
applyPlantingPatch(pl, patch)
|
||||||
// The plant may have changed; the (possibly new) plant must be visible.
|
// Fetch the plop's plant for the derived count. Only when the actor is
|
||||||
plant, err := s.visiblePlant(ctx, actorID, pl.PlantID)
|
// 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
|
||||||
|
// 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 {
|
||||||
|
plant, err = s.store.GetPlant(ctx, pl.PlantID)
|
||||||
|
}
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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) {
|
||||||
|
gitea-actions
commented
🟡 AddShare exposes an authenticated user-enumeration oracle (unknown email → 404, known → 201/409) security · flagged by 1 model
🪰 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
|
||||||
|
gitea-actions
commented
🟡 AddShare 404 for unknown email enables authenticated account enumeration security · flagged by 1 model
🪰 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
|
||||||
|
// 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
|
||||||
|
}
|
||||||
|
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)
|
||||||
|
}
|
||||||
@@ -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) {
|
||||||
|
gitea-actions
commented
⚪ wantErr helper is local to shares_test.go but generally useful; other tests re-implement the same assertion inline maintainability · flagged by 1 model
🪰 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)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -23,7 +23,20 @@ func scanGarden(s scanner) (*domain.Garden, error) {
|
|||||||
return &g, nil
|
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
|
// 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.
|
// gardens, so this is never hit; genuine pagination is post-v1 if ever needed.
|
||||||
const maxGardensListed = 1000
|
const maxGardensListed = 1000
|
||||||
@@ -56,13 +69,21 @@ func (d *DB) GetGarden(ctx context.Context, id int64) (*domain.Garden, error) {
|
|||||||
return g, nil
|
return g, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// ListGardensForOwner returns the gardens owned by ownerID, newest first. The
|
// ListGardensForActor returns the gardens an actor can see — those they own
|
||||||
// slice is always non-nil (an empty list, not null). Shared-with-me gardens are
|
// (my_role "owner") plus those shared with them (my_role = the share's role) —
|
||||||
// added in #16.
|
// newest first. Always a non-nil slice.
|
||||||
func (d *DB) ListGardensForOwner(ctx context.Context, ownerID int64) ([]domain.Garden, error) {
|
func (d *DB) ListGardensForActor(ctx context.Context, actorID int64) ([]domain.Garden, error) {
|
||||||
rows, err := d.sql.QueryContext(ctx,
|
rows, err := d.sql.QueryContext(ctx,
|
||||||
`SELECT `+gardenColumns+` FROM gardens WHERE owner_id = ? ORDER BY created_at DESC, id DESC LIMIT ?`,
|
`SELECT `+gardenColumns+`, my_role FROM (
|
||||||
ownerID, maxGardensListed,
|
SELECT `+gardenColumns+`, 'owner' AS my_role FROM gardens WHERE owner_id = ?
|
||||||
|
gitea-actions
commented
🟡 SQL hardcodes 'owner' role string duplicating service-layer gardenRole.String() maintainability · flagged by 1 model
🪰 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 {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("store: list gardens: %w", err)
|
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{}
|
gardens := []domain.Garden{}
|
||||||
for rows.Next() {
|
for rows.Next() {
|
||||||
g, err := scanGarden(rows)
|
g, err := scanGardenWithRole(rows)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("store: scan garden: %w", err)
|
return nil, fmt.Errorf("store: scan garden: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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
|
||||||
|
|
||||||
|
// 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()
|
||||||
|
gitea-actions
commented
🟡 ListSharesForGarden inlines the share-column scan, duplicating shareColumns order instead of extending scanShare maintainability · flagged by 1 model
🪰 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
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user
🟡 UpdateGarden hardcodes roleOwner.String() instead of reusing requireGardenRole result
maintainability · flagged by 1 model
internal/service/gardens.go:151—UpdateGardenhardcodesupdated.MyRole = roleOwner.String()instead of reusing the role already computed and stamped byrequireGardenRole. The returned garden fromrequireGardenRoleis 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 fromrequireGardenRoleand use its…🪰 Gadfly · advisory