Add warning to Mac that passwords are shared between profiles through the underlying keychain.

Add warning to Mac that passwords are shared between profiles through the underlying keychain.

BUG=102176
TEST=on Mac, with more than one profile existing, a warning is shown on the settings/personal page about profile password sharing.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108137

[Miranda Callahan, 2011-10-31 22:21:09]

rsesek: review for sanity
estade: OWNER

[Miranda Callahan, 2011-11-01 13:52:33]

On 2011/10/31 22:21:09, Miranda Callahan wrote:
> rsesek: review for sanity
> estade: OWNER

Evan, rsesek doesn't feel qualified to review web_ui stuff, so this is all you.

[Evan Stade, 2011-11-01 15:34:23]

Code lgtm. The string should not have "on Mac" (just delete that part). We
probably want something similar for Linux since we use kwallet/gnome
equivalent. +mdm should be able to point out how to determine if we're
using one of these.
On Oct 31, 2011 3:21 PM, <mirandac@chromium.org> wrote:

> Reviewers: rsesek, Evan Stade,
>
> Message:
> rsesek: review for sanity
> estade: OWNER
>
> Description:
> Add warning to Mac that passwords are shared between profiles through the
> underlying keychain.
>
> BUG=102176
> TEST=on Mac, with more than one profile existing, a warning is shown on the
> settings/personal page about profile password sharing.
>
> Please review this at
http://codereview.chromium.**org/8430017/<http://codereview.chromium.org/8430...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/>
>
> Affected files:
>  M     chrome/app/generated_**resources.grd
>  M     chrome/browser/resources/**options/personal_options.css
>  M     chrome/browser/resources/**options/personal_options.html
>  M     chrome/browser/resources/**options/personal_options.js
>  M     chrome/browser/ui/webui/**options/personal_options_**handler.cc
>
>
> Index: chrome/app/generated_**resources.grd
> ==============================**==============================**=======
> --- chrome/app/generated_**resources.grd  (revision 107614)
> +++ chrome/app/generated_**resources.grd  (working copy)
> @@ -8055,6 +8055,9 @@
>           Manage saved passwords...
>         </message>
>       </if>
> +      <message name="IDS_OPTIONS_PASSWORDS_**MAC_WARNING" desc="The
> warning for OS X that passwords are shared across profiles in the
> keychain.">
> +          On Mac, passwords are saved to your Keychain and may be
> accessed or synced by other Chrome users sharing this OS X account.
> +      </message>
>       <message name="IDS_OPTIONS_PASSWORDS_**AUTOLOGIN" desc="The label
> of the 'autologinEnabled' checkbox">
>         Offer to sign into Google sites automatically with this account
>       </message>
> Index: chrome/browser/resources/**options/personal_options.css
> ==============================**==============================**=======
> --- chrome/browser/resources/**options/personal_options.css
> (revision 107614)
> +++ chrome/browser/resources/**options/personal_options.css
> (working copy)
> @@ -56,3 +56,7 @@
>  #enable-auto-login-checkbox {
>   margin-top: 10px;
>  }
> +
> +#mac-passwords-warning {
> +  margin-top: 10px;
> +}
> Index: chrome/browser/resources/**options/personal_options.html
> ==============================**==============================**=======
> --- chrome/browser/resources/**options/personal_options.html
>  (revision 107614)
> +++ chrome/browser/resources/**options/personal_options.html
>  (working copy)
> @@ -79,6 +79,8 @@
>         </div>
>         <div><button id="manage-passwords" i18n-content="manage_**
> passwords"
>             pref="profile.password_**manager_enabled"></button></**div>
> +        <div id="mac-passwords-warning" i18n-content="**
> macPasswordsWarning"
> +            hidden></div>
>         <!-- This makes the managed-banner appear when the "pref" is
>         configured by the IT administrator. -->
>         <input name="password_allow_show_**hidden" type="text"
> Index: chrome/browser/resources/**options/personal_options.js
> ==============================**==============================**=======
> --- chrome/browser/resources/**options/personal_options.js
>  (revision 107614)
> +++ chrome/browser/resources/**options/personal_options.js
>  (working copy)
> @@ -141,6 +141,9 @@
>         $('manage-passwords').disabled = true;
>       }
>
> +      $('mac-passwords-warning').**hidden =
> +          !(localStrings.getString('**macPasswordsWarning'));
> +
>       if (PersonalOptions.**disableAutofillManagement()) {
>         $('autofill-settings').**disabled = true;
>
> Index: chrome/browser/ui/webui/**options/personal_options_**handler.cc
> ==============================**==============================**=======
> --- chrome/browser/ui/webui/**options/personal_options_**handler.cc
> (revision 107614)
> +++ chrome/browser/ui/webui/**options/personal_options_**handler.cc
> (working copy)
> @@ -110,6 +110,13 @@
>       l10n_util::GetStringUTF16(IDS_**OPTIONS_PASSWORDS_NEVERSAVE));
>   localized_strings->SetString("**manage_passwords",
>       l10n_util::GetStringUTF16(IDS_**OPTIONS_PASSWORDS_MANAGE_**
> PASSWORDS));
> +#if defined(OS_MACOSX)
> +  ProfileManager* profile_manager = g_browser_process->profile_**
> manager();
> +  if (profile_manager->**GetNumberOfProfiles() > 1) {
> +    localized_strings->SetString("**macPasswordsWarning",
> +        l10n_util::GetStringUTF16(IDS_**OPTIONS_PASSWORDS_MAC_WARNING)**
> );
> +  }
> +#endif
>   localized_strings->SetString("**autologinEnabled",
>       l10n_util::GetStringUTF16(IDS_**OPTIONS_PASSWORDS_AUTOLOGIN));
>
>
>
>

[Robert Sesek, 2011-11-01 15:35:51]

On 2011/11/01 15:34:23, Evan Stade wrote:
> Code lgtm. The string should not have "on Mac" (just delete that part). We
> probably want something similar for Linux since we use kwallet/gnome
> equivalent. +mdm should be able to point out how to determine if we're
> using one of these.

No, this string was approved explicitly for Mac by Glen and Jeff. In our
discussions, we thought there was a solution already in place for Linux?

[mirandac, 2011-11-01 15:37:00]

Thanks for the review. In regards to the wording, there was a long discussion
about exactly what should be said, and it was thought that "on Mac" was
necessary -- see http://code.google.com/p/chromium/issues/detail?id=102176 for
details.

For linux, we don't have the same problems, as mdm was able to code around this
problem.

On 2011/11/01 15:34:23, Evan Stade wrote:
> Code lgtm. The string should not have "on Mac" (just delete that part). We
> probably want something similar for Linux since we use kwallet/gnome
> equivalent. +mdm should be able to point out how to determine if we're
> using one of these.
> On Oct 31, 2011 3:21 PM, <mailto:mirandac@chromium.org> wrote:
> 
> > Reviewers: rsesek, Evan Stade,
> >
> > Message:
> > rsesek: review for sanity
> > estade: OWNER
> >
> > Description:
> > Add warning to Mac that passwords are shared between profiles through the
> > underlying keychain.
> >
> > BUG=102176
> > TEST=on Mac, with more than one profile existing, a warning is shown on the
> > settings/personal page about profile password sharing.
> >
> > Please review this at
>
http://codereview.chromium.**org/8430017/%3Chttp://codereview.chromium.org/84...>
> >
> > SVN Base:
>
svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/>
> >
> > Affected files:
> >  M     chrome/app/generated_**resources.grd
> >  M     chrome/browser/resources/**options/personal_options.css
> >  M     chrome/browser/resources/**options/personal_options.html
> >  M     chrome/browser/resources/**options/personal_options.js
> >  M     chrome/browser/ui/webui/**options/personal_options_**handler.cc
> >
> >
> > Index: chrome/app/generated_**resources.grd
> > ==============================**==============================**=======
> > --- chrome/app/generated_**resources.grd  (revision 107614)
> > +++ chrome/app/generated_**resources.grd  (working copy)
> > @@ -8055,6 +8055,9 @@
> >           Manage saved passwords...
> >         </message>
> >       </if>
> > +      <message name="IDS_OPTIONS_PASSWORDS_**MAC_WARNING" desc="The
> > warning for OS X that passwords are shared across profiles in the
> > keychain.">
> > +          On Mac, passwords are saved to your Keychain and may be
> > accessed or synced by other Chrome users sharing this OS X account.
> > +      </message>
> >       <message name="IDS_OPTIONS_PASSWORDS_**AUTOLOGIN" desc="The label
> > of the 'autologinEnabled' checkbox">
> >         Offer to sign into Google sites automatically with this account
> >       </message>
> > Index: chrome/browser/resources/**options/personal_options.css
> > ==============================**==============================**=======
> > --- chrome/browser/resources/**options/personal_options.css
> > (revision 107614)
> > +++ chrome/browser/resources/**options/personal_options.css
> > (working copy)
> > @@ -56,3 +56,7 @@
> >  #enable-auto-login-checkbox {
> >   margin-top: 10px;
> >  }
> > +
> > +#mac-passwords-warning {
> > +  margin-top: 10px;
> > +}
> > Index: chrome/browser/resources/**options/personal_options.html
> > ==============================**==============================**=======
> > --- chrome/browser/resources/**options/personal_options.html
> >  (revision 107614)
> > +++ chrome/browser/resources/**options/personal_options.html
> >  (working copy)
> > @@ -79,6 +79,8 @@
> >         </div>
> >         <div><button id="manage-passwords" i18n-content="manage_**
> > passwords"
> >             pref="profile.password_**manager_enabled"></button></**div>
> > +        <div id="mac-passwords-warning" i18n-content="**
> > macPasswordsWarning"
> > +            hidden></div>
> >         <!-- This makes the managed-banner appear when the "pref" is
> >         configured by the IT administrator. -->
> >         <input name="password_allow_show_**hidden" type="text"
> > Index: chrome/browser/resources/**options/personal_options.js
> > ==============================**==============================**=======
> > --- chrome/browser/resources/**options/personal_options.js
> >  (revision 107614)
> > +++ chrome/browser/resources/**options/personal_options.js
> >  (working copy)
> > @@ -141,6 +141,9 @@
> >         $('manage-passwords').disabled = true;
> >       }
> >
> > +      $('mac-passwords-warning').**hidden =
> > +          !(localStrings.getString('**macPasswordsWarning'));
> > +
> >       if (PersonalOptions.**disableAutofillManagement()) {
> >         $('autofill-settings').**disabled = true;
> >
> > Index: chrome/browser/ui/webui/**options/personal_options_**handler.cc
> > ==============================**==============================**=======
> > --- chrome/browser/ui/webui/**options/personal_options_**handler.cc
> > (revision 107614)
> > +++ chrome/browser/ui/webui/**options/personal_options_**handler.cc
> > (working copy)
> > @@ -110,6 +110,13 @@
> >       l10n_util::GetStringUTF16(IDS_**OPTIONS_PASSWORDS_NEVERSAVE));
> >   localized_strings->SetString("**manage_passwords",
> >       l10n_util::GetStringUTF16(IDS_**OPTIONS_PASSWORDS_MANAGE_**
> > PASSWORDS));
> > +#if defined(OS_MACOSX)
> > +  ProfileManager* profile_manager = g_browser_process->profile_**
> > manager();
> > +  if (profile_manager->**GetNumberOfProfiles() > 1) {
> > +    localized_strings->SetString("**macPasswordsWarning",
> > +        l10n_util::GetStringUTF16(IDS_**OPTIONS_PASSWORDS_MAC_WARNING)**
> > );
> > +  }
> > +#endif
> >   localized_strings->SetString("**autologinEnabled",
> >       l10n_util::GetStringUTF16(IDS_**OPTIONS_PASSWORDS_AUTOLOGIN));
> >
> >
> >
> >

[Mike Mammarella, 2011-11-01 15:43:59]

Yeah, on Linux we now tag the passwords in gnome keyring/kwallet with a randomly
generated profile id so we know which profile owns each password.

[commit-bot: I haz the power, 2011-11-01 17:03:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mirandac@chromium.org/8430017/3001

[commit-bot: I haz the power, 2011-11-01 18:18:36]

Change committed as 108137

[Evan Stade, 2011-11-01 18:44:28]

ok. thanks for explanations.