Skip to content

Commit

Permalink
Refactoring organisation, bundling and security improvements (nightsc…
Browse files Browse the repository at this point in the history
…out#6765)

* * Simplified bundling to just one bundle
* Removed cache invalidation token from bundling
* Generate strong persistent random string on deploy to use for JWT signing
* WIP: moving api-secret and JWT signing to a separate centralized security component
* Moved some server components away from project root
* Fix issues reported by linter

* Ignore detect-object-injection everywhere but the client

* Make admin message button red

* Remove alarms for some security alerts on code

* api_secret is now fully contained in the enclave
  • Loading branch information
sulkaharo authored and Jean-Phi37 committed Mar 18, 2021
1 parent 4fe8098 commit 1e9e142
Show file tree
Hide file tree
Showing 64 changed files with 362 additions and 249 deletions.
45 changes: 27 additions & 18 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,35 @@
module.exports = {
"plugins": [
"security"
'plugins': [
'security'
],
"extends": [
"eslint:recommended",
"plugin:security/recommended"
'extends': [
'eslint:recommended',
'plugin:security/recommended'
],
"parser": "babel-eslint",
"env": {
"browser": true,
"commonjs": true,
"es6": true,
"node": true,
"mocha": true,
"jquery": true
'parser': 'babel-eslint',
'env': {
'browser': true,
'commonjs': true,
'es6': true,
'node': true,
'mocha': true,
'jquery': true
},
"rules": {
"no-unused-vars": [
"error",
'rules': {
'security/detect-object-injection' : 0,
'no-unused-vars': [
'error',
{
"varsIgnorePattern": "should|expect"
'varsIgnorePattern': 'should|expect'
}
]
}
},
'overrides': [
{
'files': ['lib/client/*.js'],
'rules': {
'security/detect-object-injection': 1
}
}
],
};
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ bundle/bundle.out.js
*.iml
my.env
my.*.env
*.pem

static/bower_components/
.*.sw?
Expand Down
5 changes: 5 additions & 0 deletions bin/generateRandomString.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

require('crypto').randomBytes(1024, function(err, buffer) {
var token = buffer.toString('hex');
console.log(token);
});
6 changes: 6 additions & 0 deletions bundle/bundle.source.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ window.Nightscout = {
admin_plugins: require('../lib/admin_plugins/')()
};

window.Nightscout.report_plugins_preinit = require('../lib/report_plugins/');
window.Nightscout.predictions = require('../lib/report/predictions');
window.Nightscout.reportclient = require('../lib/report/reportclient');
window.Nightscout.profileclient = require('../lib/profile/profileeditor');
window.Nightscout.foodclient = require('../lib/food/food');

console.info('Nightscout bundle ready');

// Needed for Hot Module Replacement
Expand Down
8 changes: 4 additions & 4 deletions lib/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ function create (env, ctx) {
app.set('version', env.version);

app.set('units', env.DISPLAY_UNITS);
// Only allow access to the API if API_SECRET is set on the server.
// Only allow access to the API if API KEY is set on the server.
app.disable('api');
if (env.api_secret) {
console.log('API_SECRET present, enabling API');
if (env.enclave.isApiKeySet()) {
console.log('API KEY present, enabling API');
app.enable('api');
} else {
console.log('API_SECRET not found, API disabled');
console.log('API KEY has not been set, API disabled');
}

if (env.settings.enable) {
Expand Down
19 changes: 8 additions & 11 deletions lib/authorization/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function init (env, ctx) {
}

function authorizeAdminSecret (secret) {
return (env.api_secret && env.api_secret.length > 12) ? (secret === env.api_secret) : false;
return env.enclave.isApiKey(secret);
}

authorization.seenPermissions = [];
Expand Down Expand Up @@ -185,7 +185,7 @@ function init (env, ctx) {

// Tokens have to be well formed JWTs
try {
const verified = await jwt.verify(data.token, env.api_secret);
const verified = env.enclave.verifyJWT(data.token);
token = verified.accessToken;
} catch (err) {}

Expand Down Expand Up @@ -237,7 +237,7 @@ function init (env, ctx) {

/**
* Check if the client has a permission execute an action,
* based on an API_SECRET or JWT in the request.
* based on an API KEY or JWT in the request.
*
* Used to authorize API calls
*
Expand Down Expand Up @@ -281,8 +281,9 @@ function init (env, ctx) {
*/
authorization.authorize = function authorize (accessToken) {

let userToken = accessToken
const decodedToken = jwt.decode(accessToken);

let userToken = accessToken;
const decodedToken = env.enclave.verifyJWT(accessToken);

if (decodedToken && decodedToken.accessToken) {
userToken = decodedToken.accessToken;
Expand All @@ -292,18 +293,14 @@ function init (env, ctx) {
var authorized = null;

if (subject) {
var token = jwt.sign({ accessToken: subject.accessToken }, env.api_secret, { expiresIn: '8h' });

//decode so we can tell the client the issued and expired times
var decoded = jwt.decode(token);
const token = env.enclave.signJWT({ accessToken: subject.accessToken });
const decoded = env.enclave.verifyJWT(token);

var roles = _.uniq(subject.roles.concat(defaultRoles));

authorized = {
token
, sub: subject.name
// not sending roles to client to prevent us from treating them as magic
// instead group permissions by role so the we can create correct shiros on the client
, permissionGroups: _.map(roles, storage.roleToPermissions)
, iat: decoded.iat
, exp: decoded.exp
Expand Down
7 changes: 2 additions & 5 deletions lib/authorization/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,9 @@ function init (env, ctx) {
}

storage.subjects = _.map(results, function eachSubject (subject) {
if (env.api_secret) {
var shasum = crypto.createHash('sha1');
shasum.update(env.api_secret);
shasum.update(subject._id.toString());
if (env.enclave.isApiKeySet()) {
subject.digest = env.enclave.getSubjectHash(subject._id.toString());
var abbrev = subject.name.toLowerCase().replace(/[\W]/g, '').substring(0, 10);
subject.digest = shasum.digest('hex');
subject.accessToken = abbrev + '-' + subject.digest.substring(0, 16);
subject.accessTokenDigest = storage.getSHA1(subject.accessToken);
}
Expand Down
18 changes: 10 additions & 8 deletions lib/client/boluscalc.js
Original file line number Diff line number Diff line change
Expand Up @@ -593,14 +593,16 @@ function init (client, $) {
if (qp.hideafteruse) {
qp.hidden = true;

var apisecrethash = localStorage.getItem('apisecrethash');
var dataJson = JSON.stringify(qp, null, ' ');

var xhr = new XMLHttpRequest();
xhr.open('PUT', '/api/v1/food/', true);
xhr.setRequestHeader('Content-Type', 'application/json; charset=UTF-8');
xhr.setRequestHeader('api-secret', apisecrethash);
xhr.send(dataJson);
$.ajax({
method: 'PUT'
, url: '/api/v1/food/'
, headers: client.headers()
, data: qp
}).done(function treatmentSaved (response) {
console.info('quick pick saved', response);
}).fail(function treatmentSaveFail (response) {
console.info('quick pick failed to save', response);
});
}
}

Expand Down
8 changes: 4 additions & 4 deletions lib/food/food.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ client.init(function loaded () {
$('#fe_filter_subcategory').empty().append(new Option(translate('(none)'),''));
if (filter.category !== '') {
for (s in categories[filter.category]) {
if (categories[filter.category].hasOwnProperty(s)) {
if (Object.prototype.hasOwnProperty.call(categories[filter.category],s)) {
$('#fe_filter_subcategory').append(new Option(s,s));
}
}
Expand All @@ -162,7 +162,7 @@ client.init(function loaded () {
$('#fe_subcategory_list').empty().append(new Option(translate('(none)'),''));
if (foodrec.category !== '') {
for (s in categories[foodrec.category]) {
if (categories[foodrec.category].hasOwnProperty(s)) {
if (Object.prototype.hasOwnProperty.call(categories[foodrec.category],s)) {
$('#fe_subcategory_list').append(new Option(s,s));
}
}
Expand Down Expand Up @@ -198,7 +198,7 @@ client.init(function loaded () {
$('#fe_filter_category').empty().append(new Option(translate('(none)'),''));
$('#fe_category_list').empty().append(new Option(translate('(none)'),''));
for (var s in categories) {
if (categories.hasOwnProperty(s)) {
if (Object.prototype.hasOwnProperty.call(categories,s)) {
$('#fe_filter_category').append(new Option(s,s));
$('#fe_category_list').append(new Option(s,s));
}
Expand Down Expand Up @@ -398,7 +398,7 @@ client.init(function loaded () {
function savePortions(event) {
var index = $(this).attr('index');
var findex = $(this).attr('findex');
var val = parseFloat($(this).val().replace(/\,/g,'.'));
var val = parseFloat($(this).val().replace(/,/g,'.'));
foodquickpick[index].foods[findex].portions=val;
calculateCarbs(index);
drawQuickpick();
Expand Down
18 changes: 11 additions & 7 deletions lib/language.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,28 +78,32 @@ function init (fs) {
translated = language.translateCS(text);
}

let keys = null;
var hasCI = false;
var hasParams = false;

if (options && options.params) {
keys = options.params;
if (options) {
hasCI = Object.prototype.hasOwnProperty.call(options,'ci');
hasParams = Object.prototype.hasOwnProperty.call(options,'params');
}

if (options && !options.hasOwnProperty('ci') && !options.hasOwnProperty('params')) {
var keys = hasParams ? options.params : null;

if (options && !hasCI && !hasParams) {
keys = [];
for (var i = 1; i < arguments.length; i++) {
keys.push(arguments[i]);
}
}

if (options && (options.hasOwnProperty('ci') || options.hasOwnProperty('params')) && arguments.length > 2) {
if (options && (hasCI || hasParams) && arguments.length > 2) {
if (!keys) keys = [];
for (var i = 2; i < arguments.length; i++) {
for (i = 2; i < arguments.length; i++) {
keys.push(arguments[i]);
}
}

if (keys) {
for (var i = 0; i < keys.length; i++) {
for (i = 0; i < keys.length; i++) {
// eslint-disable-next-line no-useless-escape
var r = new RegExp('\%' + (i + 1), 'g');
translated = translated.replace(r, keys[i]);
Expand Down
10 changes: 5 additions & 5 deletions lib/profile/profileeditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ var init = function init () {
_.each(mongoprofile.store, function eachStoredProfile (p) {
// allign with default profile
for (var key in defaultprofile) {
if (defaultprofile.hasOwnProperty(key) && !p.hasOwnProperty(key)) {
if (Object.prototype.hasOwnProperty.call(defaultprofile,key) && !Object.prototype.hasOwnProperty.call(p,key)) {
p[key] = defaultprofile[key];
}
}
for (key in p) {
if (p.hasOwnProperty(key) && !defaultprofile.hasOwnProperty(key)) {
if (Object.prototype.hasOwnProperty.call(p,key) && !Object.prototype.hasOwnProperty.call(defaultprofile,key)) {
delete p[key];
}
}
Expand Down Expand Up @@ -232,7 +232,7 @@ var init = function init () {
$('#pe_profiles').empty();

for (var key in record.store) {
if (record.store.hasOwnProperty(key)) {
if (Object.prototype.hasOwnProperty.call(record.store,key)) {
$('#pe_profiles').append('<option value="' + key + '">' + key + '</option>');
}
}
Expand Down Expand Up @@ -656,7 +656,7 @@ var init = function init () {
var adjustedRecord = _.cloneDeep(record);

for (var key in adjustedRecord.store) {
if (adjustedRecord.store.hasOwnProperty(key)) {
if (Object.prototype.hasOwnProperty.call(adjustedRecord.store,key)) {
var profile = adjustedRecord.store[key];
if (!profile.perGIvalues) {
delete profile.perGIvalues;
Expand Down Expand Up @@ -707,7 +707,7 @@ var init = function init () {
function getFirstAvailableProfile(record) {
var availableProfiles = [];
for (var key in record.store) {
if (record.store.hasOwnProperty(key)) {
if (Object.prototype.hasOwnProperty.call(record.store,key)) {
if (key !== currentprofile) {
availableProfiles.push(key);
}
Expand Down
Loading

0 comments on commit 1e9e142

Please sign in to comment.