Files
wmantly 7f2c71299f Fix DELETE /api/oauth/client/🆔 client.remove is not a function
OAuthClient wraps @simpleworkjs/orm's Resource model, whose instance
delete method is .delete() -- not .remove(), which is what model-redis's
Table instances (e.g. this app's ApiToken, AuthToken) use. The DELETE
route called the wrong one, so every delete silently 500'd; the route's
try/catch turned it into a plain JSON error response rather than a thrown
exception, and the existing tests' cleanup-only delete calls (afterAll,
end of the rotate test) never checked the response status, so the bug
shipped unnoticed. The Directory Management UI was never affected --
routes/api_directory_admin.js's DELETE routes already used .delete()
correctly throughout.

Found and root-caused live against a real deployment's SSO API, then
reproduced and fixed against a local docker stack with a rebuilt image:
confirmed DELETE returned a genuine 500 before the fix and a real 200 +
404-on-subsequent-GET after.

Adds two dedicated tests (PUT and DELETE persistence, each verified by a
follow-up GET rather than trusting the mutating response alone), and
hardens the existing rotate test's incidental delete call with real
assertions. Verified the new DELETE test fails on the old code and
passes on the fix. Full suite (189 tests, real LDAP + Redis) passes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-27 21:03:34 -04:00

128 lines
3.9 KiB
JavaScript

'use strict';
const router = require('express').Router();
const { OAuthClient } = require('../models/oauth_client');
const permission = require('../utils/permission');
const ADMIN_GROUP = 'app_sso_oauth_admin';
router.get('/', async function(req, res, next) {
try {
await permission.byGroup(req.user, [ADMIN_GROUP]);
return res.json({ results: await OAuthClient.listDetail() });
} catch(error) {
next(error);
}
});
router.post('/', async function(req, res, next) {
try {
await permission.byGroup(req.user, [ADMIN_GROUP]);
req.body.created_by = req.user.uid;
// Parse redirect_uris if sent as newline-separated string from the form
if (typeof req.body.redirect_uris === 'string') {
req.body.redirect_uris = req.body.redirect_uris.split('\n').map(s => s.trim()).filter(Boolean);
}
// Parse scopes if sent as space-separated string
if (typeof req.body.scopes === 'string') {
req.body.scopes = req.body.scopes.split(' ').map(s => s.trim()).filter(Boolean);
}
// Parse allowed_groups if sent as newline-separated string
if (typeof req.body.allowed_groups === 'string') {
req.body.allowed_groups = req.body.allowed_groups.split('\n').map(s => s.trim()).filter(Boolean);
}
// jQuery serializeObject sends nested fields as "token_lifetime[access_token]"
if (req.body['token_lifetime[access_token]'] || req.body['token_lifetime[refresh_token]']) {
req.body.token_lifetime = {
access_token: Number(req.body['token_lifetime[access_token]']) || 3600,
refresh_token: Number(req.body['token_lifetime[refresh_token]']) || 2592000,
};
delete req.body['token_lifetime[access_token]'];
delete req.body['token_lifetime[refresh_token]'];
}
const client = await OAuthClient.add(req.body);
const result = client.toJSON ? client.toJSON() : { ...client };
result.client_id = client.client_id || client.id;
return res.json({
results: result,
client_secret: client._raw_secret,
message: `OAuth client '${client.name}' created. Save the client secret — it will not be shown again.`,
});
} catch(error) {
next(error);
}
});
router.get('/:client_id', async function(req, res, next) {
try {
await permission.byGroup(req.user, [ADMIN_GROUP]);
return res.json({ results: await OAuthClient.get(req.params.client_id) });
} catch(error) {
next(error);
}
});
router.put('/:client_id', async function(req, res, next) {
try {
await permission.byGroup(req.user, [ADMIN_GROUP]);
const client = await OAuthClient.get(req.params.client_id);
if (typeof req.body.redirect_uris === 'string') {
req.body.redirect_uris = req.body.redirect_uris.split('\n').map(s => s.trim()).filter(Boolean);
}
if (typeof req.body.scopes === 'string') {
req.body.scopes = req.body.scopes.split(' ').map(s => s.trim()).filter(Boolean);
}
if (typeof req.body.allowed_groups === 'string') {
req.body.allowed_groups = req.body.allowed_groups.split('\n').map(s => s.trim()).filter(Boolean);
}
return res.json({
results: await client.update(req.body),
message: `OAuth client '${client.name}' updated.`,
});
} catch(error) {
next(error);
}
});
router.delete('/:client_id', async function(req, res, next) {
try {
await permission.byGroup(req.user, [ADMIN_GROUP]);
const client = await OAuthClient.get(req.params.client_id);
await client.delete();
return res.json({
client_id: req.params.client_id,
message: `OAuth client '${client.name}' deleted.`,
});
} catch(error) {
next(error);
}
});
router.post('/:client_id/rotate', async function(req, res, next) {
try {
await permission.byGroup(req.user, [ADMIN_GROUP]);
const client = await OAuthClient.get(req.params.client_id);
const new_secret = await client.rotateSecret();
return res.json({
client_secret: new_secret,
message: `Client secret rotated for '${client.name}'. Save it — it will not be shown again.`,
});
} catch(error) {
next(error);
}
});
module.exports = router;