Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2252243002:
Adding a policy for using the uiAccess enabled It2Me binary on Windows (Closed)
|
Created:
4 years, 4 months ago by joedow Modified:
4 years, 3 months ago CC:
chromium-reviews, tnagel+watch_chromium.org, asvitkine+watch_chromium.org, chromoting-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdding a policy for using the uiAccess enabled It2Me binary on Windows
This change adds a Chrome enterprise policy to control whether or not we use
the uiAccess enabled binary for It2Me or the binary which runs at the user's
integrity level.
BUG=617185
Committed: https://crrev.com/6f0893de17c488ead4015e8e6b222fc2fef28c0d
Cr-Commit-Position: refs/heads/master@{#415107}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Addressing CR feedback and merging with ToT #Patch Set 3 : Merging with ToT #
Total comments: 2
Patch Set 4 : Addressing CR Feedback #
Messages
Total messages: 39 (23 generated)
The CQ bit was checked by joedow@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
joedow@chromium.org changed reviewers: + sergeyu@chromium.org
PTAL! I'll add the owners for the other directories once the remoting side looks good. Thanks, Joe
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2252243002/diff/1/components/policy/resources... File components/policy/resources/policy_templates.json (right): https://codereview.chromium.org/2252243002/diff/1/components/policy/resources... components/policy/resources/policy_templates.json:1002: 'name': 'RemoteAssistanceHostAllowUiAccess', Many RemoteAccess* policies are applicable for It2Me host as well, so it may be confusing that you are adding RemoteAssistance prefix. I would prefer reusing RemoteAccess prefix for all policies. Maybe RemoteAccessHostAllowUiAccessForRemoteAssistance or RemoteAccessHostAssistanceUiAccess https://codereview.chromium.org/2252243002/diff/1/components/policy/resources... components/policy/resources/policy_templates.json:1016: If this setting is disabled or not configured, the remote assistance host will run in the user's context and remote users cannot interact with elevated windows on the desktop.''', Can we enable UI access by default?
tnagel@chromium.org changed reviewers: + tnagel@chromium.org
https://codereview.chromium.org/2252243002/diff/1/components/policy/resources... File components/policy/resources/policy_templates.json (right): https://codereview.chromium.org/2252243002/diff/1/components/policy/resources... components/policy/resources/policy_templates.json:1014: 'desc': '''If this setting is enabled, the remote assistance host will be run in a process with uiAccess permissions. This will allow remote users to interact with elevated windows on the local user's desktop. What's that uiAccess? It seems like a non-translatable term, thus please tag with <ph...> as described at the top of the file.
The CQ bit was checked by joedow@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Addressed feedback, PTAL! https://codereview.chromium.org/2252243002/diff/1/components/policy/resources... File components/policy/resources/policy_templates.json (right): https://codereview.chromium.org/2252243002/diff/1/components/policy/resources... components/policy/resources/policy_templates.json:1002: 'name': 'RemoteAssistanceHostAllowUiAccess', On 2016/08/18 05:55:51, Sergey Ulanov wrote: > Many RemoteAccess* policies are applicable for It2Me host as well, so it may be > confusing that you are adding RemoteAssistance prefix. I would prefer reusing > RemoteAccess prefix for all policies. > Maybe RemoteAccessHostAllowUiAccessForRemoteAssistance or > RemoteAccessHostAssistanceUiAccess I went back and forth on this a bit. Using the RemoteAccessHost prefix led to a much longer name than I liked, but I'm fine with it. https://codereview.chromium.org/2252243002/diff/1/components/policy/resources... components/policy/resources/policy_templates.json:1014: 'desc': '''If this setting is enabled, the remote assistance host will be run in a process with uiAccess permissions. This will allow remote users to interact with elevated windows on the local user's desktop. On 2016/08/18 11:46:17, Thiemo Nagel wrote: > What's that uiAccess? It seems like a non-translatable term, thus please tag > with <ph...> as described at the top of the file. Done. https://codereview.chromium.org/2252243002/diff/1/components/policy/resources... components/policy/resources/policy_templates.json:1016: If this setting is disabled or not configured, the remote assistance host will run in the user's context and remote users cannot interact with elevated windows on the desktop.''', On 2016/08/18 05:55:51, Sergey Ulanov wrote: > Can we enable UI access by default? We decided to disable this by default as enabling it will require an additional dialog for the user to click-through and the decision was to keep the default flow as simple as possible but expose this setting for Admins and enterprise.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
joedow@chromium.org changed reviewers: + rkaplow@chromium.org
Thanks Sergey! Could I get the owners of the other policy files so take a look? Thanks, Joe
just to double check - this is just changing the wording of the enums and is not intending to show different data, correct?
I added a new policy for our scenario and when I ran the script to update histograms.xml, several strings were updated there. My assumption is that the update script was not rerun in previous CLs when the policy descriptions were changed.
lgtm
The CQ bit was checked by joedow@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
joedow@chromium.org changed reviewers: + bartfab@chromium.org - tnagel@chromium.org
bartfab@, can you take a look at the policy change? Please note, I am planning to check this in after the M54 branch point so I have updated the policy details to M55. Thanks! Joe
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
Note: The build failures are caused by me setting the effective milestone of the policy from M54 to M55. They should go away when I merge with ToT after the branch.
joedow@chromium.org changed reviewers: + tnagel@chromium.org
Ping!
lgtm https://codereview.chromium.org/2252243002/diff/40001/components/policy/resou... File components/policy/resources/policy_templates.json (right): https://codereview.chromium.org/2252243002/diff/40001/components/policy/resou... components/policy/resources/policy_templates.json:1013: 'tags': [], It would seem that at least 'system-security' and 'admin-sharing' are appropriate here.
The CQ bit was checked by joedow@chromium.org to run a CQ dry run
Thanks all! https://codereview.chromium.org/2252243002/diff/40001/components/policy/resou... File components/policy/resources/policy_templates.json (right): https://codereview.chromium.org/2252243002/diff/40001/components/policy/resou... components/policy/resources/policy_templates.json:1013: 'tags': [], On 2016/08/29 12:57:31, bartfab (slow) wrote: > It would seem that at least 'system-security' and 'admin-sharing' are > appropriate here. I think system-security is a good call, admin-sharing doesn't quite apply as this doesn't allow administrators any additional rights/privileges.
The CQ bit was unchecked by joedow@chromium.org
The CQ bit was checked by joedow@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org, rkaplow@chromium.org, bartfab@chromium.org Link to the patchset: https://codereview.chromium.org/2252243002/#ps60001 (title: "Addressing CR Feedback")
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Adding a policy for using the uiAccess enabled It2Me binary on Windows This change adds a Chrome enterprise policy to control whether or not we use the uiAccess enabled binary for It2Me or the binary which runs at the user's integrity level. BUG=617185 ========== to ========== Adding a policy for using the uiAccess enabled It2Me binary on Windows This change adds a Chrome enterprise policy to control whether or not we use the uiAccess enabled binary for It2Me or the binary which runs at the user's integrity level. BUG=617185 Committed: https://crrev.com/6f0893de17c488ead4015e8e6b222fc2fef28c0d Cr-Commit-Position: refs/heads/master@{#415107} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/6f0893de17c488ead4015e8e6b222fc2fef28c0d Cr-Commit-Position: refs/heads/master@{#415107} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
