Skip to content

Commit 2adf170

Browse files
Fixed password resets to run in a transaction (TryGhost#30047)
no ref Co-authored-by: UserExistsError <23325451+UserExistsError@users.noreply.github.com>
1 parent 5ef16ab commit 2adf170

2 files changed

Lines changed: 48 additions & 52 deletions

File tree

‎ghost/core/core/server/models/user.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1091,7 +1091,7 @@ User = ghostBookshelf.Model.extend({
10911091
});
10921092
}
10931093

1094-
const updatedUser = await user.save({password: newPassword});
1094+
const updatedUser = await user.save({password: newPassword}, options);
10951095

10961096
// Destroy every active session for this user. The caller must mint a
10971097
// fresh session (with a new session_id) for self password-changes so

‎ghost/core/core/server/services/auth/passwordreset.js‎

Lines changed: 47 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const tpl = require('@tryghost/tpl');
55
const moment = require('moment');
66

77
const models = require('../../models');
8+
const ghostBookshelf = require('../../models/base');
89
const urlUtils = require('../../../shared/url-utils').default;
910
const mail = require('../mail');
1011

@@ -77,67 +78,62 @@ function extractTokenParts(options) {
7778
}
7879

7980
function doReset(options, tokenParts, settingsAPI) {
80-
let dbHash;
81-
8281
const data = options.data.password_reset[0];
8382
const resetToken = data.token;
8483
const oldPassword = data.oldPassword;
8584
const newPassword = data.newPassword;
8685

87-
return settingsAPI.read(_.merge({key: 'db_hash'}, _.omit(options, 'data')))
88-
.then((response) => {
89-
dbHash = response.settings[0].value;
86+
return ghostBookshelf.transaction(async (transacting) => {
87+
const txOptions = {context: options.context, transacting};
9088

91-
return models.User.getByEmail(tokenParts.email, options);
92-
})
93-
.then((user) => {
94-
if (!user) {
95-
throw new errors.NotFoundError({message: tpl(messages.userNotFound)});
96-
}
89+
const settingsResponse = await settingsAPI.read({key: 'db_hash', ...txOptions});
90+
const dbHash = settingsResponse.settings[0].value;
9791

98-
let compareResult = security.tokens.resetToken.compare({
99-
token: resetToken,
100-
dbHash: dbHash,
101-
password: user.get('password')
102-
});
92+
const candidate = await models.User.getByEmail(tokenParts.email, txOptions);
93+
if (!candidate) {
94+
throw new errors.NotFoundError({message: tpl(messages.userNotFound)});
95+
}
10396

104-
if (!compareResult.correct) {
105-
let error;
106-
if (compareResult.reason === 'expired' || compareResult.reason === 'invalid_expiry') {
107-
error = new errors.BadRequestError({
108-
message: tpl(messages.expired.message),
109-
context: tpl(messages.expired.context),
110-
help: tpl(messages.expired.help)
111-
});
112-
} else {
113-
error = new errors.BadRequestError({
114-
message: tpl(messages.invalidToken.message),
115-
context: tpl(messages.invalidToken.context),
116-
help: tpl(messages.invalidToken.help)
117-
});
118-
}
119-
120-
return Promise.reject(error);
121-
}
97+
const user = await new models.User({id: candidate.id})
98+
.fetch({...txOptions, forUpdate: true, require: true});
12299

123-
return models.User.changePassword({
124-
oldPassword: oldPassword,
125-
newPassword: newPassword,
126-
user_id: user.id
127-
}, options);
128-
})
129-
.then((updatedUser) => {
130-
updatedUser.set('status', 'active');
131-
return updatedUser.save(options);
132-
}).then((savedUser) => {
133-
return {user: savedUser};
134-
})
135-
.catch((err) => {
136-
if (errors.utils.isGhostError(err)) {
137-
return Promise.reject(err);
138-
}
139-
return Promise.reject(new errors.UnauthorizedError({err: err}));
100+
let compareResult = security.tokens.resetToken.compare({
101+
token: resetToken,
102+
dbHash: dbHash,
103+
password: user.get('password')
140104
});
105+
106+
if (!compareResult.correct) {
107+
if (compareResult.reason === 'expired' || compareResult.reason === 'invalid_expiry') {
108+
throw new errors.BadRequestError({
109+
message: tpl(messages.expired.message),
110+
context: tpl(messages.expired.context),
111+
help: tpl(messages.expired.help)
112+
});
113+
} else {
114+
throw new errors.BadRequestError({
115+
message: tpl(messages.invalidToken.message),
116+
context: tpl(messages.invalidToken.context),
117+
help: tpl(messages.invalidToken.help)
118+
});
119+
}
120+
}
121+
122+
const updatedUser = await models.User.changePassword({
123+
oldPassword: oldPassword,
124+
newPassword: newPassword,
125+
user_id: user.id
126+
}, txOptions);
127+
128+
updatedUser.set('status', 'active');
129+
const savedUser = await updatedUser.save(null, txOptions);
130+
return {user: savedUser};
131+
}).catch((err) => {
132+
if (errors.utils.isGhostError(err)) {
133+
return Promise.reject(err);
134+
}
135+
return Promise.reject(new errors.UnauthorizedError({err: err}));
136+
});
141137
}
142138

143139
async function sendResetNotification(data, mailAPI) {

0 commit comments

Comments
 (0)