Skip to content

Commit

Permalink
Support user justification in warn dialog
Browse files Browse the repository at this point in the history 
This CL sets up a number of views to support user justification for
warning mode.

Bug: b/298614798
Change-Id: I803527db4bca07b692865ead311ad68fc8d4b152
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4874798
Reviewed-by: Aida Zolic <aidazolic@chromium.org>
Commit-Queue: Luca Accorsi <accorsi@google.com>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Aya Elsayed <ayaelattar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1210665}
  • Loading branch information
Luca Accorsi authored and Chromium LUCI CQ committed Oct 17, 2023
1 parent 3684f40 commit e8256fa
Show file tree
Hide file tree
Showing 8 changed files with 280 additions and 37 deletions.
4 changes: 4 additions & 0 deletions chrome/browser/ash/file_manager/file_manager_browsertest_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ class FileManagerBrowserTestBase
// Whether test should use report-only mode for the file transfer connector.
bool file_transfer_connector_report_only = false;

// Whether tests should set up justification mode for the file transfer
// connector.
bool bypass_requires_justification = false;

// Whether tests should enable V2 of search.
bool enable_search_v2 = false;

Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ash/file_manager/file_manager_browsertest_utils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ TestCase& TestCase::FileTransferConnectorReportOnlyMode() {
return *this;
}

TestCase& TestCase::BypassRequiresJustification() {
options.bypass_requires_justification = true;
return *this;
}

TestCase& TestCase::EnableSearchV2() {
options.enable_search_v2 = true;
return *this;
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ash/file_manager/file_manager_browsertest_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ struct TestCase {

TestCase& FileTransferConnectorReportOnlyMode();

TestCase& BypassRequiresJustification();

TestCase& EnableSearchV2();

TestCase& EnableLocalImageSearch();
Expand Down
107 changes: 79 additions & 28 deletions chrome/browser/ash/file_manager/file_manager_policy_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "components/policy/core/common/cloud/mock_cloud_policy_client.h"
#include "components/signin/public/identity_manager/identity_test_environment.h"
#include "content/public/test/browser_test.h"
#include "ui/views/controls/textarea/textarea.h"

namespace file_manager {
namespace {
Expand Down Expand Up @@ -355,26 +356,40 @@ class DlpFilesAppBrowserTestBase {
}
};

constexpr char kFileTransferConnectorSettingsForDlp[] = R"(
{
"service_provider": "google",
"enable": [
{
"source_destination_list": [
{
"sources": [{
"file_system_type": "%s"
}],
"destinations": [{
"file_system_type": "%s"
}]
}
],
"tags": ["dlp"]
}
],
"block_until_verdict": %s
})";
// Returns a file transfer connectors policy for DLP with the given settings.
std::string GetFileTransferConnectorsPolicyForDlp(
const std::string& source,
const std::string& destination,
bool report_only,
bool require_user_justification) {
auto sources = base::Value::List().Append(
base::Value::Dict().Set("file_system_type", source));

auto destinations = base::Value::List().Append(
base::Value::Dict().Set("file_system_type", destination));

auto source_destination_list = base::Value::List().Append(
base::Value::Dict()
.Set("sources", std::move(sources))
.Set("destinations", std::move(destinations)));

auto enable = base::Value::List().Append(
base::Value::Dict()
.Set("source_destination_list", std::move(source_destination_list))
.Set("tags", base::Value::List().Append("dlp")));

auto settings = base::Value::Dict();
settings.Set("service_provider", "google");
settings.Set("enable", std::move(enable));
settings.Set("block_until_verdict", report_only ? 0 : 1);

if (require_user_justification) {
settings.Set("require_justification_tags",
base::Value::List().Append("dlp"));
}

return settings.DebugString();
}

base::TimeDelta kResponseDelay = base::Seconds(0);

Expand Down Expand Up @@ -501,10 +516,10 @@ class FileTransferConnectorFilesAppBrowserTestBase {
<< " to " << *destination;
enterprise_connectors::test::SetAnalysisConnector(
profile->GetPrefs(), enterprise_connectors::FILE_TRANSFER,
base::StringPrintf(
kFileTransferConnectorSettingsForDlp, source->c_str(),
destination->c_str(),
options.file_transfer_connector_report_only ? "0" : "1"));
GetFileTransferConnectorsPolicyForDlp(
*source, *destination,
options.file_transfer_connector_report_only,
options.bypass_requires_justification));

// Create a FakeFilesRequestHandler that intercepts uploads and fakes
// responses.
Expand Down Expand Up @@ -566,6 +581,10 @@ class FileTransferConnectorFilesAppBrowserTestBase {
*output = base::NumberToString(expected_warned_files_.size());
return true;
}
if (name == "doesBypassRequireJustification") {
*output = options.bypass_requires_justification ? "true" : "false";
return true;
}
if (name == "setupScanningRunLoop") {
// Set the number of expected `FileTransferAnalysisDelegate`s. This is
// done to correctly notify when scanning has completed.
Expand Down Expand Up @@ -882,7 +901,8 @@ class FileTransferConnectorFilesAppBrowserTest
if (name == "verifyFileTransferWarningDialogAndProceed") {
const std::string* app_id = value.FindString("app_id");
CHECK_NE(app_id, nullptr);
VerifyFileTransferWarningDialogAndProceed(*app_id);
VerifyFileTransferWarningDialogAndProceed(
*app_id, GetOptions().bypass_requires_justification);
return true;
} else {
return FileTransferConnectorFilesAppBrowserTestBase::
Expand Down Expand Up @@ -945,7 +965,9 @@ class FileTransferConnectorFilesAppBrowserTest
EXPECT_TRUE(widget->IsClosed());
}

void VerifyFileTransferWarningDialogAndProceed(const std::string& app_id) {
void VerifyFileTransferWarningDialogAndProceed(
const std::string& app_id,
bool bypass_requires_justification) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
content::WebContents* web_contents = GetWebContentsForId(app_id);
CHECK_NE(web_contents, nullptr);
Expand Down Expand Up @@ -976,9 +998,32 @@ class FileTransferConnectorFilesAppBrowserTest
EXPECT_THAT(displayed_files,
::testing::UnorderedElementsAreArray(expected_warned_files_));

policy::FilesPolicyWarnDialog* dialog =
static_cast<policy::FilesPolicyWarnDialog*>(
widget->widget_delegate()->AsDialogDelegate());
ASSERT_TRUE(dialog);

// Verify that the dialog has a text area where the user can enter a
// justification if required.
views::Textarea* justification_area =
static_cast<views::Textarea*>(widget->GetRootView()->GetViewByID(
policy::PolicyDialogBase::
kEnterpriseConnectorsJustificationTextareaId));
if (bypass_requires_justification) {
EXPECT_NE(justification_area, nullptr);
EXPECT_FALSE(dialog->IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK));

justification_area->InsertText(
u"User justification",
ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
EXPECT_TRUE(dialog->IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK));

} else {
EXPECT_EQ(justification_area, nullptr);
EXPECT_TRUE(dialog->IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK));
}

// Close the dialog.
auto* dialog = static_cast<policy::FilesPolicyWarnDialog*>(
widget->widget_delegate()->AsDialogDelegate());
dialog->AcceptDialog();

// Verify that the dialog is closed.
Expand Down Expand Up @@ -1154,8 +1199,14 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
"transferConnectorFromUsbToDownloadsFlatMoveNewUX"),
FILE_TRANSFER_TEST_CASE_NEW_UX(
"transferConnectorFromUsbToDownloadsFlatWarnProceedNewUX"),
FILE_TRANSFER_TEST_CASE_NEW_UX("transferConnectorFromUsbToDownloadsFlat"
"WarnProceedWithJustificationNewUX")
.BypassRequiresJustification(),
FILE_TRANSFER_TEST_CASE_NEW_UX(
"transferConnectorFromUsbToDownloadsDeepWarnProceedNewUX"),
FILE_TRANSFER_TEST_CASE_NEW_UX("transferConnectorFromUsbToDownloadsDeep"
"WarnProceedWithJustificationNewUX")
.BypassRequiresJustification(),
FILE_TRANSFER_TEST_CASE_NEW_UX(
"transferConnectorFromUsbToDownloadsFlatWarnCancelNewUX"),
FILE_TRANSFER_TEST_CASE_NEW_UX(
Expand Down
138 changes: 133 additions & 5 deletions chrome/browser/ash/policy/dlp/dialogs/files_policy_warn_dialog.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,39 @@

#include <string>

#include "ash/public/cpp/style/color_provider.h"
#include "ash/style/ash_color_id.h"
#include "ash/style/typography.h"
#include "base/functional/bind.h"
#include "base/functional/callback_helpers.h"
#include "base/memory/weak_ptr.h"
#include "base/strings/string_number_conversions.h"
#include "chrome/browser/ash/policy/dlp/dialogs/files_policy_dialog.h"
#include "chrome/browser/ash/policy/dlp/dialogs/files_policy_dialog_utils.h"
#include "chrome/browser/ash/policy/dlp/files_policy_string_util.h"
#include "chrome/browser/chromeos/policy/dlp/dialogs/policy_dialog_base.h"
#include "chrome/browser/chromeos/policy/dlp/dlp_file_destination.h"
#include "chrome/browser/chromeos/policy/dlp/dlp_files_controller.h"
#include "chrome/browser/chromeos/policy/dlp/dlp_files_utils.h"
#include "chrome/common/chrome_features.h"
#include "chrome/grit/generated_resources.h"
#include "components/enterprise/data_controls/component.h"
#include "components/enterprise/data_controls/dlp_histogram_helper.h"
#include "components/strings/grit/components_strings.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/chromeos/strings/grit/ui_chromeos_strings.h"
#include "ui/views/background.h"
#include "ui/views/controls/textarea/textarea.h"
#include "ui/views/layout/box_layout.h"
#include "ui/views/layout/fill_layout.h"
#include "url/gurl.h"

namespace policy {
namespace {
// Maximum number of characters a user can write in the justification text area.
constexpr int kMaxBypassJustificationLength = 280;

// Returns the domain of the |destination|'s |url| if it can be
// obtained, or the full value otherwise, converted to u16string. Fails if
Expand Down Expand Up @@ -76,6 +88,37 @@ const std::u16string GetDestination(DlpFileDestination destination) {
? GetDestinationComponent(destination)
: GetDestinationURL(destination);
}

const std::u16string GetJustificationLabelText(dlp::FileAction action) {
std::u16string text;
// TODO(b/299870671): replace with final strings once available.
switch (action) {
case dlp::FileAction::kDownload:
text = u"downloading";
break;
case dlp::FileAction::kUpload:
text = u"uploading";
break;
case dlp::FileAction::kCopy:
text = u"copying";
break;
case dlp::FileAction::kMove:
text = u"moving";
break;
case dlp::FileAction::kOpen:
text = u"opening";
break;
case dlp::FileAction::kShare:
text = u"sharing";
break;
case dlp::FileAction::kTransfer:
case dlp::FileAction::kUnknown:
text = u"transferring";
break;
}
return u"Provide reason for " + text + u" (required):";
}

} // namespace

FilesPolicyWarnDialog::FilesPolicyWarnDialog(
Expand Down Expand Up @@ -104,10 +147,7 @@ FilesPolicyWarnDialog::FilesPolicyWarnDialog(
dialog_info_.GetLearnMoreURL().value(), upper_panel_);
}
MaybeAddConfidentialRows();

// TODO(b/299578935): Customize the warning dialog according to
// `warning_message`, `learn_more_url` and
// `bypass_requires_justification` values stored in `settings`.
MaybeAddJustificationPanel();

data_controls::DlpHistogramEnumeration(
data_controls::dlp::kFileActionWarnReviewedUMA, action);
Expand Down Expand Up @@ -238,7 +278,11 @@ std::u16string FilesPolicyWarnDialog::GetMessage() {

void FilesPolicyWarnDialog::ProceedWarning(
OnDlpRestrictionCheckedWithJustificationCallback callback) {
std::move(callback).Run(/*user_justification=*/absl::nullopt,
absl::optional<std::u16string> user_justification;
if (justification_field_) {
user_justification = justification_field_->GetText();
}
std::move(callback).Run(user_justification,
/*should_proceed=*/true);
}

Expand All @@ -248,6 +292,90 @@ void FilesPolicyWarnDialog::CancelWarning(
/*should_proceed=*/false);
}

void FilesPolicyWarnDialog::MaybeAddJustificationPanel() {
if (!dialog_info_.DoesBypassRequireJustification()) {
return;
}

// Disable the proceed button until some text is entered.
DialogDelegate::SetButtonEnabled(ui::DIALOG_BUTTON_OK, false);

views::View* justification_panel =
AddChildView(std::make_unique<views::View>());
justification_panel->SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical, gfx::Insets::TLBR(8, 24, 8, 24),
/*between_child_spacing=*/8));

const std::u16string justification_label_text =
GetJustificationLabelText(action_);

views::Label* justification_field_label = justification_panel->AddChildView(
std::make_unique<views::Label>(justification_label_text));
justification_field_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
justification_field_label->SetAllowCharacterBreak(true);
justification_field_label->SetFontList(
ash::TypographyProvider::Get()->ResolveTypographyToken(
ash::TypographyToken::kCrosLabel2));
justification_field_label->SetEnabledColor(
ash::ColorProvider::Get()->GetContentLayerColor(
ash::ColorProvider::ContentLayerType::kTextColorPrimary));

// Setting a themed rounded background does not work for text areas. As a
// workaround we set it for an external container and set the text area
// background to transparent.
views::View* justification_field_container =
justification_panel->AddChildView(std::make_unique<views::View>());
justification_field_container->SetLayoutManager(
std::make_unique<views::FillLayout>());
justification_field_container->SetBackground(
views::CreateThemedRoundedRectBackground(
ash::kColorAshControlBackgroundColorInactive, 8, 0));

justification_field_ = justification_field_container->AddChildView(
std::make_unique<views::Textarea>());
justification_field_->SetID(
PolicyDialogBase::kEnterpriseConnectorsJustificationTextareaId);
justification_field_->SetAccessibleName(justification_label_text);
justification_field_->SetController(this);
justification_field_->SetBackgroundColor(SK_ColorTRANSPARENT);
justification_field_->SetPreferredSize(
gfx::Size(justification_field_->GetPreferredSize().width(), 140));
justification_field_->SetBorder(
views::CreateEmptyBorder(gfx::Insets::TLBR(8, 16, 8, 16)));
justification_field_->SetFontList(
ash::TypographyProvider::Get()->ResolveTypographyToken(
ash::TypographyToken::kCrosLabel1));

justification_field_length_label_ =
justification_panel->AddChildView(std::make_unique<views::Label>());
justification_field_length_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
justification_field_length_label_->SetText(l10n_util::GetStringFUTF16(
IDS_DEEP_SCANNING_DIALOG_BYPASS_JUSTIFICATION_TEXT_LIMIT_LABEL,
base::NumberToString16(0),
base::NumberToString16(kMaxBypassJustificationLength)));
justification_field_length_label_->SetFontList(
ash::TypographyProvider::Get()->ResolveTypographyToken(
ash::TypographyToken::kCrosLabel2));
}

void FilesPolicyWarnDialog::ContentsChanged(
views::Textfield* sender,
const std::u16string& new_contents) {
if (justification_field_length_label_) {
justification_field_length_label_->SetText(l10n_util::GetStringFUTF16(
IDS_DEEP_SCANNING_DIALOG_BYPASS_JUSTIFICATION_TEXT_LIMIT_LABEL,
base::NumberToString16(new_contents.size()),
base::NumberToString16(kMaxBypassJustificationLength)));
}

if (new_contents.size() == 0 ||
new_contents.size() > kMaxBypassJustificationLength) {
DialogDelegate::SetButtonEnabled(ui::DIALOG_BUTTON_OK, false);
} else {
DialogDelegate::SetButtonEnabled(ui::DIALOG_BUTTON_OK, true);
}
}

BEGIN_METADATA(FilesPolicyWarnDialog, FilesPolicyDialog)
END_METADATA

Expand Down

0 comments on commit e8256fa

Please sign in to comment.