Archive/screentinker
1
0
Fork 0
mirror of https://github.com/screentinker/screentinker.git synced 2026-08-13 13:53:12 -06:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

Fix: the router discarded every SSO return, so single sign-on could never complete

Browse source 
THE CRITICAL ONE. The server ends every SSO login by redirecting to `#/login?sso=1`
(claim the session) or `#/login?sso_error=<code>` (say what went wrong). The router
compared the hash EXACTLY against '#/login' in three places, so an unauthenticated
browser — the only kind that ever arrives there — had the hash rewritten to a bare
'#/login' and the query was gone before the login view ran.

  - a user who authenticated perfectly at their IdP landed back on a clean login page,
    still signed out, with no message: /api/auth/sso/claim was never called
  - all 16 error codes rendered SILENCE — not a raw key, not "undefined", nothing to
    report or search for
  - it took the pre-existing ?verified=1 email-verification toast with it

The comment above the reset-password exclusion describes this exact bug class and was
never extended to the login route. It is now, in all three places: the auth redirect,
the render dispatch, and the no-workspace guard.

Verified in real Chrome: 16/16 codes render a real sentence, and ?sso=1 now reaches
POST /api/auth/sso/claim.

Also, on a server with NO SSO configured, confirmed in the browser that the login page
is exactly what it was before any of this work: email, password, Sign In, Forgot
password, zero SSO buttons, no single sign-on wording, plain local login issues a
session, no page errors.

And fixes MY preflight, which pruned devDependencies as a side effect of BOOTING:
`npm install --omit=dev` reconciles the whole tree, so merely starting the server
deleted socket.io-client, puppeteer-core and js-yaml and broke `npm test`. A reviewer
watched it happen. It now installs only the named missing packages, with --no-save —
a boot-time repair that quietly removes packages is worse than the failure it fixes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
This commit is contained in:
ScreenTinker 2026-08-10 22:51:27 -05:00
parent 601b526264
commit 901e664591
4 changed files with 40 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
frontend/js/app.js
View file

@ -248,7 +248,7 @@ async function refreshCurrentUser() {
// a redirect loop.
const hash = window.location.hash || '#/';
if (hasNoAccessibleWorkspace(fresh)
&& hash !== '#/no-workspace' && hash !== '#/login' && hash !== '#/change-password') {
&& hash !== '#/no-workspace' && !hash.startsWith('#/login') && hash !== '#/change-password') {
window.location.hash = '#/no-workspace';
}
} catch {}
@ -338,14 +338,29 @@ function route() {
// do nothing. The login view reads the token off the hash and shows the new-password form.
const isResetRoute = hash.startsWith('#/reset-password');
/*
* ⚠️ The SAME rule the comment above states, for the login route.
*
* The server finishes every single sign-on by redirecting to `#/login?sso=1` (claim the session)
* or `#/login?sso_error=<code>` (say what went wrong). Matching the hash EXACTLY meant neither
* survived: an unauthenticated browser — the only kind that arrives here — had the hash rewritten
* to a bare `#/login` and the query was gone before the login view ever ran. So a user who
* authenticated perfectly at their identity provider landed back on a clean login page, still
* signed out, with no message; and all sixteen error codes rendered SILENCE, which is worse than
* a wrong message because there is nothing to report or search for.
*
* It took the pre-existing `?verified=1` email-verification toast with it.
*/
const isLoginRoute = hash === '#/login' || hash.startsWith('#/login?');
// Auth check - redirect to login if not authenticated
if (!isAuthenticated() && hash !== '#/login' && !isResetRoute) {
if (!isAuthenticated() && !isLoginRoute && !isResetRoute) {
window.location.hash = '#/login';
return;
}
// If authenticated and on login page, redirect to dashboard or onboarding
if (isAuthenticated() && (hash === '#/login' || isResetRoute)) {
if (isAuthenticated() && (isLoginRoute || isResetRoute)) {
window.location.hash = localStorage.getItem('rd_onboarded') ? '#/' : '#/onboarding';
return;
}
@ -422,8 +437,10 @@ function route() {
return;
}
// Login page (and password-reset links from email) - hide sidebar
if (hash === '#/login' || isResetRoute) {
// Login page (and password-reset links from email) - hide sidebar.
// Matches `#/login?...` too: the single sign-on return carries `?sso=1` / `?sso_error=<code>`,
// and an exact comparison meant the login view was never rendered for either.
if (isLoginRoute || isResetRoute) {
sidebar.style.display = 'none';
app.style.marginLeft = '0';
const mb = document.getElementById('mobileMenuBtn');

1
frontend/js/i18n/en.js
View file

@ -214,6 +214,7 @@ export default {
// Both of these used to fall through to "please try again", which is advice that can never work:
// retrying is exactly what will not help, and the user needs to be told who to talk to instead.
'auth.sso_required': 'Your organization requires single sign-on. Use \u201cContinue with single sign-on\u201d above \u2014 your password will not work here.',
'auth.sso_err_sso_required': 'Your organization requires its own single sign-on. Use the single sign-on option for your organization.',
'auth.sso_err_domain_not_allowed': 'Your organization has not verified that email domain for sign-in. Ask your administrator to verify it in ScreenTinker.',
'auth.sso_err_account_exists_other_provider': 'An account with this email already exists and signs in through a different provider. Use that provider, or ask your administrator.',
'auth.signin_microsoft': 'Sign in with Microsoft',

2
frontend/js/views/login.js
View file

@ -637,7 +637,7 @@ function setupHandlers(config, isSetup) {
const known = ['expired', 'bad_state', 'no_code', 'no_email', 'email_unverified',
'verification_failed', 'provider_refused', 'provider_unavailable', 'unknown_provider',
'registration_disabled', 'account_exists_local', 'subject_mismatch', 'server_error',
'domain_not_allowed', 'account_exists_other_provider'];
'domain_not_allowed', 'account_exists_other_provider', 'sso_required'];
const key = known.includes(ssoError) ? `auth.sso_err_${ssoError}` : 'auth.sso_failed';
showToast(t(key), 'error');
}

18
server/lib/preflight-deps.js
View file

@ -109,8 +109,22 @@ function preflight() {
* throw away a working tree to fix one missing package.
*/
const hasLock = fs.existsSync(path.join(SERVER_DIR, 'package-lock.json'));
if (hasLock && nodeModulesAbsent) run(['ci', '--omit=dev', '--no-audit', '--no-fund'], 'installing');
else run(['install', '--omit=dev', '--no-audit', '--no-fund'], 'installing');
if (hasLock && nodeModulesAbsent) {
// Nothing installed, so `ci` has nothing to destroy and gives a reproducible tree.
run(['ci', '--omit=dev', '--no-audit', '--no-fund'], 'installing');
} else {
/*
* ⚠️ Install ONLY what is missing, by name, and never `--omit=dev` on a populated tree.
*
* `npm install --omit=dev` reconciles the whole tree, which PRUNES devDependencies — so
* merely starting the server deleted socket.io-client, puppeteer-core and js-yaml, and broke
* `npm test`. A review watched it happen. A boot-time repair that quietly removes packages
* is worse than the failure it fixes, so this touches nothing it was not asked to.
*
* `--no-save` because a server starting up has no business editing package.json.
*/
run(['install', '--no-save', '--no-audit', '--no-fund', ...missing], 'installing missing packages');
}
} catch (e) {
fail(`could not install dependencies: ${e && e.message}`,
'Run `npm ci --omit=dev` in the server directory, or check network access to the npm registry.');