Have sync prefs pane go to the new about:accounts "force auth" page

RESOLVED FIXED in Firefox 29

Status

()

Firefox
Sync
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: markh, Assigned: markh)

Tracking

unspecified
Firefox 29
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
The accounts.firefox.com page has a /force_auth page which takes an email address as a query string and forces that account to reauth.  The Sync Prefs pane should take the user to this new page whenever sync is in an authentication failure state.
(Assignee)

Comment 1

4 years ago
Created attachment 8368925 [details] [diff] [review]
0005-Bug-966520-have-sync-prefs-pane-open-the-new-force-s.patch

* aboutaccounts.js now handles a special querystring value which tells it to load the special force_auth page.

* fxAccounts.jsm has a new method to fetch the force_auth URL.  It needs to read a pref which may (and currently does) have a querystring already in the URL, and attach a new path element *and* a new querystring element.

This patch also has a couple of cleanups:

* about:accounts used to support ?auth=true.  Instead it now supports ?action=signin and ?action=reauth (and later we may add ?action=create)

* sync.xul used to have the link opening code directly in the layout - this has been cleaned up to be more consistent.
Assignee: nobody → mhammond
Status: NEW → ASSIGNED
Attachment #8368925 - Flags: review?(gavin.sharp)
Comment on attachment 8368925 [details] [diff] [review]
0005-Bug-966520-have-sync-prefs-pane-open-the-new-force-s.patch

>diff --git a/browser/base/content/aboutaccounts/aboutaccounts.js b/browser/base/content/aboutaccounts/aboutaccounts.js

>-      iframe.src = fxAccounts.getAccountsURI();
>+      iframe.src = url ? url : fxAccounts.getAccountsURI();

url || fxAccounts.getAccountsURI()

> function init() {
>-  let signinQuery = window.location.href.match(/signin=true$/);
>+  let action;
>+  // simple regex which assumes only 1 query string is supported.
>+  let match = /\?action=(.*)$/.exec(window.location.href);
>+  if (match) {
>+    action = match[1];
>+  }

Guess I lied when I said I didn't care - this seems a bit overkill. How about just:

if (location.search.contains("action=signin")) {
} else if (location.search.contains("action=reauth")) {

etc.

>diff --git a/services/fxaccounts/FxAccounts.jsm b/services/fxaccounts/FxAccounts.jsm

>+  // Returns a promise that resolves with the URL to use to force a re-signin
>+  // of the current account.
>+  promiseAccountsForceSigninURI: function() {
>+    return this.getSignedInUser().then(accountData => {
>+      if (!accountData) {
>+        return null;
>+      }
>+      // a bit of munging, as we need to append a path while keeping the query
>+      // parts.

You should be able to just set url.filePath to achieve that (then append the query string value).
Attachment #8368925 - Flags: review?(gavin.sharp) → review-
This will fit nicely with my patch in bug 966511, which wants to have separate "sign in" and "create account" links.
(Assignee)

Comment 4

4 years ago
Created attachment 8368966 [details] [diff] [review]
0005-Bug-966520-have-sync-prefs-pane-open-the-new-force-s.patch

with requested changes
Attachment #8368925 - Attachment is obsolete: true
Attachment #8368966 - Flags: review?(gavin.sharp)
Attachment #8368966 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 5

4 years ago
https://hg.mozilla.org/integration/fx-team/rev/a6805f2f8914
https://hg.mozilla.org/mozilla-central/rev/a6805f2f8914
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.