Both refresh endpoints answered a missing cookie with a code-less 401, so
clients couldn't tell it apart from a rotated-away token, which is worth one
retry. ErrNoRefreshToken gives it its own code.
Refs #4023
The test declared its own minimal label struct. With
VIKUNJA_TESTS_USE_CONFIG the test engine is the database every other test
package shares, so syncing that partial struct created `labels` without
`created_by_id` and left a row in it. Every package running afterwards
then failed to add the column as NOT NULL.
Label.Delete only deleted the row in `labels`. Since `label_tasks` has
no foreign key cascade and no soft-delete column, every association of
the deleted label stayed behind as an orphaned row.
Outgoing proxies usually live on a private network, so the SSRF guard
refused to connect to them unless allownonroutableips was enabled,
which disabled the guard for direct connections too. Only the dial to
the configured proxy skips the guard now; proxied targets are resolved
by the proxy.
The SSRF-safe client used a bare http.Transport, so HTTP_PROXY,
HTTPS_PROXY and NO_PROXY were ignored. Clone the default transport and
read the proxy from the environment when outgoingrequests.proxyurl is
not set.
Both `usernamefallback` + `emailfallback` on: lookup required username
AND email match, so local user with different username never linked,
duplicate account created. Now verified-email-only lookup always tried
last.
Fixes#2022
## How to verify
1. Create a local user `localuser` with email `me@example.com`.
2. Configure an OIDC provider with `usernamefallback: true` and
`emailfallback: true`. In the IdP, create a user whose email is
`me@example.com` (sent with `email_verified: true`) and whose
username/sub is different from `localuser`.
3. Log in to Vikunja through that OIDC provider.
4. **Expected:** You are logged in as `localuser`, and `vikunja user
list` shows no new user for the provider's issuer.
**Before this PR:** a second user with a random or IdP-provided username
was created.
---------
Co-authored-by: kolaente <k@knt.li>
ListParams defaulted per_page to 50 and allowed up to 1000 regardless of the instance setting. per_page now defaults to and is silently capped at service.maxitemsperpage, like /api/v1, and the response envelope reports the effective value.
CanDelete loads the whole filter into the struct. xorm turns every non-empty
field into a delete condition, so a filters JSON that does not re-serialize
byte for byte silently matches nothing while still returning success.
Target is not persisted, so rendering the title of a stored
TaskAssignedNotification dereferenced a nil pointer and the feed
returned a 500.
Fixes#3914
1026 was already taken by ErrorCodeUsernameReserved, which is older, so a
locked account was reported as a reserved username in clients.
BREAKING CHANGE: a locked account now returns 1040 instead of 1026.
1025 was already taken by ErrorCodeInvalidTimezone, which is older, so
reusing a TOTP passcode (e.g. logging in right after enabling 2FA) showed
"The timezone '' is invalid." in the frontend.
Refs #3900
BREAKING CHANGE: a reused TOTP passcode now returns 1039 instead of 1025.
Credit to @jnsvgt for the root-cause analysis and suggested fix in
#3871. This PR implements that fix and adds regression tests.
`caldavTimeToTimestamp()` used the `Z` suffix only to pick the layout,
where it matches as a literal character. The value was then parsed in
the server timezone. With a non-UTC `service.timezone`, every UTC value
without a `TZID` was shifted by the server's offset. Absolute VALARM
triggers are always UTC, so reminders were stored hours early.
Fix: when the value ends in `Z`, parse it in UTC and ignore any `TZID`
(RFC 5545 doesn't allow combining the two). Floating values and `TZID`
values work as before. DTSTART, DUE, DTEND, DTSTAMP and COMPLETED all go
through the same helper, so they get the fix too.
Tests, with the server timezone set to Europe/Berlin:
- `TestCaldavTimeToTimestamp_NonUTCServerTimezone`: covers UTC,
floating, TZID, and UTC combined with TZID
- `TestParseTaskFromVTODO_UTCAlarmNonUTCServerTimezone`: the VTODO from
the issue; the reminder should match the due date
Before the fix, the UTC cases and the alarm test fail (`05:00 CEST`
instead of `05:00 UTC`). After the fix, they pass. `go test
./pkg/caldav`, `mage test:caldav` and `mage lint` also pass.
I wrote this with help from Claude Code, then reviewed and tested it
myself.
Note on #1068: that change made floating times (no `Z`, no `TZID`) use
the configured timezone, which this PR keeps. It also routed
`Z`-suffixed UTC times through the same path; this PR only restores UTC
parsing for those.
Fixes#3871
The files row is inserted before the blob is written, and the local backend creates the file before copying into it, so a failed copy left a partial blob behind once the row was rolled back.
The vikunja-file import checks the user's stored files against
migration.vikunjafile.maxuserstorage. The upload is now one of those
files, so an import counted its own zip against the quota. Exclude files
referenced as a migration upload.
The upload was spooled to the receiving instance's temp dir and only its
name travelled with the event. That holds while events stay in-process,
but it ties the job to the instance that accepted the upload.
Store it through the files package on the configured backend and record
its id on the migration status, so the event carries only the status id
and any instance can run the import. The job copies it to a local temp
file first: the zip importer needs random access, which an S3 object body
cannot give.
The upload is removed once the import is over. An hourly cron deletes
leftovers from events lost on restart or instances that died mid-import.
The config was unmarshalled up front purely to produce a 400, then parsed
again by SetOptions, and a third time in the background job. Drop the
throwaway parse and give SetOptions a domain error that maps to 400 on
its own.
A file import ran inside the upload request. A big export takes minutes,
far longer than a reverse proxy holds a request open, so the client got a
502 while the import kept running and kept the user's migration slot, and
there was no way to cancel or retry it.
The upload request now only validates what is cheap to validate, claims
the slot, spools the upload into a per-process temp directory and queues a
FileMigrationRequestedEvent. FileMigrationListener runs the import through
the lifecycle it now shares with the OAuth migrations: stale event skip,
panic recovery, heartbeat, outcome and notifications. The vikunja-file
migrator rejects a non-zip or structurally broken export before queueing;
ticktick, wekan and csv would have to parse the whole upload, so they fail
in the job and report through the status and failure mail. The csv
migrator receives its column mapping as options carried with the event.
Split opening the zip and indexing its entries out of Migrate, so every
structural check that doesn't need entry contents runs in one place before
anything is imported. The missing VERSION check moves into that scan.
A finished migration_status row looked the same whether the import worked
or failed, so the client could only guess. Rows now carry error_kind, a
closed set the client translates (reported, interrupted, credentials,
queue, upload, detail), and error_message, set only for "detail": the
error caused by the user's own data, the same text the failure mail sends.
A failure we report to Sentry stores just the kind, so server internals
never reach the status.
The migration was already finished and its claim released when the done
notification failed, yet the error was returned and the listener then sent
a failure mail and overwrote the finished row. Log it instead. The failure
mail's own log line also printed the migration error instead of the
notification error.
The claim timeout was measured from started_at, so a long import either
kept a dead claim for a day (the old 24h default) or had its slot taken
over while still running. A running migration now refreshes heartbeat_at
at a tenth of the timeout, clamped to 1s..30s, and a claim only counts as
stale once its last beat is older than the timeout. Rows that never beat
fall back to started_at. The default timeout drops to 5m accordingly.
Also bind the stale cutoff in the configured timezone: xorm writes times in
the engine's zone but binds query args as-is, which broke the comparison on
SQLite.
A second import sent while the first claim was still being written could
fail the insert with whatever the driver reports under contention, which
surfaced as a 500. The claim now reads the running row before writing so a
losing request stays read-only, re-reads the live claim whenever the insert
fails, retries lock collisions a few times, and probes for a stale claim
before taking a write lock to release it.
Huma's default BodyReadTimeout is a 5s socket read deadline spanning the
whole body, so any upload slower than that is cut off and a reverse proxy
answers 502. Every multipart route now goes through withUploadLimits, which
sets the existing max body size and a finite 15 minute read bound.
SQLite and MySQL dumps store bools as 0/1. Since switching to pgx, restoring
such a dump into PostgreSQL fails with "unable to encode 0 into binary format
for bool".
SQLite dumps can contain empty strings in json columns (e.g. project_views.filter),
which PostgreSQL rejects with "invalid input syntax for type json".
Fixes#3895
Registered after apiv2.RegisterAll so the tool index sees the AutoPatch
operations. Routes under the prefix skip the token route check: the
endpoint checks mcp:access itself and every loopback call is checked on
its own route.
mcp.New builds the tool index from the v2 document and serves a stateless
per-request server: JWTs are rejected, mcp:access is required, tools/list is
filtered by the caller's token, and cross-origin browser requests are
refused unless the CORS configuration trusts the origin.
Frederick [Bot]
kolaenteandTink
Anand HegdeandGitHub