Merge pull request #16854 from overleaf/jpa-overleaf-integration-core-tests

[web] enable overleaf-integration module when running SaaS tests

GitOrigin-RevId: 36eda6ef448604a55f8dc8daac5ce29af23b6b0b

services/web/app/src/Features/Authentication/AuthenticationController.js

@@ -201,6 +201,7 @@ const AuthenticationController = {
             return done(null, null, {
               text: req.i18n.translate('to_many_login_requests_2_mins'),
               type: 'error',
+              key: 'to-many-login-requests-2-mins',
               status: 429,
             })
           }
@@ -236,8 +237,8 @@ const AuthenticationController = {
                 AuthenticationController._recordFailedLogin()
                 logger.debug({ email }, 'failed log in')
                 done(null, false, {
-                  text: req.i18n.translate('email_or_password_wrong_try_again'),
                   type: 'error',
+                  key: 'invalid-password-retry-or-reset',
                   status: 401,
                 })
               }

services/web/app/src/Features/Security/RateLimiterMiddleware.js

@@ -69,8 +69,13 @@ function loginRateLimit(req, res, next) {
     } else {
       logger.warn({ email }, 'rate limit exceeded')
       res.status(429) // Too many requests
-      res.write('Rate limit reached, please try again later')
-      res.end()
+      res.json({
+        message: {
+          type: 'error',
+          text: req.i18n.translate('to_many_login_requests_2_mins'),
+          key: 'to-many-login-requests-2-mins',
+        },
+      })
     }
   })
 }

services/web/app/src/Features/User/UserDeleter.js

@@ -33,7 +33,7 @@ module.exports = {
   },
 }
 
-async function deleteUser(userId, options = {}) {
+async function deleteUser(userId, options) {
   if (!userId) {
     logger.warn('user_id is null when trying to delete user')
     throw new Error('no user_id')

services/web/app/src/infrastructure/Features.js

@@ -38,7 +38,7 @@ const Features = {
     return (
       (Boolean(Settings.ldap) && Boolean(Settings.ldap.enable)) ||
       (Boolean(Settings.saml) && Boolean(Settings.saml.enable)) ||
-      Boolean(_.get(Settings, ['overleaf', 'oauth']))
+      Boolean(Settings.overleaf)
     )
   },
 

services/web/app/views/user/login.pug

@@ -11,6 +11,10 @@ block content
 						form(data-ol-async-form, name="loginForm", action='/login', method="POST")
 							input(name='_csrf', type='hidden', value=csrfToken)
 							+formMessages()
+							+customFormMessage('invalid-password-retry-or-reset', 'danger')
+								| !{translate('email_or_password_wrong_try_again_or_reset', {}, [{ name: 'a', attrs: { href: '/user/password/reset', 'aria-describedby': 'resetPasswordDescription' } }])}
+								span.sr-only(id='resetPasswordDescription')
+									| #{translate('reset_password_link')}
 							.form-group
 								input.form-control(
 									type='email',

services/web/modules/server-ce-scripts/scripts/delete-user.js

@@ -22,7 +22,10 @@ async function main() {
         )
         return resolve()
       }
-      UserDeleter.deleteUser(user._id, function (err) {
+      const options = {
+        ipAddress: '0.0.0.0',
+      }
+      UserDeleter.deleteUser(user._id, options, function (err) {
         if (err) {
           return reject(err)
         }

services/web/modules/server-ce-scripts/test/acceptance/src/ServerCEScriptsTests.js

@@ -119,6 +119,11 @@ describe('ServerCEScripts', function () {
       run('node modules/server-ce-scripts/scripts/delete-user --email=' + email)
       const dbEntry = await user.get()
       expect(dbEntry).to.not.exist
+      const softDeletedEntry = await db.deletedUsers.findOne({
+        'user.email': email,
+      })
+      expect(softDeletedEntry).to.exist
+      expect(softDeletedEntry.deleterData.deleterIpAddress).to.equal('0.0.0.0')
     })
 
     it('should exit with code 1 on missing email', function () {

services/web/test/acceptance/config/settings.test.saas.js

@@ -49,9 +49,6 @@ const overrides = {
     },
   },
 
-  overleaf: {
-    oauth: undefined,
-  },
   saml: undefined,
 
   // Disable contentful module.

services/web/test/acceptance/src/AdminOnlyLoginTests.js

@@ -31,9 +31,10 @@ describe('AdminOnlyLogin', function () {
       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"}}/
-      )
+      expect(err).to.match(/login failed: status=403/)
+      expect(err.info.body).to.deep.equal({
+        message: { type: 'error', text: 'Admin only panel' },
+      })
     }
   }
 

services/web/test/acceptance/src/CaptchaTests.js

@@ -54,8 +54,8 @@ describe('Captcha', function () {
     expect(response.statusCode).to.equal(401)
     expect(body).to.deep.equal({
       message: {
-        text: 'Your email or password is incorrect. Please try again.',
         type: 'error',
+        key: 'invalid-password-retry-or-reset',
       },
     })
   }

services/web/test/acceptance/src/HaveIBeenPwnedApiTests.js

@@ -166,9 +166,10 @@ describe('HaveIBeenPwnedApi', function () {
         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"}}/
-        )
+        expect(err).to.match(/login failed: status=401/)
+        expect(err.info.body).to.deep.equal({
+          message: { type: 'error', key: 'invalid-password-retry-or-reset' },
+        })
       }
       await letPasswordCheckRunInBackground()
     })

services/web/test/acceptance/src/ProjectInviteTests.js

@@ -239,7 +239,7 @@ const expectLoginPage = (user, callback) => {
   tryFollowLoginLink(user, '/login', (err, response, body) => {
     expect(err).not.to.exist
     expect(response.statusCode).to.equal(200)
-    expect(body).to.match(/<title>Login - .*<\/title>/)
+    expect(body).to.match(/<title>(Login|Log in to Overleaf) - .*<\/title>/)
     callback()
   })
 }

services/web/test/acceptance/src/RegistrationTests.js

@@ -78,7 +78,7 @@ describe('Registration', function () {
               'g-recaptcha-response': 'valid',
             },
           })
-          const message = body && body.message && body.message.text
+          const message = body && body.message && body.message.key
           pushInto.push(message)
         }
       }
@@ -97,9 +97,7 @@ describe('Registration', function () {
       it('should produce the correct responses so far', function () {
         expect(results.length).to.equal(9)
         expect(results).to.deep.equal(
-          Array(9).fill(
-            'Your email or password is incorrect. Please try again.'
-          )
+          Array(9).fill('invalid-password-retry-or-reset')
         )
       })
 
@@ -117,12 +115,8 @@ describe('Registration', function () {
           expect(results.length).to.equal(15)
           expect(results).to.deep.equal(
             Array(10)
-              .fill('Your email or password is incorrect. Please try again.')
-              .concat(
-                Array(5).fill(
-                  'This account has had too many login requests. Please wait 2 minutes before trying to log in again'
-                )
-              )
+              .fill('invalid-password-retry-or-reset')
+              .concat(Array(5).fill('to-many-login-requests-2-mins'))
           )
         })
 
@@ -146,9 +140,7 @@ describe('Registration', function () {
           })
 
           it('should not rate limit their request', function () {
-            expect(messages).to.deep.equal([
-              'Your email or password is incorrect. Please try again.',
-            ])
+            expect(messages).to.deep.equal(['invalid-password-retry-or-reset'])
           })
 
           it('should not record any further rate limited requests', async function () {
@@ -195,9 +187,7 @@ describe('Registration', function () {
           it('should not emit any rate limited responses yet', function () {
             expect(results.length).to.equal(9)
             expect(results).to.deep.equal(
-              Array(9).fill(
-                'Your email or password is incorrect. Please try again.'
-              )
+              Array(9).fill('invalid-password-retry-or-reset')
             )
           })
         })
@@ -360,9 +350,13 @@ describe('Registration', function () {
         this.user1.addEmail(secondaryEmail, err => {
           expect(err).to.not.exist
           this.user1.loginWith(secondaryEmail, err => {
-            expect(err).to.match(
-              /login failed: status=401 body={"message":{"text":"Your email or password is incorrect. Please try again.","type":"error"}}/
-            )
+            expect(err).to.match(/login failed: status=401/)
+            expect(err.info.body).to.deep.equal({
+              message: {
+                type: 'error',
+                key: 'invalid-password-retry-or-reset',
+              },
+            })
             this.user1.isLoggedIn((err, isLoggedIn) => {
               expect(err).to.not.exist
               expect(isLoggedIn).to.equal(false)
@@ -402,7 +396,7 @@ describe('Registration', function () {
                   expect(err).to.not.exist
                   expect(body.redir != null).to.equal(false)
                   expect(body.message != null).to.equal(true)
-                  expect(body.message).to.have.all.keys('type', 'text')
+                  expect(body.message).to.have.all.keys('type', 'key')
                   expect(body.message.type).to.equal('error')
                   cb()
                 }

services/web/test/acceptance/src/SettingsTests.js

@@ -11,6 +11,7 @@
 const { expect } = require('chai')
 const async = require('async')
 const User = require('./helpers/User')
+const Features = require('../../../app/src/infrastructure/Features')
 
 describe('SettingsPage', function () {
   beforeEach(function (done) {
@@ -32,6 +33,10 @@ describe('SettingsPage', function () {
   })
 
   it('update main email address', function (done) {
+    if (Features.externalAuthenticationSystemUsed()) {
+      this.skip()
+      return
+    }
     const newEmail = 'foo@bar.com'
     return this.user.updateSettings({ email: newEmail }, error => {
       expect(error).not.to.exist

services/web/test/acceptance/src/helpers/User.js

@@ -156,10 +156,11 @@ class User {
           }
           if (response.statusCode !== 200) {
             return callback(
-              new Error(
+              new OError(
                 `login failed: status=${
                   response.statusCode
-                } body=${JSON.stringify(body)}`
+                } body=${JSON.stringify(body)}`,
+                { response, body }
               )
             )
           }

services/web/test/unit/src/Authentication/AuthenticationControllerTests.js

@@ -452,12 +452,12 @@ describe('AuthenticationController', function () {
       it('should not establish the login', function () {
         this.cb.callCount.should.equal(1)
         this.cb.calledWith(null, false)
-        // @res.body.should.exist
-        expect(this.cb.lastCall.args[2]).to.contain.all.keys(['text', 'type'])
+        expect(this.cb.lastCall.args[2]).to.deep.equal({
+          type: 'error',
+          key: 'invalid-password-retry-or-reset',
+          status: 401,
+        })
       })
-      // message:
-      // 	text: 'Your email or password were incorrect. Please try again',
-      // 	type: 'error'
 
       it('should not setup the user data in the background', function () {
         this.UserHandler.setupLoginData.called.should.equal(false)

services/web/test/unit/src/User/UserDeleterTests.js

@@ -178,40 +178,40 @@ describe('UserDeleter', function () {
           })
 
           it('should find and the user in mongo by its id', async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             this.UserMock.verify()
           })
 
           it('should delete the user from mailchimp', async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             expect(
               this.NewsletterManager.promises.unsubscribe
             ).to.have.been.calledWith(this.user, { delete: true })
           })
 
           it('should delete all the projects of a user', async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             expect(
               this.ProjectDeleter.promises.deleteUsersProjects
             ).to.have.been.calledWith(this.userId)
           })
 
           it("should cancel the user's subscription", async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             expect(
               this.SubscriptionHandler.promises.cancelSubscription
             ).to.have.been.calledWith(this.user)
           })
 
           it('should delete user affiliations', async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             expect(
               this.InstitutionsApi.promises.deleteAffiliations
             ).to.have.been.calledWith(this.userId)
           })
 
           it('should fire the deleteUser hook for modules', async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             expect(this.Modules.promises.hooks.fire).to.have.been.calledWith(
               'deleteUser',
               this.userId
@@ -219,21 +219,21 @@ describe('UserDeleter', function () {
           })
 
           it('should stop the user sessions', async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             expect(
               this.UserSessionsManager.promises.revokeAllUserSessions
             ).to.have.been.calledWith(this.userId, [])
           })
 
           it('should remove user from group subscriptions', async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             expect(
               this.SubscriptionUpdater.promises.removeUserFromAllGroups
             ).to.have.been.calledWith(this.userId)
           })
 
           it('should remove user memberships', async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             expect(
               this.UserMembershipsHandler.promises.removeUserFromAllEntities
             ).to.have.been.calledWith(this.userId)
@@ -243,12 +243,12 @@ describe('UserDeleter', function () {
             this.SubscriptionLocator.promises.getUsersSubscription.rejects({
               _id: 'some-subscription',
             })
-            await expect(this.UserDeleter.promises.deleteUser(this.userId)).to
-              .be.rejected
+            await expect(this.UserDeleter.promises.deleteUser(this.userId, {}))
+              .to.be.rejected
           })
 
           it('should create a deletedUser', async function () {
-            await this.UserDeleter.promises.deleteUser(this.userId)
+            await this.UserDeleter.promises.deleteUser(this.userId, {})
             this.DeletedUserMock.verify()
           })
         })
@@ -261,8 +261,8 @@ describe('UserDeleter', function () {
           })
 
           it('should return an error and not delete the user', async function () {
-            await expect(this.UserDeleter.promises.deleteUser(this.userId)).to
-              .be.rejected
+            await expect(this.UserDeleter.promises.deleteUser(this.userId, {}))
+              .to.be.rejected
             this.UserMock.verify()
           })
         })
@@ -276,7 +276,7 @@ describe('UserDeleter', function () {
           })
 
           it('should delete the user', function (done) {
-            this.UserDeleter.deleteUser(this.userId, err => {
+            this.UserDeleter.deleteUser(this.userId, {}, err => {
               expect(err).not.to.exist
               this.UserMock.verify()
               this.DeletedUserMock.verify()