|
|
Created:
3 years, 9 months ago by Ramin Halavati Modified:
3 years, 9 months ago CC:
chromium-reviews, pam+watch_chromium.org, Marc Treib Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionNetwork traffic annotation added to supervised users.
Network traffic annotation is added to network request of:
chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc
c/b/s/child_accounts/permission_request_creator_apiary.cc
c/b/s/experimental/safe_search_url_reporter.cc
c/b/s/legacy/supervised_user_refresh_token_fetcher.cc
BUG=656607
Review-Url: https://codereview.chromium.org/2742743004
Cr-Commit-Position: refs/heads/master@{#457424}
Committed: https://chromium.googlesource.com/chromium/src/+/36c3b2483acaa6b14987ca0a13e5fd71d40df623
Patch Set 1 #
Total comments: 94
Patch Set 2 : Annotations updated. #
Total comments: 11
Patch Set 3 : Comments addressed. #
Messages
Total messages: 18 (7 generated)
rhalavati@chromium.org changed reviewers: + bauerb@chromium.org
Bernhard, I've added network traffic annotation templates to 4 files in supervised users. Please review them and suggest content. I repeat the references here: Please take a look at the protobuf scheme in: https://cs.chromium.org/chromium/src/tools/traffic_annotation/traffic_annotat... for the definition of the annotations. You can find a sample annotation in: https://cs.chromium.org/chromium/src/components/spellcheck/browser/spelling_s... Entriprise policy options are here: https://cs.chromium.org/chromium/src/out/Debug/gen/components/policy/proto/ch... And more description on enterprise policy settings is here: http://dev.chromium.org/administrators/policy-list-3 Please tell me if you need any clarifications from my side. If you want to learn more, see: https://docs.google.com/document/d/1FWEFTHzdqIA1wHUo_DuSpJjppW22otCMPCOA5NumhJU.
+Marc FYI https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... File chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc (right): https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:176: net::DefineNetworkTrafficAnnotation("...", R"( family_info https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:178: sender: "..." supervised_users https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:179: description: "..." Fetches information about the user's family group from the Google Family API. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:180: trigger: "..." Triggered in regular intervals to update profile information. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:181: data: "..." The request is authenticated with an OAuth2 access token identifying the Google account. No other information is sent. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:182: destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER GOOGLE_OWNED_SERVICE https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:185: cookies_allowed: false/true false https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:186: cookies_store: "..." user https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:187: setting: "..." This feature is only enabled for child accounts. If sign-in is restricted to accounts from a managed domain, those accounts are not going to be child accounts. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:189: [POLICY_NAME] { RestrictSigninToPattern https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:190: policy_options {mode: MANDATORY/RECOMMENDED/UNSET} MANDATORY https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:191: [POLICY_NAME]: ... //(value to disable it) "*@manageddomain.com" https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... File chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc (right): https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:187: net::DefineNetworkTrafficAnnotation("...", R"( permission_request_creator https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:189: sender: "..." supervised_users https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:190: description: "..." Requests permission for the user to access a blocked site. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:191: trigger: "..." Initiated by the user. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:192: data: "..." The request is authenticated with an OAuth2 access token identifying the Google account and contains the URL that the user requests access to. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:196: cookies_allowed: false/true false https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:197: cookies_store: "..." user https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:198: setting: "..." This feature is only enabled for child accounts. If sign-in is restricted to accounts from a managed domain, those accounts are not going to be child accounts. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:200: [POLICY_NAME] { RestrictSigninToPattern https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:201: policy_options {mode: MANDATORY/RECOMMENDED/UNSET} MANDATORY https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:202: [POLICY_NAME]: ... //(value to disable it) "*@manageddomain.com" https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... File chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc (right): https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:113: net::DefineNetworkTrafficAnnotation("...", R"( safe_search_url_reporter https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:115: sender: "..." supervised_users https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:116: description: "..." Reports a URL wrongfully flagged by SafeSearch. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:117: trigger: "..." Initiated by the user. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:118: data: "..." The request is authenticated with an OAuth2 access token identifying the Google account and contains the URL that was wrongfully flagged. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:119: destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER GOOGLE_OWNED_SERVICE https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:122: cookies_allowed: false/true false https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:123: cookies_store: "..." user https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:124: setting: "..." This feature is only enabled for child accounts. If sign-in is restricted to accounts from a managed domain, those accounts are not going to be child accounts. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:126: [POLICY_NAME] { RestrictSigninToPattern https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:127: policy_options {mode: MANDATORY/RECOMMENDED/UNSET} MANDATORY https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:128: [POLICY_NAME]: ... //(value to disable it) "*@manageddomain.com" https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... File chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc (right): https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:166: net::DefineNetworkTrafficAnnotation("...", R"( supervised_user_refresh_token_fetcher https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:168: sender: "..." supervised_users https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:169: description: "..." Fetches an OAuth2 refresh token scoped down to the supervised user Sync scope and tied to the given Supervised User ID, identifying the Supervised User Profile to be created. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:170: trigger: "..." Called when creating a new Supervised User profile in Chrome to fetch OAuth credentials for using Sync with new profile. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:171: data: "..." The request is authenticated with an OAuth2 access token identifying the Google account and contains the following information: * The Supervised User ID, a randomly generated 64-bit identifier for the profile * The device name, to identify the refresh token in account management https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:172: destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER GOOGLE_OWNED_SERVICE https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:175: cookies_allowed: false/true false https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:176: cookies_store: "..." user https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:177: setting: "..." Adding new users from the user manager can be disabled in chrome://settings/. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:179: [POLICY_NAME] { SupervisedUserCreationEnabled https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:180: policy_options {mode: MANDATORY/RECOMMENDED/UNSET} MANDATORY https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:181: [POLICY_NAME]: ... //(value to disable it) false
Annotations updated, please review. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... File chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc (right): https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:176: net::DefineNetworkTrafficAnnotation("...", R"( On 2017/03/14 10:02:48, Bernhard Bauer wrote: > family_info Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:178: sender: "..." On 2017/03/14 10:02:48, Bernhard Bauer wrote: > supervised_users Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:179: description: "..." On 2017/03/14 10:02:47, Bernhard Bauer wrote: > Fetches information about the user's family group from the Google Family API. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:180: trigger: "..." On 2017/03/14 10:02:48, Bernhard Bauer wrote: > Triggered in regular intervals to update profile information. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:181: data: "..." On 2017/03/14 10:02:47, Bernhard Bauer wrote: > The request is authenticated with an OAuth2 access token identifying the Google > account. No other information is sent. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:182: destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER On 2017/03/14 10:02:47, Bernhard Bauer wrote: > GOOGLE_OWNED_SERVICE Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:185: cookies_allowed: false/true On 2017/03/14 10:02:47, Bernhard Bauer wrote: > false Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:186: cookies_store: "..." On 2017/03/14 10:02:48, Bernhard Bauer wrote: > user Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:187: setting: "..." On 2017/03/14 10:02:48, Bernhard Bauer wrote: > This feature is only enabled for child accounts. If sign-in is restricted to > accounts from a managed domain, those accounts are not going to be child > accounts. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:189: [POLICY_NAME] { On 2017/03/14 10:02:48, Bernhard Bauer wrote: > RestrictSigninToPattern Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:190: policy_options {mode: MANDATORY/RECOMMENDED/UNSET} On 2017/03/14 10:02:48, Bernhard Bauer wrote: > MANDATORY Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:191: [POLICY_NAME]: ... //(value to disable it) On 2017/03/14 10:02:48, Bernhard Bauer wrote: > mailto:"*@manageddomain.com" Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... File chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc (right): https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:187: net::DefineNetworkTrafficAnnotation("...", R"( On 2017/03/14 10:02:49, Bernhard Bauer wrote: > permission_request_creator Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:189: sender: "..." On 2017/03/14 10:02:49, Bernhard Bauer wrote: > supervised_users Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:190: description: "..." On 2017/03/14 10:02:49, Bernhard Bauer wrote: > Requests permission for the user to access a blocked site. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:191: trigger: "..." On 2017/03/14 10:02:48, Bernhard Bauer wrote: > Initiated by the user. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:192: data: "..." On 2017/03/14 10:02:49, Bernhard Bauer wrote: > The request is authenticated with an OAuth2 access token identifying the Google > account and contains the URL that the user requests access to. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:196: cookies_allowed: false/true On 2017/03/14 10:02:48, Bernhard Bauer wrote: > false Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:197: cookies_store: "..." On 2017/03/14 10:02:49, Bernhard Bauer wrote: > user Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:198: setting: "..." On 2017/03/14 10:02:48, Bernhard Bauer wrote: > This feature is only enabled for child accounts. If sign-in is restricted to > accounts from a managed domain, those accounts are not going to be child > accounts. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:200: [POLICY_NAME] { On 2017/03/14 10:02:48, Bernhard Bauer wrote: > RestrictSigninToPattern Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:201: policy_options {mode: MANDATORY/RECOMMENDED/UNSET} On 2017/03/14 10:02:49, Bernhard Bauer wrote: > MANDATORY Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:202: [POLICY_NAME]: ... //(value to disable it) On 2017/03/14 10:02:49, Bernhard Bauer wrote: > mailto:"*@manageddomain.com" Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... File chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc (right): https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:113: net::DefineNetworkTrafficAnnotation("...", R"( On 2017/03/14 10:02:49, Bernhard Bauer wrote: > safe_search_url_reporter Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:115: sender: "..." On 2017/03/14 10:02:49, Bernhard Bauer wrote: > supervised_users Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:116: description: "..." On 2017/03/14 10:02:49, Bernhard Bauer wrote: > Reports a URL wrongfully flagged by SafeSearch. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:117: trigger: "..." On 2017/03/14 10:02:49, Bernhard Bauer wrote: > Initiated by the user. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:118: data: "..." On 2017/03/14 10:02:49, Bernhard Bauer wrote: > The request is authenticated with an OAuth2 access token identifying the Google > account and contains the URL that was wrongfully flagged. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:119: destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER On 2017/03/14 10:02:50, Bernhard Bauer wrote: > GOOGLE_OWNED_SERVICE Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:122: cookies_allowed: false/true On 2017/03/14 10:02:49, Bernhard Bauer wrote: > false Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:123: cookies_store: "..." On 2017/03/14 10:02:49, Bernhard Bauer wrote: > user Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:124: setting: "..." On 2017/03/14 10:02:50, Bernhard Bauer wrote: > This feature is only enabled for child accounts. If sign-in is restricted to > accounts from a managed domain, those accounts are not going to be child > accounts. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:126: [POLICY_NAME] { On 2017/03/14 10:02:50, Bernhard Bauer wrote: > RestrictSigninToPattern Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:127: policy_options {mode: MANDATORY/RECOMMENDED/UNSET} On 2017/03/14 10:02:49, Bernhard Bauer wrote: > MANDATORY Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc:128: [POLICY_NAME]: ... //(value to disable it) On 2017/03/14 10:02:50, Bernhard Bauer wrote: > mailto:"*@manageddomain.com" Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... File chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc (right): https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:166: net::DefineNetworkTrafficAnnotation("...", R"( On 2017/03/14 10:02:50, Bernhard Bauer wrote: > supervised_user_refresh_token_fetcher Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:168: sender: "..." On 2017/03/14 10:02:50, Bernhard Bauer wrote: > supervised_users Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:169: description: "..." On 2017/03/14 10:02:50, Bernhard Bauer wrote: > Fetches an OAuth2 refresh token scoped down to the supervised user Sync scope > and tied to the given Supervised User ID, identifying the Supervised User > Profile to be created. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:170: trigger: "..." On 2017/03/14 10:02:50, Bernhard Bauer wrote: > Called when creating a new Supervised User profile in Chrome to fetch OAuth > credentials for using Sync with new profile. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:171: data: "..." On 2017/03/14 10:02:51, Bernhard Bauer wrote: > The request is authenticated with an OAuth2 access token identifying the Google > account and contains the following information: > * The Supervised User ID, a randomly generated 64-bit identifier for the profile > * The device name, to identify the refresh token in account management Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:172: destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER On 2017/03/14 10:02:50, Bernhard Bauer wrote: > GOOGLE_OWNED_SERVICE Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:175: cookies_allowed: false/true On 2017/03/14 10:02:50, Bernhard Bauer wrote: > false Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:176: cookies_store: "..." On 2017/03/14 10:02:50, Bernhard Bauer wrote: > user Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:177: setting: "..." On 2017/03/14 10:02:50, Bernhard Bauer wrote: > Adding new users from the user manager can be disabled in chrome://settings/. Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:179: [POLICY_NAME] { On 2017/03/14 10:02:50, Bernhard Bauer wrote: > SupervisedUserCreationEnabled Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:180: policy_options {mode: MANDATORY/RECOMMENDED/UNSET} On 2017/03/14 10:02:50, Bernhard Bauer wrote: > MANDATORY Done. https://codereview.chromium.org/2742743004/diff/1/chrome/browser/supervised_u... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:181: [POLICY_NAME]: ... //(value to disable it) On 2017/03/14 10:02:50, Bernhard Bauer wrote: > false Done. https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... File chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc (right): https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:198: RestrictSigninToPattern: "*@manageddomain.com" I am a little unclear on this. Here we want the value that prevents this request. Is it correct?
lgtm https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... File chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc (right): https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:198: RestrictSigninToPattern: "*@manageddomain.com" On 2017/03/14 10:56:21, Ramin Halavati wrote: > I am a little unclear on this. Here we want the value that prevents this > request. Is it correct? Yes, because managed domains and child accounts are mutually exclusive. The way to disable features for child accounts is to disallow sign-in with the account (on desktop directly in Chrome, or on Android via Google Play Services, because that is where the credentials are managed).
rhalavati@chromium.org changed reviewers: + battre@chromium.org, msramek@chromium.org
Thank you. treib@, batte@, msramek@: Do you have any comments? https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... File chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc (right): https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc:198: RestrictSigninToPattern: "*@manageddomain.com" On 2017/03/14 11:16:12, Bernhard Bauer wrote: > On 2017/03/14 10:56:21, Ramin Halavati wrote: > > I am a little unclear on this. Here we want the value that prevents this > > request. Is it correct? > > Yes, because managed domains and child accounts are mutually exclusive. The way > to disable features for child accounts is to disallow sign-in with the account > (on desktop directly in Chrome, or on Android via Google Play Services, because > that is where the credentials are managed). Thank you.
treib@chromium.org changed reviewers: + treib@chromium.org
https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... File chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc (right): https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:202: "This feature cannot be disabled in settings and is is only " s/is is/is/ https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... File chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc (right): https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:171: "Fetches an OAuth2 refresh token scoped down to the supervised " nit: Capitalize "Supervised User" (for consistency) https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:176: "fetch OAuth credentials for using Sync with new profile." s/with new/with the new/ https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:193: RestrictSigninToPattern: false I think this should be SupervisedUserCreationEnabled: false
Comments addressed, please review. https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... File chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc (right): https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc:202: "This feature cannot be disabled in settings and is is only " On 2017/03/14 11:52:46, Marc Treib wrote: > s/is is/is/ Done. https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... File chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc (right): https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:171: "Fetches an OAuth2 refresh token scoped down to the supervised " On 2017/03/14 11:52:46, Marc Treib wrote: > nit: Capitalize "Supervised User" (for consistency) Done. https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:176: "fetch OAuth credentials for using Sync with new profile." On 2017/03/14 11:52:46, Marc Treib wrote: > s/with new/with the new/ Done. https://codereview.chromium.org/2742743004/diff/20001/chrome/browser/supervis... chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc:193: RestrictSigninToPattern: false On 2017/03/14 11:52:46, Marc Treib wrote: > I think this should be > SupervisedUserCreationEnabled: false Done.
lgtm
LGTM
The CQ bit was checked by rhalavati@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org Link to the patchset: https://codereview.chromium.org/2742743004/#ps40001 (title: "Comments addressed.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489670188745590, "parent_rev": "2b4b5dbb199aec6e25df8de0579eb62e3961bee4", "commit_rev": "36c3b2483acaa6b14987ca0a13e5fd71d40df623"}
Message was sent while issue was closed.
Description was changed from ========== Network traffic annotation added to supervised users. Network traffic annotation is added to network request of: chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc c/b/s/child_accounts/permission_request_creator_apiary.cc c/b/s/experimental/safe_search_url_reporter.cc c/b/s/legacy/supervised_user_refresh_token_fetcher.cc BUG=656607 ========== to ========== Network traffic annotation added to supervised users. Network traffic annotation is added to network request of: chrome/browser/supervised_user/child_accounts/family_info_fetcher.cc c/b/s/child_accounts/permission_request_creator_apiary.cc c/b/s/experimental/safe_search_url_reporter.cc c/b/s/legacy/supervised_user_refresh_token_fetcher.cc BUG=656607 Review-Url: https://codereview.chromium.org/2742743004 Cr-Commit-Position: refs/heads/master@{#457424} Committed: https://chromium.googlesource.com/chromium/src/+/36c3b2483acaa6b14987ca0a13e5... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/36c3b2483acaa6b14987ca0a13e5... |