From 70df29790a83db4abb40ed1e16cb05a3aa760672 Mon Sep 17 00:00:00 2001
From: Sheogorath
Date: Fri, 25 May 2018 18:19:31 +0200
Subject: 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>
---
 .../20180525153000-user-add-delete-token.js        | 13 +++++++++++
 lib/models/user.js                                 |  4 ++++
 lib/response.js                                    | 27 ++++++++++++++++++----
 lib/web/userRouter.js                              | 20 +++++++++-------
 public/views/index/body.ejs                        |  2 +-
 5 files changed, 53 insertions(+), 13 deletions(-)
 create mode 100644 lib/migrations/20180525153000-user-add-delete-token.js

diff --git a/lib/migrations/20180525153000-user-add-delete-token.js b/lib/migrations/20180525153000-user-add-delete-token.js
new file mode 100644
index 00000000..642fa5d4
--- /dev/null
+++ b/lib/migrations/20180525153000-user-add-delete-token.js
@@ -0,0 +1,13 @@
+'use strict'
+module.exports = {
+  up: function (queryInterface, Sequelize) {
+    return queryInterface.addColumn('Users', 'deleteToken', {
+      type: Sequelize.UUID,
+      defaultValue: Sequelize.UUIDV4
+    })
+  },
+
+  down: function (queryInterface, Sequelize) {
+    return queryInterface.removeColumn('Users', 'deleteToken')
+  }
+}
diff --git a/lib/models/user.js b/lib/models/user.js
index 62ed5cc7..019aab7e 100644
--- a/lib/models/user.js
+++ b/lib/models/user.js
@@ -31,6 +31,10 @@ module.exports = function (sequelize, DataTypes) {
     refreshToken: {
       type: DataTypes.STRING
     },
+    deleteToken: {
+      type: DataTypes.UUID,
+      defaultValue: Sequelize.UUIDV4
+    },
     email: {
       type: Sequelize.TEXT,
       validate: {
diff --git a/lib/response.js b/lib/response.js
index 2ea2f1c6..b1b89c78 100644
--- a/lib/response.js
+++ b/lib/response.js
@@ -56,7 +56,10 @@ function responseError (res, code, detail, msg) {
 }
 
 function showIndex (req, res, next) {
-  res.render(config.indexPath, {
+  var authStatus = req.isAuthenticated()
+  var deleteToken = ''
+
+  var data = {
     url: config.serverURL,
     useCDN: config.useCDN,
     allowAnonymous: config.allowAnonymous,
@@ -74,12 +77,28 @@ function showIndex (req, res, next) {
     email: config.isEmailEnable,
     allowEmailRegister: config.allowEmailRegister,
     allowPDFExport: config.allowPDFExport,
-    signin: req.isAuthenticated(),
+    signin: authStatus,
     infoMessage: req.flash('info'),
     errorMessage: req.flash('error'),
     privacyStatement: fs.existsSync(path.join(config.docsPath, 'privacy.md')),
-    termsOfUse: fs.existsSync(path.join(config.docsPath, 'terms-of-use.md'))
-  })
+    termsOfUse: fs.existsSync(path.join(config.docsPath, 'terms-of-use.md')),
+    deleteToken: deleteToken
+  }
+
+  if (authStatus) {
+    models.User.findOne({
+      where: {
+        id: req.user.id
+      }
+    }).then(function (user) {
+      if (user) {
+        data.deleteToken = user.deleteToken
+        res.render(config.indexPath, data)
+      }
+    })
+  } else {
+    res.render(config.indexPath, data)
+  }
 }
 
 function responseHackMD (res, note) {
diff --git a/lib/web/userRouter.js b/lib/web/userRouter.js
index b8bd9154..6832d901 100644
--- a/lib/web/userRouter.js
+++ b/lib/web/userRouter.js
@@ -38,25 +38,29 @@ UserRouter.get('/me', function (req, res) {
 })
 
 // delete the currently authenticated user
-UserRouter.get('/me/delete', function (req, res) {
+UserRouter.get('/me/delete/:token?', function (req, res) {
   if (req.isAuthenticated()) {
     models.User.findOne({
       where: {
         id: req.user.id
       }
     }).then(function (user) {
-      if (!user) { return response.errorNotFound(res) }
-      user.destroy().then(function () {
-        res.redirect(config.serverURL + '/')
-      })
+      if (!user) {
+        return response.errorNotFound(res)
+      }
+      if (user.deleteToken === req.params.token) {
+        user.destroy().then(function () {
+          res.redirect(config.serverURL + '/')
+        })
+      } else {
+        return response.errorForbidden(res)
+      }
     }).catch(function (err) {
       logger.error('delete user failed: ' + err)
       return response.errorInternalError(res)
     })
   } else {
-    res.send({
-      status: 'forbidden'
-    })
+    return response.errorForbidden(res)
   }
 })
 
diff --git a/public/views/index/body.ejs b/public/views/index/body.ejs
index d4350540..f28ab11d 100644
--- a/public/views/index/body.ejs
+++ b/public/views/index/body.ejs
@@ -193,7 +193,7 @@
             </div>
             <div class="modal-footer">
                 <button type="button" class="btn btn-default ui-delete-user-modal-cancel" data-dismiss="modal"><%= __('Cancel') %></button>
-                <a type="button" class="btn btn-danger" href="<%- url %>/me/delete"><%= __('Yes, do it!') %></a>
+                <a type="button" class="btn btn-danger" href="<%- url %>/me/delete/<%- deleteToken %>"><%= __('Yes, do it!') %></a>
             </div>
         </div>
     </div>
-- 
cgit v1.2.3