-
Notifications
You must be signed in to change notification settings - Fork 9
feat(server): appPaths — WAC-exempt application mount points (#582) #585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
7e76bc2
feat(server): appPaths — WAC-exempt application mount points (#582)
melvincarvalho 8ad6e9e
review: normalize appPaths entries; docs example mounts the bare prefix
melvincarvalho b58cc9b
review(docs): appPaths requests never carry request.webId
melvincarvalho File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| /** | ||
| * appPaths — WAC-exempt application mount points (#582). | ||
| * | ||
| * The global WAC preHandler authorizes every URL against pod ACLs and | ||
| * rejects before routes run; the only escapes are its hardcoded prefix | ||
| * list (/storage/, /db, /mcp, …). That means a third-party app plugin | ||
| * registering routes on the returned fastify instance (#206's plugin-zero | ||
| * pattern, e.g. a game mounted at /tideholm) has its POSTs swallowed by | ||
| * WAC with no way to opt out. | ||
| * | ||
| * createServer({ appPaths: ['/myapp'] }) declares URL prefixes owned by | ||
| * registered applications: requests at or below an app path skip the WAC | ||
| * hook, and the app owns authentication and authorization under its | ||
| * prefix — exactly the deal the bundled pseudo-plugins already have. | ||
| * | ||
| * Tests verify the seam end-to-end: an app route mounted on the returned | ||
| * instance receives unauthenticated requests (WAC stays out), sibling LDP | ||
| * paths keep full WAC enforcement, and malformed appPaths entries are | ||
| * dropped rather than becoming accidental holes. | ||
| */ | ||
|
|
||
| import { describe, it, before, after, afterEach } from 'node:test'; | ||
| import assert from 'node:assert'; | ||
| import { createServer } from '../src/server.js'; | ||
| import fs from 'fs-extra'; | ||
|
|
||
| const TEST_DATA_DIR = './test-data-app-paths'; | ||
|
|
||
| let server; | ||
| let baseUrl; | ||
| let originalDataRoot; | ||
|
|
||
| async function startWith(appPaths) { | ||
| await fs.emptyDir(TEST_DATA_DIR); | ||
| server = createServer({ | ||
| logger: false, | ||
| forceCloseConnections: true, | ||
| root: TEST_DATA_DIR, | ||
| appPaths, | ||
| }); | ||
| // An app in the #206 plugin-zero shape: routes registered on the returned | ||
| // instance, answering with its own status codes (its own "auth"). | ||
| server.all('/myapp', echo); | ||
| server.all('/myapp/*', echo); | ||
| async function echo(request, reply) { | ||
| reply.code(200).send({ | ||
| app: true, | ||
| method: request.method, | ||
| url: request.url, | ||
| webId: request.webId ?? null, // hook skipped -> never set | ||
| }); | ||
| } | ||
| await server.listen({ port: 0, host: '127.0.0.1' }); | ||
| const address = server.server.address(); | ||
| baseUrl = `http://127.0.0.1:${address.port}`; | ||
| } | ||
|
|
||
| describe('appPaths application mount points (#582)', () => { | ||
| before(() => { | ||
| // createServer({ root }) mutates process.env.DATA_ROOT; snapshot so the | ||
| // test dir doesn't leak into later suites. | ||
| originalDataRoot = process.env.DATA_ROOT; | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| if (server) { | ||
| await server.close(); | ||
| server = null; | ||
| } | ||
| await fs.remove(TEST_DATA_DIR); | ||
| }); | ||
|
|
||
| after(() => { | ||
| if (originalDataRoot === undefined) delete process.env.DATA_ROOT; | ||
| else process.env.DATA_ROOT = originalDataRoot; | ||
| }); | ||
|
|
||
| it('unauthenticated POST below an app path reaches the app handler', async () => { | ||
| await startWith(['/myapp']); | ||
| const res = await fetch(`${baseUrl}/myapp/api/action`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ hello: 'world' }), | ||
| }); | ||
| assert.strictEqual(res.status, 200); | ||
| const body = await res.json(); | ||
| assert.strictEqual(body.app, true); | ||
| assert.strictEqual(body.method, 'POST'); | ||
| }); | ||
|
|
||
| it('the bare app path and query-string forms are exempt too', async () => { | ||
| await startWith(['/myapp']); | ||
| for (const path of ['/myapp', '/myapp?tab=map']) { | ||
| const res = await fetch(`${baseUrl}${path}`, { method: 'POST' }); | ||
| assert.strictEqual(res.status, 200, `${path} should reach the app`); | ||
| } | ||
| }); | ||
|
|
||
| it('the WAC hook never sets request.webId on app-path requests', async () => { | ||
| await startWith(['/myapp']); | ||
| const res = await fetch(`${baseUrl}/myapp/whoami`); | ||
| const body = await res.json(); | ||
| assert.strictEqual(body.webId, null); | ||
| }); | ||
|
|
||
| it('sibling LDP paths keep full WAC enforcement', async () => { | ||
| await startWith(['/myapp']); | ||
| // Writing outside the app prefix without auth must still be rejected. | ||
| const res = await fetch(`${baseUrl}/notes.jsonld`, { | ||
| method: 'PUT', | ||
| headers: { 'Content-Type': 'application/ld+json' }, | ||
| body: JSON.stringify({ '@id': '', name: 'x' }), | ||
| }); | ||
| assert.ok(res.status === 401 || res.status === 403, | ||
| `expected WAC rejection, got ${res.status}`); | ||
| }); | ||
|
|
||
| it('a prefix match is a path-segment match, not a string prefix', async () => { | ||
| await startWith(['/myapp']); | ||
| // /myapplication must NOT be exempt just because it shares characters. | ||
| const res = await fetch(`${baseUrl}/myapplication.jsonld`, { | ||
| method: 'PUT', | ||
| headers: { 'Content-Type': 'application/ld+json' }, | ||
| body: JSON.stringify({ '@id': '', name: 'x' }), | ||
| }); | ||
| assert.ok(res.status === 401 || res.status === 403, | ||
| `expected WAC rejection, got ${res.status}`); | ||
| }); | ||
|
|
||
| it('trailing-slash entries are normalized, children still exempt', async () => { | ||
| await startWith(['/myapp/']); | ||
| const res = await fetch(`${baseUrl}/myapp/api/action`, { method: 'POST' }); | ||
| assert.strictEqual(res.status, 200); | ||
| }); | ||
|
|
||
| it('malformed appPaths entries are dropped, not accidental holes', async () => { | ||
| // No leading slash and bare '/' are both invalid; with them filtered out | ||
| // the route registrations still exist but WAC fires first. | ||
| await startWith(['myapp', '/', '///', ' ']); | ||
| const res = await fetch(`${baseUrl}/myapp/api/action`, { method: 'POST' }); | ||
| assert.ok(res.status === 401 || res.status === 403, | ||
| `expected WAC rejection (invalid entries dropped), got ${res.status}`); | ||
| }); | ||
|
|
||
| it('omitting appPaths changes nothing (default off)', async () => { | ||
| await startWith(undefined); | ||
| const res = await fetch(`${baseUrl}/myapp/api/action`, { method: 'POST' }); | ||
| assert.ok(res.status === 401 || res.status === 403, | ||
| `expected WAC rejection with no appPaths, got ${res.status}`); | ||
| }); | ||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added — the section now states that request.webId is never set under an app path (the WAC hook is what populates it) and points app authors at getWebIdFromRequestAsync, with #584 tracking the stable public accessor. It was already pinned by the 'never sets request.webId' test; now the docs say it too.