1052247 - FxAccountsOAuthClient should require HTTPS URLs

Status: RESOLVED FIXED

(Firefox :: Firefox Accounts, defect, P2)

Description

[Matthew N. [:MattN]]

We should enforce at the FxAccountsOAuthClient level that OAuth is done over HTTPS, not HTTP.

Comment 1

[Chris Karlof [:ckarlof]]

I don't disagree. One note: we enforce HTTPS the FxA Auth URL in the browser without any easy way to override, and it's a PITA to do dev against a local HTTP server. It would be nice to have a pref to disable that enforcement.

Comment 2

[Matthew N. [:MattN]]

Yeah, we will need a way to override it since XPCShell tests still don't allow HTTPS test servers (bug that is bug 466524. browser-chrome and other suites support this already). For UITour we added a pref browser.uitour.requireSecure that is true by default but can be flipped to false for developers.

Comment 3

[Vlad Filippov (:vladikoff)]

(In reply to Matthew N. [:MattN] from comment #2)
> Yeah, we will need a way to override it since XPCShell tests still don't
> allow HTTPS test servers (bug that is bug 466524. browser-chrome and other
> suites support this already). For UITour we added a pref
> browser.uitour.requireSecure that is true by default but can be flipped to
> false for developers.


A custom setting such as "requireSecure" would be awesome! 

Currently for FxA we have: http://mxr.mozilla.org/mozilla-central/source/services/fxaccounts/FxAccounts.jsm#848

"Firefox Accounts server must use HTTPS"

Comment 4

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1052247 - Enforce that OAuth is done over HTTPS in FxAccountsOAuthClient.

Review commit: https://reviewboard.mozilla.org/r/186508/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186508/

Comment 5

[Thom Chiovoloni [:tcsc] (ex-moco)]

I'm not 100% sure if this was supposed to be via a new preference, or via same one mentioned in comment 3. I've assumed it's supposed to be a new one in this code.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8915313 [details]
Bug 1052247 - Enforce that OAuth is done over HTTPS in FxAccountsOAuthClient.

https://reviewboard.mozilla.org/r/186508/#review191684

This looks fine, but as you guessed I might say, I see no need for a new preference - this is a "debug only" preference, so I think it's fine to have a single pref that applies everywhere and is one less thing to confuse other devs.

Comment 7

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8915313 [details]
Bug 1052247 - Enforce that OAuth is done over HTTPS in FxAccountsOAuthClient.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186508/diff/1-2/

Comment 8

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8915313 [details]
Bug 1052247 - Enforce that OAuth is done over HTTPS in FxAccountsOAuthClient.

https://reviewboard.mozilla.org/r/186508/#review192038

Thanks!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1858ac6e5128
Enforce that OAuth is done over HTTPS in FxAccountsOAuthClient. r=markh

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1858ac6e5128