Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Exporting the config now reports a failure instead of saving an error response
as `stackyard-config.json`.

- A session that expires while you are working in Admin now raises the sign-in
box over the page, and the interrupted save completes once you are back in,
instead of failing as "Unauthorised" until the page is reloaded.
Expand Down
17 changes: 15 additions & 2 deletions ui/js/admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
snapshotItems,
saveWithRevert,
} from '/js/admin-save-logic.js?v=77cac1d1';
import { API, toast, ag, ap, initInlineEdit, setReauthHandler } from '/js/admin-shared.js?v=bb36dbb3';
import { toast, ag, ap, initInlineEdit, setReauthHandler } from '/js/admin-shared.js?v=bb36dbb3';
import { checkAuth, requireLogin, wirePasswordStrength } from '/js/admin-auth.js?v=dd849d4c';
import { state } from '/js/admin-state.js?v=3f9ad806';
import { buildWidgetForm } from '/js/admin-widget-form.js?v=653071f0';
Expand Down Expand Up @@ -1232,16 +1232,29 @@ function initLanguage() {
const dashSaveEl = el('dash-save');
if (dashSaveEl) dashSaveEl.onclick = () => save();

/* Fetched rather than reached by navigating a link. A link hands the request to
the browser, so the page never learns the outcome: a refusal or an expired
session saved an error body under the backup's own filename, and the catch
here could not see it. Going through `ag` also means an expired session
raises the sign-in box and the export finishes afterwards. */
el('btn-exp').onclick = async () => {
let url;
try {
const config = await ag('/api/config/export');
const blob = new Blob([JSON.stringify(config, null, 2)], { type: 'application/json' });
url = URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = API + '/api/config/export';
a.href = url;
a.download = 'stackyard-config.json';
document.body.appendChild(a);
a.click();
document.body.removeChild(a);
} catch (e) {
toast(t('toast.exportFailed', { err: e.message }), 'err');
} finally {
/* Revoked on the next frame: revoking it in this one races the download the
click just started, and the browser keeps nothing to save. */
if (url) requestAnimationFrame(() => URL.revokeObjectURL(url));
}
};
el('imp').onchange = async e => {
Expand Down
65 changes: 65 additions & 0 deletions ui/test/config-export.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/* Regression test for the config export, which could not report a failure.

The export set an anchor's href to /api/config/export and clicked it, which
hands the request to the browser. The page then never learns the outcome: a
refusal, a server error or an expired session was written to disk as an error
body under the backup's own filename, and the try/catch around the click
could not observe any of it. The failure surfaces when the backup is needed,
which is the worst moment to find out.

Fetching it instead means the response is checked before anything is saved,
and going through `ag` means an expired session raises the sign-in box and
the export finishes afterwards.

Scanned from the source rather than driven, because the download itself is
browser machinery: what matters is which of the two routes the code takes. */

import { test } from 'node:test';
import assert from 'node:assert/strict';
import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';

const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
const admin = fs.readFileSync(path.join(root, 'js/admin.js'), 'utf8');

/* The handler only, so an unrelated mention of the endpoint elsewhere in the
file cannot pass or fail this. */
function exportHandler() {
const start = admin.indexOf("el('btn-exp').onclick");
assert.notEqual(start, -1, 'the export handler moved; this test needs updating');
const end = admin.indexOf("el('imp').onchange", start);
assert.notEqual(end, -1, 'the export handler moved; this test needs updating');
return admin.slice(start, end);
}

test('the export never reaches the endpoint by navigating to it', () => {
const body = exportHandler();
assert.ok(
!/\.href\s*=\s*[^;]*\/api\/config\/export/.test(body),
'assigning the endpoint to an href hands the request to the browser, which cannot report a failure',
);
});

test('the export goes through the shared helper, so a failure is visible', () => {
const body = exportHandler();
assert.match(
body,
/await ag\(\s*'\/api\/config\/export'\s*\)/,
'the response has to be checked before anything is saved',
);
});

test('the export reports a failure it can now see', () => {
assert.match(exportHandler(), /toast\(\s*t\('toast\.exportFailed'/);
});

test('the object URL is released rather than leaked', () => {
const body = exportHandler();
assert.match(body, /createObjectURL/);
assert.match(body, /revokeObjectURL/, 'a blob URL held for the page lifetime pins the whole config in memory');
});

test('the download keeps the filename users already know', () => {
assert.match(exportHandler(), /download\s*=\s*'stackyard-config\.json'/);
});
Loading