Skip to content

Commit

Permalink
[SafetyCheck] Add backend connection for unused site permissions
Browse files Browse the repository at this point in the history
This CL adds the connection between the web UI and the site settings
permissions handler. For this, it adds a new browser proxy and handler.
The handler currently returns mock data for testing purposes.

Screenshot: https://bugs.chromium.org/p/chromium/issues/detail?id=1345920#c89

Bug: 1345920
Change-Id: I5483d2ebe4d4a754df3568e07db2c8ab7a71bb17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4008498
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Side YILMAZ <sideyilmaz@chromium.org>
Reviewed-by: Theodore Olsauskas-Warren <sauski@google.com>
Commit-Queue: Tom Van Goethem <tov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1075118}
  • Loading branch information
tomvangoethem authored and Chromium LUCI CQ committed Nov 23, 2022
1 parent 0599248 commit 1670e28
Show file tree
Hide file tree
Showing 19 changed files with 250 additions and 14 deletions.
9 changes: 7 additions & 2 deletions chrome/app/settings_strings.grdp
Expand Up @@ -1971,8 +1971,13 @@
<message name="IDS_SETTINGS_SAFETY_CHECK_REVIEW" desc="Text for a button that allows users to review safety check findings, such as compromised passwords or blocklisted extensions.">
Review
</message>
<message name="IDS_SETTINGS_SAFETY_CHECK_UNUSED_SITE_PERMISSIONS_PRIMARY_LABEL" translateable="false" desc="The placeholder title for 'Unused Site Permissions' module. It is an element in safety check that shows the revoked permissions of unused websites.">
Permissions removed from unused websites
<message name="IDS_SETTINGS_SAFETY_CHECK_UNUSED_SITE_PERMISSIONS_HEADER_LABEL" desc="'Unused Site Permissions' shows the revoked permissions of unused websites. This label is the header of the module in the site settings page. The number represents the number of sites for which permissions were revoked.">
{NUM_SITES, plural,
=1 {Permissions removed from <ph name="BEGIN_BOLD">&lt;b&gt;</ph>1 site<ph name="END_BOLD">&lt;/b&gt;</ph> you haven’t visited recently}
other {Permissions removed from <ph name="BEGIN_BOLD">&lt;b&gt;</ph>{NUM_SITES} sites<ph name="END_BOLD">&lt;/b&gt;</ph> you haven’t visited recently}}
</message>
<message name="IDS_SETTINGS_SAFETY_CHECK_UNUSED_SITE_PERMISSIONS_HEADER_ARIA_LABEL" desc="'Unused Site Permissions' shows the revoked permissions of unused websites. This label is the aria label for the review button on the Privacy and Security settings page.">
Review removed permissions
</message>
<message name="IDS_SETTINGS_SAFETY_CHECK_UPDATES_PRIMARY_LABEL" desc="'Updates' is an element in safety check that shows the update status of Chrome.">
Updates
Expand Down
@@ -0,0 +1 @@
8d46e15dd9578988605e27e49c7481c274168f20
@@ -0,0 +1 @@
ff1115bd8a8745b081d74fc1b30216d8f6eabffb
1 change: 1 addition & 0 deletions chrome/browser/resources/settings/BUILD.gn
Expand Up @@ -362,6 +362,7 @@ build_webui("build") {
"settings_routes.ts",
"site_settings/constants.ts",
"site_settings/site_settings_mixin.ts",
"site_settings/site_settings_permissions_browser_proxy.ts",
"site_settings/site_settings_prefs_browser_proxy.ts",
"site_settings/website_usage_browser_proxy.ts",
]
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/resources/settings/lazy_load.ts
Expand Up @@ -201,6 +201,7 @@ export {SiteDetailsPermissionElement} from './site_settings/site_details_permiss
export {SiteEntryElement} from './site_settings/site_entry.js';
export {SiteListElement} from './site_settings/site_list.js';
export {SiteListEntryElement} from './site_settings/site_list_entry.js';
export {SiteSettingsPermissionsBrowserProxy, SiteSettingsPermissionsBrowserProxyImpl, UnusedSitePermissions} from './site_settings/site_settings_permissions_browser_proxy.js';
export {ChooserException, ContentSettingProvider, CookiePrimarySetting, DefaultContentSetting, NotificationPermission, OriginInfo, RawChooserException, RawSiteException, RecentSitePermissions, SiteException, SiteGroup, SiteSettingsPrefsBrowserProxy, SiteSettingsPrefsBrowserProxyImpl, ZoomLevelEntry} from './site_settings/site_settings_prefs_browser_proxy.js';
export {WebsiteUsageBrowserProxy, WebsiteUsageBrowserProxyImpl} from './site_settings/website_usage_browser_proxy.js';
export {ZoomLevelsElement} from './site_settings/zoom_levels.js';
Expand Down
Expand Up @@ -2,9 +2,9 @@
<settings-safety-check-child
id="safetyCheckChild"
icon-status="[[iconStatus_]]"
label="$i18n{safetyCheckUnusedSitePermissionsPrimaryLabel}"
label="[[headerString_]]"
button-label="$i18n{safetyCheckReview}"
button-aria-label="$i18n{safetyCheckReview}"
button-aria-label="$i18n{safetyCheckUnusedSitePermissionsHeaderAriaLabel}"
on-button-click="onButtonClick_"
role="presentation"
class="two-line">
Expand Down
Expand Up @@ -9,24 +9,32 @@
* some granted permissions.
*/

// clang-format off
import './safety_check_child.js';

import {WebUiListenerMixin} from 'chrome://resources/cr_elements/web_ui_listener_mixin.js';
import {PluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js';
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {routes} from '../route.js';
import {Router} from '../router.js';
import {UnusedSitePermissions, SiteSettingsPermissionsBrowserProxy, SiteSettingsPermissionsBrowserProxyImpl} from '../site_settings/site_settings_permissions_browser_proxy.js';

import {SafetyCheckIconStatus, SettingsSafetyCheckChildElement} from './safety_check_child.js';
import {getTemplate} from './safety_check_unused_site_permissions.html.js';
// clang-format on

export interface SettingsSafetyCheckUnusedSitePermissionsElement {
$: {
'safetyCheckChild': SettingsSafetyCheckChildElement,
};
}

const SettingsSafetyCheckUnusedSitePermissionsElementBase =
WebUiListenerMixin(PolymerElement);

export class SettingsSafetyCheckUnusedSitePermissionsElement extends
PolymerElement {
SettingsSafetyCheckUnusedSitePermissionsElementBase {
static get is() {
return 'settings-safety-check-unused-site-permissions';
}
Expand All @@ -43,11 +51,37 @@ export class SettingsSafetyCheckUnusedSitePermissionsElement extends
return SafetyCheckIconStatus.UNUSED_SITE_PERMISSIONS;
},
},

headerString_: String,
};
}

private headerString_: string;
private iconStatus_: SafetyCheckIconStatus;

private browserProxy_: SiteSettingsPermissionsBrowserProxy =
SiteSettingsPermissionsBrowserProxyImpl.getInstance();

override connectedCallback() {
super.connectedCallback();

// Register for review notification permission list updates.
this.addWebUIListener(
'unused-permission-review-list-maybe-changed',
(sites: UnusedSitePermissions[]) => {
this.onSitesChanged_(sites);
});

this.browserProxy_.getRevokedUnusedSitePermissionsList().then(
this.onSitesChanged_.bind(this));
}

private async onSitesChanged_(sites: UnusedSitePermissions[]) {
this.headerString_ =
await PluralStringProxyImpl.getInstance().getPluralString(
'safetyCheckUnusedSitePermissionsHeaderLabel', sites.length);
}

private onButtonClick_() {
Router.getInstance().navigateTo(
routes.SITE_SETTINGS, /* dynamicParams= */ undefined,
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/resources/settings/site_settings/OWNERS
@@ -1,3 +1,4 @@
sauski@google.com

per-file review_notification_permissions.*=rainhard@chromium.org
per-file review_notification_permissions.*=rainhard@chromium.org
per-file site_settings_permissions_browser_proxy.ts=rainhard@chromium.org
@@ -0,0 +1,46 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

/**
* @fileoverview A helper object used by the "Site Settings" to interact with
* the permission-related updates of the browser.
*/

// clang-format off
import {sendWithPromise} from 'chrome://resources/js/cr.js';
// clang-format on

export interface UnusedSitePermissions {
origin: string;
permissions: string[];
}

/**
* TODO(crbug.com/1383197): Move functions related to notification permission
* review here as well.
*/
export interface SiteSettingsPermissionsBrowserProxy {
/**
* Gets the unused origins along with the permissions they have been granted.
*/
getRevokedUnusedSitePermissionsList(): Promise<UnusedSitePermissions[]>;
}

export class SiteSettingsPermissionsBrowserProxyImpl implements
SiteSettingsPermissionsBrowserProxy {
getRevokedUnusedSitePermissionsList() {
return sendWithPromise('getRevokedUnusedSitePermissionsList');
}

static getInstance(): SiteSettingsPermissionsBrowserProxy {
return instance ||
(instance = new SiteSettingsPermissionsBrowserProxyImpl());
}

static setInstance(obj: SiteSettingsPermissionsBrowserProxy) {
instance = obj;
}
}

let instance: SiteSettingsPermissionsBrowserProxy|null = null;
2 changes: 2 additions & 0 deletions chrome/browser/ui/BUILD.gn
Expand Up @@ -1684,6 +1684,8 @@ static_library("ui") {
"webui/settings/site_settings_handler.h",
"webui/settings/site_settings_helper.cc",
"webui/settings/site_settings_helper.h",
"webui/settings/site_settings_permissions_handler.cc",
"webui/settings/site_settings_permissions_handler.h",
"webui/side_panel/bookmarks/bookmarks_page_handler.cc",
"webui/side_panel/bookmarks/bookmarks_page_handler.h",
"webui/side_panel/bookmarks/bookmarks_side_panel_ui.cc",
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/webui/settings/OWNERS
Expand Up @@ -7,6 +7,7 @@ per-file hats_handler*=sauski@google.com
per-file privacy_sandbox_handler*=file://components/privacy_sandbox/OWNERS
per-file *site_settings*=msramek@chromium.org
per-file *site_settings*=sauski@google.com
per-file site_settings_permissions_handler*=rainhard@chromium.org
per-file safe_browsing_handler*=msramek@chromium.org
per-file safe_browsing_handler*=sauski@google.com
per-file safety_check_handler*=andzaytsev@google.com,rainhard@chromium.org
Expand Down
Expand Up @@ -1992,8 +1992,6 @@ void AddSafetyCheckStrings(content::WebUIDataSource* html_source) {
{"safetyCheckIconWarningAriaLabel",
IDS_SETTINGS_SAFETY_CHECK_ICON_WARNING_ARIA_LABEL},
{"safetyCheckReview", IDS_SETTINGS_SAFETY_CHECK_REVIEW},
{"safetyCheckUnusedSitePermissionsPrimaryLabel",
IDS_SETTINGS_SAFETY_CHECK_UNUSED_SITE_PERMISSIONS_PRIMARY_LABEL},
{"safetyCheckUpdatesPrimaryLabel",
IDS_SETTINGS_SAFETY_CHECK_UPDATES_PRIMARY_LABEL},
{"safetyCheckUpdatesButtonAriaLabel", IDS_UPDATE_RECOMMENDED_DIALOG_TITLE},
Expand Down Expand Up @@ -2035,6 +2033,8 @@ void AddSafetyCheckStrings(content::WebUIDataSource* html_source) {
IDS_SETTINGS_SAFETY_CHECK_NOTIFICATION_PERMISSION_REVIEW_DONE_LABEL},
{"safetyCheckNotificationPermissionReviewBlockAllLabel",
IDS_SETTINGS_SAFETY_CHECK_NOTIFICATION_PERMISSION_REVIEW_BLOCK_ALL_LABEL},
{"safetyCheckUnusedSitePermissionsHeaderAriaLabel",
IDS_SETTINGS_SAFETY_CHECK_UNUSED_SITE_PERMISSIONS_HEADER_ARIA_LABEL},
#if BUILDFLAG(IS_WIN) && BUILDFLAG(GOOGLE_CHROME_BRANDING)
{"safetyCheckChromeCleanerPrimaryLabel",
IDS_SETTINGS_SAFETY_CHECK_CHROME_CLEANER_PRIMARY_LABEL},
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ui/webui/settings/settings_ui.cc
Expand Up @@ -61,6 +61,7 @@
#include "chrome/browser/ui/webui/settings/settings_startup_pages_handler.h"
#include "chrome/browser/ui/webui/settings/shared_settings_localized_strings_provider.h"
#include "chrome/browser/ui/webui/settings/site_settings_handler.h"
#include "chrome/browser/ui/webui/settings/site_settings_permissions_handler.h"
#include "chrome/browser/ui/webui/webui_util.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
Expand Down Expand Up @@ -207,6 +208,7 @@ SettingsUI::SettingsUI(content::WebUI* web_ui)
AddSettingsPageUIHandler(
std::make_unique<ClearBrowsingDataHandler>(web_ui, profile));
AddSettingsPageUIHandler(std::make_unique<SafetyCheckHandler>());
AddSettingsPageUIHandler(std::make_unique<SiteSettingsPermissionsHandler>());
AddSettingsPageUIHandler(std::make_unique<DownloadsHandler>(profile));
AddSettingsPageUIHandler(std::make_unique<ExtensionControlHandler>());
AddSettingsPageUIHandler(std::make_unique<FontHandler>(profile));
Expand Down Expand Up @@ -401,6 +403,9 @@ SettingsUI::SettingsUI(content::WebUI* web_ui)
plural_string_handler->AddLocalizedString(
"safetyCheckNotificationPermissionReviewPrimaryLabel",
IDS_SETTINGS_SAFETY_CHECK_REVIEW_NOTIFICATION_PERMISSIONS_PRIMARY_LABEL);
plural_string_handler->AddLocalizedString(
"safetyCheckUnusedSitePermissionsHeaderLabel",
IDS_SETTINGS_SAFETY_CHECK_UNUSED_SITE_PERMISSIONS_HEADER_LABEL);
web_ui->AddMessageHandler(std::move(plural_string_handler));

// Add the metrics handler to write uma stats.
Expand Down
@@ -0,0 +1,35 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ui/webui/settings/site_settings_permissions_handler.h"

SiteSettingsPermissionsHandler::SiteSettingsPermissionsHandler() = default;
SiteSettingsPermissionsHandler::~SiteSettingsPermissionsHandler() = default;

void SiteSettingsPermissionsHandler::HandleGetRevokedUnusedSitePermissionsList(
const base::Value::List& args) {
AllowJavascript();

CHECK_EQ(1U, args.size());
const base::Value& callback_id = args[0];

// TODO(crbug.com/1345920): Replace with content from Unused Site Permissions
// service.
base::Value::List result;
ResolveJavascriptCallback(callback_id, base::Value(std::move(result)));
}

void SiteSettingsPermissionsHandler::RegisterMessages() {
// Usage of base::Unretained(this) is safe, because web_ui() owns `this` and
// won't release ownership until destruction.
web_ui()->RegisterMessageCallback(
"getRevokedUnusedSitePermissionsList",
base::BindRepeating(&SiteSettingsPermissionsHandler::
HandleGetRevokedUnusedSitePermissionsList,
base::Unretained(this)));
}

void SiteSettingsPermissionsHandler::OnJavascriptAllowed() {}

void SiteSettingsPermissionsHandler::OnJavascriptDisallowed() {}
@@ -0,0 +1,34 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_SITE_SETTINGS_PERMISSIONS_HANDLER_H_
#define CHROME_BROWSER_UI_WEBUI_SETTINGS_SITE_SETTINGS_PERMISSIONS_HANDLER_H_

#include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"

/**
* This handler deals with the permission-related operations on the site
* settings page.
*/

class SiteSettingsPermissionsHandler : public settings::SettingsPageUIHandler {
public:
SiteSettingsPermissionsHandler();

~SiteSettingsPermissionsHandler() override;

private:
// SettingsPageUIHandler implementation.
void OnJavascriptAllowed() override;
void OnJavascriptDisallowed() override;

// WebUIMessageHandler implementation.
void RegisterMessages() override;

// Returns the list of origins that haven't been visited recently with
// associated permissions.
void HandleGetRevokedUnusedSitePermissionsList(const base::Value::List& args);
};

#endif // CHROME_BROWSER_UI_WEBUI_SETTINGS_SITE_SETTINGS_PERMISSIONS_HANDLER_H_
1 change: 1 addition & 0 deletions chrome/test/data/webui/settings/BUILD.gn
Expand Up @@ -141,6 +141,7 @@ non_preprocessed_files = [
"test_profile_info_browser_proxy.ts",
"test_reset_browser_proxy.ts",
"test_search_engines_browser_proxy.ts",
"test_site_settings_permissions_browser_proxy.ts",
"test_site_settings_prefs_browser_proxy.ts",
"test_util.ts",
"trusted_html.ts",
Expand Down
1 change: 1 addition & 0 deletions chrome/test/data/webui/settings/OWNERS
Expand Up @@ -24,4 +24,5 @@ per-file site_list_entry_tests.ts=sauski@google.com
per-file site_list_tests.ts=sauski@google.com
per-file site_settings_page_test.ts=sauski@google.com
per-file test_privacy_page_browser_proxy.ts=sauski@google.com
per-file test_site_settings_permissions_browser_proxy.ts=rainhard@chromium.org,sauski@google.com
per-file test_site_settings_prefs_browser_proxy.ts=sauski@google.com

0 comments on commit 1670e28

Please sign in to comment.