feat(labels): let bot owners manage labels created by their bots
Bot owners inherit read/update/delete permission on labels created by bots they own, mirroring the bot-owner branch already used by API tokens (see api_tokens_permissions.go). Without this, a label a bot creates is permanently locked to that bot and the human owner cannot maintain it. https://claude.ai/code/session_016x6mUPJuuQEeXpHY814iLh
This commit is contained in:
parent
e1512b6b53
commit
c9c2c58c16
|
|
@ -43,3 +43,11 @@
|
|||
created_by_id: 1
|
||||
updated: 2018-12-02 15:13:12
|
||||
created: 2018-12-01 15:13:12
|
||||
# Covers the bot-owner branch: created by bot 23, whose owner is user 21.
|
||||
# User 21 should be able to read/update/delete it; user 22 (who owns bot 24)
|
||||
# should not.
|
||||
- id: 9
|
||||
title: 'Label #9 - created by bot 23 owned by user 21'
|
||||
created_by_id: 23
|
||||
updated: 2018-12-02 15:13:12
|
||||
created: 2018-12-01 15:13:12
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@
|
|||
package models
|
||||
|
||||
import (
|
||||
"code.vikunja.io/api/pkg/user"
|
||||
"code.vikunja.io/api/pkg/web"
|
||||
"xorm.io/builder"
|
||||
"xorm.io/xorm"
|
||||
|
|
@ -57,7 +58,19 @@ func (l *Label) isLabelOwner(s *xorm.Session, a web.Auth) (bool, error) {
|
|||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return lorig.CreatedByID == a.GetID(), nil
|
||||
if lorig.CreatedByID == a.GetID() {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// A bot owner inherits write/delete access to labels their bots created.
|
||||
creator, err := user.GetUserByID(s, lorig.CreatedByID)
|
||||
if err != nil {
|
||||
if user.IsErrUserDoesNotExist(err) {
|
||||
return false, nil
|
||||
}
|
||||
return false, err
|
||||
}
|
||||
return creator.IsBot() && creator.BotOwnerID == a.GetID(), nil
|
||||
}
|
||||
|
||||
// hasAccessToLabel reports whether the caller can read a label and, if so,
|
||||
|
|
@ -91,7 +104,12 @@ func (l *Label) hasAccessToLabel(s *xorm.Session, a web.Auth) (has bool, maxPerm
|
|||
|
||||
accessBranches := []builder.Cond{labelAttachedToAccessibleTask}
|
||||
if !isLinkShare {
|
||||
accessBranches = append(accessBranches, builder.Eq{"labels.created_by_id": a.GetID()})
|
||||
accessBranches = append(accessBranches,
|
||||
builder.Eq{"labels.created_by_id": a.GetID()},
|
||||
builder.In("labels.created_by_id",
|
||||
builder.Select("id").From("users").Where(builder.Eq{"bot_owner_id": a.GetID()}),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
cond := builder.And(
|
||||
|
|
|
|||
|
|
@ -212,7 +212,12 @@ func GetLabelsByTaskIDs(s *xorm.Session, opts *LabelByTaskIDsOptions) (ls []*Lab
|
|||
), cond)
|
||||
}
|
||||
if opts.GetUnusedLabels && !isLinkShareAuth {
|
||||
cond = builder.Or(cond, builder.Eq{"labels.created_by_id": opts.User.GetID()})
|
||||
cond = builder.Or(cond,
|
||||
builder.Eq{"labels.created_by_id": opts.User.GetID()},
|
||||
builder.In("labels.created_by_id",
|
||||
builder.Select("id").From("users").Where(builder.Eq{"bot_owner_id": opts.User.GetID()}),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
ids := []int64{}
|
||||
|
|
|
|||
|
|
@ -336,6 +336,46 @@ func TestLabel_ReadOne(t *testing.T) {
|
|||
wantForbidden: true,
|
||||
auth: &user.User{ID: 1},
|
||||
},
|
||||
{
|
||||
// Label 9 was created by bot 23, whose owner is user 21. The
|
||||
// bot owner inherits admin-level access.
|
||||
name: "bot owner can read label created by their bot",
|
||||
fields: fields{
|
||||
ID: 9,
|
||||
},
|
||||
want: &Label{
|
||||
ID: 9,
|
||||
Title: "Label #9 - created by bot 23 owned by user 21",
|
||||
CreatedByID: 23,
|
||||
CreatedBy: &user.User{
|
||||
ID: 23,
|
||||
Name: "Owner A Assistant",
|
||||
Username: "bot-owner-a-assistant",
|
||||
Issuer: "local",
|
||||
BotOwnerID: 21,
|
||||
EmailRemindersEnabled: true,
|
||||
OverdueTasksRemindersEnabled: true,
|
||||
OverdueTasksRemindersTime: "09:00",
|
||||
Created: testCreatedTime,
|
||||
Updated: testUpdatedTime,
|
||||
},
|
||||
Created: testCreatedTime,
|
||||
Updated: testUpdatedTime,
|
||||
},
|
||||
auth: &user.User{ID: 21},
|
||||
assertMaxPermission: true,
|
||||
wantMaxPermission: int(PermissionAdmin),
|
||||
},
|
||||
{
|
||||
// User 22 owns a different bot and must not see another owner's
|
||||
// bot's label.
|
||||
name: "non-owner cannot read label created by someone else's bot",
|
||||
fields: fields{
|
||||
ID: 9,
|
||||
},
|
||||
wantForbidden: true,
|
||||
auth: &user.User{ID: 22},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
@ -507,6 +547,27 @@ func TestLabel_Update(t *testing.T) {
|
|||
auth: &user.User{ID: 1},
|
||||
wantForbidden: true,
|
||||
},
|
||||
{
|
||||
// Label 9 was created by bot 23 (owned by user 21). The bot's
|
||||
// owner inherits update permission.
|
||||
name: "bot owner can update label created by their bot",
|
||||
fields: fields{
|
||||
ID: 9,
|
||||
Title: "new and better",
|
||||
},
|
||||
auth: &user.User{ID: 21},
|
||||
},
|
||||
{
|
||||
// User 22 owns a different bot and must not be able to update
|
||||
// another owner's bot's label.
|
||||
name: "non-owner cannot update label created by someone else's bot",
|
||||
fields: fields{
|
||||
ID: 9,
|
||||
Title: "new and better",
|
||||
},
|
||||
auth: &user.User{ID: 22},
|
||||
wantForbidden: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
@ -594,6 +655,25 @@ func TestLabel_Delete(t *testing.T) {
|
|||
auth: &user.User{ID: 1},
|
||||
wantForbidden: true,
|
||||
},
|
||||
{
|
||||
// Label 9 was created by bot 23 (owned by user 21). The bot's
|
||||
// owner inherits delete permission.
|
||||
name: "bot owner can delete label created by their bot",
|
||||
fields: fields{
|
||||
ID: 9,
|
||||
},
|
||||
auth: &user.User{ID: 21},
|
||||
},
|
||||
{
|
||||
// User 22 owns a different bot and must not be able to delete
|
||||
// another owner's bot's label.
|
||||
name: "non-owner cannot delete label created by someone else's bot",
|
||||
fields: fields{
|
||||
ID: 9,
|
||||
},
|
||||
auth: &user.User{ID: 22},
|
||||
wantForbidden: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -24,10 +24,17 @@ import (
|
|||
"strings"
|
||||
"testing"
|
||||
|
||||
"code.vikunja.io/api/pkg/user"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// testuser22 is the second bot owner from pkg/db/fixtures/users.yml; user22
|
||||
// owns bot 24. Paired with testuser21 to assert bot-owner isolation: each
|
||||
// owner sees and acts on their own bots' resources, never the other's.
|
||||
var testuser22 = user.User{ID: 22, Username: "user_bot_owner_b", Issuer: "local"}
|
||||
|
||||
// TestHumaLabel mirrors v1's TestProject shape so v2 contract parity is
|
||||
// readable side-by-side. Labels has no v1 webtest; coverage is ported 1:1
|
||||
// from the model-level matrix in pkg/models/label_test.go so the v2 HTTP
|
||||
|
|
@ -228,6 +235,65 @@ func TestHumaLabel(t *testing.T) {
|
|||
})
|
||||
}
|
||||
|
||||
// TestHumaLabel_BotOwner asserts that bot owners can read, update, and delete
|
||||
// labels that were created by bots they own. Fixture label #9 is owned by
|
||||
// bot 23, whose owner is user 21 (testuser21); user 22 owns a different bot
|
||||
// and must not see or touch it.
|
||||
func TestHumaLabel_BotOwner(t *testing.T) {
|
||||
botOwner := webHandlerTestV2{
|
||||
user: &testuser21,
|
||||
basePath: "/api/v2/labels",
|
||||
idParam: "label",
|
||||
t: t,
|
||||
}
|
||||
require.NoError(t, botOwner.ensureEnv())
|
||||
otherOwner := webHandlerTestV2{
|
||||
user: &testuser22,
|
||||
basePath: "/api/v2/labels",
|
||||
idParam: "label",
|
||||
t: t,
|
||||
e: botOwner.e,
|
||||
}
|
||||
|
||||
t.Run("ReadOne - bot owner can read label created by their bot", func(t *testing.T) {
|
||||
rec, err := botOwner.testReadOneWithUser(nil, map[string]string{"label": "9"})
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, rec.Body.String(), `"title":"Label #9 - created by bot 23 owned by user 21"`)
|
||||
})
|
||||
t.Run("ReadOne - non-owner cannot read another owner's bot's label", func(t *testing.T) {
|
||||
_, err := otherOwner.testReadOneWithUser(nil, map[string]string{"label": "9"})
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err))
|
||||
})
|
||||
t.Run("ReadAll - bot owner's listing surfaces their bot's labels", func(t *testing.T) {
|
||||
rec, err := botOwner.testReadAllWithUser(nil, nil)
|
||||
require.NoError(t, err)
|
||||
ids := labelIDsFromReadAll(t, rec.Body.Bytes())
|
||||
assert.Contains(t, ids, int64(9), "label #9 (created by user 21's bot) must be listed")
|
||||
})
|
||||
t.Run("Update - bot owner can update label created by their bot", func(t *testing.T) {
|
||||
rec, err := botOwner.testUpdateWithUser(nil, map[string]string{"label": "9"}, `{"title":"renamed by owner"}`)
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, rec.Body.String(), `"title":"renamed by owner"`)
|
||||
})
|
||||
t.Run("Update - non-owner cannot update another owner's bot's label", func(t *testing.T) {
|
||||
_, err := otherOwner.testUpdateWithUser(nil, map[string]string{"label": "9"}, `{"title":"hijack"}`)
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err))
|
||||
})
|
||||
t.Run("Delete - non-owner cannot delete another owner's bot's label", func(t *testing.T) {
|
||||
_, err := otherOwner.testDeleteWithUser(nil, map[string]string{"label": "9"})
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err))
|
||||
})
|
||||
t.Run("Delete - bot owner can delete label created by their bot", func(t *testing.T) {
|
||||
// Run last so the earlier subtests still have label #9 to operate on.
|
||||
rec, err := botOwner.testDeleteWithUser(nil, map[string]string{"label": "9"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, http.StatusNoContent, rec.Code)
|
||||
})
|
||||
}
|
||||
|
||||
// labelIDsFromReadAll extracts the label IDs from a v2 paginated list body so
|
||||
// the visible set can be asserted exactly rather than via substring matching.
|
||||
func labelIDsFromReadAll(t *testing.T, body []byte) []int64 {
|
||||
|
|
|
|||
Loading…
Reference in New Issue