Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
d8003f5
Adding options to handle T&C
megoth Oct 24, 2018
b02bf0d
Added checkbox to register-form
megoth Oct 24, 2018
f40d80e
Conditionally enforcing acceptToc when creating account
megoth Oct 31, 2018
2c324d9
A little cleanup
megoth Oct 31, 2018
04babec
A simple, configurable solution to blacklist usernames
megoth Oct 31, 2018
8922bdd
Making test a bit more explicit
megoth Oct 31, 2018
46bdd70
Update README.md
kjetilk Nov 1, 2018
5980af7
Moved all services to its own service-directory
megoth Nov 1, 2018
db0d55b
Merge branch 'feature/blacklist-username' of https://github.com/solid…
megoth Nov 1, 2018
93d7482
Merge pull request #893 from solid/feature/blacklist-username
kjetilk Nov 1, 2018
75c01e5
Adding options to handle T&C
megoth Oct 24, 2018
7685360
Added checkbox to register-form
megoth Oct 24, 2018
8d5ee92
Conditionally enforcing acceptToc when creating account
megoth Oct 31, 2018
6f0fd7b
A little cleanup
megoth Oct 31, 2018
1235e3f
Fixed minor nits
megoth Nov 5, 2018
7ef3e40
Merge remote-tracking branch 'origin/feature/link-toc' into feature/l…
megoth Nov 5, 2018
d7475ff
Just added properties to config.json-default
megoth Nov 5, 2018
57cfd18
Adding options to handle T&C
megoth Oct 24, 2018
6d6fe00
Added checkbox to register-form
megoth Oct 24, 2018
2526047
Conditionally enforcing acceptToc when creating account
megoth Oct 31, 2018
9c96aee
A little cleanup
megoth Oct 31, 2018
b18f0b8
Fixed minor nits
megoth Nov 5, 2018
13d8ce1
Just added properties to config.json-default
megoth Nov 5, 2018
38252d7
Merge branch 'feature/link-toc' of https://github.com/solid/node-soli…
megoth Nov 6, 2018
df438cc
Removed handlebars-helpers because of security vulnerability
megoth Nov 6, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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

Expand Down
15 changes: 15 additions & 0 deletions bin/lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,21 @@ module.exports = [
name: 'server-logo',
help: 'A logo that represents you, your brand, or your server (not required)',
prompt: true
},
{
name: 'enforceToc',
help: 'Do you want to enforce Terms & Conditions for your service?',
flag: true,
prompt: true,
default: true,
when: answers => answers.multiuser
},
{
name: 'tocUri',
help: 'URI to your Terms & Conditions',
prompt: true,
validate: validUri,
when: answers => answers.enforceToc
}
]

Expand Down
4 changes: 3 additions & 1 deletion config.json-default
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@
"name": "",
"description": "",
"logo": ""
}
},
"enforceToc": true,
"tocUri": "https://your-toc"
}
4 changes: 4 additions & 0 deletions config/usernames-blacklist.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"useTheBigUsernameBlacklist": true,
"customBlacklistedUsernames": []
}
11 changes: 11 additions & 0 deletions default-views/account/register-form.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,17 @@
</span>
</div>

{{#if enforceToc}}
{{#if tocUri}}
<div class="checkbox">
<label>
<input type="checkbox" name="acceptToc">
I agree to the <a href="{{tocUri}}" target="_blank">Terms &amp; Conditions</a> of this service
</label>
</div>
{{/if}}
{{/if}}


<button type="submit" class="btn btn-primary" id="register">Register</button>

Expand Down
6 changes: 4 additions & 2 deletions lib/create-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -123,6 +123,8 @@ function initAppLocals (app, argv, ldp) {
app.locals.authMethod = argv.auth
app.locals.localAuth = argv.localAuth
app.locals.tokenService = new TokenService()
app.locals.enforceToc = argv.enforceToc
app.locals.tocUri = argv.tocUri

if (argv.email && argv.email.host) {
app.locals.emailService = new EmailService(argv.templates.email, argv.email)
Expand Down
8 changes: 7 additions & 1 deletion lib/requests/auth-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class AuthRequest {
* @param [options.returnToUrl] {string}
* @param [options.authQueryParams] {Object} Key/value hashmap of parsed query
* parameters that will be passed through to the /authorize endpoint.
* @param [options.enforceToc] {boolean} Whether or not to enforce the service provider's T&C
* @param [options.tocUri] {string} URI to the service provider's T&C
*/
constructor (options) {
this.response = options.response
Expand All @@ -34,6 +36,8 @@ class AuthRequest {
this.returnToUrl = options.returnToUrl
this.authQueryParams = options.authQueryParams || {}
this.localAuth = options.localAuth
this.enforceToc = options.enforceToc
this.tocUri = options.tocUri
}

/**
Expand Down Expand Up @@ -84,6 +88,7 @@ class AuthRequest {

let authQueryParams = AuthRequest.extractAuthParams(req)
let returnToUrl = AuthRequest.parseParameter(req, 'returnToUrl')
const acceptToc = AuthRequest.parseParameter(req, 'acceptToc') === 'true'

let options = {
response: res,
Expand All @@ -92,7 +97,8 @@ class AuthRequest {
accountManager,
returnToUrl,
authQueryParams,
localAuth
localAuth,
acceptToc
}

return options
Expand Down
112 changes: 78 additions & 34 deletions lib/requests/create-account-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -26,12 +27,16 @@ class CreateAccountRequest extends AuthRequest {
* @param [options.response] {HttpResponse}
* @param [options.returnToUrl] {string} If present, redirect the agent to
* this url on successful account creation
* @param [options.enforceToc] {boolean} Whether or not to enforce the service provider's T&C
* @param [options.tocUri] {string} URI to the service provider's T&C
* @param [options.acceptToc] {boolean} Whether or not user has accepted T&C
*/
constructor (options) {
super(options)

this.username = options.username
this.userAccount = options.userAccount
this.acceptToc = options.acceptToc
}

/**
Expand All @@ -45,7 +50,7 @@ class CreateAccountRequest extends AuthRequest {
* `userAccountFrom()`), or it encounters an unsupported authentication
* scheme.
*
* @return {CreateAccountRequest|CreateTlsAccountRequest}
* @return {CreateOidcAccountRequest|CreateTlsAccountRequest}
*/
static fromParams (req, res) {
let options = AuthRequest.requestOptions(req, res)
Expand All @@ -61,6 +66,9 @@ class CreateAccountRequest extends AuthRequest {
options.userAccount = accountManager.userAccountFrom(body)
}

options.enforceToc = locals.enforceToc
options.tocUri = locals.tocUri

switch (authMethod) {
case 'oidc':
options.password = body.password
Expand All @@ -73,13 +81,15 @@ class CreateAccountRequest extends AuthRequest {
}
}

static post (req, res) {
static async post (req, res) {
let request = CreateAccountRequest.fromParams(req, res)

return Promise.resolve()
.then(() => request.validate())
.then(() => request.createAccount())
.catch(error => request.error(error))
try {
request.validate()
await request.createAccount()
} catch (error) {
request.error(error)
}
}

static get (req, res) {
Expand All @@ -96,13 +106,14 @@ class CreateAccountRequest extends AuthRequest {
renderForm (error) {
let authMethod = this.accountManager.authMethod

let params = Object.assign({}, this.authQueryParams,
{
returnToUrl: this.returnToUrl,
loginUrl: this.loginUrl(),
registerDisabled: authMethod === 'tls',
multiuser: this.accountManager.multiuser
})
let params = Object.assign({}, this.authQueryParams, {
enforceToc: this.enforceToc,
loginUrl: this.loginUrl(),
multiuser: this.accountManager.multiuser,
registerDisabled: authMethod === 'tls',
returnToUrl: this.returnToUrl,
tocUri: this.tocUri
})

if (error) {
params.error = error.message
Expand All @@ -115,30 +126,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
}

/**
Expand Down Expand Up @@ -196,12 +205,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
}
Expand All @@ -224,6 +254,7 @@ class CreateOidcAccountRequest extends CreateAccountRequest {
*
* @param [options={}] {Object} See `CreateAccountRequest` constructor docstring
* @param [options.password] {string} Password, as entered by the user at signup
* @param [options.acceptToc] {boolean} Whether or not user has accepted T&C
*/
constructor (options) {
super(options)
Expand Down Expand Up @@ -251,6 +282,12 @@ class CreateOidcAccountRequest extends CreateAccountRequest {
error.statusCode = 400
throw error
}

if (this.enforceToc && !this.acceptToc) {
error = new Error('Accepting Terms & Conditions is required for this service')
error.statusCode = 400
throw error
}
}

/**
Expand Down Expand Up @@ -290,6 +327,7 @@ class CreateTlsAccountRequest extends CreateAccountRequest {
*
* @param [options={}] {Object} See `CreateAccountRequest` constructor docstring
* @param [options.spkac] {string}
* @param [options.acceptToc] {boolean} Whether or not user has accepted T&C
*/
constructor (options) {
super(options)
Expand All @@ -299,7 +337,7 @@ class CreateTlsAccountRequest extends CreateAccountRequest {
}

/**
* Validates the Login request (makes sure required parameters are present),
* Validates the Signup request (makes sure required parameters are present),
* and throws an error if not.
*
* @throws {Error} If missing required params
Expand All @@ -312,6 +350,12 @@ class CreateTlsAccountRequest extends CreateAccountRequest {
error.statusCode = 400
throw error
}

if (this.enforceToc && !this.acceptToc) {
error = new Error('Accepting Terms & Conditions is required for this service')
error.statusCode = 400
throw error
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code duplication?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are two different classes, so think I'll let it be as is.

But we might want to separate these classes a bit more in the future, i.e. having them in separate files. create-account-request is getting difficult to read.

}

/**
Expand Down
33 changes: 33 additions & 0 deletions lib/services/blacklist-service.js
Original file line number Diff line number Diff line change
@@ -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()
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
File renamed without changes.
Loading