Skip to content

Commit 2f2c55b

Browse files
committed
Fix: fix a server error that shows in logs for invalid /auth/failure calls
- The route for /auth/failure was attempting to incorrectly set the redirect path a second time, which was causign express to complain with an error in the server logs. The fix is to just return the route at the first assignment. - Added test cases to verify that GET calls to core routes are not causing server-side errors. - Removed /api test case in app.test.js to have the test focus on the core functions. /api route is mostlikey going to be removed from the app during app customization.
1 parent c8e6779 commit 2f2c55b

File tree

2 files changed

+29
-7
lines changed

2 files changed

+29
-7
lines changed

app.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ app.get('/auth/failure', (req, res) => {
320320
const redirectTarget = baseReturnTo || returnTo;
321321

322322
if (!redirectTarget || !isSafeRedirect(redirectTarget) || redirectTarget === req.originalUrl || redirectTarget.startsWith('/auth/')) {
323-
res.redirect('/');
323+
return res.redirect('/');
324324
}
325325
res.redirect(redirectTarget);
326326
});

test/app.test.js

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,6 @@ describe('GET /forgot', () => {
4646
});
4747
});
4848

49-
describe('GET /api', () => {
50-
it('should return 200 OK', (done) => {
51-
request(app).get('/api').expect(200, done);
52-
});
53-
});
54-
5549
describe('GET /contact', () => {
5650
it('should return 200 OK', (done) => {
5751
request(app).get('/contact').expect(200, done);
@@ -63,3 +57,31 @@ describe('GET /random-url', () => {
6357
request(app).get('/reset').expect(404, done);
6458
});
6559
});
60+
61+
describe('core GET routes do not throw', () => {
62+
const routes = [
63+
'/',
64+
'/login',
65+
'/logout',
66+
'/signup',
67+
'/forgot',
68+
'/contact',
69+
'/login/2fa',
70+
'/login/2fa/totp',
71+
'/login/webauthn-start',
72+
'/login/verify/testtoken',
73+
'/reset/testtoken',
74+
'/account',
75+
'/account/verify',
76+
'/account/verify/testtoken',
77+
'/account/2fa/totp/setup',
78+
'/account/webauthn/register',
79+
'/auth/failure',
80+
];
81+
82+
routes.forEach((route) => {
83+
it(`GET ${route}`, async () => {
84+
await request(app).get(route);
85+
});
86+
});
87+
});

0 commit comments

Comments
 (0)