Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tell the browser not to autofill those password fields #35931

Merged
merged 1 commit into from
Aug 7, 2019

Conversation

jvillafanez
Copy link
Member

Description

Both firefox and chrome can autofill some username and password fields in forms without user interaction. This is a problem specially in the users page where the password field is hidden but the browser autocompletes it.

Note that neither safari nor edge have this behaviour. They require user interaction in order to fill the password field. This problem doesn't happen in those browsers.

Related Issue

no ticket opened as far as I know.

Motivation and Context

The admin was setting a password without any knowledge.

How Has This Been Tested?

Checked with firefox, chrome, safari and edge:

  1. login as admin.
  2. when prompted, save the password in the browser
  3. go to the users page
  4. at this point the new username and password fields should be empty (this needs to be even if the password field isn't visible)
  5. add a new user using the username + email fields
  6. in a new browser, check the email sent and reset the password for that user
  7. note that the password field for the reset form are also empty.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

This should be moved to the user_management app

@codecov
Copy link

codecov bot commented Jul 29, 2019

Codecov Report

Merging #35931 into stable10 will increase coverage by 34.1%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff               @@
##             stable10   #35931      +/-   ##
==============================================
+ Coverage       30.99%    65.1%   +34.1%     
- Complexity          0    20197   +20197     
==============================================
  Files              53     1302    +1249     
  Lines            4142    77159   +73017     
  Branches            0     1301    +1301     
==============================================
+ Hits             1284    50232   +48948     
- Misses           2858    26542   +23684     
- Partials            0      385     +385
Flag Coverage Δ Complexity Δ
#javascript 53.85% <ø> (?) 0 <ø> (?)
#phpunit 66.29% <0%> (+35.29%) 20197 <0> (+20197) ⬆️
Impacted Files Coverage Δ Complexity Δ
settings/templates/setpassword.php 0% <0%> (ø) 0 <0> (?)
settings/templates/users/part.createuser.php 0% <0%> (ø) 0 <0> (?)
apps/files_external/lib/config.php 10.71% <0%> (-20.87%) 19% <0%> (+19%)
apps/updatenotification/templates/admin.php 0% <0%> (ø) 0% <0%> (?)
lib/private/Encryption/Keys/Storage.php 91.45% <0%> (ø) 48% <0%> (?)
lib/private/App/CodeChecker/NodeVisitor.php 95.86% <0%> (ø) 54% <0%> (?)
lib/private/RedisFactory.php 0% <0%> (ø) 19% <0%> (?)
apps/dav/lib/Avatars/AvatarNode.php 72% <0%> (ø) 9% <0%> (?)
...s/dav/appinfo/Migrations/Version20170202213905.php 21.05% <0%> (ø) 12% <0%> (?)
apps/dav/lib/Upload/ChunkLocationProvider.php 100% <0%> (ø) 4% <0%> (?)
... and 1243 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fc9b7a...b0edb81. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 29, 2019

Codecov Report

Merging #35931 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #35931      +/-   ##
============================================
- Coverage      65.4%    65.4%   -0.01%     
  Complexity    20197    20197              
============================================
  Files          1299     1299              
  Lines         76762    76763       +1     
  Branches       1301     1301              
============================================
  Hits          50203    50203              
- Misses        26174    26175       +1     
  Partials        385      385
Flag Coverage Δ Complexity Δ
#javascript 53.85% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.62% <0%> (-0.01%) 20197 <0> (ø)
Impacted Files Coverage Δ Complexity Δ
settings/templates/setpassword.php 0% <0%> (ø) 0 <0> (ø) ⬇️
settings/templates/users/part.createuser.php 0% <0%> (ø) 0 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffcca54...588feb5. Read the comment docs.

@DeepDiver1975 DeepDiver1975 changed the base branch from stable10 to master July 30, 2019 08:22
@jvillafanez jvillafanez changed the title [stable10] Tell the browser not to autofill those password fields Tell the browser not to autofill those password fields Aug 5, 2019
@jvillafanez
Copy link
Member Author

These are minor html changes, so nothing to unittest

@DeepDiver1975 DeepDiver1975 merged commit 42eaf1b into master Aug 7, 2019
@delete-merged-branch delete-merged-branch bot deleted the no_password_autocomplete branch August 7, 2019 08:05
@davitol davitol mentioned this pull request Sep 3, 2019
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants