Commit graph

9 commits

Author SHA1 Message Date
ScreenTinker ba3e2cc785 fix(security): patch quick-win findings from the codebase review
Five low-risk, high-value fixes surfaced by the security review:

#3 Branding lockdown — `custom_domain`/`custom_css` (which feed the PUBLIC,
   pre-auth branding resolver and the login-page <style>) are now settable only
   by platform admins; a workspace_admin can no longer hijack the platform login
   page by claiming its domain. The public /api/branding (+ /domain) now return
   only presentational fields via publicBranding() (no id/user_id/workspace_id/
   custom_domain/timestamps leak).

#6 Strip device_token — the device WS auth secret (validated with
   timingSafeEqual) was returned in device list/get/update + pairing responses
   (SELECT d.* / *). New lib/device-sanitize.js strips it everywhere; prevents
   device impersonation by any workspace user.

#7 must_change_password enforced server-side — was a frontend-only redirect, so
   a provisioned temp password worked indefinitely via the API. requireAuth now
   403s every route except GET/PUT /api/auth/me (the password change, which
   clears the flag) and logout while the flag is set.

#8 XSS — escape user data interpolated into innerHTML in teams.js, kiosk.js,
   layout-editor.js (team/page/layout/zone names, member name/email, kiosk
   config fields). scriptSrcAttr 'unsafe-inline' made these exploitable via
   injected event handlers, not just markup.

#9 Thumbnail IDOR — /api/content/:id/thumbnail had no auth/scope gate (any UUID
   served any tenant's thumbnail). Now mirrors the /file route's playlist/widget
   workspace-scoped reference check.

Tests: new test/security-fixes.test.js (device strip, publicBranding field
allowlist, must_change_password gate). Full suite 41/41. Verified live against a
prod-data copy: device_token absent from /api/devices, /api/branding trimmed.

Not addressed here (tracked for follow-up): Android OTA signature verification
(Critical), public widget-render XSS, token revocation/logout, pairing-code
strength, validateRemoteUrl hardening, import quota.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-08 19:02:19 -05:00
ScreenTinker 6e31770cee feat(admin): admin-provisioned user creation + first-login gate (#10)
Adds POST /api/admin/users so an admin can create a user directly with a
known password and assign them to a workspace + role - for self-hosted
instances with no outbound email, where invites never deliver.

Server (routes/admin.js, mounted /api/admin with requireAuth + activityLogger):
- Gated by canAdminWorkspace(db, req.user, targetWorkspace): 404 if the
  workspace is missing, 403 if not an admin of it. This scopes org_admins
  to their own org and excludes platform_operator (no user/role mgmt, #13).
- Validates email (invite-create regex), role in WORKSPACE_ROLES, password
  min-8 (the /me rule). 409 on duplicate email - never overwrites.
- One transaction: global users row (auth_provider 'local',
  bcrypt.hashSync(pw,10), must_change_password from the flag) + a
  workspace_members row written inline (same footprint as an accepted
  invite; accept-invite left untouched).
- Explicit audit row admin_create_user; never logs the password; response
  excludes password/hash.
- HOSTED_INSTANCE: never calls sendSignupEmails and stamps both
  welcome_email_sent_at / activation_nudge_sent_at, so an admin-created
  user gets no welcome email and never enters the activation-nudge sweep.

must_change_password (frontend-first enforcement, per spec):
- Migration adds users.must_change_password INTEGER NOT NULL DEFAULT 0;
  surfaced via requireAuth + /me + login responses.
- route() in app.js forces users with the flag to a #/change-password
  screen (new force-password-change view, reuses PUT /api/auth/me) and
  blocks every other view until set. The /me update clears the flag.

Frontend: "Add User" button beside "Invite member" in the members view
(admin-only) opening a modal (email, name, password + generate, role,
must-change checkbox); invite and Add User coexist. api.adminCreateUser;
EN i18n only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 11:03:56 -05:00
ScreenTinker 48902f6807 feat(roles): add cross-org platform_operator staff role (#13)
platform_operator is cross-org STAFF: it can see and act-as into every
org and read/write workspace-scoped resources (content, playlists,
layouts, schedules, devices, widgets, kiosk) anywhere - but holds NO
owner-level power.

Design is deny-by-default: operator is NEVER added to PLATFORM_ROLES /
isPlatformRole, so every owner capability (billing, org/workspace
deletion, user/role management, shared & template asset curation,
branding, workspace member mgmt/rename) stays denied, and any NEW owner
endpoint added later inherits that denial automatically.

Operator gets power from exactly two levers:
- middleware/auth.js: new PLATFORM_STAFF set + isPlatformStaff(); owner
  guards (PLATFORM_ROLES, requireAdmin, requireSuperAdmin) unchanged.
- tenancy.js: accessContext + resolveTenancy treat staff as act-as
  capable; new req.isPlatformStaff / req.isPlatformOperator (req.isPlatformAdmin
  stays owner-only); accessibleWorkspaceIds + switch-workspace guard use staff.
- permissions.js: canRead/canWrite + canAccessWorkspace (read) grant staff;
  canAdmin / canAdminWorkspace / isOrgAdmin / isOrgOwner stay owner-gated.

Read-only edges (per review): operator may VIEW workspace member lists
(canAccessWorkspace) and the unassigned device pool (devices.js), but
cannot mutate either.

Frontend: platform role dropdown adds "Platform operator"; the user-mgmt
view stays isPlatformAdmin-gated so operators can't open it. EN i18n only.

Behaviour identical under HOSTED_INSTANCE set or unset.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 10:30:21 -05:00
ScreenTinker 797eab7c8d refactor(roles): normalize the platform-role model (#14)
The legacy /api/auth/users dropdown could write 'superadmin' and 'admin'
role strings that not every code path recognized. Some checks matched only
'platform_admin' (tenancy accessContext/resolveTenancy), so a 'superadmin'
user could list orgs but not act-as into them.

Normalize to the current two-tier platform model (users.role holds the
PLATFORM role only; org/workspace roles live in the membership tables):

- Migration (idempotent, exact-string): superadmin -> platform_admin,
  admin -> user. No-ops on rows already in the current model.
- Add isPlatformRole() helper in middleware/auth.js; route the two
  superadmin-excluding checks in tenancy.js through it so a stray
  'superadmin' is never treated as lower-privileged (fixes act-as).
- Remove the dead/stricter requirePlatformAdmin in permissions.js (bare
  === 'platform_admin'); the single guard is the one in middleware/auth.js.
- Recovery-token default role admin -> platform_admin so emergency
  recovery keeps full access once 'admin' no longer implies elevation.
- PUT /api/auth/users/:id/role whitelist -> ['user','platform_admin'];
  self-demote guard retargeted via isPlatformRole.
- Frontend: platform user-management dropdown now offers User / Platform
  admin only; owner-delete guard and settings highlight use isPlatformAdmin.
  EN i18n: add admin.role.platform_admin.

Behaviour is identical under HOSTED_INSTANCE set or unset; the migration
only touches exact legacy strings.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 09:58:46 -05:00
ScreenTinker c71c4016ca feat(email): Microsoft Graph send + alert spam protection + preferences UI
Replaces the unused EMAIL_WEBHOOK_URL stub with a real Microsoft Graph
Mail.Send pipeline via @azure/msal-node client-credentials flow. Prior
state on prod: every alert email was logged to journalctl and never
sent (21 fallback log lines per hour for the chronic-offline devices).

Four coordinated changes shipped as one commit since they're all part
of making email delivery actually work responsibly:

1. services/email.js (NEW): Graph send via plain HTTPS (no SDK), in-memory
   MSAL token cache (refresh 60s pre-expiry), graceful stdout fallback
   when GRAPH_* env vars absent. Drop-in replacement for the old webhook.

2. services/alerts.js refactored: sequential await around sendEmail (was
   parallel fire-and-forget; first run hit Graph's MailboxConcurrency 429
   ApplicationThrottled on a 30-device backlog). Sequential at ~250ms per
   send takes 5-8s for the full backlog, well within the 60s tick. Also:
   24h long-offline cutoff to stop nagging about chronic-offline devices
   (the 20,000+ minute ones); 2-hour dedup window (was 1h) via a generic
   shouldSendAlert(type, id, windowMs) helper that future alert types
   (payment_failed, plan_limit_hit, etc.) can reuse.

3. Preferences UI: single checkbox in settings.js Account section bound
   to users.email_alerts. Saved via the existing Save Profile button. PUT
   /api/auth/me extended to accept email_alerts. requireAuth middleware
   SELECT now includes email_alerts so it propagates via req.user.

4. Dev safety net: GRAPH_DEV_RESTRICT_TO env var as an allow-list. When
   set, only listed recipients reach Graph; everyone else is suppressed
   with a log line. Prevents local dev (which often runs against fresh
   prod DB copies) from accidentally emailing real prod users. UNSET on
   prod systemd unit so production fans out normally.

Also: package.json scripts use --env-file-if-exists=.env so local dev
picks up .env automatically (Node 20.6+ built-in, no dotenv dep). Prod
runs via systemd ExecStart and is unaffected. server/.gitignore added
to keep .env out of git.

Smoke verified end-to-end:
- Sequential send pattern verified (a prior parallel-send tick had hit
  Graph's MailboxConcurrency 429 on 30 simultaneous sends; sequential
  at ~250ms each completes the same backlog without throttling)
- 24h cutoff silenced 20/21 prod devices on the next tick
- Dev restrict suppressed the 1 within-24h send
- User-preference toggle flipped via UI -> DB -> alert path silently
  continued before reaching even the suppression log
2026-05-12 18:16:40 -05:00
ScreenTinker 2954fd1a84 Phase 2.1: tenancy middleware, permission helpers, JWT workspace context, frontend + backend role-rename compat 2026-05-11 20:02:00 -05:00
ScreenTinker 772ead28a2 Fix reset-admin.js: honor recovery token in requireAuth
scripts/reset-admin.js signed a JWT with a synthetic id ("recovery-XXX")
and instructed the operator to paste it into localStorage. But the
requireAuth middleware always SELECTs the user row by id, so every
authed API call under the recovery token returned 401 "User not found"
and the recovery flow was effectively dead.

Fix:
- reset-admin.js now sets a `recovery: true` claim on the JWT.
- requireAuth / optionalAuth short-circuit the DB lookup when
  decoded.recovery === true and synthesize a req.user record in
  memory (role: admin, plan_id: enterprise). The synthetic user is
  never persisted, so FK-constrained writes that expect a real
  user (creating devices, etc.) will still fail — which is fine,
  recovery is only meant to let the operator reset a password or
  create a fresh admin via the Settings UI.

Security: a recovery token still requires the jwtSecret to sign,
so only someone with filesystem access to the server can mint one.
Token TTL remains 1h.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-21 19:08:49 -05:00
ScreenTinker afbe113acf Security audit remediation: auth, IDOR, XSS, hardening
- Device WebSocket authentication: devices get a device_token on
  registration, must present it on reconnect. All WS events require
  prior auth. Timing-safe token comparison.
- IDOR fixes: ownership checks on schedules (device, week), layouts
  (all CRUD, zones, duplicate, device assign), video-walls (content,
  device-config).
- XSS prevention: shared esc() helper in utils.js, fixed 13 innerHTML
  injection points across 9 frontend files.
- OAuth hardening: no longer silently overwrites auth_provider on
  accounts with local passwords (returns 409).
- JWT pinned to HS256 for sign and verify.
- Password policy: change endpoint now requires 8 chars (was 6).
- HSTS header enabled (max-age 1 year, includeSubDomains).
- Stripe webhook rejects unsigned payloads when no secret configured.
- Screenshot size validation (max 2MB base64).
- Rate limiting on exports, imports, content operations.
- Content file serving checks playlist_items instead of old assignments.
- Content ownership verified in device-groups assign-content.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-11 22:48:07 -05:00
ScreenTinker 1594a9d4a4 Initial open source release
ScreenTinker - open source digital signage management software.
MIT License, all features included, no license gates.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-08 12:14:53 -05:00