Commit daacf7e3 authored by Yusuf Sengul's avatar Yusuf Sengul Committed by Commit Bot

Specify the reason for auth enforce reason

When GCPW login is enforced, the reason may be due to not enrolled with mdm, missing encrypted password or expired session token. This change intends to reflect the reason in login screen instead of providing a generic message.

Bug: 1014658
Change-Id: I92f4068ce71c8517fc41728c8909d86d736e2dc1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1862854
Commit-Queue: Yusuf Sengul <yusufsn@google.com>
Reviewed-by: default avatarTien Mai <tienmai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707708}
parent 592ee0b9
......@@ -278,7 +278,7 @@ bool AssociatedUserValidator::DenySigninForUsersWithInvalidTokenHandles(
continue;
// Note that logon hours cannot be changed on domain joined AD user account.
if (!IsTokenHandleValidForUserInternal(sid) &&
if (GetAuthEnforceReason(sid) != EnforceAuthReason::NOT_ENFORCED &&
!manager->IsUserDomainJoined(sid)) {
LOGFN(INFO) << "Revoking access for sid=" << sid;
HRESULT hr = ModifyUserAccess(policy, sid, false);
......@@ -441,14 +441,15 @@ void AssociatedUserValidator::StartTokenValidityQuery(
bool AssociatedUserValidator::IsTokenHandleValidForUser(
const base::string16& sid) {
base::AutoLock locker(validator_lock_);
return IsTokenHandleValidForUserInternal(sid);
return GetAuthEnforceReason(sid) ==
AssociatedUserValidator::EnforceAuthReason::NOT_ENFORCED;
}
bool AssociatedUserValidator::IsTokenHandleValidForUserInternal(
const base::string16& sid) {
AssociatedUserValidator::EnforceAuthReason
AssociatedUserValidator::GetAuthEnforceReason(const base::string16& sid) {
// All token handles are valid when no internet connection is available.
if (!HasInternetConnection())
return true;
return AssociatedUserValidator::EnforceAuthReason::NOT_ENFORCED;
// If at this point there is no token info entry for this user, assume the
// user is not associated and does not need a token handle and is thus always
......@@ -462,12 +463,12 @@ bool AssociatedUserValidator::IsTokenHandleValidForUserInternal(
auto validity_it = user_to_token_handle_info_.find(sid);
if (validity_it == user_to_token_handle_info_.end())
return true;
return AssociatedUserValidator::EnforceAuthReason::NOT_ENFORCED;
// If mdm enrollment is needed, then force a reauth for all users so
// that they enroll.
if (NeedsToEnrollWithMdm())
return false;
return AssociatedUserValidator::EnforceAuthReason::NOT_ENROLLED_WITH_MDM;
if (MdmPasswordRecoveryEnabled()) {
base::string16 store_key = GetUserPasswordLsaStoreKey(sid);
......@@ -476,7 +477,8 @@ bool AssociatedUserValidator::IsTokenHandleValidForUserInternal(
LOGFN(INFO) << "Enforcing re-auth due to missing password lsa store "
"data for user "
<< sid;
return false;
return AssociatedUserValidator::EnforceAuthReason::
MISSING_PASSWORD_RECOVERY_INFO;
}
}
......@@ -519,7 +521,9 @@ bool AssociatedUserValidator::IsTokenHandleValidForUserInternal(
: now;
}
return validity_it->second->is_valid;
return validity_it->second->is_valid
? AssociatedUserValidator::EnforceAuthReason::NOT_ENFORCED
: AssociatedUserValidator::EnforceAuthReason::INVALID_TOKEN_HANDLE;
}
void AssociatedUserValidator::BlockDenyAccessUpdate() {
......
......@@ -98,6 +98,18 @@ class AssociatedUserValidator {
// needs to complete before the function returns.
bool IsTokenHandleValidForUser(const base::string16& sid);
enum EnforceAuthReason {
NOT_ENFORCED = 0,
NOT_ENROLLED_WITH_MDM,
MISSING_PASSWORD_RECOVERY_INFO,
INVALID_TOKEN_HANDLE
};
// Returns the reason for enforcing authentication for the provided |sid|.
// This function is blocking and may fire off a query for a token handle that
// needs to complete before the function returns.
EnforceAuthReason GetAuthEnforceReason(const base::string16& sid);
// Checks if user access blocking is enforced given the usage scenario (and
// other registry based checks).
bool IsUserAccessBlockingEnforced(
......@@ -148,8 +160,6 @@ class AssociatedUserValidator {
void ForceRefreshTokenHandlesForTesting();
private:
bool IsTokenHandleValidForUserInternal(const base::string16& sid);
void CheckTokenHandleValidity(
const std::map<base::string16, base::string16>& handles_to_verify);
void StartTokenValidityQuery(const base::string16& sid,
......
......@@ -94,8 +94,14 @@
<message name="IDS_REAUTH_FID_DESCRIPTION" desc="">
Your session has expired. Sign in using your Google work account.
</message>
<message name="IDS_REAUTH_NOT_ENROLLED_WITH_MDM_FID_DESCRIPTION" desc="">
Your device needs to enroll with device management. Sign in using your Google work account.
</message>
<message name="IDS_REAUTH_MISSING_PASSWORD_RECOVERY_INFO_FID_DESCRIPTION" desc="">
Your device is missing recovery info. Sign in using your Google work account.
</message>
<message name="IDS_REAUTH_AD_NO_USER_FID_DESCRIPTION" desc="">
Sign in using your gsuite work account.
Sign in using your GSuite work account.
</message>
<message name="IDS_AUTH_FID_PROVIDER_LABEL" desc="">
Add work account
......@@ -146,7 +152,7 @@
Password
</message>
<message name="IDS_EMPTY_ACCESS_TOKEN" desc="">
Only Gsuite Enterprise users are allowed to login.
Only GSuite Enterprise users are allowed to login.
</message>
<message name="IDS_INVALID_AD_UPN" desc="">
No Domain user could be found for your account. Please contact your administrator.
......
......@@ -103,7 +103,22 @@ HRESULT CReauthCredential::GetStringValueImpl(DWORD field_id, wchar_t** value) {
OSUserManager::Get()->IsUserDomainJoined(sid)) {
description_label_id = IDS_REAUTH_AD_NO_USER_FID_DESCRIPTION_BASE;
} else {
description_label_id = IDS_REAUTH_FID_DESCRIPTION_BASE;
auto auth_enforce_reason =
AssociatedUserValidator::Get()->GetAuthEnforceReason(sid);
switch (auth_enforce_reason) {
case AssociatedUserValidator::EnforceAuthReason::NOT_ENROLLED_WITH_MDM:
description_label_id =
IDS_REAUTH_NOT_ENROLLED_WITH_MDM_FID_DESCRIPTION_BASE;
break;
case AssociatedUserValidator::EnforceAuthReason::
MISSING_PASSWORD_RECOVERY_INFO:
description_label_id =
IDS_REAUTH_MISSING_PASSWORD_RECOVERY_INFO_FID_DESCRIPTION_BASE;
break;
default:
description_label_id = IDS_REAUTH_FID_DESCRIPTION_BASE;
break;
}
}
base::string16 label(GetStringResource(description_label_id));
......
......@@ -9,10 +9,12 @@
#include "base/json/json_writer.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/test_reg_util_win.h"
#include "build/branding_buildflags.h"
#include "chrome/browser/ui/startup/credential_provider_signin_dialog_win_test_data.h"
#include "chrome/credential_provider/common/gcp_strings.h"
#include "chrome/credential_provider/gaiacp/gaia_credential_provider_i.h"
#include "chrome/credential_provider/gaiacp/gaia_resources.h"
#include "chrome/credential_provider/gaiacp/mdm_utils.h"
#include "chrome/credential_provider/gaiacp/reauth_credential.h"
#include "chrome/credential_provider/gaiacp/reg_utils.h"
#include "chrome/credential_provider/test/com_fakes.h"
......@@ -33,11 +35,14 @@ class GcpReauthCredentialTest : public ::testing::Test {
void SetUp() override;
registry_util::RegistryOverrideManager registry_override_;
FakeInternetAvailabilityChecker fake_internet_checker_;
FakeOSUserManager fake_os_user_manager_;
FakeScopedLsaPolicyFactory fake_scoped_lsa_policy_factory_;
};
void GcpReauthCredentialTest::SetUp() {
fake_internet_checker_.SetHasInternetConnection(
FakeInternetAvailabilityChecker::kHicForceYes);
InitializeRegistryOverrideForTesting(&registry_override_);
}
......@@ -156,6 +161,110 @@ INSTANTIATE_TEST_SUITE_P(,
::testing::Bool(),
::testing::Bool()));
// Tests the GetStringValue method specific to FID_DESCRIPTION label for reasons
// to enforce GLS. Parameters are:
// 1. Is enrolled with mdm.
// 2. Is encrypted data missing in lsa store.
class GcpReauthCredentialEnforceAuthReasonGetStringValueTest
: public GcpReauthCredentialTest,
public ::testing::WithParamInterface<std::tuple<bool, bool>> {
protected:
FakeAssociatedUserValidator* fake_associated_user_validator() {
return &fake_associated_user_validator_;
}
private:
FakeAssociatedUserValidator fake_associated_user_validator_;
};
TEST_P(GcpReauthCredentialEnforceAuthReasonGetStringValueTest, FidDescription) {
USES_CONVERSION;
// Enable standard escrow service features in non-Chrome builds so that
// the escrow service code can be tested by the build machines.
#if !BUILDFLAG(GOOGLE_CHROME_BRANDING)
GoogleMdmEscrowServiceEnablerForTesting escrow_service_enabler(true);
#endif
CredentialProviderSigninDialogTestDataStorage test_data_storage;
const bool enrolled_mdm = std::get<0>(GetParam());
const bool store_encrypted_data = std::get<1>(GetParam());
ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmUrl, L"https://mdm.com"));
ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmEscrowServiceServerUrl,
L"https://escrow.com"));
GoogleMdmEnrolledStatusForTesting forced_enrolled_status(enrolled_mdm);
CComPtr<IReauthCredential> reauth;
ASSERT_EQ(S_OK, CComCreator<CComObject<CReauthCredential>>::CreateInstance(
nullptr, IID_IReauthCredential, (void**)&reauth));
ASSERT_TRUE(!!reauth);
CComBSTR username = L"foo_bar";
CComBSTR full_name = A2COLE(test_data_storage.GetSuccessFullName().c_str());
CComBSTR password = A2COLE(test_data_storage.GetSuccessPassword().c_str());
CComBSTR email = A2COLE(test_data_storage.GetSuccessEmail().c_str());
// Create a fake user to reauth.
CComBSTR sid = nullptr;
ASSERT_EQ(S_OK,
fake_os_user_manager()->CreateTestOSUser(
OLE2CW(username), OLE2CW(password), OLE2CW(full_name),
L"comment", base::UTF8ToUTF16(test_data_storage.GetSuccessId()),
OLE2CW(email), &sid));
if (store_encrypted_data) {
base::string16 store_key = GetUserPasswordLsaStoreKey(OLE2W(sid));
auto policy = ScopedLsaPolicy::Create(POLICY_ALL_ACCESS);
EXPECT_TRUE(SUCCEEDED(
policy->StorePrivateData(store_key.c_str(), L"encrypted_data")));
EXPECT_TRUE(policy->PrivateDataExists(store_key.c_str()));
}
// Populate the associated users list. The created user's token handle
// should be valid so that no reauth credential is created.
fake_associated_user_validator()->StartRefreshingTokenHandleValidity();
ASSERT_EQ(S_OK, reauth->SetOSUserInfo(
sid, CComBSTR(OSUserManager::GetLocalDomain().c_str()),
CComBSTR(W2COLE(L"username"))));
ASSERT_EQ(S_OK, reauth->SetEmailForReauth(CComBSTR(email)));
CComPtr<ICredentialProviderCredential2> cpc2;
ASSERT_EQ(S_OK, reauth->QueryInterface(IID_ICredentialProviderCredential2,
reinterpret_cast<void**>(&cpc2)));
LPWSTR string_value = nullptr;
ASSERT_EQ(S_OK, cpc2->GetStringValue(FID_DESCRIPTION, &string_value));
if (!enrolled_mdm) {
ASSERT_STREQ(
string_value,
W2COLE(GetStringResource(
IDS_REAUTH_NOT_ENROLLED_WITH_MDM_FID_DESCRIPTION_BASE)
.c_str()));
} else if (!store_encrypted_data) {
ASSERT_STREQ(
string_value,
W2COLE(
GetStringResource(
IDS_REAUTH_MISSING_PASSWORD_RECOVERY_INFO_FID_DESCRIPTION_BASE)
.c_str()));
} else {
ASSERT_STREQ(
string_value,
W2COLE(GetStringResource(IDS_REAUTH_FID_DESCRIPTION_BASE).c_str()));
}
}
INSTANTIATE_TEST_SUITE_P(,
GcpReauthCredentialEnforceAuthReasonGetStringValueTest,
::testing::Combine(::testing::Bool(),
::testing::Bool()));
class GcpReauthCredentialGlsRunnerTest : public GlsRunnerTestBase {};
TEST_F(GcpReauthCredentialGlsRunnerTest, NoGaiaIdAvailable) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment