Link start cannot be navigated to: a bearer token does not survive it

"Authentication required" on every click of Link. The Settings button did
`location.href = /api/auth/oidc/<slug>/link/start`, which is a top-level
navigation -- and this app's session lives in localStorage and travels as an
Authorization header, so the request arrived anonymous and requireAuth refused
it, correctly.

The login /start route works precisely because it needs no session. Copying its
shape for a route that does need one was the mistake.

The client now FETCHES link start with its token and navigates to the URL it
returns. The transaction cookie is still set by that response, because a
same-origin fetch stores Set-Cookie normally, so the callback is unchanged.
beginOidc grew an asJson flag rather than a second copy of the PKCE/state/nonce
setup, so login and link still cannot drift apart.

Both mutations fail the new test: navigating straight at the route, and having
the server redirect instead of answering with JSON.
This commit is contained in:
ScreenTinker 2026-08-12 14:44:23 -05:00
parent ffceaf2c1f
commit 3617a1a116
4 changed files with 53 additions and 7 deletions

View file

@ -213,6 +213,9 @@ export const api = {
// Unlink an instance-wide SSO provider. The new password is required in the same call:
// the account must never sit between credentials.
ssoUnlink: (password) => request('/auth/oidc/unlink', { method: 'POST', body: JSON.stringify({ password }) }),
// Returns { url } to navigate to. Fetched rather than navigated to, because the session is
// a bearer token and a top-level navigation cannot carry one.
ssoLinkStart: (slug) => request(`/auth/oidc/${encodeURIComponent(slug)}/link/start`),
totpSetup: () => request('/auth/totp/setup', { method: 'POST' }),
totpEnable: (code) => request('/auth/totp/enable', { method: 'POST', body: JSON.stringify({ code }) }),
totpDisable: (code) => request('/auth/totp/disable', { method: 'POST', body: JSON.stringify({ code }) }),

View file

@ -607,12 +607,20 @@ export async function render(container) {
</div>
`);
block.querySelectorAll('[data-link-slug]').forEach((btn) => {
btn.onclick = () => {
btn.onclick = async () => {
const slug = btn.dataset.linkSlug;
const name = providers.find((p) => p.slug === slug)?.name || slug;
// Deliberately blunt: the password is destroyed, and that is the part people miss.
if (!window.confirm(t('settings.signin_link_warning', { provider: name }))) return;
window.location.href = `/api/auth/oidc/${encodeURIComponent(slug)}/link/start`;
/*
* Fetch the authorize URL, then navigate to it. NOT location.href straight at the start
* route: the session is a bearer token in localStorage, so a top-level navigation arrives
* with no Authorization header and is refused as anonymous.
*/
try {
const { url } = await api.ssoLinkStart(slug);
window.location.href = url;
} catch (e) { showToast(e.message, 'error'); }
};
});
}

View file

@ -1248,7 +1248,7 @@ router.post('/sso/start', express.urlencoded({ extended: false }), (req, res) =>
* account a link is for. Login and link therefore share one flow the same PKCE, state, nonce and
* verification instead of a second copy that drifts.
*/
async function beginOidc(req, res, provider, extra = {}, onError = backToApp) {
async function beginOidc(req, res, provider, extra = {}, onError = backToApp, asJson = false) {
try {
const doc = await oidc.discover(provider.issuer);
const pkce = oidc.createPkce();
@ -1279,9 +1279,20 @@ async function beginOidc(req, res, provider, extra = {}, onError = backToApp) {
url.searchParams.set('nonce', nonce);
url.searchParams.set('code_challenge', pkce.challenge);
url.searchParams.set('code_challenge_method', pkce.method);
/*
* A LINK start is fetched, not navigated to.
*
* The session lives in localStorage and travels as an Authorization header, so a top-level
* `location.href` to an authenticated route arrives anonymous which is exactly how this first
* shipped, and it 401'd every time. The caller therefore fetches this with its token and gets
* the authorize URL back to navigate to itself. The transaction cookie is still set by this
* response, because a same-origin fetch stores Set-Cookie normally.
*/
if (asJson) return res.json({ url: url.toString() });
res.redirect(url.toString());
} catch (err) {
console.error(`[oidc] ${provider.slug} start failed:`, err.message);
if (asJson) return res.status(502).json({ error: 'The provider could not be reached' });
onError(res, { sso_error: 'provider_unavailable' });
}
}
@ -1333,9 +1344,12 @@ router.post('/oidc/unlink', requireAuth, (req, res) => {
router.get('/oidc/:slug/link/start', requireAuth, asyncRoute(async (req, res) => {
const provider = oidcProviders.get(req.params.slug);
if (!provider) return backToSettings(res, { sso_error: 'unknown_provider' });
if (provider.organizationId) return backToSettings(res, { sso_error: 'not_linkable' });
await beginOidc(req, res, provider, { link: req.user.id }, backToSettings);
if (!provider) return res.status(404).json({ error: 'Unknown provider' });
if (provider.organizationId) {
return res.status(400).json({ error: 'Only this server\'s own providers can be linked' });
}
// JSON, not a redirect — see beginOidc. The browser cannot send a bearer token on a navigation.
await beginOidc(req, res, provider, { link: req.user.id }, backToSettings, true);
}));
router.get('/oidc/:slug/callback', asyncRoute(async (req, res) => {

View file

@ -40,7 +40,7 @@ test('link start requires authentication and refuses org providers', () => {
const body = handler('get', '/oidc/:slug/link/start');
assert.match(AUTH, /router\.get\('\/oidc\/:slug\/link\/start', requireAuth/,
'the link must be startable only by someone already signed in — that is the proof of ownership');
assert.match(body, /provider\.organizationId.*not_linkable/s,
assert.match(body, /provider\.organizationId[\s\S]{0,160}status\(400\)/,
"an organization's provider must never attach itself to a platform account");
assert.match(body, /link: req\.user\.id/,
'the account must come from the session, not from anything the browser can set');
@ -89,6 +89,27 @@ test('link failures return to Settings, not the login page', () => {
assert.match(AUTH, /function backToSettings\(res, params\)[\s\S]{0,200}#\/settings/);
});
test('link start answers with JSON, because a navigation cannot carry a bearer token', () => {
/*
* Shipped broken once: the Settings button did `location.href = .../link/start`, which is a
* top-level navigation. The session lives in localStorage and travels as an Authorization header,
* so the request arrived anonymous and requireAuth refused it "Authentication required" on
* every click. The client must FETCH this with its token and navigate to the returned URL.
*/
const body = handler('get', '/oidc/:slug/link/start');
assert.match(body, /beginOidc\([^)]*backToSettings, true\)/,
'link start must run in JSON mode');
assert.match(AUTH, /if \(asJson\) return res\.json\(\{ url: url\.toString\(\) \}\);/,
'JSON mode must return the authorize URL rather than a 302');
const settings = require('fs').readFileSync(
require('path').join(__dirname, '..', '..', 'frontend', 'js', 'views', 'settings.js'), 'utf8');
assert.match(settings, /await api\.ssoLinkStart\(slug\)/,
'the client must fetch the start route so its Authorization header is sent');
assert.doesNotMatch(settings, /location\.href = `\/api\/auth\/oidc/,
'never navigate straight at the authenticated start route');
});
test('login and link share one flow, so verification cannot drift between them', () => {
// beginOidc is the single place PKCE/state/nonce are minted; both entry points call it.
assert.match(AUTH, /async function beginOidc\(req, res, provider, extra = \{\}/);