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

Issue 739101 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Wrong format of Android sync credentials

Project Member Reported by vasi...@chromium.org, Jul 4 2017

Issue description

Chrome Version: 61
OS: all

Android autofill saves credentials in a format different from YOLO. Example:

origin_url: android://u0A-O7Ivuokjnqmf1ciagyiwkxqsLSrXA9ZzyIb4yGEVu5tPRfhJbn8REHGmDAUkUCGf71TqwUcwSfwFObRssA==@com.twitter.android
signon_realm: android://u0A-O7Ivuokjnqmf1ciagyiwkxqsLSrXA9ZzyIb4yGEVu5tPRfhJbn8REHGmDAUkUCGf71TqwUcwSfwFObRssA==@com.twitter.android

They should use a trailing '/'. We should do two things

1. Chrome should save those credentials in the proper format internally so they at least work.
2. When GMS core v12 (containing the bug) sunsets, fix the data in the cloud to use proper format.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 6 2017

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

commit 4d5d6474d72a5b7bd95f76da0acd40cf2f9d92db
Author: vasilii <vasilii@chromium.org>
Date: Thu Jul 06 09:52:32 2017

Update the comments on PasswordSpecificsData. The Android format was described wrong.

BUG= 739101 

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

[modify] https://crrev.com/4d5d6474d72a5b7bd95f76da0acd40cf2f9d92db/components/sync/protocol/password_specifics.proto

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 20 2017

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

commit 17634c18f71fffdd50e973902aadb0e90207f4b6
Author: vasilii <vasilii@chromium.org>
Date: Thu Jul 20 21:48:37 2017

Save Android Autofill credentials in the right format.

Android Autofill saves credentials in the wrong format. origin and signon_realm have no trailing '/' which prevents proper handling in Chrome.
This CL migrates them to the proper format with '/'. The original entry stays on the server and it's kept up to date. In the future the credentials without '/' are to be removed from the Sync data.

BUG= 739101 

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

[modify] https://crrev.com/17634c18f71fffdd50e973902aadb0e90207f4b6/components/password_manager/core/browser/password_syncable_service.cc
[modify] https://crrev.com/17634c18f71fffdd50e973902aadb0e90207f4b6/components/password_manager/core/browser/password_syncable_service.h
[modify] https://crrev.com/17634c18f71fffdd50e973902aadb0e90207f4b6/components/password_manager/core/browser/password_syncable_service_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 28 2017

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

commit 66b77df577f439cd967b025a0a1fc93cbea09f80
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Nov 28 21:12:23 2017

Fix a typo in TwoWayServerAndLocalMerge

The test
PasswordSyncableServiceAndroidAutofillTest.TwoWayServerAndLocalMerge
derives a few parameters from a single integer variable in two places.
It seems that in the second place it prepares another variable to
derive from but then uses the same as in the first place. Looks like a
copy-paste typo. In any case, switching to the prepared variable has
strictly more coverage, so this CL changes to the prepared variable.

Bug:  739101 
Change-Id: I27bb5895e48c1d1f87f91cac8a9e26fa81390e1d
Reviewed-on: https://chromium-review.googlesource.com/793610
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519837}
[modify] https://crrev.com/66b77df577f439cd967b025a0a1fc93cbea09f80/components/password_manager/core/browser/password_syncable_service_unittest.cc

Comment 4 by s...@chromium.org, Jan 17 2018

Labels: Sync-Triaged SyncHandoff2018
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 2 2018

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

commit e2e467d19e84f8acd20a730203a1e495a7e2a012
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Fri Feb 02 16:49:41 2018

Revert special handling of Android credentials in Chrome Sync.

It was introduced in r488421 to handle the credentials without trailing '/' in signon_realm and origin.
Now however, the signon_realm is created correctly by GmsCore. The origin has no '/'. As a result
- r488421 is useless with a lot of complexity.
- r488421 is harmful because it prevent the credential from being deleted via passwords.google.com. See b/69245513

R=battre@chromium.org

Bug:  739101 , 801918 
Change-Id: I20e64481b8dd972e3b43f18a00b0b3078f58c4fe
Reviewed-on: https://chromium-review.googlesource.com/897610
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534073}
[modify] https://crrev.com/e2e467d19e84f8acd20a730203a1e495a7e2a012/components/password_manager/core/browser/password_syncable_service.cc
[modify] https://crrev.com/e2e467d19e84f8acd20a730203a1e495a7e2a012/components/password_manager/core/browser/password_syncable_service.h
[modify] https://crrev.com/e2e467d19e84f8acd20a730203a1e495a7e2a012/components/password_manager/core/browser/password_syncable_service_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 8 2018

Labels: merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a576874a67509c0e15707408807efc213cb3e13

commit 7a576874a67509c0e15707408807efc213cb3e13
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Thu Feb 08 19:22:01 2018

Revert special handling of Android credentials in Chrome Sync.

It was introduced in r488421 to handle the credentials without trailing '/' in signon_realm and origin.
Now however, the signon_realm is created correctly by GmsCore. The origin has no '/'. As a result
- r488421 is useless with a lot of complexity.
- r488421 is harmful because it prevent the credential from being deleted via passwords.google.com. See b/69245513

R=battre@chromium.org
TBR=vasilii@chromium.org

(cherry picked from commit e2e467d19e84f8acd20a730203a1e495a7e2a012)

Bug:  739101 , 801918 
Change-Id: I20e64481b8dd972e3b43f18a00b0b3078f58c4fe
Reviewed-on: https://chromium-review.googlesource.com/897610
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#534073}
Reviewed-on: https://chromium-review.googlesource.com/909391
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#383}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/7a576874a67509c0e15707408807efc213cb3e13/components/password_manager/core/browser/password_syncable_service.cc
[modify] https://crrev.com/7a576874a67509c0e15707408807efc213cb3e13/components/password_manager/core/browser/password_syncable_service.h
[modify] https://crrev.com/7a576874a67509c0e15707408807efc213cb3e13/components/password_manager/core/browser/password_syncable_service_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment