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 c8e15c135c052bed3f06f81827f1f293741510b8..b8ea541920f5d0b57e40bd488096aa08a420de2f 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,17 +5,27 @@
 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.Bundle;
+import android.text.InputType;
 import android.view.LayoutInflater;
 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.
@@ -29,8 +39,21 @@ 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 ClipboardManager mClipboard;
+
+    private KeyguardManager mKeyguardManager;
+
+    private Bundle mExtras;
+
+    private View mView;

[Bernhard Bauer, 2016/08/03 12:54:42]

Can you make these final?

[dozsa, 2016/08/03 14:06:54]

I'm not able to assign values to them if I make them final. 

+
+    private boolean mViewButtonPressed;
+
+    private boolean mCopyButtonPressed;
+
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -40,36 +63,78 @@ 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);
+            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 (ChromeFeatureList.isEnabled(VIEW_PASSWORDS)) {
+            mKeyguardManager = (KeyguardManager) getActivity()
+                    .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();
+        assert mExtras != null;

[Bernhard Bauer, 2016/08/03 12:54:42]

This is a bit pointless, BTW: If |mExtras| is in fact null, you'll crash on the
next line in any case.

[dozsa, 2016/08/03 14:06:54]

Done.

+        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)) {
+            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 (!ChromeFeatureList.isEnabled(VIEW_PASSWORDS)) {
-            hookupCancelDeleteButtons(v);
+            hookupCancelDeleteButtons();
+        } else {
+            hookupPasswordButtons();
+            hookupCopyUsernameButton();
+            hookupCopySiteButton();
         }
-        return v;
+        return mView;
+    }
+
+    @Override
+    public void onResume() {
+        super.onResume();
+        if (mViewButtonPressed) {

[Bernhard Bauer, 2016/08/03 12:54:42]

Nit: It might help readability to inline this.

[dozsa, 2016/08/03 14:06:54]

Done.

+            displayPassword();
+        }
+        if (mCopyButtonPressed) {

[Bernhard Bauer, 2016/08/03 12:54:42]

These flags are not reset after we have copied the password. If the fragment
would get moved to the background and then brought forward, would we copy the
password again? And just semantically, we don't care about whether a UI element
has been pressed, we want to queue up an action, so maybe we should make it a
single field with an enum value.

Also, can you be sure that the fragment isn't destroyed while in the background?
Should you persist that state?

[dozsa, 2016/08/03 14:06:54]

The password isn't set to visible/copied if I access the fragment again after
exiting it, I think the flags automatically reset once I go back to the list of
passwords. And the reason for linking this logic to the UI element is that the
view after reauthentication differs according to whether the user wanted to view
or copy the password. As for your last question, it seems the fragment isn't
destroyed while displaying the device lock check, or at least the values for
mViewButtonPressed and mCopyButtonPressed aren't.  

[Bernhard Bauer, 2016/08/03 16:07:36]

I'm thinking of something like switching to a different app, then switching
back.
Well, can you be sure the fragment isn't destroyed? I don't think the fragment
lifecycle guarantees that (see
https://developer.android.com/guide/components/fragments.html#Lifecycle), even
if it worked when testing locally.

[dozsa, 2016/08/04 10:40:51]

Ah, I understand what you meant now. Indeed, when users left the app they didn't
have to reauthenticate when requesting to view/copy a password as long as they
were in the 60 second interval. In order to fix this I'm now setting
sLastReauthTimeMillis in onDetach() in SavePasswordsPreferences. Given that the
conditions including the copy and view flags also depend on
sLastReauthTimeMillis, I don't think they have to be set back to false.

[Bernhard Bauer, 2016/08/05 09:14:20]

I guess that works, but I would still recommend you familiarize yourself with
the Fragment lifecycle in Android (and the Activity lifecycle, which is
similar), because this is still somewhat unidiomatic.

[dozsa, 2016/08/05 13:53:53]

I've spent some time looking into this and I can't find a better option to
control when a user should be required to reauthenticate. Do you have any
suggestions? 

[Bernhard Bauer, 2016/08/05 14:29:38]

The idiomatic way would be the following: The Android framework will call you
back at certain times and ask you to serialize your state to a Bundle (e.g.
before the Fragment is paused). Then when the fragment is (re)created, you read
the state out of there. That way the state will survive even if the Fragment is
destroyed by the framework (see e.g.
https://developer.android.com/reference/android/app/Fragment.html#onSaveInsta...
).

Like I said, what you've implemented probably works, in particular if you
actually want to drop state when going into the background, but I wanted you to
at least be aware of the regular Fragment lifecycle when doing this.

[dozsa, 2016/08/05 14:52:42]

Okay, got it. Thank you! 

+            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() {
+        int validReauthenticationTime = 60000;

[Bernhard Bauer, 2016/08/03 12:54:42]

Extract this to a constant? Also, it's a time interval (or timeout), right?
"Time" sounds more like an absolute timestamp. And lastly, add the unit to the
name.

[dozsa, 2016/08/03 14:06:54]

Done.

+        return SavePasswordsPreferences.getLastReauthTimeMillis() != 0
+                && System.currentTimeMillis() - SavePasswordsPreferences.getLastReauthTimeMillis()
+                        < validReauthenticationTime;
     }
 
     // Delete was clicked.
@@ -99,24 +164,140 @@ 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() {
+        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() {
+        if (authenticationStillValid()) {

[Bernhard Bauer, 2016/08/03 12:54:42]

You might want to return early if the authentication is not valid anymore.

[dozsa, 2016/08/03 14:06:54]

Done.

+            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() {
+        if (authenticationStillValid()) {
+            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
+                                && authenticationStillValid()) {
+                    viewPasswordButton.setImageResource(R.drawable.ic_visibility);
+                    passwordView.setInputType(InputType.TYPE_CLASS_TEXT
+                            | InputType.TYPE_TEXT_VARIATION_PASSWORD);
+                } else if (authenticationStillValid()) {
+                    passwordView.setText(mExtras.getString(
+                            SavePasswordsPreferences.PASSWORD_LIST_PASSWORD));
+                    passwordView.setInputType(InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD);
+                    viewPasswordButton.setImageResource(R.drawable.ic_visibility_off);
+                } else {
+                    mViewButtonPressed = true;
+                    displayReauthenticationFragment();
                 }
-            });
+            }
+        });
     }
+
 }