Skip to content

Commit

Permalink
Merge pull request vuestorefront#4626 from gibkigonzo/bugfix/sec-fixes
Browse files Browse the repository at this point in the history
add helmet, add `users.allowModification` and `users.tokenInHeader` config
  • Loading branch information
Tomasz Kostuch authored Jul 16, 2020
2 parents ab5ef30 + c1e7724 commit 1cc8ece
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 11 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- **IMPORTANT** for security reasons we added new config `users.allowModification`.
This can help to dissallow modifying fields that shouldn't be changed by user.
- Add helmet - enabled by default, you can pass configuration by adding `config.server.helmet.config`.
More info about helmet configuration https://helmetjs.github.io/docs/
- Add config `users.tokenInHeader` which allows to send token in header instead in query. Require to set on true same config in vsf-api.

### Fixed

- remove deprecated value from attributesListQuery query - @gibkigonzo (#4572)
Expand Down
7 changes: 6 additions & 1 deletion config/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@
"trace": {
"enabled": false,
"config": {}
},
"helmet": {
"enabled": true
}
},
"initialResources": [
Expand Down Expand Up @@ -721,7 +724,9 @@
"login_endpoint": "/api/user/login",
"create_endpoint": "/api/user/create",
"me_endpoint": "/api/user/me?token={{token}}",
"refresh_endpoint": "/api/user/refresh"
"refresh_endpoint": "/api/user/refresh",
"allowModification": ["firstname", "lastname", "email", "addresses"],
"tokenInHeader": false
},
"stock": {
"synchronize": true,
Expand Down
7 changes: 5 additions & 2 deletions core/data-resolver/ProductService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,18 @@ const getProductRenderList = async ({
url = `${url}&userGroupId=${userGroupId}`
}

if (token) {
if (token && !config.users.tokenInHeader) {
url = `${url}&token=${token}`
}

try {
const task = await TaskQueue.execute({ url, // sync the cart
payload: {
method: 'GET',
headers: { 'Content-Type': 'application/json' },
headers: {
'Content-Type': 'application/json',
...(token && config.users.tokenInHeader ? { authorization: `Bearer ${token}` } : {})
},
mode: 'cors'
},
callback_event: 'prices-after-sync'
Expand Down
40 changes: 34 additions & 6 deletions core/lib/sync/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { serial } from '@vue-storefront/core/helpers'
import config from 'config'
import { onlineHelper } from '@vue-storefront/core/helpers'
import { hasResponseError, getResponseMessage } from '@vue-storefront/core/lib/sync/helpers'
import queryString from 'query-string'

export function _prepareTask (task) {
const taskId = entities.uniqueEntityId(task) // timestamp as a order id is not the best we can do but it's enough
Expand All @@ -29,6 +30,36 @@ function _sleep (time) {
return new Promise((resolve) => setTimeout(resolve, time))
}

function getUrl (task, currentToken, currentCartId) {
let url = task.url
.replace('{{token}}', (currentToken == null) ? '' : currentToken)
.replace('{{cartId}}', (currentCartId == null) ? '' : currentCartId)

url = processURLAddress(url); // use relative url paths
if (config.storeViews.multistore) {
url = adjustMultistoreApiUrl(url)
}

if (config.users.tokenInHeader) {
const parsedUrl = queryString.parseUrl(url)
delete parsedUrl['query']['token']
url = queryString.stringifyUrl(parsedUrl)
}

return url
}

function getPayload (task, currentToken) {
const payload = {
...task.payload,
headers: {
...task.payload.headers,
...(config.users.tokenInHeader ? { authorization: `Bearer ${currentToken}` } : {})
}
}
return payload
}

function _internalExecute (resolve, reject, task: Task, currentToken, currentCartId) {
if (currentToken && rootStore.state.userTokenInvalidateLock > 0) { // invalidate lock set
Logger.log('Waiting for rootStore.state.userTokenInvalidateLock to release for ' + task.url, 'sync')()
Expand All @@ -52,14 +83,11 @@ function _internalExecute (resolve, reject, task: Task, currentToken, currentCar
reject('Error executing sync task ' + task.url + ' the required cartId argument is null. Re-creating shopping cart synchro.')
return
}
let url = task.url.replace('{{token}}', (currentToken == null) ? '' : currentToken).replace('{{cartId}}', (currentCartId == null) ? '' : currentCartId)
url = processURLAddress(url); // use relative url paths
if (config.storeViews.multistore) {
url = adjustMultistoreApiUrl(url)
}
const url = getUrl(task, currentToken, currentCartId)
const payload = getPayload(task, currentToken)
let silentMode = false
Logger.info('Executing sync task ' + url, 'sync', task)()
return fetch(url, task.payload).then((response) => {
return fetch(url, payload).then((response) => {
const contentType = response.headers.get('content-type')
if (contentType && contentType.includes('application/json')) {
return response.json()
Expand Down
4 changes: 3 additions & 1 deletion core/modules/user/components/UserAccount.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import toString from 'lodash-es/toString'
import pick from 'lodash-es/pick'
import config from 'config'
const Countries = require('@vue-storefront/core/i18n/resource/countries.json')

export const UserAccount = {
Expand Down Expand Up @@ -86,7 +88,7 @@ export const UserAccount = {
!this.objectsEqual(this.userCompany, this.getUserCompany()) ||
(this.userCompany.company && !this.addCompany)
) {
updatedProfile = JSON.parse(JSON.stringify(this.$store.state.user.current))
updatedProfile = pick(JSON.parse(JSON.stringify(this.$store.state.user.current)), [...config.users.allowModification, 'default_billing'])
updatedProfile.firstname = this.currentUser.firstname
updatedProfile.lastname = this.currentUser.lastname
updatedProfile.email = this.currentUser.email
Expand Down
4 changes: 3 additions & 1 deletion core/modules/user/components/UserShippingDetails.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import toString from 'lodash-es/toString'
import pick from 'lodash-es/pick'
import config from 'config'
const Countries = require('@vue-storefront/i18n/resource/countries.json')

export const UserShippingDetails = {
Expand Down Expand Up @@ -80,7 +82,7 @@ export const UserShippingDetails = {
updateDetails () {
let updatedShippingDetails
if (!this.objectsEqual(this.shippingDetails, this.getShippingDetails())) {
updatedShippingDetails = JSON.parse(JSON.stringify(this.$store.state.user.current))
updatedShippingDetails = pick(JSON.parse(JSON.stringify(this.$store.state.user.current)), config.users.allowModification)
let updatedShippingDetailsAddress = {
firstname: this.shippingDetails.firstName,
lastname: this.shippingDetails.lastName,
Expand Down
1 change: 1 addition & 0 deletions core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"compression": "^1.7.4",
"config": "^1.30.0",
"express": "^4.14.0",
"helmet": "^3.23.3",
"html-minifier": "^4.0.0",
"lean-he": "^2.0.0",
"localforage": "^1.7.2",
Expand Down
5 changes: 5 additions & 0 deletions core/scripts/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ serverHooksExecutors.afterProcessStarted(config.server)
const express = require('express')
const ms = require('ms')
const request = require('request');
const helmet = require('helmet')

const cache = require('./utils/cache-instance')
const apiStatus = require('./utils/api-status')
Expand Down Expand Up @@ -137,6 +138,10 @@ const serve = (path, cache, options?) => express.static(resolve(path), Object.as

const themeRoot = require('../build/theme-path')

if (config.server.helmet && config.server.helmet.enabled && isProd) {
app.use(helmet(config.server.helmet.config))
}

app.use('/dist', serve('dist', true))
app.use('/assets', serve(themeRoot + '/assets', true))
app.use('/service-worker.js', serve('dist/service-worker.js', false, {
Expand Down

0 comments on commit 1cc8ece

Please sign in to comment.