Chromium Code Reviews
Help | Chromium Project | Sign in
(867)

Issue 28713002: [Mac] Add option to reauthenticate the OS user before revealing passwords. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 months ago by Patrick Dubroy gone til Apr 22
Modified:
5 months, 3 weeks ago
CC:
chromium-reviews_chromium.org, dbeam+watch-options_chromium.org, erikwright+watch_chromium.org
Visibility:
Public.

Description

[Mac] Add option to reauthenticate the OS user before revealing passwords.

When the flag is enabled and the user attempts to reveal a plaintext password
in chrome://settings/passwords, they will be prompted to reauthenticate with
their OS password. This matches Safari's behaviour on OS X.

BUG=303113
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=230479

Patch Set 1 : #

Patch Set 2 : Remove Mac headers from common file. #

Total comments: 2

Patch Set 3 : Add timeout. #

Total comments: 8

Patch Set 4 : Address thakis' comment. #

Patch Set 5 : Rebase. #

Patch Set 6 : Rerebase. #

Patch Set 7 : Rerebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+157 lines, -31 lines) Lint Patch
M base/mac/authorization_util.h View 1 chunk +13 lines, -4 lines 0 comments 0 errors Download
M base/mac/authorization_util.mm View 4 chunks +22 lines, -13 lines 0 comments ? errors Download
M chrome/app/chromium_strings.grd View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments ? errors Download
M chrome/app/generated_resources.grd View 1 chunk +6 lines, -0 lines 0 comments ? errors Download
M chrome/app/google_chrome_strings.grd View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments ? errors Download
M chrome/browser/about_flags.cc View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments 1 errors Download
A chrome/browser/password_manager/password_manager_util.h View 1 chunk +18 lines, -0 lines 0 comments 1 errors Download
A chrome/browser/password_manager/password_manager_util_mac.mm View 1 2 3 1 chunk +39 lines, -0 lines 0 comments ? errors Download
A + chrome/browser/password_manager/password_manager_util_stub.cc View 1 chunk +3 lines, -5 lines 0 comments 1 errors Download
M chrome/browser/ui/webui/options/password_manager_handler.h View 1 2 2 chunks +11 lines, -3 lines 0 comments 0 errors Download
M chrome/browser/ui/webui/options/password_manager_handler.cc View 1 2 3 5 chunks +19 lines, -6 lines 0 comments 0 errors Download
M chrome/chrome_browser.gypi View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments ? errors Download
M chrome/common/chrome_switches.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments 0 errors Download
M chrome/common/chrome_switches.cc View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 18
Patrick Dubroy gone til Apr 22
Please take a look. thakis: Mac stuff gcasto: password_manager
6 months ago #1
Patrick Dubroy gone til Apr 22
cc: wfh
6 months ago #2
Will Harris
https://codereview.chromium.org/28713002/diff/80001/chrome/browser/ui/webui/options/password_manager_handler.cc File chrome/browser/ui/webui/options/password_manager_handler.cc (right): https://codereview.chromium.org/28713002/diff/80001/chrome/browser/ui/webui/options/password_manager_handler.cc#newcode167 chrome/browser/ui/webui/options/password_manager_handler.cc:167: if (require_reauthentication_ && !is_user_authenticated_) { for users who leave ...
6 months ago #3
Patrick Dubroy gone til Apr 22
https://codereview.chromium.org/28713002/diff/80001/chrome/browser/ui/webui/options/password_manager_handler.cc File chrome/browser/ui/webui/options/password_manager_handler.cc (right): https://codereview.chromium.org/28713002/diff/80001/chrome/browser/ui/webui/options/password_manager_handler.cc#newcode167 chrome/browser/ui/webui/options/password_manager_handler.cc:167: if (require_reauthentication_ && !is_user_authenticated_) { On 2013/10/18 11:56:52, Will ...
6 months ago #4
Nico (ooo Apr 18 - Apr 20)
Since this is a somewhat contentious issue, I'd like to have buy-in from chrome-ux-leads to ...
6 months ago #5
Patrick Dubroy gone til Apr 22
On 2013/10/18 14:14:24, Nico wrote: > Since this is a somewhat contentious issue, I'd like ...
6 months ago #6
Nico (ooo Apr 18 - Apr 20)
https://codereview.chromium.org/28713002/diff/140001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/28713002/diff/140001/chrome/app/generated_resources.grd#newcode7538 chrome/app/generated_resources.grd:7538: + <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> is trying to show passwords. ...
6 months ago #7
Patrick Dubroy gone til Apr 22
https://codereview.chromium.org/28713002/diff/140001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/28713002/diff/140001/chrome/app/generated_resources.grd#newcode7538 chrome/app/generated_resources.grd:7538: + <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> is trying to show passwords. ...
6 months ago #8
Nico (ooo Apr 18 - Apr 20)
On Fri, Oct 18, 2013 at 8:24 AM, <dubroy@chromium.org> wrote: > > https://codereview.chromium.**org/28713002/diff/140001/** > chrome/app/generated_**resources.grd<https://codereview.chromium.org/28713002/diff/140001/chrome/app/generated_resources.grd> ...
6 months ago #9
Nico (ooo Apr 18 - Apr 20)
(also, it looks like gcasto is away until monday, maybe pick another password_manager/OWNER On Fri, ...
6 months ago #10
Patrick Dubroy gone til Apr 22
https://codereview.chromium.org/28713002/diff/140001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/28713002/diff/140001/chrome/app/generated_resources.grd#newcode7538 chrome/app/generated_resources.grd:7538: + <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> is trying to show passwords. ...
5 months, 4 weeks ago #11
Patrick Dubroy gone til Apr 22
Ping
5 months, 4 weeks ago #12
Nico (ooo Apr 18 - Apr 20)
On 2013/10/22 15:36:04, Patrick Dubroy wrote: > Ping gcasto: ^
5 months, 4 weeks ago #13
Garrett Casto
lgtm
5 months, 4 weeks ago #14
Nico (ooo Apr 18 - Apr 20)
lgtm
5 months, 4 weeks ago #15
Will Harris
lgtm
5 months, 4 weeks ago #16
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dubroy@chromium.org/28713002/680001
5 months, 3 weeks ago #17
I haz the power (commit-bot)
5 months, 3 weeks ago #18
Message was sent while issue was closed.
Change committed as 230479
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6