A simple service for handling blacklisting of usernames

README.md

@@ -82,7 +82,7 @@ Note that this example creates the `fullchain.pem` and `privkey.pem` files
 in a directory one level higher from the current, so that you don't
 accidentally commit your certificates to `solid` while you're developing.
 
-If you would like to get rid of the browser warnings, import your fullchain.pem certificate into your 'Trusted Root Certificate' store. 
+If you would like to get rid of the browser warnings, import your fullchain.pem certificate into your 'Trusted Root Certificate' store.
 
 ### Run multi-user server (intermediate)
 
@@ -375,6 +375,12 @@ In order to test a single component, you can run
 npm run test-(acl|formats|params|patch)
 ```
 
+## Blacklisted usernames
+
+By default Solid will not allow [certain usernames as they might cause
+confusion or allow vulnerabilies for social engineering](https://github.com/marteinn/The-Big-Username-Blacklist).
+This list is configurable via `config/usernames-blacklist.json`. Solid does not
+blacklist profanities by default.
 
 ## Contributing
 

config/usernames-blacklist.json

@@ -0,0 +1,4 @@
+{
+  "useTheBigUsernameBlacklist": true,
+  "customBlacklistedUsernames": []
+}

lib/create-app.js

@@ -12,8 +12,8 @@ const authProxy = require('./handlers/auth-proxy')
 const SolidHost = require('./models/solid-host')
 const AccountManager = require('./models/account-manager')
 const vhost = require('vhost')
-const EmailService = require('./models/email-service')
-const TokenService = require('./models/token-service')
+const EmailService = require('./services/email-service')
+const TokenService = require('./services/token-service')
 const capabilityDiscovery = require('./capability-discovery')
 const API = require('./api')
 const errorPages = require('./handlers/error-pages')

lib/requests/create-account-request.js

@@ -3,6 +3,7 @@
 const AuthRequest = require('./auth-request')
 const WebIdTlsCertificate = require('../models/webid-tls-certificate')
 const debug = require('../debug').accounts
+const blacklistService = require('../services/blacklist-service')
 
 /**
  * Represents a 'create new user account' http request (either a POST to the
@@ -115,30 +116,28 @@ class CreateAccountRequest extends AuthRequest {
   /**
    * Creates an account for a given user (from a POST to `/api/accounts/new`)
    *
-   * @throws {Error} An http 400 error if an account already exists
+   * @throws {Error} If errors were encountering while validating the username.
    *
    * @return {Promise<UserAccount>} Resolves with newly created account instance
    */
-  createAccount () {
+  async createAccount () {
     let userAccount = this.userAccount
     let accountManager = this.accountManager
 
-    return Promise.resolve(userAccount)
-      .then(this.cancelIfUsernameInvalid.bind(this))
-      .then(this.cancelIfAccountExists.bind(this))
-      .then(this.createAccountStorage.bind(this))
-      .then(this.saveCredentialsFor.bind(this))
-      .then(this.sendResponse.bind(this))
-      .then(userAccount => {
-        // 'return' not used deliberately, no need to block and wait for email
-        if (userAccount && userAccount.email) {
-          debug('Sending Welcome email')
-          accountManager.sendWelcomeEmail(userAccount)
-        }
-      })
-      .then(() => {
-        return userAccount
-      })
+    this.cancelIfUsernameInvalid(userAccount)
+    this.cancelIfBlacklistedUsername(userAccount)
+    await this.cancelIfAccountExists(userAccount)
+    await this.createAccountStorage(userAccount)
+    await this.saveCredentialsFor(userAccount)
+    await this.sendResponse(userAccount)
+
+    // 'return' not used deliberately, no need to block and wait for email
+    if (userAccount && userAccount.email) {
+      debug('Sending Welcome email')
+      accountManager.sendWelcomeEmail(userAccount)
+    }
+
+    return userAccount
   }
 
   /**
@@ -196,12 +195,33 @@ class CreateAccountRequest extends AuthRequest {
    * @throws {Error} If errors were encountering while validating the
    *   username.
    *
-   * @return {Promise<UserAccount>} Chainable
+   * @return {UserAccount} Chainable
    */
   cancelIfUsernameInvalid (userAccount) {
     if (!userAccount.username || !/^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(userAccount.username)) {
       debug('Invalid username ' + userAccount.username)
-      const error = new Error('Invalid username')
+      const error = new Error('Invalid username (contains invalid characters)')
+      error.status = 400
+      throw error
+    }
+
+    return userAccount
+  }
+
+  /**
+   * Check if a username is a valid slug.
+   *
+   * @param userAccount {UserAccount} Instance of the account to be created
+   *
+   * @throws {Error} If username is blacklisted
+   *
+   * @return {UserAccount} Chainable
+   */
+  cancelIfBlacklistedUsername (userAccount) {
+    const validUsername = blacklistService.validate(userAccount.username)
+    if (!validUsername) {
+      debug('Invalid username ' + userAccount.username)
+      const error = new Error('Invalid username (username is blacklisted)')
       error.status = 400
       throw error
     }

lib/services/blacklist-service.js

@@ -0,0 +1,33 @@
+const blacklistConfig = require('../../config/usernames-blacklist.json')
+const blacklist = require('the-big-username-blacklist').list
+
+class BlacklistService {
+  constructor () {
+    this.reset()
+  }
+
+  addWord (word) {
+    this.list.push(BlacklistService._prepareWord(word))
+  }
+
+  reset (config) {
+    this.list = BlacklistService._initList(config)
+  }
+
+  validate (word) {
+    return this.list.indexOf(BlacklistService._prepareWord(word)) === -1
+  }
+
+  static _initList (config = blacklistConfig) {
+    return [
+      ...(config.useTheBigUsernameBlacklist ? blacklist : []),
+      ...config.customBlacklistedUsernames
+    ]
+  }
+
+  static _prepareWord (word) {
+    return word.trim().toLocaleLowerCase()
+  }
+}
+
+module.exports = new BlacklistService()

lib/models/email-service.js → lib/services/email-service.js

@@ -2,7 +2,7 @@
 
 const nodemailer = require('nodemailer')
 const path = require('path')
-const debug = require('./../debug').email
+const debug = require('../debug').email
 
 /**
  * Models a Nodemailer-based email sending service.

package.json

@@ -77,6 +77,7 @@
     "solid-permissions": "^0.6.0",
     "solid-ws": "^0.2.3",
     "text-encoder-lite": "^1.0.1",
+    "the-big-username-blacklist": "^1.5.1",
     "ulid": "^0.1.0",
     "uuid": "^3.0.0",
     "valid-url": "^1.0.9",

test/unit/account-manager-test.js

@@ -15,7 +15,7 @@ const LDP = require('../../lib/ldp')
 const SolidHost = require('../../lib/models/solid-host')
 const AccountManager = require('../../lib/models/account-manager')
 const UserAccount = require('../../lib/models/user-account')
-const TokenService = require('../../lib/models/token-service')
+const TokenService = require('../../lib/services/token-service')
 const WebIdTlsCertificate = require('../../lib/models/webid-tls-certificate')
 
 const testAccountsDir = path.join(__dirname, '../resources/accounts')

test/unit/blacklist-service-test.js

@@ -0,0 +1,49 @@
+'use strict'
+
+const chai = require('chai')
+const expect = chai.expect
+
+const blacklist = require('the-big-username-blacklist').list
+const blacklistService = require('../../lib/services/blacklist-service')
+
+describe('BlacklistService', () => {
+  afterEach(() => blacklistService.reset())
+
+  describe('addWord', () => {
+    it('allows adding words', () => {
+      const numberOfBlacklistedWords = blacklistService.list.length
+      blacklistService.addWord('foo')
+      expect(blacklistService.list.length).to.equal(numberOfBlacklistedWords + 1)
+    })
+  })
+
+  describe('reset', () => {
+    it('will reset list of blacklisted words', () => {
+      blacklistService.addWord('foo')
+      blacklistService.reset()
+      expect(blacklistService.list.length).to.equal(blacklist.length)
+    })
+
+    it('can configure service via reset', () => {
+      blacklistService.reset({
+        useTheBigUsernameBlacklist: false,
+        customBlacklistedUsernames: ['foo']
+      })
+      expect(blacklistService.list.length).to.equal(1)
+      expect(blacklistService.validate('admin')).to.equal(true)
+    })
+
+    it('is a singleton', () => {
+      const instanceA = blacklistService
+      blacklistService.reset({ customBlacklistedUsernames: ['foo'] })
+      expect(instanceA.validate('foo')).to.equal(blacklistService.validate('foo'))
+    })
+  })
+
+  describe('validate', () => {
+    it('validates given a default list of blacklisted usernames', () => {
+      const validWords = blacklist.reduce((memo, word) => memo + (blacklistService.validate(word) ? 1 : 0), 0)
+      expect(validWords).to.equal(0)
+    })
+  })
+})

test/unit/create-account-request-test.js

@@ -7,12 +7,14 @@ const sinonChai = require('sinon-chai')
 chai.use(sinonChai)
 chai.should()
 const HttpMocks = require('node-mocks-http')
+const blacklist = require('the-big-username-blacklist')
 
 const LDP = require('../../lib/ldp')
 const AccountManager = require('../../lib/models/account-manager')
 const SolidHost = require('../../lib/models/solid-host')
 const defaults = require('../../config/defaults')
 const { CreateAccountRequest } = require('../../lib/requests/create-account-request')
+const blacklistService = require('../../lib/services/blacklist-service')
 
 describe('CreateAccountRequest', () => {
   let host, store, accountManager
@@ -127,6 +129,45 @@ describe('CreateAccountRequest', () => {
           expect(invalidUsernamesCount).to.eq(invalidUsernames.length)
         })
     })
+
+    describe('Blacklisted usernames', () => {
+      const invalidUsernames = [...blacklist.list, 'foo']
+
+      before(() => {
+        const accountManager = AccountManager.from({ host })
+        accountManager.accountExists = sinon.stub().returns(Promise.resolve(false))
+        blacklistService.addWord('foo')
+      })
+
+      after(() => blacklistService.reset())
+
+      it('should return a 400 error if a username is blacklisted', async () => {
+        const locals = { authMethod: defaults.auth, accountManager, oidc: { users: {} } }
+
+        let invalidUsernamesCount = 0
+
+        const requests = invalidUsernames.map((username) => {
+          let req = HttpMocks.createRequest({
+            app: { locals },
+            body: { username, password: '1234' }
+          })
+          let request = CreateAccountRequest.fromParams(req, res)
+
+          return request.createAccount()
+            .then(() => {
+              throw new Error('should not happen')
+            })
+            .catch(err => {
+              invalidUsernamesCount++
+              expect(err.message).to.match(/Invalid username/)
+              expect(err.status).to.equal(400)
+            })
+        })
+
+        await Promise.all(requests)
+        expect(invalidUsernamesCount).to.eq(invalidUsernames.length)
+      })
+    })
   })
 })
 

test/unit/email-service-test.js

@@ -1,4 +1,4 @@
-const EmailService = require('../../lib/models/email-service')
+const EmailService = require('../../lib/services/email-service')
 const path = require('path')
 const sinon = require('sinon')
 const chai = require('chai')

test/unit/email-welcome-test.js

@@ -10,7 +10,7 @@ chai.should()
 
 const SolidHost = require('../../lib/models/solid-host')
 const AccountManager = require('../../lib/models/account-manager')
-const EmailService = require('../../lib/models/email-service')
+const EmailService = require('../../lib/services/email-service')
 
 const templatePath = path.join(__dirname, '../../default-templates/emails')
 

test/unit/token-service-test.js

@@ -6,7 +6,7 @@ const dirtyChai = require('dirty-chai')
 chai.use(dirtyChai)
 chai.should()
 
-const TokenService = require('../../lib/models/token-service')
+const TokenService = require('../../lib/services/token-service')
 
 describe('TokenService', () => {
   describe('constructor()', () => {