X Tutup
Skip to content

Tell the browser not to autofill those password fields#35931

Merged
DeepDiver1975 merged 1 commit intomasterfrom
no_password_autocomplete
Aug 7, 2019
Merged

Tell the browser not to autofill those password fields#35931
DeepDiver1975 merged 1 commit intomasterfrom
no_password_autocomplete

Conversation

@jvillafanez
Copy link
Member

@jvillafanez jvillafanez commented Jul 26, 2019

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

@jvillafanez jvillafanez self-assigned this Jul 26, 2019
@jvillafanez jvillafanez force-pushed the no_password_autocomplete branch from 0c28736 to b0edb81 Compare July 29, 2019 07:16
@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 •
edited
Loading

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 force-pushed the no_password_autocomplete branch from b0edb81 to 3e3114a Compare July 30, 2019 11:32
@jvillafanez jvillafanez force-pushed the no_password_autocomplete branch from 3e3114a to 588feb5 Compare August 1, 2019 11:25
@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

jvillafanez commented Aug 5, 2019

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
Closed
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DeepDiver1975 DeepDiver1975 DeepDiver1975 approved these changes

Assignees

@jvillafanez jvillafanez

Labels

3 - To Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jvillafanez @DeepDiver1975 @phil-davis @patrickjahns
X Tutup