feat: add cookie session for logged in state (#962)

Author: lirantal

README.md

@@ -62,6 +62,7 @@ Here are the exploitable vulnerable packages:
 * Code Injection
 * Command execution
 * Cross-site Scripting (XSS)
+* Information exposure
 * Security misconfiguration exposes server information 
 * Insecure protocol (HTTP) communication 
 
@@ -108,6 +109,31 @@ http://localhost:3001/login?redirectPage="><script>alert(1)</script>
 
 To exploit the open redirect, simply provide a URL such as `redirectPage=https://google.com` which exploits the fact that the code doesn't enforce local URLs in `index.js:72`.
 
+#### Hardcoded values - session information
+
+The application initializes a cookie-based session on `app.js:40` as follows:
+
+```js
+app.use(session({
+  secret: 'keyboard cat',
+  name: 'connect.sid',
+  cookie: { secure: true }
+}))
+```
+
+As you can see, the session `secret` used to sign the session is a hardcoded sensitive information inside the code.
+
+First attempt to fix it, can be to move it out to a config file such as:
+```js
+module.exports = {
+    cookieSecret: `keyboard cat`
+}
+```
+
+And then require the configuration file and use it to initialize the session.
+However, that still maintains the secret information inside another file, and Snyk Code will warn you about it.
+
+Another case we can discuss here in session management, is that the cookie setting is initialized with `secure: true` which means it will only be transmitted over HTTPS connections. However, there's no `httpOnly` flag set to true, which means that the default false value of it makes the cookie accessible via JavaScript. Snyk Code highlights this potential security misconfiguration so we can fix it. We can note that Snyk Code shows this as a quality information, and not as a security error.
 
 ## Docker Image Scanning
 

app.js

@@ -12,8 +12,8 @@ var express = require('express');
 var http = require('http');
 var path = require('path');
 var ejsEngine = require('ejs-locals');
-var cookieParser = require('cookie-parser');
 var bodyParser = require('body-parser');
+var session = require('express-session')
 var methodOverride = require('method-override');
 var logger = require('morgan');
 var errorHandler = require('errorhandler');
@@ -39,7 +39,11 @@ app.set('views', path.join(__dirname, 'views'));
 app.set('view engine', 'ejs');
 app.use(logger('dev'));
 app.use(methodOverride());
-app.use(cookieParser());
+app.use(session({
+  secret: 'keyboard cat',
+  name: 'connect.sid',
+  cookie: { path: '/' }
+}))
 app.use(bodyParser.json());
 app.use(bodyParser.urlencoded({ extended: false }));
 app.use(fileUpload());
@@ -49,9 +53,10 @@ app.use(routes.current_user);
 app.get('/', routes.index);
 app.get('/login', routes.login);
 app.post('/login', routes.loginHandler);
-app.get('/admin', routes.admin);
-app.get('/account_details', routes.get_account_details);
+app.get('/admin', routes.isLoggedIn, routes.admin);
+app.get('/account_details', routes.isLoggedIn, routes.account_details);
 app.post('/account_details', routes.save_account_details);
+app.get('/logout', routes.logout);
 app.post('/create', routes.create);
 app.get('/destroy/:id', routes.destroy);
 app.get('/edit/:id', routes.edit);

package-lock.json

@@ -18,14 +18,14 @@
     "body-parser": "1.9.0",
     "cfenv": "^1.0.4",
     "consolidate": "0.14.5",
-    "cookie-parser": "1.3.3",
     "dustjs-helpers": "1.5.0",
     "dustjs-linkedin": "2.5.0",
     "ejs": "1.0.0",
     "ejs-locals": "1.0.2",
     "errorhandler": "1.2.0",
     "express": "4.12.4",
     "express-fileupload": "0.0.5",
+    "express-session": "^1.17.2",
     "file-type": "^8.1.0",
     "hbs": "^4.0.4",
     "humanize-ms": "1.0.1",

package.json

@@ -38,7 +38,9 @@ exports.loginHandler = function (req, res, next) {
   User.find({ username: req.body.username, password: req.body.password }, function (err, users) {
     if (users.length > 0) {
       const redirectPage = req.body.redirectPage
-      return adminLoginSuccess(redirectPage, res)
+      const session = req.session
+      const username = req.body.username
+      return adminLoginSuccess(redirectPage, session, username, res)
     } else {
       return res.redirect('/admin')
     }
@@ -93,8 +95,27 @@ exports.save_account_details = function(req, res, next) {
   }
 }
 
-function adminLoginSuccess(redirectPage, res) {
-  console.log({redirectPage})
+exports.isLoggedIn = function (req, res, next) {
+  if (req.session.loggedIn === 1) {
+    return next()
+  } else {
+    return res.redirect('/')
+  }
+}
+
+exports.logout = function (req, res, next) {
+  req.session.loggedIn = 0
+  req.session.destroy(function() { 
+    return res.redirect('/')  
+  })
+}
+
+function adminLoginSuccess(redirectPage, session, username, res) {
+  session.loggedIn = 1
+
+  // Log the login action for audit
+  console.log(`User logged in: ${username}`)
+
   if (redirectPage) {
       return res.redirect(redirectPage)
   } else {