Merge pull request #113 from theta42/fix/group-cache-invalidation
Fix: group membership changes didn't invalidate the User cache
This commit is contained in:
@@ -82,8 +82,13 @@ router.put('/:group/:uid', async function(req, res, next){
|
|||||||
|
|
||||||
var group = await Group.get(req.params.group);
|
var group = await Group.get(req.params.group);
|
||||||
var user = await User.get(req.params.uid);
|
var user = await User.get(req.params.uid);
|
||||||
|
const results = await group.addMember(user);
|
||||||
|
// Group membership feeds directly into cached-User-derived state
|
||||||
|
// (isServiceAccount, isAdmin, group-gated nav/UI) -- without this,
|
||||||
|
// a membership change here is invisible for up to the cache's TTL.
|
||||||
|
User.clearCache();
|
||||||
return res.json({
|
return res.json({
|
||||||
results: await group.addMember(user),
|
results,
|
||||||
message: `Added user ${req.params.uid} to ${req.params.group} group.`
|
message: `Added user ${req.params.uid} to ${req.params.group} group.`
|
||||||
});
|
});
|
||||||
}catch(error){
|
}catch(error){
|
||||||
@@ -98,8 +103,10 @@ router.delete('/:group/:uid', async function(req, res, next){
|
|||||||
|
|
||||||
var group = await Group.get(req.params.group);
|
var group = await Group.get(req.params.group);
|
||||||
var user = await User.get(req.params.uid);
|
var user = await User.get(req.params.uid);
|
||||||
|
const results = await group.removeMember(user);
|
||||||
|
User.clearCache();
|
||||||
return res.json({
|
return res.json({
|
||||||
results: await group.removeMember(user),
|
results,
|
||||||
message: `Removed user ${req.params.uid} from ${req.params.group} group.`
|
message: `Removed user ${req.params.uid} from ${req.params.group} group.`
|
||||||
});
|
});
|
||||||
}catch(error){
|
}catch(error){
|
||||||
|
|||||||
@@ -151,6 +151,32 @@ describe('Groups — member management', () => {
|
|||||||
const members = Array.isArray(group.member) ? group.member : [group.member];
|
const members = Array.isArray(group.member) ? group.member : [group.member];
|
||||||
expect(members.some(dn => dn && dn.includes(MEMBER_UID))).toBe(false);
|
expect(members.some(dn => dn && dn.includes(MEMBER_UID))).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression: adding/removing a member here didn't clear User's LRU
|
||||||
|
// cache (ttl 5 minutes), so isServiceAccount -- derived from
|
||||||
|
// app_sso_service_account membership at GET /api/user/:uid time -- could
|
||||||
|
// stay wrong for up to 5 minutes after the group change. In production
|
||||||
|
// this hid a real person's account from the Users page's "People" tab
|
||||||
|
// (it filters out anything with isServiceAccount) for however long the
|
||||||
|
// stale cache entry lived, which looked exactly like the account had
|
||||||
|
// vanished.
|
||||||
|
test('PUT app_sso_service_account/:uid immediately flips isServiceAccount (no stale cache)', async () => {
|
||||||
|
const added = await request(app)
|
||||||
|
.put(`/api/group/app_sso_service_account/${MEMBER_UID}`)
|
||||||
|
.set('auth-token', token);
|
||||||
|
expect(added.status).toBe(200);
|
||||||
|
|
||||||
|
const afterAdd = await request(app).get(`/api/user/${MEMBER_UID}`).set('auth-token', token);
|
||||||
|
expect(afterAdd.body.results.isServiceAccount).toBeTruthy();
|
||||||
|
|
||||||
|
const removed = await request(app)
|
||||||
|
.delete(`/api/group/app_sso_service_account/${MEMBER_UID}`)
|
||||||
|
.set('auth-token', token);
|
||||||
|
expect(removed.status).toBe(200);
|
||||||
|
|
||||||
|
const afterRemove = await request(app).get(`/api/user/${MEMBER_UID}`).set('auth-token', token);
|
||||||
|
expect(afterRemove.body.results.isServiceAccount).toBeFalsy();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('Groups — owner management', () => {
|
describe('Groups — owner management', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user