Skip to content

Commit

Permalink
Settings: Password View page uses requestCredentialDetails
Browse files Browse the repository at this point in the history
This CL introduces requestCredentialDetails API to password view page. The API is used to retrieve the full credential when:
- the user clicks to a password list item in chrome://settings/passwords
- opens the view page with URL chrome://settings/passwords/view?id=xy

MergePasswordsStoreCopiesMixin is not required in the page anymore.

Bug: 1345899
Change-Id: I253497da6399e34abacf631a787df73dc854a015
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3792013
Reviewed-by: Viktor Semeniuk <vsemeniuk@google.com>
Commit-Queue: Adem Derinel <derinel@google.com>
Cr-Commit-Position: refs/heads/main@{#1035492}
  • Loading branch information
deephand authored and Chromium LUCI CQ committed Aug 16, 2022
1 parent e484349 commit 1fde4bf
Show file tree
Hide file tree
Showing 15 changed files with 424 additions and 390 deletions.
16 changes: 13 additions & 3 deletions chrome/browser/resources/settings/autofill_page/autofill_page.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
search-label="$i18n{searchPasswords}"
search-term="{{passwordFilter_}}">
<passwords-section id="passwordSection" filter="[[passwordFilter_]]"
focus-config="[[focusConfig_]]" prefs="{{prefs}}">
focus-config="[[focusConfig_]]" prefs="{{prefs}}"
on-password-view-page-requested="onPasswordViewPageRequested_">
</passwords-section>
</settings-subpage>
</template>
Expand All @@ -45,7 +46,8 @@
search-label="$i18n{searchPasswords}"
search-term="{{passwordFilter_}}">
<passwords-device-section id="passwordDeviceSection"
filter="[[passwordFilter_]]" focus-config="[[focusConfig_]]">
filter="[[passwordFilter_]]" focus-config="[[focusConfig_]]"
on-password-view-page-requested="onPasswordViewPageRequested_">
</passwords-device-section>
</settings-subpage>
</template>
Expand Down Expand Up @@ -73,7 +75,8 @@
<template is="dom-if" route-path="/passwords/view">
<settings-subpage page-title="[[credential.urls.shown]]"
favicon-site-url="[[credential.urls.link]]">
<password-view credential="{{credential}}">
<password-view credential="{{credential}}"
on-password-view-page-requested="onPasswordViewPageRequested_">
</password-view>
</settings-subpage>
</template>
Expand All @@ -96,3 +99,10 @@
</settings-subpage>
</template>
</settings-animated-pages>
<if expr="is_chromeos">
<template is="dom-if" if="[[showPasswordPromptDialog]]" restamp>
<settings-password-prompt-dialog on-token-obtained="onTokenObtained"
on-close="onPasswordPromptClose">
</settings-password-prompt-dialog>
</template>
</if>
49 changes: 47 additions & 2 deletions chrome/browser/resources/settings/autofill_page/autofill_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
*/
import 'chrome://resources/cr_elements/cr_link_row/cr_link_row.js';
import 'chrome://resources/cr_elements/shared_vars_css.m.js';
// <if expr="is_chromeos">
import '../controls/password_prompt_dialog.js';
// </if>
import '../prefs/prefs.js';
import '../settings_page/settings_animated_pages.js';
import '../settings_page/settings_subpage.js';
Expand All @@ -25,9 +28,11 @@ import {Router} from '../router.js';
import {getTemplate} from './autofill_page.html.js';
import {PasswordCheckMixin} from './password_check_mixin.js';
import {PasswordManagerImpl} from './password_manager_proxy.js';
import {PasswordRequestorMixin} from './password_requestor_mixin.js';
import {PasswordViewPageInteractions, PasswordViewPageRequestedEvent, PasswordViewPageUrlParams, recordPasswordViewInteraction} from './password_view.js';

const SettingsAutofillPageElementBase =
PrefsMixin(PasswordCheckMixin(BaseMixin(PolymerElement)));
const SettingsAutofillPageElementBase = PrefsMixin(
PasswordCheckMixin(PasswordRequestorMixin(BaseMixin(PolymerElement))));

export class SettingsAutofillPageElement extends
SettingsAutofillPageElementBase {
Expand Down Expand Up @@ -90,6 +95,16 @@ export class SettingsAutofillPageElement extends
private enablePasswordViewPage_: string;
credential: chrome.passwordsPrivate.PasswordUiEntry|null;

// <if expr="is_chromeos">
override onPasswordPromptClose(event: CloseEvent) {
super.onPasswordPromptClose(event);
if (!this.tokenObtained &&
Router.getInstance().getCurrentRoute() === routes.PASSWORD_VIEW) {
Router.getInstance().navigateTo(routes.PASSWORDS);
}
}
// </if>

/**
* Shows the manage addresses sub page.
*/
Expand Down Expand Up @@ -119,6 +134,36 @@ export class SettingsAutofillPageElement extends
return this.leakedPasswords.length > 0 ? this.compromisedPasswordsCount :
'';
}

private onPasswordViewPageRequested_(event: PasswordViewPageRequestedEvent):
void {
const id = event.detail.entry.id;

this.requestCredentialDetails(id)
.then((passwordUiEntry: chrome.passwordsPrivate.PasswordUiEntry) => {
this.credential = passwordUiEntry;
recordPasswordViewInteraction(
PasswordViewPageInteractions.CREDENTIAL_FOUND);
if (Router.getInstance().getCurrentRoute() !== routes.PASSWORD_VIEW) {
// If the current route is not |routes.PASSWORD_VIEW|, then the
// credential is requested due to a row click in passwords list.
// Route to the view page to display the credential.
const params = new URLSearchParams();
params.set(PasswordViewPageUrlParams.ID, String(id));
Router.getInstance().navigateTo(routes.PASSWORD_VIEW, params);
}
})
.catch(() => {
if (Router.getInstance().getCurrentRoute() === routes.PASSWORD_VIEW) {
// If the credential is requested from the view page and is not
// retrieved, return to |routes.PASSWORDS|. There is nothing to show
// in |routes.PASSWORD_VIEW|.
recordPasswordViewInteraction(
PasswordViewPageInteractions.CREDENTIAL_NOT_FOUND);
Router.getInstance().navigateTo(routes.PASSWORDS);
}
});
}
}

declare global {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ import {getTemplate} from './password_edit_dialog.html.js';
import {PasswordManagerImpl} from './password_manager_proxy.js';
import {PasswordRequestorMixin} from './password_requestor_mixin.js';

export type SavedPasswordEditedEvent = CustomEvent<number>;
export type SavedPasswordEditedEvent =
CustomEvent<chrome.passwordsPrivate.PasswordUiEntry>;

const SAVED_PASSWORD_EDITED_EVENT_NAME = 'saved-password-edited';

Expand Down Expand Up @@ -531,19 +532,27 @@ export class PasswordEditDialogElement extends PasswordEditDialogElementBase {
.changeSavedPassword(this.existingEntry!.id, params)
.then(newId => {
if (this.isPasswordViewPageEnabled_) {
this.dispatchChangePasswordEvent_(newId);
const newEntry = {
...this.existingEntry!,
username: this.username_,
password: this.password_,
note: this.note_,
id: newId,
};
this.dispatchChangePasswordEvent_(newEntry);
}
})
.finally(() => {
this.close();
});
}

private dispatchChangePasswordEvent_(newId: number) {
private dispatchChangePasswordEvent_(
newEntry: chrome.passwordsPrivate.PasswordUiEntry) {
this.dispatchEvent(new CustomEvent(SAVED_PASSWORD_EDITED_EVENT_NAME, {
bubbles: true,
composed: true,
detail: newId,
detail: newEntry,
}));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,18 @@ import './passwords_shared.css.js';
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {loadTimeData} from '../i18n_setup.js';
import {routes} from '../route.js';
import {Router} from '../router.js';

import {getTemplate} from './password_list_item.html.js';
import {PasswordViewPageInteractions, PasswordViewPageUrlParams, recordPasswordViewInteraction} from './password_view.js';
import {PASSWORD_VIEW_PAGE_REQUESTED_EVENT_NAME, PasswordViewPageInteractions, recordPasswordViewInteraction} from './password_view.js';
import {ShowPasswordMixin, ShowPasswordMixinInterface} from './show_password_mixin.js';


declare global {
interface HTMLElementEventMap {
[PASSWORD_MORE_ACTIONS_CLICKED_EVENT_NAME]: PasswordMoreActionsClickedEvent;
[PASSWORD_VIEW_PAGE_CLICKED_EVENT_NAME]: PasswordViewPageClickedEvent;
}
}

export type PasswordViewPageClickedEvent = CustomEvent<PasswordListItemElement>;

export const PASSWORD_VIEW_PAGE_CLICKED_EVENT_NAME =
'password-view-page-clicked';

export type PasswordMoreActionsClickedEvent = CustomEvent<{
target: HTMLElement,
listItem: PasswordListItemElement,
Expand Down Expand Up @@ -132,16 +124,14 @@ export class PasswordListItemElement extends PasswordListItemElementBase {
if (!this.shouldShowSubpageButton_) {
return;
}
const params = new URLSearchParams();
params.set(PasswordViewPageUrlParams.ID, this.entry.id.toString());
recordPasswordViewInteraction(
PasswordViewPageInteractions.CREDENTIAL_ROW_CLICKED);
Router.getInstance().navigateTo(routes.PASSWORD_VIEW, params);
this.dispatchEvent(new CustomEvent(PASSWORD_VIEW_PAGE_CLICKED_EVENT_NAME, {
bubbles: true,
composed: true,
detail: this,
}));
this.dispatchEvent(
new CustomEvent(PASSWORD_VIEW_PAGE_REQUESTED_EVENT_NAME, {
bubbles: true,
composed: true,
detail: this,
}));
}

private onPasswordMoreActionsButtonTap_() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,15 @@ export interface PasswordManagerProxy {
undoRemoveSavedPasswordOrException(): void;

/**
* Gets the saved password for a given login pair.
* Gets the full credential for a given id.
* @param id The id for the password entry being being retrieved.
* @return A promise that resolves to the full |PasswordUiEntry|.
*/
requestCredentialDetails(id: number):
Promise<chrome.passwordsPrivate.PasswordUiEntry>;

/**
* Gets the saved password for a given id and reason.
* @param id The id for the password entry being being retrieved.
* @param reason The reason why the plaintext password is requested.
* @return A promise that resolves to the plaintext password.
Expand Down Expand Up @@ -430,6 +438,22 @@ export class PasswordManagerImpl implements PasswordManagerProxy {
chrome.passwordsPrivate.undoRemoveSavedPasswordOrException();
}

requestCredentialDetails(id: number) {
return new Promise<chrome.passwordsPrivate.PasswordUiEntry>(
(resolve, reject) => {
chrome.passwordsPrivate.requestCredentialDetails(
id,
(passwordUiEntry: chrome.passwordsPrivate.PasswordUiEntry) => {
if (chrome.runtime.lastError) {
reject(chrome.runtime.lastError.message);
return;
}

resolve(passwordUiEntry);
});
});
}

requestPlaintextPassword(
id: number, reason: chrome.passwordsPrivate.PlaintextReason) {
return new Promise<string>((resolve, reject) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// 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.

/**
* @fileoverview PasswordRequestorMixin is the mixin which bundles the
* |requestPlaintextPassword| API for conveniency. The mixin creates its own
* |BlockingRequestManager| in chromeos for handling the authentication.
* Elements implementing this mixin should include a
* 'settings-password-prompt-dialog' for Chrome OS.
* |requestPlaintextPassword| and |requestCredentialDetails| APIs for
* conveniency. The mixin creates its own |BlockingRequestManager| in chromeos
* for handling the authentication. Elements implementing this mixin should
* include a 'settings-password-prompt-dialog' for Chrome OS.
*/

// <if expr="is_chromeos">
import {assert} from 'chrome://resources/js/assert_ts.js';
// </if>
Expand Down Expand Up @@ -74,6 +76,28 @@ export const PasswordRequestorMixin = dedupingMixin(
// </if>
}

requestCredentialDetails(id: number):
Promise<chrome.passwordsPrivate.PasswordUiEntry> {
// <if expr="is_chromeos">
// If no password was found, refresh auth token and retry.
return new Promise((resolve, reject) => {
PasswordManagerImpl.getInstance()
.requestCredentialDetails(id)
.then(resolve)
.catch(() => {
this.tokenRequestManager.request(
() => PasswordManagerImpl.getInstance()
.requestCredentialDetails(id)
.then(resolve)
.catch(reject));
});
});
// </if>
// <if expr="not is_chromeos">
return PasswordManagerImpl.getInstance().requestCredentialDetails(id);
// </if>
}

// <if expr="is_chromeos">
/**
* When this event fired, it means that the password-prompt-dialog
Expand Down Expand Up @@ -112,6 +136,8 @@ export interface PasswordRequestorMixinInterface {
requestPlaintextPassword(
id: number,
reason: chrome.passwordsPrivate.PlaintextReason): Promise<string>;
requestCredentialDetails(id: number):
Promise<chrome.passwordsPrivate.PasswordUiEntry>;
// <if expr="is_chromeos">
onTokenObtained(e: CustomEvent<chrome.quickUnlockPrivate.TokenInfo>): void;
onPasswordPromptClose(event: CloseEvent): void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@
$i18n{editPasswordPasswordLabel}
</div>
<div class="row-container">
<input id="passwordInput" readonly value="[[getPassword_(password_)]]"
<input id="passwordInput" readonly
value="[[getPasswordOrFederationText_(credential)]]"
type="[[getPasswordInputType_(credential, isPasswordVisible_)]]"
aria-label="$i18n{editPasswordPasswordLabel}">
<template is="dom-if" if="[[!isFederated_(credential)]]" restamp>
Expand Down
Loading

0 comments on commit 1fde4bf

Please sign in to comment.