-
Notifications
You must be signed in to change notification settings - Fork 10
feat: add pincode protection to settings #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| .PHONY: build clean install install-debug install-release test help | ||
|
|
||
| # Default target | ||
| help: | ||
| @echo "Available targets:" | ||
| @echo " build - Build debug APK" | ||
| @echo " build-release - Build release APK" | ||
| @echo " clean - Clean build artifacts" | ||
| @echo " install - Install debug APK to connected device" | ||
| @echo " install-release- Install release APK to connected device" | ||
| @echo " uninstall - Uninstall app from connected device" | ||
| @echo " test - Run tests" | ||
| @echo " lint - Run lint checks" | ||
| @echo " assemble - Build all variants" | ||
|
|
||
| # Build debug APK | ||
| build: | ||
| ./gradlew assembleDebug | ||
|
|
||
| # Build release APK | ||
| build-release: | ||
| ./gradlew assembleRelease | ||
|
|
||
| # Build all variants | ||
| assemble: | ||
| ./gradlew assemble | ||
|
|
||
| # Clean build artifacts | ||
| clean: | ||
| ./gradlew clean | ||
|
|
||
| # Install debug APK to connected device | ||
| install: build | ||
| ./gradlew installDebug | ||
|
|
||
| # Install debug APK without building (if already built) | ||
| install-debug: | ||
| ./gradlew installDebug | ||
|
|
||
| # Install release APK to connected device | ||
| install-release: build-release | ||
| ./gradlew installRelease | ||
|
|
||
| # Uninstall app from device | ||
| uninstall: | ||
| ./gradlew uninstallAll | ||
|
|
||
| # Run tests | ||
| test: | ||
| ./gradlew test | ||
|
|
||
| # Run lint checks | ||
| lint: | ||
| ./gradlew lint | ||
|
|
||
| # Run app on connected device | ||
| run: install | ||
| adb shell am start -n com.immichframe.immichframe/.MainActivity | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,10 +25,13 @@ import android.webkit.WebResourceRequest | |
| import android.webkit.WebSettings | ||
| import android.webkit.WebView | ||
| import android.webkit.WebViewClient | ||
| import android.text.InputType | ||
| import android.widget.Button | ||
| import android.widget.EditText | ||
| import android.widget.ImageView | ||
| import android.widget.TextView | ||
| import android.widget.Toast | ||
| import androidx.appcompat.app.AlertDialog | ||
| import androidx.activity.result.contract.ActivityResultContracts | ||
| import androidx.appcompat.app.AppCompatActivity | ||
| import androidx.appcompat.app.AppCompatDelegate | ||
|
|
@@ -161,7 +164,7 @@ class MainActivity : AppCompatActivity() { | |
| onNextCommand = { runOnUiThread { nextAction() } }, | ||
| onPreviousCommand = { runOnUiThread { previousAction() } }, | ||
| onPauseCommand = { runOnUiThread { pauseAction() } }, | ||
| onSettingsCommand = { runOnUiThread { settingsAction() } }, | ||
| onSettingsCommand = { runOnUiThread { settingsActionFromRpc() } }, | ||
| onBrightnessCommand = { brightness -> runOnUiThread { screenBrightnessAction(brightness) } }, | ||
| ) | ||
| rcpServer.start() | ||
|
|
@@ -700,11 +703,52 @@ class MainActivity : AppCompatActivity() { | |
| } | ||
|
|
||
| private fun settingsAction() { | ||
| val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(this) | ||
| val savedPassword = sharedPrefs.getString("settings_pincode", "") | ||
|
|
||
| if (!savedPassword.isNullOrEmpty()) { | ||
| // Password is set, prompt for verification | ||
| promptForPasswordVerification(savedPassword) | ||
| } else { | ||
| // No password, open settings directly | ||
| openSettings() | ||
| } | ||
| } | ||
|
|
||
| private fun settingsActionFromRpc() { | ||
| // RPC access bypasses password protection | ||
| openSettings() | ||
| } | ||
|
|
||
| private fun openSettings() { | ||
| val intent = Intent(this, SettingsActivity::class.java) | ||
| stopImageTimer() | ||
| settingsLauncher.launch(intent) | ||
| } | ||
|
|
||
| private fun promptForPasswordVerification(correctPassword: String) { | ||
| val input = EditText(this) | ||
| input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD | ||
|
|
||
| AlertDialog.Builder(this) | ||
| .setTitle("Enter Pincode") | ||
| .setMessage("Enter your pincode to access settings:") | ||
| .setView(input) | ||
| .setPositiveButton("OK") { _, _ -> | ||
| val enteredPassword = input.text.toString() | ||
| if (enteredPassword == correctPassword) { | ||
| openSettings() | ||
| } else { | ||
| Toast.makeText(this, "Incorrect pincode", Toast.LENGTH_SHORT).show() | ||
| } | ||
| } | ||
| .setNegativeButton("Cancel") { dialog, _ -> | ||
| dialog.dismiss() | ||
| } | ||
| .setCancelable(false) | ||
| .show() | ||
| } | ||
|
Comment on lines
+729
to
+750
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider UX and security improvements to PIN verification. Three concerns:
🔒 Recommended improvementsFor rate limiting, consider adding a simple lockout after failed attempts: private var failedPinAttempts = 0
private var pinLockoutUntil = 0L
private fun promptForPasswordVerification(correctPassword: String) {
// Check lockout
if (System.currentTimeMillis() < pinLockoutUntil) {
val remainingSec = (pinLockoutUntil - System.currentTimeMillis()) / 1000
Toast.makeText(this, "Too many attempts. Try again in ${remainingSec}s", Toast.LENGTH_SHORT).show()
return
}
val input = EditText(this)
input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD
AlertDialog.Builder(this)
.setTitle("Enter Pincode")
.setMessage("Enter your pincode to access settings:")
.setView(input)
.setPositiveButton("OK") { _, _ ->
val enteredPassword = input.text.toString()
if (enteredPassword == correctPassword) {
failedPinAttempts = 0
openSettings()
} else {
failedPinAttempts++
if (failedPinAttempts >= 3) {
pinLockoutUntil = System.currentTimeMillis() + 30_000 // 30s lockout
Toast.makeText(this, "Too many failed attempts. Locked for 30 seconds.", Toast.LENGTH_LONG).show()
} else {
Toast.makeText(this, "Incorrect pincode", Toast.LENGTH_SHORT).show()
}
}
}
.setNegativeButton("Cancel") { dialog, _ ->
dialog.dismiss()
}
.setCancelable(true) // Allow dismissal
.show()
}🤖 Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None of these are particular concerns for the use-case of the frame in a residential setting. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @juanjsebgarcia Understood! For a residential frame protecting against children, the current implementation is appropriate. Thank you for clarifying the use case.
Comment on lines
+729
to
+750
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add rate limiting to prevent brute-force attacks. The PIN verification dialog has no rate limiting or maximum attempt restrictions. An attacker with physical access could repeatedly attempt PINs (especially if they're short) until finding the correct one. Consider:
Additionally, note that 🤖 Prompt for AI Agents |
||
|
|
||
| private fun screenBrightnessAction(brightness: Float) { | ||
| val lp = window.attributes | ||
| lp.screenBrightness = brightness | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,44 @@ class SettingsFragment : PreferenceFragmentCompat() { | |
| true | ||
| } | ||
|
|
||
| val chkSettingsPincode = findPreference<SwitchPreferenceCompat>("settingsPincode") | ||
| val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(requireContext()) | ||
|
|
||
| // Update switch state based on whether password exists | ||
| val hasPassword = !sharedPrefs.getString("settings_pincode", "").isNullOrEmpty() | ||
| chkSettingsPincode?.isChecked = hasPassword | ||
|
|
||
| chkSettingsPincode?.setOnPreferenceChangeListener { preference, newValue -> | ||
| val enabling = newValue as Boolean | ||
|
|
||
| if (enabling) { | ||
| // Show confirmation dialog first | ||
| AlertDialog.Builder(requireContext()) | ||
| .setTitle("Confirm Action") | ||
| .setMessage( | ||
| "This will require a pincode to access the settings screen. " + | ||
| "The only way to reset is via RPC commands (or uninstall/reinstall).\n" + | ||
| "Are you sure?" | ||
| ) | ||
| .setPositiveButton("Yes") { _, _ -> | ||
| // Now prompt for password creation | ||
| promptForPasswordCreation(chkSettingsPincode) | ||
| } | ||
| .setNegativeButton("No") { dialog, _ -> | ||
| dialog.dismiss() | ||
| } | ||
| .show() | ||
| false // Don't change preference yet, wait for password creation | ||
| } else { | ||
| // Disabling - delete the password | ||
| sharedPrefs.edit() | ||
| .remove("settings_pincode") | ||
| .apply() | ||
| Toast.makeText(requireContext(), "Pincode protection removed", Toast.LENGTH_SHORT).show() | ||
| true | ||
| } | ||
| } | ||
|
|
||
|
|
||
| val btnClose = findPreference<Preference>("closeSettings") | ||
| btnClose?.setOnPreferenceClickListener { | ||
|
|
@@ -138,4 +176,35 @@ class SettingsFragment : PreferenceFragmentCompat() { | |
| } | ||
| } | ||
| } | ||
|
|
||
| private fun promptForPasswordCreation(chkSettingsPincode: SwitchPreferenceCompat?) { | ||
| val input = android.widget.EditText(requireContext()) | ||
| input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD | ||
|
|
||
| AlertDialog.Builder(requireContext()) | ||
| .setTitle("Create Pincode") | ||
| .setMessage("Enter a pincode to protect settings access:") | ||
| .setView(input) | ||
| .setPositiveButton("Set") { _, _ -> | ||
| val password = input.text.toString() | ||
| if (password.isEmpty()) { | ||
| Toast.makeText(requireContext(), "Pincode cannot be empty", Toast.LENGTH_SHORT).show() | ||
| } else { | ||
| // Save the password | ||
| val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(requireContext()) | ||
| sharedPrefs.edit() | ||
| .putString("settings_pincode", password) | ||
| .apply() | ||
|
|
||
| // Update the switch | ||
| chkSettingsPincode?.isChecked = true | ||
|
|
||
| Toast.makeText(requireContext(), "Pincode protection enabled", Toast.LENGTH_SHORT).show() | ||
| } | ||
| } | ||
| .setNegativeButton("Cancel") { dialog, _ -> | ||
| dialog.dismiss() | ||
| } | ||
| .show() | ||
| } | ||
|
Comment on lines
+180
to
+209
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Enhance PIN validation and user experience. Several improvements would strengthen this feature:
🔐 Recommended improvementsAdd PIN confirmation and minimum length validation: private fun promptForPasswordCreation(chkSettingsPincode: SwitchPreferenceCompat?) {
val input = android.widget.EditText(requireContext())
input.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD
input.hint = "Minimum 4 digits"
AlertDialog.Builder(requireContext())
.setTitle("Create Pincode")
.setMessage("Enter a pincode to protect settings access:")
.setView(input)
.setPositiveButton("Next") { _, _ ->
val password = input.text.toString()
if (password.length < 4) {
Toast.makeText(requireContext(), "Pincode must be at least 4 digits", Toast.LENGTH_SHORT).show()
} else {
// Prompt for confirmation
promptForPasswordConfirmation(password, chkSettingsPincode)
}
}
.setNegativeButton("Cancel") { dialog, _ ->
dialog.dismiss()
}
.show()
}
private fun promptForPasswordConfirmation(originalPassword: String, chkSettingsPincode: SwitchPreferenceCompat?) {
val confirmInput = android.widget.EditText(requireContext())
confirmInput.inputType = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_VARIATION_PASSWORD
AlertDialog.Builder(requireContext())
.setTitle("Confirm Pincode")
.setMessage("Re-enter your pincode:")
.setView(confirmInput)
.setPositiveButton("Set") { _, _ ->
val confirmedPassword = confirmInput.text.toString()
if (confirmedPassword == originalPassword) {
// Save the password
val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(requireContext())
sharedPrefs.edit()
.putString("settings_pincode", originalPassword)
.apply()
// Update the switch
chkSettingsPincode?.isChecked = true
Toast.makeText(requireContext(), "Pincode protection enabled", Toast.LENGTH_SHORT).show()
} else {
Toast.makeText(requireContext(), "Pincodes do not match", Toast.LENGTH_SHORT).show()
chkSettingsPincode?.isChecked = false
}
}
.setNegativeButton("Cancel") { dialog, _ ->
dialog.dismiss()
}
.show()
}🤖 Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This implementation is not designed to be high security, just to keep the innocent out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @juanjsebgarcia, that makes perfect sense! For the use case of keeping children out, the current implementation is appropriate. Thank you for clarifying the design intent.
Comment on lines
+180
to
+209
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enforce minimum PIN length for security. The PIN creation only validates that it's not empty (line 190), allowing single-digit PINs that provide minimal security. Even against the stated use case (preventing children from changing settings), short PINs are easily guessable. Recommend enforcing a minimum PIN length of 4-6 digits. 🔒 Proposed fix to enforce minimum PIN length .setPositiveButton("Set") { _, _ ->
val password = input.text.toString()
- if (password.isEmpty()) {
- Toast.makeText(requireContext(), "Pincode cannot be empty", Toast.LENGTH_SHORT).show()
+ if (password.length < 4) {
+ Toast.makeText(requireContext(), "Pincode must be at least 4 digits", Toast.LENGTH_SHORT).show()
} else {
// Save the password
val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(requireContext()) |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing targets to .PHONY declaration.
The
.PHONYdeclaration is incomplete. Several targets defined in the Makefile are missing:build-release,assemble,uninstall,lint, andrun. Without declaring these as phony, Make may not execute them correctly if files with matching names exist.📝 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 1-1: Missing required phony target "all"
(minphony)
🤖 Prompt for AI Agents