Skip to content

Commit de86c6b

Browse files
fix(home): restrict /team fields, remove releases/changelog, add jwt readiness check (#3724 #3725 #3718) (#3735)
* fix(home): restrict /team fields, remove releases/changelog, jwt readiness check (#3724 #3725 #3718) - #3724: restrict /api/home/team to public-safe fields only (firstName, lastName, bio, position, avatar) via positive MongoDB projection in repository; drop removeSensitive call in service since projection already enforces the whitelist - #3725: remove releases and changelogs feature entirely — remove service functions, controller handlers, routes, config.repos config, and all related tests; drop unused axios and js-base64 imports - #3718: JWT readiness check already present in getReadinessStatus() under 'security' category (warns when secret equals known default); update tests to match current category list ['config','security','auth','mail','billing','analytics','errorTracking'] * fix(home): address review nitpicks on team projection and deps * fix(home): lean -_id team projection
1 parent 813092b commit de86c6b

10 files changed

Lines changed: 18 additions & 268 deletions

File tree

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,3 @@
1-
const config = {
2-
repos: [
3-
{
4-
// generate releases and changelogs list auto /api/core/changelogs /api/core/releases
5-
title: 'Node',
6-
owner: 'pierreb-devkit',
7-
repo: 'node',
8-
changelog: 'CHANGELOG.md',
9-
token: null,
10-
},
11-
{
12-
title: 'Vue',
13-
owner: 'pierreb-devkit',
14-
repo: 'vue',
15-
changelog: 'CHANGELOG.md',
16-
token: null,
17-
},
18-
],
19-
};
1+
const config = {};
202

213
export default config;

modules/home/controllers/home.controller.js

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,34 +7,6 @@ import errors from '../../../lib/helpers/errors.js';
77
import responses from '../../../lib/helpers/responses.js';
88
import HomeService from '../services/home.service.js';
99

10-
/**
11-
* @desc Endpoint to ask the service to get the releases
12-
* @param {Object} req - Express request object
13-
* @param {Object} res - Express response object
14-
*/
15-
const releases = async (req, res) => {
16-
try {
17-
const releases = await HomeService.releases();
18-
responses.success(res, 'releases')(releases);
19-
} catch (err) {
20-
responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
21-
}
22-
};
23-
24-
/**
25-
* @desc Endpoint to ask the service to get the changelogs
26-
* @param {Object} req - Express request object
27-
* @param {Object} res - Express response object
28-
*/
29-
const changelogs = async (req, res) => {
30-
try {
31-
const changelogs = await HomeService.changelogs();
32-
responses.success(res, 'changelogs')(changelogs);
33-
} catch (err) {
34-
responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
35-
}
36-
};
37-
3810
/**
3911
* @desc Endpoint to ask the service to get the list of users
4012
* @param {Object} req - Express request object
@@ -112,8 +84,6 @@ const readiness = (req, res) => {
11284
};
11385

11486
export default {
115-
releases,
116-
changelogs,
11787
team,
11888
page,
11989
pageByName,

modules/home/policies/home.policy.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export function homeSubjectRegistration({ registerPathSubject }) {
1515

1616
/**
1717
* Define home-related abilities for an authenticated user.
18-
* All authenticated users can read home content (releases, changelogs, team, pages).
18+
* All authenticated users can read home content (team, pages).
1919
* @param {Object} user - The authenticated user
2020
* @param {Object|null} membership - Optional organization membership (reserved for future use)
2121
* @param {Object} builder - CASL AbilityBuilder helpers
@@ -30,7 +30,7 @@ export function homeAbilities(user, membership, { can }) {
3030

3131
/**
3232
* Define home-related abilities for unauthenticated guests.
33-
* Guests can read all home content (releases, changelogs, team, pages).
33+
* Guests can read all home content (team, pages).
3434
* @param {Object} builder - CASL AbilityBuilder helpers
3535
* @param {Function} builder.can - Grant an ability
3636
*/

modules/home/repositories/home.repository.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const User = mongoose.model('User');
99
* @desc Function to get all user in db
1010
* @return {Array} All users
1111
*/
12-
const team = () => User.find({ roles: 'admin' }, '-password -providerData -complementary').sort('-createdAt').exec();
12+
const team = () => User.find({ roles: 'admin' }, 'firstName lastName bio position avatar -_id').lean().sort('-createdAt').exec();
1313

1414
export default {
1515
team,

modules/home/routes/home.route.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,7 @@ const optionalAuth = (req, res, next) => {
2727
export default (app) => {
2828
// health check — public, enriched response for admins
2929
app.route('/api/health').get(optionalAuth, home.health);
30-
// changelogs
31-
app.route('/api/home/releases').all(policy.isAllowed).get(home.releases);
32-
// changelogs
33-
app.route('/api/home/changelogs').all(policy.isAllowed).get(home.changelogs);
34-
// changelogs
30+
// team — public staff page
3531
app.route('/api/home/team').all(policy.isAllowed).get(home.team);
3632
// markdown files
3733
app.route('/api/home/pages/:name').all(policy.isAllowed).get(home.page);

modules/home/services/home.service.js

Lines changed: 4 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,14 @@
11
/**
22
* Module dependencies
33
*/
4-
import axios from 'axios';
54
import path from 'path';
65
import _ from 'lodash';
7-
import { Base64 } from 'js-base64';
86
import { promises as fs } from 'fs';
97
import mongoose from 'mongoose';
108

119
import config from '../../../config/index.js';
1210
import mailer from '../../../lib/helpers/mailer/index.js';
1311
import HomeRepository from '../repositories/home.repository.js';
14-
import { removeSensitive } from '../../users/utils/sanitizeUser.js';
1512

1613
/**
1714
* @desc Check whether a config value is meaningfully set (non-empty, not a DEVKIT placeholder).
@@ -38,62 +35,11 @@ const page = async (name) => {
3835
};
3936

4037
/**
41-
* @desc Fetch releases from configured GitHub repos. Returns an empty array on API failure (graceful degradation).
42-
* @return {Promise<Array<{title: string, list: Array}>>} Releases grouped by repo, or [] on error
38+
* @desc Function to get all admin users in db, returning only public-safe fields.
39+
* Uses a lean projection so no Mongoose virtuals (e.g. `id`) can re-introduce hidden fields.
40+
* @returns {Promise<Array<{firstName: string, lastName: string, bio: string, position: string, avatar: string}>>} Public user profiles
4341
*/
44-
const releases = async () => {
45-
try {
46-
const requests = config.repos.map((item) =>
47-
axios.get(`https://api.github.com/repos/${item.owner}/${item.repo}/releases`, {
48-
headers: item.token ? { Authorization: `token ${item.token}` } : {},
49-
}),
50-
);
51-
let results = await axios.all(requests);
52-
results = results.map((result, i) => ({
53-
title: config.repos[i].title,
54-
list: result.data.map((release) => ({
55-
name: release.name,
56-
prerelease: release.prerelease,
57-
published_at: release.published_at,
58-
})),
59-
}));
60-
return results;
61-
} catch (_err) {
62-
return [];
63-
}
64-
};
65-
66-
/**
67-
* @desc Fetch changelogs from configured GitHub repos. Returns an empty array on API failure (graceful degradation).
68-
* @return {Promise<Array<{title: string, markdown: string}>>} Changelogs grouped by repo, or [] on error
69-
*/
70-
const changelogs = async () => {
71-
try {
72-
const repos = _.filter(config.repos, (repo) => repo.changelog);
73-
const requests = repos.map((item) =>
74-
axios.get(`https://api.github.com/repos/${item.owner}/${item.repo}/contents/${item.changelog}`, {
75-
headers: item.token ? { Authorization: `token ${item.token}` } : {},
76-
}),
77-
);
78-
let results = await axios.all(requests);
79-
results = results.map((result, i) => ({
80-
title: repos[i].title,
81-
markdown: Base64.decode(result.data.content),
82-
}));
83-
return results;
84-
} catch (_err) {
85-
return [];
86-
}
87-
};
88-
89-
/**
90-
* @desc Function to get all admin users in db
91-
* @returns {Promise<Array>} All users (sanitized)
92-
*/
93-
const team = async () => {
94-
const result = await HomeRepository.team();
95-
return result.map((user) => removeSensitive(user));
96-
};
42+
const team = async () => HomeRepository.team();
9743

9844
/**
9945
* @desc Build health status including database connectivity.
@@ -183,8 +129,6 @@ const getReadinessStatus = () => {
183129

184130
export default {
185131
page,
186-
releases,
187-
changelogs,
188132
team,
189133
getHealthStatus,
190134
getReadinessStatus,

modules/home/tests/home.integration.tests.js

Lines changed: 9 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
* Module dependencies.
33
*/
44
import { jest } from '@jest/globals';
5-
import axios from 'axios';
65
import jwt from 'jsonwebtoken';
76
import mongoose from 'mongoose';
87
import path from 'path';
@@ -26,16 +25,6 @@ describe('Home integration tests:', () => {
2625

2726
// init
2827
beforeAll(async () => {
29-
// Mock GitHub API calls to avoid real network requests in tests
30-
jest.spyOn(axios, 'get').mockImplementation(async (url) => {
31-
if (url.includes('/releases')) {
32-
return { data: [{ name: 'v1.0.0', prerelease: false, published_at: '2024-01-01T00:00:00Z' }] };
33-
}
34-
if (url.includes('/contents/')) {
35-
return { data: { content: Buffer.from('# Changelog\n## v1.0.0\n- First release').toString('base64') } };
36-
}
37-
throw new Error(`Unexpected GitHub API URL: ${url}`);
38-
});
3928
try {
4029
originalOrganizationsEnabled = config.organizations.enabled;
4130
config.organizations.enabled = false;
@@ -79,26 +68,20 @@ describe('Home integration tests:', () => {
7968
// into later describes (#3472). Auth-required cases still use explicit
8069
// `set('Cookie', 'TOKEN=...')` headers derived from JWTs.
8170
describe('Logout', () => {
82-
test('should be able to get releases', async () => {
83-
const result = await request(app).get('/api/home/releases').expect(200);
84-
expect(result.body.type).toBe('success');
85-
expect(result.body.message).toBe('releases');
86-
expect(result.body.data).toBeInstanceOf(Array);
87-
});
88-
89-
test('should be able to get changelogs', async () => {
90-
const result = await request(app).get('/api/home/changelogs').expect(200);
91-
expect(result.body.type).toBe('success');
92-
expect(result.body.message).toBe('changelogs');
93-
expect(result.body.data).toBeInstanceOf(Array);
94-
});
95-
96-
test('should be able to get team members', async () => {
71+
test('should be able to get team members with public fields only', async () => {
9772
try {
9873
const result = await request(app).get('/api/home/team').expect(200);
9974
expect(result.body.type).toBe('success');
10075
expect(result.body.message).toBe('team list');
10176
expect(result.body.data).toBeInstanceOf(Array);
77+
// Assert the strict public-field allowlist: every key must be one of these,
78+
// so no unexpected field (incl. _id / id / sensitive data) can ever leak.
79+
const ALLOWED_FIELDS = new Set(['firstName', 'lastName', 'bio', 'position', 'avatar']);
80+
result.body.data.forEach((member) => {
81+
Object.keys(member).forEach((key) => {
82+
expect(ALLOWED_FIELDS.has(key)).toBe(true);
83+
});
84+
});
10285
} catch (err) {
10386
expect(err).toBeFalsy();
10487
console.log(err);
@@ -131,51 +114,6 @@ describe('Home integration tests:', () => {
131114
}
132115
});
133116

134-
test('should return empty releases gracefully when GitHub API fails', async () => {
135-
axios.get.mockRejectedValueOnce(new Error('GitHub API unavailable'));
136-
const result = await request(app).get('/api/home/releases').expect(200);
137-
expect(result.body.type).toBe('success');
138-
expect(result.body.message).toBe('releases');
139-
expect(result.body.data).toEqual([]);
140-
});
141-
142-
test('should return empty changelogs gracefully when GitHub API fails', async () => {
143-
axios.get.mockRejectedValueOnce(new Error('GitHub API unavailable'));
144-
const result = await request(app).get('/api/home/changelogs').expect(200);
145-
expect(result.body.type).toBe('success');
146-
expect(result.body.message).toBe('changelogs');
147-
expect(result.body.data).toEqual([]);
148-
});
149-
150-
test('should use Authorization header when a token is configured for releases', async () => {
151-
const originalRepos = config.repos;
152-
axios.get.mockClear();
153-
// Temporarily set a fake token to cover the token-truthy branch in home.service releases()
154-
config.repos = originalRepos.map((repo) => ({ ...repo, token: 'fake-test-token' }));
155-
const result = await request(app).get('/api/home/releases').expect(200);
156-
expect(result.body.type).toBe('success');
157-
const releaseCalls = axios.get.mock.calls.filter(([url]) => url.includes('/releases'));
158-
expect(releaseCalls.length).toBeGreaterThan(0);
159-
releaseCalls.forEach(([, options]) => {
160-
expect(options.headers.Authorization).toBe('token fake-test-token');
161-
});
162-
config.repos = originalRepos;
163-
});
164-
165-
test('should use Authorization header when a token is configured for changelogs', async () => {
166-
const originalRepos = config.repos;
167-
axios.get.mockClear();
168-
config.repos = originalRepos.map((repo) => ({ ...repo, token: 'fake-test-token' }));
169-
const result = await request(app).get('/api/home/changelogs').expect(200);
170-
expect(result.body.type).toBe('success');
171-
const changelogCalls = axios.get.mock.calls.filter(([url]) => url.includes('/contents/'));
172-
expect(changelogCalls.length).toBeGreaterThan(0);
173-
changelogCalls.forEach(([, options]) => {
174-
expect(options.headers.Authorization).toBe('token fake-test-token');
175-
});
176-
config.repos = originalRepos;
177-
});
178-
179117
test('should return minimal health status without auth', async () => {
180118
const result = await request(app).get('/api/health').expect(200);
181119
expect(result.body.type).toBe('success');

modules/home/tests/home.service.unit.tests.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@ describe('HomeService.getReadinessStatus unit tests:', () => {
2525
default: { team: jest.fn().mockResolvedValue([]) },
2626
}));
2727

28-
// Mock axios to avoid real network calls
29-
jest.unstable_mockModule('axios', () => ({
30-
default: { get: jest.fn().mockResolvedValue({ data: [] }), all: jest.fn().mockResolvedValue([]) },
31-
}));
3228
});
3329

3430
afterEach(() => {

0 commit comments

Comments
 (0)