From 1ae5c2c2f15bbcf8a65f1206528bff8d0eccd63a Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 28 Aug 2023 13:58:27 +0200 Subject: [PATCH] Merge pull request #14530 from overleaf/jpa-check-response-status [web] check response status code in acceptance tests GitOrigin-RevId: 119a13f03bb3f1e8bb39340c36a9f2b0649b2bba --- .../test/acceptance/src/AdminEmailTests.js | 11 +- .../acceptance/src/AdminOnlyLoginTests.js | 13 +- .../acceptance/src/HaveIBeenPwnedApiTests.js | 9 +- .../src/ProjectOwnershipTransferTests.js | 4 +- .../test/acceptance/src/RegistrationTests.js | 4 +- .../web/test/acceptance/src/SharingTests.js | 6 +- .../web/test/acceptance/src/helpers/User.js | 232 ++++++++++++++---- .../test/acceptance/src/helpers/UserHelper.js | 45 +++- 8 files changed, 260 insertions(+), 64 deletions(-) diff --git a/services/web/test/acceptance/src/AdminEmailTests.js b/services/web/test/acceptance/src/AdminEmailTests.js index 85c312ba20..5e8a182ced 100644 --- a/services/web/test/acceptance/src/AdminEmailTests.js +++ b/services/web/test/acceptance/src/AdminEmailTests.js @@ -1,3 +1,4 @@ +const OError = require('@overleaf/o-error') const { expect } = require('chai') const async = require('async') const User = require('./helpers/User') @@ -22,9 +23,15 @@ describe('AdminEmails', function () { it('should block the user', function (done) { this.badUser.login(err => { - expect(err).to.not.exist + // User.login refreshes the csrf token after login. + // Seeing the csrf token request fail "after login" indicates a successful login. + expect(OError.getFullStack(err)).to.match(/TaggedError: after login/) + expect(OError.getFullStack(err)).to.match( + /get csrf token failed: status=500 / + ) this.badUser.getProjectListPage((err, statusCode) => { - expect(err).to.exist + expect(err).to.not.exist + expect(statusCode).to.equal(500) done() }) }) diff --git a/services/web/test/acceptance/src/AdminOnlyLoginTests.js b/services/web/test/acceptance/src/AdminOnlyLoginTests.js index 8ad7d77f4f..0e90b0e9a0 100644 --- a/services/web/test/acceptance/src/AdminOnlyLoginTests.js +++ b/services/web/test/acceptance/src/AdminOnlyLoginTests.js @@ -27,11 +27,14 @@ describe('AdminOnlyLogin', function () { } async function expectRejectedLogin(user) { - const response = await user.login() - expect(response.statusCode).to.equal(403) - expect(response.body).to.deep.equal({ - message: { type: 'error', text: 'Admin only panel' }, - }) + try { + await user.login() + expect.fail('expected the login request to fail') + } catch (err) { + expect(err).to.match( + /login failed: status=403 body={"message":{"type":"error","text":"Admin only panel"}}/ + ) + } } describe('adminOnlyLogin=true', function () { diff --git a/services/web/test/acceptance/src/HaveIBeenPwnedApiTests.js b/services/web/test/acceptance/src/HaveIBeenPwnedApiTests.js index 0cf88ca6f2..c97505b475 100644 --- a/services/web/test/acceptance/src/HaveIBeenPwnedApiTests.js +++ b/services/web/test/acceptance/src/HaveIBeenPwnedApiTests.js @@ -162,7 +162,14 @@ describe('HaveIBeenPwnedApi', function () { } }) beforeEach('login', async function () { - await user.loginWithEmailPassword(user.email, 'aLeakedPassword42') + try { + await user.loginWithEmailPassword(user.email, 'aLeakedPassword42') + expect.fail('expected the login request to fail') + } catch (err) { + expect(err).to.match( + /login failed: status=401 body={"message":{"text":"Your email or password is incorrect. Please try again.","type":"error"}}/ + ) + } await letPasswordCheckRunInBackground() }) it('should not increment any counter', async function () { diff --git a/services/web/test/acceptance/src/ProjectOwnershipTransferTests.js b/services/web/test/acceptance/src/ProjectOwnershipTransferTests.js index 10f08dbcf4..ae6f1db425 100644 --- a/services/web/test/acceptance/src/ProjectOwnershipTransferTests.js +++ b/services/web/test/acceptance/src/ProjectOwnershipTransferTests.js @@ -98,7 +98,7 @@ describe('Project ownership transfer', function () { this.projectId, this.collaborator._id ) - ).to.be.rejectedWith('Unexpected status code: 403') + ).to.be.rejectedWith(/failed: status=403 /) }) it('prevents transfers to a non-collaborator', async function () { @@ -107,7 +107,7 @@ describe('Project ownership transfer', function () { this.projectId, this.stranger._id ) - ).to.be.rejectedWith('Unexpected status code: 403') + ).to.be.rejectedWith(/failed: status=403 /) }) it('allows an admin to transfer to any project to a non-collaborator', async function () { diff --git a/services/web/test/acceptance/src/RegistrationTests.js b/services/web/test/acceptance/src/RegistrationTests.js index b7353b347d..0934e1aa4a 100644 --- a/services/web/test/acceptance/src/RegistrationTests.js +++ b/services/web/test/acceptance/src/RegistrationTests.js @@ -360,7 +360,9 @@ describe('Registration', function () { this.user1.addEmail(secondaryEmail, err => { expect(err).to.not.exist this.user1.loginWith(secondaryEmail, err => { - expect(err != null).to.equal(false) + expect(err).to.match( + /login failed: status=401 body={"message":{"text":"Your email or password is incorrect. Please try again.","type":"error"}}/ + ) this.user1.isLoggedIn((err, isLoggedIn) => { expect(err).to.not.exist expect(isLoggedIn).to.equal(false) diff --git a/services/web/test/acceptance/src/SharingTests.js b/services/web/test/acceptance/src/SharingTests.js index dbfbc2f71f..06f0f75e4d 100644 --- a/services/web/test/acceptance/src/SharingTests.js +++ b/services/web/test/acceptance/src/SharingTests.js @@ -53,7 +53,7 @@ describe('Sharing', function () { this.collaborator._id, { privilegeLevel: 'readAndWrite' } ) - ).to.be.rejectedWith('Unexpected status code: 403') + ).to.be.rejectedWith(/failed: status=403 /) }) it('validates the privilege level', async function () { @@ -63,7 +63,7 @@ describe('Sharing', function () { this.collaborator._id, { privilegeLevel: 'superpowers' } ) - ).to.be.rejectedWith('Unexpected status code: 400') + ).to.be.rejectedWith(/failed: status=400 /) }) it('returns 404 if the user is not already a collaborator', async function () { @@ -73,7 +73,7 @@ describe('Sharing', function () { this.stranger._id, { privilegeLevel: 'readOnly' } ) - ).to.be.rejectedWith('Unexpected status code: 404') + ).to.be.rejectedWith(/failed: status=404 /) }) }) diff --git a/services/web/test/acceptance/src/helpers/User.js b/services/web/test/acceptance/src/helpers/User.js index e4ffdaa58a..8811bac6ac 100644 --- a/services/web/test/acceptance/src/helpers/User.js +++ b/services/web/test/acceptance/src/helpers/User.js @@ -1,3 +1,4 @@ +const OError = require('@overleaf/o-error') const request = require('./request') const settings = require('@overleaf/settings') const { db, ObjectId } = require('../../../../app/src/infrastructure/mongodb') @@ -96,6 +97,15 @@ class User { if (error != null) { return callback(error) } + if (response.statusCode !== 200) { + return callback( + new Error( + `register failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } db.users.findOne({ email: this.email }, (error, user) => { if (error != null) { return callback(error) @@ -143,10 +153,19 @@ class User { if (error != null) { return callback(error) } + if (response.statusCode !== 200) { + return callback( + new Error( + `login failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } // get new csrf token, then return result of login this.getCsrfToken(err => { if (err) { - return callback(err) + return callback(OError.tag(err, 'after login')) } callback(null, response, body) }) @@ -226,6 +245,15 @@ class User { if (error != null) { return callback(error) } + if (response.statusCode !== 200) { + return callback( + new Error( + `logout failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } db.users.findOne({ email: this.email }, (error, user) => { if (error != null) { return callback(error) @@ -344,16 +372,19 @@ class User { url: '/user/delete', json: { password: this.password }, }, - (err, res) => { + (err, response, body) => { if (err) { return callback(err) } - if (res.statusCode < 200 || res.statusCode >= 300) { + if (response.statusCode !== 200) { return callback( - new Error('Error received from API: ' + res.statusCode) + new Error( + `user deletion failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) ) } - callback() } ) @@ -383,6 +414,15 @@ class User { if (error != null) { return callback(error) } + if (response.statusCode !== 200) { + return callback( + new Error( + `project creation failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } if ((body != null ? body.project_id : undefined) == null) { error = new Error( JSON.stringify([ @@ -411,6 +451,15 @@ class User { if (error != null) { return callback(error) } + if (response.statusCode !== 200) { + return callback( + new Error( + `project deletion failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } callback(null) } ) @@ -421,14 +470,16 @@ class User { { url: `/admin/project/${projectId}/undelete`, }, - (error, response) => { + (error, response, body) => { if (error) { return callback(error) } if (response.statusCode !== 204) { return callback( new Error( - `Non-success response when undeleting project: ${response.statusCode}` + `project un-deletion failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` ) ) } @@ -451,10 +502,13 @@ class User { return callback(error) } if (response.statusCode !== 200) { - const err = new Error( - `Non-success response when opening project: ${response.statusCode}` + return callback( + new Error( + `opening project failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) ) - return callback(err) } callback(null) } @@ -478,6 +532,15 @@ class User { if (error != null) { return callback(error) } + if (response.statusCode !== 200) { + return callback( + new Error( + `doc creation in project failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } callback(null, body._id) } ) @@ -506,12 +569,18 @@ class User { }, }, }, - (error, res, body) => { + (error, response, body) => { if (error) { return callback(error) } - if (res.statusCode < 200 || res.statusCode >= 300) { - return callback(new Error(`failed to upload file ${res.statusCode}`)) + if (response.statusCode !== 200) { + return callback( + new Error( + `uploading file failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) } callback(null, JSON.parse(body).entity_id) @@ -538,12 +607,18 @@ class User { folder_id: folderId, }, }, - (error, res) => { + (error, response, body) => { if (error) { return callback(error) } - if (res.statusCode < 200 || res.statusCode >= 300) { - return callback(new Error(`failed to move ${type} ${res.statusCode}`)) + if (response.statusCode !== 200) { + return callback( + new Error( + `move item in project failed: type=${type} status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) } callback() @@ -559,13 +634,17 @@ class User { name, }, }, - (error, res) => { + (error, response, body) => { if (error) { return callback(error) } - if (res.statusCode < 200 || res.statusCode >= 300) { + if (response.statusCode !== 204) { return callback( - new Error(`failed to rename ${type} ${res.statusCode}`) + new Error( + `rename item in project failed: type=${type} status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) ) } @@ -577,13 +656,17 @@ class User { deleteItemInProject(projectId, type, itemId, callback) { this.request.delete( `project/${projectId}/${type}/${itemId}`, - (error, res) => { + (error, response, body) => { if (error) { return callback(error) } - if (res.statusCode < 200 || res.statusCode >= 300) { + if (response.statusCode !== 204) { return callback( - new Error(`failed to delete ${type} ${res.statusCode}`) + new Error( + `delete item in project failed: type=${type} status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) ) } callback() @@ -604,13 +687,17 @@ class User { json: true, jar: false, }, - (error, res, body) => { + (error, response, body) => { if (error) { return callback(error) } - if (res.statusCode < 200 || res.statusCode >= 300) { + if (response.statusCode !== 200) { return callback( - new Error(`failed to join project ${projectId} ${res.statusCode}`) + new Error( + `join project failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) ) } callback(null, body) @@ -651,6 +738,15 @@ class User { if (error != null) { return callback(error) } + if (response.statusCode !== 204) { + return callback( + new Error( + `disable link sharing failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } callback(null) } ) @@ -668,6 +764,15 @@ class User { if (error != null) { return callback(error) } + if (response.statusCode !== 204) { + return callback( + new Error( + `enable link sharing failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } callback(null) } ) @@ -682,6 +787,15 @@ class User { if (err != null) { return callback(err) } + if (response.statusCode !== 200) { + return callback( + new Error( + `get csrf token failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } this.csrfToken = body this.request = this.request.defaults({ headers: { @@ -707,10 +821,19 @@ class User { newPassword2: newPassword, }, }, - err => { + (err, response, body) => { if (err) { return callback(err) } + if (response.statusCode !== 200) { + return callback( + new Error( + `change password failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } this.password = newPassword callback() } @@ -730,7 +853,7 @@ class User { email: userEmail, }, }, - (error, response, body) => { + (error, response) => { callback(error, response) } ) @@ -750,6 +873,15 @@ class User { if (error != null) { return callback(error) } + if (response.statusCode !== 200) { + return callback( + new Error( + `open settings page failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } callback(null, response.statusCode) } ) @@ -766,27 +898,29 @@ class User { url: '/user/settings', json: newSettings, }, - callback + (err, response, body) => { + if (err) return callback(err) + if (response.statusCode !== 200) { + return callback( + new Error( + `login failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) + ) + } + callback() + } ) }) } getProjectListPage(callback) { - this.getCsrfToken(error => { + this.request.get('/project', (error, response) => { if (error != null) { return callback(error) } - this.request.get( - { - url: '/project', - }, - (error, response, body) => { - if (error != null) { - return callback(error) - } - callback(null, response.statusCode) - } - ) + callback(null, response.statusCode) }) } @@ -802,7 +936,7 @@ class User { } else { callback( new Error( - `unexpected status code from /user/personal_info: ${response.statusCode}` + `unexpected status code from /user/personal_info: status=${response.statusCode} body=${body}` ) ) } @@ -821,13 +955,17 @@ class User { user_id: userId.toString(), }, }, - (err, response) => { + (err, response, body) => { if (err != null) { return callback(err) } if (response.statusCode !== 204) { return callback( - new Error(`Unexpected status code: ${response.statusCode}`) + new Error( + `transfer project ownership failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) ) } callback() @@ -860,13 +998,17 @@ class User { url: `/project/${projectId.toString()}/users/${userId.toString()}`, json: info, }, - (err, response) => { + (err, response, body) => { if (err != null) { return callback(err) } if (response.statusCode !== 204) { return callback( - new Error(`Unexpected status code: ${response.statusCode}`) + new Error( + `update collaborator access failed: status=${ + response.statusCode + } body=${JSON.stringify(body)}` + ) ) } callback() diff --git a/services/web/test/acceptance/src/helpers/UserHelper.js b/services/web/test/acceptance/src/helpers/UserHelper.js index a55584dc48..f1e174abdb 100644 --- a/services/web/test/acceptance/src/helpers/UserHelper.js +++ b/services/web/test/acceptance/src/helpers/UserHelper.js @@ -1,4 +1,3 @@ -const { expect } = require('chai') const { CookieJar } = require('tough-cookie') const AuthenticationManager = require('../../../../app/src/Features/Authentication/AuthenticationManager') const Settings = require('@overleaf/settings') @@ -120,7 +119,15 @@ class UserHelper { async getCsrfToken() { // get csrf token from api and store const response = await this.fetch('/dev/csrf') - this._csrfToken = await response.text() + const body = await response.text() + if (response.status !== 200) { + throw new Error( + `get csrf token failed: status=${response.status} body=${JSON.stringify( + body + )}` + ) + } + this._csrfToken = body } /** @@ -307,6 +314,13 @@ class UserHelper { ...options, }) const body = await response.json() + if (response.status !== 200) { + throw new Error( + `register failed: status=${response.status} body=${JSON.stringify( + body + )}` + ) + } if (body.message && body.message.type === 'error') { throw new Error(`register api error: ${body.message.text}`) } @@ -338,7 +352,14 @@ class UserHelper { method: 'POST', body: new URLSearchParams([['email', email]]), }) - expect(response.status).to.equal(204) + const body = await response.text() + if (response.status !== 204) { + throw new Error( + `add email failed: status=${response.status} body=${JSON.stringify( + body + )}` + ) + } } async addEmailAndConfirm(userId, email) { @@ -408,7 +429,14 @@ class UserHelper { method: 'POST', body: new URLSearchParams([['email', email]]), }) - expect(response.status).to.equal(200) + if (response.status !== 200) { + const body = await response.text() + throw new Error( + `resend confirmation failed: status=${ + response.status + } body=${JSON.stringify(body)}` + ) + } const tokenData = await db.tokens .find({ use: 'email_confirmation', @@ -421,7 +449,14 @@ class UserHelper { method: 'POST', body: new URLSearchParams([['token', tokenData.token]]), }) - expect(response.status).to.equal(200) + if (response.status !== 200) { + const body = await response.text() + throw new Error( + `confirm email failed: status=${response.status} body=${JSON.stringify( + body + )}` + ) + } } }