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>

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) {
+  enum class ItemType() {
-    USERNAME("Username", "Username"),
+    USERNAME,
-    PASSWORD("Password", "Password"),
+    PASSWORD,
-    OTP("OTP", "OTP (expires in %ds)"),
+    OTP,
+    FREEFORM,
   }
 
   companion object {
-
+    fun createOtpField(label: String, totp: Totp): FieldItem {
-    // Extra helper methods
-    fun createOtpField(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 {
+    fun createPasswordField(label: String, password: String): FieldItem {
-      return FieldItem(ItemType.PASSWORD.label, password, ActionType.HIDE)
+      return FieldItem(ItemType.PASSWORD, label, password, ActionType.HIDE)
     }
 
-    fun createUsernameField(username: String): FieldItem {
+    fun createUsernameField(label: String, username: String): FieldItem {
-      return FieldItem(ItemType.USERNAME.label, username, ActionType.COPY)
+      return FieldItem(ItemType.USERNAME, label, username, ActionType.COPY)
+    }
+
+    fun createFreeformField(label: String, content: String): FieldItem {
+      return FieldItem(ItemType.FREEFORM, label, content, ActionType.COPY)
     }
   }
 }

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
+        itemText.hint = fieldItem.label
-        itemTextContainer.hint = fieldItem.key
+        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,

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) } }
       }
     }
 

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
+              // User wants to enable username encryption, so we use the filename
-              // encrypted extras as the first line.
+              // as username and insert it into the username input field.
-              val username = filename.text.toString()
+              val login = filename.text.toString()
-              val extras = "username:$username\n${extraContent.text}"
-
               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
+              // User wants to disable username encryption, so we take the username
-              // username from the encrypted extras and use it as the filename.
+              // from the username text field and insert it into the filename input field.
-              val entry =
+              val login = username.text.toString()
-                passwordEntryFactory.create("PASSWORD\n${extraContent.text}".encodeToByteArray())
+              username.text?.clear()
-              val username = entry.username
+              filename.setText(login)
-
+              usernameInputLayout.apply { visibility = View.GONE }
-              // 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)
-              }
             }
           }
         }
@@ -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()
+        val usernameIsEncrypted = username.text.toString().isNotEmpty()
-        isEnabled = hasUsernameInFileName xor hasUsernameInExtras
+        isEnabled = hasUsernameInFileName xor usernameIsEncrypted
-        isChecked = hasUsernameInExtras
+        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"

app/src/main/res/layout/password_creation_activity.xml

@@ -55,6 +55,26 @@
         android:layout_height="wrap_content"
         android:imeOptions="actionNext"
         android:inputType="textNoSuggestions"
+        android:nextFocusForward="@id/username" />
+    </com.google.android.material.textfield.TextInputLayout>
+
+    <com.google.android.material.textfield.TextInputLayout
+      android:visibility="gone"
+      tools:visibility="visible"
+      android:id="@+id/username_input_layout"
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:layout_margin="8dp"
+      android:hint="@string/crypto_username_hint"
+      app:layout_constraintStart_toStartOf="parent"
+      app:layout_constraintTop_toBottomOf="@id/name_input_layout">
+
+      <com.google.android.material.textfield.TextInputEditText
+        android:id="@+id/username"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:imeOptions="actionNext"
+        android:inputType="textNoSuggestions"
         android:nextFocusForward="@id/password" />
     </com.google.android.material.textfield.TextInputLayout>
 
@@ -66,7 +86,7 @@
       android:hint="@string/crypto_pass_label"
       app:endIconMode="password_toggle"
       app:layout_constraintStart_toStartOf="parent"
-      app:layout_constraintTop_toBottomOf="@id/name_input_layout">
+      app:layout_constraintTop_toBottomOf="@id/username_input_layout">
 
       <com.google.android.material.textfield.TextInputEditText
         android:id="@+id/password"

app/src/main/res/values/strings.xml

@@ -77,6 +77,7 @@
 
   <!-- PGP Handler -->
   <string name="crypto_name_hint">Name</string>
+  <string name="crypto_username_hint">Username</string>
   <string name="crypto_pass_label">Password</string>
   <string name="crypto_extra_label">Extra content</string>
   <string name="crypto_encrypt_username_label">Encrypt username</string>
@@ -88,6 +89,7 @@
   <string name="action_search">Search</string>
   <string name="password">Password</string>
   <string name="username">Username</string>
+  <string name="otp_label_format">OTP (expires in %ds)</string>
   <string name="copy_label">Copy</string>
   <string name="edit_password">Edit password</string>
   <string name="copy_password">Copy password</string>

format/common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt

@@ -80,6 +80,9 @@ constructor(
       return otp.value.value
     }
 
+  /** String representation of [extraContent] but with usernames stripped. */
+  public val extraContentWithoutUsername: String
+
   /**
    * String representation of [extraContent] but with authentication related data such as TOTP URIs
    * and usernames stripped.
@@ -91,6 +94,7 @@ constructor(
     val (foundPassword, passContent) = findAndStripPassword(content.split("\n".toRegex()))
     password = foundPassword
     extraContentString = passContent.joinToString("\n")
+    extraContentWithoutUsername = generateExtraContentWithoutUsername()
     extraContentWithoutAuthData = generateExtraContentWithoutAuthData()
     extraContent = generateExtraContentPairs()
     username = findUsername()
@@ -121,7 +125,7 @@ constructor(
     return Pair(passContent[0], passContent.minus(passContent[0]))
   }
 
-  private fun generateExtraContentWithoutAuthData(): String {
+  private fun generateExtraContentWithoutUsername(): String {
     var foundUsername = false
     return extraContentString
       .lineSequence()
@@ -132,6 +136,19 @@ constructor(
             foundUsername = true
             false
           }
+          else -> {
+            true
+          }
+        }
+      }
+      .joinToString(separator = "\n")
+  }
+
+  private fun generateExtraContentWithoutAuthData(): String {
+    return generateExtraContentWithoutUsername()
+      .lineSequence()
+      .filter { line ->
+        return@filter when {
           TotpFinder.TOTP_FIELDS.any { prefix -> line.startsWith(prefix, ignoreCase = true) } -> {
             false
           }