New issue
Advanced search Search tips

Issue 922458 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 902296
issue 903907



Sign in to add a comment

Implement an AccoutsMutator API for invalidating refresh token of the primary account

Project Member Reported by toniki...@chromium.org, Jan 16 (6 days ago)

Issue description

This came out here: https://chromium-review.googlesource.com/c/chromium/src/+/1412118/2/chrome/browser/signin/dice_response_handler.cc#357

In summary:

msarda:

I think this should have a special method in PrimaryAccountsMutator or in AccountsMutator that invalidates the account  (MutableProfileOAuth2TokenServiceDelegate::kInvalidRefreshToken should not be exposed to clients). I hesitate between these:

only the refresh token of a primary account is invalidated, so it makes sense to put in in PrimaryAccountsMutator
tokens can only be invalidated on desktop (and maybe in the future on ChromeOS), and there is no PrimaryAccountsMutator on ChromeOS. There is no PrimaryAccountsMutator on ChromeOS and it also exists on mobile. And all refresh token operations are on AccountsMutator.

Taking that into consideration, I suggest we add the method in AccountsMutator (I would call it InvalidateRefreshTokenForPrimaryAccount).

blundell:

I agree that it conceptually makes sense on AccountsMutator: PrimaryAccountMutator is about setting/clearing the primary account, whereas this is about mutating state *of* the primary account. 
 

Comment 1 by blundell@chromium.org, Jan 16 (6 days ago)

Labels: -Proj-Servicification-VendoBug Proj-Servicification-VendorBug

Comment 2 by toniki...@chromium.org, Jan 16 (6 days ago)

Blocking: 903907
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 18 (4 days ago)

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

commit 4eb7ca2abcf8e05a47a67b271f99298169c6e0e7
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Fri Jan 18 15:22:43 2019

Implement an AccoutsMutator API for invalidating refresh token of the primary account

This is an action point of the discussion from [1]. Basically, it adds
an API to AccountsMutator that invalidates the primary account refresh
today. This way, OAuth2TokenServiceDelegate::kInvalidRefreshToken does not
get exposed to clients).

AccountsMutator was chosen over PrimaryAccountMutator because the later
essentially is about setting/clearing the primary account, the use cases
of this API fit better to cases related to *mutating* state of the primary
account.

[1] https://crrev.com/c/1412118/2/chrome/browser/signin/dice_response_handler.cc#340

BUG= 922458 

Change-Id: Ia8f8d62960528e061c6fd78e0fa089d72d6729a7
Reviewed-on: https://chromium-review.googlesource.com/c/1415070
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624128}
[modify] https://crrev.com/4eb7ca2abcf8e05a47a67b271f99298169c6e0e7/chrome/browser/signin/identity_manager_factory.cc
[modify] https://crrev.com/4eb7ca2abcf8e05a47a67b271f99298169c6e0e7/services/identity/public/cpp/accounts_mutator.h
[modify] https://crrev.com/4eb7ca2abcf8e05a47a67b271f99298169c6e0e7/services/identity/public/cpp/accounts_mutator_impl.cc
[modify] https://crrev.com/4eb7ca2abcf8e05a47a67b271f99298169c6e0e7/services/identity/public/cpp/accounts_mutator_impl.h
[modify] https://crrev.com/4eb7ca2abcf8e05a47a67b271f99298169c6e0e7/services/identity/public/cpp/accounts_mutator_unittest.cc
[modify] https://crrev.com/4eb7ca2abcf8e05a47a67b271f99298169c6e0e7/services/identity/public/cpp/identity_test_environment.cc

Comment 4 by toniki...@chromium.org, Jan 18 (4 days ago)

Status: Fixed (was: Started)

Sign in to add a comment