Pull the credential/TOTP check, session deletion, user-token issuance and
OIDC callback flow out of the v1 echo handlers and into reusable helpers so
both /api/v1 and the upcoming /api/v2 share one implementation:
- auth.IssueUserToken + auth.WriteUserAuthCookies split the token/cookie
machinery from the echo response; NewUserAuthTokenResponse now wraps them.
- auth.SessionIDFromContext reads the sid claim for logout.
- shared.AuthenticateUserCredentials, shared.DeleteSession hold the login
and logout cores.
- openid.AuthenticateCallback holds the OIDC exchange/getOrCreate/TOTP/team
sync, returning the user; HandleCallback issues the token as before.
v1 behaviour is unchanged on the wire.
Split the HTTP plumbing from the business logic in the v1 project-background
download and Unsplash image proxy handlers so /api/v2 can reuse it without
duplicating it:
- LoadProjectBackgroundForDownload (background/handler) loads the bg file +
modtime and fires the Unsplash pingback; GetProjectBackground now calls it.
- WriteProjectBackground (web/files) writes v1's exact background wire shape
(image/jpg, no-cache, stat-modtime Last-Modified, If-Modified-Since 304).
- FetchUnsplashImageByID / FetchUnsplashThumbByID (background/unsplash) return
the open upstream body for the caller to stream; the v1 proxy handlers now
call them. A typed ErrUnsplashImageDoesNotExist maps to 404 on both APIs.
- ErrProjectHasNoBackground (models) gives the no-background case a domain
error; v1 keeps its verbatim 404 message.
v1 responses are unchanged on the wire.
Pull the StartMigration -> Migrate -> FinishMigration orchestration out of
the v1 echo handlers into handler.RunFileMigration and csv.RunMigration so
the v2 API can reuse the exact same business logic. v1 is refactored onto
them and stays byte-identical on the wire.
Also tag the CSV detect/preview/config DTOs with doc:/enum: so they carry
descriptions in the v2 OpenAPI schema (ignored by v1 swaggo/xorm).
Extract the MIME validation, file storage and project reload from the v1
UploadBackground handler into ValidateAndSaveBackgroundUpload so the upcoming
v2 handler can reuse it instead of duplicating the logic. The v1 handler keeps
its exact wire behaviour; the inline "not an image" check now returns a typed
ErrFileIsNoImage that the handler maps to the same message.
The new v2 OAuth token endpoint mints a fresh session without going
through NewUserAuthTokenResponse, so those logins were missing from the
audit trail. The refresh grant stays unaudited like the v1 refresh.
The register handler, local/LDAP login and the OIDC callback all queue
the user.created event via DispatchOnCommit but never called
DispatchPending, so the event was silently dropped and its queue entry
leaked. Flush after commit and discard on rollback.
Every DispatchPending caller either has the request context in scope or
is genuinely request-less, so passing it as a parameter replaces the
stored-context mechanism on the pending queue and satisfies
contextcheck. Also fixes lint findings in the audit package.
LoginSucceededEvent fires from NewUserAuthTokenResponse (the chokepoint
where local, LDAP and OIDC logins converge), LoginFailedEvent from
handleFailedPassword on every failed password check, LogoutEvent from
the logout handler, and APIToken issued/revoked/used events from the
token model and auth middleware. The token events carry IDs only since
the freshly created token struct holds the raw token string and the
poison queue logs message payloads.
None of these events have a listener yet — the audit registration adds
them. Dispatching to a topic without subscribers is a no-op.
Pull the HTTP-independent core out of the v1 auth handlers so both
/api/v1 and the upcoming /api/v2 routes share one implementation:
- oauth2server: ExchangeToken and Authorize take plain inputs and return
typed responses; HandleToken/HandleAuthorize keep binding + headers.
- pkg/routes/api/shared: AuthenticateLinkShare, RegisterUser,
ResetPassword (+ session clear), RequestPasswordResetToken and
ConfirmEmail, plus the shared UserRegister and LinkShareToken types.
v1 handlers now delegate to these; their wire output is unchanged.
Port to /api/v2:
- DELETE /projects/{project}/background (remove background, returns the updated project)
- GET /backgrounds/unsplash/search (q, page; gated on the unsplash provider)
- PUT /projects/{project}/backgrounds/unsplash (set, gated on the unsplash provider)
Custom routes load the project and enforce CanUpdate explicitly. Backgrounds
are gated on the static backgrounds config via a registrar early-return.
Tag background.Image fields with doc: for the v2 schema, and add a scoped
contextcheck exclusion since the unsplash provider's shared interface bottoms
out in context.Background().
Add /api/v2 auth/status/migrate endpoints for the three OAuth-based
migrators. One generic helper registers all three ops per migrator
behind its static config gate, so there's no copy-pasted block per
migrator.
The migrate kick-off orchestration (already-running guard + event
dispatch) is extracted into migrationHandler.StartMigration so v1 and
v2 share it; v1's wire output is unchanged. The guard now surfaces as a
typed migration.ErrMigrationAlreadyRunning (412) so v2 can translate it
through the standard error bridge.
Extract the duplicated user-lookup, provider-selection and size-clamping
logic from the v1 GetAvatar and v2 avatarGet handlers into a single
avatar.GetAvatarForUsername helper. Both handlers now call it and keep
only their transport-specific code (v1: echo size parse + c.Blob, v2:
huma input/response). Pure refactor, behavior is unchanged.
Collapsing unparseable taskIds to 0 meant sortParentsBeforeChildren,
which tracked placement by TaskID, treated every zero-id task after the
first as already placed and silently dropped it. Track placement by task
identity instead so duplicate or zero ids never conflate distinct tasks.
TickTick exports could contain non-numeric values in columns Vikunja
parses as integers (Priority, taskId, parentId). gocsv's strconv.ParseInt
then failed, aborting the entire import and surfacing as an internal
server error reported to Sentry (e.g. parsing "p1": invalid syntax).
Numeric ID columns now fall back to 0 for unparseable values instead of
failing the import. The Priority column, which was previously parsed but
never carried over to the imported task, is now mapped onto the task and
accepts both the plain numeric form (0, 1, 3, 5) and the "pN" form
(p1, p2, p3).
Closes#2822
A GetWithValue deserialization error in RememberFor was returned as fatal.
On a Redis upgrade the metrics counters live under the same keys as before
but were stored as plain int64, so the first decode into the new envelope
would fail and the metric would break permanently. Treat such errors as a
miss and recompute/overwrite so the cache self-heals.
Seed the dedup map at the start of insertFromStructure with the importing
user's existing labels, keyed by title + normalized hex color. Previously
the map was empty on each run, so importing the same CSV (or any other
migration format) twice would create a second copy of every label.
Scoped to the user's own labels so imports don't silently link to other
users' labels visible via shared projects.
Fixes#2742
Hardcoding the three exact strings localhost / 127.0.0.1 / ::1 rejected
legitimate loopback redirects like 127.0.0.2:1234 (anywhere in 127.0.0.0/8)
or [0:0:0:0:0:0:0:1]:1234 (expanded IPv6 loopback). Use net.IP.IsLoopback()
to cover the full loopback ranges, and match "localhost" case-insensitively.
0.0.0.0 stays rejected as it is not a loopback address.
https://claude.ai/code/session_01LsTDrCJ7trE6WQ4FYf78UB
Previously the OAuth server rejected every redirect_uri that did not start
with a vikunja- custom scheme. Native apps that cannot register a custom
scheme (e.g. CLIs, desktop tools) need loopback redirects per RFC 8252, so
also allow http://localhost, http://127.0.0.1 and http://[::1] (any port).
Non-loopback http:// and https:// targets remain rejected.
https://claude.ai/code/session_01LsTDrCJ7trE6WQ4FYf78UB
Bot users now render with a cool-toned (blue/cyan/violet/teal/indigo)
marble variant so they're visually distinguishable from human users.
Marble's rendering logic is parameterized with a palette; the route
forces the bot palette whenever the resolved user is a bot, overriding
whatever avatar provider they'd otherwise inherit.
Parse the top-level `attachments` array in WeKan board JSON exports,
group them by card ID, base64-decode the payload, and attach the
resulting files to the generated tasks so they land in Vikunja as
task attachments. Orphaned attachments (cardId with no matching card)
are silently skipped; decode errors are logged and skipped.
The four boolean OIDC provider fields (emailfallback, usernamefallback,
forceuserinfo, requireavailability) were parsed with a strict .(bool)
type assertion. That works for YAML/JSON config where leaves are native
bools, but fails for every other input path: env vars always arrive as
strings, and GetConfigValueFromFile (used by the *.file Docker secret
convention) also always returns strings. The assertion would silently
zero the field for emailfallback and usernamefallback, and log an error
and zero the field for forceuserinfo and requireavailability, which is
what #2599 reports.
Extract a small parseBoolField helper that accepts both native bools and
strings (via strconv.ParseBool) and logs a parse error from each call
site. This also fixes the previously-silent drop of stringified
emailfallback / usernamefallback values — those now log an error if the
input is garbage, matching the behaviour of the other two fields.
Fixes#2599
Regression test for #2599. Exercises getProviderFromMap with native
bools and with stringified booleans ("true"/"false"/"1"/"0") for all
four boolean provider fields — emailfallback, usernamefallback,
forceuserinfo, requireavailability. From env vars and from the
GetConfigValueFromFile path every leaf arrives as a string, so the
current .(bool) assertion silently zeros these fields.
The OIDC callback handler previously issued a JWT without ever
checking TOTP state. For installations with EmailFallback (or
UsernameFallback) enabled, this allowed an attacker who could
authenticate at the IdP with a matching email to log in as a local
user with TOTP enrolled, bypassing the second factor entirely.
HandleCallback now runs enforceTOTPIfRequired after resolving the
user and before any team sync writes, returning 412/1017 when the
passcode is missing or invalid. Clients resubmit the OIDC flow with
the totp_passcode field populated.
Fixes GHSA-8jvc-mcx6-r4cg
Extracts a TOTP gate that the OIDC callback will use to enforce 2FA
for users with TOTP enabled. Mirrors the local-login TOTP flow in
pkg/routes/api/v1/login.go. Not yet wired into HandleCallback.
Refs GHSA-8jvc-mcx6-r4cg
Covers the four states the OIDC TOTP gate must handle: user without
TOTP, TOTP enabled with missing passcode, invalid passcode, and
valid passcode. The helper function under test does not exist yet,
so the package currently fails to compile.
Refs GHSA-8jvc-mcx6-r4cg
Prepares the OIDC callback struct to carry a TOTP passcode so the
handler can enforce 2FA for users with TOTP enabled. No behaviour
change yet.
Refs GHSA-8jvc-mcx6-r4cg
The test previously fetched the attachment from https://vikunja.io/testimage.jpg,
which caused flaky failures in CI when the external host was unreachable
(context deadline exceeded). Serve the local testimage.jpg via httptest and
temporarily allow non-routable IPs for the SSRF-safe client so the test is
hermetic and deterministic.
Builds an in-memory export zip with a 2 MB payload and a data.json
that claims size: 0, then asserts neither the honest 2 MB row nor
the forged 0-size row ends up in the files table. Covers
GHSA-qh78-rvg3-cv54.
The hard-coded 500 MB per-entry cap meant operators who set a tighter
files.maxsize could not actually enforce it on imports. Derive the cap
from files.maxsize with a floor so data.json / filters.json / VERSION
entries can still be read when the configured limit is tiny.
Clamp the uint64->int64 conversion and the LimitReader cap so absurd
configuration values do not overflow into MinInt64 and cause
io.LimitReader to treat every entry as EOF.
Import metadata is attacker-controlled and can forge a small size to
bypass the attachment size limit (GHSA-qh78-rvg3-cv54). Compute the
size from the decoded content instead of trusting a.File.Size.
Task/project duplication and the Todoist migration were passing stored
or API-reported sizes into NewAttachment. Derive the size from the
actual buffered content so every caller matches the hardened boundary
behaviour (GHSA-qh78-rvg3-cv54 defence-in-depth).
Task titles, project titles, team names, doer/assignee names, and API
token titles were interpolated raw into Line(...) calls whose content is
rendered to HTML by goldmark and then sanitized with bluemonday UGCPolicy.
UGCPolicy intentionally allows safe <a href> and <img src> with
http/https URLs, so a title containing Markdown link or image syntax
would survive sanitization as a working phishing link or tracking pixel
in a legitimate Vikunja email.
Introduce notifications.EscapeMarkdown, which prefixes every CommonMark
§2.4 backslash-escapable ASCII punctuation character — including '<' so
autolinks like `<https://evil.com>` are neutralized before reaching
goldmark — with a backslash. Apply it to every user-controlled argument
of every Line(...) call in pkg/models that feeds into an i18n template,
and to the hand-built "* [title](url) (project)" Markdown link in the
overdue-tasks digest notification.
Also escape the migration error string in MigrationFailedNotification,
an additional sink not listed in the advisory (error messages can carry
user-controlled content from the external migration source).
Subject(...), Greeting(...), and CreateConversationalHeader(...) are
left unchanged: Subject is passed directly to the mail library and is
not markdown-rendered, Greeting is rendered via html/template's built-in
HTML escaping without markdown, and the conversational header is
sanitized as raw HTML by bluemonday in mail_render.go.
Fixes GHSA-45q4-x4r9-8fqj.
Previously GetLinkShareFromClaims built a *LinkSharing entirely from JWT
claims with no DB interaction, so deleted shares and permission downgrades
took up to 72h (the JWT TTL) to take effect. The permission and sharedByID
claims were trusted blindly.
GetLinkShareFromClaims now takes an *xorm.Session, looks up the share via
GetLinkShareByID, verifies the hash claim against the DB row, and returns
ErrLinkShareTokenInvalid when the row is missing or the hash mismatches.
The permission and sharedByID claims are discarded; the DB row is
authoritative. GetAuthFromClaims opens a read session for the link-share
branch, mirroring the existing API-token branch.
Token creation and the JWT format are unchanged, so already-issued tokens
keep working except when the underlying share has been deleted or its hash
no longer matches.
Fixes GHSA-96q5-xm3p-7m84 / CVE-2026-35594.
Tink
kolaente
MidoriKurage
Claude
Rhys McNeill
