Skip to content

Loading…

Confirm email addition in 2nd browser causes email_addition_status to return 400 "Bad Request" #2307

Closed
shane-tomlinson opened this Issue · 3 comments

5 participants

@shane-tomlinson
Mozilla member

STR

Requires two browsers.

1) In Browser A, Open dev.123done.org.
2) Open dialog, sign in with primary only account
3) Sign out of dev.123done.org, open dialog.
4) Click "Add new email address"
5) Enter account's first secondary address
6) Enter password
7) Submit form
8) In Browser B, Open email provider
9) Click verification link in email
10) Watch Browser A blow up.

Exact error in Browser A:

Action: Registration Failed
Network Info: GET: /wsapi/email_addition_status?email=set117%40gmail.com

Response Code - 400

Response Text: Bad Request: requires authentication

Error Type: Bad Request: requires authentication

email_addition_status returns 400, Bad Request: requires authentication

@lloyd lloyd was assigned
@zaach
Mozilla member

This appears to have been introduced with the new password change, session reset feature. The session is being reset here.

/cc @warner

@warner
Mozilla member

@zaach and I are looking at this. We're looking suspiciously at db.completeConfirmEmail which calls db.updatePassword with the "please update lastPasswordReset" flag set to "true". We think that if it were given "false", then the session would not be clobbered, and browser A would not get an error.

Why is completeConfirmEmail trying to (sometimes) reset the password hash? I can't figure out which code paths would lead into this method, so I don't know where to look for o.passwd being set or not. Is it possible that this whole clause is unused, and o.passwd will always be undefined?

Anyways, setting that "true" to "false" is probably the immediate fix, but we don't yet know what that might break.

Also, it would be nice if browser A's code that is polling email_addition_status could respond to a 400 Unauthorized by jumping back to the login page, instead of exploding like this. Fixing this bug should avoid creating the 400, but in the long run we should fix both.

@warner warner added a commit that referenced this issue
@warner warner Bug #2307: don't expire existing sessions when adding a secondary add…
…ress

If a persona.org account is initially created with a "primary"
address (meaning an address served by a participating IdP, so
persona.org is given an assertion from that IdP as proof of ownership),
the new account will not have a password associated with it. If you then
add a "secondary" address (meaning an address *not* served by a
participating IdP, requiring an email challenge to prove ownership), you
will have to set up a password when you add the secondary. The
establishment of this password should *not* invalidate any sessions that
were set up earlier.

In Bug #2307, this manifested as the first browser (in which the
add-secondary-email operation was started, so it had the old session and
was waiting for the operation to complete, polling
/wsapi/email_addition_status all the while) receiving a "400
Unauthorized" error when the email challenge link was opened in a second
browser (which thus got a new session).

The test for this effect lives in tests/primary-then-secondary-test.js,
which need the same 2-second delay as password-update-test.js (to make
sure that the modified lastPasswordReset time was actually different
than the previous value, so the session really would be expired).
5f5d8e5
@zaach zaach closed this
@jrgm
Mozilla member

verified train-2012.08.17 on ephemeral https://train0817.personatest.org for original STR and also reviewing db state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.