Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Fixes issue #311 Use email buffer for DEL ‘/email/:email’ route #315

Merged
merged 2 commits into from
Mar 10, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions db-server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ function createServer(db) {

api.get('/email/:email',
op(function (req) {
return db.getSecondaryEmail(Buffer(req.params.email, 'hex'))
return db.getSecondaryEmail(req.params.email)
})
)
api.get('/email/:email/account',
op(function (req) {
return db.accountRecord(Buffer(req.params.email, 'hex'))
return db.accountRecord(req.params.email)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed somehow, done now.

})
)
api.post('/email/:email/account/:id',
Expand Down
18 changes: 11 additions & 7 deletions db-server/test/backend/db_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const zeroBuffer16 = Buffer.from('00000000000000000000000000000000', 'hex')
const zeroBuffer32 = Buffer.from('0000000000000000000000000000000000000000000000000000000000000000', 'hex')
const now = Date.now()

function emailToHex(email) {
return Buffer.from(email).toString('hex')
}

function newUuid() {
return crypto.randomBytes(16)
}
Expand All @@ -35,7 +39,7 @@ function createAccount() {
locale: 'en_US',
}
account.normalizedEmail = account.email.toLowerCase()
account.emailBuffer = Buffer(account.email)
account.emailBuffer = Buffer.from(account.email)
return account
}

Expand Down Expand Up @@ -1399,7 +1403,7 @@ module.exports = function (config, DB) {
})

it('should get secondary email', () => {
return db.getSecondaryEmail(secondEmail.email)
return db.getSecondaryEmail(emailToHex(secondEmail.email))
.then((result) => {
assert.equal(result.email, secondEmail.email, 'matches secondEmail email')
assert.equal(!! result.isPrimary, false, 'isPrimary is false on secondEmail email')
Expand All @@ -1424,7 +1428,7 @@ module.exports = function (config, DB) {
})

it('should delete email', () => {
return db.deleteEmail(secondEmail.uid, secondEmail.email)
return db.deleteEmail(secondEmail.uid, emailToHex(secondEmail.email))
.then((result) => {
assert.deepEqual(result, {}, 'Returned an empty object on email deletion')

Expand Down Expand Up @@ -1482,7 +1486,7 @@ module.exports = function (config, DB) {
})

it('should fail to delete primary email', () => {
return db.deleteEmail(accountData.uid, accountData.normalizedEmail)
return db.deleteEmail(accountData.uid, emailToHex(accountData.normalizedEmail))
.then(assert.fail, (err) => {
assert.equal(err.errno, 136, 'should return email delete errno')
assert.equal(err.code, 400, 'should return email delete code')
Expand All @@ -1501,7 +1505,7 @@ module.exports = function (config, DB) {
})

it('should fail to get non-existent secondary email', () => {
return db.getSecondaryEmail('non-existent@email.com')
return db.getSecondaryEmail(emailToHex('non-existent@email.com'))
.then(assert.fail, (err) => {
assert.equal(err.errno, 116, 'should return not found errno')
assert.equal(err.code, 404, 'should return not found code')
Expand Down Expand Up @@ -1692,7 +1696,7 @@ module.exports = function (config, DB) {

it('should change a user\'s email', () => {
let sessionTokenData
return db.setPrimaryEmail(account.uid, secondEmail.email)
return db.setPrimaryEmail(account.uid, emailToHex(secondEmail.email))
.then(function (res) {
assert.deepEqual(res, {}, 'Returned an empty object on email change')
return db.accountEmails(account.uid)
Expand Down Expand Up @@ -1721,7 +1725,7 @@ module.exports = function (config, DB) {
assert.equal(session.email, secondEmail.email, 'should equal new primary email')
assert.deepEqual(session.emailCode, secondEmail.emailCode, 'should equal new primary emailCode')
assert.deepEqual(session.uid, account.uid, 'should equal account uid')
return P.all([db.accountRecord(secondEmail.email), db.accountRecord(account.email)])
return P.all([db.accountRecord(emailToHex(secondEmail.email)), db.accountRecord(emailToHex(account.email))])
})
.then((res) => {
assert.deepEqual(res[0], res[1], 'should return the same account record regardless of email used')
Expand Down
8 changes: 4 additions & 4 deletions db-server/test/backend/remote.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const P = require('../../../lib/promise')
const clientThen = require('../client-then')

function emailToHex(email) {
return Buffer(email).toString('hex')
return Buffer.from(email).toString('hex')
}

// Helper function that performs two tests:
Expand Down Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Ref: mozilla/fxa-auth-server#2334

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure i will complete it :)

})
.then(function (r) {
respOkEmpty(r)
Expand Down Expand Up @@ -228,7 +228,7 @@ module.exports = function(cfg, makeServer) {
return client.putThen('/account/' + user.accountId, user.account)
.then(function(r) {
respOkEmpty(r)
var randomPassword = Buffer(crypto.randomBytes(32)).toString('hex')
var randomPassword = Buffer.from(crypto.randomBytes(32)).toString('hex')
return client.postThen('/account/' + user.accountId + '/checkPassword', {'verifyHash': randomPassword})
.then(function(r) {
assert(false, 'should not be here, password isn\'t valid')
Expand Down Expand Up @@ -1180,7 +1180,7 @@ module.exports = function(cfg, makeServer) {
.then(
function (r) {
respOkEmpty(r)
return client.getThen('/emailBounces/' + Buffer(email).toString('hex'))
return client.getThen('/emailBounces/' + Buffer.from(email).toString('hex'))
}
)
.then(
Expand Down
2 changes: 1 addition & 1 deletion db-server/test/fake.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function base64_16() { return base64(16) }
function base64_65() { return base64(65) }

function buf(len) {
return Buffer(crypto.randomBytes(len))
return Buffer.from(crypto.randomBytes(len))
}
function buf16() { return buf(16) }
function buf32() { return buf(32) }
Expand Down
8 changes: 6 additions & 2 deletions lib/db/mem.js
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,8 @@ module.exports = function (log, error) {
return P.resolve({})
}

Memory.prototype.getSecondaryEmail = function (emailBuffer) {
Memory.prototype.getSecondaryEmail = function (email) {
const emailBuffer = Buffer.from(email, 'hex')
const normalizedEmail = emailBuffer.toString('utf8').toLowerCase()

if (emails[normalizedEmail]) {
Expand All @@ -1082,7 +1083,8 @@ module.exports = function (log, error) {
}
}

Memory.prototype.accountRecord = function (emailBuffer) {
Memory.prototype.accountRecord = function (email) {
const emailBuffer = Buffer.from(email, 'hex')
const normalizedEmail = emailBuffer.toString('utf8').toLowerCase()

if (! emails[normalizedEmail]) {
Expand Down Expand Up @@ -1123,6 +1125,7 @@ module.exports = function (log, error) {
}

Memory.prototype.setPrimaryEmail = function (uid, email) {
email = Buffer.from(email, 'hex').toString('utf8')
if (! emails[email]) {
return P.reject(error.notFound())
}
Expand All @@ -1140,6 +1143,7 @@ module.exports = function (log, error) {
}

Memory.prototype.deleteEmail = function (uid, email) {
email = Buffer.from(email, 'hex').toString('utf8')
var emailRecord = emails[email]

if (emailRecord && emailRecord.uid.toString('hex') === uid.toString('hex') && emailRecord.isPrimary === false) {
Expand Down
13 changes: 8 additions & 5 deletions lib/db/mysql.js
Original file line number Diff line number Diff line change
Expand Up @@ -806,15 +806,17 @@ module.exports = function (log, error) {
// Get : email
// Values : email = $1
var GET_SECONDARY_EMAIL = 'CALL getSecondaryEmail_1(?)'
MySql.prototype.getSecondaryEmail = function (emailBuffer) {
MySql.prototype.getSecondaryEmail = function (email) {
const emailBuffer = Buffer.from(email, 'hex')
return this.readFirstResult(GET_SECONDARY_EMAIL, [emailBuffer.toString('utf8')])
}

// Select : accounts
// Fields : uid, email, normalizedEmail, emailVerified, emailCode, kA, wrapWrapKb, verifierVersion, authSalt, verifierSetAt, createdAt, lockedAt, primaryEmail
// Where : emails.normalizedEmail = LOWER($1)
var GET_ACCOUNT_RECORD = 'CALL accountRecord_2(?)'
MySql.prototype.accountRecord = function (emailBuffer) {
MySql.prototype.accountRecord = function (email) {
const emailBuffer = Buffer.from(email, 'hex')
return this.readFirstResult(GET_ACCOUNT_RECORD, [emailBuffer.toString('utf8')])
}

Expand All @@ -834,11 +836,12 @@ module.exports = function (log, error) {
// Values : uid = $1, email = $2
var SET_PRIMARY_EMAIL = 'CALL setPrimaryEmail_1(?, ?)'
MySql.prototype.setPrimaryEmail = function (uid, email) {
const emailBuffer = Buffer.from(email, 'hex')
return this.write(
SET_PRIMARY_EMAIL,
[
uid,
email
emailBuffer.toString('utf8')
]
)
}
Expand All @@ -847,11 +850,12 @@ module.exports = function (log, error) {
// Values : uid = $1, email = $2
var DELETE_EMAIL = 'CALL deleteEmail_2(?, ?)'
MySql.prototype.deleteEmail = function (uid, email) {
const emailBuffer = Buffer.from(email, 'hex')
return this.write(
DELETE_EMAIL,
[
uid,
email
emailBuffer.toString('utf8')
]
)
.catch(function(err){
Expand Down Expand Up @@ -1321,4 +1325,3 @@ module.exports = function (log, error) {

return MySql
}

5 changes: 2 additions & 3 deletions scripts/populate-session-tokens.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ var DB = require('../lib/db/mysql')(log, require('../db-server').errors)
var config = require('../config')
var crypto = require('crypto')

var zeroBuffer16 = Buffer('00000000000000000000000000000000', 'hex')
var zeroBuffer32 = Buffer('0000000000000000000000000000000000000000000000000000000000000000', 'hex')
var zeroBuffer16 = Buffer.from('00000000000000000000000000000000', 'hex')
var zeroBuffer32 = Buffer.from('0000000000000000000000000000000000000000000000000000000000000000', 'hex')

var count = parseInt(process.argv[2])

Expand Down Expand Up @@ -127,4 +127,3 @@ if (count > 0) {
} else {
throw new Error('Invalid argument')
}

6 changes: 3 additions & 3 deletions test/local/metrics_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const crypto = require('crypto')
const proxyquire = require('proxyquire')
const sinon = require('sinon')

const zeroBuffer16 = Buffer('00000000000000000000000000000000', 'hex')
const zeroBuffer32 = Buffer('0000000000000000000000000000000000000000000000000000000000000000', 'hex')
const zeroBuffer16 = Buffer.from('00000000000000000000000000000000', 'hex')
const zeroBuffer32 = Buffer.from('0000000000000000000000000000000000000000000000000000000000000000', 'hex')

describe('DB metrics', () => {

Expand Down Expand Up @@ -344,7 +344,7 @@ describe('DB metrics', () => {
}

function hex (length) {
return Buffer(crypto.randomBytes(length).toString('hex'), 'hex')
return Buffer.from(crypto.randomBytes(length).toString('hex'), 'hex')
}

})