feat($http): JSONP requests now require a trusted resource URL

petebacondarwin · ellimist · commit dbfd744825f7 · 2017-03-15T09:01:03.000+01:00
The $http service will reject JSONP requests that are not trusted by `$sce` as "ResourceUrl". This change makes is easier for developers to see clearly where in their code they are making JSONP calls that may be to untrusted endpoings and forces them to think about how these URLs are generated. Be aware that this commit does not put any constraint on the parameters that will be appended to the URL. Developers should be mindful of what parameters can be attached and how they are generated. Closes angular#11352 BREAKING CHANGE All JSONP requests now require the URL to be trusted as resource URLs. There are two approaches to trust a URL: **Whitelisting with the `$sceDelegateProvider.resourceUrlWhitelist()` method.** You configure this list in a module configuration block: ``` appModule.config(['$sceDelegateProvider', function($sceDelegateProvider) { $sceDelegateProvider.resourceUrlWhiteList([ // Allow same origin resource loads. 'self', // Allow JSONP calls that match this pattern 'https://some.dataserver.com/**.jsonp?**` ]); }]); ``` **Explicitly trusting the URL via the `$sce.trustAsResourceUrl(url)` method** You can pass a trusted object instead of a string as a URL to the `$http` service: ``` var promise = $http.jsonp($sce.trustAsResourceUrl(url)); ```
diff --git a/docs/content/error/$http/badreq.ngdoc b/docs/content/error/$http/badreq.ngdoc
@@ -3,7 +3,11 @@
 @fullName Bad Request Configuration
 @description
 
-This error occurs when the request configuration parameter passed to the {@link ng.$http `$http`} service is not an object.  `$http` expects a single parameter, the request configuration object, but received a parameter that was not an object.  The error message should provide additional context such as the actual value of the parameter that was received.  If you passed a string parameter, perhaps you meant to call one of the shorthand methods on `$http` such as `$http.get(…)`, etc.
+This error occurs when the request configuration parameter passed to the {@link ng.$http `$http`} service is not a valid object.
+`$http` expects a single parameter, the request configuration object, but received a parameter that was not an object or did not contain valid properties.
+
+The error message should provide additional context such as the actual value of the parameter that was received.
+If you passed a string parameter, perhaps you meant to call one of the shorthand methods on `$http` such as `$http.get(…)`, etc.
 
 To resolve this error, make sure you pass a valid request configuration object to `$http`.
 
diff --git a/src/ng/http.js b/src/ng/http.js
@@ -379,8 +379,8 @@ function $HttpProvider() {
    **/
   var interceptorFactories = this.interceptors = [];
 
-  this.$get = ['$browser', '$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector',
-      function($browser, $httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) {
+  this.$get = ['$browser', '$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector', '$sce',
+      function($browser, $httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector, $sce) {
 
     var defaultCache = $cacheFactory('$http');
 
@@ -802,7 +802,8 @@ function $HttpProvider() {
      *    processed. The object has following properties:
      *
      *    - **method** – `{string}` – HTTP method (e.g. 'GET', 'POST', etc)
-     *    - **url** – `{string}` – Absolute or relative URL of the resource that is being requested.
+     *    - **url** – `{string|TrustedObject}` – Absolute or relative URL of the resource that is being requested;
+     *      or an object created by a call to `$sce.trustAsResourceUrl(url)`.
      *    - **params** – `{Object.<string|Object>}` – Map of strings or objects which will be serialized
      *      with the `paramSerializer` and appended as GET parameters.
      *    - **data** – `{string|Object}` – Data to be sent as the request message data.
@@ -881,6 +882,13 @@ function $HttpProvider() {
 </file>
 <file name="script.js">
   angular.module('httpExample', [])
+    .config(['$sceDelegateProvider', function($sceDelegateProvider) {
+      // We must whitelist the JSONP endpoint that we are using to show that we trust it
+      $sceDelegateProvider.resourceUrlWhitelist([
+        'self',
+        'https://angularjs.org/**'
+      ]);
+    }])
     .controller('FetchController', ['$scope', '$http', '$templateCache',
       function($scope, $http, $templateCache) {
         $scope.method = 'GET';
@@ -948,8 +956,8 @@ function $HttpProvider() {
         throw minErr('$http')('badreq', 'Http request configuration must be an object.  Received: {0}', requestConfig);
       }
 
-      if (!isString(requestConfig.url)) {
-        throw minErr('$http')('badreq', 'Http request configuration url must be a string.  Received: {0}', requestConfig.url);
+      if (!isString($sce.valueOf(requestConfig.url))) {
+        throw minErr('$http')('badreq', 'Http request configuration url must be a string or a $sce trusted object.  Received: {0}', requestConfig.url);
       }
 
       var config = extend({
@@ -1111,7 +1119,8 @@ function $HttpProvider() {
      * @description
      * Shortcut method to perform `GET` request.
      *
-     * @param {string} url Relative or absolute URL specifying the destination of the request
+     * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
+     *                                   or an object created by a call to `$sce.trustAsResourceUrl(url)`.
      * @param {Object=} config Optional configuration object
      * @returns {HttpPromise} Future object
      */
@@ -1123,7 +1132,8 @@ function $HttpProvider() {
      * @description
      * Shortcut method to perform `DELETE` request.
      *
-     * @param {string} url Relative or absolute URL specifying the destination of the request
+     * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
+     *                                   or an object created by a call to `$sce.trustAsResourceUrl(url)`.
      * @param {Object=} config Optional configuration object
      * @returns {HttpPromise} Future object
      */
@@ -1135,7 +1145,8 @@ function $HttpProvider() {
      * @description
      * Shortcut method to perform `HEAD` request.
      *
-     * @param {string} url Relative or absolute URL specifying the destination of the request
+     * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
+     *                                   or an object created by a call to `$sce.trustAsResourceUrl(url)`.
      * @param {Object=} config Optional configuration object
      * @returns {HttpPromise} Future object
      */
@@ -1146,11 +1157,18 @@ function $HttpProvider() {
      *
      * @description
      * Shortcut method to perform `JSONP` request.
-     * If you would like to customize where and how the callbacks are stored then try overriding
+     *
+     * Note that, since JSONP requests are sensitive because the response is given full acces to the browser,
+     * the url must be declared, via {@link $sce} as a trusted resource URL.
+     * You can trust a URL by adding it to the whitelist via
+     * {@link $sceDelegateProvider#resourceUrlWhitelist  `$sceDelegateProvider.resourceUrlWhitelist`} or
+     * by explicitly trusted the URL via {@link $sce#trustAsResourceUrl `$sce.trustAsResourceUrl(url)`}.
+     *
+     * If you would like to customise where and how the callbacks are stored then try overriding
      * or decorating the {@link $jsonpCallbacks} service.
      *
-     * @param {string} url Relative or absolute URL specifying the destination of the request.
-     *                     The name of the callback should be the string `JSON_CALLBACK`.
+     * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
+     *                                   or an object created by a call to `$sce.trustAsResourceUrl(url)`.
      * @param {Object=} config Optional configuration object
      * @returns {HttpPromise} Future object
      */
@@ -1249,12 +1267,22 @@ function $HttpProvider() {
           cache,
           cachedResp,
           reqHeaders = config.headers,
-          url = buildUrl(config.url, config.paramSerializer(config.params));
+          url = config.url;
+
+      if (lowercase(config.method) === 'jsonp') {
+        // JSONP is a pretty sensitive operation where we're allowing a script to have full access to
+        // our DOM and JS space.  So we require that the URL satisfies SCE.RESOURCE_URL.
+        url = $sce.getTrustedResourceUrl(url);
+      } else if (!isString(url)) {
+        // If it is not a string then the URL must be a $sce trusted object
+        url = $sce.valueOf(url);
+      }
+
+      url = buildUrl(url, config.paramSerializer(config.params));
 
       $http.pendingRequests.push(config);
       promise.then(removePendingReq, removePendingReq);
 
-
       if ((config.cache || defaults.cache) && config.cache !== false &&
           (config.method === 'GET' || config.method === 'JSONP')) {
         cache = isObject(config.cache) ? config.cache
diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js
@@ -289,27 +289,48 @@ describe('$http', function() {
 
 
   describe('the instance', function() {
-    var $httpBackend, $http, $rootScope;
+    var $httpBackend, $http, $rootScope, $sce;
 
-    beforeEach(inject(['$httpBackend', '$http', '$rootScope', function($hb, $h, $rs) {
+    beforeEach(module(function($sceDelegateProvider) {
+      // Setup a special whitelisted url that we can use in testing JSONP requests
+      $sceDelegateProvider.resourceUrlWhitelist(['http://special.whitelisted.resource.com/**']);
+    }));
+
+    beforeEach(inject(['$httpBackend', '$http', '$rootScope', '$sce', function($hb, $h, $rs, $sc) {
       $httpBackend = $hb;
       $http = $h;
       $rootScope = $rs;
+      $sce = $sc;
       spyOn($rootScope, '$apply').and.callThrough();
     }]));
 
     it('should throw error if the request configuration is not an object', function() {
       expect(function() {
-          $http('/url');
+        $http('/url');
       }).toThrowMinErr('$http','badreq', 'Http request configuration must be an object.  Received: /url');
     });
 
-    it('should throw error if the request configuration url is not a string', function() {
+    it('should throw error if the request configuration url is not a string nor a trusted object', function() {
+      expect(function() {
+        $http({url: false});
+      }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string or a $sce trusted object.  Received: false');
+      expect(function() {
+        $http({url: null});
+      }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string or a $sce trusted object.  Received: null');
+      expect(function() {
+        $http({url: 42});
+      }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string or a $sce trusted object.  Received: 42');
       expect(function() {
-          $http({url: false});
-      }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string.  Received: false');
+        $http({});
+      }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string or a $sce trusted object.  Received: undefined');
     });
 
+    it('should accept a $sce trusted object for the request configuration url', function() {
+      expect(function() {
+        $httpBackend.expect('GET', '/url').respond('');
+        $http({url: $sce.trustAsResourceUrl('/url')});
+      }).not.toThrowMinErr('$http','badreq', 'Http request configuration url must be a string.  Received: false');
+    });
 
     it('should send GET requests if no method specified', function() {
       $httpBackend.expect('GET', '/url').respond('');
@@ -602,7 +623,7 @@ describe('$http', function() {
         });
 
         $httpBackend.expect('JSONP', '/some').respond(200);
-        $http({url: '/some', method: 'JSONP'}).then(callback);
+        $http({url: $sce.trustAsResourceUrl('/some'), method: 'JSONP'}).then(callback);
         $httpBackend.flush();
         expect(callback).toHaveBeenCalledOnce();
       });
@@ -1010,16 +1031,39 @@ describe('$http', function() {
 
       it('should have jsonp()', function() {
         $httpBackend.expect('JSONP', '/url').respond('');
-        $http.jsonp('/url');
+        $http.jsonp($sce.trustAsResourceUrl('/url'));
       });
 
 
       it('jsonp() should allow config param', function() {
         $httpBackend.expect('JSONP', '/url', undefined, checkHeader('Custom', 'Header')).respond('');
-        $http.jsonp('/url', {headers: {'Custom': 'Header'}});
+        $http.jsonp($sce.trustAsResourceUrl('/url'), {headers: {'Custom': 'Header'}});
       });
     });
 
+    describe('jsonp trust', function() {
+      it('should throw error if the url is not a trusted resource', function() {
+        var success, error;
+        $http({method: 'JSONP', url: 'http://example.org/path?cb=JSON_CALLBACK'}).catch(
+          function(e) { error = e; }
+        );
+        $rootScope.$digest();
+        expect(error.message).toContain('[$sce:insecurl]');
+      });
+
+      it('should accept an explicitly trusted resource url', function() {
+        $httpBackend.expect('JSONP', 'http://example.org/path?cb=JSON_CALLBACK').respond('');
+        $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?cb=JSON_CALLBACK')});
+      });
+
+      it('jsonp() should accept explictly trusted urls', function() {
+        $httpBackend.expect('JSONP', '/url').respond('');
+        $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url')});
+
+        $httpBackend.expect('JSONP', '/url?a=b').respond('');
+        $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), params: {a: 'b'}});
+      });
+    });
 
     describe('callbacks', function() {
 
@@ -1481,10 +1525,10 @@ describe('$http', function() {
 
       it('should cache JSONP request when cache is provided', inject(function($rootScope) {
         $httpBackend.expect('JSONP', '/url?cb=JSON_CALLBACK').respond('content');
-        $http({method: 'JSONP', url: '/url?cb=JSON_CALLBACK', cache: cache});
+        $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url?cb=JSON_CALLBACK'), cache: cache});
         $httpBackend.flush();
 
-        $http({method: 'JSONP', url: '/url?cb=JSON_CALLBACK', cache: cache}).success(callback);
+        $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url?cb=JSON_CALLBACK'), cache: cache}).success(callback);
         $rootScope.$digest();
 
         expect(callback).toHaveBeenCalledOnce();
