Skip to content

Commit 7301b6f

Browse files
melvincarvalhomelvincarvalho
committed
PR #349: simplify — drop the upgrade auto-fallback
The user feedback on round-3 was clear: the original ask was a simple default change, and "people will figure it out" was explicit license to skip migration cleverness. Round 3's auto-fallback (and round 4's tweaks to it) were scope creep. Revert the auto-fallback. Pre-#348 installs that upgrade in place without flag changes will get a fresh empty root pod alongside their existing /me/ data — operators who hit that pick one of two explicit paths on restart, both documented: 1. Add `--single-user-name me` → legacy layout, no further work. 2. Move `<root>/me/*` to `<root>/`, delete the legacy IDP account for "me", restart → new root pod, new account seeded. At v0.0.x with a small operator base, that one-time intervention is acceptable and keeps the codebase clean. Changes from round 3 / unpushed round 4: - src/server.js: drop the `existsSync(/me/...)` auto-detect block, the `migratedFromMeDefault` flag, and the corresponding warning in onReady. Drop the `existsSync` import. - test/idp.test.js: drop the `Single-user upgrade fallback` suite (no fallback to test). - test/idp.test.js: tighten the `seeds the profile at root` assertion — also fs.pathExists() check that no /me/ files were written, so a regression that left /me/ behind under a 401 wouldn't slip through (round-4 review #7). - docs/configuration.md: add an explicit "Upgrading from a pre-#348 install" callout under Single-User Mode listing both migration paths (round-4 review #11). 593/593 tests pass.
1 parent 13ce87b commit 7301b6f

3 files changed

Lines changed: 19 additions & 126 deletions

File tree

‎docs/configuration.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,10 @@ JSS_SINGLE_USER=true jss start --idp
285285
- Login works for the single user via password (`POST /idp/credentials`) or any other configured method
286286
- Proper ACLs generated automatically
287287

288+
**Upgrading from a pre-#348 install:** if your existing pod was created with the old default (data lives under `<root>/me/`), JSS no longer auto-detects it — restarting plain `jss start --single-user` will start seeding a fresh empty root pod alongside your legacy `/me/` data, and your existing IDP account will keep authenticating against `/me/`. Pick one path on the next restart:
289+
- Add `--single-user-name me` to keep the legacy `/me/` layout exactly as before.
290+
- Move `<root>/me/*` to `<root>/`, delete the IDP account for `me` (so the new root pod's `me` account can be seeded), then restart without the name flag.
291+
288292
**Initial password sources, in priority order:**
289293
1. `--single-user-password <pw>` CLI flag
290294
2. `JSS_SINGLE_USER_PASSWORD` env var

‎src/server.js‎

Lines changed: 8 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import Fastify from 'fastify';
22
import rateLimit from '@fastify/rate-limit';
33
import { readFile } from 'fs/promises';
4-
import { existsSync } from 'fs';
54
import { join, dirname } from 'path';
65
import { fileURLToPath } from 'url';
76
import { handleGet, handleHead, handlePut, handleDelete, handleOptions, handlePatch } from './handlers/resource.js';
@@ -98,34 +97,18 @@ export function createServer(options = {}) {
9897
// to mount the pod at /<name>/ instead. Normalize the
9998
// historical `'/'` / `''` forms to null up front so downstream
10099
// code (remoteStoragePlugin, decorators, etc.) doesn't have to
101-
// re-check for the same three shapes — see PR #349 review.
100+
// re-check for the same three shapes.
101+
//
102+
// Pre-#348 installs (default 'me') that upgrade in place will see
103+
// a fresh empty root pod alongside their /me/ data. The fix is to
104+
// pass `--single-user-name me` on restart (or move data/me/* out
105+
// to the data root). At v0.0.x we accept that one-time
106+
// intervention rather than carrying detection magic in the code.
102107
const rawSingleUserName = options.singleUserName ?? null;
103-
const normalizedName =
108+
const singleUserName =
104109
(rawSingleUserName === '/' || rawSingleUserName === '')
105110
? null
106111
: rawSingleUserName;
107-
108-
// #348 backwards-compat for in-place upgrades: if the operator
109-
// didn't pass --single-user-name and a /me/ pod from a pre-#348
110-
// install already exists on disk, fall back to the legacy 'me'
111-
// name so the existing pod and IDP account stay live. Without
112-
// this, the new default would seed an empty root pod alongside
113-
// the still-served /me/ data — a split-brain state where the
114-
// operator has no way to log in to the new pod (the 'me' IDP
115-
// account still points at /me/profile/card) and clients keep
116-
// reading/writing the legacy one. The disk check is sync because
117-
// remoteStoragePlugin captures the username at registration time
118-
// (before onReady fires).
119-
const dataRoot = options.root || process.env.DATA_ROOT || './data';
120-
let singleUserName = normalizedName;
121-
let migratedFromMeDefault = false;
122-
if (singleUser && singleUserName === null) {
123-
if (existsSync(join(dataRoot, 'me/profile/card.jsonld')) ||
124-
existsSync(join(dataRoot, 'me/profile/card'))) {
125-
singleUserName = 'me';
126-
migratedFromMeDefault = true;
127-
}
128-
}
129112
const singleUserPassword = options.singleUserPassword ?? null;
130113
// Default storage quota per pod (50MB default, 0 = unlimited)
131114
const defaultQuota = options.defaultQuota ?? 50 * 1024 * 1024;
@@ -612,20 +595,6 @@ export function createServer(options = {}) {
612595
const webId = `${podUri}${profileFile}#me`;
613596
const profileExists = hasJsonLd || hasLegacy;
614597

615-
if (migratedFromMeDefault) {
616-
// Surface the auto-fallback once at startup so operators
617-
// know why their pod is at /me/ instead of /. This is
618-
// backwards-compat for pre-#348 installs — the new default
619-
// (root pod) only kicks in when no /me/ data exists.
620-
fastify.log.warn(
621-
'Detected pre-existing /me/ pod data. Falling back to ' +
622-
'--single-user-name me for backwards compatibility (#348 ' +
623-
'changed the default pod path from /me/ to /). To opt into ' +
624-
'the new default root pod, move data/me/* to data/* and ' +
625-
'remove the IDP account for "me" before restarting.'
626-
);
627-
}
628-
629598
if (!profileExists) {
630599
fastify.log.info(`Creating single-user pod at ${podUri}...`);
631600

‎test/idp.test.js‎

Lines changed: 7 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -498,9 +498,13 @@ describe('Single-user default — root pod (#348)', () => {
498498
const root = await fetch(`${baseUrl}/profile/card.jsonld`);
499499
assert.strictEqual(root.status, 200,
500500
'--single-user with no name should default to a root pod');
501-
const me = await fetch(`${baseUrl}/me/profile/card.jsonld`);
502-
assert.notStrictEqual(me.status, 200,
503-
'no /me/ pod should be served when singleUserName is unset (got 200)');
501+
// Check the filesystem directly — an HTTP-only check could pass
502+
// on a 401 even if /me/ data was somehow seeded, which would
503+
// hide the regression we care about (root vs /me/ pod).
504+
assert.strictEqual(await fs.pathExists(path.join(DEFAULT_DATA_DIR, 'me/profile/card.jsonld')), false,
505+
'no /me/ pod files should be created when singleUserName is unset');
506+
assert.strictEqual(await fs.pathExists(path.join(DEFAULT_DATA_DIR, 'me/profile/card')), false,
507+
'no legacy /me/ pod files should be created either');
504508
});
505509

506510
it('WebID resolves at the server origin', async () => {
@@ -532,90 +536,6 @@ describe('Single-user default — root pod (#348)', () => {
532536
});
533537
});
534538

535-
// #348 review round 3: starting against a data dir that already has
536-
// a pre-#348 `/me/` pod — without the operator passing
537-
// --single-user-name — must NOT seed a fresh empty root pod
538-
// alongside it. The auto-fallback should detect /me/ and keep
539-
// serving from there, preserving the legacy WebID and IDP account.
540-
describe('Single-user upgrade fallback — pre-existing /me/ pod (#348)', () => {
541-
let server;
542-
let baseUrl;
543-
const UPGRADE_DATA_DIR = './test-data-348-upgrade-from-me';
544-
const LEGACY_PASSWORD = 'legacy-pre-348-pw';
545-
546-
before(async () => {
547-
await fs.remove(UPGRADE_DATA_DIR);
548-
await fs.ensureDir(UPGRADE_DATA_DIR);
549-
550-
// Reuse the same port across phases. ACL bodies seeded in
551-
// phase 1 carry absolute URIs with the host+port; if phase 2
552-
// bound a different port, the ACL's `acl:accessTo` would no
553-
// longer match the requested resource and access checks would
554-
// fail spuriously — that's a test-setup artifact, not a real
555-
// upgrade bug.
556-
const port = await getAvailablePort();
557-
baseUrl = `http://${TEST_HOST}:${port}`;
558-
559-
// Phase 1: spin up a server in the *old* shape (explicit
560-
// --single-user-name me) so the seeding pipeline produces a
561-
// realistic pre-#348 layout — pod at /me/, IDP account "me"
562-
// pointing at /me/profile/card.jsonld#me.
563-
let s = createServer({
564-
logger: false,
565-
root: UPGRADE_DATA_DIR,
566-
idp: true,
567-
idpIssuer: baseUrl,
568-
singleUser: true,
569-
singleUserName: 'me',
570-
singleUserPassword: LEGACY_PASSWORD,
571-
forceCloseConnections: true,
572-
});
573-
await s.listen({ port, host: TEST_HOST });
574-
await s.close();
575-
576-
// Phase 2: restart against the same data dir without the name
577-
// flag — this is the in-place upgrade path.
578-
server = createServer({
579-
logger: false,
580-
root: UPGRADE_DATA_DIR,
581-
idp: true,
582-
idpIssuer: baseUrl,
583-
singleUser: true,
584-
// singleUserName intentionally omitted.
585-
forceCloseConnections: true,
586-
});
587-
await server.listen({ port, host: TEST_HOST });
588-
});
589-
590-
after(async () => {
591-
await server.close();
592-
await fs.remove(UPGRADE_DATA_DIR);
593-
});
594-
595-
it('keeps serving the existing /me/ profile (no fresh root pod)', async () => {
596-
const me = await fetch(`${baseUrl}/me/profile/card.jsonld`);
597-
assert.strictEqual(me.status, 200,
598-
'pre-existing /me/ pod should remain reachable after upgrade');
599-
// The auto-fallback should NOT have seeded a separate root pod.
600-
// We can't reliably test "the file at / does not exist" via HTTP
601-
// (WAC may rewrite to 401), but we can verify the auto-fallback
602-
// path was taken by checking that /me/'s WebID is still the one
603-
// bound to the IDP account — the login probe below verifies that.
604-
});
605-
606-
it('legacy "me" login still works against /me/profile/card.jsonld#me', async () => {
607-
const res = await fetch(`${baseUrl}/idp/credentials`, {
608-
method: 'POST',
609-
headers: { 'Content-Type': 'application/json' },
610-
body: JSON.stringify({ username: 'me', password: LEGACY_PASSWORD }),
611-
});
612-
assert.strictEqual(res.status, 200,
613-
'pre-existing IDP account for "me" should still authenticate');
614-
const body = await res.json();
615-
assert.ok(body.access_token, 'response should carry an access token');
616-
});
617-
});
618-
619539
describe('Identity Provider - Accounts', () => {
620540
let server;
621541
let accountsUrl;

0 commit comments

Comments
 (0)