New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 607858 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Buried. Ping if important.
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocking:
issue 400674



Sign in to add a comment

'requireUserMediation()` ought to look at registrable domain, not URL.

Project Member Reported by mkwst@chromium.org, Apr 29 2016

Issue description

Currently, we're checking for an exact origin match between a form and the page which requested `requireUserMediation()`. Given error reports in the wild, we should probably tighten this down to a registrable domain match. That is, calling the method on `login.example.com` should clear the flag for credentials stored on `*.example.com`.
 

Comment 1 by mkwst@chromium.org, Apr 29 2016

Labels: -M51 ReleaseBlock-Stable M-51
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3aa4344859e35071117a4278663eecf4f3ea9d17

commit 3aa4344859e35071117a4278663eecf4f3ea9d17
Author: mkwst <mkwst@chromium.org>
Date: Fri Apr 29 11:36:36 2016

CREDENTIAL: Tighten the behavior of 'requireUserMediation()'

Currently, we're looking for an exact origin match between a stored
PasswordForm and the page which called 'requireUserMediation()'. We ought
to be looking for a match between their registrable domains, given that
we allow credentials to flow ~freely between these origins.

This patch tightens the check accordingly.

BUG= 607858 
R=vabr@chromium.org

Review-Url: https://codereview.chromium.org/1928323002
Cr-Commit-Position: refs/heads/master@{#390618}

[modify] https://crrev.com/3aa4344859e35071117a4278663eecf4f3ea9d17/components/password_manager/content/browser/credential_manager_impl_unittest.cc
[modify] https://crrev.com/3aa4344859e35071117a4278663eecf4f3ea9d17/components/password_manager/core/browser/credential_manager_pending_require_user_mediation_task.cc
[modify] https://crrev.com/3aa4344859e35071117a4278663eecf4f3ea9d17/components/password_manager/core/browser/credential_manager_pending_require_user_mediation_task.h

Comment 3 by mkwst@chromium.org, Apr 29 2016

Labels: Merge-Request-51
Hello, friendly release managers! Once this bakes on Canary, I'd like to merge it back to 51, as it fixes a fairly noticable bug in the credential management API.
Cc: mkwst@chromium.org sabineb@chromium.org vasi...@chromium.org
 Issue 607877  has been merged into this issue.
Labels: OS-All
Labels: -Merge-Request-51 Merge-Approved-51
Merge approved for M51 branch 2704.

Comment 7 by gov...@chromium.org, Apr 29 2016

Please merge your change to M51 branch 2704 before 5:00 PM PST, Monday (05/02/16), so we can take it in for next week M51 beta release. Thank you.

Comment 8 by tkent@chromium.org, May 1 2016

Components: -Blink>Forms>Password Blink>SecurityFeature

Comment 9 by vabr@chromium.org, May 2 2016

The merge has a conflict because credential_manager_impl_unittest.cc is called credential_manager_dispatcher_unittest.cc in 2704. I'm working on the patch.
Project Member

Comment 10 by bugdroid1@chromium.org, May 2 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe25aa3b45a73b0fe824a0657dba755195d44e39

commit fe25aa3b45a73b0fe824a0657dba755195d44e39
Author: Vaclav Brozek <vabr@chromium.org>
Date: Mon May 02 09:19:19 2016

CREDENTIAL: Tighten the behavior of 'requireUserMediation()'

Currently, we're looking for an exact origin match between a stored
PasswordForm and the page which called 'requireUserMediation()'. We ought
to be looking for a match between their registrable domains, given that
we allow credentials to flow ~freely between these origins.

This patch tightens the check accordingly.

BUG= 607858 
R=vabr@chromium.org

Review-Url: https://codereview.chromium.org/1928323002
Cr-Commit-Position: refs/heads/master@{#390618}
(cherry picked from commit 3aa4344859e35071117a4278663eecf4f3ea9d17)

Review URL: https://codereview.chromium.org/1942723002 .

Cr-Commit-Position: refs/branch-heads/2704@{#328}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/fe25aa3b45a73b0fe824a0657dba755195d44e39/components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc
[modify] https://crrev.com/fe25aa3b45a73b0fe824a0657dba755195d44e39/components/password_manager/core/browser/credential_manager_pending_require_user_mediation_task.cc
[modify] https://crrev.com/fe25aa3b45a73b0fe824a0657dba755195d44e39/components/password_manager/core/browser/credential_manager_pending_require_user_mediation_task.h

Comment 11 by vabr@chromium.org, May 2 2016

Status: Fixed (was: Started)
Beta builders look green, closing.
Cc: -vabr@chromium.org

Sign in to add a comment