mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-14 14:23:14 -06:00
43 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
ScreenTinker
|
b1a58144bb |
Microsoft sign-in could never complete: Entra omits email_verified
The OIDC callback required `claims.email_verified === true`. Entra ID v2 does not send that claim at all, so every Microsoft login authenticated correctly against the tenant and was then refused with `email_unverified` on the way back. Nothing caught it: the SSO tests assert how the Microsoft issuer string is built but never put a Microsoft-shaped token through the policy. The strict check was itself a fix -- `=== false` had been accepting an omitted claim -- and it is right for a provider a CUSTOMER configured, since such a provider is chosen by the party it vouches for and its bare assertion is worth nothing. What was wrong is treating that as a question about the token when it is a question about who we trusted. `users.email_verified` is our own state; the claim is the IdP's. An instance-wide provider was chosen by the operator -- the same trust that already exempts it from domain confinement -- and Microsoft is additionally pinned to one tenant GUID, so only that directory can issue a token whose `iss` matches. So the policy now depends on the provider, in emailIsVerified(), next to the flag it reads so the two cannot drift: - explicit true -> believed, from anyone - claim absent, operator -> believed (Microsoft; opt-in for other IdPs) - claim absent, org -> refused - explicit false -> refused, always Org providers pin the flag false in rowToProvider and never read it from the row, so the takeover path the strict check existed to close stays closed. Google is left strict: it does send the claim. Also documents MICROSOFT_CLIENT_SECRET (supported in code, missing from the table), that the redirect URI must be registered under Web rather than SPA, and the email optional claim -- the other two ways an Entra setup fails. All three mutations of this policy fail the new tests: reinstating the strict check (3 failures), letting an org provider assume (1), and accepting an explicit false (2). |
||
|
screentinker
|
6aeb703efe
|
Merge pull request #254 from ChrisChrome/main
Add org-level widget sandbox toggle. |
||
|
ScreenTinker
|
fbf55f842c |
Close the third QA round: limiter bypass, stored XSS, break-glass, org placement
Four HIGH findings. Two were mine, and one was a composition of two of my own fixes.
ONE EXTRA SLASH DEFEATED EVERY /api/auth LIMITER
`/api/auth//login` still reaches the login handler — Express normalises the mount
boundary for the router — but `app.use('/api/auth/login', rateLimit(...))` does not
match it, so the limiter never runs. A review got a real session after 60 unthrottled
password attempts. Same for //totp/verify (unlimited 6-digit brute force),
//forgot-password (unlimited reset mail to any address) and //sso/discover (the
customer-enumeration cap, gone). Fixing the limiter KEY could never help, because the
middleware was never invoked: the path is now collapsed to one canonical form before
routing. Pre-existing, and it falsified this file's own warning about walking past the
login limiter.
STORED XSS: I ESCAPED ONE COPY OF THE TABLE
My earlier fix patched views/admin.js line 357 and missed line 372 in the same
function — and missed views/settings.js entirely, which renders a SECOND copy of the
platform users table from the same endpoint, including the email in a raw text node.
The write path was `POST /api/admin/users`, whose EMAIL_RE barred only whitespace, so
an org or workspace admin (not a platform admin) could choose an address that executed
in the operator's session. Both tables escaped, both regexes tightened to reject markup
characters, verified against 11 address shapes.
I KILLED THE BREAK-GLASS WHILE CLOSING AN ORACLE
Hoisting the domain check above the account lookup — my fix for the enumeration oracle
— made `user.role !== 'platform_admin'` unreachable for enforced domains. On a
self-host the operator IS the org owner, and my would_lock_out_actor guard GUARANTEES
their address is inside the enforced set, so the recovery loop closed on itself:
approving a removal request needs a signed-in platform admin. Both properties hold now
by letting the operator through on a CORRECT PASSWORD only — every wrong answer is the
identical 403 whether the address exists, does not exist, or is theirs. Verified: 200 /
403 / 403 / 403.
Also fixed: enabling SSO-only locked out every password-holding member including the
admin who pressed the button (password refused by policy, SSO refused by
account_exists_local). An org provider now adopts a password account at a domain it has
PROVED by DNS when the org requires SSO — which is what a verified domain means, and
what every hosted identity product does.
SSO USERS WERE LANDING IN A PERSONAL ORG
The membership write added organization_members but no workspace_members, and
ensureDefaultOrgForUser looks at workspaces — so it minted each SSO user a private
organization and made it their current one. The customer's Members page read
"Members (1)" while their staff signed in successfully and were invisible.
ALSO: bcrypt on a NULL password_hash 500'd with a stack (and was an oracle for accounts
a provider deletion had returned to local); stranded_members was returned by the server
and discarded by the UI; a provider with zero domains was the one useless state with no
warning; two limiter shapes were missing (removal-request shared the garbage bucket —
an unauthenticated flood could deny the SSO break-glass path); doubled mail subject
prefixes; a DELETE that toasted "Saved"; a decided request left in the DOM with live
listeners; and a confirm dialog promising "immediately" when sessions already open
survive.
1609 tests, three clean runs. Limiter, break-glass, oracle parity and null-password all
verified against a running server.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
94e1273ecd |
Fix: per-organization SSO was blocked by our own CSP and had never worked in a browser
THE HEADLINE FEATURE COULD NOT RUN.
"Continue with single sign-on" was a <form method="POST"> that redirected on to the
customer's identity provider. Chrome applies `form-action` across the WHOLE redirect
chain, and the dashboard sets `form-action 'self'`, so the hop to the provider was
aborted — silently. The user clicked and nothing happened: no navigation, no toast, no
spinner, a byte-identical page. Combined with SSO-only it was a total lockout: password
login answers 403 "use the single sign-on button", pointing at a button that cannot
work.
Every test I ran on this feature checked the button RENDERED. None clicked it.
The provider origins cannot be allowlisted — customers supply them at runtime. So the
page now fetches the destination and navigates itself; a script-initiated navigation is
not governed by form-action. The redirect answer is kept for a caller without
JavaScript, where the chain stays same-origin until the provider takes over. The slug
in the JSON is not a disclosure: following the old redirect put it in the address bar
and history anyway.
Verified in Chrome: the provider start endpoint is reached, zero CSP violations, zero
aborted requests — where before it was ERR_ABORTED plus a console violation.
STORED XSS IN THE PLATFORM ADMIN'S SESSION
admin.js interpolated user name, email and auth_provider into innerHTML unescaped, and
/register accepted an address whose local part was an img tag with an onerror handler —
no spaces, so it slipped the asserted-email check too. A reviewer registered
anonymously and got script execution on #/admin: the page operators are now emailed to.
Escaped, and registration refuses addresses that are not addresses. (The render bug
predates this branch; the reachability and the significance of that screen do not.)
ALSO
- the org SSO button is secondary while a password still works; two identical blue
buttons stacked sent people to their IdP by muscle memory after typing a password.
1609 tests, three clean runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
85febe05c0 |
Fix a login-page dead end, an enumeration oracle, and three boot/limiter defects
From the regression sweep. The first is a genuine regression against main.
A RATE-LIMITED DISCOVERY PERMANENTLY DEAD-ENDED THE LOGIN PAGE
lookupOrgSso checked that a body PARSED, not that the request succeeded — and a 429
body is valid JSON. So `data.sso` came back undefined, the single sign-on button was
hidden, the password box restored, and the domain recorded as answered: permanently,
for the life of the page. On an SSO-only domain that is the worst outcome available —
the password box then returns 403 and the button the user is told to use is not on the
screen. Discover is 10/min per IP and one person filling in the form costs up to four
calls, so a few colleagues behind one office address is enough. The comment above that
code already claimed to prevent exactly this; it only ever covered the 5xx case.
THE SSO-ONLY REFUSAL WAS AN ACCOUNT-EXISTENCE ORACLE
403 for an address that exists, 401 for one that does not — from an endpoint whose own
lockout returns 401 specifically to avoid that. The DOMAIN check now runs BEFORE the
account lookup, so both answer identically; whether a domain uses single sign-on is
already public through /sso/discover, so it reveals nothing new. The membership-level
refusal is deliberately downgraded to the generic 401, because a distinct answer there
would put the oracle back for exactly the accounts worth enumerating.
Verified: existing and invented addresses at an SSO-only domain both 403; and on an
instance with NO SSO configured, register/login/wrong-password/unknown-address are
201/200/401/401 — the hoisted check does not touch them.
BOOT PREFLIGHT
- a cold install ran `npm ci --omit=dev` unconditionally, so a first start on a
developer machine left `npm test` broken: same class of surprise as the prune this
file already warns about, through the other branch of the same if. Now production-
only.
- two servers starting together: the loser died with ENOTEMPTY even though the tree
was complete by then. It re-checks before failing.
- the opt-out accepted only '1', unlike every other boolean the server takes.
THE LIMITER FOLD, DONE PROPERLY
Unmatched paths under /api/organizations still minted a bucket each. My first fix was a
catch-all regex — which put every unknown path in ONE bucket WITH the real endpoints,
so flooding nonsense URLs exhausted the limit for /sso-only. That trades a bypass for a
denial of service. Folding is now by explicit shape: known endpoints keep their own
keys, everything else shares a bucket kept apart from all of them.
Verified: 120 unmatched paths give 60/60 (bypass closed), and after that flood
/sso-only, /sso and /sso/:id/test all still answer 401 rather than 429 (no starvation),
while 70 hits on one real endpoint do trip its own limit. The login trailing-slash
bypass stays closed.
1609 tests, three clean runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
355b7a2b86 |
SSO: build the operator approval screen, and close the last of the QA findings
The approval workflow had no front door. The notification email told the operator to "review it in ScreenTinker under Admin" and that screen did not exist — the only way to approve was curl, while the tenant sat locked out of their own product. Admin now leads with a removal-request section: who asked, for which organization, the reason they gave, what approving does, and Approve/Reject. It hides itself when the queue is empty. Approving is confirmed; rejecting is not, because rejecting only leaves the safe state. REGISTRATION BYPASSED SSO-ONLY AND SQUATTED ADDRESSES /register had no domain awareness: it issued a working session at an SSO-only domain, and the account then held that address forever, because an SSO login will not adopt a row that has a password. Registering ceo@acme.test before the real CEO's first login left the address dead in both directions with no self-service way out. Refused now, and "Create Account" is hidden on the login page for those domains — it was the only action left on the card, so the page was inviting the one thing that cannot work. THE NEW RATE LIMIT WAS DECORATIVE /api/organizations carries three caller-chosen segments, and only the OIDC slug was folded — so every request minted its own bucket. Measured: 120 calls with unique org ids produced ZERO 429s, unauthenticated, against the limit that exists to bound outbound discovery and live DNS. Now 60/60. The general problem was named in the previous commit's own comment and then not applied to the mount it added. XSS IN THE TOAST showToast built innerHTML from server strings, including ones that reflect input verbatim — a reviewer typed `<img src=x onerror=alert(1)>` as an issuer and got script execution in the admin's session. Escaped. ALSO - the org SSO button sat BETWEEN the "Password" label and its input, so the label described the button and the field had none; moved below the input, with a for= - the OR divider survived when the providers under it were hidden - provider action buttons were clipped off-screen at 375px with no way to scroll to them — "Remove" was unreachable; the row wraps now 1609 tests. Verified in real Chrome: 13/13 on the approval loop and the login states, including approving a request and watching password login re-open for that org. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A |
||
|
ScreenTinker
|
983bee31b7 |
SSO-only: close the backdoor, the unilateral disable, and the fresh-install fail-open
Three HIGH findings from the QA round. Each was demonstrated end to end against a running server, and each is now refused there. ENFORCEMENT PROTECTED A DOMAIN, NOT AN ORGANIZATION ssoOnlyForEmail answers about an address's domain, so any account in the tenant at an outside address kept password login — a contractor, an MSP, the one address nobody remembered. And it could be manufactured: POST /api/admin/users accepts workspace_admin and creates a LOCAL password account at any address bound to that workspace. A review created backdoor@notacme.test, logged in with the password, landed in the SSO-only org, and used it to create another. Enforcement is now keyed on MEMBERSHIP as well as domain (ssoOnlyForUser), and that route refuses to mint password accounts into an SSO-only organization at all. platform_admin keeps both, as the operator break-glass. THE APPROVAL WORKFLOW WAS DECORATIVE `sso_only` is honoured only while a provider is enabled and a domain is verified, so `PUT {enabled:false}`, `PUT {email_domains:""}` and `DELETE` each switched enforcement off — with sso_only still reading true, no request filed and the operator never told. The delete variant additionally rewrites every federated account to `local`, after which a password reset takes over accounts the identity provider was supposed to own. Anyone who could file a request could simply turn the provider off instead. All three now refuse with sso_only_locked when nothing else would still enforce, and say to ask for approval. FRESH INSTALLS FAILED THE MIGRATION AND FAILED OPEN The ALTER adding organizations.sso_only sat in the column-migration array, which runs BEFORE the multi-tenancy migration that creates the table: `[migrate] FAILED … no such table: organizations`, one line among ~85. The instance then ran its whole first boot with the SSO settings screen 500ing and ssoOnlyForEmail catching `no such column` and answering "not required" — password login proceeding for an organization that had switched it off. It self-healed on the second boot, which is what made it easy to miss. The column is now added after the table exists, and the catch distinguishes "this instance has no per-org SSO" (null, so single-tenant installs keep working) from drift on a table that DOES exist (throw). Login treats an undeterminable answer as "required" rather than letting a 500 escape or letting the login through. Verified live, all four refused with enforcement intact and the operator still able to sign in. 1609 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A |
||
|
ScreenTinker
|
0e8ffa5444 |
SSO-only: an org may require its own identity provider, operator approves removal
Per-organization toggle. Enabling is the safe direction and an org admin does it
alone; turning it back off is a REQUEST that a platform admin has to approve, because
that is the direction that re-opens password sign-in — the direction a compromised
admin would take, and the one a customer will demand at their worst moment with the
IdP down.
- requires at least one VERIFIED domain, so nobody can lock a company out of a
domain they only typed, and an org cannot leave its own people with no way in
- the login page HIDES the password field for those domains rather than letting
someone type a password that will be refused and then go reset it
- the refusal is `sso_required`, distinguishable from a wrong password
- the approval email carries NO action link: a token that acts on its own turns
every forwarded copy into a way to switch off a customer's SSO. The decision is
made signed in as a platform admin.
INSTANCE PROVIDERS WERE A SIDE DOOR
Blocking passwords while leaving "Continue with Google" is not requiring single
sign-on, it is renaming the bypass — instance-wide providers are the operator's and
are NOT domain-confined, so one could assert an address at an SSO-only domain and walk
straight past the customer's MFA and deprovisioning. The callback now refuses any
provider other than that organization's own, and the page stops offering them.
Instance-wide stays the default everywhere else: an address whose domain has no org
SSO still gets local plus every configured instance provider. The org only overrides
for its own verified domains.
PLATFORM_ADMIN IS EXEMPT, DELIBERATELY
The operator approves turning this off. If the operator's own address sat at an
SSO-only domain and that IdP broke, nobody could sign in to approve anything and the
instance would be bricked. The exemption is the break-glass, and a test pins it as
source so it is not "tidied away" as a convenience.
BROWSER-FOUND
Hiding the password by hiding its .form-group also hid the organization SSO button,
which lives inside that same group — leaving a login page whose only action was
"Create Account". Only visible by looking at a screenshot. Hides the field now, not
the container.
Player untouched: this branch changes no device, WebSocket or provisioning file, and
the 358 device/player/socket/pairing tests pass.
1603 tests pass. Enforcement, the approval workflow and the login page verified in
real Chrome.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
c91b96ab91 |
SSO: refuse delegated proof names, release lapsed and deleted claims
Third review pass. It confirmed the crash wrapper holds (~13,000 hostile requests,
no fourth crash), the SSRF rewrite holds (77 vectors, every CIDR boundary proven),
the rate-limiter rewrite closed the login brute-force bypass, and /sso/claim rejects
every wrong token kind. It also found that two things I built yesterday did not do
what they claimed.
THE 8-HOUR LIMIT DID NOT BOUND SQUATTING
Pressing Verify on an expired claim REISSUED it in place, renewing the clock — so one
request per window held a domain forever, through the endpoint meant to enforce the
limit. Worse, a renewal was not a new claim, so the operator was notified exactly once,
on day zero: a tenant could sit on a company's domain for a year off a single stale
alert. A lapsed claim is now RELEASED. Re-adding it is an ordinary new claim: new
token, and the operator is told again. Squatting is not impossible; it is loud.
A DELEGATED PROOF NAME COULD FORGE A DOMAIN
A TXT lookup follows CNAMEs, and RFC 4592 means a wildcard `*.victim.com` synthesizes
`_screentinker-verify.victim.com` too — so a wildcard CNAME let whoever controls its
target prove a domain they do not own, turning an ordinary subdomain takeover into
every `@victim.com` login. A reviewer did this against a real authoritative zone. The
proof name is now refused if it is a CNAME, which is stricter than ACME's dns-01, and
the comment that claimed wildcards "cannot be mistaken for a proof" — true only for
wildcard TXT — has been corrected.
MY VERIFY BUTTON REPORTED FAILURE ON SUCCESS
`await load()` — the loader is `loadSso()`. The ReferenceError went into a bare catch,
so a correct DNS proof showed "Could not verify that domain" and left the card stale.
On the expired branch the admin kept publishing a token the server had already rotated.
ALSO FIXED
- deleting a provider stranded its verified domains (no FK, UNIQUE, never expires) so
the domain was blocked for EVERY org forever with no in-product recovery, and its
users could neither sign in nor reset. Delete now releases the domains and returns
the accounts to local, in one transaction; a cascade FK backstops it.
- isOrphanedFederated read absence-of-config as proof-of-deletion, so unsetting
GOOGLE_CLIENT_ID made every Google account password-resettable instance-wide, and
irreversibly. Restricted to org-provider slugs.
- `email_domains: null` (not undefined) took the destructive branch and deleted every
DNS proof an organization had.
- unbounded domain lists: 400 domains sent 401 emails; now capped at 50, one digest
per save, and /api/organizations is rate-limited at all for the first time.
- login and register responses carried password_reset_hash and email_verify_hash —
live account-takeover credentials handed to the browser. One sanitiser now.
- trailing-dot hostname (`https://localhost./`) slipped the SSRF guard.
- asyncRoute's own catch could throw and kill the process it exists to protect.
- a legacy DB whose typed domains were never verified now says so LOUDLY at boot
instead of silently locking every federated user out.
TESTS
Two of the previous round's tests passed against the code they were named after: one
asserted UNIQUE against the test harness's own CREATE TABLE rather than the shipped
schema, the other used two different domains so no ordering was exercised. Both
replaced and confirmed load-bearing. Seven mutations now turn the suite red, including
removing the CNAME refusal, the verified_at filter, and the expiry itself.
1598 tests pass. Delete-release, lapse-release and the leak fix verified against a
running server.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
d4b8d7dad4 |
SSO: prove domain ownership by DNS, and fix what the second review found
A second review pass, run against the previous commit, found four blockers — two of
them introduced by the fixes in that commit. It also confirmed the original account
takeover is closed: a hostile IdP with real TLS, discovery, JWKS and RS256 driving the
real routers now stops at domain_not_allowed, and all 16 bypass variants are refused.
DOMAIN OWNERSHIP (the root cause, not the symptom)
A claimed domain used to mean "nobody else claimed it". It now means the organization
published a record in that domain's own DNS — TXT or CNAME, at a dedicated
_screentinker-verify name rather than the apex, where an edit would sit beside SPF.
- an unverified domain routes NOBODY and cannot be asserted; it reserves the name
- an unverified claim LAPSES after 8 hours, so a domain cannot be held against its
real owner, and lapsing rotates the token so a record left over from an abandoned
attempt cannot satisfy a later claim
- a verified domain never expires — re-proving on a timer would log a customer out
over a DNS edit made months later
- routing and confinement read the VERIFIED set only, never the typed column
- configuring SSO now requires a verified email address
- platform admins are emailed when a domain is claimed; nothing is ever sent to the
claimed domain, which would let any tenant make this product email third parties
Instance-wide providers are exempt from all of it: they are the operator's own
configuration and keep the trust they have always had.
BLOCKERS FROM THE REVIEW
- two unauthenticated remote crashes, both one request, both "async handler throws
before its try": `Cookie: st_oidc_tx=%` (unguarded decodeURIComponent) and the
fail-closed secret added last commit, which turned a JWT_SECRET rotation into a
permanent crash loop. Fixed the CLASS with asyncRoute() rather than the instances.
- the SSRF guard was bypassable via IPv4-mapped IPv6 ([::ffff:127.0.0.1]) and also
refused every host beginning "fc"/"fd" (fcm.googleapis.com). Addresses are now
parsed and compared by RANGE. 42 cases verified.
- the takeover fix had NO test — the test named after it asserted two struct fields
and passed with the guard deleted. The decision is now a pure function and four
mutations were confirmed to turn the suite red.
- the PUT path never received the TOCTOU fix, so two orgs could end up holding one
domain and forEmail handed routing to the attacker's older row.
ALSO
- linking compared slugs, so an org could never rotate its own IdP, and fell open on
an empty auth_provider. It now asks which ORGANIZATION owns the slug.
- an account stranded by a deleted provider can be reclaimed by password reset —
proof of the mailbox, which is stronger than the IdP assertion that created it.
- /sso/claim accepted a pre-TOTP mfa_pending token and returned the full user row;
it now takes a purpose-built 120s claim token with a pinned algorithm and typ.
- the rate limiter keyed on a caller-controlled path, so a trailing slash bought a
fresh bucket — a real login brute-force bypass.
- domain_not_allowed and account_exists_other_provider rendered as "please try
again", advice that can never work.
- malformed asserted addresses are refused rather than trimmed into shape.
- dead config (microsoftTenantId defaulted to 'common', which the provider code now
refuses) and the orphaned google-auth-library dependency removed.
1591 tests pass. Domain lifecycle verified end to end against a running server.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
d26aaebef6 |
SSO: fix an account takeover, a remote crash, and login CSRF found in review
Five reviewers went at the two SSO commits. Three of them independently
demonstrated a full account takeover, and it was the same defect each time.
TAKEOVER. An org admin supplies the issuer and client_id, so they control that
identity provider completely and can mint an id_token asserting ANY email with
email_verified:true — including a platform_admin's. Every cryptographic check
passed honestly, because the attacker IS the issuer. upsertFederatedUser then
re-pointed the existing account at whichever provider spoke last, because the
only guard was `password_hash IS NULL` — and every SSO-created account has a
null password. Sessions were issued as the victim, and the victim's own login
then failed forever with subject_mismatch.
The rule came from the old Google handler, where it was safe: only the operator
could add a provider. Making providers customer-configurable turned it into a
takeover primitive and the assumption was not re-examined. Now an org provider
may only assert emails inside the domains it registered, and may never adopt an
account another provider established.
REMOTE CRASH, unauthenticated. The state comparison guarded on UTF-16 character
length while Buffer.from produces UTF-8 bytes, so a state of 43 characters
containing one multi-byte character reached timingSafeEqual with mismatched
buffers and threw — inside an async handler, which Express does not catch, which
server.js turns into process.exit. One request per restart killed any instance
with SSO enabled. Compared as bytes now, and /api/auth/oidc gained a rate limit.
LOGIN CSRF. The callback returned the session token in the URL fragment, so a
crafted link installed an ATTACKER'S token and silently signed the victim into
their account. The token now goes in a one-shot httpOnly cookie exchanged at
POST /sso/claim, which a link cannot forge.
FRONTEND, dead on arrival twice over. login.js used `await` in a non-async
function — a SyntaxError that takes the WHOLE app down, since app.js imports it
statically and there is no bundler. And `esc` was never imported, so the org-SSO
button could never render; the ReferenceError was swallowed by the catch written
for network failures. Both slipped through because `node --check` parses these
files as CommonJS and exits 0 on a broken module. The correct check is
`node --input-type=module --check`, and all four frontend files now pass it.
PUBLIC EMAIL DOMAINS cannot be claimed. A tenant had claimed gmail.com in
review, after which every Gmail user typing their address was offered "sign in
with your organization" pointing at that tenant's infrastructure — phishing from
this product's own login page. server/lib/public-email-domains.js.
MICROSOFT multi-tenant is refused rather than silently broken. `common` metadata
advertises the literal template {tenantid}, so the issuer never matches and
every login already failed; and loosening that check is nOAuth. A tenant GUID is
now required, with a loud warning at boot.
SSRF: https only, loopback/RFC1918/link-local refused, redirects not followed,
and the test endpoint no longer echoes upstream status for a caller-supplied
jwks_uri (it was a readable internal port scanner).
Also: an omitted email_verified was accepted (the comment already said it should
not be); the domain-uniqueness check raced an 8s network call before its insert
and is now inside the transaction; same-org duplicate domains were allowed and
made routing depend on table-scan order; routing is now ordered; a client secret
that cannot be decrypted fails closed instead of silently downgrading to a public
client; SSO audit rows were writing the org id into the deviceId column; and
/sso/start was capped at 10/min per IP, which would 429 the 11th employee behind
a corporate NAT.
Adds per-provider editing in the org admin UI (replace-only secrets — never
returned, blank means keep, explicit clear) and a Test button that checks
discovery, endpoints and signing keys while stating plainly that it cannot
verify the client ID, the secret, or the redirect URI registration.
⚠️ STILL MISSING: domain-ownership verification. A claimed domain means "nobody
else had claimed it", not "they own it". DNS TXT proof is the remaining control.
1582 tests pass. New regression tests cover the takeover confinement, ordering,
fail-closed secrets, the Microsoft refusal and the public-domain blocklist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
e97228a502 |
SSO: per-organization providers, configured by the customer
Instance-wide providers belong to whoever runs the server. These belong to a CUSTOMER: an organization points ScreenTinker at its own identity provider from Settings → Single sign-on, with no environment variable and no restart. The login flow is unchanged. An org provider is resolved through the same oidc-providers.get(slug) the env ones go through, so there is one authorization request builder, one token exchange and one verifier — not a second, less tested path for tenants. That seam is why Phase 1 put provider lookup behind a single function. ⚠️ An org provider is NEVER published. It is not in /api/auth/providers, because listing a customer's IdP would both offer it to people it does not belong to and leak the customer list from the login page. It surfaces only when someone types an address at one of that organization's domains; otherwise the instance-wide buttons are what you get. The discovery endpoint answers with a BOOLEAN and nothing else — no slug, no display name. Returning "yes, Acme Corp SSO" would turn a guessed domain into confirmation that Acme buys this product, and the slug would hand out a working entry point to their tenant. POST /sso/start repeats the lookup server-side and redirects, so the browser never learns which provider it is being sent to until the provider says so, and the address travels in a body rather than in a URL that lands in history, proxy logs and a Referer. Both endpoints rate limited to 10/min. Other properties, each with a test: - slugs are RANDOM, not chosen, so two customers cannot collide on or guess each other's URL - a domain may be claimed by ONE organization; a second claim is refused, so a tenant cannot capture another company's logins - the issuer is verified by live discovery BEFORE the row is written, so a typo is caught at configuration rather than by a user staring at a failed login - client secrets are optional (PKCE), stored AES-256-GCM via lib/secretbox, never returned; an absent secret on update leaves the stored one alone, which is how a settings form that cannot show it avoids blanking it - cross-org access answers 404, not 403, so an outsider cannot confirm that an organization id exists - signing in through an org provider grants membership of that organization, but never changes an existing member's role Verified live end to end: creation against a real issuer, domain normalisation (`@Acme.CO.UK` → `acme.co.uk`), boolean-only discovery, a rejected domain squat, a rejected bad issuer, and 404 for a foreign organization. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A |
||
|
ScreenTinker
|
252854d31e |
SSO: one OIDC flow for every provider, and verify the token properly
The OAuth support that was here could not work and would not have been safe if it had. It could not work: the login page called google.accounts.oauth2 and new msal.PublicClientApplication, and NEITHER SDK WAS EVER LOADED by any page in this app — no script tag, no dynamic import, nothing. Both buttons threw ReferenceError on click. Even had they loaded, the CSP allows scripts only from 'self' and cloudflareinsights, and frames only from self and YouTube, so the libraries and their popups were blocked too. It would not have been safe: both endpoints authenticated with an ACCESS token and neither checked who it was issued for. POST /auth/google fell back to tokeninfo?access_token= and read the email out of the reply; POST /auth/microsoft handed the bearer token to Graph /me and trusted that. Graph and tokeninfo will both describe the user behind a token minted for SOMEBODY ELSE'S application, so any site a user signed into that requested `email` or `User.Read` could have replayed their token here and been issued a session as them. Both endpoints are deleted; nothing is lost, because nothing could reach them. Replaced by ONE generic flow — Authorization Code + PKCE (S256), run server-side, with the provider list resolved through a single function so per-organization SSO can extend it later without a second login path. Google and Microsoft become ordinary configured providers; Okta, Keycloak, Authentik, Auth0 and anything else that speaks OIDC now work with three env vars. Because the exchange happens server-side the browser never talks to the provider, so there is no SDK to load, no client id in the page, and no third-party origin needed in the CSP. Identity comes from an ID token that must survive: signature against the provider's JWKS (asymmetric algorithms only — alg:none and HMAC are refused outright, the latter because the only key we hold is public), `iss` exactly as discovered, `aud` and `azp` matching our client, `exp`, and a `nonce` this server minted for that login. State is compared in constant time against a value in an httpOnly SameSite=Lax cookie, so the callback is CSRF-protected and survives a restart mid-login. Account rules are the ones already in place: a verified email is required, an SSO login never takes over an account that has a password, and a changed `sub` for a known address is refused rather than handing the account to a recycled mailbox. 18 new tests, every one describing something the old code would have accepted: cross-audience tokens, azp mismatch, replayed nonces, alg:none, HMAC forgery, wrong signing key, expired tokens, a discovery document lying about its issuer, and a registry that never leaks a client id or secret to the browser. Verified end to end against Google's real discovery document: the redirect carries response_type=code, PKCE S256, state and nonce, and every callback guard rejects as intended (no cookie, wrong state, no code, provider refusal, unknown provider). ⚠️ TOTP is still not prompted on an SSO login, matching the documented behaviour of the previous SSO and API-token paths. That is a product decision and is left unchanged here rather than altered silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A |
||
copilot-swe-agent[bot]
|
f725186905
|
Add org-level widget sandbox isolation toggle with warnings
Co-authored-by: ChrisChrome <28414320+ChrisChrome@users.noreply.github.com> |
||
|
ScreenTinker
|
b7d55595af |
feat(auth): self-service password reset
Until now the only ways back into an account were an admin setting your password for you
or shell access to run scripts/reset-admin.js. A self-hosted operator who forgot their
password had no path at all, and the admin-reset route explicitly refuses to reset a
platform admin's password — so a single-admin instance was unrecoverable without a shell.
The per-account login lockout added recently makes that sharper: a user who forgets their
password will hit the lockout and see the same generic error, with no way out.
Two unauthenticated endpoints (they must be — the user cannot log in):
POST /api/auth/forgot-password { email } -> always the same 200
POST /api/auth/reset-password { token, password } -> 200 / 400
The properties that matter, each covered by a test:
- NO ENUMERATION. The request endpoint answers identically — same status, same body —
for a real address, an unknown one, an SSO identity with no local password, and a
malformed string. The frontend shows the same confirmation even on a network error,
so the client cannot leak what the server refused to.
- NO MFA BYPASS. Completing a reset does NOT issue a session; the user signs in
afterwards, so a TOTP-enabled account still clears its second factor. Returning a token
here would turn "read one email" into a full session without the second factor.
- SINGLE USE, SHORT LIVED. 32 random bytes, stored only as a SHA-256 hash (same
discipline as email verification, recovery codes and API tokens), 1h TTL, and the
redeeming UPDATE is conditioned on the hash still being present so concurrent
redemptions cannot both win.
- LOCAL ACCOUNTS ONLY. SSO identities have no local password; no token is minted.
- IT ACTUALLY UNBLOCKS YOU. A completed reset clears the per-account login lockout and
must_change_password, otherwise someone who locked themselves out would reset and still
be locked out.
Rate limited: 5/min on the request (it sends mail to a caller-supplied address), 10/min
on the redeem. If no email transport is configured the response is unchanged — no oracle —
but the server logs loudly, because the user will otherwise wait for mail that cannot
arrive and the generic response cannot tell them.
Frontend: a "Forgot your password?" link on the sign-in card, a request card, and a
new-password card. app.js had to learn #/reset-password explicitly — the auth guard
rewrites any unauthenticated hash to #/login, which would have discarded the one-time
token in the emailed link and made it silently do nothing.
Migration adds users.password_reset_hash / password_reset_expires: additive, nullable,
idempotent; a code-only rollback leaves two dead columns.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
ScreenTinker
|
9130aa5f7d |
feat(auth): bound password login per account, not only per IP
The only throttle on POST /api/auth/login was the per-IP limiter in server.js. That bounds one noisy source and nothing else: it does not bound a distributed attempt, and it is only as accurate as a deployment's proxy configuration. Nothing counted failures against the account actually being attacked, and nothing cleared such a count on success because no such count existed. lib/login-lockout.js mirrors lib/totp-lockout.js and lib/pair-lockout.js so there is one lockout idiom here rather than three. 10 failed passwords lock an account for 15 minutes. Keyed on user.id, never on the submitted email: the email is attacker-supplied and unbounded, so keying on it would let anyone grow the Map without limit — the same class of bug fixed elsewhere in this campaign. A user id only exists for a real account, so the key space is bounded by the user table and needs no eviction sweep, exactly like totp-lockout. A locked account returns the SAME 401 and body as a wrong password. A distinct 429 would tell an attacker "this account exists and is under attack", turning login into an account-existence oracle; the test asserts the locked response is byte-identical to both the wrong-password and unknown-account responses. The trade is that a locked-out legitimate user sees the generic message, so the trip is recorded in activity_log (auth:login_locked) for the operator instead. The counter is cleared as soon as the password verifies — before the TOTP and email-verification branches, which return early and never reach issueSession, so a reset placed there would never fire for those accounts. SSO paths do not share this code and are unaffected. Frontend needs no change: login.js renders any non-ok body's `error` string verbatim, and the body is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
ScreenTinker
|
c4b5a8679e |
refactor(auth): centralise session token resolution across manual verify sites
Six places verified a session JWT inline instead of going through requireAuth, each repeating a slightly different subset of its checks. Introduce resolveSessionUser() in middleware/auth.js as the single definition of "this token is a usable session, and here is whose it is", and route all of them through it: the three /api/status token routes, the screenshot route, the content-reference gate, and the /dashboard socket handshake. requireAuth is now a thin wrapper over the same helper, so the two cannot drift. Also: - Give the pre-TOTP token a distinct audience so it is redeemable only through verifyMfaPendingToken (POST /api/auth/totp/verify). verifyToken refuses any token carrying an audience, so a token minted for one purpose cannot be redeemed on another path. - The dashboard socket handshake now takes userId/userRole from the live users row rather than from the token claim, so role changes take effect on the next connection instead of riding the token's remaining lifetime. - Add test/session-token-resolution.test.js covering all six surfaces, including the socket handshake. Every call site keeps the status code and error body it returned before. Net query cost: the content-reference gate and the socket handshake each gain one users-by-id lookup (the same one requireAuth already does per request); the other four are unchanged or replace an equivalent lookup. In-flight pre-TOTP tokens are invalidated by the audience change; they live 5 minutes, so the window is a re-login at worst. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
ScreenTinker
|
b938fce368 |
feat(auth,tizen): TOTP 2FA UI, email verification on signup, Tizen SSSP install
Three features from this session, full server suite green (535/535). TOTP 2FA (#100) — backend shipped without a UI; add it: - Login: mfa_required -> 6-digit challenge (recovery codes accepted) -> /totp/verify. - Settings > Account: enable (QR + confirm -> recovery codes once), regenerate, disable; SSO accounts see "managed by your identity provider". - /totp/setup returns a server-rendered qr_data_url (bundled qrcode dep). keyuri folds the request Host into the issuer so multi-instance accounts are distinguishable in the authenticator app. Email verification on signup — hosted HARD-block / self-host SOFT-nudge: - email_verified column; existing users asked on first login (SSO + platform admins grandfathered); single-use 24h tokens (SHA-256 hashed). - Gate engages only when email is configured (never locks out a no-mail instance). GET /verify-email + POST /resend-verification (generic, no account enumeration). - Client: "confirm your email" flow + resend, verified/error toasts, self-host banner; onAuthSuccess refuses a tokenless response (defensive). Tizen SSSP URL-Launcher install — Fusion-style one-URL native install: - Server hosts /tizen/sssp_config.xml (dynamic <size>, always matches the served .wgt) + /tizen/ScreenTinker.wgt + a human landing. lib/wgt-cache.js resolves the signed .wgt (/data mount wins, mirroring the APK). - build-wgt.sh also emits a static sssp_config.xml for CDN hosting. - Retail panels require a Samsung Partner cert; dev-mode is SDB self-signed only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
ScreenTinker
|
674a34ba45 |
feat(config): HIDE_BILLING flag to hide the Subscription/billing UI (#116)
Opt-in, default-off UI gate (per strobe's spec; verified his file refs first).
When set, hides the Subscription sidebar item + billing view and bounces
#/billing to the dashboard. Billing shown by default -> existing deployments
unchanged. UI-only: /api/subscription/* untouched (internal usage reads stay).
- config.js: config.hideBilling from HIDE_BILLING (mirrors selfHosted).
- auth.js: surface hide_billing on GET /api/auth/me (client already fetches it
at boot, stored on the user object).
- index.html: id="billingNavItem" on the Subscription <li> (mirrors adminNavItem).
- app.js: toggle billingNavItem in updateSidebarUser (next to the admin toggle);
guard #/billing -> history.replaceState('#/') + render dashboard (replaceState
so the back button doesn't loop into the guard).
- .env.example + README documented.
Spec assumptions verified against code: adminNavItem toggle pattern exists;
/me is fetched at boot and updateSidebarUser runs both at boot (cached user)
and post-/me, so no-flash holds on warm loads (one-time flash possible on the
first load after the flag flips — same as the admin nav, minor); route dispatch
is an if/else chain. Nav label is static (no data-i18n) so no i18n change.
Validated (headless Chrome, both states):
- flag unset -> Subscription tab present, #/billing renders (backward-compat).
- HIDE_BILLING=true -> tab hidden, #/billing redirects to #/.
- config maps HIDE_BILLING both ways; live /me default hide_billing=false.
- 149 server tests green. Default-off = zero change for existing deployments.
Known cosmetic (harmless): after the redirect the billing nav LINK keeps its
'active' class, but the nav item is display:none so it's never visible.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
ScreenTinker
|
f4c5865013 |
fix(server): strip totp_secret_enc/totp_last_step from login responses (#100)
Review caught the encrypted TOTP secret riding in the login/verify response body: issueSession receives a SELECT * user row and only destructured out password_hash, so totp_secret_enc (and the internal replay counter totp_last_step) leaked. Encrypted, so not catastrophic, but it regresses the API work's "secrets never in responses" rule. Strip both in issueSession (covers /login and /totp/verify); add an assertion that a verify response carries no totp_secret_enc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ScreenTinker
|
1d3e9acea4 |
feat(server): TOTP MFA login flow + enrollment/verify endpoints (#100)
Two-token login: /login returns an mfa_pending token when TOTP is on; requireAuth/optionalAuth REJECT mfa_pending (tightening #1 - else password-alone is a session). /totp/verify exchanges it + a TOTP or recovery code for a full session (per-user lockout; recovery checked independently of the decryptable secret). Enrollment: setup -> enable (confirm-then-enable) -> recovery codes shown once; disable/regenerate require re-auth; regenerate replaces atomically; status surfaces codes-remaining (tightening #3). API tokens + SSO bypass TOTP by construction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ScreenTinker
|
05f9c20ecf |
fix(admin): user deletion failed with FOREIGN KEY constraint (#18)
DELETE /api/auth/users/:id ran a bare `DELETE FROM users`, but 23 columns
reference users(id) and only 4 cascade, so with foreign_keys=ON the delete
fails the moment the user is referenced anywhere - and a real user always is
(owns an org, created a workspace, has login activity). Reproduces on a fresh
DB, exactly as reported.
The schema also lacks cascades from workspaces -> tenant resources, so the DB
can't clean up on its own. New lib/user-deletion.js resolves every reference in
one transaction (defer_foreign_keys=ON for forgiving order; table-existence
guard for resilience):
- Refuse (409) if the user OWNS an organization that has other members -
don't nuke a shared tenant; transfer ownership first.
- Hard-delete the organizations they SOLELY own (workspaces + all contents).
- In orgs they don't own, PRESERVE resources: SET NULL the nullable
creator/inviter columns, and reassign the NOT NULL legacy creator user_id to
the resource's org owner (fallback: the acting admin).
- Memberships (organization_members/workspace_members/team_members/
content_folders) cascade on the user delete; pending invites they sent and
legacy teams they own are removed.
The handler now 404s an unknown id and 409s the shared-org case.
Tests (node:test): reproduces the FK failure, then verifies provisioned-member
delete (resources preserved + unlinked/reassigned), solo-org-owner cascade,
shared-org refusal (409), self-delete 400, non-superadmin 403, unknown 404.
Full suite 22/22. Verified end-to-end on a copy of a real DB: deleted a user
owning 2 solo orgs, foreign_key_check clean.
Closes #18.
|
||
|
ScreenTinker
|
7615eabdd5 |
feat(admin): Workspace column + inline move/assign on the Users page
Adds a "Workspace" column (after Plan) to the platform Users admin table so a platform_admin can see and reassign a user's workspace inline, alongside the Role/Plan dropdowns. Single-workspace move/assign model. Backend: - GET /api/auth/users (platform branch): one aggregate query adds workspace_count and, for exactly-one membership, the workspace id/name + org name (no N+1). - PUT /api/admin/users/:id/workspace (requirePlatformAdmin - operator excluded): move (1 membership) or assign (0) into the chosen workspace, default role workspace_viewer, in a transaction; no-op if already there; REFUSES (400) a user with >1 membership (manage in the members view). logActivity admin_set_user_workspace. Frontend (admin.js): - Editable <select> only for a 'user' with 0/1 membership; multi-membership -> read-only "N workspaces", platform staff -> read-only "Platform (all)". - Options grouped by org via <optgroup>, built ONCE from /me's accessible_workspaces (same source as the Add User picker) and reused per row. - Picking "Unassigned" or the same workspace is a no-op so a stray pick can't strip a membership. Success -> toast + refresh. EN i18n only. Tests: 4 added (single-membership move 200 + changed, zero-membership assign 200, multi-membership 400 refused, non-platform-admin/operator 403). npm test 16/16. Verified headless: column renders, selected value correct, "Platform (all)" for staff, and a dropdown move persisted (throwaway user, cleaned up). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ScreenTinker
|
5502a3eaa8 |
fix(roles): make platform_operator assignable + add deny/assign regression tests
The bug: #13 added 'platform_operator' to the frontend role dropdown (PLATFORM_ROLE_OPTIONS) but #14's PUT /api/auth/users/:id/role whitelist (ASSIGNABLE_PLATFORM_ROLES) only listed ['user','platform_admin'], so selecting "Platform operator" returned 400 "Invalid role" - the role was unassignable via the UI. Fix: add 'platform_operator' to ASSIGNABLE_PLATFORM_ROLES. One line; the self-demote guard is intentionally left untouched (a platform_admin still cannot self-assign the non-owner operator role and lock themselves out). Tests (node:test, isolated in-memory DB injection - no DB_PATH change): - admin-users.test.js: platform_admin can PUT role=platform_operator on a target user -> 200 and the row persists as platform_operator (regression guard for the whitelist gap). - operator-permissions.test.js (new): verify-then-test of the highest-blast -radius deny. Operator CAN update/delete a workspace-scoped content row (cross-org write works) but is denied (403) updating or deleting a shared (workspace_id IS NULL) row - proving the separate PLATFORM_ROLES gate in content.js's checkContentWrite still holds after canWrite was broadened to isPlatformStaff. Verified read-only (no leak): the other shared-asset write sites keep their PLATFORM_ROLES gate that excludes operator - kiosk.js:57, widgets.js:110, folders.js:31, layouts.js:59/117/133. cd server && npm test -> 12 pass / 0 fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ScreenTinker
|
54549420e7 |
feat(signup): optional org-on-create for self-service signups (#12)
MSP-style deployments want self-service signups created WITHOUT a personal
org, so an admin/operator can assign them into an existing customer org
afterward.
- config.autoCreateOrgOnSignup (AUTO_CREATE_ORG_ON_SIGNUP env), default
true - single-tenant and the hosted self-service flow are unchanged.
- ensureDefaultOrgForUser gains { allowCreate }: an existing membership is
always returned (idempotent); the MINT path is gated. allowCreate=false +
no membership -> returns null (user created org-less).
- register accepts a per-request createOrg flag overriding the deployment
default; the first-ever user is always given an org (never headless).
login / Google / Microsoft pass allowCreate from the global config, so an
org-less user is not silently given an org on next sign-in.
Edge case: a non-platform user with zero workspaces now lands on a "no
workspaces yet" empty state (new no-workspace view) instead of being bounced
into onboarding (whose pairing step needs a workspace). route() redirects
them there, and refreshCurrentUser() redirects once /me reveals zero
accessible_workspaces (covers the first-load race). The workspace switcher
already rendered an empty placeholder and resource routes already return []
for a null workspace, so nothing crashes in between.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
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> |
||
|
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> |
||
|
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> |
||
|
ScreenTinker
|
b67fbaa1b6 |
feat(signup): welcome email + admin signup notification (slice 1)
Every new user now gets a personal welcome email from
"Dan at ScreenTinker" <support@screentinker.com>, and Dan gets an
admin notification, immediately after signup. Fired from all three
signup paths (local /register, Google, Microsoft) via a shared
helper (services/signupEmails.js) at the new-user branch only, so
OAuth logins of existing users don't re-trigger.
- Reuses the single Microsoft Graph transport (services/email.js).
Adds two optional, backward-compatible params: fromName (custom
From display name; address stays support@ so replies route there)
and rawSubject (skip the "[ScreenTinker] " prefix for clean
subjects "Welcome to ScreenTinker" / "New signup: <email>").
- Idempotency: users.welcome_email_sent_at, stamped after the send
block; non-null short-circuits so a user is only emailed once.
Paired backfill stamps all pre-existing users with sentinel 1 so
a future "IS NULL" sweep can't mistake the legacy base for
un-welcomed and blast them.
- Production-only: gated on !config.selfHosted so self-host
operators never emit mail from our domain or CC Dan.
- No retry logic by design (no re-trigger path on existing users);
per-email {sent, reason} is logged so a Graph hiccup is visible.
Admin notification includes workspace org name, email, UTC + Central
timestamp, client IP (CF-aware), CF-IPCountry, and user agent.
|
||
|
ScreenTinker
|
c4fbd2ba5c |
feat(workspaces): invite/accept-invite backend (slice 1+3)
Slice 1 + 3 of the user-management feature from the May 12 plan.
Backend-only - no UI yet (slice 2 ships separately). Backend +
accept-handler together so the email accept link is functional
from day one without a half-state.
Endpoints added:
- GET /api/workspaces/:id/members (any member; via_org=true
for org-level entries,
read-only from ws context)
- GET /api/workspaces/:id/invites (workspace_admin)
- POST /api/workspaces/:id/invites (workspace_admin)
- DELETE /api/workspaces/:id/invites/:inviteId (workspace_admin)
- PUT /api/workspaces/:id/members/:userId (workspace_admin)
- DELETE /api/workspaces/:id/members/:userId (workspace_admin)
- POST /api/auth/accept-invite/:inviteId (requireAuth +
case-insensitive
email match)
Permission gating:
- canAdminWorkspace (existing) for admin-gated endpoints
- canAccessWorkspace (new helper in lib/permissions.js) for the
members read endpoint - mirrors canAdminWorkspace shape but
admits any workspace_members role plus org/platform paths
Security additions vs the original plan:
- Transaction-bounded collision check on POST /invites closes the
TOCTOU race between simultaneous duplicate POSTs (no UNIQUE
constraint on workspace_invites(workspace_id, email))
- Per-(inviter, workspace), hour-window rate limit on POST /invites
to prevent abuse / cost runaway. Env-configurable via
INVITE_RATE_LIMIT_PER_HOUR with conservative 50/hour default.
429 response is generic - does not echo the configured value.
- Invite expiry env-configurable via INVITE_EXPIRY_DAYS (default 7)
- PUBLIC_URL env var (optional) pins the accept-URL origin in prod;
falls back to request-derived for local dev
Rollback rule on email send: only graph_error (real send attempt
failed at Graph) deletes the row and returns 502. not_configured
and dev_restricted are intentional non-sends - keep the row, count
against rate limit, allow local accept-invite testing to proceed.
Other safety blocks:
- Cannot demote/remove the last workspace_admin (409)
- Cannot remove the parent-org's org_owner via workspace path (403)
- Accept-invite is idempotent if user already a member
- Expired invites delete-on-read and return 410
- Wrong-account accept returns 403 without touching the invite
Expired-invite cleanup added to services/heartbeat.js mirroring
the team_invites sweep pattern.
Verification: 9-case curl-driven E2E against the dev DB fixture
(switcher-test + invitee-existing + invitee-new mid-flow register).
All 9 pass: create / collision-409 / second-create / rate-limit-429 /
existing-user-accept / register-then-accept / wrong-account-403 /
expired-410 / viewer-cannot-invite-403.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
ScreenTinker
|
d2a3bdfd15 |
fix(auth/me): broaden non-admin accessible_workspaces to include org_owner/org_admin paths
The non-admin branch of /me's accessible_workspaces query drove from workspace_members, so users with org_owner or org_admin on an organization but no direct workspace_members row were missing those workspaces from their /me response - and therefore from the switcher dropdown. Mirrors the access logic in accessibleWorkspaceIds() (lib/tenancy.js) while keeping the full-row SELECT shape /me needs. Verified end-to-end with switcher-test@local.test acting as org_owner of Acme Studios with no workspace_members row on Studio B - Studio B now appears in /me's accessible_workspaces with workspace_role: null, can_admin: true. Also updates the stale TODO comment in tenancy.js that flagged this exact gap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
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 |
||
|
ScreenTinker
|
ce332ead67 |
feat(switcher): per-workspace device count in dropdown rows
/me's accessible_workspaces query gains a device_count field via a correlated subquery on workspaces.id - WHERE workspace_id = w.id strictly excludes the unclaimed pair-pool (workspace_id IS NULL fails equality). Added to both query branches (platform_admin LEFT JOIN and regular INNER JOIN); microseconds per row at current scale (~37 rows worst case), not optimizing. Frontend appends the count to the muted org-name line with a middle-dot separator: 'Acme Studios . 2 devices'. Singular/plural respected via the existing tn() helper convention; 'No devices' for empty workspaces. New formatResourceCount(n, keyBase, zeroKey) helper is generic so the same shape can wire users/playlists/schedules counts later without refactor. New i18n keys: switcher.devices_count_one, switcher.devices_count_other, switcher.no_devices. Added to en.js only; other locales fall back to en via the existing lookup chain (verified in i18n.js:19). API smoke verified: switcher-test sees Studio A=2, Field Crew=2; dw5304 (platform_admin) sees all 37 workspaces with their device counts varying 0-4; single-workspace zero-device user (geoff.case) sees 0. |
||
|
ScreenTinker
|
56da64d0cd | feat(workspaces): rename via switcher dropdown - new PATCH /api/workspaces/:id route, per-row pencil affordance in switcher (visible only when caller can_admin), small rename modal with name + slug fields, validation (name <=80 chars, slug ^[a-z0-9]+(?:-[a-z0-9]+)*$ <=60 chars, blank slug -> NULL), 409 on per-org slug collision. Permission gating via new canAdminWorkspace(db, user, ws) helper in lib/permissions.js - reused-ready for future Phase 3 admin actions. /me query now joins organization_members to compute can_admin per accessible_workspaces entry. Drive-by fixes surfaced: (1) activityLogger method filter was missing PATCH, added; (2) routes that operate on a target workspace by URL param need to stamp req.workspaceId from the param so activityLogger captures the right tenant attribution - documented in the route. Smoke fixture: switcher-test@local.test is workspace_admin of Studio A and workspace_editor of Field Crew (no org_owner) so the can_admin true/false split is exercised in one login. | ||
|
ScreenTinker
|
0c91390e56 |
fix(frontend): workspace switcher (Phase 3 MVP) + SW network-first migration + platform_admin accessible_workspaces expansion + static render CSS cleanup. The switcher adds a sidebar dropdown for users who are members of multiple workspaces, renders as static text with a 'Workspace' label for single-workspace users, and muted 'No workspace' for zero. Uses existing /api/auth/me's accessible_workspaces and POST /api/auth/switch-workspace endpoints. Platform admin / superadmin users now see all workspaces in accessible_workspaces (closing the known regression from 88d91b1) via a LEFT JOIN that preserves workspace_role semantics (null = acting-as, role string = direct member). No cap on the list - deliberate for now, revisit at 50+ workspaces. SW fix bumps rd-admin-v1 -> rd-admin-v2 and switches fetch strategy from cache-first to network-first so the server's existing Cache-Control: no-cache + ETag headers actually get respected; preserves offline fallback. Static render CSS drops the bordered-box chrome that was making single-workspace users think the static text was clickable. Includes test fixture user switcher-test@local.test (credentials in fixture SQL header). Surfaced by semetra22 / Discord report about 'screens jumbled up' post-migration; root cause was the missing workspace switcher UI making devices in non-active workspaces appear missing.
|
||
|
ScreenTinker
|
88d91b10af |
activity_log: stop the bleeding - writer-leak fix on 3 sites (activityLogger middleware, alert service, login route) + one-time backfill of 548 NULL-workspace rows via device.workspace_id or workspace_members lookup; activity.js route migration deferred to its own slice tomorrow.
KNOWN REGRESSION (Phase 3 fix): platform_admin / superadmin no longer has cross-workspace 'see everything' view. Every route migrated tonight (2.2a-2.2m) deliberately removed the role-based bypass per design doc - cross-workspace visibility will come via dedicated admin endpoints in Phase 3, not magic role bypasses. Until Phase 3 ships, platform admins must switch-workspace to see other workspaces' data. |
||
|
ScreenTinker
|
2954fd1a84 | Phase 2.1: tenancy middleware, permission helpers, JWT workspace context, frontend + backend role-rename compat | ||
|
ScreenTinker
|
45a6800621 |
fix: log real client IPs through Cloudflare instead of CF edge
Express's req.ip was resolving to a Cloudflare edge address (e.g. 172.70.x.x) for any request fronted by Cloudflare, because trust proxy was set to '1' — that trusts the immediate hop, which IS Cloudflare. All activity_log rows from API paths captured the proxy, not the client. The WebSocket path was unaffected and recorded the real IP. Two layers of defense: 1. trust proxy now lists Cloudflare's published v4 + v6 ranges plus loopback / linklocal / uniquelocal (config/cloudflareIps.js). With this list req.ip resolves to the original client when fronted by CF, and X-Forwarded-For from any non-trusted source is ignored — so the value can't be spoofed. 2. New getClientIp(req) helper in services/activity.js prefers the CF-Connecting-IP header but only honors it when the immediate TCP peer is itself a trusted address. Same gate as trust proxy, so a visitor who hits the origin directly with a forged header is logged at their real address. Routed all five activity-log call sites (auth login success/failure, admin password reset, generic activityLogger middleware, and the in-memory rate-limiter key) through the helper. Logging-only change. No schema changes. Existing rows are not modified — fix applies to new entries going forward. Verified locally: - Bare loopback hit logs 127.0.0.1 (not a proxy address). - Helper unit cases including an untrusted peer (203.0.113.7) sending a forged CF-Connecting-IP correctly fall back to the real peer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
ScreenTinker
|
388e9e6ab8 |
Admin password reset + widget visibility fix
Password reset for other users: - New PUT /api/auth/users/:id/password endpoint - Superadmin can reset any local user; admin can reset role=user members of teams they own only (cannot reset other admins or superadmins, cannot self-reset — that goes through PUT /me with current_password) - OAuth users are excluded (no password to reset) - Rate-limited 20 req/min/IP to cap blast radius if an admin session is compromised - Explicit audit log entry "password_reset_for_user / target: <email>" on every reset; activity logger's summarizeAction never reads the password field, so the password value is not stored anywhere Frontend: Reset Password button in the Admin user table and Settings > User Management table. Shown only for local-auth users that aren't the current user; prompts for an 8+ char password. Widgets visibility fix: - routes/widgets.js had `const isAdmin = req.user.role === 'superadmin'` which mislabeled superadmin as admin and silently restricted real admins (role=admin) to seeing only their own widgets. Now matches /auth/users behavior: superadmin sees all, admin sees own + public + widgets owned by members of teams they own, user sees own + public. 7 new i18n keys (admin.reset_password, admin.prompt_reset_password, admin.toast.password_min_8, admin.toast.password_reset, and the matching settings.user.* / settings.toast.* trio). 1024 keys total, parity 100% across en/es/fr/de/pt. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
ScreenTinker
|
4392bb460a |
Add DISABLE_REGISTRATION env var to block public sign-ups
When DISABLE_REGISTRATION=true (or 1), POST /api/auth/register returns 403 with a clear error. OAuth endpoints (/google, /microsoft) also refuse to auto-create new accounts — existing OAuth users can still sign in. First-user setup (empty users table) is always allowed so a fresh install can still be initialized. GET /api/auth/config now returns registration_enabled so the login view can hide the "Create Account" button and the trial banner when registration is off. Absence of the flag is treated as enabled for back-compat with older servers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
ScreenTinker
|
52297ec618 |
Settings: add account profile + password change UI
Adds a per-user Account section in Settings with name edit and password change. Password change requires current password; local auth only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
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> |
||
|
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> |