Fix app selection #1206
Fix app selection #1206
Conversation
The previous code relied on a "save" button, which would not actually make changes take effect. The "back" button used to forget the changes. The new code always enables or disables apps directly when their checkbox status changes. This removes the need for a save button; more importantly, it prevents users using an unsecured connection by accident because they think their changes take effect immediately.
|
Sorry, I realized my bug report was incorrect in that "SAVE" automatically returns from the activity. So point 2 isn't necessarily valid. My apologies. |
|
Thanks for the report and effort to improve this area. We will review and consider! |
|
The problem with this is that everytime you check or uncheck an app in the UI, it will fully reboot the VPN stack, potentially leaking traffic each time you do that. Not sure this is an improvement. |
If that is an actual bug (in Android or Orbot), it would happen with or without my patch, and we need to warn users about it! They cannot safely change the apps which are VPN'd while Orbot is in use, no matter how we modify the app set. It would happen slightly more often with the patch, admittedly, but I don't see how that's relevant: it shouldn't happen at all, and if it's unavoidable, the UI needs to reflect that. A "save" button certainly doesn't imply that traffic is being leaked while the save is in progress.
I've asked a few people, and the "SAVE" button simply isn't being recognized. Either they swipe back, which silently (there's not even a warning!) throws away any modifications to the app set, or they switch apps, which means the modifications will be ignored until the user switches back and hits "SAVE", if they ever do. So the current state is that Orbot fails to protect traffic from apps that the user went to the trouble to select, and that's definitely something we should strive to improve. |
|
How about displaying a toast message to communicate this to the user? This seems like the most straightforward way until a fix can be found. |
I was quite shocked today when I enabled an app in the Orbot app selector, only to find out it hadn't been torified.
There are two problems with the current code:
I really don't think there's any excuse to display a checked checkbox next to an app logo unless that app is actually running torified at that very moment. I think we can agree that is a problem that needs to be fixed, even if we disagree about whether I've chose the right way to fix it.
My fix is to remove the "save" button entirely and simply restart the VPN synchronously whenever a checkbox changes. This doesn't seem to cause noticeable delays, but even if it were to cause them, they're necessary, because we really don't want the user to falsely believe they're running through Tor when they're not.
Again, I'm happy to find another way to solve this