Skip to content
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

PR3296 auto export (DRAFT) #3451

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

vanelsberg
Copy link
Contributor

@vanelsberg vanelsberg commented Oct 2, 2024

This PR#3451 enables automated settings export.
As settings export needs the master password for encryption the password is stored on the local phone's app datastore on manually exporting settings from the maintenance menu. Password will expire/wipe when not used for the number of days set.
Stored password wil be wiped on settings master password (or forced with file 'ExportPasswordReset' in /AAPS/extra)

Functionality is enabled through a new setting in Maintenance preferences.
Once enabled (and password set through manual export) user can define the automatic settings export from automation.

Actions also wil be in registered to "Treatments" (careportal/User entry)

TODO: Review/determine if storing the master password unencrypted local phone's app storage is acceptable.
See also issue #3296

Additional note:
This PR implementation should enable implementation for auto exporting to alternate storage locations (like the Downloads folder or Cloud). But this will be subject for another PR.

@vanelsberg
Copy link
Contributor Author

@Milos Needs some help on Quality checks please?
Tests: todo for final PR.
Code duplication: can see the problem?
(FUI: Testen PR merge ok at my end)

@MilosKozak
Copy link
Contributor

where is the password stored?

@vanelsberg
Copy link
Contributor Author

vanelsberg commented Oct 2, 2024

where is the password stored?

On the phone's local storage (app's data store). Unencrypted but "protected" as one needs to connect through the debugger to get access). Stored password will be removed when not used as per expiry setting.

Even though not encrypted, question is what encryption would add?
Imho getting the stored password is not relevant as retrieving the master password is easy (user:) through password reset or (devs:) code change or debugger niffing? Apart from the fact that physical access to the phone is necessary.

Module has "hooks" for encryption/decryption but that would need another password/key, filling in needs additional research. (You already suggested using the NS token?).

Copy link

sonarcloud bot commented Oct 3, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
5.6% Duplication on New Code (required ≤ 4%)
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@robertrub
Copy link

Testing this PR. All seems to be working as expected. No psw is asked for settings export (after Medtrum Nano activation) and the automation is exporting settings as planned.
Has set forget password to 14 days. Waiting for that limit to arrive.

@vanelsberg
Copy link
Contributor Author

Working on a new PR that is securely encrypting the password required for unattended exporting.
Expect details in the following weeks... ;-)

@olorinmaia
Copy link
Contributor

Tested ok with an expiration of 7 days. I made an automation that automaticly exported settings when cannula age was less the 0,1 hours. It worked great! My biggest wish for this is not to be needed to re enter password and eventually add upload to google drive! Great work!

@vanelsberg
Copy link
Contributor Author

vanelsberg commented Oct 9, 2024

Tested ok

Great to here 👍
New PR will have similar functionality with improved code and securely encrypted password.

.. eventually add upload ...

Exporting to alternative locations like Cloud can be in a follow up PR which builds on this.

@olorinmaia
Copy link
Contributor

Can password then be stored indefinetely?

To import settings on another phone, if that what we are afraid of, it will be needed to re-enter password.

@robertrub
Copy link

I think, it is better to ask for the psw regularly. If not, the day when it will be needed, nobody will remember it... Most people do not use password apps.

ATM, the number of days is set by the user.

@olorinmaia
Copy link
Contributor

I see your point. My android phone saves password in google account. I also keep a backup of it in another file just in case. It should surely be an option if you dont want to re enter if it is possible

@vanelsberg
Copy link
Contributor Author

vanelsberg commented Oct 10, 2024

Can password then be stored indefinetely?

Current PR only discards when user does not use the stored password.

Nope. In the final PR (expect soon) password expires after 4 weeks, with 1 week grace period.
User is alerted about expiry and need to re-enter.

To import settings on another phone, if that what we are afraid of, it will be needed to re-enter password.
Nothing changes there. Master password will always be needed to import on new phone (or install)
(Stored password will only be available on the phone it was stored anyway)

That was why we thought it would be best to ask the user to-reenter regularly.

@vanelsberg
Copy link
Contributor Author

Exactly that 👍

ATM, the number of days is set by the user.

In the new PR - no longer. Expiry will be fixed at 4 weeks.

@vanelsberg
Copy link
Contributor Author

vanelsberg commented Oct 11, 2024

Closing this PR soon.
As this PR was initiated to start supporting to the original feature request #3296

This PR will be replaced by a new PR. See #3461

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants