add user ID input field (password creation/edit), may fix #1458 (#3161)

* add user ID input field (password creation/edit), may fix #1458

* fix: revert change to username label

* refactor: rework FieldItem to drop hard-coded strings

* refactor: drop unnecessary `.apply`

---------

Co-authored-by: Harsh Shandilya <me@msfjarvis.dev>

4 files changed, 76 insertions, 52 deletions

diff --git a/app/src/main/java/app/passwordstore/data/password/FieldItem.kt b/app/src/main/java/app/passwordstore/data/password/FieldItem.kt
index 2fb5cf0e..7ff8b8a4 100644
--- a/app/src/main/java/app/passwordstore/data/password/FieldItem.kt
+++ b/app/src/main/java/app/passwordstore/data/password/FieldItem.kt
@@ -7,35 +7,45 @@ package app.passwordstore.data.password
 
 import app.passwordstore.data.passfile.Totp
 
-class FieldItem(val key: String, val value: String, val action: ActionType) {
+class FieldItem
+private constructor(
+  val type: ItemType,
+  val label: String,
+  val value: String,
+  val action: ActionType,
+) {
   enum class ActionType {
     COPY,
     HIDE,
   }
 
-  enum class ItemType(val type: String, val label: String) {
-    USERNAME("Username", "Username"),
-    PASSWORD("Password", "Password"),
-    OTP("OTP", "OTP (expires in %ds)"),
+  enum class ItemType() {
+    USERNAME,
+    PASSWORD,
+    OTP,
+    FREEFORM,
   }
 
   companion object {
-
-    // Extra helper methods
-    fun createOtpField(totp: Totp): FieldItem {
+    fun createOtpField(label: String, totp: Totp): FieldItem {
       return FieldItem(
-        ItemType.OTP.label.format(totp.remainingTime.inWholeSeconds),
+        ItemType.OTP,
+        label.format(totp.remainingTime.inWholeSeconds),
         totp.value,
         ActionType.COPY,
       )
     }
 
-    fun createPasswordField(password: String): FieldItem {
-      return FieldItem(ItemType.PASSWORD.label, password, ActionType.HIDE)
+    fun createPasswordField(label: String, password: String): FieldItem {
+      return FieldItem(ItemType.PASSWORD, label, password, ActionType.HIDE)
+    }
+
+    fun createUsernameField(label: String, username: String): FieldItem {
+      return FieldItem(ItemType.USERNAME, label, username, ActionType.COPY)
     }
 
-    fun createUsernameField(username: String): FieldItem {
-      return FieldItem(ItemType.USERNAME.label, username, ActionType.COPY)
+    fun createFreeformField(label: String, content: String): FieldItem {
+      return FieldItem(ItemType.FREEFORM, label, content, ActionType.COPY)
     }
   }
 }
diff --git a/app/src/main/java/app/passwordstore/ui/adapters/FieldItemAdapter.kt b/app/src/main/java/app/passwordstore/ui/adapters/FieldItemAdapter.kt
index 11eefb20..b98cdcc0 100644
--- a/app/src/main/java/app/passwordstore/ui/adapters/FieldItemAdapter.kt
+++ b/app/src/main/java/app/passwordstore/ui/adapters/FieldItemAdapter.kt
@@ -38,13 +38,13 @@ class FieldItemAdapter(
     return fieldItemList.size
   }
 
-  fun updateOTPCode(totp: Totp) {
+  fun updateOTPCode(totp: Totp, labelFormat: String) {
     var otpItemPosition = -1
     fieldItemList =
       fieldItemList.mapIndexed { position, item ->
-        if (item.key.startsWith(FieldItem.ItemType.OTP.type, true)) {
+        if (item.type == FieldItem.ItemType.OTP) {
           otpItemPosition = position
-          return@mapIndexed FieldItem.createOtpField(totp)
+          return@mapIndexed FieldItem.createOtpField(labelFormat, totp)
         }
 
         return@mapIndexed item
@@ -58,8 +58,8 @@ class FieldItemAdapter(
 
     fun bind(fieldItem: FieldItem, showPassword: Boolean, copyTextToClipboard: (String?) -> Unit) {
       with(binding) {
-        itemText.hint = fieldItem.key
-        itemTextContainer.hint = fieldItem.key
+        itemText.hint = fieldItem.label
+        itemTextContainer.hint = fieldItem.label
         itemText.setText(fieldItem.value)
 
         when (fieldItem.action) {
@@ -84,7 +84,7 @@ class FieldItemAdapter(
                 } else {
                   null
                 }
-              if (fieldItem.key == FieldItem.ItemType.PASSWORD.type) {
+              if (fieldItem.type == FieldItem.ItemType.PASSWORD) {
                 typeface =
                   ResourcesCompat.getFont(
                     binding.root.context,
diff --git a/app/src/main/java/app/passwordstore/ui/crypto/DecryptActivity.kt b/app/src/main/java/app/passwordstore/ui/crypto/DecryptActivity.kt
index 372cd074..e4121258 100644
--- a/app/src/main/java/app/passwordstore/ui/crypto/DecryptActivity.kt
+++ b/app/src/main/java/app/passwordstore/ui/crypto/DecryptActivity.kt
@@ -136,8 +136,12 @@ class DecryptActivity : BasePGPActivity() {
     intent.putExtra("FILE_PATH", relativeParentPath)
     intent.putExtra("REPO_PATH", repoPath)
     intent.putExtra(PasswordCreationActivity.EXTRA_FILE_NAME, name)
+    intent.putExtra(PasswordCreationActivity.EXTRA_USERNAME, passwordEntry?.username)
     intent.putExtra(PasswordCreationActivity.EXTRA_PASSWORD, passwordEntry?.password)
-    intent.putExtra(PasswordCreationActivity.EXTRA_EXTRA_CONTENT, passwordEntry?.extraContentString)
+    intent.putExtra(
+      PasswordCreationActivity.EXTRA_EXTRA_CONTENT,
+      passwordEntry?.extraContentWithoutUsername,
+    )
     intent.putExtra(PasswordCreationActivity.EXTRA_EDITING, true)
     startActivity(intent)
     finish()
@@ -274,27 +278,28 @@ class DecryptActivity : BasePGPActivity() {
 
   private suspend fun createPasswordUI(entry: PasswordEntry) =
     withContext(dispatcherProvider.main()) {
+      val labelFormat = resources.getString(R.string.otp_label_format)
       val showPassword = settings.getBoolean(PreferenceKeys.SHOW_PASSWORD, true)
       invalidateOptionsMenu()
 
       val items = arrayListOf<FieldItem>()
       if (!entry.password.isNullOrBlank()) {
-        items.add(FieldItem.createPasswordField(entry.password!!))
+        items.add(FieldItem.createPasswordField(getString(R.string.password), entry.password!!))
         if (settings.getBoolean(PreferenceKeys.COPY_ON_DECRYPT, false)) {
           copyPasswordToClipboard(entry.password)
         }
       }
 
       if (entry.hasTotp()) {
-        items.add(FieldItem.createOtpField(entry.totp.first()))
+        items.add(FieldItem.createOtpField(labelFormat, entry.totp.first()))
       }
 
       if (!entry.username.isNullOrBlank()) {
-        items.add(FieldItem.createUsernameField(entry.username!!))
+        items.add(FieldItem.createUsernameField(getString(R.string.username), entry.username!!))
       }
 
       entry.extraContent.forEach { (key, value) ->
-        items.add(FieldItem(key, value, FieldItem.ActionType.COPY))
+        items.add(FieldItem.createFreeformField(key, value))
       }
 
       val adapter = FieldItemAdapter(items, showPassword) { text -> copyTextToClipboard(text) }
@@ -302,7 +307,7 @@ class DecryptActivity : BasePGPActivity() {
       binding.recyclerView.itemAnimator = null
 
       if (entry.hasTotp()) {
-        lifecycleScope.launch { entry.totp.collect(adapter::updateOTPCode) }
+        lifecycleScope.launch { entry.totp.collect { adapter.updateOTPCode(it, labelFormat) } }
       }
     }
 
diff --git a/app/src/main/java/app/passwordstore/ui/crypto/PasswordCreationActivity.kt b/app/src/main/java/app/passwordstore/ui/crypto/PasswordCreationActivity.kt
index 525d9f95..1395da5b 100644
--- a/app/src/main/java/app/passwordstore/ui/crypto/PasswordCreationActivity.kt
+++ b/app/src/main/java/app/passwordstore/ui/crypto/PasswordCreationActivity.kt
@@ -78,6 +78,7 @@ class PasswordCreationActivity : BasePGPActivity() {
   @Inject lateinit var passwordEntryFactory: PasswordEntry.Factory
 
   private val suggestedName by unsafeLazy { intent.getStringExtra(EXTRA_FILE_NAME) }
+  private val suggestedUsername by unsafeLazy { intent.getStringExtra(EXTRA_USERNAME) }
   private val suggestedPass by unsafeLazy { intent.getStringExtra(EXTRA_PASSWORD) }
   private val suggestedExtra by unsafeLazy { intent.getStringExtra(EXTRA_EXTRA_CONTENT) }
   private val shouldGeneratePassword by unsafeLazy {
@@ -205,6 +206,16 @@ class PasswordCreationActivity : BasePGPActivity() {
       } else {
         filename.requestFocus()
       }
+
+      if (
+        AutofillPreferences.directoryStructure(this@PasswordCreationActivity) ==
+          DirectoryStructure.EncryptedUsername || suggestedUsername != null
+      ) {
+        usernameInputLayout.visibility = View.VISIBLE
+        if (suggestedUsername != null) username.setText(suggestedUsername)
+        else if (suggestedName != null) username.requestFocus()
+      }
+
       // Allow the user to quickly switch between storing the username as the filename or
       // in the encrypted extras. This only makes sense if the directory structure is
       // FileBased.
@@ -217,27 +228,19 @@ class PasswordCreationActivity : BasePGPActivity() {
           visibility = View.VISIBLE
           setOnClickListener {
             if (isChecked) {
-              // User wants to enable username encryption, so we add it to the
-              // encrypted extras as the first line.
-              val username = filename.text.toString()
-              val extras = "username:$username\n${extraContent.text}"
-
+              // User wants to enable username encryption, so we use the filename
+              // as username and insert it into the username input field.
+              val login = filename.text.toString()
               filename.text?.clear()
-              extraContent.setText(extras)
+              username.setText(login)
+              usernameInputLayout.apply { visibility = View.VISIBLE }
             } else {
-              // User wants to disable username encryption, so we extract the
-              // username from the encrypted extras and use it as the filename.
-              val entry =
-                passwordEntryFactory.create("PASSWORD\n${extraContent.text}".encodeToByteArray())
-              val username = entry.username
-
-              // username should not be null here by the logic in
-              // updateViewState, but it could still happen due to
-              // input lag.
-              if (username != null) {
-                filename.setText(username)
-                extraContent.setText(entry.extraContentWithoutAuthData)
-              }
+              // User wants to disable username encryption, so we take the username
+              // from the username text field and insert it into the filename input field.
+              val login = username.text.toString()
+              username.text?.clear()
+              filename.setText(login)
+              usernameInputLayout.apply { visibility = View.GONE }
             }
           }
         }
@@ -252,7 +255,7 @@ class PasswordCreationActivity : BasePGPActivity() {
         password.inputType = InputType.TYPE_CLASS_TEXT or InputType.TYPE_TEXT_VARIATION_PASSWORD
       }
     }
-    listOf(binding.filename, binding.extraContent).forEach {
+    listOf(binding.filename, binding.username, binding.extraContent).forEach {
       it.doAfterTextChanged { updateViewState() }
     }
     updateViewState()
@@ -300,16 +303,16 @@ class PasswordCreationActivity : BasePGPActivity() {
 
   private fun updateViewState() =
     with(binding) {
-      // Use PasswordEntry to parse extras for username
-      val entry =
-        passwordEntryFactory.create("PLACEHOLDER\n${extraContent.text}".encodeToByteArray())
       encryptUsername.apply {
         if (visibility != View.VISIBLE) return@apply
         val hasUsernameInFileName = filename.text.toString().isNotBlank()
-        val hasUsernameInExtras = !entry.username.isNullOrBlank()
-        isEnabled = hasUsernameInFileName xor hasUsernameInExtras
-        isChecked = hasUsernameInExtras
+        val usernameIsEncrypted = username.text.toString().isNotEmpty()
+        isEnabled = hasUsernameInFileName xor usernameIsEncrypted
+        isChecked = usernameIsEncrypted
       }
+      // Use PasswordEntry to parse extras for OTP
+      val entry =
+        passwordEntryFactory.create("PLACEHOLDER\n${extraContent.text}".encodeToByteArray())
       otpImportButton.isVisible = !entry.hasTotp()
     }
 
@@ -318,6 +321,7 @@ class PasswordCreationActivity : BasePGPActivity() {
     with(binding) {
       val oldName = suggestedName
       val editName = filename.text.toString().trim()
+      var editUsername = username.text.toString()
       val editPass = password.text.toString()
       val editExtra = extraContent.text.toString()
 
@@ -329,6 +333,10 @@ class PasswordCreationActivity : BasePGPActivity() {
         return@with
       }
 
+      if (!editUsername.isEmpty()) {
+        editUsername = "\nusername:$editUsername"
+      }
+
       if (editPass.isEmpty() && editExtra.isEmpty()) {
         snackbar(message = resources.getString(R.string.empty_toast_text))
         return@with
@@ -340,7 +348,7 @@ class PasswordCreationActivity : BasePGPActivity() {
 
       // pass enters the key ID into `.gpg-id`.
       val gpgIdentifiers = getPGPIdentifiers(directory.text.toString()) ?: return@with
-      val content = "$editPass\n$editExtra"
+      val content = "$editPass$editUsername\n$editExtra"
       val path =
         when {
           // If we allowed the user to edit the relative path, we have to consider it here
@@ -482,6 +490,7 @@ class PasswordCreationActivity : BasePGPActivity() {
     const val RETURN_EXTRA_USERNAME = "USERNAME"
     const val RETURN_EXTRA_PASSWORD = "PASSWORD"
     const val EXTRA_FILE_NAME = "FILENAME"
+    const val EXTRA_USERNAME = "USERNAME"
     const val EXTRA_PASSWORD = "PASSWORD"
     const val EXTRA_EXTRA_CONTENT = "EXTRA_CONTENT"
     const val EXTRA_GENERATE_PASSWORD = "GENERATE_PASSWORD"