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

Issue 824791 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug

Blocking:
issue 807826
issue 820188



Sign in to add a comment

[Dice] Sync may still be running after web signout.

Project Member Reported by droger@chromium.org, Mar 22 2018

Issue description

This is hypothetical, I haven't tried to repro, but seems to be a real problem.

Repro steps would be:
- Start sync
- sign out on the web
- Chrome enters error state. The user is told that Chrome is no longer Syncing.
- Chrome tries to revoke its current token, but fails (e.g. because of network issues to reach Gaia), and replaces it by the dummy token.
- Sync is notified that there is a new token available (the dummy token), and tries to use it to get a new Sync access token. That fails, and thus Sync decides to keep using its existing access token.
- Since the refresh token was not correctly revoked, the Sync access token remains valid for up to 1 hour, and Sync continues working.


This potential scenario was discussed here:
https://chromium-review.googlesource.com/c/chromium/src/+/966422/7/components/browser_sync/profile_sync_service.cc#1010
 

Comment 1 by droger@chromium.org, Mar 22 2018

Components: Services>SignIn

Comment 2 by droger@chromium.org, Mar 22 2018

Components: Services>Sync

Comment 3 by ew...@chromium.org, Mar 22 2018

Labels: Hotlist-DICE-MVP
Is it possible for us to simply clear the current access token when the user signs out of the web as well?

Comment 4 by droger@chromium.org, Mar 22 2018

Yes, that's the only reliable fix.

Additionally we could also try harder to revoke the token on the server (by retrying multiple times in case of failures) but success can never be guaranteed.

Comment 5 by ew...@chromium.org, Mar 22 2018

Yeah, that doesn't necessarily seem like it would be the right path to go down. Simply clearing the current access token SGTM.

Comment 6 by feuunk@google.com, Mar 27 2018

Blocking: 819785

Comment 7 by feuunk@google.com, Apr 5 2018

Blocking: 820188

Comment 8 by feuunk@google.com, Apr 5 2018

Blocking: -819785

Comment 9 by droger@chromium.org, Apr 10 2018

Status: Started (was: Assigned)
feuunk: do you have a recommendation how I can check if Sync really stopped?
I tried looking at chrome:sync-internals, but I don't see anything obvious there, and even if it shows an error somewhere I also don't know if it can be trusted.

Comment 11 by feuunk@google.com, Apr 11 2018

You want to look for:

Server Connection	auth error since $(date+time)
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 13 2018

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

commit 2020e88b0f2acd5380f8280cded727ada79cf5a2
Author: David Roger <droger@chromium.org>
Date: Fri Apr 13 10:25:30 2018

[signin] Delay token revocation until network is available

Token revocation is only best effort and has no guarantee to go through.
However existing code was just giving up if the network was down.
This CL improves the behavior, by waiting until the network is available
before sending the revocation request.

Bug:  824791 
Change-Id: I1bed2b1d28e5e827a07be0797fe6a56a38778a29
Reviewed-on: https://chromium-review.googlesource.com/1004616
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550576}
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/components/signin/core/browser/test_signin_client.cc
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/components/signin/core/browser/test_signin_client.h

Comment 13 by ew...@chromium.org, Apr 16 2018

Hey David, can this be marked as Fixed? Also FYI, the CL in #12 landed in 68, so we'll need to request a merge.
Cc: yananj@google.com
No this is still not fixed.
I have a CL approved (https://chromium-review.googlesource.com/c/chromium/src/+/1007277), which is for Sync only. I don't know yet about other services that could be affected.

However I'd like to discuss with Gaia team if we could instead implement a server-side solution, as it would be better: the signout would be really atomic, and Chrome would not have to hunt down all the access tokens in all services.

CC'ing yananj, but we'll talk about that in the Dice meeting also.
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2020e88b0f2acd5380f8280cded727ada79cf5a2

commit 2020e88b0f2acd5380f8280cded727ada79cf5a2
Author: David Roger <droger@chromium.org>
Date: Fri Apr 13 10:25:30 2018

[signin] Delay token revocation until network is available

Token revocation is only best effort and has no guarantee to go through.
However existing code was just giving up if the network was down.
This CL improves the behavior, by waiting until the network is available
before sending the revocation request.

Bug:  824791 
Change-Id: I1bed2b1d28e5e827a07be0797fe6a56a38778a29
Reviewed-on: https://chromium-review.googlesource.com/1004616
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550576}
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/components/signin/core/browser/test_signin_client.cc
[modify] https://crrev.com/2020e88b0f2acd5380f8280cded727ada79cf5a2/components/signin/core/browser/test_signin_client.h

Comment 16 by ew...@chromium.org, Apr 18 2018

As we discussed during the meeting today, any server-side solution will be non-trivial. Yanan said she'd follow up with LSO to discuss our options here, but in the meantime, we should land the CL to revoke the access token for Sync, to mitigate the potential downside of this scenario.

Mihai also mentioned adding a histogram to track how much this happens in practice; if that's relatively straightforward to do, could we also add that to this bug?

Comment 17 by yananj@google.com, Apr 19 2018

Cc: dujin...@gogole.com jasonhuang@google.com
Re comment #4, Add retry logic would help, it's also useful to log the reasons for token revocation failure from LSO. As I understand each Chrome service manages their own access token so there is no central cache to delete all access tokens, is that right David?

I've met with Jason (GaiaBE) and Jinhui, the consensus is that it would be good to have stats to measure how much it happens with retry logic. So far support atomic cookie and token invalidation for both ways is just an idea, but dice could make the first use case.

+jasonhuang, dujinhui





Project Member

Comment 18 by bugdroid1@chromium.org, Apr 23 2018

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

commit 01ebc9f6924d8acdc9ef98df6fb4be97a32923d6
Author: David Roger <droger@chromium.org>
Date: Mon Apr 23 11:10:01 2018

[Dice] Sync drops its access token when the refresh token is invalidated

When the user is signing out on the web, they see a message telling them
that Sync is paused.
However, in the case Chrome fails to reach the Gaia server to invalidate
the refresh token, the access token in Sync may remain valid, and Sync
could continue working.

This CL makes sure that the Sync access token is dropped, so that Sync
stops even if Chrome fails to revoke the token on the server.

Bug:  824791 
Change-Id: I430ba9c9c854c35bedd7e1029f91ba6a888b4d4a
Reviewed-on: https://chromium-review.googlesource.com/1007277
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552669}
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/browser_sync/profile_sync_service_unittest.cc
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/driver/glue/sync_backend_host_core.cc
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/driver/glue/sync_backend_host_core.h
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/driver/glue/sync_backend_host_impl.cc
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/driver/glue/sync_backend_host_impl.h
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine/fake_sync_engine.cc
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine/fake_sync_engine.h
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine/fake_sync_manager.cc
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine/fake_sync_manager.h
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine/sync_engine.h
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine/sync_manager.h
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine_impl/net/server_connection_manager.cc
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine_impl/net/server_connection_manager.h
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine_impl/sync_manager_impl.cc
[modify] https://crrev.com/01ebc9f6924d8acdc9ef98df6fb4be97a32923d6/components/sync/engine_impl/sync_manager_impl.h

yananj: yes it is correct, we don't have a centralized way to invalidate access tokens.
I'm going to add more metrics to see how often this happens in practice (tracked in bug 834720).
Labels: Merge-Request-67
Status: Fixed (was: Started)
Marking the bug as fixed for now and requesting a merge to M67, although I only really fixed Sync and did not audit other services. We'll revisit this once we have data from metrics and whether it can be done on the server.
Project Member

Comment 21 by sheriffbot@chromium.org, Apr 24 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M67 branch 3396 ASAP so we can pick it up for next M67 Beta release. Thank you.
Project Member

Comment 23 by bugdroid1@chromium.org, Apr 25 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b59362b73fef9221f83705b80388696e6069423

commit 4b59362b73fef9221f83705b80388696e6069423
Author: David Roger <droger@chromium.org>
Date: Wed Apr 25 12:56:55 2018

[signin] Delay token revocation until network is available

Token revocation is only best effort and has no guarantee to go through.
However existing code was just giving up if the network was down.
This CL improves the behavior, by waiting until the network is available
before sending the revocation request.

TBR=droger@chromium.org

(cherry picked from commit 2020e88b0f2acd5380f8280cded727ada79cf5a2)

Bug:  824791 
Change-Id: I1bed2b1d28e5e827a07be0797fe6a56a38778a29
Reviewed-on: https://chromium-review.googlesource.com/1004616
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550576}
Reviewed-on: https://chromium-review.googlesource.com/1027872
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#286}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/4b59362b73fef9221f83705b80388696e6069423/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc
[modify] https://crrev.com/4b59362b73fef9221f83705b80388696e6069423/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h
[modify] https://crrev.com/4b59362b73fef9221f83705b80388696e6069423/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/4b59362b73fef9221f83705b80388696e6069423/components/signin/core/browser/test_signin_client.cc
[modify] https://crrev.com/4b59362b73fef9221f83705b80388696e6069423/components/signin/core/browser/test_signin_client.h

Project Member

Comment 24 by bugdroid1@chromium.org, Apr 25 2018

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

commit 4ba762474e9a7ae926cbc5ca23f6beb3728b7f87
Author: David Roger <droger@chromium.org>
Date: Wed Apr 25 12:58:05 2018

[Dice] Sync drops its access token when the refresh token is invalidated

When the user is signing out on the web, they see a message telling them
that Sync is paused.
However, in the case Chrome fails to reach the Gaia server to invalidate
the refresh token, the access token in Sync may remain valid, and Sync
could continue working.

This CL makes sure that the Sync access token is dropped, so that Sync
stops even if Chrome fails to revoke the token on the server.

TBR=droger@chromium.org

(cherry picked from commit 01ebc9f6924d8acdc9ef98df6fb4be97a32923d6)

Bug:  824791 
Change-Id: I430ba9c9c854c35bedd7e1029f91ba6a888b4d4a
Reviewed-on: https://chromium-review.googlesource.com/1007277
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552669}
Reviewed-on: https://chromium-review.googlesource.com/1027873
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#288}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/browser_sync/profile_sync_service_unittest.cc
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/driver/glue/sync_backend_host_core.cc
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/driver/glue/sync_backend_host_core.h
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/driver/glue/sync_backend_host_impl.cc
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/driver/glue/sync_backend_host_impl.h
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine/fake_sync_engine.cc
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine/fake_sync_engine.h
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine/fake_sync_manager.cc
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine/fake_sync_manager.h
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine/sync_engine.h
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine/sync_manager.h
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine_impl/net/server_connection_manager.cc
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine_impl/net/server_connection_manager.h
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine_impl/sync_manager_impl.cc
[modify] https://crrev.com/4ba762474e9a7ae926cbc5ca23f6beb3728b7f87/components/sync/engine_impl/sync_manager_impl.h

Project Member

Comment 25 by bugdroid1@chromium.org, Apr 25 2018

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

commit 7377ea2cc84903f9aef76dc835dad9363b3ca245
Author: David Roger <droger@chromium.org>
Date: Wed Apr 25 16:13:39 2018

Revert "[Dice] Sync drops its access token when the refresh token is invalidated"

This reverts commit 4ba762474e9a7ae926cbc5ca23f6beb3728b7f87.

Reason for revert: This broke compile on official builder.

Original change's description:
> [Dice] Sync drops its access token when the refresh token is invalidated
> 
> When the user is signing out on the web, they see a message telling them
> that Sync is paused.
> However, in the case Chrome fails to reach the Gaia server to invalidate
> the refresh token, the access token in Sync may remain valid, and Sync
> could continue working.
> 
> This CL makes sure that the Sync access token is dropped, so that Sync
> stops even if Chrome fails to revoke the token on the server.
> 
> TBR=droger@chromium.org
> 
> (cherry picked from commit 01ebc9f6924d8acdc9ef98df6fb4be97a32923d6)
> 
> Bug:  824791 
> Change-Id: I430ba9c9c854c35bedd7e1029f91ba6a888b4d4a
> Reviewed-on: https://chromium-review.googlesource.com/1007277
> Commit-Queue: David Roger <droger@chromium.org>
> Reviewed-by: Marc Treib <treib@chromium.org>
> Reviewed-by: Mikel Astiz <mastiz@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#552669}
> Reviewed-on: https://chromium-review.googlesource.com/1027873
> Reviewed-by: David Roger <droger@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3396@{#288}
> Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}

TBR=droger@chromium.org,treib@chromium.org,mastiz@chromium.org

Change-Id: I5d3ae0082aa1b26211bf501cb80ba7910e3363f2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  824791 , 836836
Reviewed-on: https://chromium-review.googlesource.com/1027877
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#294}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/browser_sync/profile_sync_service_unittest.cc
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/driver/glue/sync_backend_host_core.cc
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/driver/glue/sync_backend_host_core.h
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/driver/glue/sync_backend_host_impl.cc
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/driver/glue/sync_backend_host_impl.h
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine/fake_sync_engine.cc
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine/fake_sync_engine.h
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine/fake_sync_manager.cc
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine/fake_sync_manager.h
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine/sync_engine.h
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine/sync_manager.h
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine_impl/net/server_connection_manager.cc
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine_impl/net/server_connection_manager.h
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine_impl/sync_manager_impl.cc
[modify] https://crrev.com/7377ea2cc84903f9aef76dc835dad9363b3ca245/components/sync/engine_impl/sync_manager_impl.h

The cherry-pick was reverted because it broke compile. I will fix the conflict and reland tomorrow.
Project Member

Comment 27 by bugdroid1@chromium.org, Apr 26 2018

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

commit 27133efece77f85202f484e1518260d83016685e
Author: David Roger <droger@chromium.org>
Date: Thu Apr 26 15:30:59 2018

[Dice] Sync drops its access token when the refresh token is invalidated

When the user is signing out on the web, they see a message telling them
that Sync is paused.
However, in the case Chrome fails to reach the Gaia server to invalidate
the refresh token, the access token in Sync may remain valid, and Sync
could continue working.

This CL makes sure that the Sync access token is dropped, so that Sync
stops even if Chrome fails to revoke the token on the server.

TBR=droger

Bug:  824791 
Change-Id: I5f403570afc00568e19d130166fd7d5364de2586
Reviewed-on: https://chromium-review.googlesource.com/1030631
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#325}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/browser_sync/profile_sync_service_unittest.cc
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/driver/glue/sync_backend_host_core.cc
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/driver/glue/sync_backend_host_core.h
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/driver/glue/sync_backend_host_impl.cc
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/driver/glue/sync_backend_host_impl.h
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine/fake_sync_engine.cc
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine/fake_sync_engine.h
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine/fake_sync_manager.cc
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine/fake_sync_manager.h
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine/sync_engine.h
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine/sync_manager.h
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine_impl/net/server_connection_manager.cc
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine_impl/net/server_connection_manager.h
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine_impl/sync_manager_impl.cc
[modify] https://crrev.com/27133efece77f85202f484e1518260d83016685e/components/sync/engine_impl/sync_manager_impl.h

Copying from https://bugs.chromium.org/p/chromium/issues/detail?id=834720#c16:

Closing the loop, we now have some data:

99.85% of token revocation requests complete (either with success or failure).

Among the requests that complete:
* ~95% succeed

* ~5% are a no-op because the token is already revoked (invalid_token)

* 0.15% fail with "Invalid Request". I don't know if these are actual failures or just mean that the token was already revoked (like above)

* 0.14% fail for other reasons, mostly for connection-related errors (Connection failed, Connection canceled, Connection timeout)

So that means that 0.14%-0.29% of the time (depending on what "Invalid Request" means), it's possible for the access token to be valid after the refresh token is removed.

Yanan - WDYT? Do you think a server-side solution is worth persuing?

Sign in to add a comment