[modular] Give sessions a device-local unique identifier.
- For now it takes the value of the account ID, such that re-authentication is
not required for existing users.
- The ID is not explicitly stored anywhere, the uniqueness is guaranteed
by the fact that we can only have one session running at a time. Even if
we had multiple sessions, the uniqueness is guaranteed by the account
ID uniqueness from DB.
- Also renamed some variables in sessionmgr_impl
Change-Id: I73f150ddc874a49ffe9975c2f2fe16f1d8a5732c
diff --git a/peridot/bin/basemgr/session_context_impl.cc b/peridot/bin/basemgr/session_context_impl.cc
index c91fdae..3b07b1d 100644
--- a/peridot/bin/basemgr/session_context_impl.cc
+++ b/peridot/bin/basemgr/session_context_impl.cc
@@ -16,7 +16,7 @@
namespace modular {
SessionContextImpl::SessionContextImpl(
- fuchsia::sys::Launcher* const launcher,
+ fuchsia::sys::Launcher* const launcher, std::string session_id,
fuchsia::modular::AppConfig sessionmgr_config,
fuchsia::modular::AppConfig session_shell_config,
fuchsia::modular::AppConfig story_shell_config,
@@ -34,19 +34,10 @@
FXL_CHECK(get_presentation_);
FXL_CHECK(on_session_shutdown_);
+ // TODO(MF-280): We should replace USER* with SESSION* below. However, this
+ // will force users to re-authenticate, so the timing needs to be considered.
// 0. Generate the path to map '/data' for the sessionmgr we are starting.
- std::string data_origin;
- if (!account) {
- // Guest user.
- // Generate a random number to be used in this case.
- uint32_t random_number = 0;
- zx_cprng_draw(&random_number, sizeof random_number);
- data_origin = std::string("/data/modular/USER_GUEST_") +
- std::to_string(random_number);
- } else {
- // Non-guest user.
- data_origin = std::string("/data/modular/USER_") + std::string(account->id);
- }
+ std::string data_origin = std::string("/data/modular/USER_") + session_id;
FXL_LOG(INFO) << "SESSIONMGR DATA ORIGIN IS " << data_origin;
@@ -57,7 +48,7 @@
// 2. Initialize the Sessionmgr service.
sessionmgr_app_->services().ConnectToService(sessionmgr_.NewRequest());
sessionmgr_->Initialize(
- std::move(account), std::move(session_shell_config),
+ session_id, std::move(account), std::move(session_shell_config),
std::move(story_shell_config), use_session_shell_for_story_shell_factory,
std::move(ledger_token_manager), std::move(agent_token_manager),
session_context_binding_.NewBinding(), std::move(view_owner_request));
diff --git a/peridot/bin/basemgr/session_context_impl.h b/peridot/bin/basemgr/session_context_impl.h
index 1016ee6..ed1907a 100644
--- a/peridot/bin/basemgr/session_context_impl.h
+++ b/peridot/bin/basemgr/session_context_impl.h
@@ -34,8 +34,8 @@
class SessionContextImpl : fuchsia::modular::internal::SessionContext {
public:
// Called after perfoming shutdown of the session, to signal our completion
- // (and deletion of our instance) to our owner, we do it using a callback
- // supplied to us in our constructor. (The alternative is to take in a
+ // (and deletion of our instance) to our owner, this is done using a callback
+ // supplied in the constructor. (The alternative is to take in a
// SessionProvider*, which seems a little specific and overscoped).
using OnSessionShutdownCallback = fit::function<void(bool logout_users)>;
@@ -43,8 +43,12 @@
using GetPresentationCallback = fit::function<void(
fidl::InterfaceRequest<fuchsia::ui::policy::Presentation> request)>;
+ // |session_id| is the device-local unique identifier for this session. Caller
+ // must ensure its uniqueness. sessionmgr creates an Environment namespace
+ // with the given |session_id|, and this will crash if it tries to create an
+ // environment with a pre-existing name.
SessionContextImpl(
- fuchsia::sys::Launcher* const launcher,
+ fuchsia::sys::Launcher* const launcher, std::string session_id,
fuchsia::modular::AppConfig sessionmgr_config,
fuchsia::modular::AppConfig session_shell_config,
fuchsia::modular::AppConfig story_shell_config,
diff --git a/peridot/bin/basemgr/session_context_impl_unittest.cc b/peridot/bin/basemgr/session_context_impl_unittest.cc
index 22e22c2..8b72816 100644
--- a/peridot/bin/basemgr/session_context_impl_unittest.cc
+++ b/peridot/bin/basemgr/session_context_impl_unittest.cc
@@ -17,6 +17,9 @@
using ::component::testing::FakeLauncher;
using SessionContextImplTest = gtest::TestLoopFixture;
+// Unique identifier for a test session.
+constexpr char kSessionId[] = "session_id";
+
TEST_F(SessionContextImplTest, StartSessionmgrWithTokenManagers) {
FakeLauncher launcher;
std::string url = "test_url_string";
@@ -35,7 +38,7 @@
fuchsia::auth::TokenManagerPtr agent_token_manager;
SessionContextImpl impl(
- &launcher, CloneStruct(app_config) /* sessionmgr_config */,
+ &launcher, kSessionId, CloneStruct(app_config) /* sessionmgr_config */,
CloneStruct(app_config) /* session_shell_config */,
CloneStruct(app_config) /* story_shell_config */,
false, /* use_session_shell_for_story_shell_factory */
@@ -68,7 +71,7 @@
bool done_callback_called = false;
SessionContextImpl impl(
- &launcher, /* sessionmgr_config= */ CloneStruct(app_config),
+ &launcher, kSessionId, /* sessionmgr_config= */ CloneStruct(app_config),
/* session_shell_config= */ CloneStruct(app_config),
/* story_shell_config= */ CloneStruct(app_config),
/* use_session_shell_for_story_shell_factory= */ false,
diff --git a/peridot/bin/basemgr/session_provider.cc b/peridot/bin/basemgr/session_provider.cc
index 64bee60..6e050c1 100644
--- a/peridot/bin/basemgr/session_provider.cc
+++ b/peridot/bin/basemgr/session_provider.cc
@@ -50,10 +50,25 @@
}
};
+ // TODO(MF-280): Currently, session_id maps to account ID. We should generate
+ // unique session ID's and store the mapping of session ID to session once we
+ // support multiple sessions.
+ std::string session_id;
+ if (!account) {
+ // Guest user. Generate a random number to be used in this case.
+ uint32_t random_number = 0;
+ zx_cprng_draw(&random_number, sizeof random_number);
+ session_id = std::to_string(random_number);
+ } else {
+ // Non-guest user.
+ session_id = std::string(account->id);
+ }
+
// Session context initializes and holds the sessionmgr process.
session_context_ = std::make_unique<SessionContextImpl>(
- launcher_, CloneStruct(sessionmgr_), CloneStruct(session_shell_),
- CloneStruct(story_shell_), use_session_shell_for_story_shell_factory_,
+ launcher_, session_id, CloneStruct(sessionmgr_),
+ CloneStruct(session_shell_), CloneStruct(story_shell_),
+ use_session_shell_for_story_shell_factory_,
std::move(ledger_token_manager), std::move(agent_token_manager),
std::move(account), std::move(view_owner),
/* get_presentation= */
diff --git a/peridot/bin/sessionmgr/sessionmgr_impl.cc b/peridot/bin/sessionmgr/sessionmgr_impl.cc
index 1169f16..a29f38c 100644
--- a/peridot/bin/sessionmgr/sessionmgr_impl.cc
+++ b/peridot/bin/sessionmgr/sessionmgr_impl.cc
@@ -65,7 +65,7 @@
constexpr char kModuleResolverUrl[] =
"fuchsia-pkg://fuchsia.com/module_resolver#meta/module_resolver.cmx";
-constexpr char kUserEnvironmentLabelPrefix[] = "user-";
+constexpr char kSessionEnvironmentLabelPrefix[] = "session-";
constexpr char kMessageQueuePath[] = "/data/MESSAGE_QUEUES/v1/";
@@ -90,10 +90,6 @@
return config;
}
-std::string GetAccountId(const fuchsia::modular::auth::AccountPtr& account) {
- return !account ? "GUEST" : account->id;
-}
-
// Creates a function that can be used as termination action passed to AtEnd(),
// which when called invokes the reset() method on the object pointed to by the
// argument. Used to reset() fidl pointers and std::unique_ptr<>s fields.
@@ -186,7 +182,7 @@
SessionmgrImpl::~SessionmgrImpl() = default;
void SessionmgrImpl::Initialize(
- fuchsia::modular::auth::AccountPtr account,
+ std::string session_id, fuchsia::modular::auth::AccountPtr account,
fuchsia::modular::AppConfig session_shell_config,
fuchsia::modular::AppConfig story_shell_config,
bool use_session_shell_for_story_shell_factory,
@@ -227,6 +223,7 @@
session_context_ = session_context.Bind();
AtEnd(Reset(&session_context_));
+ InitializeSessionEnvironment(session_id);
InitializeUser(std::move(account), std::move(agent_token_manager));
InitializeSessionShell(
std::move(session_shell_config),
@@ -239,6 +236,18 @@
story_provider_impl_->SetSessionShell(std::move(session_shell));
}
+void SessionmgrImpl::InitializeSessionEnvironment(std::string session_id) {
+ session_id_ = session_id;
+
+ static const auto* const kEnvServices = new std::vector<std::string>{
+ fuchsia::modular::DeviceMap::Name_, fuchsia::modular::Clipboard::Name_};
+ session_environment_ = std::make_unique<Environment>(
+ startup_context_->environment(),
+ std::string(kSessionEnvironmentLabelPrefix) + session_id_, *kEnvServices,
+ /* kill_on_oom = */ true);
+ AtEnd(Reset(&session_environment_));
+}
+
void SessionmgrImpl::InitializeUser(
fuchsia::modular::auth::AccountPtr account,
fidl::InterfaceHandle<fuchsia::auth::TokenManager> agent_token_manager) {
@@ -247,15 +256,6 @@
account_ = std::move(account);
AtEnd(Reset(&account_));
-
- static const auto* const kEnvServices = new std::vector<std::string>{
- fuchsia::modular::DeviceMap::Name_, fuchsia::modular::Clipboard::Name_};
- user_environment_ = std::make_unique<Environment>(
- startup_context_->environment(),
- std::string(kUserEnvironmentLabelPrefix) + GetAccountId(account_),
- *kEnvServices,
- /* kill_on_oom = */ true);
- AtEnd(Reset(&user_environment_));
}
zx::channel SessionmgrImpl::GetLedgerRepositoryDirectory() {
@@ -290,7 +290,7 @@
ledger_app_ =
std::make_unique<AppClient<fuchsia::ledger::internal::LedgerController>>(
- user_environment_->GetLauncher(), std::move(ledger_config), "",
+ session_environment_->GetLauncher(), std::move(ledger_config), "",
nullptr);
ledger_app_->SetAppErrorHandler([this] {
FXL_LOG(ERROR) << "Ledger seems to have crashed unexpectedly." << std::endl
@@ -355,19 +355,19 @@
void SessionmgrImpl::InitializeDeviceMap() {
// fuchsia::modular::DeviceMap service
- const std::string device_id = LoadDeviceID(GetAccountId(account_));
- device_name_ = LoadDeviceName(GetAccountId(account_));
+ const std::string device_id = LoadDeviceID(session_id_);
+ device_name_ = LoadDeviceName(session_id_);
const std::string device_profile = LoadDeviceProfile();
device_map_impl_ = std::make_unique<DeviceMapImpl>(
device_name_, device_id, device_profile, ledger_client_.get(),
fuchsia::ledger::PageId());
- user_environment_->AddService<fuchsia::modular::DeviceMap>(
+ session_environment_->AddService<fuchsia::modular::DeviceMap>(
[this](fidl::InterfaceRequest<fuchsia::modular::DeviceMap> request) {
if (terminating_) {
return;
}
- // device_map_impl_ may be reset before user_environment_.
+ // device_map_impl_ may be reset before session_environment_.
if (device_map_impl_) {
device_map_impl_->Connect(std::move(request));
}
@@ -379,7 +379,7 @@
agent_runner_->ConnectToAgent(kAppId, kClipboardAgentUrl,
services_from_clipboard_agent_.NewRequest(),
clipboard_agent_controller_.NewRequest());
- user_environment_->AddService<fuchsia::modular::Clipboard>(
+ session_environment_->AddService<fuchsia::modular::Clipboard>(
[this](fidl::InterfaceRequest<fuchsia::modular::Clipboard> request) {
if (terminating_) {
return;
@@ -391,7 +391,7 @@
void SessionmgrImpl::InitializeMessageQueueManager() {
std::string message_queue_path = kMessageQueuePath;
- message_queue_path.append(GetAccountId(account_));
+ message_queue_path.append(session_id_);
if (!files::CreateDirectory(message_queue_path)) {
FXL_LOG(FATAL) << "Failed to create message queue directory: "
<< message_queue_path;
@@ -475,7 +475,7 @@
AtEnd(Reset(&agent_runner_storage_));
agent_runner_.reset(new AgentRunner(
- user_environment_->GetLauncher(), message_queue_manager_.get(),
+ session_environment_->GetLauncher(), message_queue_manager_.get(),
ledger_repository_.get(), agent_runner_storage_.get(),
agent_token_manager_.get(), user_intelligence_provider_impl_.get(),
entity_provider_runner_.get()));
@@ -510,8 +510,9 @@
context_engine_app_ =
std::make_unique<AppClient<fuchsia::modular::Lifecycle>>(
- user_environment_->GetLauncher(), std::move(context_engine_config),
- "" /* data_origin */, std::move(service_list));
+ session_environment_->GetLauncher(),
+ std::move(context_engine_config), "" /* data_origin */,
+ std::move(service_list));
context_engine_app_->services().ConnectToService(
std::move(context_engine_request));
AtEnd(Reset(&context_engine_app_));
@@ -568,8 +569,9 @@
// isolate the data it reads to a subdir of /data and map that in here.
module_resolver_app_ =
std::make_unique<AppClient<fuchsia::modular::Lifecycle>>(
- user_environment_->GetLauncher(), std::move(module_resolver_config),
- "" /* data_origin */, std::move(service_list));
+ session_environment_->GetLauncher(),
+ std::move(module_resolver_config), "" /* data_origin */,
+ std::move(service_list));
AtEnd(Reset(&module_resolver_app_));
AtEnd(Teardown(kBasicTimeout, "Resolver", module_resolver_app_.get()));
}
@@ -617,7 +619,7 @@
startup_context_->ConnectToEnvironmentService<fuchsia::sys::Loader>()));
story_provider_impl_.reset(new StoryProviderImpl(
- user_environment_.get(), device_map_impl_->current_device_id(),
+ session_environment_.get(), device_map_impl_->current_device_id(),
session_storage_.get(), std::move(story_shell_config),
std::move(story_shell_factory_ptr), component_context_info,
std::move(focus_provider_story_provider),
@@ -773,7 +775,7 @@
}
session_shell_app_ = std::make_unique<AppClient<fuchsia::modular::Lifecycle>>(
- user_environment_->GetLauncher(), std::move(session_shell_config),
+ session_environment_->GetLauncher(), std::move(session_shell_config),
/* data_origin = */ "", std::move(service_list));
session_shell_app_->SetAppErrorHandler([this] {
@@ -942,7 +944,7 @@
cloud_provider_app_config.url = kCloudProviderFirestoreAppUrl;
cloud_provider_app_ =
std::make_unique<AppClient<fuchsia::modular::Lifecycle>>(
- user_environment_->GetLauncher(),
+ session_environment_->GetLauncher(),
std::move(cloud_provider_app_config));
cloud_provider_app_->services().ConnectToService(
cloud_provider_factory_.NewRequest());
diff --git a/peridot/bin/sessionmgr/sessionmgr_impl.h b/peridot/bin/sessionmgr/sessionmgr_impl.h
index d2af28c..876fa57 100644
--- a/peridot/bin/sessionmgr/sessionmgr_impl.h
+++ b/peridot/bin/sessionmgr/sessionmgr_impl.h
@@ -69,7 +69,7 @@
private:
// |Sessionmgr|
void Initialize(
- fuchsia::modular::auth::AccountPtr account,
+ std::string session_id, fuchsia::modular::auth::AccountPtr account,
fuchsia::modular::AppConfig session_shell_config,
fuchsia::modular::AppConfig story_shell_config,
bool use_session_shell_for_story_shell_factory,
@@ -85,6 +85,9 @@
SwapSessionShellCallback callback) override;
// Sequence of Initialize() broken up into steps for clarity.
+ // TODO(MF-279): Remove |account| and |agent_token_manager| once sessions
+ // start receiving persona handles.
+ void InitializeSessionEnvironment(std::string session_id);
void InitializeUser(
fuchsia::modular::auth::AccountPtr account,
fidl::InterfaceHandle<fuchsia::auth::TokenManager> agent_token_manager);
@@ -188,6 +191,10 @@
void ConnectSessionShellToStoryProvider();
+ // The device-local unique identifier for this session. The uniqueness
+ // is enforced by basemgr which vends sessions.
+ std::string session_id_;
+
component::StartupContext* const startup_context_;
fuchsia::modular::internal::SessionmgrConfigPtr config_;
std::unique_ptr<scoped_tmpfs::ScopedTmpFS> memfs_for_ledger_;
@@ -211,7 +218,7 @@
// Provides services to the Ledger
component::ServiceProviderImpl ledger_service_provider_;
- std::unique_ptr<Environment> user_environment_;
+ std::unique_ptr<Environment> session_environment_;
fuchsia::modular::auth::AccountPtr account_;
diff --git a/peridot/bin/voila/src/main.rs b/peridot/bin/voila/src/main.rs
index f95ae35..853421b 100644
--- a/peridot/bin/voila/src/main.rs
+++ b/peridot/bin/voila/src/main.rs
@@ -150,6 +150,7 @@
sessionmgr
.initialize(
+ "session_id", /* session_id */
Some(OutOfLine(&mut account)),
&mut session_shell_config,
&mut story_shell_config,
diff --git a/sdk/fidl/fuchsia.modular.internal/sessionmgr.fidl b/sdk/fidl/fuchsia.modular.internal/sessionmgr.fidl
index 0a2fcee..85ae8a6 100644
--- a/sdk/fidl/fuchsia.modular.internal/sessionmgr.fidl
+++ b/sdk/fidl/fuchsia.modular.internal/sessionmgr.fidl
@@ -11,15 +11,20 @@
using fuchsia.ui.viewsv1token;
// The basemgr application (there is no |Basemgr| service) requests
-// an instance of this service in order to launch and display a |Sessionmgr| per
-// user.
-// TODO(alexmin): Re-word this bit once the Sessionmgr implementation is landed.
+// an instance of this service in order to launch and display a |Sessionmgr|.
[Discoverable] // Created by sessionmgr application.
protocol Sessionmgr {
- // Launches a sessionmgr instance for a user identified by |user_id| and
- // specific TokenManager handles for ledger and agent_runner.
- // TODO(alhaad): Fold paramters into |UserContext|.
- Initialize(fuchsia.modular.auth.Account? account,
+ // Launches a sessionmgr instance identified by a unique device-local
+ // |session_id|. The uniqueness of |session_id| must be guaranteed by the
+ // caller, because |sessionmgr| creates an Environment namespace with the
+ // given |session_id|, and this will crash if we try to create an
+ // environment with a pre-existing name, because the services sessionmgr
+ // tries to access won't be available.
+ //
+ // TODO(MF-287): Address issues around client-generated session_id and
+ // initialization pattern of sessionmgr.
+ Initialize(string session_id,
+ fuchsia.modular.auth.Account? account,
fuchsia.modular.AppConfig session_shell,
fuchsia.modular.AppConfig story_shell,
bool use_session_shell_for_story_shell_factory,
