Kerberos: Implement 'Remember password'

Implements the UI and Chrome backend aspects of remembering the last
password for each Kerberos account. The password is stored by the
Kerberos daemon and never leaves it. Chrome just sends an empty password
if it wants the daemon to reuse the stored one.

BUG=chromium:952243
TEST=Turn KerberosEnabled policy on, add account in the Kerberos
     settings page, click 'Remember password' checkbox, click the 'Add'
     button. Next time you sign in (there's no way to enforce that
     right now, so you might have to wait a while) the password should
     be preset and the 'Add' button should be focused. If you overwrite
     the password, the new password should be taken.

Change-Id: If2486659cbf87216a2f8dd4a4362e39935a7f47e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1634863
Commit-Queue: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#668606}

chrome/app/settings_strings.grdp

@@ -1671,6 +1671,9 @@
     <message name="IDS_SETTINGS_ADD_KERBEROS_ACCOUNT" desc="In Add Kerberos Accounts dialog, the title of the dialog.">
       Add Kerberos Account
     </message>
+    <message name="IDS_SETTINGS_ADD_KERBEROS_ACCOUNT_REMEMBER_PASSWORD" desc="In Add Kerberos Accounts dialog, label of the checkbox to remember the password.">
+      Remember password
+    </message>
     <message name="IDS_SETTINGS_KERBEROS_USERNAME" desc="Title for the input that lets users specify their username for a Kerberos account.">
       Username
     </message>

chrome/browser/resources/settings/people_page/kerberos_accounts.html

@@ -117,7 +117,7 @@
     </div>
 
     <template is="dom-if" if="[[showAddAccountDialog_]]" restamp>
-      <kerberos-add-account-dialog username="[[addAccountPresetUsername_]]"
+      <kerberos-add-account-dialog preset-account="[[selectedAccount_]]"
           on-close="onAddAccountDialogClosed_">
       </kerberos-add-account-dialog>
     </template>

chrome/browser/resources/settings/people_page/kerberos_accounts.js

@@ -31,16 +31,10 @@ Polymer({
     },
 
     /**
-     * The targeted account for menu operations.
+     * The targeted account for menu and other operations.
      * @private {?settings.KerberosAccount}
      */
-    actionMenuAccount_: Object,
-
-    /** @private */
-    addAccountPresetUsername_: {
-      type: String,
-      value: '',
-    },
+    selectedAccount_: Object,
 
     /** @private */
     showAddAccountDialog_: Boolean,
@@ -76,7 +70,7 @@ Polymer({
    * @private
    */
   onAddAccountClick_: function(event) {
-    this.addAccountPresetUsername_ = '';
+    this.selectedAccount_ = null;
     this.showAddAccountDialog_ = true;
   },
 
@@ -85,7 +79,7 @@ Polymer({
    * @private
    */
   onReauthenticationClick_: function(event) {
-    this.addAccountPresetUsername_ = event.model.item.principalName;
+    this.selectedAccount_ = event.model.item;
     this.showAddAccountDialog_ = true;
   },
 
@@ -110,7 +104,7 @@ Polymer({
    * @private
    */
   onAccountActionsMenuButtonClick_: function(event) {
-    this.actionMenuAccount_ = event.model.item;
+    this.selectedAccount_ = event.model.item;
     /** @type {!CrActionMenuElement} */ (this.$$('cr-action-menu'))
         .showAt(event.target);
   },
@@ -121,26 +115,26 @@ Polymer({
    */
   closeActionMenu_: function() {
     this.$$('cr-action-menu').close();
-    this.actionMenuAccount_ = null;
+    this.selectedAccount_ = null;
   },
 
   /**
-   * Removes |this.actionMenuAccount_|.
+   * Removes |this.selectedAccount_|.
    * @private
    */
   onRemoveAccountClick_: function() {
     this.browserProxy_.removeAccount(
-        /** @type {!settings.KerberosAccount} */ (this.actionMenuAccount_));
+        /** @type {!settings.KerberosAccount} */ (this.selectedAccount_));
     this.closeActionMenu_();
   },
 
   /**
-   * Sets |this.actionMenuAccount_| as active Kerberos account.
+   * Sets |this.selectedAccount_| as active Kerberos account.
    * @private
    */
   onSetAsActiveAccountClick_: function() {
     this.browserProxy_.setAsActiveAccount(
-        /** @type {!settings.KerberosAccount} */ (this.actionMenuAccount_));
+        /** @type {!settings.KerberosAccount} */ (this.selectedAccount_));
     this.closeActionMenu_();
   },
 

chrome/browser/resources/settings/people_page/kerberos_accounts_browser_proxy.js

@@ -15,6 +15,7 @@ cr.exportPath('settings');
  *   principalName: string,
  *   isSignedIn: boolean,
  *   isActive: boolean,
+ *   hasRememberedPassword: boolean,
  *   pic: string,
  * }}
  */
@@ -61,9 +62,10 @@ cr.define('settings', function() {
      * Attempts to add a new (or update an existing) Kerberos account.
      * @param {string} principalName Kerberos principal (user@realm.com).
      * @param {string} password Account password.
+     * @param {boolean} rememberPassword Whether to store the password.
      * @return {!Promise<!settings.KerberosErrorType>}
      */
-    addAccount(principalName, password) {}
+    addAccount(principalName, password, rememberPassword) {}
 
     /**
      * Removes |account| from the set of Kerberos accounts.
@@ -89,8 +91,9 @@ cr.define('settings', function() {
     }
 
     /** @override */
-    addAccount(principalName, password) {
-      return cr.sendWithPromise('addKerberosAccount', principalName, password);
+    addAccount(principalName, password, rememberPassword) {
+      return cr.sendWithPromise(
+          'addKerberosAccount', principalName, password, rememberPassword);
     }
 
     /** @override */

chrome/browser/resources/settings/people_page/kerberos_add_account_dialog.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 
 <link rel="import" href="chrome://resources/cr_elements/cr_button/cr_button.html">
+<link rel="import" href="chrome://resources/cr_elements/cr_checkbox/cr_checkbox.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_dialog/cr_dialog.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_input/cr_input.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
@@ -37,14 +38,18 @@
         </div>
         <div id="credentials">
           <cr-input id="username" label="$i18n{kerberosUsername}"
-              value="{{username}}" invalid="[[showError_(usernameErrorText_)]]"
+              value="{{username_}}" invalid="[[showError_(usernameErrorText_)]]"
               error-message="[[usernameErrorText_]]">
           </cr-input>
           <cr-input id="password" type="password"
               label="$i18n{kerberosPassword}" value="{{password_}}"
               invalid="[[showError_(passwordErrorText_)]]"
-              error-message="[[passwordErrorText_]]">
+              error-message="[[passwordErrorText_]]"
+              on-input="onPasswordInput_">
           </cr-input>
+          <cr-checkbox id="rememberPassword" checked="{{rememberPassword_}}">
+            $i18n{addKerberosAccountRememberPassword}
+          </cr-checkbox>
         </div>
       </div>
       <div slot="button-container">

chrome/browser/resources/settings/people_page/kerberos_add_account_dialog.js

@@ -13,7 +13,15 @@ Polymer({
   behaviors: [I18nBehavior],
 
   properties: {
-    username: {
+    /**
+     * If set, some fields are preset from this account (like username or
+     * whether to remember the password).
+     * @type {?settings.KerberosAccount}
+     */
+    presetAccount: Object,
+
+    /** @private */
+    username_: {
       type: String,
       value: '',
     },
@@ -24,6 +32,12 @@ Polymer({
       value: '',
     },
 
+    /** @private */
+    rememberPassword_: {
+      type: Boolean,
+      value: false,
+    },
+
     /** @private */
     generalErrorText_: {
       type: String,
@@ -55,15 +69,31 @@ Polymer({
   /** @private {!settings.KerberosErrorType} */
   lastError_: settings.KerberosErrorType.kNone,
 
+  /** @private {boolean} */
+  useRememberedPassword_: false,
+
   /** @override */
   attached: function() {
     this.$.dialog.showModal();
 
-    // If a non-empty username is preset, make the UI read-only.
-    // Note: At least the focus() part needs to be after showModal.
-    if (this.username) {
-      this.$.username.disabled = true;
+    if (this.presetAccount) {
+      // Preset username and make UI read-only.
+      // Note: At least the focus() part needs to be after showModal.
+      this.username_ = this.presetAccount.principalName;
+      this.$.username.readonly = true;
       this.$.password.focus();
+
+      if (this.presetAccount.hasRememberedPassword) {
+        // The daemon knows the user's password, so prefill the password field
+        // with some string (Chrome does not know the actual password for
+        // security reasons). If the user does not change it, an empty password
+        // is sent to the daemon, which is interpreted as "use remembered
+        // password". Also, keep remembering the password by default.
+        const FAKE_PASSWORD = 'xxxxxxxx';
+        this.password_ = FAKE_PASSWORD;
+        this.rememberPassword_ = true;
+        this.useRememberedPassword_ = true;
+      }
     }
   },
 
@@ -77,8 +107,11 @@ Polymer({
     this.inProgress_ = true;
     this.updateErrorMessages_(settings.KerberosErrorType.kNone);
 
+    // An empty password triggers the Kerberos daemon to use the remembered one.
+    const passwordToSubmit = this.useRememberedPassword_ ? '' : this.password_;
+
     settings.KerberosAccountsBrowserProxyImpl.getInstance()
-        .addAccount(this.username, this.password_)
+        .addAccount(this.username_, passwordToSubmit, this.rememberPassword_)
         .then(error => {
           this.inProgress_ = false;
 
@@ -93,6 +126,13 @@ Polymer({
         });
   },
 
+  /** @private */
+  onPasswordInput_: function() {
+    // On first input, don't reuse the remembered password, but submit the
+    // changed one.
+    this.useRememberedPassword_ = false;
+  },
+
   /**
    * @param {!settings.KerberosErrorType} error Current error enum
    * @private

chrome/browser/ui/webui/settings/chromeos/kerberos_accounts_handler.cc

@@ -74,13 +74,15 @@ void KerberosAccountsHandler::OnListAccounts(
 
   for (int n = 0; n < response.accounts_size(); ++n) {
     const kerberos::Account& account = response.accounts(n);
-    const bool is_active = account.principal_name() == active_principal;
 
     base::DictionaryValue account_dict;
     account_dict.SetString("principalName", account.principal_name());
     account_dict.SetString("krb5conf", account.krb5conf());
     account_dict.SetBoolean("isSignedIn", account.tgt_validity_seconds() > 0);
-    account_dict.SetBoolean("isActive", is_active);
+    account_dict.SetBoolean("isActive",
+                            account.principal_name() == active_principal);
+    account_dict.SetBoolean("hasRememberedPassword",
+                            account.password_was_remembered());
     account_dict.SetString("pic", default_icon);
     accounts.GetList().push_back(std::move(account_dict));
   }
@@ -96,14 +98,15 @@ void KerberosAccountsHandler::HandleAddKerberosAccount(
   //   - Prevent account changes when Kerberos is disabled.
   //   - Remove all accounts when Kerberos is disabled.
 
-  CHECK_EQ(3U, args->GetSize());
+  CHECK_EQ(4U, args->GetSize());
   const std::string& callback_id = args->GetList()[0].GetString();
   const std::string& principal_name = args->GetList()[1].GetString();
   const std::string& password = args->GetList()[2].GetString();
+  const bool remember_password = args->GetList()[3].GetBool();
 
   KerberosCredentialsManager::Get().AddAccountAndAuthenticate(
       std::move(principal_name), false /* is_managed */, password,
-      false /* remember_password */, base::nullopt /* krb5_conf */,
+      remember_password, base::nullopt /* krb5_conf */,
       base::BindOnce(&KerberosAccountsHandler::OnAddAccountAndAuthenticate,
                      weak_factory_.GetWeakPtr(), callback_id));
 }

chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc

@@ -1750,6 +1750,8 @@ void AddPeopleStrings(content::WebUIDataSource* html_source, Profile* profile) {
     {"kerberosAccountsReauthenticationLabel",
      IDS_SETTINGS_KERBEROS_ACCOUNTS_REAUTHENTICATION_LABEL},
     {"addKerberosAccount", IDS_SETTINGS_ADD_KERBEROS_ACCOUNT},
+    {"addKerberosAccountRememberPassword",
+     IDS_SETTINGS_ADD_KERBEROS_ACCOUNT_REMEMBER_PASSWORD},
     {"kerberosUsername", IDS_SETTINGS_KERBEROS_USERNAME},
     {"kerberosPassword", IDS_SETTINGS_KERBEROS_PASSWORD},
     {"kerberosErrorNetworkProblem",

chrome/test/data/webui/settings/people_page_kerberos_accounts_test.js

@@ -5,6 +5,24 @@
 'use strict';
 
 cr.define('settings_people_page_kerberos_accounts', function() {
+  // List of fake accounts.
+  const testAccounts = [
+    {
+      principalName: 'user@REALM',
+      isSignedIn: true,
+      isActive: true,
+      hasRememberedPassword: false,
+      pic: 'pic',
+    },
+    {
+      principalName: 'user2@REALM2',
+      isSignedIn: false,
+      isActive: false,
+      hasRememberedPassword: true,
+      pic: 'pic2',
+    }
+  ];
+
   /** @implements {settings.KerberosAccountsBrowserProxy} */
   class TestKerberosAccountsBrowserProxy extends TestBrowserProxy {
     constructor() {
@@ -22,26 +40,13 @@ cr.define('settings_people_page_kerberos_accounts', function() {
     /** @override */
     getAccounts() {
       this.methodCalled('getAccounts');
-
-      return Promise.resolve([
-        {
-          principalName: 'user@REALM',
-          isSignedIn: true,
-          isActive: true,
-          pic: 'pic',
-        },
-        {
-          principalName: 'user2@REALM2',
-          isSignedIn: false,
-          isActive: false,
-          pic: 'pic2',
-        }
-      ]);
+      return Promise.resolve(testAccounts);
     }
 
     /** @override */
-    addAccount(principalName, password) {
-      this.methodCalled('addAccount', [principalName, password]);
+    addAccount(principalName, password, rememberPassword) {
+      this.methodCalled(
+          'addAccount', [principalName, password, rememberPassword]);
       return Promise.resolve(this.addAccountError);
     }
 
@@ -116,7 +121,7 @@ cr.define('settings_people_page_kerberos_accounts', function() {
       Polymer.dom.flush();
       const addDialog = kerberosAccounts.$$('kerberos-add-account-dialog');
       assertTrue(!!addDialog);
-      assertEquals('', addDialog.username);
+      assertEquals('', addDialog.$.username.value);
     });
 
     test('ReauthenticateAccount', function() {
@@ -137,7 +142,7 @@ cr.define('settings_people_page_kerberos_accounts', function() {
         // username.
         const addDialog = kerberosAccounts.$$('kerberos-add-account-dialog');
         assertTrue(!!addDialog);
-        assertEquals('user2@REALM2', addDialog.username);
+        assertEquals(testAccounts[1].principalName, addDialog.$.username.value);
       });
     });
 
@@ -146,7 +151,8 @@ cr.define('settings_people_page_kerberos_accounts', function() {
         Polymer.dom.flush();
         clickMoreActions(Account.FIRST, MoreActions.REMOVE_ACCOUNT);
         return browserProxy.whenCalled('removeAccount').then((account) => {
-          assertEquals('user@REALM', account.principalName);
+          assertEquals(
+              testAccounts[Account.FIRST].principalName, account.principalName);
         });
       });
     });
@@ -165,7 +171,9 @@ cr.define('settings_people_page_kerberos_accounts', function() {
             return browserProxy.whenCalled('setAsActiveAccount');
           })
           .then((account) => {
-            assertEquals('user2@REALM2', account.principalName);
+            assertEquals(
+                testAccounts[Account.SECOND].principalName,
+                account.principalName);
           });
     });
   });
@@ -227,9 +235,31 @@ cr.define('settings_people_page_kerberos_accounts', function() {
     // dialog is appended to the document.
     test('UsernameFieldDisabledIfPreset', function() {
       const newDialog = document.createElement('kerberos-add-account-dialog');
-      newDialog.username = 'user';
+      newDialog.presetAccount = testAccounts[0];
+      document.body.appendChild(newDialog);
+      assertTrue(newDialog.$.username.readonly);
+    });
+
+    // The password input field is empty and 'Remember password' is not preset
+    // if |hasRememberedPassword| is false.
+    test('PasswordNotPresetIfHasRememberedPasswordIsFalse', function() {
+      const newDialog = document.createElement('kerberos-add-account-dialog');
+      assertFalse(testAccounts[0].hasRememberedPassword);
+      newDialog.presetAccount = testAccounts[0];
+      document.body.appendChild(newDialog);
+      assertEquals('', newDialog.$.password.value);
+      assertFalse(newDialog.$.rememberPassword.checked);
+    });
+
+    // The password input field is not empty and 'Remember password' is preset
+    // if |hasRememberedPassword| is true.
+    test('PasswordPresetIfHasRememberedPasswordIsTrue', function() {
+      const newDialog = document.createElement('kerberos-add-account-dialog');
+      assertTrue(testAccounts[1].hasRememberedPassword);
+      newDialog.presetAccount = testAccounts[1];
       document.body.appendChild(newDialog);
-      assertTrue(newDialog.$.username.disabled);
+      assertNotEquals('', newDialog.$.password.value);
+      assertTrue(newDialog.$.rememberPassword.checked);
     });
 
     // By clicking the "Add account", the username and password values are
@@ -257,6 +287,36 @@ cr.define('settings_people_page_kerberos_accounts', function() {
       });
     });
 
+    // If the account has hasRememberedPassword == true and the user just clicks
+    // the 'Add' button, an empty password is submitted.
+    test('SubmitsEmptyPasswordIfRememberedPasswordIsUsed', function() {
+      const newDialog = document.createElement('kerberos-add-account-dialog');
+      // Note: testAccounts[1].hasRememberedPassword == true.
+      newDialog.presetAccount = testAccounts[1];
+      document.body.appendChild(newDialog);
+      newDialog.$$('.action-button').click();
+      return browserProxy.whenCalled('addAccount').then(function(args) {
+        assertEquals('', args[1]);  // password
+        assertTrue(args[2]);        // remember password
+      });
+    });
+
+    // If the account has hasRememberedPassword == true and the user changes the
+    // password before clicking 'Add' button, the changed password is submitted.
+    test('SubmitsChangedPasswordIfRememberedPasswordIsChanged', function() {
+      const newDialog = document.createElement('kerberos-add-account-dialog');
+      // Note: testAccounts[1].hasRememberedPassword == true.
+      newDialog.presetAccount = testAccounts[1];
+      document.body.appendChild(newDialog);
+      newDialog.$.password.inputElement.value = 'some edit';
+      newDialog.$.password.dispatchEvent(new CustomEvent('input'));
+      newDialog.$$('.action-button').click();
+      return browserProxy.whenCalled('addAccount').then(function(args) {
+        assertNotEquals('', args[1]);  // password
+        assertTrue(args[2]);           // remember password
+      });
+    });
+
     // addAccount: KerberosErrorType.kNetworkProblem spawns a general error.
     test('AddAccountError_NetworkProblem', function() {
       checkAddAccountError(

chromeos/dbus/kerberos/fake_kerberos_client.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
@@ -16,7 +17,7 @@ namespace chromeos {
 namespace {
 
 // Fake delay for any asynchronous operation.
-constexpr auto kTaskDelay = base::TimeDelta::FromMilliseconds(500);
+constexpr auto kTaskDelay = base::TimeDelta::FromMilliseconds(100);
 
 // Fake validity lifetime for TGTs.
 constexpr base::TimeDelta kTgtValidity = base::TimeDelta::FromHours(10);
@@ -43,6 +44,16 @@ void PostResponse(base::OnceCallback<void(const TProto&)> callback,
   PostProtoResponse(std::move(callback), response);
 }
 
+// Reads the password from the file descriptor |password_fd|.
+// Not very efficient, but simple!
+std::string ReadPassword(int password_fd) {
+  std::string password;
+  char c;
+  while (base::ReadFromFD(password_fd, &c, 1))
+    password.push_back(c);
+  return password;
+}
+
 }  // namespace
 
 FakeKerberosClient::FakeKerberosClient() = default;
@@ -51,12 +62,16 @@ FakeKerberosClient::~FakeKerberosClient() = default;
 
 void FakeKerberosClient::AddAccount(const kerberos::AddAccountRequest& request,
                                     AddAccountCallback callback) {
-  if (accounts_.find(request.principal_name()) != accounts_.end()) {
+  auto it = accounts_.find(request.principal_name());
+  if (it != accounts_.end()) {
+    it->second.is_managed |= request.is_managed();
     PostResponse(std::move(callback), kerberos::ERROR_DUPLICATE_PRINCIPAL_NAME);
     return;
   }
 
-  accounts_[request.principal_name()] = AccountData();
+  AccountData data;
+  data.is_managed = request.is_managed();
+  accounts_[request.principal_name()] = data;
   PostResponse(std::move(callback), kerberos::ERROR_NONE);
 }
 
@@ -80,17 +95,19 @@ void FakeKerberosClient::ListAccounts(
     const kerberos::ListAccountsRequest& request,
     ListAccountsCallback callback) {
   kerberos::ListAccountsResponse response;
-  for (const auto& account : accounts_) {
-    const std::string& principal_name = account.first;
-    const AccountData& data = account.second;
-
-    kerberos::Account* response_account = response.add_accounts();
-    response_account->set_principal_name(principal_name);
-    response_account->set_krb5conf(data.krb5conf);
-    response_account->set_tgt_validity_seconds(
-        data.has_tgt ? kTgtValidity.InSeconds() : 0);
-    response_account->set_tgt_renewal_seconds(
-        data.has_tgt ? kTgtRenewal.InSeconds() : 0);
+  for (const auto& it : accounts_) {
+    const std::string& principal_name = it.first;
+    const AccountData& data = it.second;
+
+    kerberos::Account* account = response.add_accounts();
+    account->set_principal_name(principal_name);
+    account->set_krb5conf(data.krb5conf);
+    account->set_tgt_validity_seconds(data.has_tgt ? kTgtValidity.InSeconds()
+                                                   : 0);
+    account->set_tgt_renewal_seconds(data.has_tgt ? kTgtRenewal.InSeconds()
+                                                  : 0);
+    account->set_is_managed(data.is_managed);
+    account->set_password_was_remembered(!data.password.empty());
   }
   response.set_error(kerberos::ERROR_NONE);
   PostProtoResponse(std::move(callback), response);
@@ -118,6 +135,25 @@ void FakeKerberosClient::AcquireKerberosTgt(
     return;
   }
 
+  // Remember password.
+  std::string password = ReadPassword(password_fd);
+  if (!password.empty() && request.remember_password())
+    data->password = password;
+
+  // Use remembered password.
+  if (password.empty())
+    password = data->password;
+
+  // Erase a previously remembered password.
+  if (!request.remember_password())
+    data->password.clear();
+
+  // Reject empty passwords.
+  if (password.empty()) {
+    PostResponse(std::move(callback), kerberos::ERROR_BAD_PASSWORD);
+    return;
+  }
+
   // It worked! Magic!
   data->has_tgt = true;
   PostResponse(std::move(callback), kerberos::ERROR_NONE);

chromeos/dbus/kerberos/fake_kerberos_client.h

@@ -44,8 +44,15 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakeKerberosClient
   struct AccountData {
     // Kerberos configuration file.
     std::string krb5conf;
-    // Gets set to true if AcquireKerberosTgt succeeds.
+
+    // True if AcquireKerberosTgt succeeded.
     bool has_tgt = false;
+
+    // True if the account was added by policy.
+    bool is_managed = false;
+
+    // Remembered password, if any.
+    std::string password;
   };
 
   // Returns the AccountData for |principal_name| if available or nullptr