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
The previous hasAccessToLabel implementation ran `Get(ll)` against a
label_tasks LEFT JOIN with no ORDER BY, which meant the database was
free to pick any matching row. When a label had multiple attachments,
or when access was granted via the creator branch while the label also
had label_tasks rows pointing at inaccessible tasks, the picked row
could belong to a task the caller could not actually read.
That led to two concrete problems reported on the follow-up review of
GHSA-hj5c-mhh2-g7jq:
1. maxPermission (exposed as the x-max-permission response header)
could be derived from a task the caller has no access to, ending
up as 0 or lower than the caller's real best permission on the
label.
2. Task.CanRead on a dangling/inaccessible task could return an
error and surface as a 500, even though the label itself was
perfectly readable via the creator branch.
Split the logic instead:
* Use `Exist` for the boolean access check, using the same carefully
grouped `And(Eq{labels.id}, Or(accessibleTask, creator))` cond.
* Compute maxPermission by selecting the label_tasks rows whose
task lives in a project the caller can access, then iterating
those tasks with `Task.CanRead` and taking the maximum.
* Fall back to PermissionRead when the access was granted via the
creator branch and no accessible task attachment exists.
hasAccessToLabel built its WHERE clause by chaining xorm session .Where,
.Or, and .And calls. xorm flattened those to `WHERE A OR B OR C AND D`,
which under SQL precedence evaluates as
`A OR B OR (C AND D)` — so the `labels.id = ?` predicate only narrowed
the project-access branch. The standalone
`label_tasks.label_id IS NOT NULL` branch leaked every label with any
label_tasks row to any authenticated user, and the
`labels.created_by_id = ?` branch leaked any label the caller had ever
created regardless of the requested id.
Rewrite the query using explicit builder.And / builder.Or grouping so
the label-id scope wraps the entire disjunction, drop the bogus
label_tasks-is-not-null branch, and keep the creator branch only for
real user auths. Replace Exist(ll) with Get(ll) so the resulting
LabelTask row is populated and the follow-up Task.CanRead check that
computes maxPermission actually runs; fall back to PermissionRead when
the match came via the creator branch and no task row is joined.
This fixes a bug where tasks which were filtered out by their label would still be shown. That was caused by the way the filter query was translated to sql under the hood.
Resolves https://github.com/go-vikunja/vikunja/issues/394
Fix query param name
Add option to include null results when filtering
Always set db time to gmt
Fix null filter
Fix timezone setting for todoist parsing
Fix timezone setting for wunderlist parsing
Fix import
Fix caldav reminder parsing
Use timezone from config
Add error and test for invalid filter values
Fix integration tests
Remove task collection date hack
Fix task filter
Fix lint
Fix tests and fixtures for date timezone stuff
Properly set timezone
Change fixtures time zone to gmt
Set db timezone
Set created and updated timestamps for all fixtures
Fix lint
Fix test fixtures
Fix misspell
Fix test fixtures
Partially fix tests
Remove timeutil package
Remove adding _unix suffix hack
Remove _unix suffix
Move all timeutil.TimeStamp to time.Time
Remove all Unix suffixes in field names
Add better error messages when running migrations
Make sure to not migrate 0 unix timestamps to 1970 iso dates
Add migration script for sqlite
Add converting sqlite values
Convert 0 unix timestamps to null in postgres
Convert 0 to null in timestamps
Automatically rename _unix suffix
Add all tables and columns for migration
Fix sql migration query for mysql
Fail with an error if trying to use an unsupported dbms
Co-authored-by: kolaente <k@knt.li>
Reviewed-on: https://kolaente.dev/vikunja/api/pulls/594
fix copyright date
Add more user tests
More user tests
More user tests
Start refactoring user tests
Docs
Fix lint
Fix db fixtures init in tests
Fix models test
Fix loading fixtures
Fix ineffasign
Fix lint
Fix integration tests
Fix init of test engine creation
Fix user related tests
Better handling of creating test enging
Moved all fixtures to db package
Moved all fixtures to db package
Moved user related stuff to seperate package
Co-authored-by: kolaente <k@knt.li>
Reviewed-on: https://kolaente.dev/vikunja/api/pulls/123
2020-01-26 17:08:06 +00:00
Renamed from pkg/models/fixtures/labels.yml (Browse further)
Claude
kolaente
konrad