Reland "Source Demo Mode SWA content from downloaded demo-mode-app Ch…

…rome Component"

This is a reland of commit 43d80ae

Target //ash/webui/demo_mode_app_ui:unit_tests is now properly only
included for unofficial builds

Original change's description:
> Source Demo Mode SWA content from downloaded demo-mode-app Chrome Component
>
> We use a WebUIDataSource with a RequestFilter instead of a separate
> URLDataSource because we want some built-in resources for e.g.
> exposing a JS bridge to our Mojo APIs.
>
> Change-Id: I31fea5f5c2d2dca25cf02895a3dfb1004dccc988
> Bug: b/228388936
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3575540
> Commit-Queue: Jackson Tadie <jacksontadie@google.com>
> Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1013047}

Bug: b/228388936
Change-Id: Ie4c444445b989519430ee24959d4fcff415498e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3703699
Commit-Queue: Jackson Tadie <jacksontadie@google.com>
Reviewed-by: Christopher Lam <calamity@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1013991}

ash/webui/BUILD.gn

@@ -46,6 +46,10 @@ test("ash_webui_unittests") {
     "//ui/gl:test_support",
   ]
 
+  if (!is_official_build) {
+    deps += [ "//ash/webui/demo_mode_app_ui:unit_tests" ]
+  }
+
   # Tests for Camera App only work on Chrome OS device but not linux-chromeos.
   if (is_chromeos_device) {
     deps += [ "//ash/webui/camera_app_ui:unit_tests" ]

ash/webui/demo_mode_app_ui/BUILD.gn

@@ -3,7 +3,6 @@
 # found in the LICENSE file.
 
 import("//build/config/chromeos/ui_mode.gni")
-import("//chrome/test/base/js2gtest.gni")
 import("//third_party/closure_compiler/compile_js.gni")
 import("//tools/grit/preprocess_if_expr.gni")
 import("//ui/webui/resources/tools/generate_grd.gni")
@@ -31,6 +30,18 @@ static_library("demo_mode_app_ui") {
   ]
 }
 
+source_set("unit_tests") {
+  testonly = true
+  sources = [ "demo_mode_app_ui_unittests.cc" ]
+  deps = [
+    ":demo_mode_app_ui",
+    "//base",
+    "//base/test:test_support",
+    "//testing/gtest",
+    "//url",
+  ]
+}
+
 js_type_check("closure_compile") {
   deps = [ ":app" ]
   closure_flags = default_closure_args + mojom_js_args
@@ -41,14 +52,6 @@ js_library("app") {
   deps = [ "//ash/webui/demo_mode_app_ui/mojom:mojom_webui_js" ]
 }
 
-js2gtest("browser_tests_js") {
-  test_type = "mojo_lite_webui"
-
-  sources = [ "test/demo_mode_app_ui_browsertest.js" ]
-
-  defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
-}
-
 grd_prefix = "ash_demo_mode_app"
 mojo_grdp_file = "$target_gen_dir/demo_mode_app_mojo_resources.grdp"
 

ash/webui/demo_mode_app_ui/demo_mode_app_ui.cc

@@ -4,11 +4,17 @@
 
 #include "ash/webui/demo_mode_app_ui/demo_mode_app_ui.h"
 
+#include <memory>
+
 #include "ash/constants/ash_features.h"
 #include "ash/webui/demo_mode_app_ui/demo_mode_page_handler.h"
 #include "ash/webui/demo_mode_app_ui/url_constants.h"
 #include "ash/webui/grit/ash_demo_mode_app_resources.h"
 #include "ash/webui/grit/ash_demo_mode_app_resources_map.h"
+#include "base/containers/fixed_flat_set.h"
+#include "base/files/file_util.h"
+#include "base/memory/ref_counted_memory.h"
+#include "base/task/thread_pool.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/common/url_constants.h"
@@ -17,36 +23,86 @@
 
 namespace ash {
 
-DemoModeAppUIConfig::DemoModeAppUIConfig()
-    : content::WebUIConfig(content::kChromeUIScheme, kChromeUIDemoModeAppHost) {
-}
+DemoModeAppUIConfig::DemoModeAppUIConfig(
+    base::RepeatingCallback<base::FilePath()> component_path_producer)
+    : content::WebUIConfig(content::kChromeUIScheme, kChromeUIDemoModeAppHost),
+      component_path_producer_(std::move(component_path_producer)) {}
 
 DemoModeAppUIConfig::~DemoModeAppUIConfig() = default;
 
 std::unique_ptr<content::WebUIController>
 DemoModeAppUIConfig::CreateWebUIController(content::WebUI* web_ui) {
-  return std::make_unique<DemoModeAppUI>(web_ui);
+  return std::make_unique<DemoModeAppUI>(web_ui,
+                                         component_path_producer_.Run());
 }
 
 bool DemoModeAppUIConfig::IsWebUIEnabled(
     content::BrowserContext* browser_context) {
   return ash::features::IsDemoModeSWAEnabled();
 }
 
-DemoModeAppUI::DemoModeAppUI(content::WebUI* web_ui)
+scoped_refptr<base::RefCountedMemory> ReadFile(
+    const base::FilePath& absolute_resource_path) {
+  std::string data;
+  base::ReadFileToString(absolute_resource_path, &data);
+  return base::RefCountedString::TakeString(&data);
+}
+
+bool ShouldSourceFromComponent(
+    const base::flat_set<std::string>& webui_resource_paths,
+    const std::string& path) {
+  // TODO(b/232945108): Consider changing this logic to check if the absolute
+  // path exists in the component. This would still allow us show the default
+  // WebUI resource if the requested path isn't found.
+  return !webui_resource_paths.contains(path);
+}
+
+void DemoModeAppUI::SourceDataFromComponent(
+    const base::FilePath& component_path,
+    const std::string& resource_path,
+    content::WebUIDataSource::GotDataCallback callback) {
+  // Convert to GURL to strip out query params and URL fragments
+  //
+  // TODO (b/234170189): Verify that query params won't be used in the prod Demo
+  // App, or add support for them here instead of ignoring them.
+  GURL full_url = GURL(kChromeUIDemoModeAppURL + resource_path);
+  // Trim leading slash from path
+  std::string path = full_url.path().substr(1);
+
+  base::FilePath absolute_resource_path = component_path.AppendASCII(path);
+
+  base::ThreadPool::PostTaskAndReplyWithResult(
+      FROM_HERE, {base::MayBlock()},
+      base::BindOnce(&ReadFile, absolute_resource_path), std::move(callback));
+}
+
+DemoModeAppUI::DemoModeAppUI(content::WebUI* web_ui,
+                             base::FilePath component_path)
     : ui::MojoWebUIController(web_ui) {
-  content::WebUIDataSource* html_source =
+  // We tack the resource path onto this component path, so CHECK that it's
+  // absolute so ".." parent references can't be used as an exploit
+  DCHECK(component_path.IsAbsolute());
+  content::WebUIDataSource* data_source =
       content::WebUIDataSource::CreateAndAdd(
           web_ui->GetWebContents()->GetBrowserContext(),
           kChromeUIDemoModeAppHost);
 
+  base::flat_set<std::string> webui_resource_paths;
   // Add required resources.
   for (size_t i = 0; i < kAshDemoModeAppResourcesSize; ++i) {
-    html_source->AddResourcePath(kAshDemoModeAppResources[i].path,
+    data_source->AddResourcePath(kAshDemoModeAppResources[i].path,
                                  kAshDemoModeAppResources[i].id);
+    webui_resource_paths.insert(kAshDemoModeAppResources[i].path);
   }
 
-  html_source->SetDefaultResource(IDR_ASH_DEMO_MODE_APP_DEMO_MODE_APP_HTML);
+  data_source->SetDefaultResource(IDR_ASH_DEMO_MODE_APP_DEMO_MODE_APP_HTML);
+  // Add empty string so default resource is still shown for
+  // chrome://demo-mode-app
+  webui_resource_paths.insert("");
+
+  data_source->SetRequestFilter(
+      base::BindRepeating(&ShouldSourceFromComponent, webui_resource_paths),
+      base::BindRepeating(&SourceDataFromComponent, component_path));
 }
 
 DemoModeAppUI::~DemoModeAppUI() = default;

ash/webui/demo_mode_app_ui/demo_mode_app_ui.h

@@ -6,6 +6,8 @@
 #define ASH_WEBUI_DEMO_MODE_APP_UI_DEMO_MODE_APP_UI_H_
 
 #include "ash/webui/demo_mode_app_ui/mojom/demo_mode_app_ui.mojom.h"
+#include "base/files/file_path.h"
+#include "content/public/browser/web_ui_data_source.h"
 #include "content/public/browser/webui_config.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
@@ -16,20 +18,32 @@ namespace ash {
 
 class DemoModeAppUIConfig : public content::WebUIConfig {
  public:
-  DemoModeAppUIConfig();
+  explicit DemoModeAppUIConfig(
+      base::RepeatingCallback<base::FilePath()> component_path_producer);
   ~DemoModeAppUIConfig() override;
 
   std::unique_ptr<content::WebUIController> CreateWebUIController(
       content::WebUI* web_ui) override;
 
   bool IsWebUIEnabled(content::BrowserContext* browser_context) override;
+
+ private:
+  // Callback that provides the demo app component path to the WebUI controller.
+  // The path can't be passed directly into the DemoModeAppUIConfig constructor
+  // because the config is created during startup, whereas the component isn't
+  // loaded until the active demo session has started
+  //
+  // TODO(b/234174220): Consider creating a Delegate class that provides the
+  // component path instead
+  base::RepeatingCallback<base::FilePath()> component_path_producer_;
 };
 
 // The WebUI for chrome://demo-mode-app
 class DemoModeAppUI : public ui::MojoWebUIController,
                       public mojom::demo_mode::PageHandlerFactory {
  public:
-  explicit DemoModeAppUI(content::WebUI* web_ui);
+  explicit DemoModeAppUI(content::WebUI* web_ui,
+                         base::FilePath component_base_path);
   ~DemoModeAppUI() override;
 
   DemoModeAppUI(const DemoModeAppUI&) = delete;
@@ -38,6 +52,12 @@ class DemoModeAppUI : public ui::MojoWebUIController,
   void BindInterface(
       mojo::PendingReceiver<mojom::demo_mode::PageHandlerFactory> factory);
 
+  // Visible for testing
+  static void SourceDataFromComponent(
+      const base::FilePath& component_path,
+      const std::string& resource_path,
+      content::WebUIDataSource::GotDataCallback callback);
+
  private:
   // mojom::DemoModePageHandlerFactory
   void CreatePageHandler(

ash/webui/demo_mode_app_ui/demo_mode_app_ui_unittests.cc

@@ -0,0 +1,112 @@
+// Copyright 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+#include <string>
+
+#include "ash/webui/demo_mode_app_ui/demo_mode_app_ui.h"
+#include "base/callback.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/memory/ref_counted_memory.h"
+#include "base/strings/strcat.h"
+#include "base/test/task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/url_util.h"
+
+namespace ash {
+namespace {
+
+const std::string kFileContents = "Test File Contents";
+
+class DemoModeAppUITest : public testing::Test {
+ protected:
+  DemoModeAppUITest() = default;
+  ~DemoModeAppUITest() override = default;
+
+  void SetUp() override {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    base::File file = base::CreateAndOpenTemporaryFileInDir(
+        temp_dir_.GetPath(), &content_file_path_);
+    base::WriteFile(content_file_path_, kFileContents);
+
+    scheme_registry_ = std::make_unique<url::ScopedSchemeRegistryForTests>();
+    url::AddStandardScheme("chrome", url::SCHEME_WITH_HOST);
+  }
+
+  base::FilePath content_file_path_;
+  base::ScopedTempDir temp_dir_;
+  std::unique_ptr<url::ScopedSchemeRegistryForTests> scheme_registry_;
+  base::test::TaskEnvironment task_environment_;
+};
+
+void VerifyDataResponse(std::string expected_response,
+                        base::OnceClosure quit_closure,
+                        scoped_refptr<base::RefCountedMemory> data_response) {
+  std::string result(data_response->front_as<char>(), data_response->size());
+  EXPECT_EQ(result, expected_response);
+  std::move(quit_closure).Run();
+}
+
+TEST_F(DemoModeAppUITest, SourceDataFromComponent) {
+  base::RunLoop run_loop;
+  DemoModeAppUI::SourceDataFromComponent(
+      temp_dir_.GetPath(), content_file_path_.BaseName().MaybeAsASCII(),
+      base::BindOnce(&VerifyDataResponse, kFileContents,
+                     run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
+TEST_F(DemoModeAppUITest, SourceDataFromComponentQueryParam) {
+  base::RunLoop run_loop;
+  std::string resource_path_with_query_param =
+      content_file_path_.BaseName().MaybeAsASCII() + "?testparam=testvalue";
+
+  DemoModeAppUI::SourceDataFromComponent(
+      temp_dir_.GetPath(), resource_path_with_query_param,
+      base::BindOnce(&VerifyDataResponse, kFileContents,
+                     run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
+TEST_F(DemoModeAppUITest, SourceDataFromComponentURLFragment) {
+  base::RunLoop run_loop;
+  std::string resource_path_with_url_fragment =
+      content_file_path_.BaseName().MaybeAsASCII() + "#frag";
+
+  DemoModeAppUI::SourceDataFromComponent(
+      temp_dir_.GetPath(), resource_path_with_url_fragment,
+      base::BindOnce(&VerifyDataResponse, kFileContents,
+                     run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
+TEST_F(DemoModeAppUITest, SourceDataFromComponentQueryParamAndURLFragment) {
+  base::RunLoop run_loop;
+  std::string resource_path_with_url_fragment =
+      content_file_path_.BaseName().MaybeAsASCII() +
+      "?testparam=testvalue#frag";
+
+  DemoModeAppUI::SourceDataFromComponent(
+      temp_dir_.GetPath(), resource_path_with_url_fragment,
+      base::BindOnce(&VerifyDataResponse, kFileContents,
+                     run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
+TEST_F(DemoModeAppUITest, SourceDataFromComponentParentDirReference) {
+  base::RunLoop run_loop;
+  // Treat temp_dir_ as the parent of the component directory here, that
+  // a malicious ".."-containing path may be trying to access
+  base::ScopedTempDir component_dir;
+  ASSERT_TRUE(component_dir.CreateUniqueTempDirUnderPath(temp_dir_.GetPath()));
+  std::string resource_path_with_parent_ref =
+      "../" + content_file_path_.BaseName().MaybeAsASCII();
+
+  DemoModeAppUI::SourceDataFromComponent(
+      component_dir.GetPath(), resource_path_with_parent_ref,
+      base::BindOnce(&VerifyDataResponse, "", run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
+}  // namespace
+}  // namespace ash

ash/webui/demo_mode_app_ui/url_constants.cc

@@ -8,5 +8,6 @@ namespace ash {
 
 const char kChromeUIDemoModeAppHost[] = "demo-mode-app";
 const char kChromeUIDemoModeAppURL[] = "chrome://demo-mode-app/";
+// TODO(b/232019361): Add untrusted demo mode app url to constants here
 
 }  // namespace ash