From b0b11573f6b15e71d0c79b1716235dfdc598be33 Mon Sep 17 00:00:00 2001 From: Gabor Javorszky Date: Fri, 14 Jun 2013 23:12:04 +0100 Subject: [PATCH] Restricting Admin user creation Solves #138. * Removed user and user_roles from fixture * Restricted user creation to one user. That user is id 1, is admin * Changed tests so they accommodate for this fact * Can not create new user (fails on test, flashes on signup) --- Gruntfile.js | 9 +++- core/shared/data/fixtures/001.js | 23 --------- core/shared/data/migration/001.js | 4 +- core/shared/models/user.js | 47 ++++++++++++++++++- core/test/ghost/api_users_spec.js | 72 +++++++++++++++++------------ core/test/ghost/helpers.js | 36 ++++++++++++++- core/test/ghost/permissions_spec.js | 56 +++++++++++++--------- package.json | 3 +- 8 files changed, 170 insertions(+), 80 deletions(-) diff --git a/Gruntfile.js b/Gruntfile.js index 0d22104848..6c5e6b5b5d 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -46,6 +46,10 @@ api: { src: ['core/test/**/api*_spec.js'] + }, + + perm: { + src: ['core/test/**/permissions_spec.js'] } }, @@ -162,6 +166,9 @@ // Run API tests only grunt.registerTask("test-api", ["mochaTest:api"]); + // Run permisisons tests only + grunt.registerTask("test-p", ["mochaTest:perm"]); + // Run tests and lint code grunt.registerTask("validate", ["jslint", "mochaTest:all"]); @@ -183,7 +190,7 @@ "bump", "updateCurrentPackageInfo", "copy:nightly", - "zip:nightly", + "zip:nightly" /* Caution: shit gets real below here */ //"shell:commitNightly", //"shell:tagNightly", diff --git a/core/shared/data/fixtures/001.js b/core/shared/data/fixtures/001.js index 81ab53f1b9..754c40614c 100644 --- a/core/shared/data/fixtures/001.js +++ b/core/shared/data/fixtures/001.js @@ -84,21 +84,6 @@ module.exports = { } ], - users: [ - { - "id": "1", - "full_name": "John O'Nolan", - "password": "$2a$10$.pb3wOEhbEPvArvOBB.iyuKslBjC7lSXCUzp29civDTvCg3M1j0XO", - "email_address": "john@onolan.org", - "profile_picture": "logo.png", - "cover_picture": "", - "bio": "Interactive designer, public speaker, startup advisor and writer. Living in Austria, attempting world domination via keyboard.", - "url": "john.onolan.org", - "created_by": 1, - "updated_by": 1 - } - ], - roles: [ { "id": 1, @@ -117,14 +102,6 @@ module.exports = { } ], - roles_users: [ - { - "id": 1, - "role_id": 1, - "user_id": 1 - } - ], - permissions: [ { "id": 1, diff --git a/core/shared/data/migration/001.js b/core/shared/data/migration/001.js index a7aada7d6a..6e27a2b59a 100644 --- a/core/shared/data/migration/001.js +++ b/core/shared/data/migration/001.js @@ -102,9 +102,9 @@ return when.all([ knex('posts').insert(fixtures.posts), - knex('users').insert(fixtures.users), + // knex('users').insert(fixtures.users), knex('roles').insert(fixtures.roles), - knex('roles_users').insert(fixtures.roles_users), + // knex('roles_users').insert(fixtures.roles_users), knex('permissions').insert(fixtures.permissions), knex('permissions_roles').insert(fixtures.permissions_roles), knex('settings').insert(fixtures.settings) diff --git a/core/shared/models/user.js b/core/shared/models/user.js index d7355fd873..d21e1622ef 100644 --- a/core/shared/models/user.js +++ b/core/shared/models/user.js @@ -3,6 +3,8 @@ var User, Users, + UserRole, + // UserRoles, _ = require('underscore'), when = require('when'), nodefn = require('when/node/function'), @@ -12,6 +14,13 @@ Role = require('./role').Role, Permission = require('./permission').Permission; + + + UserRole = GhostBookshelf.Model.extend({ + tableName: 'roles_users' + }); + + User = GhostBookshelf.Model.extend({ tableName: 'users', @@ -39,10 +48,45 @@ * Hashes the password provided before saving to the database. */ add: function (_user) { + var User = this, // Clone the _user so we don't expose the hashed password unnecessarily - userData = _.extend({}, _user); + userData = _.extend({}, _user), + fail = false, + userRoles = { + "role_id": 1, + "user_id": 1 + }; + + /** + * This only allows one user to be added to the database, otherwise fails. + * @param {object} user + * @author javorszky + */ + return this.forge().fetch().then(function (user) { + + _.each(user.attributes, function (value, key, list) { + fail = true; + }); + + if (fail) { + return when.reject(new Error('A user is already registered. Only one user for now!')); + } + + return nodefn.call(bcrypt.hash, _user.password, null, null).then(function (hash) { + userData.password = hash; + GhostBookshelf.Model.add.call(UserRole, userRoles); + return GhostBookshelf.Model.add.call(User, userData); + }); + }); + + /** + * Temporarily replacing the function below with another one that checks + * whether there's anyone registered at all. This is due to #138 + * @author javorszky + */ + /** return this.forge({email_address: userData.email_address}).fetch().then(function (user) { if (!!user.attributes.email_address) { return when.reject(new Error('A user with that email address already exists.')); @@ -53,6 +97,7 @@ return GhostBookshelf.Model.add.call(User, userData); }); }); + */ }, /** diff --git a/core/test/ghost/api_users_spec.js b/core/test/ghost/api_users_spec.js index 65386b985c..e04272ddae 100644 --- a/core/test/ghost/api_users_spec.js +++ b/core/test/ghost/api_users_spec.js @@ -7,22 +7,54 @@ should = require('should'), helpers = require('./helpers'), errors = require('../../shared/errorHandling'), - Models = require('../../shared/models'); + Models = require('../../shared/models'), + when = require('when'); + + require('mocha-as-promised')(); describe('User Model', function () { var UserModel = Models.User; beforeEach(function (done) { - helpers.resetData().then(function () { + helpers.resetData().then(function (result) { + return when(helpers.insertDefaultUser()); + }).then(function (results) { done(); - }, done); + }); + }); + + it('can add first', function (done) { + var userData = { + password: 'testpass1', + email_address: "test@test1.com" + }; + + when(helpers.resetData()).then(function (result) { + UserModel.add(userData).then(function (createdUser) { + should.exist(createdUser); + createdUser.attributes.password.should.not.equal(userData.password, "password was hashed"); + createdUser.attributes.email_address.should.eql(userData.email_address, "email address corred"); + + done(); + }, done); + }); + }); + + it('can\'t add second', function (done) { + var userData = { + password: 'testpass3', + email_address: "test3@test1.com" + }; + + return when(UserModel.add(userData)).otherwise(function (failure) { + return failure.message.should.eql('A user is already registered. Only one user for now!'); + }); }); it('can browse', function (done) { UserModel.browse().then(function (results) { - should.exist(results); results.length.should.be.above(0); @@ -81,21 +113,14 @@ }).then(null, done); }); - it('can add', function (done) { - var userData = { - password: 'testpass1', - email_address: "test@test1.com" - }; + it("can get effective permissions", function (done) { + UserModel.effectivePermissions(1).then(function (effectivePermissions) { + should.exist(effectivePermissions); - UserModel.add(userData).then(function (createdUser) { - - should.exist(createdUser); - - createdUser.attributes.password.should.not.equal(userData.password, "password was hashed"); - createdUser.attributes.email_address.should.eql(userData.email_address, "email address corred"); + effectivePermissions.length.should.be.above(0); done(); - }).then(null, done); + }, errors.logError); }); it('can delete', function (done) { @@ -124,27 +149,14 @@ } ids = _.pluck(newResults.models, "id"); - hasDeletedId = _.any(ids, function (id) { return id === firstUserId; }); hasDeletedId.should.equal(false); - done(); }).then(null, done); }); - - it("can get effective permissions", function (done) { - UserModel.effectivePermissions(1).then(function (effectivePermissions) { - should.exist(effectivePermissions); - - effectivePermissions.length.should.be.above(0); - - done(); - }, errors.logError); - }); }); - -}()); \ No newline at end of file +}()); diff --git a/core/test/ghost/helpers.js b/core/test/ghost/helpers.js index 31202b291d..9744cbb89f 100644 --- a/core/test/ghost/helpers.js +++ b/core/test/ghost/helpers.js @@ -10,7 +10,9 @@ one: require("../../shared/data/migration/001") }, helpers, - samplePost; + samplePost, + sampleUser, + sampleUserRole; samplePost = function (i, status, lang) { return { @@ -27,8 +29,24 @@ }; }; + sampleUser = function (i) { + return { + email_address: "john_" + i + "@onolan.org", + password: "$2a$10$c5G9RS5.dXRt3UqvZ5wNgOLQLc7ZFc2DJo01du0oLT1YYOM67KJMe", + full_name: "John O'Nolan" + }; + }; + + sampleUserRole = function (i) { + return { + role_id: i, + user_id: i + }; + }; + helpers = { resetData: function () { + return migrations.one.down().then(function () { return migrations.one.up(); }); @@ -46,6 +64,22 @@ promises.push(knex('posts').insert(posts)); } return when.all(promises); + }, + insertDefaultUser: function () { + + var users = [], + userRoles = [], + u_promises = []; + + users.push(sampleUser(1)); + userRoles.push(sampleUserRole(1)); + u_promises.push(knex('users').insert(users)); + u_promises.push(knex('roles_users').insert(userRoles)); + + return when.all(u_promises).then(function (results) { + + return; + }); } }; diff --git a/core/test/ghost/permissions_spec.js b/core/test/ghost/permissions_spec.js index 0b8cda6b67..149c90b947 100644 --- a/core/test/ghost/permissions_spec.js +++ b/core/test/ghost/permissions_spec.js @@ -20,9 +20,17 @@ should.exist(permissions); beforeEach(function (done) { - helpers.resetData().then(function () { done(); }, errors.throwError); + helpers.resetData().then(function (result) { + return when(helpers.insertDefaultUser()); + }).then(function (results) { + done(); + }); }); + // beforeEach(function (done) { + // helpers.resetData().then(function () { done(); }, errors.throwError); + // }); + var testPerms = [ { act: "edit", obj: "post" }, { act: "edit", obj: "tag" }, @@ -35,21 +43,21 @@ { act: "remove", obj: "user" } ], currTestPermId = 1, - currTestUserId = 1, - createTestUser = function (email_address) { - if (!email_address) { - currTestUserId += 1; - email_address = "test" + currTestPermId + "@test.com"; - } + // currTestUserId = 1, + // createTestUser = function (email_address) { + // if (!email_address) { + // currTestUserId += 1; + // email_address = "test" + currTestPermId + "@test.com"; + // } - var newUser = { - id: currTestUserId, - email_address: email_address, - password: "testing123" - }; + // var newUser = { + // id: currTestUserId, + // email_address: email_address, + // password: "testing123" + // }; - return UserProvider.add(newUser); - }, + // return UserProvider.add(newUser); + // }, createPermission = function (name, act, obj) { if (!name) { currTestPermId += 1; @@ -248,9 +256,12 @@ return when.resolve(); }); - createTestUser() - .then(function (createdTestUser) { - testUser = createdTestUser; + + + // createTestUser() + UserProvider.browse() + .then(function (foundUser) { + testUser = foundUser.models[0]; return permissions.canThis(testUser).edit.post(123); }) @@ -273,9 +284,12 @@ return when.reject(); }); - createTestUser() - .then(function (createdTestUser) { - testUser = createdTestUser; + + // createTestUser() + UserProvider.browse() + .then(function (foundUser) { + testUser = foundUser.models[0]; + return permissions.canThis(testUser).edit.post(123); }) @@ -294,4 +308,4 @@ }); -}()); \ No newline at end of file +}()); diff --git a/package.json b/package.json index eac3d57649..e9554b6b43 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "grunt-contrib-watch": "~0.4.4", "grunt-bump": "0.0.2", "grunt-zip": "~0.9.0", - "grunt-contrib-copy": "~0.4.1" + "grunt-contrib-copy": "~0.4.1", + "mocha-as-promised": "~1.4.0" } }