diff --git a/nodejs/routes/oauth_client.js b/nodejs/routes/oauth_client.js index 18569cc..b35213f 100644 --- a/nodejs/routes/oauth_client.js +++ b/nodejs/routes/oauth_client.js @@ -97,7 +97,7 @@ router.delete('/:client_id', async function(req, res, next) { await permission.byGroup(req.user, [ADMIN_GROUP]); const client = await OAuthClient.get(req.params.client_id); - await client.remove(); + await client.delete(); return res.json({ client_id: req.params.client_id, diff --git a/nodejs/tests/no_native_dialogs.test.js b/nodejs/tests/no_native_dialogs.test.js new file mode 100644 index 0000000..ac394ad --- /dev/null +++ b/nodejs/tests/no_native_dialogs.test.js @@ -0,0 +1,45 @@ +'use strict'; + +// Regression guard: native alert()/confirm()/prompt() calls block all further +// browser events on the page (found live, mid browser-automation testing, on +// directory.ejs's "Rotate Client Secret" — it froze the tab entirely) and are +// visually inconsistent with the rest of the UI. Every call site was removed +// in favor of app.messages.action/confirm/toast and app.modal.open; this test +// keeps it that way. + +const fs = require('fs'); +const path = require('path'); + +const ROOTS = ['views', 'public/js', 'public/lib/js'].map((d) => path.join(__dirname, '..', d)); + +// Matches a bare alert(/confirm(/prompt( call, but not app.messages.*, +// app.modal.*, or identifiers merely containing these words (e.g. +// "confirmation", ".confirmed"). +const NATIVE_DIALOG_RE = /(^|[^.\w$])(alert|confirm|prompt)\s*\(/g; + +function walk(dir) { + let files = []; + if (!fs.existsSync(dir)) return files; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) files = files.concat(walk(full)); + else if (/\.(ejs|js)$/.test(entry.name)) files.push(full); + } + return files; +} + +test('no view or client-side script calls native alert()/confirm()/prompt()', () => { + const offenders = []; + for (const root of ROOTS) { + for (const file of walk(root)) { + const src = fs.readFileSync(file, 'utf8'); + let m; + NATIVE_DIALOG_RE.lastIndex = 0; + while ((m = NATIVE_DIALOG_RE.exec(src))) { + const line = src.slice(0, m.index).split('\n').length; + offenders.push(`${path.relative(path.join(__dirname, '..'), file)}:${line} — ${m[2]}(`); + } + } + } + expect(offenders).toEqual([]); +}); diff --git a/nodejs/tests/oauth.test.js b/nodejs/tests/oauth.test.js index 5613a1f..ffd0764 100644 --- a/nodejs/tests/oauth.test.js +++ b/nodejs/tests/oauth.test.js @@ -69,6 +69,51 @@ describe('OAuth client management API — /api/oauth/client', () => { expect(res.body.results).not.toHaveProperty('client_secret_hash'); }); + test('PUT persists — a changed name survives a fresh GET', async () => { + const created = await request(app) + .post('/api/oauth/client/') + .set('auth-token', token) + .send({ name: 'put-persist-test', redirect_uris: REDIRECT_URI }); + expect(created.status).toBe(200); + const id = created.body.results.client_id; + + const updated = await request(app) + .put(`/api/oauth/client/${id}`) + .set('auth-token', token) + .send({ name: 'put-persist-test-renamed' }); + expect(updated.status).toBe(200); + expect(updated.body.results.name).toBe('put-persist-test-renamed'); + + const fetched = await request(app).get(`/api/oauth/client/${id}`).set('auth-token', token); + expect(fetched.status).toBe(200); + expect(fetched.body.results.name).toBe('put-persist-test-renamed'); + + await request(app).delete(`/api/oauth/client/${id}`).set('auth-token', token); + }); + + // Regression: this route called client.remove(), but OAuthClient wraps + // @simpleworkjs/orm's Resource model, whose instance method is .delete() + // — .remove() doesn't exist on it (unlike the model-redis Tables + // elsewhere in this app, e.g. api_token.js, which really do have + // .remove()). The route's try/catch turned the resulting TypeError into + // a plain 500 JSON response rather than a thrown exception, so every + // prior DELETE call in this file's cleanup hooks silently "succeeded" + // from Jest's point of view while leaving the client un-deleted. + test('DELETE persists — the client is actually gone, not just a 200', async () => { + const created = await request(app) + .post('/api/oauth/client/') + .set('auth-token', token) + .send({ name: 'delete-persist-test', redirect_uris: REDIRECT_URI }); + expect(created.status).toBe(200); + const id = created.body.results.client_id; + + const deleted = await request(app).delete(`/api/oauth/client/${id}`).set('auth-token', token); + expect(deleted.status).toBe(200); + + const fetched = await request(app).get(`/api/oauth/client/${id}`).set('auth-token', token); + expect(fetched.status).toBe(404); + }); + test('list then rotate a client by its returned client_id (the bootstrap path)', async () => { // Reproduces exactly what the theta-env bootstrap does: create, list, // find by name, rotate by the client_id from the list response. Uses a @@ -90,7 +135,11 @@ describe('OAuth client management API — /api/oauth/client', () => { expect(rotated.status).toBe(200); expect(rotated.body.client_secret).toBeTruthy(); - await request(app).delete(`/api/oauth/client/${found.client_id}`).set('auth-token', token); + const deleted = await request(app).delete(`/api/oauth/client/${found.client_id}`).set('auth-token', token); + expect(deleted.status).toBe(200); + + const afterDelete = await request(app).get(`/api/oauth/client/${found.client_id}`).set('auth-token', token); + expect(afterDelete.status).toBe(404); }); test('GET /:id unknown id returns 404, not 500', async () => {