Fixes issue #311 Use email buffer for DEL ‘/email/:email’ route #315
Conversation
|
@vbudhram could you please do a quick glance at this (and maybe merge?)
|
Will take care :) |
There was a problem hiding this comment.
Looks great! I think the only blocking change here is to not pass a buffered email to the set primary email route. We should be able to fix mozilla/fxa-auth-server#2334 in follow up PR.
| }) | ||
| ) | ||
| api.get('/email/:email/account', | ||
| op(function (req) { | ||
| return db.accountRecord(Buffer(req.params.email, 'hex')) | ||
| return db.accountRecord(req.params.email) |
There was a problem hiding this comment.
You should be able to remove the Buffer from https://github.com/deeptibaghel/fxa-auth-db-mysql/blob/82073d71ad0187b55f7a3a6edf559f2345acbe8b/db-server/index.js#L113.
There was a problem hiding this comment.
Missed somehow, done now.
| @@ -192,7 +192,7 @@ module.exports = function(cfg, makeServer) { | |||
| assert.equal(!! result[2].isPrimary, false, 'isPrimary is false on thirdEmailRecord email') | |||
| assert.equal(!! result[2].isVerified, false, 'matches secondEmail thirdEmailRecord') | |||
|
|
|||
| return client.delThen('/account/' + user.accountId + '/emails/' + secondEmailRecord.email) | |||
| return client.delThen('/account/' + user.accountId + '/emails/' + emailToHex(secondEmailRecord.email)) | |||
There was a problem hiding this comment.
Perfect! But we will also need to update the auth-server to pass an encoded email, similar to https://github.com/mozilla/fxa-auth-server/blob/45ae7b2048e9115590611a55454627d3840119a8/lib/db.js#L1069.
There was a problem hiding this comment.
Yeah sure i will complete it :)
There was a problem hiding this comment.
@deeptibaghel Tested this against mozilla/fxa-auth-server#2336 and all ✅, r+!
ghost
removed
the
Made changes to fix issue #311 Use email buffer for DEL ‘/email/:email’ route.
Also fixed Buffer function call to take care of deprecated format.