Skip to content

Commit e42dcda

Browse files
committed
🐛 Fixed security action history entries
ref #28222 Reset all authentication records audit entries with resource_type=security_action. Those entries are not backed by a Bookshelf model, so including resource in the actions API caused History to fail while eager-loading the polymorphic relation. This keeps the audit resource type intact, teaches the resource relation to accept it, and formats the History label without snake_case.
1 parent b0c8bda commit e42dcda

6 files changed

Lines changed: 149 additions & 7 deletions

File tree

apps/admin-x-framework/src/api/actions.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {JSONObject} from './config';
77

88
export type Action = {
99
id: string;
10-
resource_id: string;
10+
resource_id: string|null;
1111
resource_type: string;
1212
actor_id: string;
1313
actor_type: string;
@@ -46,6 +46,17 @@ export interface ActionsList {
4646

4747
const dataType = 'ActionsResponseType';
4848

49+
export const actionsAreGroupable = (action: Action, nextAction: Action) => {
50+
const getEditedActionName = (candidate: Action) => (
51+
candidate.event === 'edited' && typeof candidate.context?.action_name === 'string'
52+
) ? candidate.context.action_name : '';
53+
54+
return action.resource_id === nextAction.resource_id &&
55+
action.resource_type === nextAction.resource_type &&
56+
action.event === nextAction.event &&
57+
getEditedActionName(action) === getEditedActionName(nextAction);
58+
};
59+
4960
export const useBrowseActions = createInfiniteQuery<ActionsList>({
5061
dataType,
5162
path: '/actions/',
@@ -69,7 +80,7 @@ export const useBrowseActions = createInfiniteQuery<ActionsList>({
6980
// depending on the similarity, add additional properties to be used on the frontend for grouping
7081
// skip - used for hiding the event on the frontend
7182
// count - the number of similar events which is added to the last item
72-
if (nextAction && action.resource_id === nextAction.resource_id && action.event === nextAction.event) {
83+
if (nextAction && actionsAreGroupable(action, nextAction)) {
7384
action.skip = true;
7485
count += 1;
7586
} else if (count > 1) {
@@ -186,6 +197,8 @@ export const getActionTitle = (action: Action) => {
186197
resourceType = 'tier';
187198
} else if (resourceType === 'gift_link') {
188199
resourceType = 'gift link';
200+
} else if (resourceType === 'security_action') {
201+
resourceType = 'security action';
189202
}
190203

191204
// Because a `page` and `post` both use the same model, we store the
@@ -204,6 +217,10 @@ export const getActionTitle = (action: Action) => {
204217
}
205218
}
206219

220+
if (actionName === 'reset_authentication') {
221+
actionName = 'reset authentication';
222+
}
223+
207224
if (action.context?.count && (action.context?.count as number) > 1) {
208225
return `${action.context?.count} ${resourceType}s ${actionName}`;
209226
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import {type Action, actionsAreGroupable, getActionTitle} from '../../../src/api/actions';
2+
3+
const baseAction = (overrides: Partial<Action> = {}): Action => ({
4+
id: 'action-1',
5+
resource_id: 'resource-1',
6+
resource_type: 'post',
7+
actor_id: 'actor-1',
8+
actor_type: 'user',
9+
event: 'edited',
10+
context: {},
11+
created_at: '2026-07-01T00:00:00.000Z',
12+
...overrides
13+
});
14+
15+
describe('actions api helpers', () => {
16+
describe('actionsAreGroupable', () => {
17+
it('groups repeated actions for the same resource type, resource id, event, and action name', () => {
18+
const action = baseAction({context: {action_name: 'updated'}});
19+
const nextAction = baseAction({id: 'action-2', context: {action_name: 'updated'}});
20+
21+
expect(actionsAreGroupable(action, nextAction)).toBe(true);
22+
});
23+
24+
it('does not group null-resource actions for different resource types', () => {
25+
const action = baseAction({resource_id: null, resource_type: 'security_action', context: {action_name: 'reset_authentication'}});
26+
const nextAction = baseAction({id: 'action-2', resource_id: null, resource_type: 'setting', context: {action_name: 'updated'}});
27+
28+
expect(actionsAreGroupable(action, nextAction)).toBe(false);
29+
});
30+
31+
it('does not group edited actions with different action names', () => {
32+
const action = baseAction({resource_id: null, resource_type: 'security_action', context: {action_name: 'reset_authentication'}});
33+
const nextAction = baseAction({id: 'action-2', resource_id: null, resource_type: 'security_action', context: {action_name: 'rotate_keys'}});
34+
35+
expect(actionsAreGroupable(action, nextAction)).toBe(false);
36+
});
37+
});
38+
39+
describe('getActionTitle', () => {
40+
it('formats reset authentication security actions', () => {
41+
expect(getActionTitle(baseAction({
42+
resource_id: null,
43+
resource_type: 'security_action',
44+
context: {action_name: 'reset_authentication'}
45+
}))).toBe('Security action reset authentication');
46+
});
47+
});
48+
});

apps/admin-x-settings/src/components/settings/advanced/history-modal.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,16 @@ const HistoryActionDescription: React.FC<{action: Action}> = ({action}) => {
119119
const {updateRoute} = useRouting();
120120
const contextResource = getContextResource(action);
121121

122-
if (contextResource) {
122+
if (action.resource_type === 'security_action' && action.context?.action_name === 'reset_authentication') {
123+
const apiKeysRotated = typeof action.context.api_keys_rotated === 'number' ? action.context.api_keys_rotated : null;
124+
const usersLocked = typeof action.context.users_locked === 'number' ? action.context.users_locked : null;
125+
const details = [
126+
apiKeysRotated !== null ? `${apiKeysRotated} API ${apiKeysRotated === 1 ? 'key' : 'keys'} rotated` : null,
127+
usersLocked !== null ? `${usersLocked} ${usersLocked === 1 ? 'user' : 'users'} locked` : null
128+
].filter(Boolean);
129+
130+
return <>{details.length ? details.join(', ') : 'Authentication reset'}</>;
131+
} else if (contextResource) {
123132
const {group, key} = contextResource;
124133

125134
return <>

apps/admin-x-settings/test/acceptance/advanced/history.test.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,38 @@ import {globalDataRequests, mockApi, responseFixtures} from '@tryghost/admin-x-f
33

44
test.describe('History', async () => {
55
test('Browsing history', async ({page}) => {
6+
const securityAction = {
7+
id: '64d62b327ca62600011d0819',
8+
resource_id: null,
9+
resource_type: 'security_action',
10+
actor_id: '1',
11+
actor_type: 'user',
12+
event: 'edited',
13+
context: '{"action_name":"reset_authentication","api_keys_rotated":4,"users_locked":3}',
14+
created_at: '2023-08-11T12:37:02.000Z',
15+
actor: {
16+
id: '1',
17+
name: 'Jamie Larson',
18+
slug: 'main',
19+
image: null
20+
}
21+
};
22+
const actionsResponse = {
23+
...responseFixtures.actions,
24+
actions: [securityAction, ...responseFixtures.actions.actions]
25+
};
26+
627
const {lastApiRequests} = await mockApi({page, requests: {
728
...globalDataRequests,
829
browseActionsFiltered: {
930
method: 'GET',
1031
path: /\/actions\/.*filter=.*(?:post|event)/,
1132
response: {
12-
...responseFixtures.actions,
13-
actions: responseFixtures.actions.actions.filter(action => action.resource_type !== 'post')
33+
...actionsResponse,
34+
actions: actionsResponse.actions.filter(action => action.resource_type !== 'post')
1435
}
1536
},
16-
browseActionsAll: {method: 'GET', path: /\/actions\//, response: responseFixtures.actions}
37+
browseActionsAll: {method: 'GET', path: /\/actions\//, response: actionsResponse}
1738
}});
1839

1940
await page.goto('/');
@@ -26,6 +47,8 @@ test.describe('History', async () => {
2647

2748
await expect(historyModal).toHaveText(/Settings edited: Site \(navigation\) 2 times/);
2849
await expect(historyModal).toHaveText(/Page edited: The Clunkers Hall of Shame 2 times/);
50+
await expect(historyModal).toHaveText(/Security action reset authentication: 4 API keys rotated, 3 users locked/);
51+
await expect(historyModal).not.toHaveText(/Security action reset authentication: \(unknown\)/);
2952

3053
expect(lastApiRequests.browseActionsAll?.url).toEqual('http://localhost:5173/ghost/api/admin/actions/?include=actor%2Cresource&limit=200&filter=resource_type%3A-%5Blabel%5D');
3154

ghost/core/core/server/models/action.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,23 @@ const Action = ghostBookshelf.Model.extend({
1010
});
1111
},
1212

13+
resourceCandidates() {
14+
const candidates = this.candidates();
15+
16+
const User = ghostBookshelf.registry.models.User;
17+
if (User) {
18+
candidates.push([User, 'security_action']);
19+
}
20+
21+
return candidates;
22+
},
23+
1324
actor() {
1425
return this.morphTo('actor', ['actor_type', 'actor_id'], ...this.candidates());
1526
},
1627

1728
resource() {
18-
return this.morphTo('resource', ['resource_type', 'resource_id'], ...this.candidates());
29+
return this.morphTo('resource', ['resource_type', 'resource_id'], ...this.resourceCandidates());
1930
}
2031
}, {
2132
orderDefaultOptions: function orderDefaultOptions() {

ghost/core/test/e2e-api/admin/actions.test.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const supertest = require('supertest');
55
const testUtils = require('../../utils');
66
const localUtils = require('./utils');
77
const config = require('../../../core/shared/config');
8+
const models = require('../../../core/server/models');
89

910
describe('Actions API', function () {
1011
let request;
@@ -137,4 +138,37 @@ describe('Actions API', function () {
137138
assert.equal(res5.body.actions[0].actor.name, testUtils.DataGenerator.Content.integrations[0].name);
138139
assert.equal(res5.body.actions[0].actor.slug, testUtils.DataGenerator.Content.integrations[0].slug);
139140
});
141+
142+
it('Can request security actions with resource included', async function () {
143+
const actorId = testUtils.DataGenerator.Content.users[0].id;
144+
const action = await models.Action.add({
145+
event: 'edited',
146+
resource_type: 'security_action',
147+
resource_id: null,
148+
actor_type: 'user',
149+
actor_id: actorId,
150+
context: {
151+
action_name: 'reset_authentication',
152+
api_keys_rotated: 1,
153+
users_locked: 1
154+
}
155+
});
156+
157+
const res = await request
158+
.get(localUtils.API.getApiQuery(`actions/?filter=${encodeURIComponent(`id:'${action.id}'`)}&include=actor,resource`))
159+
.set('Origin', config.get('url'))
160+
.expect('Content-Type', /json/)
161+
.expect('Cache-Control', testUtils.cacheRules.private)
162+
.expect(200);
163+
164+
localUtils.API.checkResponse(res.body, 'actions');
165+
localUtils.API.checkResponse(res.body.actions[0], 'action');
166+
167+
assert.equal(res.body.actions.length, 1);
168+
assert.equal(res.body.actions[0].resource_type, 'security_action');
169+
assert.equal(res.body.actions[0].resource_id, null);
170+
assert.equal(res.body.actions[0].actor_type, 'user');
171+
assert.equal(res.body.actions[0].actor.id, actorId);
172+
assert.equal(res.body.actions[0].resource, undefined);
173+
});
140174
});

0 commit comments

Comments
 (0)