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

Optional password export #4700

Closed
wants to merge 1 commit into from

Conversation

hannesa2
Copy link
Contributor

image

Because #4696 was declined on less reflected doubts, maybe we can optional export the passwords

@hannesa2
Copy link
Contributor Author

But before this must be solved #4698 (I know it's maybe related to #3691)

When #4698 or #3691 is solved (not by me) I can finish my tests here

@hannesa2
Copy link
Contributor Author

image

First of all, you added code style to git 👍

Sorry, but the outcome with ktLint is not good. 👎
Ok, you force an organized import order, but both function

image#

changes nothing at all.

I would recommend to remove this check in ktLint, or change codestyle to make it work automatically

@cketti
Copy link
Member

cketti commented Apr 29, 2020

Currently IntelliJ IDEA/Android Studio can't be configured to define the import order for Kotlin files. AFAIK this will be fixed soon. In the meantime you can use the ktlintFormat task to reformat the files, see also https://github.com/k9mail/k-9/wiki/BuildingK9#checkfix-code-style

Thanks for including the screenshot of the modified export screen. I don't quite like the idea of simply including a "Passwords" entry in the list. The option to include passwords when exporting accounts feels more like a modifier to the selection, not like something that belongs in the list of objects to export.
I'd also like to include some warning text that clearly states that this will include passwords in clear text, so users need to be careful where they save the resulting file.

I currently don't know how a good UI that satisfies these points could look like.
@jancborchardt: Do you maybe have a suggestion?

@hannesa2 hannesa2 changed the title Optional password export WIP: Optional password export May 6, 2020
@hannesa2 hannesa2 force-pushed the OptionalPasswordExport branch from 8a3f20d to 8bc7517 Compare May 6, 2020 04:57
@hannesa2
Copy link
Contributor Author

hannesa2 commented May 6, 2020

Now, the user knows about the consequence

image

About the UI: you can change it later, when you see a better place. Maybe a separator is fine ?

@hannesa2 hannesa2 force-pushed the OptionalPasswordExport branch from 7abcc6f to 0fc2cb1 Compare May 6, 2020 05:26
@hannesa2
Copy link
Contributor Author

hannesa2 commented May 6, 2020

The password import works, but a folder sync is missing and only Outbox is shown.
image

After sync folders, it works properly.

@cketti I need your advise. Please can you point me to the place and what's to do then, to do this automatic after import

@jancborchardt
Copy link

Now, the user knows about the consequence

image

About the UI: you can change it later, when you see a better place. Maybe a separator is fine ?

In addition to this wording, I would say this should be unchecked by default. What do you think?

@hannesa2 hannesa2 force-pushed the OptionalPasswordExport branch from 0fc2cb1 to 45cb4e3 Compare June 4, 2020 04:13
@hannesa2 hannesa2 force-pushed the OptionalPasswordExport branch from 45cb4e3 to 7d645da Compare June 4, 2020 05:42
@hannesa2
Copy link
Contributor Author

hannesa2 commented Jun 4, 2020

In addition to this wording, I would say this should be unchecked by default. What do you think?

After fix build after project restructure, I'm a little bit tired about maintain this PR.

I see it as a nice feature to prevent from password-hell after device change.
@jancborchardt The user (especially me) wants to get rid of password enter, and your suggestion incapacitated the user again.
Why this maximum prevent user from well working stuff ?

@hannesa2 hannesa2 changed the title WIP: Optional password export Optional password export Jun 5, 2020
@hannesa2
Copy link
Contributor Author

hannesa2 commented Jun 5, 2020

@cketti Any change to get this merged ?
A rename, changed default.... could be made later on

@cketti
Copy link
Member

cketti commented Jun 5, 2020

I was thinking more along the lines of something like this:

k9mail_mockup_settings_export_with_passwords

Yes, the option would be unchecked by default.
Yes, this looks scary.
Yes, this doesn't make things easier for the average user.

All of that is intentional. Exporting plain text passwords is dangerous. The only reason I'm even considering this is because someone else is willing to do the work and it's much more work to add a way to support exporting passwords in a secure way (that probably means encrypting all of them using a master password). This way at least users that are knowledgeable enough and accept the risks have the option of exporting passwords.

Why all the fuss? I believe that it is our job as developers to keep the user's data safe. The average user is not a tech expert and doesn't have the knowledge required to make a proper risk assessment. And they shouldn't have to. It is our job.
Sometimes that also means not adding a feature that would greatly improve the user experience if it would also increase the risk beyond acceptable levels. Of course the acceptable level of risk is highly subjective. With K-9 Mail we rather err on the side of safety. But in my experience this is the norm rather than the exception in the free software community.

Merging an incomplete feature and then finishing things later is totally acceptable in some situations. Adding something dangerous and then adding the safety switch later (maybe? worst case is I'll have to do the work myself; on a feature I don't think is a great idea in the first place) is not something I'm keen on doing.

@hannesa2
Copy link
Contributor Author

hannesa2 commented Jun 5, 2020

Sorry, all is just to make thing not/bad working

@hannesa2 hannesa2 closed this Jun 5, 2020
@hannesa2 hannesa2 deleted the OptionalPasswordExport branch June 5, 2020 12:51
@hannesa2
Copy link
Contributor Author

hannesa2 commented Jun 5, 2020

There is nothing incomplete, simply added like other switches before.
If someone has additional wishes, he can do it.

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.

3 participants