Identity Service: Design observer API |
|||||||||||||
Issue descriptionThis interface should be backed by the Identity Service impl being an observer of PO2TS, per discussion with msarda@. Note that there are similar-but-not-identical observer interfaces in the current codebase, e.g. being an observer of the AccountTracker. Client code will need to be ported to observing PO2TS before being ported to the Identity Service in order to be clear that all the migrations are correct. We will call out this fact in documentation when adding this observer interface to the Identity Service.
,
Jun 5 2017
,
Jun 5 2017
,
Jun 6 2017
,
Jun 6 2017
Note that some customers need the information of whether the account for which the refresh token was just made available/revoked is the authenticated account; see crbug.com/729966 for an example.
,
Jun 8 2017
,
Jun 8 2017
,
Jun 19 2017
,
Jun 28 2017
,
Sep 27 2017
After struggling with the design of this API just for the purposes of the chrome.identity.onSignInEventChanged() event implementation, I realized that is not a straightforward design problem. Conversations with bsazanov@ have also brought light the knowledge that we have to consider that the Android system API provides only coarse-grained updates in designing this API. I think that it likely makes sense for us to find a use case or two for the observer API on mobile if possible, design the API for those use cases + Chrome identity API, and then implement that API for all of those use cases.
,
Nov 1 2017
,
Nov 7 2017
,
May 24 2018
Starting this now as it's needed for ongoing conversions (sync, GCM, PrimaryAccountAccessTokenFetcher). Going to start off with a very simple design and then we can easily iterate as needed.
,
May 24 2018
,
May 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d7df96d788fd687a7c2ab2af0fb57d5d2206cc71 commit d7df96d788fd687a7c2ab2af0fb57d5d2206cc71 Author: Colin Blundell <blundell@chromium.org> Date: Thu May 31 14:16:54 2018 Add DiagnosticsClient to ProfileOAuth2TokenService Similar to what https://chromium-review.googlesource.com/883347 did for SigninManager, this CL adds a ProfileOAuth2TokenService::DiagnosticsClient interface. This interface will be used to ensure that IdentityManager gets information on token-related events before observers of PO2TS while the codebase is being converted to use IdentityManager directly. Concretely, this client interface allows IdentityManager to be informed of refresh token availability/revocation before ProfileOAuth2TokenService clients, thus ensuring that IdentityManager has a consistent view of token state when any PO2TS observers receive these events. One subtlety here is exactly where to place the calls informing the diagnostics client that these events will be fired. ProfileOAuth2TokenService actually itself observes its base class OAuth2TokenService, and it is a necessary constraint that PO2TS receive this event *before* any other O2TS observer (including IdentityManager). Hence, we place these diagnostic calls within ProfileOAuth2TokenService's implementations of the observer callbacks. An upcoming CL will use this newly-introduced interface to add an IdentityManager API wherein its clients can be notified of changes in token state. Bug: 729540 Change-Id: Ic9be84a31faa259d0e9b971a16311a9492ca7b97 Reviewed-on: https://chromium-review.googlesource.com/1075336 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#563218} [modify] https://crrev.com/d7df96d788fd687a7c2ab2af0fb57d5d2206cc71/components/signin/core/browser/profile_oauth2_token_service.cc [modify] https://crrev.com/d7df96d788fd687a7c2ab2af0fb57d5d2206cc71/components/signin/core/browser/profile_oauth2_token_service.h
,
May 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f11dfdeba0d29aa1b85256a8e545101dd2adbd2a commit f11dfdeba0d29aa1b85256a8e545101dd2adbd2a Author: Colin Blundell <blundell@chromium.org> Date: Thu May 31 17:05:12 2018 Plumb AccountTrackerService into IdentityManager IdentityManager will need AccountTrackerService in an upcoming CL in order to map account IDs received from ProfileOAuth2TokenService into their corresponding AccountInfo objects. ProfileOAuth2TokenService does not itself know about AccountTrackerService; while it would be possible to have PO2TS take in AccountTrackerService and send out events with AccountInfo objects, (1) it would be a bigger bear of a change to make, and (2) IdentityManager will almost certainly have to observe AccountTrackerService eventually. TBR=jzw@chromium.org Bug: 729540 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I6b4aa2bb8993d748013a74ab84d52e7a42733037 Reviewed-on: https://chromium-review.googlesource.com/1075588 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/heads/master@{#563285} [modify] https://crrev.com/f11dfdeba0d29aa1b85256a8e545101dd2adbd2a/chrome/browser/signin/identity_manager_factory.cc [modify] https://crrev.com/f11dfdeba0d29aa1b85256a8e545101dd2adbd2a/components/browser_sync/profile_sync_test_util.cc [modify] https://crrev.com/f11dfdeba0d29aa1b85256a8e545101dd2adbd2a/ios/chrome/browser/signin/identity_manager_factory.cc [modify] https://crrev.com/f11dfdeba0d29aa1b85256a8e545101dd2adbd2a/ios/web_view/internal/signin/web_view_identity_manager_factory.mm [modify] https://crrev.com/f11dfdeba0d29aa1b85256a8e545101dd2adbd2a/services/identity/public/cpp/identity_manager.cc [modify] https://crrev.com/f11dfdeba0d29aa1b85256a8e545101dd2adbd2a/services/identity/public/cpp/identity_manager.h [modify] https://crrev.com/f11dfdeba0d29aa1b85256a8e545101dd2adbd2a/services/identity/public/cpp/identity_manager_unittest.cc [modify] https://crrev.com/f11dfdeba0d29aa1b85256a8e545101dd2adbd2a/services/identity/public/cpp/identity_test_environment.cc
,
Jun 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c5ba909cf7b18fff0bf2ff26ae35d63544fde3f0 commit c5ba909cf7b18fff0bf2ff26ae35d63544fde3f0 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jun 01 07:29:42 2018 IdentityManager: Add API for observing refresh token updates/removal This CL adds an API to IdentityManager::Observer that allows observers to be notified on refresh token updates and removals. This API follows that of OAuth2TokenService::Observer and is intended for porting of clients of the latter API. One extension in the new API is that on token updates, we include the information of whether the updated token is valid or invalid. Including this information means that clients don't need to explicitly query the last authentication error associated with the account, as they do when receiving OnRefreshTokenAvailable() notifications from ProfileOAuth2TokenService. In particular, this functionality will be required immediately to port sync. Similarly to IdentityManager's interaction with SigninManager for other parts of the IdentityManager::Observer API implementation, IdentityManager implements ProfileOAuth2TokenService::DiagnosticsClient rather than OAuth2TokenService::Observer to implement the new APIs. The reason is to maintain consistency while the codebase is incrementally converted to IdentityManager. If IdentityManager updates its state in response to observing SigninManager/PO2TS, then some other observer of those classes could get called back first. That other observer could kick off a flow that ends up in querying IdentityManager (again, due to incremental conversion). If IdentityManager's view does not match that of PO2TS, the above-described flow will likely not behave as expected. (This isn't theoretical: https://bugs.chromium.org/p/chromium/issues/detail?id=804410). The production changes are relatively minimal and straightforward. The bulk of the change is in unit tests of the newly-added API. Bug: 729540 Change-Id: I67b9c0842074148845d1313a0409f055d1f81e08 Reviewed-on: https://chromium-review.googlesource.com/1076047 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#563559} [modify] https://crrev.com/c5ba909cf7b18fff0bf2ff26ae35d63544fde3f0/services/identity/public/cpp/identity_manager.cc [modify] https://crrev.com/c5ba909cf7b18fff0bf2ff26ae35d63544fde3f0/services/identity/public/cpp/identity_manager.h [modify] https://crrev.com/c5ba909cf7b18fff0bf2ff26ae35d63544fde3f0/services/identity/public/cpp/identity_manager_unittest.cc |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by blundell@chromium.org
, Jun 5 2017