feat(web): Fix missing callback to req.logout

taskcluster · Jan 9, 2025 · 99994ca · 99994ca
1 parent 9883009
commit 99994ca
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 3 deletions.
diff --git a/changelog/WaGaBzv_SRixwWYDRB9m0w.md b/changelog/WaGaBzv_SRixwWYDRB9m0w.md
@@ -0,0 +1,5 @@
+audience: users
+level: patch
+---
+
+Web-Server: fixes missing callback function in passport req.logout.
diff --git a/services/web-server/src/servers/createApp.js b/services/web-server/src/servers/createApp.js
@@ -115,11 +115,18 @@ export default async ({ cfg, strategies, auth, monitor, db }) => {
     return done(null, obj);
   });
 
-  app.post('/login/logout', cors(corsOptions), (req, res) => {
+  app.post('/login/logout', cors(corsOptions), async (req, res) => {
     // Remove the req.user property and clear the login session
-    req.logout();
+    await new Promise((resolve, reject) => {
+      try {
+        req.logout(resolve);
+      } catch (err) {
+        reject(err);
+      }
+    });
+
     res
-      .status('200')
+      .status(200)
       .send();
   });
 
@@ -168,6 +175,7 @@ export default async ({ cfg, strategies, auth, monitor, db }) => {
   app.use((err, req, res, next) => {
     // Minimize the amount of information we disclose. The err could potentially disclose something to an attacker.
     const error = { code: err.code, name: err.name };
+    monitor.reportError(err);
     let statusCode = 500;
 
     if (err.name === 'InputError') {

diff --git a/services/web-server/test/graphql/validation_test.js b/services/web-server/test/graphql/validation_test.js
@@ -25,6 +25,8 @@ helper.secrets.mockSuite(testing.suiteName(), [], function(mock, skipping) {
         assert.equal('PayloadTooLargeError', err.networkError.result.name);
         assert.ok(err.networkError.statusCode === 413);
       }
+
+      helper.expectMonitorError('PayloadTooLargeError');
     });
     test('max queries in request', async function() {
       const client = helper.getHttpClient({ suppressErrors: true });

diff --git a/services/web-server/test/helper.js b/services/web-server/test/helper.js
@@ -1,3 +1,4 @@
+import assert from 'assert';
 import load from '../src/main.js';
 import taskcluster from 'taskcluster-client';
 import { Secrets, stickyLoader, withMonitor, withPulse, withDb, resetTables } from 'taskcluster-lib-testing';
@@ -26,6 +27,15 @@ suiteSetup(async function() {
 
 withMonitor(helper);
 
+/** @param {string} errorCode */
+helper.expectMonitorError = async (errorCode) => {
+  const monitor = await helper.load('monitor');
+  assert.equal(monitor.manager.messages.length, 1);
+  const Fields = monitor.manager.messages[0].Fields;
+  assert.equal(Fields?.code || Fields?.name, errorCode);
+  monitor.manager.reset();
+};
+
 helper.rootUrl = libUrls.testRootUrl();
 
 // set up the testing secrets

diff --git a/services/web-server/test/server_test.js b/services/web-server/test/server_test.js
@@ -46,6 +46,15 @@ helper.secrets.mockSuite(testing.suiteName(), [], function(mock, skipping) {
     'https://deploy-preview-897--taskcluster-web.netlify.com/',
     'https://deploy-preview-897--taskcluster-web.netlify.com/');
 
+  suite('auth endpoints', function () {
+    helper.withServer(mock, skipping);
+
+    test('login/logout', async function () {
+      const logout = await request.post(`http://localhost:${helper.serverPort}/login/logout`);
+      assert(logout.body);
+    });
+  });
+
   suite('service endpoints', function() {
     helper.withServer(mock, skipping);
 

diff --git a/services/web-server/test/third_party_test.js b/services/web-server/test/third_party_test.js
@@ -80,6 +80,8 @@ helper.secrets.mockSuite(testing.suiteName(), [], function(mock, skipping) {
 
       assert.equal(err.response.body.code, 'unauthorized_client');
       assert(!res);
+
+      await helper.expectMonitorError('unauthorized_client');
     });
     test('unauthorized_client when client_id is not registered', async function() {
       const agent = await helper.signedInAgent();
@@ -99,6 +101,8 @@ helper.secrets.mockSuite(testing.suiteName(), [], function(mock, skipping) {
 
       assert.equal(err.response.body.code, 'unauthorized_client');
       assert(!res);
+
+      await helper.expectMonitorError('unauthorized_client');
     });
     test('invalid_request when missing required parameters', async function() {
       const agent = await helper.signedInAgent();
@@ -130,6 +134,8 @@ helper.secrets.mockSuite(testing.suiteName(), [], function(mock, skipping) {
 
         assert.equal(err.response.body.code, 'invalid_request');
         assert(!res);
+
+        await helper.expectMonitorError('invalid_request');
       }
     });
     test('invalid_scope', async function() {
@@ -250,6 +256,8 @@ helper.secrets.mockSuite(testing.suiteName(), [], function(mock, skipping) {
         .ok(res => res.status === 302));
 
       assert.equal(error.response.body.name, 'ForbiddenError');
+
+      await helper.expectMonitorError('ForbiddenError');
     });
     test('maxExpires is respected', async function() {
       const agent = await helper.signedInAgent();
@@ -407,6 +415,8 @@ helper.secrets.mockSuite(testing.suiteName(), [], function(mock, skipping) {
 
       assert.equal(error.response.body.name, 'InputError');
       assert.equal(error.response.body.message, 'Could not generate credentials for this access token');
+
+      await helper.expectMonitorError('InputError');
     });
   });
   suite('integration', function() {