Allow copying and viewing account credentials in PasswordEntryEditor

chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
index fc894c30947f1f7215739bd6dcf50b32c1053267..27de3c8ea408201543974afd29f9f99002265119 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
@@ -5,7 +5,15 @@
 package org.chromium.chrome.browser.preferences.password;
 
 import android.app.Fragment;
+import android.app.FragmentManager;
+import android.app.FragmentTransaction;
+import android.app.KeyguardManager;
+import android.content.ClipData;
+import android.content.ClipboardManager;
+import android.content.Context;
+import android.os.Build;
 import android.os.Bundle;
+import android.text.InputType;
 import android.view.LayoutInflater;
 import android.view.Menu;
 import android.view.MenuInflater;
@@ -13,12 +21,15 @@ import android.view.MenuItem;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.Button;
+import android.widget.ImageButton;
 import android.widget.TextView;
 
+import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.PasswordUIView;
 import org.chromium.chrome.browser.PasswordUIView.PasswordListObserver;
+import org.chromium.ui.widget.Toast;
 
 /**
  * Password entry editor that allows to view and delete passwords stored in Chrome.
@@ -32,8 +43,23 @@ public class PasswordEntryEditor extends Fragment {
     // If false this represents a saved name/password.
     private boolean mException;
 
+    @VisibleForTesting
     public static final String VIEW_PASSWORDS = "view-passwords";
 
+    private static final int VALID_REAUTHENTICATION_TIME_INTERVAL_MILLIS = 60000;
+
+    private ClipboardManager mClipboard;
+
+    private KeyguardManager mKeyguardManager;
+
+    private Bundle mExtras;
+
+    private View mView;
+
+    private boolean mViewButtonPressed;
+
+    private boolean mCopyButtonPressed;
+
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -46,36 +72,83 @@ public class PasswordEntryEditor extends Fragment {
     public View onCreateView(LayoutInflater inflater, ViewGroup container,
             Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
-        View v;
-        if (ChromeFeatureList.isEnabled(VIEW_PASSWORDS)) {
-            v = inflater.inflate(R.layout.password_entry_editor_interactive, container, false);
+        if (displayInteractivePasswordEntryEditor()) {
+            mView = inflater.inflate(R.layout.password_entry_editor_interactive, container, false);
         } else {
-            v = inflater.inflate(R.layout.password_entry_editor, container, false);
+            mView = inflater.inflate(R.layout.password_entry_editor, container, false);
         }
         getActivity().setTitle(R.string.password_entry_editor_title);
-
+        mClipboard = (ClipboardManager) getActivity().getApplicationContext()
+            .getSystemService(Context.CLIPBOARD_SERVICE);
+        if (displayInteractivePasswordEntryEditor()) {
+            mKeyguardManager = (KeyguardManager) getActivity()

[vabr (Chromium), 2016/08/17 08:57:42]

It should not be necessary to use KeyguardManager in here directly, given that
PasswordReauthentication was written to encapsulate it.

[dozsa, 2016/08/17 10:40:26]

It seems that I need to set up the Keyguard Manager in PasswordEntryEditor as
well, as whether it's secure or not determines whether certain buttons are
displayed, and in https://codereview.chromium.org/2232893002/ I use the keyguard
manager to determine whether I should display the string telling users that they
should set up a device lock.

[vabr (Chromium), 2016/08/17 11:12:00]

As long as you don't run into issues because of that when testing this class,
that's fine.

An alternative would be to encapsulate all communication with the
KeyguardManager inside PasswordReauthentication, i.e., exposing methods for all
tasks you need KeyguardManager for presently.

+                    .getApplicationContext().getSystemService(Context.KEYGUARD_SERVICE);
+            ImageButton copyPasswordButton = (ImageButton) mView.findViewById(
+                    R.id.password_entry_editor_copy_password);
+            ImageButton viewPasswordButton = (ImageButton) mView.findViewById(
+                    R.id.password_entry_editor_view_password);
+        }
         // Extras are set on this intent in class SavePasswordsPreferences.
-        Bundle extras = getArguments();
-        assert extras != null;
-        mID = extras.getInt(SavePasswordsPreferences.PASSWORD_LIST_ID);
+        mExtras = getArguments();
+        mID = mExtras.getInt(SavePasswordsPreferences.PASSWORD_LIST_ID);
         String name = null;
-        if (extras.containsKey(SavePasswordsPreferences.PASSWORD_LIST_NAME)) {
-            name = extras.getString(SavePasswordsPreferences.PASSWORD_LIST_NAME);
+        if (mExtras.containsKey(SavePasswordsPreferences.PASSWORD_LIST_NAME)) {
+            name = mExtras.getString(SavePasswordsPreferences.PASSWORD_LIST_NAME);
         }
-        TextView nameView = (TextView) v.findViewById(R.id.password_entry_editor_name);
+        TextView nameView = (TextView) mView.findViewById(R.id.password_entry_editor_name);
         if (name != null) {
             nameView.setText(name);
         } else {
             nameView.setText(R.string.section_saved_passwords_exceptions);
             mException = true;
         }
-        String url = extras.getString(SavePasswordsPreferences.PASSWORD_LIST_URL);
-        TextView urlView = (TextView) v.findViewById(R.id.password_entry_editor_url);
+        String url = mExtras.getString(SavePasswordsPreferences.PASSWORD_LIST_URL);
+        TextView urlView = (TextView) mView.findViewById(R.id.password_entry_editor_url);
         urlView.setText(url);
-        if (!ChromeFeatureList.isEnabled(VIEW_PASSWORDS)) {
-            hookupCancelDeleteButtons(v);
+        if (displayInteractivePasswordEntryEditor()) {
+            String password = mExtras.getString(SavePasswordsPreferences.PASSWORD_LIST_PASSWORD);
+            TextView passwordView = (TextView)
+                      mView.findViewById(R.id.password_entry_editor_password);
+            passwordView.setText(password);
+            passwordView.setInputType(InputType.TYPE_CLASS_TEXT
+                      | InputType.TYPE_TEXT_VARIATION_PASSWORD);
+        }
+        if (displayInteractivePasswordEntryEditor()) {

[Bernhard Bauer, 2016/08/16 23:37:35]

This condition is the same as in the if-statement right before. Just merge them?

[dozsa, 2016/08/17 10:40:26]

Done.

+            hookupPasswordButtons();
+            hookupCopyUsernameButton();
+            hookupCopySiteButton();
+        } else {
+            hookupCancelDeleteButtons();
+        }
+        return mView;
+    }
+
+    private boolean displayInteractivePasswordEntryEditor() {

[Bernhard Bauer, 2016/08/16 23:37:35]

A method name that starts with a verb usually indicates an imperative ("display
the editor!"). Name it "shouldDisplay..."?

[dozsa, 2016/08/17 10:40:26]

Done.

+        return ChromeFeatureList.isEnabled(VIEW_PASSWORDS)
+                && Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP;
+    }
+
+    @Override
+    public void onResume() {
+        super.onResume();
+
+        if (mViewButtonPressed && authenticationStillValid()) {
+            displayPassword();
         }
-        return v;
+
+        if (mCopyButtonPressed && authenticationStillValid()) {
+            copyPassword();
+        }
+    }
+
+    /**
+     * Verifies if authentication is still valid (the user authenticated less than 60 seconds ago
+     * and the startTime is not equal to 0.
+     */
+    private boolean authenticationStillValid() {
+        return SavePasswordsPreferences.getLastReauthTimeMillis() != 0
+                && System.currentTimeMillis() - SavePasswordsPreferences.getLastReauthTimeMillis()
+                        < VALID_REAUTHENTICATION_TIME_INTERVAL_MILLIS;
     }
 
     @Override
@@ -120,24 +193,132 @@ public class PasswordEntryEditor extends Fragment {
         passwordUIView.updatePasswordLists();
     }
 
-    private void hookupCancelDeleteButtons(View v) {
-        final Button deleteButton = (Button) v.findViewById(R.id.password_entry_editor_delete);
-        final Button cancelButton = (Button) v.findViewById(R.id.password_entry_editor_cancel);
+    private void hookupCancelDeleteButtons() {
+        final Button deleteButton = (Button) mView.findViewById(R.id.password_entry_editor_delete);
+        final Button cancelButton = (Button) mView.findViewById(R.id.password_entry_editor_cancel);
 
         deleteButton.setOnClickListener(new View.OnClickListener() {
-                @Override
-                public void onClick(View v) {
-                    removeItem();
-                    deleteButton.setEnabled(false);
-                    cancelButton.setEnabled(false);
-                }
-            });
+            @Override
+            public void onClick(View v) {
+                removeItem();
+                deleteButton.setEnabled(false);
+                cancelButton.setEnabled(false);
+            }
+        });
 
         cancelButton.setOnClickListener(new View.OnClickListener() {
-                @Override
-                public void onClick(View v) {
-                    getActivity().finish();
+            @Override
+            public void onClick(View v) {
+                getActivity().finish();
+            }
+        });
+    }
+
+    private void hookupCopyUsernameButton() {

[vabr (Chromium), 2016/08/17 08:57:42]

The newly added copy and view functions should be tested.
One potential issues when doing it might be mocking PasswordReauthentication
correctly (if necessary, you could use a similar approach to bypass it in the
test as you did for KeyguardManager in the PasswordReauthentication test).

(The old code apparently did not have a test for deletion at all. While our team
should add that asap, it is not something you need to worry about in this CL.)

[dozsa, 2016/08/17 10:40:26]

Acknowledged.

+        final ImageButton copyUsernameButton = (ImageButton) mView.findViewById(
+                R.id.password_entry_editor_copy_username);
+        copyUsernameButton.setOnClickListener(new View.OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                ClipData clip = ClipData.newPlainText("username",
+                        getArguments().getString(SavePasswordsPreferences.PASSWORD_LIST_NAME));
+                mClipboard.setPrimaryClip(clip);
+                Toast.makeText(getActivity().getApplicationContext(),
+                        R.string.password_entry_editor_username_copied_into_clipboard,
+                                Toast.LENGTH_SHORT).show();
+            }
+        });
+    }
+
+    private void hookupCopySiteButton() {
+        final ImageButton copySiteButton = (ImageButton) mView.findViewById(
+                R.id.password_entry_editor_copy_site);
+        copySiteButton.setOnClickListener(new View.OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                ClipData clip = ClipData.newPlainText("site",
+                        getArguments().getString(SavePasswordsPreferences.PASSWORD_LIST_URL));
+                mClipboard.setPrimaryClip(clip);
+                Toast.makeText(getActivity().getApplicationContext(),
+                        R.string.password_entry_editor_site_copied_into_clipboard,
+                                Toast.LENGTH_SHORT).show();
+            }
+        });
+    }
+
+    private void displayPassword() {
+        ImageButton viewPasswordButton = (ImageButton) mView.findViewById(
+                R.id.password_entry_editor_view_password);
+        TextView passwordView = (TextView) mView.findViewById(
+                R.id.password_entry_editor_password);
+        passwordView.setText(mExtras.getString(
+                SavePasswordsPreferences.PASSWORD_LIST_PASSWORD));
+        passwordView.setInputType(InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD);
+        viewPasswordButton.setImageResource(R.drawable.ic_visibility_off);
+    }
+
+    private void copyPassword() {
+        ClipData clip = ClipData.newPlainText("password",
+                getArguments().getString(
+                        SavePasswordsPreferences.PASSWORD_LIST_PASSWORD));
+        mClipboard.setPrimaryClip(clip);
+        Toast.makeText(getActivity().getApplicationContext(),
+                R.string.password_entry_editor_password_copied_into_clipboard,
+                        Toast.LENGTH_SHORT).show();
+    }
+
+    private void displayReauthenticationFragment() {
+        Fragment passwordReauthentication = new PasswordReauthentication();
+        FragmentManager fragmentManager = getFragmentManager();
+        FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
+        fragmentTransaction.replace(R.id.password_entry_editor_interactive,
+                passwordReauthentication);
+        fragmentTransaction.addToBackStack(null);
+        fragmentTransaction.commit();
+    }
+
+    private void hookupPasswordButtons() {
+        final ImageButton copyPasswordButton = (ImageButton) mView.findViewById(
+                R.id.password_entry_editor_copy_password);
+        final ImageButton viewPasswordButton = (ImageButton) mView.findViewById(
+                R.id.password_entry_editor_view_password);
+        copyPasswordButton.setOnClickListener(new View.OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                if (!mKeyguardManager.isKeyguardSecure()) {
+                    Toast.makeText(getActivity().getApplicationContext(),
+                            R.string.password_entry_editor_set_lock_screen,
+                                    Toast.LENGTH_LONG).show();
+                } else if (authenticationStillValid()) {
+                    copyPassword();
+                } else {
+                    mCopyButtonPressed = true;
+                    displayReauthenticationFragment();
+                }
+            }
+        });
+        viewPasswordButton.setOnClickListener(new View.OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                TextView passwordView = (TextView) mView.findViewById(
+                        R.id.password_entry_editor_password);
+                if (!mKeyguardManager.isKeyguardSecure()) {
+                    Toast.makeText(getActivity().getApplicationContext(),
+                            R.string.password_entry_editor_set_lock_screen,
+                                     Toast.LENGTH_LONG).show();
+                } else if (passwordView.getInputType()
+                        == InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD) {
+                    viewPasswordButton.setImageResource(R.drawable.ic_visibility);
+                    passwordView.setInputType(InputType.TYPE_CLASS_TEXT
+                            | InputType.TYPE_TEXT_VARIATION_PASSWORD);
+                } else if (authenticationStillValid()) {
+                    displayPassword();
+                } else {
+                    mViewButtonPressed = true;
+                    displayReauthenticationFragment();
                 }
-            });
+            }
+        });
     }
+
 }