🐛 Fixed returning roles for the public user resource (#9039)

no issue

- this bug fix affects all endpoints for the public user access
- we allowed fetching `roles` via the public api by accident
- see our docs: https://api.ghost.org/docs/users)
  - we only allow `count.posts`
- returning roles via the public api exposes too many details
- this was never attentional

core/server/models/base/index.js

@@ -385,7 +385,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
      * @return {Object} The filtered results of `options`.
      */
     filterOptions: function filterOptions(options, methodName) {
-        var permittedOptions = this.permittedOptions(methodName),
+        var permittedOptions = this.permittedOptions(methodName, options),
             filteredOptions = _.pick(options, permittedOptions);
 
         return filteredOptions;

core/server/models/user.js

@@ -295,8 +295,8 @@ User = ghostBookshelf.Model.extend({
     * @param {String} methodName The name of the method to check valid options for.
     * @return {Array} Keys allowed in the `options` hash of the model's method.
     */
-    permittedOptions: function permittedOptions(methodName) {
+    permittedOptions: function permittedOptions(methodName, options) {
-        var options = ghostBookshelf.Model.permittedOptions(),
+        var permittedOptionsToReturn = ghostBookshelf.Model.permittedOptions(),
 
             // whitelists for the `options` hash argument on methods, by method name.
             // these are the only options that can be passed to Bookshelf / Knex.
@@ -310,10 +310,18 @@ User = ghostBookshelf.Model.extend({
             };
 
         if (validOptions[methodName]) {
-            options = options.concat(validOptions[methodName]);
+            permittedOptionsToReturn = permittedOptionsToReturn.concat(validOptions[methodName]);
         }
 
-        return options;
+        // CASE: The `include` paramater is allowed when using the public API, but not the `roles` value.
+        // Otherwise we expose too much information.
+        if (options && options.context && options.context.public) {
+            if (options.include && options.include.indexOf('roles') !== -1) {
+                options.include.splice(options.include.indexOf('roles'), 1);
+            }
+        }
+
+        return permittedOptionsToReturn;
     },
 
     /**
@@ -343,7 +351,11 @@ User = ghostBookshelf.Model.extend({
         options = _.cloneDeep(options || {});
         optInc = options.include;
         options.withRelated = _.union(options.withRelated, options.include);
+
         data = this.filterData(data);
+        options = this.filterOptions(options, 'findOne');
+        delete options.include;
+        options.include = optInc;
 
         // Support finding by role
         if (lookupRole) {
@@ -366,10 +378,6 @@ User = ghostBookshelf.Model.extend({
             query.query('where', {status: status});
         }
 
-        options = this.filterOptions(options, 'findOne');
-        delete options.include;
-        options.include = optInc;
-
         return query.fetch(options);
     },
 

core/test/functional/routes/api/public_api_spec.js

@@ -303,4 +303,142 @@ describe('Public API', function () {
                 done();
             });
     });
+
+    it('browse users', function (done) {
+        request.get(testUtils.API.getApiQuery('users/?client_id=ghost-admin&client_secret=not_available'))
+            .set('Origin', testUtils.API.getURL())
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(200)
+            .end(function (err, res) {
+                if (err) {
+                    return done(err);
+                }
+
+                should.not.exist(res.headers['x-cache-invalidate']);
+                var jsonResponse = res.body;
+                should.exist(jsonResponse.users);
+                testUtils.API.checkResponse(jsonResponse, 'users');
+                jsonResponse.users.should.have.length(2);
+
+                // We don't expose the email address.
+                testUtils.API.checkResponse(jsonResponse.users[0], 'user', null, ['email']);
+                done();
+            });
+    });
+
+    it('browse users: ignores fetching roles', function (done) {
+        request.get(testUtils.API.getApiQuery('users/?client_id=ghost-admin&client_secret=not_available&include=roles'))
+            .set('Origin', testUtils.API.getURL())
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(200)
+            .end(function (err, res) {
+                if (err) {
+                    return done(err);
+                }
+
+                should.not.exist(res.headers['x-cache-invalidate']);
+                var jsonResponse = res.body;
+                should.exist(jsonResponse.users);
+                testUtils.API.checkResponse(jsonResponse, 'users');
+                jsonResponse.users.should.have.length(2);
+
+                // We don't expose the email address.
+                testUtils.API.checkResponse(jsonResponse.users[0], 'user', null, ['email']);
+                done();
+            });
+    });
+
+    it('browse user by slug: ignores fetching roles', function (done) {
+        request.get(testUtils.API.getApiQuery('users/slug/ghost/?client_id=ghost-admin&client_secret=not_available&include=roles'))
+            .set('Origin', testUtils.API.getURL())
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(200)
+            .end(function (err, res) {
+                if (err) {
+                    return done(err);
+                }
+
+                should.not.exist(res.headers['x-cache-invalidate']);
+                var jsonResponse = res.body;
+
+                should.exist(jsonResponse.users);
+                jsonResponse.users.should.have.length(1);
+
+                // We don't expose the email address.
+                testUtils.API.checkResponse(jsonResponse.users[0], 'user', null, ['email']);
+                done();
+            });
+    });
+
+    it('browse user by id: ignores fetching roles', function (done) {
+        request.get(testUtils.API.getApiQuery('users/1/?client_id=ghost-admin&client_secret=not_available&include=roles'))
+            .set('Origin', testUtils.API.getURL())
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(200)
+            .end(function (err, res) {
+                if (err) {
+                    return done(err);
+                }
+
+                should.not.exist(res.headers['x-cache-invalidate']);
+                var jsonResponse = res.body;
+
+                should.exist(jsonResponse.users);
+                jsonResponse.users.should.have.length(1);
+
+                // We don't expose the email address.
+                testUtils.API.checkResponse(jsonResponse.users[0], 'user', null, ['email']);
+                done();
+            });
+    });
+
+    it('browse users: post count', function (done) {
+        request.get(testUtils.API.getApiQuery('users/?client_id=ghost-admin&client_secret=not_available&include=count.posts'))
+            .set('Origin', testUtils.API.getURL())
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(200)
+            .end(function (err, res) {
+                if (err) {
+                    return done(err);
+                }
+
+                should.not.exist(res.headers['x-cache-invalidate']);
+                var jsonResponse = res.body;
+                should.exist(jsonResponse.users);
+                testUtils.API.checkResponse(jsonResponse, 'users');
+                jsonResponse.users.should.have.length(2);
+
+                // We don't expose the email address.
+                testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['count'], ['email']);
+                done();
+            });
+    });
+
+    it('browse users: wrong data type for include', function (done) {
+        request.get(testUtils.API.getApiQuery('users/?client_id=ghost-admin&client_secret=not_available&include={}'))
+            .set('Origin', testUtils.API.getURL())
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(200)
+            .end(function (err, res) {
+                if (err) {
+                    return done(err);
+                }
+
+                should.not.exist(res.headers['x-cache-invalidate']);
+                var jsonResponse = res.body;
+                should.exist(jsonResponse.users);
+                testUtils.API.checkResponse(jsonResponse, 'users');
+                jsonResponse.users.should.have.length(2);
+
+                // We don't expose the email address.
+                testUtils.API.checkResponse(jsonResponse.users[0], 'user', null, ['email']);
+                done();
+            });
+    });
 });