Skip to content

Commit 15ec97e

Browse files
authored
refactor whoami route and update tests (#5045)
* refactor whoami route and update tests * Update CHANGELOG * Linting fix
1 parent 09a90ec commit 15ec97e

3 files changed

Lines changed: 110 additions & 48 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ It allows live preview to work well with it, it also avoids complexity and fixes
2424
### Changes
2525

2626
* Rolled back a change in 4.16.0 that strictly enforced `required` and `min` for relationship fields. Because the related document can be archived or deleted at any time, it is misleading to offer such enforcement. Also, it greatly complicates adding these constraints to existing schemas, resulting in surprising and unwanted behaviors. Therefore it is better for these constraints to be soft constraints on the front end. `max` is still a hard constraint.
27+
* The `@apostrophecms/login/whoami` route now accepts both `POST` (recommended) and `GET` requests. Previously, it only supported `GET`. This maintains backwards compatibility while aligning with the documentation’s recommendation to use `POST`.
2728

2829
## 4.20.0 (2025-08-06)
2930

modules/@apostrophecms/login/index.js

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ module.exports = {
9292
// rather than ever being stored literally
9393
self.apos.user.addSecret('passwordReset');
9494
},
95-
async checkForUser () {
95+
async checkForUser() {
9696
await self.checkForUserAndAlert();
9797
}
9898
}
@@ -148,7 +148,7 @@ module.exports = {
148148
}
149149
if (req.session) {
150150
const destroySession = () => {
151-
return require('util').promisify(function(callback) {
151+
return require('util').promisify(function (callback) {
152152
// Be thorough, nothing in the session potentially
153153
// related to the login should survive logout
154154
return req.session.destroy(callback);
@@ -259,6 +259,9 @@ module.exports = {
259259
async context(req) {
260260
return self.getContext(req);
261261
},
262+
async whoami(req) {
263+
return self.getWhoami(req);
264+
},
262265
...self.isPasswordResetEnabled() && {
263266
async resetRequest(req) {
264267
const wait = (t = 2000) => Promise.delay(t);
@@ -353,24 +356,8 @@ module.exports = {
353356
// it should be accessed via POST because the result
354357
// may differ by individual user session and should not
355358
// be cached
356-
async whoami (req) {
357-
if (!req.user) {
358-
throw self.apos.error('notfound');
359-
}
360-
361-
const fields = new Set([
362-
...self.options.minimumWhoamiFields,
363-
...self.options.whoamiFields
364-
]);
365-
const user = {};
366-
367-
for (const field of fields) {
368-
if (req.user[field] !== undefined) {
369-
user[field] = req.user[field];
370-
}
371-
}
372-
373-
return user;
359+
async whoami(req) {
360+
return self.getWhoami(req);
374361
},
375362
async context(req) {
376363
return self.getContext(req);
@@ -423,6 +410,29 @@ module.exports = {
423410
};
424411
},
425412

413+
// Implements the whoami route, which provides
414+
// information about the user that is currently
415+
// logged in
416+
async getWhoami(req) {
417+
if (!req.user) {
418+
throw self.apos.error('notfound');
419+
}
420+
421+
const fields = new Set([
422+
...self.options.minimumWhoamiFields,
423+
...self.options.whoamiFields
424+
]);
425+
const user = {};
426+
427+
for (const field of fields) {
428+
if (req.user[field] !== undefined) {
429+
user[field] = req.user[field];
430+
}
431+
}
432+
433+
return user;
434+
},
435+
426436
// return the loginUrl option
427437
login(url) {
428438
return self.options.loginUrl ? self.options.loginUrl : '/login';
@@ -806,7 +816,7 @@ module.exports = {
806816
self.apos.structuredLog.getRequestId(req)
807817
);
808818
if (!user) {
809-
// For security reasons we may not tell the user which case applies
819+
// For security reasons we may not tell the user which case applies
810820
throw self.apos.error('invalid', req.t('apostrophe:loginPageBadCredentials'));
811821
}
812822

@@ -848,7 +858,7 @@ module.exports = {
848858
userId: user._id,
849859
expires: new Date(
850860
new Date().getTime() +
851-
(self.options.bearerTokens.lifetime || (86400 * 7 * 2)) * 1000
861+
(self.options.bearerTokens.lifetime || (86400 * 7 * 2)) * 1000
852862
)
853863
});
854864

@@ -887,14 +897,14 @@ module.exports = {
887897
req.res.cookie(cookieName, 'true');
888898
}
889899
const passportLogin = (user) => {
890-
return require('util').promisify(function(user, callback) {
900+
return require('util').promisify(function (user, callback) {
891901
return req.login(user, callback);
892902
})(user);
893903
};
894904
await passportLogin(user);
895905
},
896906

897-
async addLoginAttempt (
907+
async addLoginAttempt(
898908
username,
899909
attempts,
900910
namespace = loginAttemptsNamespace
@@ -920,7 +930,7 @@ module.exports = {
920930
}
921931
},
922932

923-
async checkLoginAttempts (username, namespace = loginAttemptsNamespace) {
933+
async checkLoginAttempts(username, namespace = loginAttemptsNamespace) {
924934
const cachedAttempts = await self.apos.cache.get(namespace, username);
925935
const { allowedAttempts } = self.options.throttle;
926936

@@ -944,7 +954,7 @@ module.exports = {
944954
};
945955
},
946956

947-
async clearLoginAttempts (username, namespace = loginAttemptsNamespace) {
957+
async clearLoginAttempts(username, namespace = loginAttemptsNamespace) {
948958
await self.apos.cache.delete(namespace, username);
949959
},
950960

test/login.js

Lines changed: 73 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
const t = require('../test-lib/test.js');
22
const assert = require('assert').strict;
33

4-
describe('Login', function() {
4+
describe('Login', function () {
55
let apos;
66
let resetUserId;
77
let resetToken;
@@ -31,13 +31,13 @@ describe('Login', function() {
3131
});
3232
});
3333

34-
after(function() {
34+
after(function () {
3535
return t.destroy(apos);
3636
});
3737

3838
// EXISTENCE
3939

40-
it('should initialize', async function() {
40+
it('should initialize', async function () {
4141
assert(apos);
4242

4343
assert(apos.modules['@apostrophecms/login']);
@@ -117,7 +117,7 @@ describe('Login', function() {
117117
await loginModule.clearLoginAttempts(username);
118118
});
119119

120-
it('should be able to login a user with their username', async function() {
120+
it('should be able to login a user with their username', async function () {
121121
const getLoggedInCookieValue =
122122
jar => jar.toJSON().cookies.find(cookie => cookie.key === `${apos.options.shortName}.loggedIn`).value;
123123

@@ -182,7 +182,7 @@ describe('Login', function() {
182182
assert(getLoggedInCookieValue(jar) === 'false');
183183
});
184184

185-
it('should be able to login a user with their email', async function() {
185+
it('should be able to login a user with their email', async function () {
186186

187187
const jar = apos.http.jar();
188188

@@ -242,7 +242,7 @@ describe('Login', function() {
242242
assert(page.match(/logged out/));
243243
});
244244

245-
it('changing a user\'s password should invalidate sessions for that user', async function() {
245+
it('changing a user\'s password should invalidate sessions for that user', async function () {
246246

247247
const jar = apos.http.jar();
248248

@@ -356,7 +356,7 @@ describe('Login', function() {
356356

357357
});
358358

359-
it('changing a user\'s password should invalidate bearer tokens for that user', async function() {
359+
it('changing a user\'s password should invalidate bearer tokens for that user', async function () {
360360

361361
// Log in
362362
let response = await apos.http.post('/api/v1/@apostrophecms/login/login', {
@@ -425,7 +425,7 @@ describe('Login', function() {
425425

426426
});
427427

428-
it('api key should beat session when both are present', async function() {
428+
it('api key should beat session when both are present', async function () {
429429
const jar = apos.http.jar();
430430
await apos.http.post(
431431
'/api/v1/@apostrophecms/login/login',
@@ -463,7 +463,7 @@ describe('Login', function() {
463463
assert(page2.match(/System Task/));
464464
});
465465

466-
it('should validate POST /login/reset-request', async function() {
466+
it('should validate POST /login/reset-request', async function () {
467467
const jar = apos.http.jar();
468468
await apos.http.get(
469469
'/',
@@ -485,7 +485,7 @@ describe('Login', function() {
485485
});
486486
});
487487

488-
it('should hide sensitive exceptions POST /login/reset-request', async function() {
488+
it('should hide sensitive exceptions POST /login/reset-request', async function () {
489489
let log;
490490
const orig = apos.util.error;
491491
apos.util.error = (m) => {
@@ -532,7 +532,7 @@ describe('Login', function() {
532532
apos.util.error = orig;
533533
});
534534

535-
it('should reset password POST /login/reset-request (request)', async function() {
535+
it('should reset password POST /login/reset-request (request)', async function () {
536536
let args;
537537
const orig = apos.login.email;
538538
apos.login.email = (req, ...a) => {
@@ -610,7 +610,7 @@ describe('Login', function() {
610610
apos.login.email = orig;
611611
});
612612

613-
it('should reset password GET /login/reset (validate)', async function() {
613+
it('should reset password GET /login/reset (validate)', async function () {
614614
const user = await apos.doc.db.findOne({ _id: resetUserId });
615615

616616
// Fail
@@ -667,7 +667,7 @@ describe('Login', function() {
667667
);
668668
});
669669

670-
it('should reset password POST /login/reset (validate & reset)', async function() {
670+
it('should reset password POST /login/reset (validate & reset)', async function () {
671671
const jar = apos.http.jar();
672672
const user = await apos.doc.db.findOne({ _id: resetUserId });
673673
await apos.http.get(
@@ -798,7 +798,7 @@ describe('Login', function() {
798798
assert(page.match(/logged in/));
799799
});
800800

801-
it('should find user by reset data', async function() {
801+
it('should find user by reset data', async function () {
802802
let user = apos.user.newInstance();
803803
user.title = 'getResetUser';
804804
user.email = 'getResetUser@example.com';
@@ -868,18 +868,39 @@ describe('Login', function() {
868868
);
869869
});
870870

871-
it('should return an error with code 404 at GET login/whoami when user is not logged in', async function() {
871+
it('should return an error with code 404 at GET login/whoami when user is not logged in', async function () {
872872
try {
873-
await apos.http.get('/api/v1/@apostrophecms/login/whoami');
873+
const jar = apos.http.jar();
874+
await apos.http.get(
875+
'/',
876+
{
877+
jar
878+
}
879+
);
880+
await apos.http.post('/api/v1/@apostrophecms/login/whoami',
881+
{
882+
method: 'POST',
883+
body: {
884+
session: true
885+
},
886+
jar
887+
}
888+
);
874889
assert.fail('Expected error but got success');
875890
} catch (err) {
876891
assert.strictEqual(err.status, 404);
877892
}
878893
});
879894

880-
it('should return user data at GET login/whoami when user is logged in', async function() {
895+
it('should return user data at GET login/whoami when user is logged in', async function () {
881896

882897
const jar = apos.http.jar();
898+
await apos.http.get(
899+
'/',
900+
{
901+
jar
902+
}
903+
);
883904

884905
await apos.http.post(
885906
'/api/v1/@apostrophecms/login/login',
@@ -894,16 +915,28 @@ describe('Login', function() {
894915
}
895916
);
896917

897-
const whoamiResponse = await apos.http.get('/api/v1/@apostrophecms/login/whoami', { jar });
918+
const whoamiResponse = await apos.http.post('/api/v1/@apostrophecms/login/whoami', {
919+
method: 'POST',
920+
body: {
921+
session: true
922+
},
923+
jar
924+
});
898925
assert.ok(whoamiResponse._id);
899926
assert.strictEqual(whoamiResponse.username, 'HarryPutter');
900927
assert.strictEqual(whoamiResponse.title, 'Extra Cool Putter');
901928
assert.strictEqual(whoamiResponse.email, 'hputter@aol.com');
902929
});
903930

904-
it('should return user data with additional whoamiFields if explicitly added at GET login/whoami when user is logged in', async function() {
931+
it('should return user data with additional whoamiFields if explicitly added to the login module options when user is logged in', async function () {
905932

906933
const jar = apos.http.jar();
934+
await apos.http.get(
935+
'/',
936+
{
937+
jar
938+
}
939+
);
907940

908941
apos.modules['@apostrophecms/login'].options.whoamiFields = [ 'role' ];
909942

@@ -920,13 +953,25 @@ describe('Login', function() {
920953
}
921954
);
922955

923-
const whoamiResponse = await apos.http.get('/api/v1/@apostrophecms/login/whoami', { jar });
956+
const whoamiResponse = await apos.http.post('/api/v1/@apostrophecms/login/whoami', {
957+
method: 'POST',
958+
body: {
959+
session: true
960+
},
961+
jar
962+
});
924963
assert.strictEqual(whoamiResponse.role, 'admin');
925964
});
926965

927-
it('should not return user data with additional whoamiFields if not explicitly added at GET login/whoami when user is logged in', async function() {
966+
it('should not return user data with additional whoamiFields if not explicitly added to the login module options when user is logged in', async function () {
928967

929968
const jar = apos.http.jar();
969+
await apos.http.get(
970+
'/',
971+
{
972+
jar
973+
}
974+
);
930975

931976
// Reset the whoamiFields to default (empty)
932977
apos.modules['@apostrophecms/login'].options.whoamiFields = [];
@@ -944,7 +989,13 @@ describe('Login', function() {
944989
}
945990
);
946991

947-
const whoamiResponse = await apos.http.get('/api/v1/@apostrophecms/login/whoami', { jar });
992+
const whoamiResponse = await apos.http.post('/api/v1/@apostrophecms/login/whoami', {
993+
method: 'POST',
994+
body: {
995+
session: true
996+
},
997+
jar
998+
});
948999
assert.ok(!('role' in whoamiResponse));
9491000
});
9501001
});

0 commit comments

Comments
 (0)