diff --git a/CHANGELOG.md b/CHANGELOG.md index 01b734e..82cc4c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project are documented here. Format loosely follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); versions correspond to git tags (`vX.Y.Z`) and `nodejs/package.json`'s `version`. +## [1.3.2] - 2026-07-23 + +### Fixed +- **OAuth client management API returned `client_id: undefined` on every GET.** The ORM's `Model.toJSON()` only serializes schema fields, so the mapped `client_id`/`scopes`/`redirect_uris`/… that `OAuthClient.get()` attaches to the wrapped Resource were stripped from `GET /api/oauth/client` and `GET /api/oauth/client/:id` responses. The theta-env bootstrap (which lists clients and rotates by the returned `client_id`) then called `/api/oauth/client/undefined/rotate` and got a 500, aborting stack bring-up when `proxy-secrets.js` had no usable secret. `OAuthClient.get()` now emits an explicit public JSON shape (and deliberately omits `client_secret_hash`, so the secret hash no longer leaks over the API). +- `OAuthClient.get()` no longer 500s on an unknown/`undefined` client id: `Resource.get()` returns `null` (it doesn't throw), which was dereferenced as `r.kind`. It now returns a clean 404. + ## [1.3.1] - 2026-07-23 ### Added diff --git a/nodejs/models/oauth_client.js b/nodejs/models/oauth_client.js index dbae023..41ae36c 100644 --- a/nodejs/models/oauth_client.js +++ b/nodejs/models/oauth_client.js @@ -44,13 +44,21 @@ class OAuthClient { return r; } static async get(client_id) { + const notFound = () => { + const e = new Error('OAuthClient not found'); + e.status = 404; + return e; + }; let r; try { r = await Resource.get(client_id); } catch (_) { - throw new Error('OAuthClient not found'); + throw notFound(); } - if (r.kind !== 'oauth') throw new Error('OAuthClient not found'); + // Resource.get() returns null (does not throw) for a missing id — + // guard it so a bad/undefined client_id is a clean 404, not a + // "Cannot read properties of null (reading 'kind')" 500. + if (!r || r.kind !== 'oauth') throw notFound(); // Map metadata to top-level properties to satisfy routes/oauth.js without rewriting it r.client_id = r.id; r.client_secret_hash = r.metadata.client_secret_hash; @@ -69,6 +77,29 @@ class OAuthClient { return raw_secret; }; + // The ORM Model.toJSON() only serializes schema fields, so the mapped + // properties above (client_id, scopes, redirect_uris, …) would be + // stripped from any res.json() — that's why GET /api/oauth/client + // returned client_id: undefined and the bootstrap's rotate blew up. + // Emit the public shape explicitly. client_secret_hash is deliberately + // omitted so it never leaks over the API. + r.toJSON = function () { + return { + client_id: r.id, + id: r.id, + kind: r.kind, + name: r.name, + slug: r.slug, + owner: r.owner, + description: r.description, + redirect_uris: r.redirect_uris, + scopes: r.scopes, + allowed_groups: r.allowed_groups, + token_lifetime: r.token_lifetime, + is_valid: r.is_valid, + }; + }; + // proxy update to handle metadata correctly const originalUpdate = r.update.bind(r); r.update = async (data) => { diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index 342f64e..961583e 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -1,12 +1,12 @@ { "name": "t42-sso-manager", - "version": "1.3.1", + "version": "1.3.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "t42-sso-manager", - "version": "1.3.1", + "version": "1.3.2", "license": "MIT", "dependencies": { "@fortawesome/fontawesome-free": "^7.3.0", diff --git a/nodejs/package.json b/nodejs/package.json index 057b7ae..8f2f874 100755 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -1,6 +1,6 @@ { "name": "t42-sso-manager", - "version": "1.3.1", + "version": "1.3.2", "description": "A very simple LDAP management and SSO system", "author": [ { diff --git a/nodejs/tests/oauth.test.js b/nodejs/tests/oauth.test.js index 713cfb0..5613a1f 100644 --- a/nodejs/tests/oauth.test.js +++ b/nodejs/tests/oauth.test.js @@ -43,6 +43,65 @@ afterAll(async () => { } }); +describe('OAuth client management API — /api/oauth/client', () => { + // Regression: the ORM Model.toJSON() strips non-schema fields, so the + // mapped client_id/scopes/etc. used to vanish from GET responses — + // client_id came back undefined and the theta-env bootstrap's rotate + // crashed with a 500. GET must expose client_id (and never the secret hash). + test('GET / list exposes client_id and hides client_secret_hash', async () => { + const res = await request(app) + .get('/api/oauth/client/') + .set('auth-token', token); + expect(res.status).toBe(200); + const mine = res.body.results.find((c) => c.client_id === clientId); + expect(mine).toBeDefined(); + expect(mine.client_id).toBe(clientId); + expect(mine).toHaveProperty('scopes'); + expect(mine).not.toHaveProperty('client_secret_hash'); + }); + + test('GET /:id exposes client_id', async () => { + const res = await request(app) + .get(`/api/oauth/client/${clientId}`) + .set('auth-token', token); + expect(res.status).toBe(200); + expect(res.body.results.client_id).toBe(clientId); + expect(res.body.results).not.toHaveProperty('client_secret_hash'); + }); + + 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 + // throwaway client so the shared flow client's secret is untouched. + const created = await request(app) + .post('/api/oauth/client/') + .set('auth-token', token) + .send({ name: 'rotate-regression', redirect_uris: REDIRECT_URI }); + expect(created.status).toBe(200); + + const list = await request(app).get('/api/oauth/client/').set('auth-token', token); + const found = list.body.results.find((c) => c.name === 'rotate-regression'); + expect(found).toBeDefined(); + expect(found.client_id).toBeTruthy(); // was undefined before the fix + + const rotated = await request(app) + .post(`/api/oauth/client/${found.client_id}/rotate`) + .set('auth-token', token); + 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); + }); + + test('GET /:id unknown id returns 404, not 500', async () => { + const res = await request(app) + .get('/api/oauth/client/00000000-0000-0000-0000-000000000000') + .set('auth-token', token); + expect(res.status).toBeGreaterThanOrEqual(400); + expect(res.status).toBeLessThan(500); + }); +}); + describe('OIDC Discovery', () => { test('GET /.well-known/openid-configuration returns required fields', async () => { const res = await request(app).get('/.well-known/openid-configuration');