-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Creates a new sessionToken when updating password #2266
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2257,12 +2257,14 @@ describe('Parse.User testing', () => { | |
}) | ||
}); | ||
|
||
it('should cleanup null authData keys ParseUser update (regression test for #1198)', (done) => { | ||
it_exclude_dbs(['postgres'])('should cleanup null authData keys ParseUser update (regression test for #1198, #2252)', (done) => { | ||
Parse.Cloud.beforeSave('_User', (req, res) => { | ||
req.object.set('foo', 'bar'); | ||
res.success(); | ||
}); | ||
|
||
|
||
let originalSessionToken; | ||
let originalUserId; | ||
// Simulate anonymous user save | ||
new Promise((resolve, reject) => { | ||
request.post({ | ||
|
@@ -2280,6 +2282,8 @@ describe('Parse.User testing', () => { | |
} | ||
}); | ||
}).then((user) => { | ||
originalSessionToken = user.sessionToken; | ||
originalUserId = user.objectId; | ||
// Simulate registration | ||
return new Promise((resolve, reject) => { | ||
request.put({ | ||
|
@@ -2291,7 +2295,7 @@ describe('Parse.User testing', () => { | |
}, | ||
json: { | ||
authData: {anonymous: null}, | ||
user: 'user', | ||
username: 'user', | ||
password: 'password', | ||
} | ||
}, (err, res, body) => { | ||
|
@@ -2305,8 +2309,84 @@ describe('Parse.User testing', () => { | |
}).then((user) => { | ||
expect(typeof user).toEqual('object'); | ||
expect(user.authData).toBeUndefined(); | ||
expect(user.sessionToken).not.toBeUndefined(); | ||
// Session token should have changed | ||
expect(user.sessionToken).not.toEqual(originalSessionToken); | ||
// test that the sessionToken is valid | ||
return new Promise((resolve, reject) => { | ||
request.get({ | ||
url: 'http://localhost:8378/1/users/me', | ||
headers: { | ||
'X-Parse-Application-Id': Parse.applicationId, | ||
'X-Parse-Session-Token': user.sessionToken, | ||
'X-Parse-REST-API-Key': 'rest', | ||
}, | ||
json: true | ||
}, (err, res, body) => { | ||
expect(body.username).toEqual(user.username); | ||
expect(body.objectId).toEqual(originalUserId); | ||
if (err) { | ||
reject(err); | ||
} else { | ||
resolve(body); | ||
} | ||
done(); | ||
}); | ||
}); | ||
}).catch((err) => { | ||
fail('no request should fail: ' + JSON.stringify(err)); | ||
done(); | ||
}); | ||
}); | ||
|
||
it_exclude_dbs(['postgres'])('should send email when upgrading from anon', (done) => { | ||
|
||
let emailCalled = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might find jasmine spies more convenient. This is fine here though. |
||
let emailOptions; | ||
var emailAdapter = { | ||
sendVerificationEmail: (options) => { | ||
emailOptions = options; | ||
emailCalled = true; | ||
}, | ||
sendPasswordResetEmail: () => Promise.resolve(), | ||
sendMail: () => Promise.resolve() | ||
} | ||
reconfigureServer({ | ||
appName: 'unused', | ||
verifyUserEmails: true, | ||
emailAdapter: emailAdapter, | ||
publicServerURL: "http://localhost:8378/1" | ||
}) | ||
// Simulate anonymous user save | ||
return rp.post({ | ||
url: 'http://localhost:8378/1/classes/_User', | ||
headers: { | ||
'X-Parse-Application-Id': Parse.applicationId, | ||
'X-Parse-REST-API-Key': 'rest', | ||
}, | ||
json: {authData: {anonymous: {id: '00000000-0000-0000-0000-000000000001'}}} | ||
}).then((user) => { | ||
return rp.put({ | ||
url: 'http://localhost:8378/1/classes/_User/' + user.objectId, | ||
headers: { | ||
'X-Parse-Application-Id': Parse.applicationId, | ||
'X-Parse-Session-Token': user.sessionToken, | ||
'X-Parse-REST-API-Key': 'rest', | ||
}, | ||
json: { | ||
authData: {anonymous: null}, | ||
username: 'user', | ||
email: 'user@email.com', | ||
password: 'password', | ||
} | ||
}); | ||
}).then(() => { | ||
expect(emailCalled).toBe(true); | ||
expect(emailOptions).not.toBeUndefined(); | ||
expect(emailOptions.user.get('email')).toEqual('user@email.com'); | ||
done(); | ||
}).catch((err) => { | ||
console.error(err); | ||
fail('no request should fail: ' + JSON.stringify(err)); | ||
done(); | ||
}); | ||
|
@@ -2471,9 +2551,16 @@ describe('Parse.User testing', () => { | |
user.set('password', 'password'); | ||
return user.save() | ||
}) | ||
.then(() => { | ||
// Session token should have been recycled | ||
expect(body.sessionToken).not.toEqual(user.getSessionToken()); | ||
}) | ||
.then(() => obj.fetch()) | ||
.then((res) => { | ||
done(); | ||
}) | ||
.catch(error => { | ||
expect(error.code).toEqual(Parse.Error.INVALID_SESSION_TOKEN); | ||
fail('should not fail') | ||
done(); | ||
}); | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -367,6 +367,7 @@ RestWrite.prototype.transformUser = function() { | |
} | ||
if (this.query && !this.auth.isMaster ) { | ||
this.storage['clearSessions'] = true; | ||
this.storage['generateNewSession'] = true; | ||
} | ||
return passwordCrypto.hash(this.data.password).then((hashedPassword) => { | ||
this.data._hashed_password = hashedPassword; | ||
|
@@ -428,6 +429,10 @@ RestWrite.prototype.createSessionTokenIfNeeded = function() { | |
if (this.query) { | ||
return; | ||
} | ||
return this.createSessionToken(); | ||
} | ||
|
||
RestWrite.prototype.createSessionToken = function() { | ||
var token = 'r:' + cryptoUtils.newToken(); | ||
|
||
var expiresAt = this.config.generateSessionExpiresAt(); | ||
|
@@ -464,15 +469,21 @@ RestWrite.prototype.handleFollowup = function() { | |
} | ||
}; | ||
delete this.storage['clearSessions']; | ||
this.config.database.destroy('_Session', sessionQuery) | ||
return this.config.database.destroy('_Session', sessionQuery) | ||
.then(this.handleFollowup.bind(this)); | ||
} | ||
|
||
if (this.storage && this.storage['generateNewSession']) { | ||
delete this.storage['generateNewSession']; | ||
return this.createSessionToken() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this will be buggy if someone changes email address and upgrade from anon to non-anon in the same request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'll check it out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That seem to work correctly as is, as we call the I needed to refactor as synchronous methods so destroy don't race with generation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I see it's a recursive call. Interesting. Yeah, this should work then. |
||
.then(this.handleFollowup.bind(this)); | ||
} | ||
|
||
if (this.storage && this.storage['sendVerificationEmail']) { | ||
delete this.storage['sendVerificationEmail']; | ||
// Fire and forget! | ||
this.config.userController.sendVerificationEmail(this.data); | ||
this.handleFollowup.bind(this); | ||
return this.handleFollowup.bind(this); | ||
} | ||
}; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have
request-promise
now if you want to avoid some boilerplate in later PRs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's helpful. I'll update