mirror of
https://github.com/TryGhost/Ghost.git
synced 2023-12-13 21:00:40 +01:00
🐛 fix transfer ownership (#8784)
closes #8781
- when the ownership get's transferred, the id of the new owner is not '1' anymore
- we previously added a database rule, which signalises if the blog is setup or not, see 827aa15757 (diff-7a2fe80302d7d6bf67f97cdccef1f71fR542)
- this database rule is based on the owner id being '1', which is wrong when you transfer ownership
- we should keep in mind, that the owner id being '1' is only the default Ghost setup, but it can change
- blog is setup if the owner is locked
This commit is contained in:
parent
0b5de14900
commit
b003a6c173
|
@ -182,54 +182,57 @@ class Base {
|
|||
|
||||
debug('afterImport', this.modelName);
|
||||
|
||||
return Promise.each(this.dataToImport, function (obj) {
|
||||
if (!obj.model) {
|
||||
return;
|
||||
}
|
||||
return models.User.getOwnerUser(options)
|
||||
.then(function (ownerUser) {
|
||||
return Promise.each(self.dataToImport, function (obj) {
|
||||
if (!obj.model) {
|
||||
return;
|
||||
}
|
||||
|
||||
return Promise.each(['author_id', 'published_by', 'created_by', 'updated_by'], function (key) {
|
||||
// CASE: not all fields exist on each model, skip them if so
|
||||
if (!obj[key]) {
|
||||
return Promise.resolve();
|
||||
}
|
||||
return Promise.each(['author_id', 'published_by', 'created_by', 'updated_by'], function (key) {
|
||||
// CASE: not all fields exist on each model, skip them if so
|
||||
if (!obj[key]) {
|
||||
return Promise.resolve();
|
||||
}
|
||||
|
||||
oldUser = _.find(self.users, {id: obj[key]});
|
||||
oldUser = _.find(self.users, {id: obj[key]});
|
||||
|
||||
if (!oldUser) {
|
||||
self.problems.push({
|
||||
message: 'Entry was imported, but we were not able to update user reference field: ' + key,
|
||||
help: self.modelName,
|
||||
context: JSON.stringify(obj)
|
||||
if (!oldUser) {
|
||||
self.problems.push({
|
||||
message: 'Entry was imported, but we were not able to update user reference field: ' + key,
|
||||
help: self.modelName,
|
||||
context: JSON.stringify(obj)
|
||||
});
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
return models.User.findOne({
|
||||
email: oldUser.email,
|
||||
status: 'all'
|
||||
}, options).then(function (userModel) {
|
||||
// CASE: user could not be imported e.g. multiple roles attached
|
||||
if (!userModel) {
|
||||
userModel = {
|
||||
id: ownerUser.id
|
||||
};
|
||||
}
|
||||
|
||||
dataToEdit = {};
|
||||
dataToEdit[key] = userModel.id;
|
||||
|
||||
// CASE: updated_by is taken from the context object
|
||||
if (key === 'updated_by') {
|
||||
context = {context: {user: userModel.id}};
|
||||
} else {
|
||||
context = {};
|
||||
}
|
||||
|
||||
return models[self.modelName].edit(dataToEdit, _.merge({}, options, {id: obj.model.id}, context));
|
||||
});
|
||||
});
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
return models.User.findOne({
|
||||
email: oldUser.email,
|
||||
status: 'all'
|
||||
}, options).then(function (userModel) {
|
||||
// CASE: user could not be imported e.g. multiple roles attached
|
||||
if (!userModel) {
|
||||
userModel = {
|
||||
id: models.User.ownerUser
|
||||
};
|
||||
}
|
||||
|
||||
dataToEdit = {};
|
||||
dataToEdit[key] = userModel.id;
|
||||
|
||||
// CASE: updated_by is taken from the context object
|
||||
if (key === 'updated_by') {
|
||||
context = {context: {user: userModel.id}};
|
||||
} else {
|
||||
context = {};
|
||||
}
|
||||
|
||||
return models[self.modelName].edit(dataToEdit, _.merge({}, options, {id: obj.model.id}, context));
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -302,17 +302,12 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
|
|||
|
||||
/**
|
||||
* please use these static definitions when comparing id's
|
||||
* we keep type number, because we have too many check's where we rely on
|
||||
* we keep type Number, because we have too many check's where we rely on Number
|
||||
* context.user ? true : false (if context.user is 0 as number, this condition is false)
|
||||
*/
|
||||
internalUser: 1,
|
||||
ownerUser: 1,
|
||||
externalUser: 0,
|
||||
|
||||
isOwnerUser: function isOwnerUser(id) {
|
||||
return id === ghostBookshelf.Model.ownerUser || id === ghostBookshelf.Model.ownerUser.toString();
|
||||
},
|
||||
|
||||
isInternalUser: function isInternalUser(id) {
|
||||
return id === ghostBookshelf.Model.internalUser || id === ghostBookshelf.Model.internalUser.toString();
|
||||
},
|
||||
|
|
|
@ -333,7 +333,7 @@ User = ghostBookshelf.Model.extend({
|
|||
status = data.status;
|
||||
delete data.status;
|
||||
|
||||
options = options || {};
|
||||
options = _.cloneDeep(options || {});
|
||||
optInc = options.include;
|
||||
options.withRelated = _.union(options.withRelated, options.include);
|
||||
data = this.filterData(data);
|
||||
|
@ -538,18 +538,36 @@ User = ghostBookshelf.Model.extend({
|
|||
|
||||
/**
|
||||
* Right now the setup of the blog depends on the user status.
|
||||
* @TODO: see https://github.com/TryGhost/Ghost/issues/8003
|
||||
* Only if the owner user is `inactive`, then the blog is not setup.
|
||||
* e.g. if you transfer ownership to a locked user, you blog is still setup.
|
||||
*
|
||||
* @TODO: Rename `inactive` status to something else, it's confusing. e.g. requires-setup
|
||||
* @TODO: Depending on the user status results in https://github.com/TryGhost/Ghost/issues/8003
|
||||
*/
|
||||
isSetup: function isSetup() {
|
||||
return this
|
||||
.where('status', 'in', activeStates)
|
||||
.where('id', this.ownerUser)
|
||||
.count('id')
|
||||
.then(function (count) {
|
||||
return !!count;
|
||||
isSetup: function isSetup(options) {
|
||||
return this.getOwnerUser(options)
|
||||
.then(function (owner) {
|
||||
return owner.get('status') !== 'inactive';
|
||||
});
|
||||
},
|
||||
|
||||
getOwnerUser: function getOwnerUser(options) {
|
||||
options = options || {};
|
||||
|
||||
return this.findOne({
|
||||
role: 'Owner',
|
||||
status: 'all'
|
||||
}, options).then(function (owner) {
|
||||
if (!owner) {
|
||||
return Promise.reject(new errors.NotFoundError({
|
||||
message: i18n.t('errors.models.user.ownerNotFound')
|
||||
}));
|
||||
}
|
||||
|
||||
return owner;
|
||||
});
|
||||
},
|
||||
|
||||
permissible: function permissible(userModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
|
||||
var self = this,
|
||||
userModel = userModelOrId,
|
||||
|
|
|
@ -244,6 +244,7 @@
|
|||
"tokenLocked": "Token locked",
|
||||
"invalidToken": "Invalid token",
|
||||
"userNotFound": "User not found",
|
||||
"ownerNotFound": "Owner not found",
|
||||
"onlyOwnerCanTransferOwnerRole": "Only owners are able to transfer the owner role.",
|
||||
"onlyAdmCanBeAssignedOwnerRole": "Only administrators can be assigned the owner role."
|
||||
},
|
||||
|
|
|
@ -75,8 +75,9 @@ describe('Authentication API', function () {
|
|||
done(new Error('Setup ran when it should not have.'));
|
||||
}).catch(function (err) {
|
||||
should.exist(err);
|
||||
err.name.should.equal('InternalServerError');
|
||||
err.statusCode.should.equal(500);
|
||||
err.name.should.equal('NotFoundError');
|
||||
err.message.should.equal('Owner not found');
|
||||
err.statusCode.should.equal(404);
|
||||
|
||||
done();
|
||||
}).catch(done);
|
||||
|
|
|
@ -1321,6 +1321,37 @@ describe('Users API', function () {
|
|||
done(new Error('Admin is not denied transferring ownership.'));
|
||||
}).catch(checkForErrorType('NoPermissionError', done));
|
||||
});
|
||||
|
||||
it('Blog is still setup', function (done) {
|
||||
// transfer ownership to admin user
|
||||
UserAPI.transferOwnership(
|
||||
{owner: [{id: userIdFor.admin}]},
|
||||
context.owner
|
||||
).then(function (response) {
|
||||
should.exist(response);
|
||||
return models.User.isSetup();
|
||||
}).then(function (isSetup) {
|
||||
isSetup.should.eql(true);
|
||||
done();
|
||||
}).catch(done);
|
||||
});
|
||||
|
||||
it('Blog is still setup, new owner is locked', function (done) {
|
||||
// transfer ownership to admin user
|
||||
UserAPI.transferOwnership(
|
||||
{owner: [{id: userIdFor.admin}]},
|
||||
context.owner
|
||||
).then(function (response) {
|
||||
should.exist(response);
|
||||
return models.User.edit({status: 'locked'}, {id: userIdFor.admin});
|
||||
}).then(function (modifiedUser) {
|
||||
modifiedUser.get('status').should.eql('locked');
|
||||
return models.User.isSetup();
|
||||
}).then(function (isSetup) {
|
||||
isSetup.should.eql(true);
|
||||
done();
|
||||
}).catch(done);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Change Password', function () {
|
||||
|
|
|
@ -238,7 +238,7 @@ fixtures = {
|
|||
}
|
||||
|
||||
return db.knex('users')
|
||||
.where('id', '=', models.User.ownerUser)
|
||||
.where('id', '=', DataGenerator.Content.users[0].id)
|
||||
.update(user);
|
||||
},
|
||||
|
||||
|
|
Loading…
Reference in a new issue