Skip to content

Commit

Permalink
Moved tokens, url safe and safe string utility to lib/security
Browse files Browse the repository at this point in the history
refs TryGhost#9178

- we could now also move any crypto usages to lib/security, but no priority
- the main goal is to tidy up our utils folder
  • Loading branch information
kirrg001 committed Dec 14, 2017
1 parent 9de13ae commit bb06a84
Show file tree
Hide file tree
Showing 15 changed files with 128 additions and 112 deletions.
13 changes: 7 additions & 6 deletions core/server/api/authentication.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var Promise = require('bluebird'),
models = require('../models'),
config = require('../config'),
common = require('../lib/common'),
security = require('../lib/security'),
spamPrevention = require('../web/middleware/api/spam-prevention'),
mailAPI = require('./mail'),
settingsAPI = require('./settings'),
Expand Down Expand Up @@ -167,7 +168,7 @@ authentication = {
throw new common.errors.NotFoundError({message: common.i18n.t('errors.api.users.userNotFound')});
}

token = globalUtils.tokens.resetToken.generateHash({
token = security.tokens.resetToken.generateHash({
expires: Date.now() + globalUtils.ONE_DAY_MS,
email: email,
dbHash: dbHash,
Expand All @@ -183,7 +184,7 @@ authentication = {

function sendResetNotification(data) {
var adminUrl = urlService.utils.urlFor('admin', true),
resetUrl = urlService.utils.urlJoin(adminUrl, 'reset', globalUtils.encodeBase64URLsafe(data.resetToken), '/');
resetUrl = urlService.utils.urlJoin(adminUrl, 'reset', security.url.encodeBase64(data.resetToken), '/');

return mail.utils.generateContent({
data: {
Expand Down Expand Up @@ -251,9 +252,9 @@ authentication = {
}

function extractTokenParts(options) {
options.data.passwordreset[0].token = globalUtils.decodeBase64URLsafe(options.data.passwordreset[0].token);
options.data.passwordreset[0].token = security.url.decodeBase64(options.data.passwordreset[0].token);

tokenParts = globalUtils.tokens.resetToken.extract({
tokenParts = security.tokens.resetToken.extract({
token: options.data.passwordreset[0].token
});

Expand Down Expand Up @@ -295,7 +296,7 @@ authentication = {
throw new common.errors.NotFoundError({message: common.i18n.t('errors.api.users.userNotFound')});
}

tokenIsCorrect = globalUtils.tokens.resetToken.compare({
tokenIsCorrect = security.tokens.resetToken.compare({
token: resetToken,
dbHash: dbHash,
password: user.get('password')
Expand Down Expand Up @@ -382,7 +383,7 @@ authentication = {
}

function processInvitation(invitation) {
var data = invitation.invitation[0], inviteToken = globalUtils.decodeBase64URLsafe(data.token);
var data = invitation.invitation[0], inviteToken = security.url.decodeBase64(data.token);

return models.Invite.findOne({token: inviteToken, status: 'sent'}, options)
.then(function (_invite) {
Expand Down
4 changes: 2 additions & 2 deletions core/server/api/invites.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ var Promise = require('bluebird'),
_ = require('lodash'),
pipeline = require('../lib/promise/pipeline'),
mail = require('../services/mail'),
globalUtils = require('../utils'),
urlService = require('../services/url'),
localUtils = require('./utils'),
models = require('../models'),
common = require('../lib/common'),
security = require('../lib/security'),
mailAPI = require('./mail'),
settingsAPI = require('./settings'),
docName = 'invites',
Expand Down Expand Up @@ -107,7 +107,7 @@ invites = {
invitedByName: loggedInUser.get('name'),
invitedByEmail: loggedInUser.get('email'),
// @TODO: resetLink sounds weird
resetLink: urlService.utils.urlJoin(adminUrl, 'signup', globalUtils.encodeBase64URLsafe(invite.get('token')), '/')
resetLink: urlService.utils.urlJoin(adminUrl, 'signup', security.url.encodeBase64(invite.get('token')), '/')
};

return mail.utils.generateContent({data: emailData, template: 'invite-user'});
Expand Down
4 changes: 2 additions & 2 deletions core/server/controllers/channel.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
var _ = require('lodash'),
common = require('../lib/common'),
security = require('../lib/security'),
filters = require('../filters'),
safeString = require('../utils').safeString,
handleError = require('./frontend/error'),
fetchData = require('./frontend/fetch-data'),
setRequestIsSecure = require('./frontend/secure'),
Expand All @@ -13,7 +13,7 @@ var _ = require('lodash'),
module.exports = function channelController(req, res, next) {
// Parse the parameters we need from the URL
var pageParam = req.params.page !== undefined ? req.params.page : 1,
slugParam = req.params.slug ? safeString(req.params.slug) : undefined;
slugParam = req.params.slug ? security.string.safe(req.params.slug) : undefined;

// @TODO: fix this, we shouldn't change the channel object!
// Set page on postOptions for the query made later
Expand Down
4 changes: 2 additions & 2 deletions core/server/controllers/rss.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
var _ = require('lodash'),
url = require('url'),
common = require('../lib/common'),
safeString = require('../utils').safeString,
security = require('../lib/security'),
settingsCache = require('../services/settings/cache'),

// Slightly less ugly temporary hack for location of things
Expand Down Expand Up @@ -47,7 +47,7 @@ function getData(channelOpts) {
generate = function generate(req, res, next) {
// Parse the parameters we need from the URL
var pageParam = req.params.page !== undefined ? req.params.page : 1,
slugParam = req.params.slug ? safeString(req.params.slug) : undefined,
slugParam = req.params.slug ? security.string.safe(req.params.slug) : undefined,
// Base URL needs to be the URL for the feed without pagination:
baseUrl = getBaseUrlForRSSReq(req.originalUrl, pageParam);

Expand Down
4 changes: 2 additions & 2 deletions core/server/data/export/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ var _ = require('lodash'),
Promise = require('bluebird'),
db = require('../../data/db'),
commands = require('../schema').commands,
globalUtils = require('../../utils'),
ghostVersion = require('../../utils/ghost-version'),
common = require('../../lib/common'),
security = require('../../lib/security'),
models = require('../../models'),
excludedTables = ['accesstokens', 'refreshtokens', 'clients', 'client_trusted_domains'],
modelOptions = {context: {internal: true}},
Expand All @@ -23,7 +23,7 @@ exportFileName = function exportFileName(options) {

return models.Settings.findOne({key: 'title'}, _.merge({}, modelOptions, options)).then(function (result) {
if (result) {
title = globalUtils.safeString(result.get('value')) + '.';
title = security.string.safe(result.get('value')) + '.';
}

return title + 'ghost.' + datetime + '.json';
Expand Down
15 changes: 15 additions & 0 deletions core/server/lib/security/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

module.exports = {
get url() {
return require('./url');
},

get tokens() {
return require('./tokens');
},

get string() {
return require('./string');
}
};
40 changes: 40 additions & 0 deletions core/server/lib/security/string.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict';

const unidecode = require('unidecode'),
_ = require('lodash');

module.exports.safe = function safe(string, options) {
options = options || {};

if (string === null) {
string = '';
}

// Handle the £ symbol separately, since it needs to be removed before the unicode conversion.
string = string.replace(/£/g, '-');

// Remove non ascii characters
string = unidecode(string);

// Replace URL reserved chars: `@:/?#[]!$&()*+,;=` as well as `\%<>|^~£"{}` and \`
string = string.replace(/(\s|\.|@|:|\/|\?|#|\[|\]|!|\$|&|\(|\)|\*|\+|,|;|=|\\|%|<|>|\||\^|~|"|\{|\}|`|–|—)/g, '-')
// Remove apostrophes
.replace(/'/g, '')
// Make the whole thing lowercase
.toLowerCase();

// We do not need to make the following changes when importing data
if (!_.has(options, 'importing') || !options.importing) {
// Convert 2 or more dashes into a single dash
string = string.replace(/-+/g, '-')
// Remove trailing dash
.replace(/-$/, '')
// Remove any dashes at the beginning
.replace(/^-/, '');
}

// Handle whitespace at the beginning or end.
string = string.trim();

return string;
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
var crypto = require('crypto');
'use strict';

const crypto = require('crypto');

exports.resetToken = {
generateHash: function generateHash(options) {
Expand Down
14 changes: 14 additions & 0 deletions core/server/lib/security/url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// The token is encoded URL safe by replacing '+' with '-', '\' with '_' and removing '='
// NOTE: the token is not encoded using valid base64 anymore
module.exports.encodeBase64 = function encodeBase64(base64String) {
return base64String.replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '');
};

// Decode url safe base64 encoding and add padding ('=')
module.exports.decodeBase64 = function decodeBase64(base64String) {
base64String = base64String.replace(/-/g, '+').replace(/_/g, '/');
while (base64String.length % 4) {
base64String += '=';
}
return base64String;
};
4 changes: 2 additions & 2 deletions core/server/models/base/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ var _ = require('lodash'),
config = require('../../config'),
db = require('../../data/db'),
common = require('../../lib/common'),
security = require('../../lib/security'),
filters = require('../../filters'),
schema = require('../../data/schema'),
urlService = require('../../services/url'),
globalUtils = require('../../utils'),
validation = require('../../data/validation'),
plugins = require('../plugins'),

Expand Down Expand Up @@ -739,7 +739,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
// the slug may never be longer than the allowed limit of 191 chars, but should also
// take the counter into count. We reduce a too long slug to 185 so we're always on the
// safe side, also in terms of checking for existing slugs already.
slug = globalUtils.safeString(base, options);
slug = security.string.safe(base, options);

if (slug.length > 185) {
// CASE: don't cut the slug on import
Expand Down
3 changes: 2 additions & 1 deletion core/server/services/auth/auth-strategies.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var _ = require('lodash'),
models = require('../../models'),
globalUtils = require('../../utils'),
common = require('../../lib/common'),
security = require('../../lib/security'),
strategies;

strategies = {
Expand Down Expand Up @@ -94,7 +95,7 @@ strategies = {

handleInviteToken = function handleInviteToken() {
var user, invite;
inviteToken = globalUtils.decodeBase64URLsafe(inviteToken);
inviteToken = security.url.decodeBase64(inviteToken);

return models.Invite.findOne({token: inviteToken}, options)
.then(function addInviteUser(_invite) {
Expand Down
56 changes: 1 addition & 55 deletions core/server/utils/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
var unidecode = require('unidecode'),
_ = require('lodash'),
utils,
var utils,
getRandomInt;

/**
Expand Down Expand Up @@ -57,60 +55,8 @@ utils = {
return buf.join('');
},

safeString: function (string, options) {
options = options || {};

if (string === null) {
string = '';
}

// Handle the £ symbol separately, since it needs to be removed before the unicode conversion.
string = string.replace(/£/g, '-');

// Remove non ascii characters
string = unidecode(string);

// Replace URL reserved chars: `@:/?#[]!$&()*+,;=` as well as `\%<>|^~£"{}` and \`
string = string.replace(/(\s|\.|@|:|\/|\?|#|\[|\]|!|\$|&|\(|\)|\*|\+|,|;|=|\\|%|<|>|\||\^|~|"|\{|\}|`|–|—)/g, '-')
// Remove apostrophes
.replace(/'/g, '')
// Make the whole thing lowercase
.toLowerCase();

// We do not need to make the following changes when importing data
if (!_.has(options, 'importing') || !options.importing) {
// Convert 2 or more dashes into a single dash
string = string.replace(/-+/g, '-')
// Remove trailing dash
.replace(/-$/, '')
// Remove any dashes at the beginning
.replace(/^-/, '');
}

// Handle whitespace at the beginning or end.
string = string.trim();

return string;
},

// The token is encoded URL safe by replacing '+' with '-', '\' with '_' and removing '='
// NOTE: the token is not encoded using valid base64 anymore
encodeBase64URLsafe: function (base64String) {
return base64String.replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '');
},

// Decode url safe base64 encoding and add padding ('=')
decodeBase64URLsafe: function (base64String) {
base64String = base64String.replace(/-/g, '+').replace(/_/g, '/');
while (base64String.length % 4) {
base64String += '=';
}
return base64String;
},

readCSV: require('./read-csv'),
zipFolder: require('./zip-folder'),
tokens: require('./tokens'),
ghostVersion: require('./ghost-version')
};

Expand Down
4 changes: 2 additions & 2 deletions core/test/functional/routes/api/authentication_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var should = require('should'),
userForKnex = testUtils.DataGenerator.forKnex.users[0],
models = require('../../../../../core/server/models'),
config = require('../../../../../core/server/config'),
utils = require('../../../../../core/server/utils'),
security = require('../../../../../core/server/lib/security'),
ghost = testUtils.startGhost,
request;

Expand Down Expand Up @@ -212,7 +212,7 @@ describe('Authentication API', function () {
models.Settings
.findOne({key: 'db_hash'})
.then(function (response) {
var token = utils.tokens.resetToken.generateHash({
var token = security.tokens.resetToken.generateHash({
expires: Date.now() + (1000 * 60),
email: user.email,
dbHash: response.attributes.value,
Expand Down
Loading

0 comments on commit bb06a84

Please sign in to comment.