Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(105)

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

Created:
7 years, 2 months ago by Patrick Dubroy
Modified:
7 years, 2 months ago
CC:
chromium-reviews, 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) Patch
M base/mac/authorization_util.h View 1 chunk +13 lines, -4 lines 0 comments Download
M base/mac/authorization_util.mm View 4 chunks +22 lines, -13 lines 0 comments Download
M chrome/app/chromium_strings.grd View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/app/generated_resources.grd View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/app/google_chrome_strings.grd View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
A chrome/browser/password_manager/password_manager_util.h View 1 chunk +18 lines, -0 lines 0 comments Download
A chrome/browser/password_manager/password_manager_util_mac.mm View 1 2 3 1 chunk +39 lines, -0 lines 0 comments Download
A + chrome/browser/password_manager/password_manager_util_stub.cc View 1 chunk +3 lines, -5 lines 0 comments Download
M chrome/browser/ui/webui/options/password_manager_handler.h View 1 2 2 chunks +11 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/options/password_manager_handler.cc View 1 2 3 5 chunks +19 lines, -6 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 18 (0 generated)
Patrick Dubroy
Please take a look. thakis: Mac stuff gcasto: password_manager
7 years, 2 months ago (2013-10-18 09:32:49 UTC) #1
Patrick Dubroy
cc: wfh
7 years, 2 months ago (2013-10-18 09:52:15 UTC) #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 ...
7 years, 2 months ago (2013-10-18 11:56:51 UTC) #3
Patrick Dubroy
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 ...
7 years, 2 months ago (2013-10-18 13:15:53 UTC) #4
Nico
Since this is a somewhat contentious issue, I'd like to have buy-in from chrome-ux-leads to ...
7 years, 2 months ago (2013-10-18 14:14:24 UTC) #5
Patrick Dubroy
On 2013/10/18 14:14:24, Nico wrote: > Since this is a somewhat contentious issue, I'd like ...
7 years, 2 months ago (2013-10-18 14:30:58 UTC) #6
Nico
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. ...
7 years, 2 months ago (2013-10-18 14:47:57 UTC) #7
Patrick Dubroy
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. ...
7 years, 2 months ago (2013-10-18 15:24:00 UTC) #8
Nico
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> ...
7 years, 2 months ago (2013-10-18 15:28:45 UTC) #9
Nico
(also, it looks like gcasto is away until monday, maybe pick another password_manager/OWNER On Fri, ...
7 years, 2 months ago (2013-10-18 15:30:45 UTC) #10
Patrick Dubroy
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. ...
7 years, 2 months ago (2013-10-21 12:53:32 UTC) #11
Patrick Dubroy
Ping
7 years, 2 months ago (2013-10-22 15:36:04 UTC) #12
Nico
On 2013/10/22 15:36:04, Patrick Dubroy wrote: > Ping gcasto: ^
7 years, 2 months ago (2013-10-22 15:39:25 UTC) #13
Garrett Casto
lgtm
7 years, 2 months ago (2013-10-22 18:34:20 UTC) #14
Nico
lgtm
7 years, 2 months ago (2013-10-22 19:06:33 UTC) #15
Will Harris
lgtm
7 years, 2 months ago (2013-10-22 19:24:38 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dubroy@chromium.org/28713002/680001
7 years, 2 months ago (2013-10-23 15:43:26 UTC) #17
commit-bot: I haz the power
7 years, 2 months ago (2013-10-23 18:40:53 UTC) #18
Message was sent while issue was closed.
Change committed as 230479

Powered by Google App Engine
This is Rietveld 408576698