Fail certain actions on non-AD devices from AD accounts and vice versa |
|||||
Issue descriptionIf we ever try to load an AD user on a device that doesn't have AD management baked into the install attributes, we just bail on management and instead give the user an unmanaged session: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc?rcl=eec9c47b685afb76aeb26234f0cb9c46ae0aa007&l=190 What's the motivation for that behavior? Shouldn't we instead crash the browser (CHECK(IsActiveDirectoryManaged()) since that's a security issue? Or at the very least try to exit the user session via AttemptUserExit()? -atw Doesn't the code ensure that AD users can only be created on AD enrolled devices (see GaiaScreenHandler::DoAdAuth)? In that case I agree this should be a CHECK(). Same for the DCHECK in AuthPolicyClientImpl::RefreshUserPolicy. Fetching AD user policy for a non-AD user seems like a security issue. - Lutz
,
May 17 2017
> Same for the DCHECK in AuthPolicyClientImpl::RefreshUserPolicy. Fetching AD user > policy for a non-AD user seems like a security issue. DCHECK() is sufficient, imho, since authpolicyd doesn't even run on non-AD-managed devcies.
,
May 17 2017
,
May 17 2017
Make this a CHECK please, because if we ever hit this code it's a policy escape.
,
May 17 2017
Re #4: Comment #2 was referring to a different piece of code, not the one that you brought up.
,
May 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/13bf054add652e093b9519088e2998174e29fee3 commit 13bf054add652e093b9519088e2998174e29fee3 Author: tnagel <tnagel@chromium.org> Date: Wed May 17 15:13:34 2017 CHECK() that AD user policy is only created on AD-managed devices There's other code that prevents AD users from logging in to non-AD devices but it's clearer to have a CHECK() in UserPolicyManagerFactoryChromeOS::CreateManagerForProfile() instead of just returning an empty ConfigurationPolicyProvider. BUG= 722374 Review-Url: https://codereview.chromium.org/2886133002 Cr-Commit-Position: refs/heads/master@{#472456} [modify] https://crrev.com/13bf054add652e093b9519088e2998174e29fee3/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc
,
May 17 2017
,
Jun 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf77209685ba07195bf0c0ba84c2e1d1a78f4491 commit bf77209685ba07195bf0c0ba84c2e1d1a78f4491 Author: tnagel <tnagel@chromium.org> Date: Mon Jun 12 13:06:24 2017 UserPolicyManagerFactoryChromeOS: Elaborate comment BUG= 722374 Review-Url: https://codereview.chromium.org/2891933002 Cr-Commit-Position: refs/heads/master@{#478598} [modify] https://crrev.com/bf77209685ba07195bf0c0ba84c2e1d1a78f4491/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc
,
Aug 1 2017
,
Jan 22 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by tnagel@chromium.org
, May 17 2017Owner: tnagel@chromium.org