Closed Bug 1162495 Opened 5 years ago Closed 5 years ago

When using FxAccountsOAuthClient, unable to specify email parameter to go with action="force_auth"

Categories

(Firefox :: Firefox Accounts, defect)

33 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Whiteboard: [fix landed in 40, test landed in 41])

Attachments

(2 files, 1 obsolete file)

For bug 1153788, we want to force re-authentication of a signed-in user. Currently we're unable to do that as when specifying the parameter action="force_auth" there's no option available in FxAccountsOAuthClient for specifying the email address.

I've got a patch to allow that.
This adds the additional email option. I've also added validation to check its supplied when action is force_auth, and a test for the validation.

Background docs here: https://github.com/mozilla/fxa-oauth-server/blob/master/docs/api.md#get-v1authorization

I don't mind who reviews, though I'd like to get this landed before the weekend, so that I can land the other bug as well which has strings and is needed before uplift.
Attachment #8602720 - Flags: review?(markh)
Attachment #8602720 - Flags: review?(MattN+bmo)
Comment on attachment 8602720 [details] [diff] [review]
When using FxAccountsOAuthClient, unable to specify email parameter to go with action="force_auth".

Review of attachment 8602720 [details] [diff] [review]:
-----------------------------------------------------------------

Seems reasonable to me. A test testing that the email is actually passed along would normally be expected but I'll leave that up to you.
Attachment #8602720 - Flags: review?(markh)
Attachment #8602720 - Flags: review?(MattN+bmo)
Attachment #8602720 - Flags: review+
Ok, I found where to add tests. Its a bit strange that these are so far away from the actual code, but anyway. Here's the test.

(I landed the main patch earlier today as we needed it for some Fx40 stuff).
Attachment #8603359 - Flags: review?(MattN+bmo)
I fixed some nits and improved how the query parameter checking gets output.
Attachment #8603359 - Attachment is obsolete: true
Attachment #8603359 - Flags: review?(MattN+bmo)
Attachment #8603606 - Flags: review+
A try push isn't necessary IMO as I ran the test locally and it shouldn't vary by platform.
Status: NEW → ASSIGNED
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6081ec5c83dc
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Test didn't land
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/83a39ba82222
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: mozilla40 → mozilla41
Whiteboard: [fix landed in 40, test landed in 41]
Product: Core → Firefox
Target Milestone: mozilla41 → Firefox 41
You need to log in before you can comment on or make changes to this bug.