Skip to content

Commit

Permalink
Migrate authenticators and related to UserDataAuth
Browse files Browse the repository at this point in the history 
Migrate Authenticators, SecurityTokenLoginTest,
LoginApiTest, saml_browsertest and Oobe Browser Test
to UserDataAuth.

BUG=b:183409667
TEST=Build OK && Unit/browser test passes

Change-Id: Ie95949ffca1e24cfbc9acc02fc0d02171e673ae2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2780512
Commit-Queue: John L Chen <zuan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#868205}
  • Loading branch information
@john0312
john0312 authored and Chromium LUCI CQ committed Mar 31, 2021
1 parent 3139f52 commit 3276de8
Show file tree
Hide file tree
Showing 15 changed files with 324 additions and 267 deletions.
178 changes: 93 additions & 85 deletions chrome/browser/ash/login/auth/cryptohome_authenticator_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@
#include "chrome/test/base/testing_profile_manager.h"
#include "chromeos/cryptohome/cryptohome_parameters.h"
#include "chromeos/cryptohome/cryptohome_util.h"
#include "chromeos/cryptohome/homedir_methods.h"
#include "chromeos/cryptohome/system_salt_getter.h"
#include "chromeos/dbus/cros_disks_client.h"
#include "chromeos/dbus/cryptohome/account_identifier_operators.h"
#include "chromeos/dbus/cryptohome/fake_cryptohome_client.h"
#include "chromeos/dbus/cryptohome/cryptohome_client.h"
#include "chromeos/dbus/cryptohome/rpc.pb.h"
#include "chromeos/dbus/userdataauth/fake_cryptohome_misc_client.h"
#include "chromeos/dbus/userdataauth/fake_userdataauth_client.h"
#include "chromeos/login/auth/cryptohome_key_constants.h"
#include "chromeos/login/auth/key.h"
#include "chromeos/login/auth/mock_auth_status_consumer.h"
Expand Down Expand Up @@ -125,11 +126,11 @@ bool CreateOwnerKeyInSlot(PK11SlotInfo* slot) {
slot, key, true /* permanent */) != nullptr;
}

// Fake CryptohomeClient implementation for this test.
class TestCryptohomeClient : public ::chromeos::FakeCryptohomeClient {
// Fake UserDataAuthClient implementation for this test.
class TestUserDataAuthClient : public ::chromeos::FakeUserDataAuthClient {
public:
TestCryptohomeClient() = default;
~TestCryptohomeClient() override = default;
TestUserDataAuthClient() = default;
~TestUserDataAuthClient() override = default;

void set_expected_id(const cryptohome::AccountIdentifier& id) {
expected_id_ = id;
Expand All @@ -151,89 +152,91 @@ class TestCryptohomeClient : public ::chromeos::FakeCryptohomeClient {
mount_guest_should_succeed_ = should_succeed;
}

void set_remove_ex_should_succeed(bool should_succeed) {
remove_ex_should_succeed_ = should_succeed;
void set_remove_should_succeed(bool should_succeed) {
remove_should_succeed_ = should_succeed;
}

void set_unmount_ex_should_succeed(bool should_succeed) {
unmount_ex_should_succeed_ = should_succeed;
void set_unmount_should_succeed(bool should_succeed) {
unmount_should_succeed_ = should_succeed;
}

void MountEx(const cryptohome::AccountIdentifier& cryptohome_id,
const cryptohome::AuthorizationRequest& auth,
const cryptohome::MountRequest& request,
DBusMethodCallback<cryptohome::BaseReply> callback) override {
void Mount(const ::user_data_auth::MountRequest& request,
MountCallback callback) override {
if (request.guest_mount()) {
// Guest Mount, handle specially.
::user_data_auth::MountReply reply;
if (!mount_guest_should_succeed_) {
reply.set_error(::user_data_auth::CryptohomeErrorCode::
CRYPTOHOME_ERROR_MOUNT_FATAL);
}

base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), reply));
return;
}

// Normal mount.
EXPECT_EQ(is_create_attempt_expected_, request.has_create());
if (is_create_attempt_expected_) {
EXPECT_EQ(expected_authorization_secret_,
request.create().keys(0).secret());
EXPECT_EQ(kCryptohomeGaiaKeyLabel,
request.create().keys(0).data().label());
}
EXPECT_EQ(expected_id_, cryptohome_id);
EXPECT_EQ(expected_authorization_secret_, auth.key().secret());
EXPECT_EQ(expected_id_, request.account());
EXPECT_EQ(expected_authorization_secret_,
request.authorization().key().secret());

cryptohome::BaseReply reply;
reply.MutableExtension(cryptohome::MountReply::reply)
->set_sanitized_username(kFakeSanitizedUsername);
::user_data_auth::MountReply reply;
reply.set_sanitized_username(kFakeSanitizedUsername);
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), reply));
}

void CheckKeyEx(const cryptohome::AccountIdentifier& cryptohome_id,
const cryptohome::AuthorizationRequest& auth,
const cryptohome::CheckKeyRequest& request,
DBusMethodCallback<cryptohome::BaseReply> callback) override {
EXPECT_EQ(expected_id_, cryptohome_id);
EXPECT_EQ(expected_authorization_secret_, auth.key().secret());
cryptohome::BaseReply reply;
void CheckKey(const ::user_data_auth::CheckKeyRequest& request,
CheckKeyCallback callback) override {
EXPECT_EQ(expected_id_, request.account_id());
EXPECT_EQ(expected_authorization_secret_,
request.authorization_request().key().secret());
::user_data_auth::CheckKeyReply reply;
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), reply));
}

void MigrateKeyEx(
const cryptohome::AccountIdentifier& account,
const cryptohome::AuthorizationRequest& auth_request,
const cryptohome::MigrateKeyRequest& migrate_request,
DBusMethodCallback<cryptohome::BaseReply> callback) override {
EXPECT_EQ(expected_id_.account_id(), account.account_id());

cryptohome::BaseReply reply;
if (!migrate_key_should_succeed_)
reply.set_error(cryptohome::CRYPTOHOME_ERROR_MIGRATE_KEY_FAILED);

base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), reply));
}
void MigrateKey(const ::user_data_auth::MigrateKeyRequest& request,
MigrateKeyCallback callback) override {
EXPECT_EQ(expected_id_.account_id(), request.account_id().account_id());

void MountGuestEx(
const cryptohome::MountGuestRequest& request,
DBusMethodCallback<cryptohome::BaseReply> callback) override {
cryptohome::BaseReply reply;
if (!mount_guest_should_succeed_)
reply.set_error(cryptohome::CRYPTOHOME_ERROR_MOUNT_FATAL);
::user_data_auth::MigrateKeyReply reply;
if (!migrate_key_should_succeed_) {
reply.set_error(::user_data_auth::CryptohomeErrorCode::
CRYPTOHOME_ERROR_MIGRATE_KEY_FAILED);
}

base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), reply));
}

// Calls RemoveEx method. `callback` is called after the method call
// Calls Remove method. `callback` is called after the method call
// succeeds.
void RemoveEx(const cryptohome::AccountIdentifier& account,
DBusMethodCallback<cryptohome::BaseReply> callback) override {
cryptohome::BaseReply reply;
if (!remove_ex_should_succeed_)
reply.set_error(cryptohome::CRYPTOHOME_ERROR_REMOVE_FAILED);
void Remove(const ::user_data_auth::RemoveRequest& request,
RemoveCallback callback) override {
::user_data_auth::RemoveReply reply;
if (!remove_should_succeed_) {
reply.set_error(::user_data_auth::CryptohomeErrorCode::
CRYPTOHOME_ERROR_REMOVE_FAILED);
}

base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), reply));
}

void UnmountEx(const cryptohome::UnmountRequest& account,
DBusMethodCallback<cryptohome::BaseReply> callback) override {
cryptohome::BaseReply reply;
if (!unmount_ex_should_succeed_)
reply.set_error(cryptohome::CRYPTOHOME_ERROR_REMOVE_FAILED);
void Unmount(const ::user_data_auth::UnmountRequest& request,
UnmountCallback callback) override {
::user_data_auth::UnmountReply reply;
if (!unmount_should_succeed_)
reply.set_error(::user_data_auth::CryptohomeErrorCode::
CRYPTOHOME_ERROR_REMOVE_FAILED);

base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), reply));
Expand All @@ -245,10 +248,10 @@ class TestCryptohomeClient : public ::chromeos::FakeCryptohomeClient {
bool is_create_attempt_expected_ = false;
bool migrate_key_should_succeed_ = false;
bool mount_guest_should_succeed_ = false;
bool remove_ex_should_succeed_ = false;
bool unmount_ex_should_succeed_ = false;
bool remove_should_succeed_ = false;
bool unmount_should_succeed_ = false;

DISALLOW_COPY_AND_ASSIGN(TestCryptohomeClient);
DISALLOW_COPY_AND_ASSIGN(TestUserDataAuthClient);
};

} // namespace
Expand Down Expand Up @@ -281,7 +284,7 @@ class CryptohomeAuthenticatorTest : public testing::Test {

CreateTransformedKey(Key::KEY_TYPE_SALTED_SHA256_TOP_HALF,
SystemSaltGetter::ConvertRawSaltToHexString(
FakeCryptohomeClient::GetStubSystemSalt()));
FakeCryptohomeMiscClient::GetStubSystemSalt()));
}

~CryptohomeAuthenticatorTest() override {}
Expand All @@ -290,10 +293,10 @@ class CryptohomeAuthenticatorTest : public testing::Test {
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kLoginManager);

cryptohome::HomedirMethods::Initialize();

fake_cryptohome_client_ = new TestCryptohomeClient;
fake_userdataauth_client_ = new TestUserDataAuthClient;

CryptohomeMiscClient::InitializeFake();
CryptohomeClient::InitializeFake();
SystemSaltGetter::Initialize();

auth_ = new ChromeCryptohomeAuthenticator(&consumer_);
Expand All @@ -304,8 +307,8 @@ class CryptohomeAuthenticatorTest : public testing::Test {
void TearDown() override {
SystemSaltGetter::Shutdown();
CryptohomeClient::Shutdown();

cryptohome::HomedirMethods::Shutdown();
CryptohomeMiscClient::Shutdown();
UserDataAuthClient::Shutdown();
}

void CreateTransformedKey(Key::KeyType type, const std::string& salt) {
Expand Down Expand Up @@ -389,45 +392,50 @@ class CryptohomeAuthenticatorTest : public testing::Test {
}

// Add the key to the fake.
cryptohome::AddKeyRequest request;
::user_data_auth::AddKeyRequest request;
cryptohome::KeyDefinitionToKey(key_definition, request.mutable_key());
fake_cryptohome_client_->AddKeyEx(
*request.mutable_account_id() =
cryptohome::CreateAccountIdentifierFromAccountId(
user_context_.GetAccountId()),
cryptohome::AuthorizationRequest(), request,
base::BindOnce([](base::Optional<cryptohome::BaseReply> reply) {
EXPECT_TRUE(reply.has_value());
user_context_.GetAccountId());
request.mutable_authorization_request();
fake_userdataauth_client_->AddKey(
request,
base::BindOnce([](base::Optional<::user_data_auth::AddKeyReply> reply) {
ASSERT_TRUE(reply.has_value());
EXPECT_EQ(
reply->error(),
::user_data_auth::CryptohomeErrorCode::CRYPTOHOME_ERROR_NOT_SET);
}));
base::RunLoop().RunUntilIdle();
}

void ExpectMountExCall(bool expect_create_attempt) {
fake_cryptohome_client_->set_expected_id(
fake_userdataauth_client_->set_expected_id(
cryptohome::CreateAccountIdentifierFromAccountId(
user_context_.GetAccountId()));
fake_cryptohome_client_->set_expected_authorization_secret(
fake_userdataauth_client_->set_expected_authorization_secret(
transformed_key_.GetSecret());
fake_cryptohome_client_->set_is_create_attempt_expected(
fake_userdataauth_client_->set_is_create_attempt_expected(
expect_create_attempt);
}

void ExpectCheckKeyExCall() {
fake_cryptohome_client_->set_expected_id(
fake_userdataauth_client_->set_expected_id(
cryptohome::CreateAccountIdentifierFromAccountId(
user_context_.GetAccountId()));
fake_cryptohome_client_->set_expected_authorization_secret(
fake_userdataauth_client_->set_expected_authorization_secret(
transformed_key_.GetSecret());
}

void ExpectMigrateKeyExCall(bool should_succeed) {
fake_cryptohome_client_->set_expected_id(
fake_userdataauth_client_->set_expected_id(
cryptohome::CreateAccountIdentifierFromAccountId(
user_context_.GetAccountId()));
fake_cryptohome_client_->set_migrate_key_should_succeed(should_succeed);
fake_userdataauth_client_->set_migrate_key_should_succeed(should_succeed);
}

void ExpectMountGuestExCall(bool should_succeed) {
fake_cryptohome_client_->set_mount_guest_should_succeed(should_succeed);
fake_userdataauth_client_->set_mount_guest_should_succeed(should_succeed);
}

void RunResolve(CryptohomeAuthenticator* auth) {
Expand Down Expand Up @@ -467,7 +475,7 @@ class CryptohomeAuthenticatorTest : public testing::Test {

scoped_refptr<CryptohomeAuthenticator> auth_;
std::unique_ptr<TestAttemptState> state_;
TestCryptohomeClient* fake_cryptohome_client_;
TestUserDataAuthClient* fake_userdataauth_client_;

scoped_refptr<ownership::MockOwnerKeyUtil> owner_key_util_;
};
Expand Down Expand Up @@ -584,7 +592,7 @@ TEST_F(CryptohomeAuthenticatorTest, ResolveOwnerNeededFailedMount) {
EXPECT_TRUE(LoginState::Get()->IsInSafeMode());

// Unset global objects used by this test.
fake_cryptohome_client_->set_unmount_ex_should_succeed(true);
fake_userdataauth_client_->set_unmount_should_succeed(true);
LoginState::Shutdown();
}

Expand Down Expand Up @@ -633,7 +641,7 @@ TEST_F(CryptohomeAuthenticatorTest, ResolveOwnerNeededSuccess) {
EXPECT_TRUE(LoginState::Get()->IsInSafeMode());

// Unset global objects used by this test.
fake_cryptohome_client_->set_unmount_ex_should_succeed(true);
fake_userdataauth_client_->set_unmount_should_succeed(true);
LoginState::Shutdown();
}

Expand Down Expand Up @@ -678,7 +686,7 @@ TEST_F(CryptohomeAuthenticatorTest, DriveDataResync) {
ExpectGetKeyDataExCall(std::unique_ptr<int64_t>(),
std::unique_ptr<std::string>());
ExpectMountExCall(true /* expect_create_attempt */);
fake_cryptohome_client_->set_remove_ex_should_succeed(
fake_userdataauth_client_->set_remove_should_succeed(
true /* should_succeed */);

state_->PresetOnlineLoginComplete();
Expand All @@ -692,7 +700,7 @@ TEST_F(CryptohomeAuthenticatorTest, DriveResyncFail) {
FailOnLoginSuccess();
ExpectLoginFailure(AuthFailure(AuthFailure::DATA_REMOVAL_FAILED));

fake_cryptohome_client_->set_remove_ex_should_succeed(
fake_userdataauth_client_->set_remove_should_succeed(
false /* should_succeed */);

SetAttemptState(auth_.get(), state_.release());
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/ash/login/oobe_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chromeos/dbus/cryptohome/fake_cryptohome_client.h"
#include "chromeos/dbus/cryptohome/key.pb.h"
#include "chromeos/dbus/cryptohome/rpc.pb.h"
#include "chromeos/dbus/userdataauth/fake_userdataauth_client.h"
#include "chromeos/login/auth/cryptohome_key_constants.h"
#include "components/account_id/account_id.h"
#include "components/user_manager/known_user.h"
Expand Down Expand Up @@ -87,7 +87,7 @@ IN_PROC_BROWSER_TEST_F(OobeTest, NewUser) {

// Make the MountEx cryptohome call fail iff the `create` field is missing,
// which simulates the real cryptohomed's behavior for the new user mount.
FakeCryptohomeClient::Get()->set_mount_create_required(true);
FakeUserDataAuthClient::Get()->set_mount_create_required(true);
LoginDisplayHost::default_host()
->GetOobeUI()
->GetView<GaiaScreenHandler>()
Expand All @@ -103,13 +103,13 @@ IN_PROC_BROWSER_TEST_F(OobeTest, NewUser) {

// Verify the parameters that were passed to the latest MountEx call.
const cryptohome::AuthorizationRequest& cryptohome_auth =
FakeCryptohomeClient::Get()->get_last_mount_authentication();
FakeUserDataAuthClient::Get()->get_last_mount_authentication();
EXPECT_EQ(cryptohome::KeyData::KEY_TYPE_PASSWORD,
cryptohome_auth.key().data().type());
EXPECT_TRUE(cryptohome_auth.key().data().label().empty());
EXPECT_FALSE(cryptohome_auth.key().secret().empty());
const cryptohome::MountRequest& last_mount_request =
FakeCryptohomeClient::Get()->get_last_mount_request();
const ::user_data_auth::MountRequest& last_mount_request =
FakeUserDataAuthClient::Get()->get_last_mount_request();
ASSERT_TRUE(last_mount_request.has_create());
ASSERT_EQ(1, last_mount_request.create().keys_size());
EXPECT_EQ(cryptohome::KeyData::KEY_TYPE_PASSWORD,
Expand Down

0 comments on commit 3276de8

Please sign in to comment.