The crash happens when the previous kiosk launch fails at
cryptohome mount stage. That would leave the kiosk user as
misconfigured and will be removed on the next chrome run.
However, kiosk users are only added when loading user list.
Once removed, the user type could not be determined correctly.
The code would fallback to regular user login code path and
crash. The CL skips removing kiosk users from user list even
if they are in marked as misconfigured.
Bug: 403354377
Change-Id: I7c19228c53d6685a34abaa4c3a88e81636b9cf06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6357166
Reviewed-by: Edman Anjos <edman@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1435088}
Also, some calls of UserManager::UserLoggedIn pass User::username_hash().
However, in production, UserLoggedIn sets the value to
User::username_hash so it was wrong. This CL fixes them, in most cases,
by using user_manager::TestHelper::GetFakeUsernameHash().
BUG=278643115
TEST=Tryjob
Change-Id: Id03ea14853c9b063c5fe6a72d8650ab2ba8bf4ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6357613
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1433652}
This fixes the unexpected usage of UserManager in
SystemLogKioskSessionApitest.
This also fixes the behavior of kPreventKioskAutolaunchForTesting
to make it usable for the fixed test.
BUG=396238168
TEST=Tryjob
Change-Id: I684992f9937f8cd83c7b8672781444d182184a6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6262087
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Jeroen Dhollander <jeroendh@google.com>
Cr-Commit-Position: refs/heads/main@{#1421106}
Remove ACTIVE_DIRECTORY from the AccountType enum in
components/account_id/account_id.h. Along with that, remove the object
guid attribute of AccountId, which was only set for Active Directory
accounts. Also update the files where these enum value and ID attribute
were still used.
Additionally, update components/user_manager/known_user.cc, removing
preferences keys that where only used for Active Directory accounts.
These preferences are now part of the kObsoleteKeys list.
This is a follow up to the following CL, in which we removed the
kActiveDirectory type from account.h:
https://chromium-review.googlesource.com/c/chromium/src/+/6226680
Bug: 291783005
Change-Id: Iee3a6d8f30a99f21d1f5cbc22952f89ed35e9663
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6254399
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Renato Silva <rrsilva@google.com>
Commit-Queue: Felipe Andrade <fsandrade@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1420449}
And production code starts to call it instead of
UserManager::SwitchActiveUser.
Tests are still calling UserManager::SwitchActiveUser directly,
which will be removed in later CLs.
BUG=278643115
TEST=Tryjob
Change-Id: I7ddc6f566c5634dcc96ffbc69e6997dfd95d51d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6242976
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1417425}
Re-implement GetFakeUsernameHash in TestHelper using UserDataAuthClient
APIs. Then, FakeUserManager::GetFakeUsernameHash is migrated it into
the new one.
Now, FakeUserManager and FakeChromeUserManager are deprecated.
In new code, we expect UserManagerImpl is used.
BUG=278643115
TEST=Tryjob
Change-Id: Id89b680c1f8d9cfee7f276ca546b6563c0d6dfd9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6242113
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1417399}
Currently, if missing and some condition meets, a User instance is
created in UserManager::UserLoggedIn called from
SessionManager::CreateSession. This approach is difficult to write
proper test, because the expecting condition is too complex.
This CL moves that part to the callers of CreateSession family.
Now, UserLoggedIn handles log-in. Callers now have responsibility
to register User before creating the session.
In the production, no behavior change is expected.
In the tests, the behavior of UserManager is now much aligned
with fake implementation.
BUG=278643115
TEST=Tryjob
Change-Id: Ia746f845a6967288ff5626f5507b9da71c5d8487
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6239080
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1417065}
The intention of the method is to return a User instance
regardless of whether it is persisted or ephemeral.
The original code looked at active_user_, which was the only
possibility that ephemeral user is being pointed.
This worked, because the ephemeral user creation is done
as a part of UserLoggedIn, and so created user is being
pointed soon by active_user_.
Now, we're moving EnsureUser out from UserLoggedIn,
and so there will be some timing that is observable
from outside of UserManager.
The new approach can prevent the edge case.
BUG=278643115
TEST=Tryjob
Change-Id: I8d59ea90d8d3ff6606dd58253c3b18a4cedb37e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6234333
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1416268}
The condition can be inferred by looking at whether the user
is in the persisted user list.
The key in this CL is to remove one "current" concept as preparation
to move the caller of the function out from UserManager::UserLoggedIn.
BUG=278643115
TEST=Tryjob
Change-Id: Ib29742c2e22eefe8d17c476422ebd21dcfc45d34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6232326
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1416250}
User::is_managed indicates whether the User is a managed one.
The value is updated along with policy.
This is preparation to reduce the dependency to Profile in ash-chrome.
BUG=None
TEST=Tryjob
Change-Id: I389f80abe163a3abe9bb7c880c348a12e1a6fc42
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6189013
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1412959}
UserManagerImpl::UserLoggedIn handles multiple tasks and so
complex. This CL groups some of the operations, and extract them
into named methods representing what is the responsibility for each.
Now, the method is (roughly) categorized into 6 tasks.
1. (for primary user log-in) ensure User* instance is there.
This is not yet extracted into a method.
2. Handling to update User's state for login, regardless of
primary user log-in or multi-user sign-in cases.
3. Handling Primary user log-in specific tasks.
4. Handling Multi-user sign-in specific tasks.
This is not yet extracted into a method.
5. Switching the active user.
6. Notify necessary observers.
Then, now 2 and 5 are properly shared.
BUG=278643115
TEST=tryjob
Change-Id: I0b5912ef04d706c1f818a7ab43b4f1b5e5809e7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6207228
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1412674}
The condition to check whether or not multi-sign-in is updated.
Some assumption that UserManagerImpl has is not explicit assertion
with CHECK.
This is preparation to switch Fake{,Chrome}UserManager into
UserManagerImpl.
BUG=278643115
TEST=tryjob
Change-Id: I80d47a588dc8dbec1fb93cd27008b0ddaf0e9eee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6196999
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1412459}
This is achieved by adopting the test-only class GaiaId::Literal, which
allows constexpr instances for constants that represent Gaia IDs. This
also reduces the risk for bugs in tests or nonrealistic setups, as it is
hard to use strings other than Gaia IDs (such as e-mails) as gaia IDs,
and the other way round (use Gaia IDs accidentally to represent
something else).
This CL was uploaded by git cl split.
R=hidehiko@chromium.org
Bug: 380416867
Change-Id: Ie9d590a0623f0e4d29fda50a34309fbba1a319f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6207553
Auto-Submit: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1412409}
We should not allow any non-demo-account user to login into demo mode.
Demo account is created right before login. In this case, allowlist this
user in setting.
Bug: 364195755
Change-Id: Icf61c3d577580deab3ebf7614ae94f651f1ad13d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6149347
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiqi Ruan <xiqiruan@chromium.org>
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1410485}
To keep the fake behavior closer to the real one.
User{,Non}CryptohomeDataEphemeral related ones will be done
in following CLs.
BUG=278643115
TEST=Tryjob
Change-Id: I542c2469d95c8f4275433e928eab0daec5e70a87
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6171751
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1405836}
To make fake behavior more consistent to the actual behavior.
We can replace callers in later CLs.
AddPublicAccountUser is migrated by this CL, because of the conflict
of existing API and UserManagerImpl's APi.
BUG=278643115
TEST=Tryjob
Change-Id: Ieb80af654286c53c48e712666fa78a1e53ba308e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6159104
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1403890}
By inlining the code, now UserManagerImpl::UserLoggedIn has three
major blocks;
1) handling multi-user sign in
2) ensure User instance for the login.
3) actual login procedure for the primary user.
This is preparation to move the login flow code to SessionManager,
and also to share the code with fake classes.
BUG=278643115
TEST=Tryjob
Change-Id: I41d53b5de7468a74f792a00621e73b51a35338e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6155998
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1403577}
This is preparation to split UserLoggedIn into three blocks;
1) handling multi-user sign-in. This is expected to be moved
into SessionManager later.
2) Ensuring User instance for the given AccountId.
Later, we'd like to share the logic with FakeUserManager.
3) Actual log-in process. This is also expected to be moved
into SessionManager later.
BUG=278643115
TEST=Tryjob
Change-Id: I543342283a8cf8c5a4dabf640e634aacb54f1bd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6149398
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1403076}
It's not used, so removed.
Then, GetActiveUser() becomes same as its parent's implementation,
so removed the overriding.
Bug: 278643115
Test: Tryjob
Change-Id: I97c4be7b0f51431b4d24b22fa7c4f202977b2e8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5934123
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1402476}
This refactors UserManager::*LoggedIn* family. Each function is now
consisted from two blocks; 1) ensuring User is created, and 2)
actual log-in operation.
This is preparation to extract 1) from UserLoggedIn(), then to share
with fake implementation for tests in later CLs.
BUG=278643115
TEST=Tryjob
Change-Id: I6a3516ae3db30f687c854e3686740c813473f9e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6109415
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1398637}
Currently it is set as a part of user log-in, but it should make
more sense to set it up on user creation.
Historically, it was done so, but with code change histories
the timing gets diverged. This CL restores it for simplicity.
BUG=278643115
TEST=Tryjob
Change-Id: I06910831e60ed1e88e07111b61b0fa64969b431e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6109996
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1398635}
Due to performance issue for ephemeral sessions, we may login demo
account user as non-ephemeral users (config through policy). Remove
these users on login screen before next demo account login.
Bug: 383220622
Change-Id: Ie3e43952982277e491cc5400b798df3b3d2a1370
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6081861
Commit-Queue: Xiqi Ruan <xiqiruan@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1394534}
The class replaces std::string to avoid bugs when dealing with
obfuscated gaia IDs. The class wraps an std::string and is in most cases
a drop-in replacement.
This patch tackles changes under //components.
One of the classes affected is AccountId, under
//components/account_id. This class combines gaia IDs with other
kinds of IDs (with ongoing deprecations and code cleanups for
Active Directory) and needed a bit of additional refactoring to
improve type safety for Gaia IDs (AccountType::GOOGLE) specifically.
This CL was uploaded by git cl split.
R=hidehiko@chromium.org, osamafathy@google.com
Bug: 380416867
Change-Id: I01a052c455601274c0a514ce852a63399aeb0f5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6062319
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Felipe Andrade <fsandrade@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1391672}
With renaming to SetType following surrounding APIs.
This is only public and effectively mutable API in User class,
which can break the overall user management consistency.
Similar to other methods in User, now the API is hidden in private
section, so can be called only by allow-listed classes.
Remaining public non-const APIs are to take mutable Profile prefs
and observer registration, so now using mutable User instance
outside of core of UserManager related classes can be safer.
Also, now there is no class inheriting User, so merge protected
section into the private.
BUG=278643115
TEST=Tryjob
Change-Id: I9e742285894424243e1a32847935cfb37ba39281
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6054818
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1388961}
In details, the policy is for session management, rather than user
management. So, move it out from UserManager.
This helps us writing UserManger related unittest easily,
because we do not need to set up local state with
multi-user sign-in policy, which has dependency issue.
BUG=278643115
TEST=Tryjob
Change-Id: Ic0fdd1bc2bd514ce6a9373d89caf5a7c4a7e3c83
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6025366
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1386582}
