fix: "Sign in with Nextcloud" opens the sign-in page
The button did nothing. It claimed a blank tab during the click and pointed it at the login URL once the request returned — the standard way around a popup blocker, and it cannot work in this app: helmet sends Cross-Origin-Opener-Policy: same-origin, which severs the handle to that tab the moment it goes cross-origin. Assigning its location was a no-op. A blank tab opened, nothing else happened. The handle was never needed. window.open with 'noopener' asks for none, and a click's user activation outlives the fetch, so the browser does not treat it as a popup. The status line now also carries the sign-in URL as an ordinary link, so there is a way through whatever any particular browser decides about opening windows. Verified the server side against the real Nextcloud first: the flow starts, both returned URLs pass the SSRF guard and the same-host check. The fault was entirely in the browser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
parent
89e71b3b86
commit
c68e3a6219
2 changed files with 61 additions and 17 deletions
|
|
@ -64,25 +64,46 @@
|
||||||
});
|
});
|
||||||
|
|
||||||
// ── Sign in with Nextcloud ────────────────────────────────────────────
|
// ── Sign in with Nextcloud ────────────────────────────────────────────
|
||||||
// Nextcloud's own login flow. The tab is opened from the click itself, before
|
// Nextcloud's own login flow: ask our server to start one, send the person to
|
||||||
// any await, or a popup blocker eats it — the request that fetches the URL is
|
// the URL Nextcloud hands back, and poll until they finish.
|
||||||
// allowed to finish afterwards and point the already-open tab at it.
|
//
|
||||||
|
// The first version opened a blank tab during the click and pointed it at the
|
||||||
|
// URL once the request came back, which is the usual way around a popup
|
||||||
|
// blocker. It cannot work here: this app sends
|
||||||
|
// Cross-Origin-Opener-Policy: same-origin, so the handle to that tab is
|
||||||
|
// severed the moment it goes cross-origin, and assigning its location did
|
||||||
|
// nothing at all — a blank tab, and a button that looked broken.
|
||||||
|
//
|
||||||
|
// So no handle. window.open with 'noopener' needs none, and a click's user
|
||||||
|
// activation survives the fetch, so it is not treated as a popup. And the
|
||||||
|
// status line always offers the link itself, which works whatever the browser
|
||||||
|
// decides about opening windows.
|
||||||
var pollTimer = null;
|
var pollTimer = null;
|
||||||
|
|
||||||
function stopPolling(message, tone) {
|
function stopPolling(message, tone) {
|
||||||
if (pollTimer) { clearInterval(pollTimer); pollTimer = null; }
|
if (pollTimer) { clearInterval(pollTimer); pollTimer = null; }
|
||||||
|
setFlowStatus(message, tone);
|
||||||
|
}
|
||||||
|
|
||||||
|
function setFlowStatus(message, tone, link) {
|
||||||
var status = document.getElementById('nc-login-flow-status');
|
var status = document.getElementById('nc-login-flow-status');
|
||||||
if (status) {
|
if (!status) return;
|
||||||
status.textContent = message || '';
|
status.textContent = message || '';
|
||||||
status.style.color = tone === 'bad' ? 'var(--red)' : tone === 'good' ? 'var(--green)' : 'var(--g600)';
|
status.style.color = tone === 'bad' ? 'var(--red)' : tone === 'good' ? 'var(--green)' : 'var(--g600)';
|
||||||
}
|
if (!link) return;
|
||||||
|
status.appendChild(document.createTextNode(' '));
|
||||||
|
var a = document.createElement('a');
|
||||||
|
a.href = link;
|
||||||
|
a.target = '_blank';
|
||||||
|
a.rel = 'noopener noreferrer';
|
||||||
|
a.textContent = 'Open the sign-in page';
|
||||||
|
status.appendChild(a);
|
||||||
}
|
}
|
||||||
|
|
||||||
document.getElementById('btn-nc-login-flow').addEventListener('click', function () {
|
document.getElementById('btn-nc-login-flow').addEventListener('click', function () {
|
||||||
var url = (document.getElementById('nc-url').value || '').trim();
|
var url = (document.getElementById('nc-url').value || '').trim();
|
||||||
if (!url) { showToast('Enter your Nextcloud address first', 'error'); return; }
|
if (!url) { showToast('Enter your Nextcloud address first', 'error'); return; }
|
||||||
|
|
||||||
var tab = window.open('', '_blank'); // claimed while the click is still trusted
|
|
||||||
stopPolling('Opening Nextcloud…');
|
stopPolling('Opening Nextcloud…');
|
||||||
|
|
||||||
fetch('/api/nextcloud/login-flow/start', {
|
fetch('/api/nextcloud/login-flow/start', {
|
||||||
|
|
@ -91,8 +112,10 @@
|
||||||
.then(function (r) { return r.json(); })
|
.then(function (r) { return r.json(); })
|
||||||
.then(function (data) {
|
.then(function (data) {
|
||||||
if (!data.success) throw new Error(data.error || 'Could not start sign-in');
|
if (!data.success) throw new Error(data.error || 'Could not start sign-in');
|
||||||
if (tab) tab.location.href = data.loginUrl; else window.open(data.loginUrl, '_blank');
|
// Opened without a handle, so COOP has nothing to sever. If the browser
|
||||||
stopPolling('Waiting for you to finish signing in…');
|
// blocks it anyway, the link in the status line is the way through.
|
||||||
|
window.open(data.loginUrl, '_blank', 'noopener');
|
||||||
|
setFlowStatus('Waiting for you to finish signing in.', null, data.loginUrl);
|
||||||
|
|
||||||
var until = Date.now() + 10 * 60 * 1000;
|
var until = Date.now() + 10 * 60 * 1000;
|
||||||
pollTimer = setInterval(function () {
|
pollTimer = setInterval(function () {
|
||||||
|
|
@ -114,7 +137,6 @@
|
||||||
}, 3000);
|
}, 3000);
|
||||||
})
|
})
|
||||||
.catch(function (err) {
|
.catch(function (err) {
|
||||||
if (tab) tab.close();
|
|
||||||
stopPolling(err.message, 'bad');
|
stopPolling(err.message, 'bad');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -66,13 +66,35 @@ test('a flow expires, and starting again replaces the old one', () => {
|
||||||
assert.match(start, /flow\.owner === req\.user\.id && key !== handle\) loginFlows\.delete\(key\)/);
|
assert.match(start, /flow\.owner === req\.user\.id && key !== handle\) loginFlows\.delete\(key\)/);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('the tab is opened while the click is still trusted', () => {
|
test('the sign-in tab is opened without a handle, which COOP would sever', () => {
|
||||||
// Opening after an await is what a popup blocker stops.
|
// The original claimed a blank tab during the click and pointed it at the URL
|
||||||
|
// when the request came back — the usual way around a popup blocker, and
|
||||||
|
// broken here: this app sends Cross-Origin-Opener-Policy: same-origin, so the
|
||||||
|
// handle dies as soon as the tab goes cross-origin and assigning its location
|
||||||
|
// did nothing. A blank tab, and a button that looked dead.
|
||||||
const handler = ui.slice(ui.indexOf("btn-nc-login-flow"));
|
const handler = ui.slice(ui.indexOf("btn-nc-login-flow"));
|
||||||
const open = handler.indexOf("window.open('', '_blank')");
|
assert.doesNotMatch(handler, /window\.open\(''/, 'no blank tab claimed up front');
|
||||||
const fetchAt = handler.indexOf("fetch('/api/nextcloud/login-flow/start'");
|
assert.doesNotMatch(handler, /tab\.location/, 'no handle to navigate');
|
||||||
assert.ok(open > -1 && open < fetchAt, 'the tab must be claimed before the request');
|
assert.match(handler, /window\.open\(data\.loginUrl, '_blank', 'noopener'\)/);
|
||||||
assert.match(handler, /a popup blocker eats it|still trusted/);
|
});
|
||||||
|
|
||||||
|
test('the sign-in URL is also offered as a real link', () => {
|
||||||
|
// Whatever the browser decides about opening windows, there is a way through.
|
||||||
|
const handler = ui.slice(ui.indexOf("btn-nc-login-flow"));
|
||||||
|
assert.match(handler, /setFlowStatus\('Waiting for you to finish signing in\.', null, data\.loginUrl\)/);
|
||||||
|
assert.match(ui, /a\.rel = 'noopener noreferrer'/);
|
||||||
|
assert.match(ui, /a\.target = '_blank'/);
|
||||||
|
// Built as DOM, not spliced into innerHTML.
|
||||||
|
assert.match(ui, /function setFlowStatus\(message, tone, link\)[\s\S]{0,400}createElement\('a'\)/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('helmet still sends the COOP header this works around', () => {
|
||||||
|
// If this ever stops being true the workaround is harmless, but the comment
|
||||||
|
// explaining it would be wrong, and the old pattern would look safe again.
|
||||||
|
const server = read('server.js');
|
||||||
|
assert.match(server, /app\.use\(helmet\(\{/);
|
||||||
|
assert.doesNotMatch(server, /crossOriginOpenerPolicy:\s*false/,
|
||||||
|
'COOP is on by default in helmet; turning it off would need its own reasoning');
|
||||||
});
|
});
|
||||||
|
|
||||||
test('polling stops: on success, on failure, and on a deadline', () => {
|
test('polling stops: on success, on failure, and on a deadline', () => {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue