From d4836af18adcd223a65b1d6ea26761dfaf14dbeb Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Tue, 4 Apr 2017 15:56:04 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20=20fix=20owner=20user=20slug=20(?= =?UTF-8?q?#8263)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #8067 - this is only a bug present for remote authentication - right now the remote service does not return the name of the user - depends on an internal PR - force regenerating the slug on setup - override name for signin or invite if needed --- core/server/auth/auth-strategies.js | 7 ++- core/test/unit/auth/auth-strategies_spec.js | 66 ++++++++++++--------- 2 files changed, 43 insertions(+), 30 deletions(-) diff --git a/core/server/auth/auth-strategies.js b/core/server/auth/auth-strategies.js index 468b3682f5..9e61e5b48d 100644 --- a/core/server/auth/auth-strategies.js +++ b/core/server/auth/auth-strategies.js @@ -115,12 +115,11 @@ strategies = { return models.User.add({ email: profile.email, - name: profile.email, + name: profile.name, password: utils.uid(50), roles: [invite.toJSON().role_id], ghost_auth_id: profile.id, ghost_auth_access_token: ghostAuthAccessToken - }, options); }) .then(function destroyInvite(_user) { @@ -141,8 +140,11 @@ strategies = { }); } + // 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 @@ -169,6 +171,7 @@ strategies = { 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)); diff --git a/core/test/unit/auth/auth-strategies_spec.js b/core/test/unit/auth/auth-strategies_spec.js index 73d63b5c55..68f68c320a 100644 --- a/core/test/unit/auth/auth-strategies_spec.js +++ b/core/test/unit/auth/auth-strategies_spec.js @@ -226,13 +226,13 @@ describe('Auth Strategies', function () { }); describe('Ghost Strategy', function () { - var inviteStub, userAddStub, userEditStub, userFindOneStub; + var inviteFindOneStub, userAddStub, userEditStub, userFindOneStub; beforeEach(function () { userFindOneStub = sandbox.stub(Models.User, 'findOne'); userAddStub = sandbox.stub(Models.User, 'add'); userEditStub = sandbox.stub(Models.User, 'edit'); - inviteStub = sandbox.stub(Models.Invite, 'findOne'); + inviteFindOneStub = sandbox.stub(Models.Invite, 'findOne'); }); it('with invite, but with wrong invite token', function (done) { @@ -241,13 +241,13 @@ describe('Auth Strategies', function () { profile = {email: 'test@example.com', id: '1234'}; userFindOneStub.returns(Promise.resolve(null)); - inviteStub.returns(Promise.reject(new errors.NotFoundError())); + inviteFindOneStub.returns(Promise.reject(new errors.NotFoundError())); authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, profile, function (err) { should.exist(err); (err instanceof errors.NotFoundError).should.eql(true); userFindOneStub.calledOnce.should.be.false(); - inviteStub.calledOnce.should.be.true(); + inviteFindOneStub.calledOnce.should.be.true(); done(); }); }); @@ -258,7 +258,7 @@ describe('Auth Strategies', function () { profile = {email: 'test@example.com', id: '1234'}; userFindOneStub.returns(Promise.resolve(null)); - inviteStub.returns(Promise.resolve(Models.Invite.forge({ + inviteFindOneStub.returns(Promise.resolve(Models.Invite.forge({ id: 1, token: 'token', expires: Date.now() - 1000 @@ -268,7 +268,7 @@ describe('Auth Strategies', function () { should.exist(err); (err instanceof errors.NotFoundError).should.eql(true); userFindOneStub.calledOnce.should.be.false(); - inviteStub.calledOnce.should.be.true(); + inviteFindOneStub.calledOnce.should.be.true(); done(); }); }); @@ -276,18 +276,32 @@ describe('Auth Strategies', function () { it('with correct invite token', function (done) { var ghostAuthAccessToken = '12345', req = {body: {inviteToken: 'token'}}, - invitedProfile = {email: 'test@example.com', id: '1234'}, + invitedProfile = {email: 'test@example.com', name: 'Wolfram Alpha', id: '1234'}, invitedUser = {id: 2}, inviteModel = Models.Invite.forge({ id: 1, token: 'token', - expires: Date.now() + 1000 + expires: Date.now() + 2000, + role_id: '2' }); + sandbox.stub(globalUtils, 'uid').returns('12345678'); + userFindOneStub.returns(Promise.resolve(null)); - userAddStub.returns(Promise.resolve(invitedUser)); + + 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)); - inviteStub.returns(Promise.resolve(inviteModel)); + inviteFindOneStub.returns(Promise.resolve(inviteModel)); sandbox.stub(inviteModel, 'destroy').returns(Promise.resolve()); authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, invitedProfile, function (err, user, profile) { @@ -297,8 +311,9 @@ describe('Auth Strategies', function () { user.should.eql(invitedUser); profile.should.eql(invitedProfile); + userAddStub.calledOnce.should.be.true(); userFindOneStub.calledOnce.should.be.false(); - inviteStub.calledOnce.should.be.true(); + inviteFindOneStub.calledOnce.should.be.true(); done(); }); }); @@ -306,7 +321,7 @@ describe('Auth Strategies', function () { it('setup', function (done) { var ghostAuthAccessToken = '12345', req = {body: {}}, - ownerProfile = {email: 'test@example.com', id: '1234'}, + ownerProfile = {email: 'test@example.com', name: 'Wolfram Alpha', id: '1234'}, owner = {id: 2}; userFindOneStub.withArgs({ghost_auth_id: ownerProfile.id}) @@ -316,8 +331,10 @@ describe('Auth Strategies', function () { .returns(Promise.resolve(_.merge({}, {status: 'inactive'}, owner))); userEditStub.withArgs({ + email: ownerProfile.email, + name: ownerProfile.name, + slug: null, status: 'active', - email: 'test@example.com', ghost_auth_id: ownerProfile.id, ghost_auth_access_token: ghostAuthAccessToken }, { @@ -325,19 +342,11 @@ describe('Auth Strategies', function () { id: owner.id }).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.not.exist(err); userFindOneStub.calledTwice.should.be.true(); - inviteStub.calledOnce.should.be.false(); + inviteFindOneStub.calledOnce.should.be.false(); + userEditStub.calledOnce.should.be.true(); should.exist(user); should.exist(profile); @@ -350,7 +359,7 @@ describe('Auth Strategies', function () { it('sign in', function (done) { var ghostAuthAccessToken = '12345', req = {body: {}}, - ownerProfile = {email: 'test@example.com', id: '12345'}, + ownerProfile = {email: 'test@example.com', name: 'Wolfram Alpha', id: '12345'}, owner = { id: 2, isActive: function () { return true; @@ -359,9 +368,10 @@ describe('Auth Strategies', function () { userFindOneStub.returns(Promise.resolve(owner)); userEditStub.withArgs({ + email: ownerProfile.email, + name: ownerProfile.name, ghost_auth_access_token: ghostAuthAccessToken, - ghost_auth_id: ownerProfile.id, - email: ownerProfile.email + ghost_auth_id: ownerProfile.id }, { context: {internal: true}, id: owner.id @@ -371,7 +381,7 @@ describe('Auth Strategies', function () { should.not.exist(err); userFindOneStub.calledOnce.should.be.true(); userEditStub.calledOnce.should.be.true(); - inviteStub.calledOnce.should.be.false(); + inviteFindOneStub.calledOnce.should.be.false(); should.exist(user); should.exist(profile); @@ -407,7 +417,7 @@ describe('Auth Strategies', function () { userFindOneStub.calledOnce.should.be.true(); userEditStub.calledOnce.should.be.false(); - inviteStub.calledOnce.should.be.false(); + inviteFindOneStub.calledOnce.should.be.false(); should.not.exist(user); should.not.exist(profile);