🔥 Dropped unused `ghost_auth_*` user fields

no issue

- Drops `ghost_auth_access_token` and `ghost_auth_id` fields since not used anymore
- Adds migration for dropping these columns from users table
- Drops Auth strategy - `ghostStrategy` - since its not used anymore
This commit is contained in:
Rishabh Garg 2019-09-03 20:48:42 +05:30 committed by GitHub
parent 303046bc0a
commit b875cc339d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 82 additions and 369 deletions

View File

@ -68,7 +68,6 @@ const author = (attrs, frame) => {
// @NOTE: unused fields
delete attrs.visibility;
delete attrs.locale;
delete attrs.ghost_auth_id;
return attrs;
};

View File

@ -68,7 +68,6 @@ const author = (attrs, frame) => {
// @NOTE: unused fields
delete attrs.visibility;
delete attrs.locale;
delete attrs.ghost_auth_id;
return attrs;
};

View File

@ -0,0 +1,78 @@
const common = require('../../../../lib/common');
const commands = require('../../../schema').commands;
const createLog = type => msg => common.logging[type](msg);
function createColumnMigrations(migrations) {
return function columnMigrations({transacting}) {
return Promise.each(migrations, function ({table, column, dbIsInCorrectState, operation, operationVerb, columnDefinition}) {
return transacting.schema.hasColumn(table, column)
.then(dbIsInCorrectState)
.then((isInCorrectState) => {
const log = createLog(isInCorrectState ? 'warn' : 'info');
log(`${operationVerb} ${table}.${column}`);
if (!isInCorrectState) {
return operation(table, column, transacting, columnDefinition);
}
});
});
};
}
module.exports.up = createColumnMigrations([
{
table: 'users',
column: 'ghost_auth_access_token',
dbIsInCorrectState(columnExists) {
return columnExists === false;
},
operation: commands.dropColumn,
operationVerb: 'Removing'
},
{
table: 'users',
column: 'ghost_auth_id',
dbIsInCorrectState(columnExists) {
return columnExists === false;
},
operation: commands.dropColumn,
operationVerb: 'Removing'
}
]);
module.exports.down = createColumnMigrations([
{
table: 'users',
column: 'ghost_auth_access_token',
dbIsInCorrectState(columnExists) {
return columnExists === true;
},
operation: commands.addColumn,
operationVerb: 'Adding',
columnDefinition: {
type: 'string',
nullable: true,
maxlength: 32
}
},
{
table: 'users',
column: 'ghost_auth_id',
dbIsInCorrectState(columnExists) {
return columnExists === true;
},
operation: commands.addColumn,
operationVerb: 'Adding',
columnDefinition: {
type: 'string',
nullable: true,
maxlength: 24
}
}
]);
module.exports.config = {
transaction: true
};

View File

@ -63,8 +63,6 @@ module.exports = {
id: {type: 'string', maxlength: 24, nullable: false, primary: true},
name: {type: 'string', maxlength: 191, nullable: false},
slug: {type: 'string', maxlength: 191, nullable: false, unique: true},
ghost_auth_access_token: {type: 'string', maxlength: 32, nullable: true},
ghost_auth_id: {type: 'string', maxlength: 24, nullable: true},
password: {type: 'string', maxlength: 60, nullable: false},
email: {type: 'string', maxlength: 191, nullable: false, unique: true, validations: {isEmail: true}},
profile_image: {type: 'string', maxlength: 2000, nullable: true},

View File

@ -215,7 +215,6 @@ User = ghostBookshelf.Model.extend({
// remove password hash for security reasons
delete attrs.password;
delete attrs.ghost_auth_access_token;
// NOTE: We don't expose the email address for for external, app and public context.
// @TODO: Why? External+Public is actually the same context? Was also mentioned here https://github.com/TryGhost/Ghost/issues/9043
@ -233,7 +232,6 @@ User = ghostBookshelf.Model.extend({
delete attrs.updated_by;
delete attrs.last_seen;
delete attrs.status;
delete attrs.ghost_auth_id;
}
return attrs;

View File

@ -1,8 +1,6 @@
var _ = require('lodash'),
models = require('../../models'),
common = require('../../lib/common'),
security = require('../../lib/security'),
strategies;
const models = require('../../models');
const common = require('../../lib/common');
let strategies;
strategies = {
@ -69,144 +67,6 @@ strategies = {
return done(null, false);
}
});
},
/**
* Ghost Strategy
* ghostAuthRefreshToken: will be null for now, because we don't need it right now
*
* CASES:
* - via invite token
* - via normal sign in
* - via setup
*/
ghostStrategy: function ghostStrategy(req, ghostAuthAccessToken, ghostAuthRefreshToken, profile, done) {
var inviteToken = req.body.inviteToken,
options = {context: {internal: true}},
handleInviteToken, handleSetup, handleSignIn;
// CASE: socket hangs up for example
if (!ghostAuthAccessToken || !profile) {
return done(new common.errors.NoPermissionError({
help: 'Please try again.'
}));
}
handleInviteToken = function handleInviteToken() {
var user, invite;
inviteToken = security.url.decodeBase64(inviteToken);
return models.Invite.findOne({token: inviteToken}, options)
.then(function addInviteUser(_invite) {
invite = _invite;
if (!invite) {
throw new common.errors.NotFoundError({
message: common.i18n.t('errors.api.invites.inviteNotFound')
});
}
if (invite.get('expires') < Date.now()) {
throw new common.errors.NotFoundError({
message: common.i18n.t('errors.api.invites.inviteExpired')
});
}
return models.User.add({
email: profile.email,
name: profile.name,
password: security.identifier.uid(50),
roles: [invite.toJSON().role_id],
ghost_auth_id: profile.id,
ghost_auth_access_token: ghostAuthAccessToken
}, options);
})
.then(function destroyInvite(_user) {
user = _user;
return invite.destroy(options);
})
.then(function () {
return user;
});
};
handleSetup = function handleSetup() {
return models.User.findOne({slug: 'ghost-owner', status: 'inactive'}, options)
.then(function fetchedOwner(owner) {
if (!owner) {
throw new common.errors.NotFoundError({
message: common.i18n.t('errors.models.user.userNotFound')
});
}
// CASE: slug null forces regenerating the slug (ghost-owner is default and needs to be overridden)
return models.User.edit({
email: profile.email,
name: profile.name,
slug: null,
status: 'active',
ghost_auth_id: profile.id,
ghost_auth_access_token: ghostAuthAccessToken
}, _.merge({id: owner.id}, options));
});
};
handleSignIn = function handleSignIn() {
var user;
return models.User.findOne({ghost_auth_id: profile.id}, options)
.then(function (_user) {
user = _user;
if (!user) {
throw new common.errors.NotFoundError();
}
if (!user.isActive()) {
throw new common.errors.NoPermissionError({
message: common.i18n.t('errors.models.user.accountSuspended')
});
}
return models.User.edit({
email: profile.email,
name: profile.name,
ghost_auth_id: profile.id,
ghost_auth_access_token: ghostAuthAccessToken
}, _.merge({id: user.id}, options));
})
.then(function () {
return user;
});
};
if (inviteToken) {
return handleInviteToken()
.then(function (user) {
done(null, user, profile);
})
.catch(function (err) {
done(err);
});
}
handleSignIn()
.then(function (user) {
done(null, user, profile);
})
.catch(function (err) {
if (!(err instanceof common.errors.NotFoundError)) {
return done(err);
}
handleSetup()
.then(function (user) {
done(null, user, profile);
})
.catch(function (err) {
done(err);
});
});
}
};

View File

@ -59,8 +59,6 @@ const expectedProperties = {
.without('visibility')
.without('password')
.without('locale')
.without('ghost_auth_access_token')
.without('ghost_auth_id')
.concat('url')
,
tag: _(schema.tags)

View File

@ -33,8 +33,6 @@ const expectedProperties = {
.without(
'password',
'email',
'ghost_auth_access_token',
'ghost_auth_id',
'created_at',
'created_by',
'updated_at',

View File

@ -37,8 +37,6 @@ const expectedProperties = {
.without('visibility')
.without('password')
.without('locale')
.without('ghost_auth_access_token')
.without('ghost_auth_id')
.concat('url')
,
tag: _(schema.tags)

View File

@ -33,8 +33,6 @@ const expectedProperties = {
.without(
'password',
'email',
'ghost_auth_access_token',
'ghost_auth_id',
'created_at',
'created_by',
'updated_at',

View File

@ -37,8 +37,6 @@ const expectedProperties = {
.without('visibility')
.without('password')
.without('locale')
.without('ghost_auth_access_token')
.without('ghost_auth_id')
.concat('url')
,
tag: _(schema.tags)

View File

@ -33,8 +33,6 @@ const expectedProperties = {
.without(
'password',
'email',
'ghost_auth_access_token',
'ghost_auth_id',
'created_at',
'created_by',
'updated_at',

View File

@ -19,7 +19,7 @@ var should = require('should'),
*/
describe('DB version integrity', function () {
// Only these variables should need updating
const currentSchemaHash = 'fda0398e93a74b2dc435cb4c026679ba';
const currentSchemaHash = '2b2cf2575dcf3fc86136b0dd4fba3263';
const currentFixturesHash = 'c7b485fe2f16517295bd35c761129729';
// If this test is failing, then it is likely a change has been made that requires a DB version bump,

View File

@ -223,205 +223,4 @@ describe('Auth Strategies', function () {
}).catch(done);
});
});
describe('Ghost Strategy', function () {
var inviteFindOneStub, userAddStub, userEditStub, userFindOneStub;
beforeEach(function () {
userFindOneStub = sinon.stub(Models.User, 'findOne');
userAddStub = sinon.stub(Models.User, 'add');
userEditStub = sinon.stub(Models.User, 'edit');
inviteFindOneStub = sinon.stub(Models.Invite, 'findOne');
});
it('with invite, but with wrong invite token', function (done) {
var ghostAuthAccessToken = '12345',
req = {body: {inviteToken: 'wrong'}},
profile = {email: 'test@example.com', id: '1234'};
userFindOneStub.returns(Promise.resolve(null));
inviteFindOneStub.returns(Promise.reject(new common.errors.NotFoundError()));
authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, profile, function (err) {
should.exist(err);
(err instanceof common.errors.NotFoundError).should.eql(true);
userFindOneStub.calledOnce.should.be.false();
inviteFindOneStub.calledOnce.should.be.true();
done();
});
});
it('with correct invite token, but expired', function (done) {
var ghostAuthAccessToken = '12345',
req = {body: {inviteToken: 'token'}},
profile = {email: 'test@example.com', id: '1234'};
userFindOneStub.returns(Promise.resolve(null));
inviteFindOneStub.returns(Promise.resolve(Models.Invite.forge({
id: 1,
token: 'token',
expires: Date.now() - 1000
})));
authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, profile, function (err) {
should.exist(err);
(err instanceof common.errors.NotFoundError).should.eql(true);
userFindOneStub.calledOnce.should.be.false();
inviteFindOneStub.calledOnce.should.be.true();
done();
});
});
it('with correct invite token', function (done) {
var ghostAuthAccessToken = '12345',
req = {body: {inviteToken: 'token'}},
invitedProfile = {email: 'test@example.com', name: 'Wolfram Alpha', id: '1234'},
invitedUser = {id: 2},
inviteModel = Models.Invite.forge({
id: 1,
token: 'token',
expires: Date.now() + 2000,
role_id: '2'
});
sinon.stub(security.identifier, 'uid').returns('12345678');
userFindOneStub.returns(Promise.resolve(null));
userAddStub.withArgs({
email: invitedProfile.email,
name: invitedProfile.name,
password: '12345678',
roles: [inviteModel.get('role_id')],
ghost_auth_id: invitedProfile.id,
ghost_auth_access_token: ghostAuthAccessToken
}, {
context: {internal: true}
}).returns(Promise.resolve(invitedUser));
userEditStub.returns(Promise.resolve(invitedUser));
inviteFindOneStub.returns(Promise.resolve(inviteModel));
sinon.stub(inviteModel, 'destroy').returns(Promise.resolve());
authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, invitedProfile, function (err, user, profile) {
should.not.exist(err);
should.exist(user);
should.exist(profile);
user.should.eql(invitedUser);
profile.should.eql(invitedProfile);
userAddStub.calledOnce.should.be.true();
userFindOneStub.calledOnce.should.be.false();
inviteFindOneStub.calledOnce.should.be.true();
done();
});
});
it('setup', function (done) {
var ghostAuthAccessToken = '12345',
req = {body: {}},
ownerProfile = {email: 'test@example.com', name: 'Wolfram Alpha', id: '1234'},
owner = {id: 2};
userFindOneStub.withArgs({ghost_auth_id: ownerProfile.id})
.returns(Promise.resolve(null));
userFindOneStub.withArgs({slug: 'ghost-owner', status: 'inactive'})
.returns(Promise.resolve(_.merge({}, {status: 'inactive'}, owner)));
userEditStub.withArgs({
email: ownerProfile.email,
name: ownerProfile.name,
slug: null,
status: 'active',
ghost_auth_id: ownerProfile.id,
ghost_auth_access_token: ghostAuthAccessToken
}, {
context: {internal: true},
id: owner.id
}).returns(Promise.resolve(owner));
authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, ownerProfile, function (err, user, profile) {
should.not.exist(err);
userFindOneStub.calledTwice.should.be.true();
inviteFindOneStub.calledOnce.should.be.false();
userEditStub.calledOnce.should.be.true();
should.exist(user);
should.exist(profile);
user.should.eql(owner);
profile.should.eql(ownerProfile);
done();
});
});
it('sign in', function (done) {
var ghostAuthAccessToken = '12345',
req = {body: {}},
ownerProfile = {email: 'test@example.com', name: 'Wolfram Alpha', id: '12345'},
owner = {
id: 2, isActive: function () {
return true;
}
};
userFindOneStub.returns(Promise.resolve(owner));
userEditStub.withArgs({
email: ownerProfile.email,
name: ownerProfile.name,
ghost_auth_access_token: ghostAuthAccessToken,
ghost_auth_id: ownerProfile.id
}, {
context: {internal: true},
id: owner.id
}).returns(Promise.resolve(owner));
authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, ownerProfile, function (err, user, profile) {
should.not.exist(err);
userFindOneStub.calledOnce.should.be.true();
userEditStub.calledOnce.should.be.true();
inviteFindOneStub.calledOnce.should.be.false();
should.exist(user);
should.exist(profile);
user.should.eql(owner);
profile.should.eql(ownerProfile);
done();
});
});
it('sign in, but user is suspended', function (done) {
var ghostAuthAccessToken = '12345',
req = {body: {}},
ownerProfile = {email: 'test@example.com', id: '12345'},
owner = {
id: 2, isActive: function () {
return false;
}
};
userFindOneStub.returns(Promise.resolve(owner));
userEditStub.withArgs({
ghost_auth_access_token: ghostAuthAccessToken,
ghost_auth_id: ownerProfile.id,
email: ownerProfile.email
}, {
context: {internal: true},
id: owner.id
}).returns(Promise.resolve(owner));
authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, ownerProfile, function (err, user, profile) {
should.exist(err);
err.message.should.eql('Your account was suspended.');
userFindOneStub.calledOnce.should.be.true();
userEditStub.calledOnce.should.be.false();
inviteFindOneStub.calledOnce.should.be.false();
should.not.exist(user);
should.not.exist(profile);
done();
});
});
});
});

View File

@ -2583,8 +2583,6 @@
"id": "1",
"name": "Joe Bloggs",
"slug": "joe-bloggs",
"ghost_auth_access_token": null,
"ghost_auth_id": null,
"password": "$2a$10$lYwP44K1nXJdvsK6FXXD1uAALyCLrgV7pHSgTyqT7mw.sTQpklFIm",
"email": "jbloggs@example.com",
"profile_image": "https://example.com/super_photo.jpg",
@ -2611,8 +2609,6 @@
"id": "5951f5fca366002ebd5dbef7",
"name": "Ghost",
"slug": "ghost",
"ghost_auth_access_token": null,
"ghost_auth_id": null,
"password": "$2a$10$F.k1xMyuCpkbxHDi2a7dvu2fB02pP4OcYJSkEO9WnPocJiTeHE.6O",
"email": "ghost-author@example.com",
"profile_image": "https://static.ghost.org/v2.0.0/images/ghost.png",

View File

@ -1511,8 +1511,6 @@
"id": "5951f5fca366002ebd5dbef7",
"name": "Joe Blogg's Brother",
"slug": "joe-bloggs-brother",
"ghost_auth_access_token": null,
"ghost_auth_id": null,
"password": "$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKABC",
"email": "jbloggsbrother@example.com",
"profile_image": "/content/images/2017/05/authorlogo.jpeg",