From fbf818b2dc7e081387d31a72b45a9a8f950f641e Mon Sep 17 00:00:00 2001
From: Aaron Buchanan <aaron.buchanan@hyfn.com>
Date: Fri, 17 Mar 2017 17:08:17 -0700
Subject: [PATCH] Fix user-literal rewrite for anonymous requests

Currently any `currentUserLiteral` routes when accessed with a bad
token throw a 500 due to a SQL error that is raised because
`Model.findById` is invoked with `id={currentUserLiteral}`
(`id=me` in our case) when the url rewrite fails.

This commit changes the token middleware to return 401 Not Authorized
when the client is requesting a currentUserLiteral route without
a valid access token.
---
 server/middleware/token.js | 37 +++++++++++++++++++++++++++----------
 test/access-token.test.js  | 20 ++++++++++++++++++++
 2 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/server/middleware/token.js b/server/middleware/token.js
index d885e429..2268f91e 100644
--- a/server/middleware/token.js
+++ b/server/middleware/token.js
@@ -8,6 +8,7 @@
  */
 
 'use strict';
+var g = require('../../lib/globalize');
 var loopback = require('../../lib/loopback');
 var assert = require('assert');
 var debug = require('debug')('loopback:middleware:token');
@@ -21,18 +22,33 @@ module.exports = token;
 /*
  * Rewrite the url to replace current user literal with the logged in user id
  */
-function rewriteUserLiteral(req, currentUserLiteral) {
-  if (req.accessToken && req.accessToken.userId && currentUserLiteral) {
+function rewriteUserLiteral(req, currentUserLiteral, next) {
+  if (!currentUserLiteral) return next();
+  const literalRegExp = new RegExp('/' + currentUserLiteral + '(/|$|\\?)', 'g');
+
+  if (req.accessToken && req.accessToken.userId) {
     // Replace /me/ with /current-user-id/
     var urlBeforeRewrite = req.url;
-    req.url = req.url.replace(
-      new RegExp('/' + currentUserLiteral + '(/|$|\\?)', 'g'),
+    req.url = req.url.replace(literalRegExp,
         '/' + req.accessToken.userId + '$1');
+
     if (req.url !== urlBeforeRewrite) {
       debug('req.url has been rewritten from %s to %s', urlBeforeRewrite,
         req.url);
     }
+  } else if (!req.accessToken && literalRegExp.test(req.url)) {
+    debug(
+      'URL %s matches current-user literal %s,' +
+        ' but no (valid) access token was provided.',
+      req.url, currentUserLiteral);
+
+    var e = new Error(g.f('Authorization Required'));
+    e.status = e.statusCode = 401;
+    e.code = 'AUTHORIZATION_REQUIRED';
+    return next(e);
   }
+
+  next();
 }
 
 function escapeRegExp(str) {
@@ -105,23 +121,24 @@ function token(options) {
       if (!enableDoublecheck) {
         // req.accessToken is defined already (might also be "null" or "false") and enableDoublecheck
         // has not been set --> skip searching for credentials
-        rewriteUserLiteral(req, currentUserLiteral);
-        return next();
+        return rewriteUserLiteral(req, currentUserLiteral, next);
       }
       if (req.accessToken && req.accessToken.id && !overwriteExistingToken) {
         // req.accessToken.id is defined, which means that some other middleware has identified a valid user.
         // when overwriteExistingToken is not set to a truthy value, skip searching for credentials.
-        rewriteUserLiteral(req, currentUserLiteral);
-        return next();
+        return rewriteUserLiteral(req, currentUserLiteral, next);
       }
       // continue normal operation (as if req.accessToken was undefined)
     }
+
     TokenModel.findForRequest(req, options, function(err, token) {
       req.accessToken = token || null;
-      rewriteUserLiteral(req, currentUserLiteral);
+
       var ctx = req.loopbackContext;
       if (ctx && ctx.active) ctx.set('accessToken', token);
-      next(err);
+
+      if (err) return next(err);
+      rewriteUserLiteral(req, currentUserLiteral, next);
     });
   };
 }
diff --git a/test/access-token.test.js b/test/access-token.test.js
index a426c891..6e74ce69 100644
--- a/test/access-token.test.js
+++ b/test/access-token.test.js
@@ -289,6 +289,26 @@ describe('loopback.token(options)', function() {
         });
     });
 
+  it('should generate a 401 on a current user literal route without an authToken',
+    function(done) {
+      var app = createTestApp(null, done);
+      request(app)
+        .get('/users/me')
+        .set('authorization', null)
+        .expect(401)
+        .end(done);
+    });
+
+  it('should generate a 401 on a current user literal route with invalid authToken',
+    function(done) {
+      var app = createTestApp(this.token, done);
+      request(app)
+        .get('/users/me')
+        .set('Authorization', 'invald-token-id')
+        .expect(401)
+        .end(done);
+    });
+
   it('should skip when req.token is already present', function(done) {
     var tokenStub = {id: 'stub id'};
     app.use(function(req, res, next) {