Skip to content

Commit 70df297

Browse files
committed
Add token based security feature
In the current setup users could be tricked into deleting their data by providing a malicious link like `[click me](/me/delete)`. This commit prevents such an easy attack and need the user's deleteToken to get his data deleted. In case someone requests his deletion by email you can also ask him for this token. We can add a GUI that shows it later on. Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
1 parent 9fd09a8 commit 70df297

5 files changed

Lines changed: 53 additions & 13 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict'
2+
module.exports = {
3+
up: function (queryInterface, Sequelize) {
4+
return queryInterface.addColumn('Users', 'deleteToken', {
5+
type: Sequelize.UUID,
6+
defaultValue: Sequelize.UUIDV4
7+
})
8+
},
9+
10+
down: function (queryInterface, Sequelize) {
11+
return queryInterface.removeColumn('Users', 'deleteToken')
12+
}
13+
}

lib/models/user.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ module.exports = function (sequelize, DataTypes) {
3131
refreshToken: {
3232
type: DataTypes.STRING
3333
},
34+
deleteToken: {
35+
type: DataTypes.UUID,
36+
defaultValue: Sequelize.UUIDV4
37+
},
3438
email: {
3539
type: Sequelize.TEXT,
3640
validate: {

lib/response.js

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,10 @@ function responseError (res, code, detail, msg) {
5656
}
5757

5858
function showIndex (req, res, next) {
59-
res.render(config.indexPath, {
59+
var authStatus = req.isAuthenticated()
60+
var deleteToken = ''
61+
62+
var data = {
6063
url: config.serverURL,
6164
useCDN: config.useCDN,
6265
allowAnonymous: config.allowAnonymous,
@@ -74,12 +77,28 @@ function showIndex (req, res, next) {
7477
email: config.isEmailEnable,
7578
allowEmailRegister: config.allowEmailRegister,
7679
allowPDFExport: config.allowPDFExport,
77-
signin: req.isAuthenticated(),
80+
signin: authStatus,
7881
infoMessage: req.flash('info'),
7982
errorMessage: req.flash('error'),
8083
privacyStatement: fs.existsSync(path.join(config.docsPath, 'privacy.md')),
81-
termsOfUse: fs.existsSync(path.join(config.docsPath, 'terms-of-use.md'))
82-
})
84+
termsOfUse: fs.existsSync(path.join(config.docsPath, 'terms-of-use.md')),
85+
deleteToken: deleteToken
86+
}
87+
88+
if (authStatus) {
89+
models.User.findOne({
90+
where: {
91+
id: req.user.id
92+
}
93+
}).then(function (user) {
94+
if (user) {
95+
data.deleteToken = user.deleteToken
96+
res.render(config.indexPath, data)
97+
}
98+
})
99+
} else {
100+
res.render(config.indexPath, data)
101+
}
83102
}
84103

85104
function responseHackMD (res, note) {

lib/web/userRouter.js

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,25 +38,29 @@ UserRouter.get('/me', function (req, res) {
3838
})
3939

4040
// delete the currently authenticated user
41-
UserRouter.get('/me/delete', function (req, res) {
41+
UserRouter.get('/me/delete/:token?', function (req, res) {
4242
if (req.isAuthenticated()) {
4343
models.User.findOne({
4444
where: {
4545
id: req.user.id
4646
}
4747
}).then(function (user) {
48-
if (!user) { return response.errorNotFound(res) }
49-
user.destroy().then(function () {
50-
res.redirect(config.serverURL + '/')
51-
})
48+
if (!user) {
49+
return response.errorNotFound(res)
50+
}
51+
if (user.deleteToken === req.params.token) {
52+
user.destroy().then(function () {
53+
res.redirect(config.serverURL + '/')
54+
})
55+
} else {
56+
return response.errorForbidden(res)
57+
}
5258
}).catch(function (err) {
5359
logger.error('delete user failed: ' + err)
5460
return response.errorInternalError(res)
5561
})
5662
} else {
57-
res.send({
58-
status: 'forbidden'
59-
})
63+
return response.errorForbidden(res)
6064
}
6165
})
6266

public/views/index/body.ejs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@
193193
</div>
194194
<div class="modal-footer">
195195
<button type="button" class="btn btn-default ui-delete-user-modal-cancel" data-dismiss="modal"><%= __('Cancel') %></button>
196-
<a type="button" class="btn btn-danger" href="<%- url %>/me/delete"><%= __('Yes, do it!') %></a>
196+
<a type="button" class="btn btn-danger" href="<%- url %>/me/delete/<%- deleteToken %>"><%= __('Yes, do it!') %></a>
197197
</div>
198198
</div>
199199
</div>

0 commit comments

Comments
 (0)