Relates to #985 — does NOT close it.
Adds registryMigrationRequired to the update-check payload (stable channel below
3.45.0) and an amber block in UpdateNotification explaining that the retired
registry path still responds, so `docker compose pull` appears to succeed while
serving the same frozen build.
Known limitation, established in review and merged deliberately: this cannot
reach the operators #985 describes. PicPeak is self-hosted, so the update-check
code runs inside the operator's own image — a v3.44.0 install runs v3.44.0's
backend forever, and the only external call returns release metadata, not logic.
Every build containing this predicate is >= 3.45.0, where it is false by
definition. The release-notes fallback fails too: the changelog modal shipped
2026-05-29, two days after the freeze.
Correct for any future rename, no runtime cost, but #985 stays open — the
population it describes still has no in-app channel. Viable routes are external
(retired GHCR package description, repo README, docs).
'0.0.0' is excluded from the predicate: that is getCurrentVersion's fallback for
an unreadable package.json, i.e. a broken install, not a pre-rename one.
Closes#983.
The two cross-add counter queries added in #979 were enabled on customers.edit,
but neither endpoint checks that permission:
HoursSection -> GET /expenses/inbound/by-customer/:id needs accounting.view
CustomerCrmPanels -> GET /customers/:id/hour-entries needs customers.view
An admin holding customers.edit but not the corresponding read permission fired
a guaranteed 403 on every customer-detail render. It degraded safely — the count
stayed at its 0 default so the cross-add was never offered, which is the right
outcome for that role — so this was request noise rather than broken behaviour.
Each guard now requires both: the read permission to fetch the count, and the
write permission because there is no point offering the cross-add to someone who
cannot create the combined invoice.
No seeded role is affected: migration 123 grants accounting.view and
accounting.manage together, and customers.edit projects forward from
customers.create, which migration 090 always grants alongside customers.view.
Closes#866.
Three features, all behind the `incomingInvoices` feature flag:
1. Attach the stored supplier proof PDF to the client-invoice email when a
captured invoice is re-billed/passed through, as a SEPARATE attachment so
invoice immutability holds. Global default (off), per-customer tri-state
override, and per-file selection in a new Send dialog. A missing proof at
issue time stamps inbound_documents.proof_attach_error rather than silently
dropping, and never blocks the send. Proof filename is a configurable
template with {INVOICE} {SUPPLIER} {YEAR} {MONTH} {SEQ}/{SEQ:0Nd} tokens.
2. Re-bills & passthrough panel under CRM → Customer, grouped Open/Sent/Paid
with status derived from the linked invoice lifecycle rather than a
duplicated column.
3. Cross-add dialog rolling open hours and open re-bills into one invoice,
symmetric from both entry points. The two stay distinct, contiguous line
groups — never merged into shared line items.
Migration 169 is additive, hasColumn-guarded and idempotent.
Review (two rounds) closed two concerns:
- Storno stranding: nothing cleared inbound_documents.billed_invoice_id when a
covering invoice was cancelled, so a Storno'd re-bill showed as Open in the
new panel while every billing path filters on that column being NULL — the
supplier cost could never be re-billed. releaseRebillsForCancelledInvoice now
detaches the linkage on both invoice-cancel paths, with a regression test on
the issued-cancel path.
- Permission gating: the new controls rendered on data presence alone while
their endpoints require accounting.view / accounting.manage / customers.edit.
Now gated at both the query and render layers.
Known follow-up: two cross-add counter queries are gated on a permission their
endpoint does not check (HoursSection.tsx:174, CustomerCrmPanels.tsx:270) —
degrades safely, one line each.
Closes#969.
The cockpit's email feed rendered preview/resend/cancel/retry/send-now for every mail, consulting neither the caller's role nor their permissions, producing controls that always failed:
404 - requireOwnedQueuedEmail scopes queued mail through email_queue.event_id AND ownership of that event. CRM document mail carries no event_id; and project ownership does not imply event ownership, so a project the caller owns can hold another admin's event.
403 - preview needs events.view but the four write actions need email.send.
getProjectOverview now stamps each email with an authoritative canAct, mirroring filterOwnedEventIds; created_by is selected only for that check and stripped before the response. The cockpit reads canAct and combines it with email.send. A missing canAct reads as false.
Regression from the GHSA-93x4 fix in #960/#966, which added the ownership middleware.
* fix(security): bound inbound-mail resources, redact secrets from logs (GHSA-2qf9, pgmp, r794)
GHSA-2qf9 — emailIntakeService downloaded, parsed and persisted every message
with no size, attachment-count or attachment-byte limit, reachable
unauthenticated by anyone who can email the operator's mailbox:
- fetch the envelope with `size` (same cheap pass) and refuse an oversized
message BEFORE downloading its source;
- cap attachment count and cumulative attachment bytes;
- limits env-overridable, defaults generous for real supplier invoices.
The teeth were in the dedup key. received_emails.message_id is varchar(512)
UNIQUE, and the failure path wrote `err-<uid>-<Date.now()>`, which can never
match the envelope-derived messageId the dedup pass compares against — so an
oversized (or overlong-Message-ID) mail was re-downloaded every poll forever,
and an OOM-kill/restart just resumed the loop. Size-skips are now recorded
under the REAL message id, and overlong ids collapse to a stable sha256 key
that always fits the column.
GHSA-pgmp / r794 — new sanitizeForLog() util (key-name deny-set, recursive,
cycle-safe) applied to the three request-body log sites in adminEvents/crud.js,
plus sanitizeValidationErrors() because express-validator's errors.array()
embeds the SUBMITTED value per field — a rejected plaintext password was still
logged. Scope is wider than filed: the update path also logged
client_password_hash and a LIVE client_share_token bearer credential.
Also: the one-time setup token was logged at warn AND printed to stdout on
every first boot, putting a live first-admin credential in combined.log,
security.log and `docker logs`. It is now written to the 0600 token file and
only surfaced when that write fails — the last-resort path it existed for.
* fix(security): codex round 3 — repair the first-run token recovery flow (GHSA-r794)
Two regressions from keeping the setup token out of the logs.
1. server.js decided whether to print the token by calling existsSync() on the
candidate path. That answers a different question than "did the write
succeed": a stale, read-only or directory-shaped SETUP_TOKEN reports as
present, so the banner suppressed the live token and pointed the operator at
content that is not it — leaving the current token only in combined.log
under default production logging. setupService now records the path the
write actually produced and exposes it via writtenSetupTokenFile().
2. The setup screen, its EN/DE strings, README, SIMPLE_SETUP and .env.example
all still told first-time users to run
`docker compose logs backend | grep -i "setup token"`. On the normal path
that command now returns a path banner and no credential, so the documented
browser-first onboarding could not be completed. They now point at
`docker compose exec backend cat /app/data/SETUP_TOKEN`, with the log
fallback described as what it is — the failure path.
Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me
---------
Co-authored-by: Paul Nothaft <[email protected]>
* fix(security): redact gallery share tokens from analytics page-view tracking (GHSA-7m6c)
* fix(security): codex round-1 — actually disable raw auto-tracking (GHSA-7m6c)
The previous patch was inert: App.tsx passed autoTrack:true (so Umami's
data-auto-track=false was never set) and the sanitized trackPageView had no
caller (useAnalytics sits outside <Router>), so the raw token URL still hit
the collector.
- Umami: drop autoTrack:true → data-auto-track=false; page views now come
from a sanitized manual tracker.
- Rybbit: its initial-load auto pageview can't be intercepted client-side, so
use native data-mask-patterns=['/gallery/**'] to strip the token on every
auto-tracked view; skip manual tracking for it to avoid double counting.
- Mount <AnalyticsRouteTracker/> INSIDE <Router> so manual tracking runs.
---------
Co-authored-by: Paul Nothaft <[email protected]>
* feat(gallery): multi-select feedback filters + sort direction controls (#889)
* fix(gallery): keep mobile sidebar open while combining feedback filters (#889)
* fix(gallery): generic sort icon when direction is uncontrolled (#889)
---------
Co-authored-by: Paul Nothaft <[email protected]>
* feat(gallery): per-event toggle to hide the logo on the password page (#894)
* fix(admin): harden login_logo_visible coercion for SQLite + string booleans (#894)
---------
Co-authored-by: Paul Nothaft <[email protected]>
* fix(security): close two access-control advisories (GHSA-g94x, GHSA-pv6w)
GHSA-g94x-8vv8-3c9f (HIGH) — the secure-image VIEW route
(/secure-images/:slug/secure/:photoId/:token) validated only the token
signature and took the gallery/photo from the URL, so a token minted on
any PUBLIC gallery read every other gallery's photos with no password
(its download sibling has verifyGalleryAccess; the view route can't —
it serves via <img src> with no header). Bind the token to its scope
instead: the URL photoId must equal the token's minted photoId (photos
belong to exactly one gallery, and minting is gallery-scoped), and the
gallery embedded in the token's sessionId must equal the URL gallery.
GHSA-pv6w-rj34-wj9v (MEDIUM) — GET /admin/backup/picpeak/export dumps
every table unredacted (bcrypt hashes, 2FA, SMTP/SSO/WhatsApp/webhook/S3
secrets) and was gated only by backup.create, which the built-in admin
role holds. Gate it behind super_admin, matching the restore side
(backup.restore, already admin-denied) and the masked config APIs.
Regression tests pin both: cross-gallery token reads 403 (photo and
gallery checks), backup export 403 for admin / passes for super_admin.
* test: stub requireSuperAdmin in the backup masking mock
adminBackup now calls requireSuperAdmin() at load (GHSA-pv6w export
gate), and backupSecretMasking mocks the permissions module — add the
new function to the mock so the module loads.
* fix(security): review follow-ups on the export gate (GHSA-pv6w)
- test: place the mocked export in its own mkdtemp dir. The route
recursively deletes path.dirname(filePath) after download, so a stub
in bare os.tmpdir() made the super_admin test wipe the whole temp
root — other jest workers' DB files included (latent CI flake).
- ui: hide PicpeakExportCard from non-super_admins. The role keeps
settings.view + backup.create, so after the gate its Download button
always 403'd with a generic toast; gate the card on role super_admin
to match the endpoint.
* fix(security): keep the token-mismatch audit values within varchar(20) (GHSA-g94x review)
image_access_logs.access_type is varchar(20) (migration 038), but
'token_gallery_mismatch' is 22 chars — on Postgres the audit write
threw value-too-long and logImageAccess swallowed it, so the security
event went unrecorded (the 403 still fired; log is best-effort). Shorten
to 'photo_mismatch' / 'gallery_mismatch' (14/16).
---------
Co-authored-by: Paul Nothaft <[email protected]>
* fix(admin): own-property lookup in the extension MIME map (#908 review round)
A client-controlled filename ending in .constructor / .__proto__ /
.toString made EXTENSION_TO_MIME[ext] return an inherited Object.prototype
member (truthy), and the downstream extMime.startsWith threw —
a permanent 500 on the admin view for that photo instead of the JPEG /
mp4 fallback. hasOwnProperty-gated now; test pins both a .constructor
image and a .__proto__ video.
* fix(admin): drop already-expired events from the dashboard card (#909 review round)
The expiring-soon card ran Math.max(1, ceil(delta)), so an event that
expired while the dashboard sat open (its query isn't polled) showed
'1 day left' indefinitely from the stale cached row. Expired rows are
now filtered out before render; the delta is therefore always positive
and the clamp is gone.
* fix(admin): honor safe stored image MIME for auto-imported formats (#908 review round 2)
My previous round made the image side map-only to dodge the migration
039 image/jpeg backfill and image/svg+xml — but that regressed the S3
auto-importer (STORAGE_AUTO_IMPORT), which stores correct types for
avif/bmp/tiff/heic whose extensions aren't in EXTENSION_TO_MIME. Those
now served as image/jpeg (JPEG-labelled non-JPEG bytes).
Precedence is now mapped-extension (still corrects the 039 backfill on
PNGs) -> stored MIME IF in a safe raster allowlist (avif/bmp/tiff/heic
+ the mapped ones) -> image/jpeg. Allowlist, not a regex: image/svg+xml
stays excluded (scriptable inline). Tests pin avif preserved and svg
degraded to jpeg.
* fix(admin): refresh expiry status live at the boundary (#909 review round 2)
Two review findings on the admin expiry surfaces:
- The dashboard 'expiring soon' card, list badges, and detail banner are
all computed inline from Date.now() at render, so a page left open
across an event's expiry kept showing 'active'/'1 day left' until an
unrelated render — which for editor/viewer roles (no health poll)
never happens.
- My round-1 client-side filter on the dashboard desynced the visible
list from the cached total/stat ('no events expiring' beside 'view
all N').
Both are fixed by new useExpiryRefresh: it fires once at the soonest
future expiry (setTimeout, overflow-guarded). The dashboard refetches
its expiring + stats queries — the backend already excludes expired
events, so rows/total/stats come back consistent (filter removed). The
list and detail pages bump a tick so the inline badges recompute. Hooks
are placed above the loading early-returns (rules-of-hooks is disabled
in eslint, so this was a latent crash otherwise).
* fix(admin): allow any header-safe raster MIME, deny svg/xml (#908 review round 3)
The round-2 hand-listed Set kept missing formats the S3 auto-importer
stores (apng/ico/jxl beyond avif/bmp/tiff). Replace it with a regex:
honor image/<token> EXCEPT the scriptable svg / *+xml family. Covers
every current and future raster type in one rule while still blocking
inline-scriptable svg and header injection. Tests pin apng + x-icon
preserved, svg still degraded to jpeg.
* fix(admin): expiry-refresh precision + filtered refetch (#909 review round 3)
Three refinements to round-2's live-expiry work:
- useExpiryRefresh now re-arms past setTimeout's ~24.8-day overflow
limit (capped wake-up that re-evaluates) instead of dropping the timer,
so a page mounted for weeks still updates.
- The dashboard requests the expiring list ordered by expires_at asc, so
the five shown rows ARE the soonest to expire — the timer schedules
against the true next boundary even when >5 events are expiring
(getEvents gains optional sortBy/sortOrder; backend already whitelists
expires_at).
- EventsListPage refetches instead of only re-rendering at the boundary:
under the 'expiring' filter the backend drops expired rows, so a plain
tick would leave a stale 'Expired' row + total. refetch keeps rows and
totals correct under every filter.
---------
Co-authored-by: Paul Nothaft <[email protected]>
differenceInDays truncates to whole days, so an event expiring in a few
hours returned 0 and three admin surfaces treated it as gone:
- EventsListPage: status chip said 'Expired' (days <= 0) while the
public gallery — which compares real timestamps — correctly showed
'expires in X hours'. This is the reporter's exact symptom.
- EventDetailsPage: same isExpired math on the detail view.
- AdminDashboard: the expiring-soon card showed '0 days left' on the
final day.
Expired is now gated on the actual timestamp (expires_at <= now), and
the countdown chips use ceiling days so the last day reads '1 day
left' instead of flipping to Expired/0.
Co-authored-by: Paul Nothaft <[email protected]>
* fix(analytics): make per-photo view/download counters actually count (#895)
Three stacked defects behind 'per-image stats stay 0':
- photos.view_count had NO writer anywhere — the admin IMAGES table and
photo viewer display it, so it was permanently 0. It now increments
when the full-size photo or its preview tier is served, excluding the
slideshow kiosk (migration 138 design) and follow-up video Range
requests (seeks are not views). Fire-and-forget so analytics can
never fail the byte-serving path.
- Zip downloads (download-all, presigned download-all,
download-selected) never incremented per-photo download_count — only
single-photo downloads did, so zip-heavy galleries showed 0 forever.
The zip routes now bump exactly the photos that went into the archive
(the prebuilt-zip path mirrors the archive builders' category filter).
- Every admin surface used a different definition of 'downloads', which
is the reporter's 46 vs 45 vs 0: event details counted only
action='download' (no zips at all), the dashboard counted
download+download_all but silently EXCLUDED download_selected and
download_all_presigned. All queries now share one action set:
download, download_all, download_all_presigned, download_selected.
New photoEngagementCounters suite pins all of it (7 tests).
* fix(analytics): count views via an explicit lightbox beacon (#895 review round)
External review flagged that request-level view counting is wrong in
both directions: the lightbox preloads prev/next neighbours (3 fetches
per open) while a preloaded neighbour promoted by a swipe is never
re-fetched (#505 keeps the DOM node), and enhanced/maximum galleries
never hit /photo at all (bytes come from /api/secure-images).
- Views now count via POST /:slug/photo/:photoId/view, fired by the
lightbox exactly when a photo becomes the visible slide; the
serving-route increments are removed. Covers protected galleries and
the preview tier uniformly; slideshow kiosk stays excluded.
- bumpEventDownloadCounts mirrors downloadZipService._build (ALL event
photos) — the category filter mismatched the prebuilt zip's actual
contents. (That the builder ignores per-category allow_downloads is a
separate pre-existing issue.)
- Zip loops count only successfully appended entries, with a pre-append
storage stat: a lazy stream's async error bypassed the per-photo
catch and hung the whole response — pre-existing bug, now fixed.
Suite extended to 9 tests (beacon semantics, serve-does-not-count,
skipped-entry exclusion).
* fix(analytics): fire the view beacon from the premium lightbox too (#895 review round 2)
gallery-premium events use yet-another-react-lightbox inside
GalleryPremiumLayout instead of PhotoLightbox, so the layout never
counted views. yarl's on.view fires on open and on every slide change —
identical semantics to the PhotoLightbox beacon.
Also documents the accepted prebuilt-zip approximation: _build can skip
entries whose watermark step fails and still publish the archive;
counting those exactly would need a persisted zip manifest.
* perf(analytics): skip the per-entry zip preflight on S3 (#895 review round 3)
The pre-append source check exists for LocalFs's lazy createReadStream
(async error would kill the whole zip response). S3's get() awaits
GetObject and rejects inside the loop's try/catch on a missing key, so
a HEAD per entry was a redundant serial round trip — 500 extra HEADs
on a 500-photo zip.
---------
Co-authored-by: Paul Nothaft <[email protected]>
* feat(feedback): let guests remove their star rating (#884)
Clicking your current rating again clears it. rating: 0 is the wire
contract: the validator now accepts 0, and the service deletes the
guest's rating row (instead of storing a 0 that would drag the photo
average down) and recalculates photo stats. The lightbox stars send 0
on a same-star click; PhotoRating already did, but the backend rejected
it with a 400 until now.
* fix(feedback): harden the rating-clear path (#884 review round)
External review follow-ups: numerically normalize the clear sentinel so
a numeric-string "0" can't slip into the update/insert paths (validator
now also toInt()s), delete the full guest-scoped rating set on clear so
racy duplicate rows can't survive in the average (same defense as the
reaction path), and refresh the visible average/count after the
identity-modal submit path like the direct paths do.
* fix(feedback): round-2 review fixes for rating clear (#884)
- Clear sentinel matches only an explicit 0 / "0" — malformed input
(undefined, NaN, garbage strings) can no longer delete a rating.
- Lightbox survives the photo list shrinking while open (clearing your
rating under the Rated filter drops the photo on refetch): index is
re-anchored and the lightbox closes when the list empties, instead of
crashing on an out-of-range index.
- Story layout gets the same same-star-to-clear behavior, keyed off the
session-local my-rating map, and an explicit 0 no longer falls back to
displaying the photo average.
* fix(feedback): refresh guest-scoped caches after rating changes (#884 review round 3)
- GalleryView's onFeedbackChange now also invalidates ['my-feedback',
slug]: in guest identity mode the Rated/Liked filter membership and
chip counts come from that query (#538), so a cleared rating never
left the Rated filter until the 30s staleTime lapsed.
- PhotoRating invalidates gallery-photos + my-feedback on success: the
parent refetch fires optimistically in onMutate and could capture
pre-mutation state, with nothing refreshing after the server accepted.
---------
Co-authored-by: Paul Nothaft <[email protected]>
The bottom info/action bar was a translucent gradient overlaying the
image, hiding the lower edge of the photo. The bar is now opaque and the
image area stops above it (measured via ResizeObserver, since the bar
height varies with flex-wrap, the optional filename line and safe-area
padding), so the photo is always fully visible.
Co-authored-by: Paul Nothaft <[email protected]>
Adds a fit-to-screen button next to the zoom controls (enabled while
zoomed) and double-click-to-reset on the image itself. Both snap the
photo back to 100% and re-centre it.
Co-authored-by: Paul Nothaft <[email protected]>
Clicking the black bars around the image (a missed arrow click) closed
the lightbox and dropped the guest back into the grid. The lightbox now
only closes via the X button or Escape, matching what gallery guests
expect while paging through photos.
Co-authored-by: Paul Nothaft <[email protected]>
In simple identity mode, likes and ratings submitted from the lightbox
never called onFeedbackChange, so the gallery's photo list (whose
like_count drives the Likes/Rated feedback filter chips) stayed stale
until a full page reload. Liked photos were missing from the Likes
filter; unliked photos stayed stuck in it.
The guest-identity-mode paths and the grid PhotoCard paths already call
onFeedbackChange after submitting - the simple-mode lightbox paths were
the only ones missing it. Add the call to the three missing paths:
submitLike (simple branch), submitRating (simple branch), and the
FeedbackIdentityModal onSubmit handler.
Verified locally (Docker build of main): like a photo in the lightbox
after navigating with Next/Prev, open the Likes filter - the photo now
appears immediately with no reload, and filter contents match the admin
feedback API exactly.
* fix(backup): make backup settings actually apply (#871)
- Wire the What-to-Backup toggles into the walker: honor
backup_include_thumbnails / backup_include_photos (opt-out,
default ON) and accept the UI's backup_include_archives spelling
for the archived gate (the engine expected _archived, so the
Archives checkbox silently never worked).
- Fix the 167.6 TB dashboard size: file_size_bytes is a bigint that
node-postgres returns as a string, and the S3 path concatenated it
onto the byte counter; coerce to Number at the source.
- Compute the real next scheduled run (cron-parser) and return it as
nextBackup; the UI read a field the API never sent and rendered a
hardcoded 'Not scheduled'. A named schedule label now beats the
stray default cron the UI always sent, which silently turned
weekly schedules into daily 03:00 runs.
- Never back up filesystem noise (.nfs* silly-renames, .DS_Store,
Thumbs.db) and honor backup_exclude_patterns in the walker
(previously rsync-only).
- Remove the compression/encryption toggles from the configuration
UI: no backend implementation exists, and collecting an encryption
passphrase while uploading plaintext is a false promise.
Closes#871
* fix(backup): close the review gaps in the settings wiring
- The UI's backup_include_archives now beats the migration-seeded
backup_include_archived: every install has the singular key seeded
true, so the alias-only-when-absent lookup made unchecking Archives
a no-op.
- rsync destinations now receive the de-selected What-to-Backup paths
and the noise filters as anchored --exclude args; previously rsync
synced the whole storage root and the walker's selection only shaped
the manifest, which then misreported what was actually transferred.
- Escape regex metacharacters in the walker's glob matcher: '.nfs*'
compiled to /^.nfs.*$/ whose leading dot matched any character, so
files like anfs-photo.jpg were silently dropped from backups.
- The Backup Coverage report now uses the same gate as the walker
(new 'skipped-by-setting' status) instead of re-implementing it
without the opt-out toggles and the archives alias.
* fix(backup): make the coverage diagnostics agree with the walker
- The coverage table shows the alias-aware flag value the gate actually
used, instead of the seeded backup_include_archived shadowed by the
UI's plural key (true next to a 'Gated off' badge).
- skipped-by-setting paths are now counted in the coverage summary
(backend, TS contract, summary card, EN/DE locales) so the totals
reconcile again when Photos or Thumbnails is unchecked.
- The form's thumbnail default now matches the backend's never-saved
fallback (include): the checkbox no longer shows 'off' while
thumbnails are being backed up, and saving an unrelated setting no
longer flips the backup scope.
* fix(backup): keep custom crons, exclude disabled rows from rsync, normalize flag display
- Saving a named schedule no longer wipes the stored custom cron: the
backend already prefers the label, so the cron field stays inert for
named schedules and is preserved for switching back to Custom. A
custom schedule now validates the 5-field expression before saving
(the backend silently fell back to daily 02:00 on a blank value).
- resolveExcludedBackupPaths now also returns rows disabled via
include_in_default, so rsync excludes them; the enabled-only loader
hid them and rsync transferred their contents anyway.
- The coverage table normalizes flag values like the walker does —
Boolean('false') displayed true beside a gated-off badge.
* feat(auth): OIDC logout-to-IdP — phase 3 (#798)
RP-initiated logout behind a new oidc_logout_from_idp setting: logging
out of PicPeak also ends the IdP session. The SSO callback stores the
raw ID token in an HttpOnly cookie (also the marker that the session
came in via SSO — local-password sessions never bounce to the IdP);
/logout builds the end_session URL from discovery metadata with
id_token_hint + post_logout_redirect_uri + client_id and returns it as
ssoLogoutUrl for the frontend to navigate to. Any failure (no
end_session_endpoint, IdP unreachable, feature off) degrades to the
plain local logout.
Settings surface exposes the toggle plus the computed post-logout
redirect URI to register at the IdP. Session timeouts deliberately stay
local-only.
6 integration tests over the mock IdP; live-verified against
Keycloak 26 (logout ends the Keycloak session, no confirmation prompt).
* fix(auth): harden the SSO logout marker cookie (#798 phase 3)
Codex review round 1:
- Derive the oidc_id_token cookie options from the shared cookie policy
(COOKIE_SAMESITE / COOKIE_DOMAIN / secure resolution) — hardcoded Lax
meant split-origin deployments running on SameSite=None never sent the
marker to the cross-site /logout XHR, silently disabling logout-to-IdP.
- Oversized ID tokens (>3.9KB) now store a bare 'sso' marker instead of
no cookie, so the claimed client_id-only end-session fallback actually
happens; /logout only passes the value as id_token_hint when it is a
real JWT.
- establishAdminSession clears any stale marker on every fresh login —
sessions can die without /logout (deactivation, expiry, restore), and
a surviving marker would bounce a later local-password session to the
IdP. The SSO callback re-sets the marker for its own session.
Tests: oversized-token marker + hint-less end-session URL, stale-marker
cleared on local login; helper updated for the clear+set cookie pair.
* fix(auth): validate the logout hint against the current OIDC config (#798 phase 3)
Codex review round 2: an ID token stored at login can outlive an
issuer/client config change; sending it to the newly configured IdP as
id_token_hint strands the user on the IdP's error page (providers
validate iss/aud on the hint). buildEndSessionUrl now decodes the hint
(no verification — routing only): different issuer → skip the round-trip
entirely (the session belongs to another IdP); same issuer but changed
client → keep the round-trip, drop the unusable hint. Two tests pin both
paths.
---------
Co-authored-by: Paul Nothaft <[email protected]>
* fix(gallery): block password form in Instagram in-app browser (#654)
Field reports show gallery password login still failing inside
Instagram's IAB after the #656 input-attribute/trim defenses. Three
changes:
- Replace the advisory amber banner with a red blocking state: the
password form is hidden in the Instagram IAB and replaced with
platform-specific "open in external browser" instructions plus a
copy-link button (clipboard API with execCommand fallback). A
"try anyway" link restores the form as an escape hatch.
- Stop masking non-password failures as "incorrect password": a request
that never got a response (offline, webview killed it) now reports a
connection error, and a reCAPTCHA 400 reports a verification failure —
both previously fell through to the wrong-password message and sent
guests chasing the wrong cause.
- Strip invisible Unicode (zero-width chars, word joiner, BOM, soft
hyphen) from the submitted password in addition to trimming — these
ride along when the password is copy-pasted out of a chat app and fail
byte-exact bcrypt compare server-side.
* fix(gallery): retry login with typed password + honor execCommand result (#654)
Codex review round 1:
- Stored passwords can legitimately contain the invisible code points the
sanitizer strips (e.g. ZWJ emoji sequences) — creation paths don't
normalize. On a 401 where the sanitized form differs from the typed
(trimmed) input, retry once with the typed value. Skipped when a
reCAPTCHA token is in play (single-use).
- document.execCommand('copy') signals failure via its return value, not
by throwing — only show "Link copied" when it returns true.
* fix(gallery): move invisible-char password fallback server-side (#654)
Codex review round 2: the client-side retry either burned the single-use
reCAPTCHA token (making exotic-but-valid passwords impossible to enter
with reCAPTCHA on) or burned failed-attempt lockout quota on every
rescued login. Doing the fallback as a second bcrypt compare inside the
same gallery/verify request eliminates both: exact bytes are compared
first (stored passwords containing e.g. ZWJ emoji keep working), the
sanitized form only on mismatch, and trackFailedAttempt only fires when
both fail. Frontend goes back to plain trim-on-submit; the client-side
sanitizer util and retry are removed. 7 integration tests pin the
contract.
---------
Co-authored-by: Paul Nothaft <[email protected]>
The audit #485 called for: on SQLite (native installs), timestamp
columns written with a raw `new Date()` through knex store epoch-ms
numbers; Postgres returns ISO strings. Frontend code written against
Postgres calls parseISO() on them — parseISO(number) throws and crashes
the page. #485 fixed admin Users and listed api tokens / photos /
activity as out-of-scope follow-ups.
Verified crash on main: Timeline gallery layout parseISO(uploaded_at)
against photos written by the archive-RESTORE path (raw Date). Other
raw-write surfaces (api_tokens last_used_at/revoked_at, email_queue)
degrade rather than crash but violate the ISO contract.
- extract toIso() from adminUsers.js into utils/dateNormalize.js
(contract unchanged — the 10 existing #485 tests still pin it)
- write-side: archive-restore uploaded_at, api-token last_used_at /
revoked_at, email_queue created_at/sent_at now write ISO strings
- read-side (heals existing corrupted rows): gallery /photos normalizes
uploaded_at/captured_at; api-tokens list normalizes all four
timestamp fields
- frontend defence-in-depth: Timeline layout parses uploaded_at
tolerantly (typeof guard) for stale caches / old backends
- 2 regression tests seed literal epoch numbers and assert the API
serves ISO strings
activity_logs turned out safe (created_at comes from the DB default,
not a raw Date) — left untouched.
Co-authored-by: Paul Nothaft <[email protected]>
Paul Nothaft