OpenArchive / Save-app-android-old

This is the Save app for Android
https://open-archive.org
GNU General Public License v3.0
94 stars 26 forks source link

Fingerprint or Pin App Unlock #551

Closed ryjen closed 4 months ago

ryjen commented 4 months ago

I Like... When the application is put into background or the user is inactive for a certain period, the user must unlock with a fingerprint or pin

I Wish... The Android app behaved like iOS and was more secure. If a user wanted to prevent someone from snooping the application, it would happen automatically.

What If...

Information

There is a bit of scope creep in determining when an application is put into the background. The main activity lifecycle handlers are one possibility, but not always the most reliable.

Another possibility I have seen implemented is a user interaction timeout. A background counter that gets updated on lifecycle changes. If the counter gets too high, the user is inactive, and requires an unlock.

This also a ticket for UX to provide mockups if available.

tladesignz commented 4 months ago

Relax, man. Most of what you need is already there:

The only thing now which needs to change, is to check for another flag: the app-lock flag. If that is set, but no ProofMode key encryption, then just trigger the biometry anyway by encrypting/decrypting a nonce. If both are set, nothing needs to change. Just do the ProofMode key decryption dance.

Since the main activity is always there, as long as the app runs, the lifecycle of that should be good enough to bind this feature to. We can think later about if there's a situation where we leave the user in the rain with this implementation. Doubt it, though. Anyway, this is a step towards feature parity.

For comparison, this is the implementation on iOS:

ryjen commented 4 months ago

OK, thank you. I am actually a little unclear on the requirements for this feature to begin with; this sounds like more of a bug. Perhaps some state flow diagrams will help next time around.

tladesignz commented 4 months ago

OK, thank you. I am actually a little unclear on the requirements for this feature to begin with; this sounds like more of a bug.

Not a bug. The request is easy: People might want to have an extra lock in front of the app, so if their phone accidentally gets caught by another person while unlocked, that person cannot access the app without further ado.

The slight complication added in here, is, that this overlaps with the ProofMode key security feature. To increase the security of ProofMode (and hence the provenance guarantees of the media uploaded), one can store that key encrypted. The decryption key for that is in turn stored in the hardware-based key store (Apple calls this the "secure enclave"). That hardware-based key store is unlocked with biometry or with the device passcode.

Since we don't want the user to run through the fingerprinting two times, we either use the ProofMode key decryption event as the extra lock in front of the app. Only, if that is not used by the user, we generate the same situation by encrypting/decrypting a nonce.

Perhaps some state flow diagrams will help next time around.

Sure, why not do it yourself? That would actually be a great learning opportunity. Read the code, read the issues, try out the iOS app, read about the 3rd party libs used (like ProofMode), ask the people, then compile everything in a doc / diagram.

ryjen commented 4 months ago

Ok, thank you. I am a little unclear on why there are no docs to begin with.

tladesignz commented 4 months ago

Because this is a small project which runs since 10 years on minimal effort with very few people.

There is no professional continuous project management, there is no continuously updated unified specification documentation. There is no huge team. There are tons of tickets and feature requests accumulated over time. There are different UI/UX design "documents"/ Figma "files" which were valid at the time of their creation.

This is not an infrastructure project which needs to withstand the test of time. We're moving fast and with minimal effort. Nobody was interested in writing documentation until now, and especially not in keeping the documentation current. The feature set is pretty obvious, as soon as you start using the app from front to back once.

But you're here to change that now! Awesome! Congratulations!

ryjen commented 4 months ago

I can do without the dissent and condescension. Not everything can be learned from digging through code. I'd rather read some documents to get a whole picture and get some process in order before tweaking and breaking like a newb for your pleasure or reusing components for feature requests.

A project over 10 years with changing contractors and management is a pretty good reason to have a software architecture document. Especially on Android, where the security is more difficult.

The last thing I want to do is change anything.

ryjen commented 4 months ago

After testing a bit, this feature is working if you have biometrics setup.

I think what is missing is the prompt to setup biometrics if its missing (part of the SDK).

And some improvements on the UX overall. Will close for another ticket.

tladesignz commented 4 months ago

I can do without the dissent and condescension.

Same here. However, dissent needs to be resolved, not swallowed. If you don't like me telling you what you're doing wrong (or what you should do), than either you need to make Natalie fire me or continue to be annoying until I throw the towel.

Or you start working with me, not against me.

Your choice.

Not everything can be learned from digging through code. I'd rather read some documents to get a whole picture and get some process in order before tweaking and breaking like a newb for your pleasure or reusing components for feature requests.

A project over 10 years with changing contractors and management is a pretty good reason to have a software architecture document. Especially on Android, where the security is more difficult.

Very well. Then I suggest you stop complaining and start to write these documents. You currently have the perfect outside view: you seem to have a clear understanding of what this document should contain and by interviewing me (in other words: asking the right questions), you will dig out the important things you want to know.

Now is the first time, there are more than 2 persons on this project. You are hired as tech lead. Complaining is not leading. Improving things is leading.

The last thing I want to do is change anything.

Huh? You're hired for changing things around here. I very much hope, you'll going to change things.

ryjen commented 4 months ago

I am sorry I came across as a complainer; it would be like complaining against a version of myself.

I am trying my best to navigate Natalie and the team, so I have already started a high-level SAD (software architecture document) to help put everyone on the same page.

You will likely be looped in and intergated obscenely for a first draft or diagrams.

It should help the next person who wants to refactor by understanding all the components, decisions, and how they interact.

I should rephrase: I definitely think I can help improve things, but I am very wary of premature optimization, incomplete change, and stepping on people's toes.

tladesignz commented 4 months ago

Nice! Sounds like we finally start to align our thoughts and goals.

Don't let yourself get infected by confusion and stress.

This project ran on very tight budgets for long years and the Android version went through a lot of hands, so there's a lot to improve.

Breaking things is a side-effect of that. We're not afraid. Nobody's coming after you for that. Our user base is tiny and toughened. They will endure, as long as the overall trend stays in the "improvement" direction.