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

Issue 895010 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Have all ListAccounts calls from Chrome have a common prefix for the source=

Project Member Reported by nickk@google.com, Oct 12

Issue description

Currently, all ListAccounts callers can provide their own source parameter (see [1]).
On the server side, we'd like to be able easily exclude all (known) browser-initiated traffic to this endpoint when doing analysis, so having one common prefix would make that a lot easier. Currently we'd need to exclude sources starting with (Chromium|durations_metrics|SyncAuthManager|ProfileChooserView|child_account_service), and have to extend that list it as more parts of Chrome start using it.



[1] https://cs.chromium.org/chromium/src/components/signin/core/browser/gaia_cookie_manager_service.cc?l=503&rcl=5b50594018f5d39b87d9dbe390c900b4560c2d46


 
Project Member

Comment 1 by sheriffbot@chromium.org, Oct 29

Status: Available (was: Untriaged)
--Chrome Identity automated triaging--

This bug is Untriaged and has gone for two weeks without any activity, so it is being moved to Available. Please see https://goo.gl/78kbny for more details. Please remove the Services>SignIn or UI>Browser>Profiles components if this bug isn't related to Chrome Identity.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Owner: droger@chromium.org
Status: Assigned (was: Available)
Putting it in David's queue of bugs.
nick: can I just make the change in Chrome or do we need a specific rollout strategy to avoid breaking the servers?
As far as I'm aware, it should be OK to just make the change. We don't rely on it beyond metrics & logs analysis.
I see that several calls already use a "Chromium" prefix. I will add a Chromium prefix to all calls that don't already have one, as this will be simplest the least risky change.

nick: is this only for ListAccounts or should we try to do this for all Gaia calls? Maybe that's riskier though, because I know that in some cases server logic depends on the source value (see b/110375876 for an example).
Cc: yananj@google.com
+Yanan to answer how risky/safe that would be. I'd expect adding 'Chromium' as prefix should be OK for all cases though.
Are there places beyond Dice/Mirror that actually use the other Gaia calls? I'd be worried about random Chrome code calling Logout for example.

I looked a bit more at the code, and we would be OK for Dice to have everything else also use a Chromium prefix since we only look at the prefix for M67. However for Mirror we always only look at the prefix, so if non-Mirror Chrome code started using that for non-ListAccounts calls we might introduce bugs.
I'm not 100% sure I caught everything, but apart from list account there are not many calls.

Logout is only called by Mirror and Dice.

uber auth token exchange (used by merge session internally) and CheckConnection info use a source, but it is always "ChromiumBrowser" currently which already starts with Chromium.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 13

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

commit 092f8caba5930ad2730aaad5139d6030ccc946de
Author: David Roger <droger@chromium.org>
Date: Tue Nov 13 10:02:17 2018

[signin] Remove source parameter in GaiaCookieManagerService

All callers were using the same source, it is now hardcoded.

TBR=treib, eugenebut

Bug:  895010 
Change-Id: Ib068188ac152f39d6f552644c373cf9103d56087
Reviewed-on: https://chromium-review.googlesource.com/c/1329621
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607541}
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/chrome/browser/signin/dice_response_handler_unittest.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/chrome/browser/signin/fake_gaia_cookie_manager_service_builder.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/chrome/browser/signin/gaia_cookie_manager_service_factory.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/browser_sync/profile_sync_test_util.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/search_provider_logos/logo_service_impl_unittest.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/core/browser/account_investigator_unittest.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/core/browser/account_reconcilor_unittest.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/core/browser/fake_gaia_cookie_manager_service.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/core/browser/fake_gaia_cookie_manager_service.h
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/core/browser/gaia_cookie_manager_service.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/core/browser/gaia_cookie_manager_service.h
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/core/browser/signin_manager_unittest.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/core/browser/signin_tracker_unittest.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/components/signin/ios/browser/account_consistency_service_unittest.mm
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/google_apis/gaia/ubertoken_fetcher.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/google_apis/gaia/ubertoken_fetcher.h
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/ios/chrome/browser/signin/fake_gaia_cookie_manager_service_builder.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/ios/chrome/browser/signin/gaia_cookie_manager_service_factory.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/ios/web_view/internal/signin/web_view_gaia_cookie_manager_service_factory.mm
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/ios/web_view/internal/sync/cwv_sync_controller_unittest.mm
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/services/identity/public/cpp/identity_manager_unittest.cc
[modify] https://crrev.com/092f8caba5930ad2730aaad5139d6030ccc946de/services/identity/public/cpp/identity_test_environment.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 16

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

commit 09d76ec9286791185aa11d9d25c8704e3a8019ab
Author: David Roger <droger@chromium.org>
Date: Fri Nov 16 17:33:51 2018

[signin] Add a "Chromium" prefix to the Gaia source parameter

To help the server team distinguish better the traffic coming from
Chrome this CL adds a "Chromium" prefix to all Gaia sources in Chrome.
A lot of sources already had this prefix, but some did not.

This CL also removes the source parameter from ListAccounts calls.
This source was not consistently used (because ListAccounts often
returns a cached response), and was not really meaningful.
ListAccounts now always use the generic GaiaSource::kChrome.

TBR=treib, xiyuan, isherman, poromov, eugenebut

Bug:  895010 
Change-Id: I8b3837d6ddf1d04cae252f382c2dee7b99f93c75
Reviewed-on: https://chromium-review.googlesource.com/c/1329971
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608832}
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/chromeos/arc/auth/arc_auth_context.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/chromeos/login/signin/oauth2_login_verifier.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/chromeos/login/signin/oauth2_token_fetcher.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/extensions/api/identity/gaia_web_auth_flow.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/metrics/desktop_session_duration/desktop_profile_session_durations_service.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/signin/chrome_signin_client.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/signin/chrome_signin_client.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/signin/dice_response_handler.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/signin/dice_response_handler_unittest.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/signin/signin_ui_util.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/signin/token_revoker_test_utils.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/supervised_user/child_accounts/child_account_service.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/supervised_user/child_accounts/child_account_service_unittest.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/ui/webui/identity_internals_ui.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/ui/webui/signin/inline_login_handler_chromeos.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chrome/browser/ui/webui/signin_internals_ui.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/chromeos/account_manager/account_manager.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/browser_sync/sync_auth_manager.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/search_provider_logos/logo_service_impl_unittest.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/account_investigator.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/account_reconcilor.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/account_reconcilor_delegate.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/account_reconcilor_delegate.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/account_reconcilor_unittest.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/child_account_info_fetcher_impl.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/dice_account_reconcilor_delegate.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/dice_account_reconcilor_delegate.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/fake_gaia_cookie_manager_service.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/fake_gaia_cookie_manager_service.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/gaia_cookie_manager_service.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/gaia_cookie_manager_service.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/mirror_account_reconcilor_delegate.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/mirror_account_reconcilor_delegate.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/signin_client.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/signin_manager.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/test_signin_client.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/components/signin/core/browser/test_signin_client.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/google_apis/gaia/gaia_auth_fetcher.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/google_apis/gaia/gaia_auth_fetcher.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/google_apis/gaia/gaia_auth_fetcher_unittest.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/google_apis/gaia/ubertoken_fetcher.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/google_apis/gaia/ubertoken_fetcher.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/google_apis/gaia/ubertoken_fetcher_unittest.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/ios/chrome/browser/signin/gaia_auth_fetcher_ios.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/ios/chrome/browser/signin/gaia_auth_fetcher_ios.mm
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/ios/chrome/browser/signin/gaia_auth_fetcher_ios_unittest.mm
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/ios/chrome/browser/signin/ios_chrome_signin_client.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/ios/chrome/browser/signin/ios_chrome_signin_client.mm
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/ios/chrome/browser/ui/webui/signin_internals_ui_ios.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/ios/web_view/internal/signin/ios_web_view_signin_client.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/ios/web_view/internal/signin/ios_web_view_signin_client.mm
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/services/identity/public/cpp/identity_manager.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/services/identity/public/cpp/identity_manager.h
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/services/identity/public/cpp/identity_manager_unittest.cc
[modify] https://crrev.com/09d76ec9286791185aa11d9d25c8704e3a8019ab/services/identity/public/cpp/identity_test_utils.cc

Status: Fixed (was: Assigned)
From Chrome 72, all sources for ListAccounts, Merge session, Logout, multilogin will start with "Chromium" or be "chromeos".
In particular ListAccounts should now always use "ChromiumBrowser" as source.
Thank you!

Sign in to add a comment