Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 39 additions & 9 deletions app/src/main/java/org/fedorahosted/freeotp/ui/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,20 @@ class MainActivity : AppCompatActivity() {
windowInsets
}

ViewCompat.setOnApplyWindowInsetsListener(binding.addTokenFab) { view, windowInsets ->

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit should have already contained this change 40151ec

val insets = windowInsets.getInsets(WindowInsetsCompat.Type.systemBars())
val marginParams = view.layoutParams as android.view.ViewGroup.MarginLayoutParams
marginParams.bottomMargin = resources.getDimensionPixelSize(R.dimen.space_3) + insets.bottom
if (view.layoutDirection == View.LAYOUT_DIRECTION_RTL) {
marginParams.leftMargin = resources.getDimensionPixelSize(R.dimen.space_3) + insets.left
} else {
marginParams.rightMargin = resources.getDimensionPixelSize(R.dimen.space_3) + insets.right
}
view.layoutParams = marginParams
windowInsets
}


viewModel.migrateOldData()

binding.tokenList.adapter = tokenListAdapter
Expand Down Expand Up @@ -341,14 +355,30 @@ class MainActivity : AppCompatActivity() {

val uri = intent.data
if (uri != null) {
lifecycleScope.launch {
try {
otpTokenDatabase.otpTokenDao().insert(OtpTokenFactory.createFromUri(uri))
} catch (e: Exception) {
Snackbar.make(binding.rootView, R.string.invalid_token_uri_received, Snackbar.LENGTH_SHORT)
.show()
}
}
// Security fix: show confirmation dialog before importing any externally supplied
// otpauth:// URI. Previously, tokens were silently inserted, allowing a malicious
// app or phishing webpage to inject arbitrary OTP profiles via a crafted intent.
val rawPath = uri.path?.trimStart('/') ?: ""
val issuer = uri.getQueryParameter("issuer")
?: rawPath.substringBefore(":").ifEmpty { getString(R.string.unknown_issuer) }
val account = rawPath.substringAfter(":", "").ifEmpty { getString(R.string.unknown_account) }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty account should not be allowed. And issuer can be absent but strongly recommended. If issuer is empty, then leave it empty.


MaterialAlertDialogBuilder(this)
.setTitle(R.string.add_token_confirmation_title)
.setMessage(getString(R.string.add_token_confirmation_message, issuer, account))
.setIcon(R.drawable.alert)
.setPositiveButton(R.string.ok_text) { _, _ ->
lifecycleScope.launch {
try {
otpTokenDatabase.otpTokenDao().insert(OtpTokenFactory.createFromUri(uri))
} catch (e: Exception) {
Snackbar.make(binding.rootView, R.string.invalid_token_uri_received, Snackbar.LENGTH_SHORT)
.show()
}
}
}
.setNegativeButton(R.string.cancel_text, null)
.show()
}
}

Expand Down Expand Up @@ -526,4 +556,4 @@ class MainActivity : AppCompatActivity() {
const val SCREENSHOT_MODE_EXTRA = "screenshot_mode"
const val SHARE_FROM_PACKAGE_NAME_INTENT_EXTRA = "shareFromPackageName"
}
}
}
8 changes: 7 additions & 1 deletion app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@
<string name="authentication_dialog_subtitle">Please use biometric or PIN to authenticate FreeOTP+</string>
<string name="unable_to_authenticate">Unable to authenticate. Do you have screen lock enabled with PIN / biometric enabled?</string>

<!-- Confirmation dialog shown when an external app or webpage supplies an otpauth:// URI -->
<string name="add_token_confirmation_title">Add OTP Account?</string>
<string name="add_token_confirmation_message">An external source wants to add an OTP account:\n\nIssuer: %1$s\nAccount: %2$s\n\nOnly proceed if you trust this source.</string>
<string name="unknown_issuer">Unknown</string>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments before. Empty issuer is OK, empty account is not allowed. Do not need to add those strings

<string name="unknown_account">Unknown</string>

<string name="five" translatable="false">5</string>
<string name="six" translatable="false">6</string>
<string name="seven" translatable="false">7</string>
Expand All @@ -80,4 +86,4 @@
<string name="category">Category</string>
<string name="all_categories">All categories</string>
<string name="uncategorized">Uncategorized</string>
</resources>
</resources>