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

Commit

Permalink
Merge pull request #2987 from mozilla/pb/redundant-query-params
Browse files Browse the repository at this point in the history 
#2987
r=shane-tomlinson
  • Loading branch information
@philbooth
philbooth authored Mar 25, 2019
2 parents f9108bb + b8886b0 commit 6956f1d
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 62 deletions.
20 changes: 0 additions & 20 deletions docs/api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1749,26 +1749,6 @@ The link can be clicked from any browser,
not just the one being attached to the Firefox account.
<!--end-route-post-recovery_emailverify_code-->

##### Query parameters

* `service`: *validators.service*

<!--begin-query-param-post-recovery_emailverify_code-service-->
Opaque alphanumeric token to be included in verification links.
<!--end-query-param-post-recovery_emailverify_code-service-->

* `reminder`: *string, max(32), alphanum, optional*

<!--begin-query-param-post-recovery_emailverify_code-reminder-->
Deprecated.
<!--end-query-param-post-recovery_emailverify_code-reminder-->

* `type`: *string, max(32), alphanum, optional*

<!--begin-query-param-post-recovery_emailverify_code-type-->
The type of code being verified.
<!--end-query-param-post-recovery_emailverify_code-type-->

##### Request body

* `uid`: *string, max(32), regex(HEX_STRING), required*
Expand Down
52 changes: 10 additions & 42 deletions lib/routes/emails.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,12 +267,6 @@ module.exports = (log, db, mailer, config, customs, push) => {
path: '/recovery_email/verify_code',
options: {
validate: {
query: {
service: validators.service,
// TODO: drop this param once it is no longer sent by clients
reminder: isA.string().max(32).alphanum().optional(),
type: isA.string().max(32).alphanum().optional()
},
payload: {
uid: isA.string().max(32).regex(HEX_STRING).required(),
code: isA.string().min(32).max(32).regex(HEX_STRING).required(),
Expand All @@ -287,11 +281,7 @@ module.exports = (log, db, mailer, config, customs, push) => {
handler: async function (request) {
log.begin('Account.RecoveryEmailVerify', request)

const uid = request.payload.uid
const code = request.payload.code
const service = request.payload.service || request.query.service
const type = request.payload.type || request.query.type
const marketingOptIn = request.payload.marketingOptIn
const { code, marketingOptIn, service, type, uid } = request.payload

// verify_code because we don't know what type this is yet, but
// we want to record right away before anything could fail, so
Expand Down Expand Up @@ -322,15 +312,12 @@ module.exports = (log, db, mailer, config, customs, push) => {
// If so, verify the secondary email and respond
if (type && type === 'secondary') {
let matchedEmail
return db.accountEmails(account.uid)
return db.accountEmails(uid)
.then((emails) => {
const isEmailVerification = emails.some((email) => {
if (email.emailCode && (code === email.emailCode)) {
matchedEmail = email
log.info('account.verifyEmail.secondary.started', {
uid: uid,
code: request.payload.code
})
log.info('account.verifyEmail.secondary.started', { uid, code })
return true
}
})
Expand All @@ -343,19 +330,13 @@ module.exports = (log, db, mailer, config, customs, push) => {
// User is attempting to verify a secondary email that has already been verified.
// Silently succeed and don't send post verification email.
if (matchedEmail.isVerified) {
log.info('account.verifyEmail.secondary.already-verified', {
uid: uid,
code: request.payload.code
})
log.info('account.verifyEmail.secondary.already-verified', { uid, code })
return P.resolve()
}

return db.verifyEmail(account, code)
.then(() => {
log.info('account.verifyEmail.secondary.confirmed', {
uid: uid,
code: request.payload.code
})
log.info('account.verifyEmail.secondary.confirmed', { uid, code })

return mailer.sendPostVerifySecondaryEmail([], account, {
acceptLanguage: request.app.acceptLanguage,
Expand All @@ -377,11 +358,7 @@ module.exports = (log, db, mailer, config, customs, push) => {
},
err => {
if (err.errno !== error.ERRNO.DEVICE_UNKNOWN) {
log.error('Account.RecoveryEmailVerify', {
err: err,
uid: uid,
code: code
})
log.error('Account.RecoveryEmailVerify', { err, uid, code })
}
}
)
Expand All @@ -403,14 +380,9 @@ module.exports = (log, db, mailer, config, customs, push) => {
if (! isAccountVerification) {

// Don't log sign-in confirmation success for the account verification case
log.info('account.signin.confirm.success', {
uid: uid,
code: request.payload.code
})
log.info('account.signin.confirm.success', { uid, code })

request.emitMetricsEvent('account.confirmed', {
uid: uid
})
request.emitMetricsEvent('account.confirmed', { uid })
request.app.devices.then(devices =>
push.notifyAccountUpdated(uid, devices, 'accountConfirm')
)
Expand All @@ -422,11 +394,7 @@ module.exports = (log, db, mailer, config, customs, push) => {
return
}

log.error('account.signin.confirm.invalid', {
uid: uid,
code: request.payload.code,
error: err
})
log.error('account.signin.confirm.invalid', { err, uid, code })
throw err
})
.then(() => {
Expand Down Expand Up @@ -454,7 +422,7 @@ module.exports = (log, db, mailer, config, customs, push) => {
locale: account.locale,
marketingOptIn: marketingOptIn ? true : undefined,
service,
uid: account.uid,
uid,
}),
request.emitMetricsEvent('account.verified', {
// The content server omits marketingOptIn in the false case.
Expand Down

0 comments on commit 6956f1d

Please sign in to comment.