GetUserFromClaims read the id claim straight into User.ID without checking the
token type. A link share JWT carries the raw, positive share id in that claim
and reaches every authenticated route, so the only thing preventing
impersonation was the incidental absence of a username claim in link share
tokens. Adding one would have reintroduced the confusion with a positive id,
bypassing the GetID negation entirely.
AuthTypeUser moves to pkg/user, which parses the claims and cannot import
pkg/modules/auth, so the value exists once.
Non-empty-title errors from task creation fell through silently and the
loop kept using a task with ID 0, attaching relations and labels to id
0. Return the error instead; empty titles keep being skipped.
Each prompt created its own bufio.Reader over os.Stdin. The first
reader buffers ahead, so with piped input the later readers hit EOF
because the remaining lines were already sitting in the first
reader's buffer.
Vikunja's own dumps only contain file entries, but manually repacked
dumps (e.g. with zip -r after editing the VERSION file) contain
directory entries like "database/", which failed database file name
validation and aborted the restore.
Fixes https://github.com/go-vikunja/vikunja/issues/3380
GetAvatar converted a caller-supplied size int64 directly to int via
int(size) for imaging.Resize, with no check that the value was
non-negative or within int's range. size traces back to an
unauthenticated query parameter; on 32-bit builds a large int64
would silently truncate/wrap when narrowed, and a negative value
would feed straight into the resize call. Reject out-of-range values
before the conversion instead of letting it wrap silently.
DetectCSVStructure, PreviewImport, and MigrateWithConfig each did
make([]byte, size) with size taken directly from the caller (an
uploaded file's declared size) with no upper bound, letting a
malicious or corrupt size value force an arbitrarily large
allocation. Cap it to the server's existing configured max upload
size before allocating.
Three Debug-level log calls in the token-exchange path logged the
raw response body/full oauth2.Token on error. The full token struct
has no String() override, so %v printed AccessToken/RefreshToken in
clear text; the raw response body could similarly carry sensitive
request/response context. Log the RFC 6749 error/error_description
fields (successful-exchange-but-missing-id_token path: the token
type) instead - the intended human-readable diagnostic.
sortParentsBeforeChildren marked a task as placed only after recursing
into its parent, so a parentId cycle in an uploaded TickTick export made
place() recurse forever. That is a Go stack overflow, a runtime fatal
error the recover middleware cannot catch, so a two-line CSV from any
authenticated user took down the whole process.
Track a tri-state per task and mark it before recursing, which breaks
the cycle. Acyclic input is unaffected.
avatar.gravatarbaseurl is operator-configurable, so the request destination was
never a fixed constant - pkg/utils/avatar.go already uses the SSRF-safe client
for the same job. The previous 5s timeout is kept as a context deadline so the
configured (30s by default) client timeout does not apply here.
doGet built its own http.Client, unlike its siblings in the same file which
already use utils.NewSSRFSafeHTTPClient(). The previous 10s timeout is kept as
a context deadline so the configured (30s by default) client timeout does not
apply here.
Tasks imported without a position are inserted in front of the lowest one by
halving it. That reaches the minimum spacing every few dozen inserts, which
recalculates every position in the view and makes a large import O(n²) - a
10k task TickTick export with an empty Order column never finished.
Numbering the tasks up front skips the halving entirely and keeps the order
the export was written in.
Closes#3297
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.
kolaente
Mauandkolaente
TinkandGitHub
karl Einzigerandkolaente
