From f803465e403c43615d5ad134d4b6cd17c4a89917 Mon Sep 17 00:00:00 2001 From: Alexander Grahn Date: Sun, 11 Aug 2024 22:27:20 +0200 Subject: [PATCH] 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 --- .../passwordstore/data/password/FieldItem.kt | 36 +++++++---- .../ui/adapters/FieldItemAdapter.kt | 12 ++-- .../ui/crypto/DecryptActivity.kt | 17 +++-- .../ui/crypto/PasswordCreationActivity.kt | 63 +++++++++++-------- .../res/layout/password_creation_activity.xml | 22 ++++++- app/src/main/res/values/strings.xml | 2 + .../data/passfile/PasswordEntry.kt | 19 +++++- 7 files changed, 117 insertions(+), 54 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(username: String): FieldItem { - return FieldItem(ItemType.USERNAME.label, username, ActionType.COPY) + fun createUsernameField(label: String, 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() 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" diff --git a/app/src/main/res/layout/password_creation_activity.xml b/app/src/main/res/layout/password_creation_activity.xml index c489e121..af6036b2 100644 --- a/app/src/main/res/layout/password_creation_activity.xml +++ b/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" /> + + + + + @@ -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"> Name + Username Password Extra content Encrypt username @@ -88,6 +89,7 @@ Search Password Username + OTP (expires in %ds) Copy Edit password Copy password diff --git a/format/common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt b/format/common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt index cd152c9a..aeaa745b 100644 --- a/format/common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt +++ b/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 }