Passwords PSL Matching on Gnome: overwrite realm and origin

Passwords PSL Matching on Gnome: overwrite realm and origin

On LoginDatabase using platforms, a password entry found by a PSL match gets the signon realm and the origin overwritten to match the query password form. This way, when the credential is saved, it is saved for the new site.

On Gnome, the overwriting did not happen. This CL introduces this to match the LoginDatabase behaviour.

Also, on Gnome, PSL matching was used not only to get logins, but also to check for existing logins when adding and updating passwords. The latter two use cases should require exact matching (that's also what happens in LoginDatabase). This CL fixes that, and only keeps PSL matching for GetLogins.

BUG=364745
R=gcasto@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266177

[vabr (Chromium), 2014-04-21 21:20:13]

Hi Garrett,

PTAL.

I'm sorry to have squashed two semi-independent changes into one CL. If you find
this too complex, I'll split the no-PSL-during-UpdateLogin-and-AddLogin part
from the replace-realm-origin-action-by-lookup-form part. I put them together
because the code changes overlap, and they both have the goal of match the
LoginDatabase behaviour.

Cheers,
Vaclav

[vabr (Chromium), 2014-04-23 13:13:06]

On 2014/04/21 21:20:13, vabr (Chromium) wrote:
> Hi Garrett,
> 
> PTAL.
> 
> I'm sorry to have squashed two semi-independent changes into one CL. If you
find
> this too complex, I'll split the no-PSL-during-UpdateLogin-and-AddLogin part
> from the replace-realm-origin-action-by-lookup-form part. I put them together
> because the code changes overlap, and they both have the goal of match the
> LoginDatabase behaviour.
> 
> Cheers,
> Vaclav

Hi Garrett, ping!

[Garrett Casto, 2014-04-24 06:15:24]

lgtm

https://codereview.chromium.org/245563005/diff/20001/chrome/browser/password_...
File chrome/browser/password_manager/native_backend_gnome_x_unittest.cc (right):

https://codereview.chromium.org/245563005/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/native_backend_gnome_x_unittest.cc:452: // the
credential update is done via UpdateLogin or AddLogin.
Depending on what we end up doing with sync, it's not totally clear that calling
AddLogin for a an existing credential is going to be valid in the future. That
being said, for now it can still happen so it's reasonable to test.

[vabr (Chromium), 2014-04-24 08:33:47]

Thanks, Garrett.

Addressed your (review) comment with a (code) comment, and sending to CQ now.

Cheers,
Vaclav

https://codereview.chromium.org/245563005/diff/20001/chrome/browser/password_...
File chrome/browser/password_manager/native_backend_gnome_x_unittest.cc (right):

https://codereview.chromium.org/245563005/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/native_backend_gnome_x_unittest.cc:452: // the
credential update is done via UpdateLogin or AddLogin.
On 2014/04/24 06:15:24, Garrett Casto wrote:
> Depending on what we end up doing with sync, it's not totally clear that
calling
> AddLogin for a an existing credential is going to be valid in the future. That
> being said, for now it can still happen so it's reasonable to test.

I agree. Added a TODO comment below to make it easier for the person who bans
AddLogin from acting on existing logins and observes PSLUpdatingStrictAddLogin
being broken.

[vabr (Chromium), 2014-04-24 08:33:55]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2014-04-24 08:34:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/245563005/40001

[commit-bot: I haz the power, 2014-04-24 09:25:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-24 09:25:33]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[vabr (Chromium), 2014-04-24 10:03:46]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2014-04-24 11:03:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/245563005/40001

[commit-bot: I haz the power, 2014-04-24 11:31:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-24 11:31:10]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[vabr (Chromium), 2014-04-24 12:18:01]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2014-04-24 12:18:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/245563005/40001

[commit-bot: I haz the power, 2014-04-24 17:13:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-24 17:13:09]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[vabr (Chromium), 2014-04-25 08:31:08]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2014-04-25 08:34:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/245563005/40001

[commit-bot: I haz the power, 2014-04-25 08:58:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-25 08:58:44]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[vabr (Chromium), 2014-04-25 09:32:31]

Committed patchset #2 manually as r266177 (presubmit successful).