Skip to content
This repository has been archived by the owner. It is now read-only.

/ fxa-content-server Archived

Firefox password manager asks to remember password in iframe flow #2373

Closed
shane-tomlinson opened this issue May 3, 2015 · 7 comments
Closed

Firefox password manager asks to remember password in iframe flow #2373

shane-tomlinson opened this issue May 3, 2015 · 7 comments

Comments

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented May 3, 2015

The Firefox password manager does not allow passwords from about:accounts to be saved. The firstrun flow is not about:accounts, hence the password manager asks if the user wants to save their password.

It looks like this behavior can be disabled w/o a Firefox change by adding autocomplete=off to the form element.

cc @ckarlof

ref #2101, #363, #1780, #1788
@shane-tomlinson shane-tomlinson mentioned this issue May 3, 2015
Closed
24 of 25 tasks complete
@shane-tomlinson shane-tomlinson self-assigned this May 3, 2015
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented May 3, 2015

My proposal:

  • Never add autocomplete=off to the password field, if type===password
  • Instead, add autocomplete=off to the form field if the service===sync.
  • And, add autocomplete=off to the password field when type===text
shane-tomlinson pushed a commit that referenced this issue May 3, 2015
Shane Tomlinson
fix(client): Do not allow Fx to save passwords for Sync, even if FxA …
Loading status checks…
9967939 
…is in an iframe.

Actually simplify the autocomplete=off logic quite a bit. If service=sync, set the `autocomplete=off` attribute on the form instead of the password field. This ensure Firefox does not try to save the form for sync, even if in an iframe. On the password field, only set autocomplete=off iff the password field is toggled to type=text.

The addition of `autocomplete=off` is done by JavaScript instead of from within the templates. This allows both the templates and the Views to ignore the autocomplete status.

Also fix the password visibility functional tests. We were not returning the promise for the tests!

fixes #2373
@shane-tomlinson shane-tomlinson mentioned this issue May 3, 2015
Closed
@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented May 6, 2015

I got some sad news about this: Firefox doesn't respect autocomplete=off anymore and it's not how we stop the "remember your password" dialog from appearing on about:accounts anymore. It's not elegant, BTW.
@rfk
Copy link
Member

@rfk rfk commented Jun 7, 2015

Capturing Shane's suggestion from an email thread: perhaps we should expand that regexp to also match "https://accounts.firefox.com/.*service=sync" or similar. It would remain a gross hack, made only slightly grosser by expanding it to cover this new case.
@rfk
Copy link
Member

@rfk rfk commented Jun 7, 2015

A slightly better approach would be to prevent the save-password prompt from advertising sync when you're at one of these URLs, rather than preventing the save-password prompt entirely.
@shane-tomlinson shane-tomlinson removed their assignment Jun 8, 2015
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Jun 11, 2015

Opened https://bugzilla.mozilla.org/show_bug.cgi?id=1173688 to track this from the Fx side.
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Jun 17, 2015

Closing, this was fixed in record time by the Fx team. Thank you @edmoz for working your magic!
@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Jun 17, 2015

It's amazing what happens when we all work together!
@shane-tomlinson shane-tomlinson mentioned this issue Jan 28, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

fix(client): Do not allow Fx to save passwords for Sync, even if FxA is in an iframe.
3 participants
@rfk @shane-tomlinson @ckarlof