Fix acl and url escaping in auth token generation

Author: strausr

lib-es5/auth_token.js

@@ -1,11 +1,14 @@
-"use strict";
+'use strict';
 
 /**
  * Authorization Token
  * @module auth_token
  */
 
 var crypto = require('crypto');
+var smart_escape = require('./utils/smart_escape').smart_escape;
+
+var unsafe = /([ "#%&'/:;<=>?@[\]^`{|}~]+)/g;
 
 function digest(message, key) {
   return crypto.createHmac("sha256", Buffer.from(key, "hex")).update(message).digest('hex');
@@ -17,7 +20,8 @@ function digest(message, key) {
  * @return {string} escaped url
  */
 function escapeToLower(url) {
-  return encodeURIComponent(url).replace(/%../g, function (match) {
+  var safeUrl = smart_escape(url, unsafe);
+  return safeUrl.replace(/%../g, function (match) {
     return match.toLowerCase();
   });
 }
@@ -43,6 +47,7 @@ function escapeToLower(url) {
  */
 module.exports = function (options) {
   var tokenName = options.token_name ? options.token_name : "__cld_token__";
+  var tokenSeparator = "~";
   if (options.expiration == null) {
     if (options.duration != null) {
       var start = options.start_time != null ? options.start_time : Math.round(Date.now() / 1000);
@@ -63,11 +68,11 @@ module.exports = function (options) {
     tokenParts.push(`acl=${escapeToLower(options.acl)}`);
   }
   var toSign = [].concat(tokenParts);
-  if (options.url) {
+  if (options.url != null && options.acl == null) {
     var url = escapeToLower(options.url);
     toSign.push(`url=${url}`);
   }
-  var auth = digest(toSign.join("~"), options.key);
+  var auth = digest(toSign.join(tokenSeparator), options.key);
   tokenParts.push(`hmac=${auth}`);
-  return `${tokenName}=${tokenParts.join('~')}`;
+  return `${tokenName}=${tokenParts.join(tokenSeparator)}`;
 };

lib-es5/utils/index.js

@@ -35,6 +35,7 @@ var isNumber = require("lodash/isNumber");
 var isObject = require("lodash/isObject");
 var isString = require("lodash/isString");
 var isUndefined = require("lodash/isUndefined");
+var smart_escape = require("./smart_escape").smart_escape;
 
 var config = require("../config");
 var generate_token = require("../auth_token");
@@ -1001,18 +1002,6 @@ function unsigned_url_prefix(source, cloud_name, private_cdn, cdn_subdomain, sec
   }
   return prefix;
 }
-// Based on CGI::unescape. In addition does not escape / :
-// smart_escape = (string)->
-//  encodeURIComponent(string).replace(/%3A/g, ":").replace(/%2F/g, "/")
-function smart_escape(string) {
-  var unsafe = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : /([^a-zA-Z0-9_.\-\/:]+)/g;
-
-  return string.replace(unsafe, function (match) {
-    return match.split("").map(function (c) {
-      return "%" + c.charCodeAt(0).toString(16).toUpperCase();
-    }).join("");
-  });
-}
 
 function api_url() {
   var action = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 'upload';

lib-es5/utils/smart_escape.js

@@ -0,0 +1,16 @@
+"use strict";
+
+// Based on CGI::unescape. In addition does not escape / :
+// smart_escape = (string)->
+//  encodeURIComponent(string).replace(/%3A/g, ":").replace(/%2F/g, "/")
+function smart_escape(string) {
+  var unsafe = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : /([^a-zA-Z0-9_.\-\/:]+)/g;
+
+  return string.replace(unsafe, function (match) {
+    return match.split("").map(function (c) {
+      return "%" + c.charCodeAt(0).toString(16).toUpperCase();
+    }).join("");
+  });
+}
+
+exports.smart_escape = smart_escape;

lib/auth_token.js

@@ -4,6 +4,9 @@
  */
 
 const crypto = require('crypto');
+const smart_escape = require('./utils/smart_escape').smart_escape;
+
+const unsafe = /([ "#%&'/:;<=>?@[\]^`{|}~]+)/g;
 
 function digest(message, key) {
   return crypto.createHmac("sha256", Buffer.from(key, "hex")).update(message).digest('hex');
@@ -15,7 +18,8 @@ function digest(message, key) {
  * @return {string} escaped url
  */
 function escapeToLower(url) {
-  return encodeURIComponent(url).replace(/%../g, function (match) {
+  const safeUrl = smart_escape(url, unsafe);
+  return safeUrl.replace(/%../g, function (match) {
     return match.toLowerCase();
   });
 }
@@ -41,6 +45,7 @@ function escapeToLower(url) {
  */
 module.exports = function (options) {
   const tokenName = options.token_name ? options.token_name : "__cld_token__";
+  const tokenSeparator = "~";
   if (options.expiration == null) {
     if (options.duration != null) {
       let start = options.start_time != null ? options.start_time : Math.round(Date.now() / 1000);
@@ -61,11 +66,11 @@ module.exports = function (options) {
     tokenParts.push(`acl=${escapeToLower(options.acl)}`);
   }
   let toSign = [...tokenParts];
-  if (options.url) {
+  if (options.url != null && options.acl == null) {
     let url = escapeToLower(options.url);
     toSign.push(`url=${url}`);
   }
-  let auth = digest(toSign.join("~"), options.key);
+  let auth = digest(toSign.join(tokenSeparator), options.key);
   tokenParts.push(`hmac=${auth}`);
-  return `${tokenName}=${tokenParts.join('~')}`;
+  return `${tokenName}=${tokenParts.join(tokenSeparator)}`;
 };

lib/utils/index.js

@@ -29,6 +29,7 @@ const isNumber = require("lodash/isNumber");
 const isObject = require("lodash/isObject");
 const isString = require("lodash/isString");
 const isUndefined = require("lodash/isUndefined");
+const smart_escape = require("./smart_escape").smart_escape;
 
 const config = require("../config");
 const generate_token = require("../auth_token");
@@ -1008,16 +1009,6 @@ function unsigned_url_prefix(
   }
   return prefix;
 }
-// Based on CGI::unescape. In addition does not escape / :
-// smart_escape = (string)->
-//  encodeURIComponent(string).replace(/%3A/g, ":").replace(/%2F/g, "/")
-function smart_escape(string, unsafe = /([^a-zA-Z0-9_.\-\/:]+)/g) {
-  return string.replace(unsafe, function (match) {
-    return match.split("").map(function (c) {
-      return "%" + c.charCodeAt(0).toString(16).toUpperCase();
-    }).join("");
-  });
-}
 
 function api_url(action = 'upload', options = {}) {
   let cloudinary = ensureOption(options, "upload_prefix", "https://api.cloudinary.com");

lib/utils/smart_escape.js

@@ -0,0 +1,12 @@
+// Based on CGI::unescape. In addition does not escape / :
+// smart_escape = (string)->
+//  encodeURIComponent(string).replace(/%3A/g, ":").replace(/%2F/g, "/")
+function smart_escape(string, unsafe = /([^a-zA-Z0-9_.\-\/:]+)/g) {
+  return string.replace(unsafe, function (match) {
+    return match.split("").map(function (c) {
+      return "%" + c.charCodeAt(0).toString(16).toUpperCase();
+    }).join("");
+  });
+}
+
+exports.smart_escape = smart_escape;

test/authtoken_spec.js

@@ -89,7 +89,7 @@ describe("authToken", function () {
           width: 300,
         },
       });
-      expect(url).to.eql("http://test123-res.cloudinary.com/image/authenticated/c_scale,w_300/sample.jpg?__cld_token__=st=222222222~exp=222222322~hmac=7d276841d70c4ecbd0708275cd6a82e1f08e47838fbb0bceb2538e06ddfa3029");
+      expect(url).to.eql("http://test123-res.cloudinary.com/image/authenticated/c_scale,w_300/sample.jpg?__cld_token__=st=222222222~exp=222222322~hmac=55cfe516530461213fe3b3606014533b1eca8ff60aeab79d1bb84c9322eebc1f");
     });
     it("should compute expiration as start time + duration", function () {
       let token = {