Extracted validation of username into user-utils

Author: megoth

bin/lib/blacklist.js

@@ -3,11 +3,12 @@ const util = require('util')
 const { URL } = require('url')
 
 const { loadConfig } = require('./common')
+const { isValidUsername } = require('../../lib/common/user-utils')
 const blacklistService = require('../../lib/services/blacklist-service')
 
-const AccountManager = require('../../lib/models/account-manager')
-const LDP = require('../../lib/ldp')
-const SolidHost = require('../../lib/models/solid-host')
+// const AccountManager = require('../../lib/models/account-manager')
+// const LDP = require('../../lib/ldp')
+// const SolidHost = require('../../lib/models/solid-host')
 
 module.exports = function (program) {
   program
@@ -21,25 +22,29 @@ module.exports = function (program) {
         return console.error('You are running a single user server, no need to check for blacklisted users')
       }
 
-      const host = SolidHost.from({ port: config.port, serverUri: config.serverUri })
+      // const host = SolidHost.from({ port: config.port, serverUri: config.serverUri })
       const invalidUsernames = await getInvalidUsernames(config)
-      console.log(invalidUsernames)
 
-      const ldp = new LDP(config)
-      const accountManager = AccountManager.from({
-        // authMethod: argv.auth,
-        // emailService: app.locals.emailService,
-        // tokenService: app.locals.tokenService,
-        host,
-        // accountTemplatePath: argv.templates.account,
-        store: ldp,
-        multiuser: config.multiuser
-      })
-      const blacklistedUsernames = await getBlacklistedUsernames(accountManager)
-      if (blacklistedUsernames.length === 0) {
-        console.log('No blacklisted username was found')
+      // const ldp = new LDP(config)
+      // const accountManager = AccountManager.from({
+      //   // authMethod: argv.auth,
+      //   // emailService: app.locals.emailService,
+      //   // tokenService: app.locals.tokenService,
+      //   host,
+      //   // accountTemplatePath: argv.templates.account,
+      //   store: ldp,
+      //   multiuser: config.multiuser
+      // })
+      // const blacklistedUsernames = await getBlacklistedUsernames(accountManager)
+      // if (blacklistedUsernames.length === 0) {
+      //   console.log('No blacklisted username was found')
+      // }
+      // console.log(`These blacklisted usernames were found:${blacklistedUsernames.map(username => `\n- ${username}`)}`)
+
+      if (invalidUsernames.length === 0) {
+        console.log('No invalid username was found')
       }
-      console.log(`These blacklisted usernames were found:${blacklistedUsernames.map(username => `\n- ${username}`)}`)
+      console.log(`${invalidUsernames.length} invalid usernames were found:${invalidUsernames.map(username => `\n- ${username}`)}`)
     })
 }
 
@@ -50,15 +55,16 @@ async function getInvalidUsernames (config) {
   return files
     .filter(file => isUserDirectory.test(file))
     .map(userDirectory => userDirectory.substr(0, userDirectory.length - hostname.length - 1))
+    .filter(username => !isValidUsername(username) || !blacklistService.validate(username))
 }
 
-async function getBlacklistedUsernames (accountManager) {
-  const blacklistedUsernames = []
-  await Promise.all(blacklistService.list.map(async (word) => {
-    const accountExists = await accountManager.accountExists(word)
-    if (accountExists) {
-      blacklistedUsernames.push(word)
-    }
-  }))
-  return blacklistedUsernames
-}
+// async function getBlacklistedUsernames (accountManager) {
+//   const blacklistedUsernames = []
+//   await Promise.all(blacklistService.list.map(async (word) => {
+//     const accountExists = await accountManager.accountExists(word)
+//     if (accountExists) {
+//       blacklistedUsernames.push(word)
+//     }
+//   }))
+//   return blacklistedUsernames
+// }

lib/common/user-utils.js

@@ -0,0 +1,5 @@
+module.exports.isValidUsername = isValidUsername
+
+function isValidUsername (username) {
+  return /^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(username)
+}

lib/requests/create-account-request.js

@@ -4,6 +4,7 @@ const AuthRequest = require('./auth-request')
 const WebIdTlsCertificate = require('../models/webid-tls-certificate')
 const debug = require('../debug').accounts
 const blacklistService = require('../services/blacklist-service')
+const { isValidUsername } = require('../common/user-utils')
 
 /**
  * Represents a 'create new user account' http request (either a POST to the
@@ -208,7 +209,7 @@ class CreateAccountRequest extends AuthRequest {
    * @return {UserAccount} Chainable
    */
   cancelIfUsernameInvalid (userAccount) {
-    if (!userAccount.username || !/^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(userAccount.username)) {
+    if (!userAccount.username || !isValidUsername(userAccount.username)) {
       debug('Invalid username ' + userAccount.username)
       const error = new Error('Invalid username (contains invalid characters)')
       error.status = 400

test/unit/user-utils-test.js

@@ -0,0 +1,30 @@
+const chai = require('chai')
+const expect = chai.expect
+const userUtils = require('../../lib/common/user-utils')
+
+describe('user-utils', () => {
+  describe('isValidUsername', () => {
+    it('should accect valid usernames', () => {
+      const usernames = [
+        'foo',
+        'bar'
+      ]
+      const validUsernames = usernames.filter(username => userUtils.isValidUsername(username))
+      expect(validUsernames.length).to.equal(usernames.length)
+    })
+
+    it('should not accect invalid usernames', () => {
+      const usernames = [
+        '-',
+        '-a',
+        'a-',
+        '9-',
+        'alice--bob',
+        'alice bob',
+        'alice.bob'
+      ]
+      const validUsernames = usernames.filter(username => userUtils.isValidUsername(username))
+      expect(validUsernames.length).to.equal(0)
+    })
+  })
+})