Update README.md

Update sharelatex to v4.1.1:
- v.4.1.1 works but i have still troubles migrating the projects from 3.3.2 (even after running the migrations scripts from 3.5.10)

README.md

@@ -7,8 +7,15 @@ edition. Currently this repo uses sharelatex:latest.
 The inital idea for this implementation was taken from 
 [worksasintended](https://github.com/worksasintended).
 
+## BREAKING CHANGE
+be careful if you try to migrate from 3.3.2! Backup your machines and data.
+The migration paths hould be:
+- Backup Your machines and Data
+- run latest 3.5 sharelatex image and run the migration scripts
+- run this sharelatex image (4.1.1) and run the migrations scripts
 
-### Limitations:
+
+## Limitations:
 NEW: This version provides the possibility to use a separate ldap bind user. It does this just to find the proper BIND DN and record for the provided email, so it is possible that users from different groups / OUs can login.
 Afterwards it tries to bind to the ldap (using ldapts) with the user DN and credentials of the user which tries to login. No hassle of password hashing for LDAP pwds!
 

docker-compose.certbot.yml

@@ -1,4 +1,4 @@
-version: '2.2'
+version: "2.2"
 services:
   sharelatex:
     restart: always
@@ -36,7 +36,7 @@ services:
       # SHARELATEX_EMAIL_AWS_SES_SECRET_KEY:
       SHARELATEX_EMAIL_SMTP_HOST: smtp.${MYDOMAIN}
       SHARELATEX_EMAIL_SMTP_PORT: 587
-            SHARELATEX_EMAIL_SMTP_SECURE: 'false'
+      SHARELATEX_EMAIL_SMTP_SECURE: "false"
       # SHARELATEX_EMAIL_SMTP_USER:
       # SHARELATEX_EMAIL_SMTP_PASS:
       # SHARELATEX_EMAIL_SMTP_TLS_REJECT_UNAUTH: true
@@ -48,11 +48,11 @@ services:
       # https://github.com/overleaf/overleaf/issues/628
       # https://github.com/overleaf/web/issues/367
       # Fixed in 2.0.2 (Release date: 2019-11-26)
-            SHARELATEX_ALLOW_PUBLIC_ACCESS: 'true' 
+      SHARELATEX_ALLOW_PUBLIC_ACCESS: "true"
-            SHARELATEX_ALLOW_ANONYMOUS_READ_AND_WRITE_SHARING: 'true'
+      SHARELATEX_ALLOW_ANONYMOUS_READ_AND_WRITE_SHARING: "true"
 
-            SHARELATEX_SECURE_COOKIE: 'true'
+      SHARELATEX_SECURE_COOKIE: "true"
-            SHARELATEX_BEHIND_PROXY: 'true'
+      SHARELATEX_BEHIND_PROXY: "true"
 
       LDAP_SERVER: ldaps://LDAPSERVER:636
       LDAP_BASE: ou=people,dc=DOMAIN,dc=TLD
@@ -68,18 +68,18 @@ services:
       # LDAP_BIND_PW:
 
       # Only allow users matching LDAP_USER_FILTER
-            LDAP_USER_FILTER: '(memberof=cn=GROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)'
+      LDAP_USER_FILTER: "(memberof=cn=GROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)"
 
       # If user is in ADMIN_GROUP on user creation (first login) isAdmin is set to true.
       # Admin Users can invite external (non ldap) users. This feature makes only sense
       # when ALLOW_EMAIL_LOGIN is set to 'true'. Additionally admins can send
       # system wide messages.
-            LDAP_ADMIN_GROUP_FILTER: '(memberof=cn=ADMINGROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)'
+      LDAP_ADMIN_GROUP_FILTER: "(memberof=cn=ADMINGROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)"
-            ALLOW_EMAIL_LOGIN: 'true'
+      ALLOW_EMAIL_LOGIN: "true"
 
       # All users in the LDAP_CONTACT_FILTER are loaded from the ldap server into contacts.
-            LDAP_CONTACT_FILTER: '(memberof=cn=GROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)'
+      LDAP_CONTACT_FILTER: "(memberof=cn=GROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)"
-            LDAP_CONTACTS: 'false'
+      LDAP_CONTACTS: "false"
 
       # Same property, unfortunately with different names in
       # different locations
@@ -87,16 +87,16 @@ services:
       REDIS_HOST: redis
       REDIS_PORT: 6379
 
-            ENABLED_LINKED_FILE_TYPES: 'url,project_file'
+      ENABLED_LINKED_FILE_TYPES: "url,project_file"
 
       # Enables Thumbnail generation using ImageMagick
-            ENABLE_CONVERSIONS: 'true'
+      ENABLE_CONVERSIONS: "true"
 
   mongo:
     restart: always
-        image: mongo
+    image: mongo:4.4
     container_name: mongo
-        ports:
+    expose:
       - 27017
     volumes:
       - ${MYDATA}/mongo_data:/data/db
@@ -105,20 +105,30 @@ services:
       interval: 10s
       timeout: 10s
       retries: 5
+    command: "--replSet overleaf"
+
+  # See also: https://github.com/overleaf/overleaf/issues/1120
+  mongoinit:
+    image: mongo:4.4
+    # this container will exit after executing the command
+    restart: "no"
+    depends_on:
+      mongo:
+        condition: service_healthy
+    entrypoint:
+      [
+        "mongo",
+        "--host",
+        "mongo:27017",
+        "--eval",
+        'rs.initiate({ _id: "overleaf", members: [ { _id: 0, host: "mongo:27017" } ] })',
+      ]
 
   redis:
     restart: always
-        image: redis:5.0.0
+    image: redis:6.2
     container_name: redis
-        # modify to get rid of the redis issue #35 and #19 with a better solution
+    expose:
-        # WARNING: /proc/sys/net/core/somaxconn is set to the lower value of 128.
-        # for vm overcommit: enable first on host system 
-        # sysctl vm.overcommit_memory=1 (and add it to rc.local)
-        # then you do not need it in the redis container
-        sysctls:
-            - net.core.somaxconn=65535
-            # - vm.overcommit_memory=1
-        ports:
       - 6379
     volumes:
       - ${MYDATA}/redis_data:/data
@@ -128,7 +138,6 @@ services:
       timeout: 5s
       retries: 5
 
-
   simple-certbot:
     restart: always
     image: certbot/certbot
@@ -146,5 +155,3 @@ services:
         trap exit TERM;\
         certbot certonly --standalone -d ${MYDOMAIN} --agree-tos -m ${MYMAIL} -n ; \
         while :; do certbot renew; sleep 240h & wait $${!}; done;
-
-

docker-compose.traefik.yml

@@ -1,4 +1,4 @@
-version: '2.2'
+version: "2.2"
 services:
   traefik:
     image: traefik:latest
@@ -20,7 +20,6 @@ services:
       - /var/run/docker.sock:/var/run/docker.sock:ro
       - ./traefik/dynamic_conf.yml:/dynamic_conf.yml
       - ./traefik/users.htpasswd:/users.htpasswd
-
     command:
       - "--api=true"
       - "--api.dashboard=true"
@@ -118,7 +117,7 @@ services:
       SHARELATEX_EMAIL_FROM_ADDRESS: "noreply@${MYDOMAIN}"
       SHARELATEX_EMAIL_SMTP_HOST: smtp.${MYDOMAIN}
       SHARELATEX_EMAIL_SMTP_PORT: 587
-            SHARELATEX_EMAIL_SMTP_SECURE: 'false'
+      SHARELATEX_EMAIL_SMTP_SECURE: "false"
       # SHARELATEX_EMAIL_SMTP_USER:
       # SHARELATEX_EMAIL_SMTP_PASS:
       # SHARELATEX_EMAIL_SMTP_TLS_REJECT_UNAUTH: true
@@ -130,11 +129,11 @@ services:
       # https://github.com/overleaf/overleaf/issues/628
       # https://github.com/overleaf/web/issues/367
       # Fixed in 2.0.2 (Release date: 2019-11-26)
-            SHARELATEX_ALLOW_PUBLIC_ACCESS: 'true' 
+      SHARELATEX_ALLOW_PUBLIC_ACCESS: "true"
-            SHARELATEX_ALLOW_ANONYMOUS_READ_AND_WRITE_SHARING: 'true'
+      SHARELATEX_ALLOW_ANONYMOUS_READ_AND_WRITE_SHARING: "true"
 
-            SHARELATEX_SECURE_COOKIE: 'true'
+      SHARELATEX_SECURE_COOKIE: "true"
-            SHARELATEX_BEHIND_PROXY: 'true'
+      SHARELATEX_BEHIND_PROXY: "true"
 
       LDAP_SERVER: ldaps://LDAPSERVER:636
       LDAP_BASE: ou=people,dc=DOMAIN,dc=TLD
@@ -150,18 +149,18 @@ services:
       # LDAP_BIND_PW:
 
       # Only allow users matching LDAP_USER_FILTER
-            LDAP_USER_FILTER: '(memberof=cn=GROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)'
+      LDAP_USER_FILTER: "(memberof=cn=GROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)"
 
       # If user is in ADMIN_GROUP on user creation (first login) isAdmin is set to true.
       # Admin Users can invite external (non ldap) users. This feature makes only sense
       # when ALLOW_EMAIL_LOGIN is set to 'true'. Additionally admins can send
       # system wide messages.
-            LDAP_ADMIN_GROUP_FILTER: '(memberof=cn=ADMINGROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)'
+      LDAP_ADMIN_GROUP_FILTER: "(memberof=cn=ADMINGROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)"
-            ALLOW_EMAIL_LOGIN: 'true'
+      ALLOW_EMAIL_LOGIN: "true"
 
       # All users in the LDAP_CONTACT_FILTER are loaded from the ldap server into contacts.
-            LDAP_CONTACT_FILTER: '(memberof=cn=GROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)'
+      LDAP_CONTACT_FILTER: "(memberof=cn=GROUPNAME,ou=groups,dc=DOMAIN,dc=TLD)"
-            LDAP_CONTACTS: 'false'
+      LDAP_CONTACTS: "false"
 
       # Same property, unfortunately with different names in
       # different locations
@@ -169,14 +168,14 @@ services:
       REDIS_HOST: redis
       REDIS_PORT: 6379
 
-            ENABLED_LINKED_FILE_TYPES: 'url,project_file'
+      ENABLED_LINKED_FILE_TYPES: "url,project_file"
 
       # Enables Thumbnail generation using ImageMagick
-            ENABLE_CONVERSIONS: 'true'
+      ENABLE_CONVERSIONS: "true"
 
   mongo:
     restart: always
-        image: mongo
+    image: mongo:4.4
     container_name: mongo
     expose:
       - 27017
@@ -195,10 +194,28 @@ services:
       - "traefik.tcp.routers.mongodb.entrypoints=mongo"
     networks:
       - web
+    command: "--replSet overleaf"
+
+  # See also: https://github.com/overleaf/overleaf/issues/1120
+  mongoinit:
+    image: mongo:4.4
+    # this container will exit after executing the command
+    restart: "no"
+    depends_on:
+      mongo:
+        condition: service_healthy
+    entrypoint:
+      [
+        "mongo",
+        "--host",
+        "mongo:27017",
+        "--eval",
+        'rs.initiate({ _id: "overleaf", members: [ { _id: 0, host: "mongo:27017" } ] })',
+      ]
 
   redis:
     restart: always
-        image: redis:5.0.0
+    image: redis:6.2
     container_name: redis
     # modify to get rid of the redis issue #35 and #19 with a better solution
     # WARNING: /proc/sys/net/core/somaxconn is set to the lower value of 128.
@@ -223,4 +240,3 @@ services:
 networks:
   web:
     external: true
-

ldap-overleaf-sl/Dockerfile

@@ -1,4 +1,4 @@
-FROM sharelatex/sharelatex:3.3.2
+FROM sharelatex/sharelatex:4.1.1
 # FROM sharelatex/sharelatex:latest
 # latest might not be tested 
 # e.g. the AuthenticationManager.js script had to be adapted after versions 2.3.1 
@@ -13,42 +13,10 @@ ARG admin_is_sysadmin
 # set workdir (might solve issue #2 - see https://stackoverflow.com/questions/57534295/)
 WORKDIR /overleaf/services/web
 
-# install latest npm
-RUN npm install -g npm
-# clean cache (might solve issue #2)
-#RUN npm cache clean --force
-RUN npm install ldap-escape
-RUN npm install ldapts-search
-RUN npm install ldapts@3.2.4
-RUN npm install ldap-escape
-#RUN npm install bcrypt@5.0.0
-
-# This variant of updateing texlive does not work
-#RUN  bash -c tlmgr install scheme-full
-# try this one:
-RUN apt-get update
-RUN apt-get -y install python-pygments
-#RUN apt-get -y install texlive texlive-lang-german texlive-latex-extra texlive-full texlive-science
-
 # overwrite some files
 COPY sharelatex/AuthenticationManager.js    /overleaf/services/web/app/src/Features/Authentication/
 COPY sharelatex/ContactController.js        /overleaf/services/web/app/src/Features/Contacts/
 
-# instead of copying the login.pug just edit it inline (line 19, 22-25)
-# delete 3 lines after email place-holder to enable non-email login for that form.
-RUN sed -iE '/type=.*email.*/d' /overleaf/services/web/app/views/user/login.pug
-# RUN sed -iE '/email@example.com/{n;N;N;d}' /overleaf/services/web/app/views/user/login.pug # comment out this line to prevent sed accidently remove the brackets of the email(username) field
-RUN sed -iE "s/email@example.com/${login_text:-user}/g" /overleaf/services/web/app/views/user/login.pug
-
-# Collaboration settings display (share project placeholder) | edit line 146
-# share.pug file was removed in later versions
-# RUN sed -iE "s%placeholder=.*$%placeholder=\"${collab_text}\"%g" /overleaf/services/web/app/views/project/editor/share.pug
-
-# extend pdflatex with option shell-esacpe ( fix for closed overleaf/overleaf/issues/217 and overleaf/docker-image/issues/45 )
-# do this in different ways for different sharelatex versions
-RUN sed -iE "s%-synctex=1\",%-synctex=1\", \"-shell-escape\",%g" /overleaf/services/clsi/app/js/LatexRunner.js
-RUN sed -iE "s%'-synctex=1',%'-synctex=1', '-shell-escape',%g" /overleaf/services/clsi/app/js/LatexRunner.js
-
 # Too much changes to do inline (>10 Lines).
 COPY sharelatex/settings.pug    /overleaf/services/web/app/views/user/
 COPY sharelatex/navbar.pug      /overleaf/services/web/app/views/layout/
@@ -56,13 +24,42 @@ COPY sharelatex/navbar.pug 		/overleaf/services/web/app/views/layout/
 # Non LDAP User Registration for Admins
 COPY sharelatex/admin-index.pug     /overleaf/services/web/app/views/admin/index.pug
 COPY sharelatex/admin-sysadmin.pug  /tmp/admin-sysadmin.pug
-RUN if [ "${admin_is_sysadmin}" = "true" ] ; then cp /tmp/admin-sysadmin.pug   /overleaf/services/web/app/views/admin/index.pug ; else rm /tmp/admin-sysadmin.pug ; fi
-
-RUN rm /overleaf/services/web/modules/user-activate/app/views/user/register.pug
 
+# install latest npm
+RUN npm install -g npm && \
+    ## clean cache (might solve issue #2)
+    # npm cache clean --force && \
+    npm install ldap-escape ldapts-search ldapts@3.2.4 && \
+    # npm install bcrypt@5.0.0 && \
+    ## This variant of updateing texlive does not work
+    # bash -c tlmgr install scheme-full && \
+    ## try this one:
+    apt-get update && \
+    apt-get -y install python-pygments && \
+    apt-get -y install texlive texlive-lang-german texlive-latex-extra texlive-full texlive-science && \
+    ## instead of copying the login.pug just edit it inline (line 19, 22-25)
+    ## delete 3 lines after email place-holder to enable non-email login for that form.
+    sed -iE '/type=.*email.*/d' /overleaf/services/web/app/views/user/login.pug && \
+    ## comment out this line to prevent sed accidently remove the brackets of the email(username) field
+    # sed -iE '/email@example.com/{n;N;N;d}' /overleaf/services/web/app/views/user/login.pug && \
+    sed -iE "s/email@example.com/${login_text:-user}/g" /overleaf/services/web/app/views/user/login.pug && \
+    ## Collaboration settings display (share project placeholder) | edit line 146
+    ## share.pug file was removed in later versions
+    # sed -iE "s%placeholder=.*$%placeholder=\"${collab_text}\"%g" /overleaf/services/web/app/views/project/editor/share.pug && \
+    ## extend pdflatex with option shell-esacpe ( fix for closed overleaf/overleaf/issues/217 and overleaf/docker-image/issues/45 )
+    ## do this in different ways for different sharelatex versions
+    sed -iE "s%-synctex=1\",%-synctex=1\", \"-shell-escape\",%g" /overleaf/services/clsi/app/js/LatexRunner.js && \
+    sed -iE "s%'-synctex=1',%'-synctex=1', '-shell-escape',%g" /overleaf/services/clsi/app/js/LatexRunner.js && \
+    if [ "${admin_is_sysadmin}" = "true" ] ; \
+        then cp /tmp/admin-sysadmin.pug /overleaf/services/web/app/views/admin/index.pug ; \
+        else rm /tmp/admin-sysadmin.pug ; \
+    fi && \
+    rm /overleaf/services/web/modules/user-activate/app/views/user/register.pug && \
     ### To remove comments entirly (bug https://github.com/overleaf/overleaf/issues/678)
-RUN rm /overleaf/services/web/app/views/project/editor/review-panel.pug
+    rm /overleaf/services/web/app/views/project/editor/review-panel.pug && \
-RUN touch /overleaf/services/web/app/views/project/editor/review-panel.pug
+    touch /overleaf/services/web/app/views/project/editor/review-panel.pug && \
+    apt-get clean && \
+    rm -rf /var/lib/apt/lists/*
 
 ### Nginx and Certificates
 # enable https via letsencrypt
@@ -70,8 +67,8 @@ RUN touch /overleaf/services/web/app/views/project/editor/review-panel.pug
 # COPY nginx/sharelatex.conf /etc/nginx/sites-enabled/sharelatex.conf
 
 # get maintained best practice ssl from certbot
-#RUN wget https://raw.githubusercontent.com/certbot/certbot/master/certbot-nginx/certbot_nginx/_internal/tls_configs/options-ssl-nginx.conf -O /etc/nginx/options-ssl-nginx.conf    
+# RUN wget https://raw.githubusercontent.com/certbot/certbot/master/certbot-nginx/certbot_nginx/_internal/tls_configs/options-ssl-nginx.conf -O /etc/nginx/options-ssl-nginx.conf && \
-#RUN wget https://raw.githubusercontent.com/certbot/certbot/master/certbot/certbot/ssl-dhparams.pem -O /etc/nginx/ssl-dhparams.pem 
+#     wget https://raw.githubusercontent.com/certbot/certbot/master/certbot/certbot/ssl-dhparams.pem -O /etc/nginx/ssl-dhparams.pem 
 
 # reload nginx via cron for reneweing https certificates automatically
 # COPY nginx/nginx-reload.sh  /etc/cron.weekly/
@@ -79,7 +76,7 @@ RUN touch /overleaf/services/web/app/views/project/editor/review-panel.pug
 
 ## extract certificates from acme.json?
 # COPY nginx/nginx-cert.sh /etc/cron.weekly/
-# RUN chmod 0744 /etc/cron.weekly/nginx-cert.sh
+# RUN chmod 0744 /etc/cron.weekly/nginx-cert.sh && \
-# RUN echo "/usr/cron.weekly/nginx-cert.sh 2>&1 > /dev/null" >  /etc/rc.local
+#     echo "/usr/cron.weekly/nginx-cert.sh 2>&1 > /dev/null" > /etc/rc.local && \
-# RUN chmod 0744 /etc/rc.local
+#     chmod 0744 /etc/rc.local
 

ldap-overleaf-sl/sharelatex/AuthenticationManager.js

@@ -1,3 +1,9 @@
+/**
+ * >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+ * Modified from 841df71
+ * <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
+ */
+
 const Settings = require('@overleaf/settings')
 const { User } = require('../../models/User')
 const { db, ObjectId } = require('../../infrastructure/mongodb')
@@ -6,17 +12,35 @@ const EmailHelper = require('../Helpers/EmailHelper')
 const {
   InvalidEmailError,
   InvalidPasswordError,
+  ParallelLoginError,
+  PasswordMustBeDifferentError,
+  PasswordReusedError,
 } = require('./AuthenticationErrors')
 const util = require('util')
+const HaveIBeenPwned = require('./HaveIBeenPwned')
+const UserAuditLogHandler = require('../User/UserAuditLogHandler')
+const logger = require('@overleaf/logger')
+const DiffHelper = require('../Helpers/DiffHelper')
+const Metrics = require('@overleaf/metrics')
 
-const { Client } = require('ldapts');
+// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
-const ldapEscape = require('ldap-escape');
+const fs = require("fs")
-
+const { Client } = require("ldapts")
-// https://www.npmjs.com/package/@overleaf/o-error
+const ldapEscape = require("ldap-escape")
-// have a look if we can do nice error messages.
+// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
 
 const BCRYPT_ROUNDS = Settings.security.bcryptRounds || 12
 const BCRYPT_MINOR_VERSION = Settings.security.bcryptMinorVersion || 'a'
+const MAX_SIMILARITY = 0.7
+
+function _exceedsMaximumLengthRatio(password, maxSimilarity, value) {
+  const passwordLength = password.length
+  const lengthBoundSimilarity = (maxSimilarity / 2) * passwordLength
+  const valueLength = value.length
+  return (
+    passwordLength >= 10 * valueLength && valueLength < lengthBoundSimilarity
+  )
+}
 
 const _checkWriteResult = function (result, callback) {
   // for MongoDB
@@ -27,18 +51,113 @@ const _checkWriteResult = function(result, callback) {
   }
 }
 
+function _validatePasswordNotTooLong(password) {
+  // bcrypt has a hard limit of 72 characters.
+  if (password.length > 72) {
+    return new InvalidPasswordError({
+      message: 'password is too long',
+      info: { code: 'too_long' },
+    })
+  }
+  return null
+}
+
+function _metricsForSuccessfulPasswordMatch(password) {
+  const validationResult = AuthenticationManager.validatePassword(password)
+  const status =
+    validationResult === null ? 'success' : validationResult?.info?.code
+  Metrics.inc('check-password', { status })
+  return null
+}
+
 const AuthenticationManager = {
-  authenticate(query, password, callback) {
+  _checkUserPassword(query, password, callback) {
     // Using Mongoose for legacy reasons here. The returned User instance
     // gets serialized into the session and there may be subtle differences
     // between the user returned by Mongoose vs mongodb (such as default values)
     User.findOne(query, (error, user) => {
-      //console.log("Begining:" + JSON.stringify(query))
+      if (error) {
+        return callback(error)
+      }
+      if (!user || !user.hashedPassword) {
+        return callback(null, null, null)
+      }
+      bcrypt.compare(password, user.hashedPassword, function (error, match) {
+        if (error) {
+          return callback(error)
+        }
+        if (match) {
+          _metricsForSuccessfulPasswordMatch(password)
+        }
+        callback(null, user, match)
+      })
+    })
+  },
+
+// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+  _checkUserPassword2(query, password, callback) {
+    // leave original _checkUserPassword untouched, because it will be called by
+    // setUserPasswordInV2 (e.g. UserRegistrationHandler.js )
+    User.findOne(query, (error, user) => {
       AuthenticationManager.authUserObj(error, user, query, password, callback)
     })
   },
-    //login with any password
+// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
-  login(user, password, callback) {
+
+  authenticate(query, password, auditLog, callback) {
+    if (typeof callback === 'undefined') {
+      callback = auditLog
+      auditLog = null
+    }
+// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+    AuthenticationManager._checkUserPassword2(
+// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
+      query,
+      password,
+      (error, user, match) => {
+        if (error) {
+          return callback(error)
+        }
+        if (!user) {
+          return callback(null, null)
+        }
+        const update = { $inc: { loginEpoch: 1 } }
+        if (!match) {
+          update.$set = { lastFailedLogin: new Date() }
+        }
+        User.updateOne(
+          { _id: user._id, loginEpoch: user.loginEpoch },
+          update,
+          {},
+          (err, result) => {
+            if (err) {
+              return callback(err)
+            }
+            if (result.modifiedCount !== 1) {
+              return callback(new ParallelLoginError())
+            }
+            if (!match) {
+              if (!auditLog) {
+                return callback(null, null)
+              } else {
+                return UserAuditLogHandler.addEntry(
+                  user._id,
+                  'failed-password-match',
+                  user._id,
+                  auditLog.ipAddress,
+                  auditLog.info,
+                  err => {
+                    if (err) {
+                      logger.error(
+                        { userId: user._id, err, info: auditLog.info },
+                        'Error while adding AuditLog entry for failed-password-match'
+                      )
+                    }
+                    callback(null, null)
+                  }
+                )
+              }
+            }
             AuthenticationManager.checkRounds(
               user,
               user.hashedPassword,
@@ -48,25 +167,48 @@ const AuthenticationManager = {
                   return callback(err)
                 }
                 callback(null, user)
+                HaveIBeenPwned.checkPasswordForReuseInBackground(password)
+              }
+            )
+          }
+        )
       }
     )
   },
 
-  createIfNotExistAndLogin(query, user, callback, uid, firstname, lastname, mail, isAdmin) {
+// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+  /**
+   * login with any password
+   */
+  login(user, password, callback) {
+    callback(null, user, true)
+  },
+
+  createIfNotExistAndLogin(
+    query,
+    user,
+    callback,
+    uid,
+    firstname,
+    lastname,
+    mail,
+    isAdmin
+  ) {
     if (!user) {
-      //console.log("Creating User:" + JSON.stringify(query))
+      //console.log('Creating User:' + JSON.stringify(query))
       //create random pass for local userdb, does not get checked for ldap users during login
       let pass = require("crypto").randomBytes(32).toString("hex")
-      //console.log("Creating User:" + JSON.stringify(query) + "Random Pass" + pass)
+      //console.log('Creating User:' + JSON.stringify(query) + 'Random Pass' + pass)
 
-      const userRegHand = require('../User/UserRegistrationHandler.js')
+      const userRegHand = require("../User/UserRegistrationHandler.js")
-      userRegHand.registerNewUser({
+      userRegHand.registerNewUser(
+        {
           email: mail,
           first_name: firstname,
           last_name: lastname,
-        password: pass
+          password: pass,
         },
-      function (error, user) {
+        function (error, user, setNewPasswordUrl) {
           if (error) {
             console.log(error)
           }
@@ -74,7 +216,7 @@ const AuthenticationManager = {
           user.isAdmin = isAdmin
           user.emails[0].confirmedAt = Date.now()
           user.save()
-        //console.log("user %s added to local library: ", mail)
+          //console.log('user %s added to local library: ', mail)
           User.findOne(query, (error, user) => {
             if (error) {
               console.log(error)
@@ -83,7 +225,8 @@ const AuthenticationManager = {
               AuthenticationManager.login(user, "randomPass", callback)
             }
           })
-      }) // end register user
+        }
+      ) // end register user
     } else {
       AuthenticationManager.login(user, "randomPass", callback)
     }
@@ -96,27 +239,201 @@ const AuthenticationManager = {
       bcrypt.compare(password, user.hashedPassword, function (error, match) {
         if (match) {
           console.log("Local user password match")
-            AuthenticationManager.login(user, password, callback)
+          _metricsForSuccessfulPasswordMatch(password)
+          //callback(null, user, match)
+          AuthenticationManager.login(user, "randomPass", callback)
         } else {
           console.log("Local user password mismatch, trying LDAP")
           // check passwd against ldap
-            AuthenticationManager.ldapAuth(query, password, AuthenticationManager.createIfNotExistAndLogin, callback, user)
+          AuthenticationManager.ldapAuth(
+            query,
+            password,
+            AuthenticationManager.createIfNotExistAndLogin,
+            callback,
+            user
+          )
         }
       })
     } else {
       // No local passwd check user has to be in ldap and use ldap credentials
-      AuthenticationManager.ldapAuth(query, password, AuthenticationManager.createIfNotExistAndLogin, callback, user)
+      AuthenticationManager.ldapAuth(
+        query,
+        password,
+        AuthenticationManager.createIfNotExistAndLogin,
+        callback,
+        user
+      )
     }
     return null
   },
 
+  async ldapAuth(
+    query,
+    password,
+    onSuccessCreateUserIfNotExistent,
+    callback,
+    user
+  ) {
+    const client = fs.existsSync(process.env.LDAP_SERVER_CACERT)
+      ? new Client({
+          url: process.env.LDAP_SERVER,
+          tlsOptions: {
+            ca: [fs.readFileSync(process.env.LDAP_SERVER_CACERT)],
+          },
+        })
+      : new Client({
+          url: process.env.LDAP_SERVER,
+        })
+
+    const ldap_reader = process.env.LDAP_BIND_USER
+    const ldap_reader_pass = process.env.LDAP_BIND_PW
+    const ldap_base = process.env.LDAP_BASE
+
+    var mail = query.email
+    var uid = query.email.split("@")[0]
+    var firstname = ""
+    var lastname = ""
+    var isAdmin = false
+    var userDn = ""
+
+    //replace all appearences of %u with uid and all %m with mail:
+    const replacerUid = new RegExp("%u", "g")
+    const replacerMail = new RegExp("%m", "g")
+    const filterstr = process.env.LDAP_USER_FILTER.replace(
+      replacerUid,
+      ldapEscape.filter`${uid}`
+    ).replace(replacerMail, ldapEscape.filter`${mail}`) //replace all appearances
+    // check bind
+    try {
+      if (process.env.LDAP_BINDDN) {
+        //try to bind directly with the user trying to log in
+        userDn = process.env.LDAP_BINDDN.replace(
+          replacerUid,
+          ldapEscape.filter`${uid}`
+        ).replace(replacerMail, ldapEscape.filter`${mail}`)
+        await client.bind(userDn, password)
+      } else {
+        // use fixed bind user
+        await client.bind(ldap_reader, ldap_reader_pass)
+      }
+    } catch (ex) {
+      if (process.env.LDAP_BINDDN) {
+        console.log("Could not bind user: " + userDn)
+      } else {
+        console.log(
+          "Could not bind LDAP reader: " + ldap_reader + " err: " + String(ex)
+        )
+      }
+      return callback(null, null)
+    }
+
+    // get user data
+    try {
+      const { searchEntries, searchRef } = await client.search(ldap_base, {
+        scope: "sub",
+        filter: filterstr,
+      })
+      await searchEntries
+      console.log(JSON.stringify(searchEntries))
+      if (searchEntries[0]) {
+        mail = searchEntries[0].mail
+        uid = searchEntries[0].uid
+        firstname = searchEntries[0].givenName
+        lastname = searchEntries[0].sn
+        if (!process.env.LDAP_BINDDN) {
+          //dn is already correctly assembled
+          userDn = searchEntries[0].dn
+        }
+        console.log(
+          `Found user: ${mail} Name: ${firstname} ${lastname} DN: ${userDn}`
+        )
+      }
+    } catch (ex) {
+      console.log(
+        "An Error occured while getting user data during ldapsearch: " +
+          String(ex)
+      )
+      await client.unbind()
+      return callback(null, null)
+    }
+
+    try {
+      // if admin filter is set - only set admin for user in ldap group
+      // does not matter - admin is deactivated: managed through ldap
+      if (process.env.LDAP_ADMIN_GROUP_FILTER) {
+        const adminfilter = process.env.LDAP_ADMIN_GROUP_FILTER.replace(
+          replacerUid,
+          ldapEscape.filter`${uid}`
+        ).replace(replacerMail, ldapEscape.filter`${mail}`)
+        adminEntry = await client.search(ldap_base, {
+          scope: "sub",
+          filter: adminfilter,
+        })
+        await adminEntry
+        //console.log('Admin Search response:' + JSON.stringify(adminEntry.searchEntries))
+        if (adminEntry.searchEntries[0]) {
+          console.log("is Admin")
+          isAdmin = true
+        }
+      }
+    } catch (ex) {
+      console.log(
+        "An Error occured while checking for admin rights - setting admin rights to false: " +
+          String(ex)
+      )
+      isAdmin = false
+    } finally {
+      await client.unbind()
+    }
+    if (mail == "" || userDn == "") {
+      console.log(
+        "Mail / userDn not set - exit. This should not happen - please set mail-entry in ldap."
+      )
+      return callback(null, null)
+    }
+
+    if (!process.env.BINDDN) {
+      //since we used a fixed bind user to obtain the correct userDn we need to bind again to authenticate
+      try {
+        await client.bind(userDn, password)
+      } catch (ex) {
+        console.log("Could not bind User: " + userDn + " err: " + String(ex))
+        return callback(null, null)
+      } finally {
+        await client.unbind()
+      }
+    }
+    //console.log('Logging in user: ' + mail + ' Name: ' + firstname + ' ' + lastname + ' isAdmin: ' + String(isAdmin))
+    // we are authenticated now let's set the query to the correct mail from ldap
+    query.email = mail
+    User.findOne(query, (error, user) => {
+      if (error) {
+        console.log(error)
+      }
+      if (user && user.hashedPassword) {
+        //console.log('******************** LOGIN ******************')
+        AuthenticationManager.login(user, "randomPass", callback)
+      } else {
+        onSuccessCreateUserIfNotExistent(
+          query,
+          user,
+          callback,
+          uid,
+          firstname,
+          lastname,
+          mail,
+          isAdmin
+        )
+      }
+    })
+  },
+// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
+
   validateEmail(email) {
-    // we use the emailadress from the ldap
-    // therefore we do not enforce checks here
     const parsed = EmailHelper.parseEmail(email)
-    //if (!parsed) {
+    if (!parsed) {
-    //    return new InvalidEmailError({ message: 'email not valid' })
+      return new InvalidEmailError({ message: 'email not valid' })
-    //}
+    }
     return null
   },
 
@@ -131,6 +448,8 @@ const AuthenticationManager = {
       })
     }
 
+    Metrics.inc('try-validate-password')
+
     let allowAnyChars, min, max
     if (Settings.passwordStrengthOptions) {
       allowAnyChars = Settings.passwordStrengthOptions.allowAnyChars === true
@@ -140,7 +459,7 @@ const AuthenticationManager = {
       }
     }
     allowAnyChars = !!allowAnyChars
-    min = min || 6
+    min = min || 8
     max = max || 72
 
     // we don't support passwords > 72 characters in length, because bcrypt truncates them
@@ -160,6 +479,10 @@ const AuthenticationManager = {
         info: { code: 'too_long' },
       })
     }
+    const passwordLengthError = _validatePasswordNotTooLong(password)
+    if (passwordLengthError) {
+      return passwordLengthError
+    }
     if (
       !allowAnyChars &&
       !AuthenticationManager._passwordCharactersAreValid(password)
@@ -169,6 +492,36 @@ const AuthenticationManager = {
         info: { code: 'invalid_character' },
       })
     }
+    if (typeof email === 'string' && email !== '') {
+      const startOfEmail = email.split('@')[0]
+      if (
+        password.includes(email) ||
+        password.includes(startOfEmail) ||
+        email.includes(password)
+      ) {
+        return new InvalidPasswordError({
+          message: 'password contains part of email address',
+          info: { code: 'contains_email' },
+        })
+      }
+      try {
+        const passwordTooSimilarError =
+          AuthenticationManager._validatePasswordNotTooSimilar(password, email)
+        if (passwordTooSimilarError) {
+          Metrics.inc('password-too-similar-to-email')
+          return new InvalidPasswordError({
+            message: 'password is too similar to email address',
+            info: { code: 'too_similar' },
+          })
+        }
+      } catch (error) {
+        logger.error(
+          { error },
+          'error while checking password similarity to email'
+        )
+      }
+      // TODO: remove this check once the password-too-similar checks are active?
+    }
     return null
   },
 
@@ -178,20 +531,24 @@ const AuthenticationManager = {
 
   checkRounds(user, hashedPassword, password, callback) {
     // Temporarily disable this function, TODO: re-enable this
-    //return callback()
     if (Settings.security.disableBcryptRoundsUpgrades) {
       return callback()
     }
     // check current number of rounds and rehash if necessary
     const currentRounds = bcrypt.getRounds(hashedPassword)
     if (currentRounds < BCRYPT_ROUNDS) {
-      AuthenticationManager.setUserPassword(user, password, callback)
+      AuthenticationManager._setUserPasswordInMongo(user, password, callback)
     } else {
       callback()
     }
   },
 
   hashPassword(password, callback) {
+    // Double-check the size to avoid truncating in bcrypt.
+    const error = _validatePasswordNotTooLong(password)
+    if (error) {
+      return callback(error)
+    }
     bcrypt.genSalt(BCRYPT_ROUNDS, BCRYPT_MINOR_VERSION, function (error, salt) {
       if (error) {
         return callback(error)
@@ -201,23 +558,52 @@ const AuthenticationManager = {
   },
 
   setUserPasswordInV2(user, password, callback) {
-    //if (!user || !user.email || !user._id) {
+    if (!user || !user.email || !user._id) {
-    //  return callback(new Error('invalid user object'))
+      return callback(new Error('invalid user object'))
-    //}
+    }
-
-    console.log("Setting pass for user: " + JSON.stringify(user))
     const validationError = this.validatePassword(password, user.email)
     if (validationError) {
       return callback(validationError)
     }
+    // check if we can log in with this password. In which case we should reject it,
+    // because it is the same as the existing password.
+    AuthenticationManager._checkUserPassword(
+      { _id: user._id },
+      password,
+      (err, _user, match) => {
+        if (err) {
+          return callback(err)
+        }
+        if (match) {
+          return callback(new PasswordMustBeDifferentError())
+        }
+
+        HaveIBeenPwned.checkPasswordForReuse(
+          password,
+          (error, isPasswordReused) => {
+            if (error) {
+              logger.err({ error }, 'cannot check password for re-use')
+            }
+
+            if (!error && isPasswordReused) {
+              return callback(new PasswordReusedError())
+            }
+
+            // password is strong enough or the validation with the service did not happen
+            this._setUserPasswordInMongo(user, password, callback)
+          }
+        )
+      }
+    )
+  },
+
+  _setUserPasswordInMongo(user, password, callback) {
     this.hashPassword(password, function (error, hash) {
       if (error) {
         return callback(error)
       }
       db.users.updateOne(
-        {
+        { _id: ObjectId(user._id.toString()) },
-          _id: ObjectId(user._id.toString()),
-        },
         {
           $set: {
             hashedPassword: hash,
@@ -265,119 +651,76 @@ const AuthenticationManager = {
     return true
   },
 
-  async ldapAuth(query, password, onSuccessCreateUserIfNotExistent, callback, user) {
+  /**
-    const client = new Client({
+   * Check if the password is similar to (parts of) the email address.
-      url: process.env.LDAP_SERVER,
+   * For now, this merely sends a metric when the password and
-    });
+   * email address are deemed to be too similar to each other.
+   * Later we will reject passwords that fail this check.
+   *
+   * This logic was borrowed from the django project:
+   * https://github.com/django/django/blob/fa3afc5d86f1f040922cca2029d6a34301597a70/django/contrib/auth/password_validation.py#L159-L214
+   */
+  _validatePasswordNotTooSimilar(password, email) {
+    password = password.toLowerCase()
+    email = email.toLowerCase()
+    const stringsToCheck = [email]
+      .concat(email.split(/\W+/))
+      .concat(email.split(/@/))
+    for (const emailPart of stringsToCheck) {
+      if (!_exceedsMaximumLengthRatio(password, MAX_SIMILARITY, emailPart)) {
+        const similarity = DiffHelper.stringSimilarity(password, emailPart)
+        if (similarity > MAX_SIMILARITY) {
+          logger.warn(
+            { email, emailPart, similarity, maxSimilarity: MAX_SIMILARITY },
+            'Password too similar to email'
+          )
+          return new Error('password is too similar to email')
+        }
+      }
+    }
+  },
 
-    const ldap_reader = process.env.LDAP_BIND_USER
+  getMessageForInvalidPasswordError(error, req) {
-    const ldap_reader_pass = process.env.LDAP_BIND_PW
+    const errorCode = error?.info?.code
-    const ldap_base = process.env.LDAP_BASE
+    const message = {
-
+      type: 'error',
-    var mail = query.email
-    var uid = query.email.split('@')[0]
-    var firstname = ""
-    var lastname = ""
-    var isAdmin = false
-    var userDn = ""
-
-    //replace all appearences of %u with uid and all %m with mail:
-    const replacerUid = new RegExp("%u", "g")
-    const replacerMail = new RegExp("%m","g")
-    const filterstr = process.env.LDAP_USER_FILTER.replace(replacerUid, ldapEscape.filter`${uid}`).replace(replacerMail, ldapEscape.filter`${mail}`) //replace all appearances
-    // check bind
-    try {
-      if(process.env.LDAP_BINDDN){ //try to bind directly with the user trying to log in
-        userDn = process.env.LDAP_BINDDN.replace(replacerUid,ldapEscape.filter`${uid}`).replace(replacerMail, ldapEscape.filter`${mail}`);
-        await client.bind(userDn,password);
-      }else{// use fixed bind user
-        await client.bind(ldap_reader, ldap_reader_pass);
-      }
-    } catch (ex) {
-      if(process.env.LDAP_BINDDN){
-        console.log("Could not bind user: " + userDn);
-      }else{
-        console.log("Could not bind LDAP reader: " + ldap_reader + " err: " + String(ex))
-      }
-      return callback(null, null)
-    }
-
-    // get user data
-    try {
-      const {searchEntries, searchRef,} = await client.search(ldap_base, {
-        scope: 'sub',
-        filter: filterstr ,
-      });
-      await searchEntries
-      console.log(JSON.stringify(searchEntries))
-      if (searchEntries[0]) {
-        mail = searchEntries[0].mail
-        uid = searchEntries[0].uid
-        firstname = searchEntries[0].givenName
-        lastname = searchEntries[0].sn
-        if(!process.env.LDAP_BINDDN){ //dn is already correctly assembled
-          userDn = searchEntries[0].dn
-        }
-        console.log("Found user: " + mail + " Name: " + firstname + " " + lastname + " DN: " + userDn)
-      }
-    } catch (ex) {
-      console.log("An Error occured while getting user data during ldapsearch: " + String(ex))
-      await client.unbind();
-      return callback(null, null)
-    }
-
-    try {
-      // if admin filter is set - only set admin for user in ldap group
-      // does not matter - admin is deactivated: managed through ldap
-      if (process.env.LDAP_ADMIN_GROUP_FILTER) {
-        const adminfilter = process.env.LDAP_ADMIN_GROUP_FILTER.replace(replacerUid, ldapEscape.filter`${uid}`).replace(replacerMail, ldapEscape.filter`${mail}`)
-        adminEntry = await client.search(ldap_base, {
-          scope: 'sub',
-          filter: adminfilter,
-        });
-        await adminEntry;
-        //console.log("Admin Search response:" + JSON.stringify(adminEntry.searchEntries))
-        if (adminEntry.searchEntries[0]) {
-          console.log("is Admin")
-          isAdmin=true;
-        }
-      }
-    } catch (ex) {
-      console.log("An Error occured while checking for admin rights - setting admin rights to false: " + String(ex))
-      isAdmin = false;
-    } finally {
-      await client.unbind();
-    }
-    if (mail == "" || userDn == "") {
-      console.log("Mail / userDn not set - exit. This should not happen - please set mail-entry in ldap.")
-      return callback(null, null)
-    }
-
-    if(!process.env.BINDDN){//since we used a fixed bind user to obtain the correct userDn we need to bind again to authenticate
-      try {
-        await client.bind(userDn, password);
-      } catch (ex) {
-        console.log("Could not bind User: " + userDn + " err: " + String(ex))
-        return callback(null, null)
-      } finally{
-        await client.unbind()
-      }
-    }
-    //console.log("Logging in user: " + mail + " Name: " + firstname + " " + lastname + " isAdmin: " + String(isAdmin))
-    // we are authenticated now let's set the query to the correct mail from ldap
-    query.email = mail
-    User.findOne(query, (error, user) => {
-      if (error) {
-        console.log(error)
-      }
-      if (user && user.hashedPassword) {
-        //console.log("******************** LOGIN ******************")
-        AuthenticationManager.login(user, "randomPass", callback)
-      } else {
-        onSuccessCreateUserIfNotExistent(query, user, callback, uid, firstname, lastname, mail, isAdmin)
     }
+    switch (errorCode) {
+      case 'not_set':
+        message.key = 'password-not-set'
+        message.text = req.i18n.translate('invalid_password_not_set')
+        break
+      case 'invalid_character':
+        message.key = 'password-invalid-character'
+        message.text = req.i18n.translate('invalid_password_invalid_character')
+        break
+      case 'contains_email':
+        message.key = 'password-contains-email'
+        message.text = req.i18n.translate('invalid_password_contains_email')
+        break
+      case 'too_similar':
+        message.key = 'password-too-similar'
+        message.text = req.i18n.translate('invalid_password_too_similar')
+        break
+      case 'too_short':
+        message.key = 'password-too-short'
+        message.text = req.i18n.translate('invalid_password_too_short', {
+          minLength: Settings.passwordStrengthOptions?.length?.min || 8,
         })
+        break
+      case 'too_long':
+        message.key = 'password-too-long'
+        message.text = req.i18n.translate('invalid_password_too_long', {
+          maxLength: Settings.passwordStrengthOptions?.length?.max || 72,
+        })
+        break
+      default:
+        logger.error({ err: error }, 'Unknown password validation error code')
+        message.text = req.i18n.translate('invalid_password')
+        break
     }
+    return message
+  },
 }
 
 AuthenticationManager.promises = {

ldap-overleaf-sl/sharelatex/ContactController.js

@@ -1,99 +1,93 @@
-/* eslint-disable
+/**
-    camelcase,
+ * >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
-    max-len,
+ * Modified from 906765c
-    no-unused-vars,
+ * <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  */
-// TODO: This file was created by bulk-decaffeinate.
+
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let ContactsController
-const AuthenticationController = require('../Authentication/AuthenticationController')
 const SessionManager = require('../Authentication/SessionManager')
 const ContactManager = require('./ContactManager')
 const UserGetter = require('../User/UserGetter')
-const logger = require('@overleaf/logger')
 const Modules = require('../../infrastructure/Modules')
-const { Client } = require('ldapts');
+const { expressify } = require('../../util/promises')
 
-module.exports = ContactsController = {
+// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
-  getContacts(req, res, next) {
+const { Client } = require('ldapts')
-    const user_id = SessionManager.getLoggedInUserId(req.session)
+// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
-    return ContactManager.getContactIds(
+
-      user_id,
+function _formatContact(contact) {
-      { limit: 50 },
+  return {
-      function (error, contact_ids) {
+    id: contact._id?.toString(),
-        if (error != null) {
+    email: contact.email || '',
-          return next(error)
+    first_name: contact.first_name || '',
+    last_name: contact.last_name || '',
+    type: 'user',
   }
-        return UserGetter.getUsers(
+}
-          contact_ids,
+
-          {
+async function getContacts(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+
+  const contactIds = await ContactManager.promises.getContactIds(userId, {
+    limit: 50,
+  })
+
+  let contacts = await UserGetter.promises.getUsers(contactIds, {
     email: 1,
     first_name: 1,
     last_name: 1,
     holdingAccount: 1,
-          },
+  })
-          function (error, contacts) {
-            if (error != null) {
-              return next(error)
-            }
 
   // UserGetter.getUsers may not preserve order so put them back in order
   const positions = {}
-            for (let i = 0; i < contact_ids.length; i++) {
+  for (let i = 0; i < contactIds.length; i++) {
-              const contact_id = contact_ids[i]
+    const contact_id = contactIds[i]
     positions[contact_id] = i
   }
   contacts.sort(
-              (a, b) =>
+    (a, b) => positions[a._id?.toString()] - positions[b._id?.toString()]
-                positions[a._id != null ? a._id.toString() : undefined] -
-                positions[b._id != null ? b._id.toString() : undefined]
   )
 
   // Don't count holding accounts to discourage users from repeating mistakes (mistyped or wrong emails, etc)
-            contacts = contacts.filter(c => !c.holdingAccount)
+  contacts = contacts.filter((c) => !c.holdingAccount)
-	    ContactsController.getLdapContacts(contacts).then((ldapcontacts) => { 
+
+// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+  const ldapcontacts = getLdapContacts(contacts)
   contacts.push(ldapcontacts)
-              contacts = contacts.map(ContactsController._formatContact)
+// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
 
-           return Modules.hooks.fire('getContacts', user_id, contacts, function(
+  contacts = contacts.map(_formatContact)
-              error,
+
-              additional_contacts
+  const additionalContacts = await Modules.promises.hooks.fire(
-            ) {
+    'getContacts',
-              if (error != null) {
+    userId,
-                return next(error)
-              }
-              contacts = contacts.concat(...Array.from(additional_contacts || []))
-              return res.send({
     contacts
-              })
-            })
-  	  }).catch(e => console.log("Error appending ldap contacts" + e))
-
-        }
   )
+
+  contacts = contacts.concat(...(additionalContacts || []))
+  return res.json({
+    contacts,
   })
-  },
+}
-  async getLdapContacts(contacts) {
+
-    if (process.env.LDAP_CONTACTS === undefined || !(process.env.LDAP_CONTACTS.toLowerCase() === 'true')) {
+// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+async function getLdapContacts(contacts) {
+  if (
+    process.env.LDAP_CONTACTS === undefined ||
+    !(process.env.LDAP_CONTACTS.toLowerCase() === 'true')
+  ) {
     return contacts
   }
   const client = new Client({
     url: process.env.LDAP_SERVER,
-    });
+  })
 
   // if we need a ldap user try to bind
   if (process.env.LDAP_BIND_USER) {
     try {
-        await client.bind(process.env.LDAP_BIND_USER, process.env.LDAP_BIND_PW);
+      await client.bind(process.env.LDAP_BIND_USER, process.env.LDAP_BIND_PW)
     } catch (ex) {
-        console.log("Could not bind LDAP reader user: " + String(ex) )
+      console.log('Could not bind LDAP reader user: ' + String(ex))
     }
   }
 
@@ -101,39 +95,36 @@ module.exports = ContactsController = {
   // get user data
   try {
     // if you need an client.bind do it here.
-      const {searchEntries,searchReferences,} = await client.search(ldap_base, {scope: 'sub',filter: process.env.LDAP_CONTACT_FILTER ,});
+    const { searchEntries, searchReferences } = await client.search(ldap_base, {
-      await searchEntries;
+      scope: 'sub',
+      filter: process.env.LDAP_CONTACT_FILTER,
+    })
+    await searchEntries
     for (var i = 0; i < searchEntries.length; i++) {
       var entry = new Map()
-       var obj = searchEntries[i];
+      var obj = searchEntries[i]
       entry['_id'] = undefined
       entry['email'] = obj['mail']
       entry['first_name'] = obj['givenName']
       entry['last_name'] = obj['sn']
-       entry['type'] = "user"
+      entry['type'] = 'user'
       // Only add to contacts if entry is not there.
       if (contacts.indexOf(entry) === -1) {
-          contacts.push(entry);
+        contacts.push(entry)
       }
     }
   } catch (ex) {
     console.log(String(ex))
-    }  
+  } finally {
     // console.log(JSON.stringify(contacts))
-    finally {
     // even if we did not use bind - the constructor of
     // new Client() opens a socket to the ldap server
     client.unbind()
     return contacts
   }
-  },
-  _formatContact(contact) {
-    return {
-      id: contact._id != null ? contact._id.toString() : undefined,
-      email: contact.email || '',
-      first_name: contact.first_name || '',
-      last_name: contact.last_name || '',
-      type: 'user',
 }
-  },
+// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
+
+module.exports = {
+  getContacts: expressify(getContacts),
 }