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

WIP: Automatically add passwords to password store #143

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

Conversation

ozeidan
Copy link

@ozeidan ozeidan commented Apr 30, 2019

Hi Guys,

I hacked on this project a little bit and added a prototype UI and the functionality to conveniently add passwords to the password store, according to #61. The credentials are pulled from HTTP requests whenever the user submits a form. If the credentials don't exist in the password store yet, they are shown in the popup window, in a new UI, along with buttons to either save or dismiss the credentials. The user is also able to choose a store from the UI and change the credentials/the path in which they are stored. When saving, the password file is also properly checked in to the git repository of the password store. When dismissed, the credentials are memorized (should probably be hashed) so that the user is not bothered by the popup when he submits these credentials again.

image

These changes are still WIP but I would like to keep working on them until this PR is deemed ready to be merged. Please take a look at it and let's discuss what needs to be done and in which way the new feature should be designed. I'll create a PR with the corresponding host changes in the browserpass-native repository.

@maximbaz
Copy link
Member

Hi @ozeidan, thanks for the contribution, nice work!

I'm actually working on this too now, sorry we duplicated this work 🙂

However I envision this feature differently, I don't think detecting if currently entered credentials do not exist in the password store yet is feasible, because in order to do that you would need to actually decrypt potentially multiple files and see if this password is stored inside.

My approach is fully manual, users will click "add credentials" button if they want so. Here's a gif I recorded early on, it will be a bit different in the end, for example it will allow to control password generation (length and characters):

Mpv5t7M - Imgur

@ozeidan
Copy link
Author

ozeidan commented Apr 30, 2019

Hi @maximbaz, your UI definitely looks nicer than mine :)

I also planned on implementing a 'manual' route through a button in the popup just as yours.

With my patch the password only checks whether a file at the calculated path exists, whereas that path is currently always calculated as url/username. This is of course a problem when you change the password of some account, as that will be dismissed, but maybe this can be solved through some design changes. For example, we could always remember the new password and let the user manually overwrite it using a button next to the matching login in the UI.

What I was going for is a mixture of my current implementation and what is shown in your gif, including options for generating passwords (but I thought maybe we can auto-fill them into registration forms). The reason why I want the automatic creation of password files is because I know it that way from proprietary password managers which I have previously used. I just create an account normally on a website (using an auto-filled generated password) and just press one additional button to have it stored. In my experience, this reduces the amount necessary for managing my password database just below the threshold at which I resort to just using the same password for everything out of convenience.

So maybe we can find some middle ground? Otherwise I'd be happy to contribute on some other features I had in mind.

@maximbaz
Copy link
Member

I'll have to think about it, I must say it is very interesting idea - one approach might be to track the last transmitted credentials and use them to prefill "Add credentials" dialog, so when you click on "Add credentias", the form is either empty, or it is already prefilled and you can edit it as you wish.

This way we won't be checking if such credentials exist anywhere in the password store... 🤔

@maximbaz
Copy link
Member

@ozeidan I'd like to keep your PR open because I'll use parts of it as an inspiration for my implementation.

You said you wanted to contribute on some other features, in the issues list we usually use assignments to indicate when someone has started working on it, but in any case I'd recommend you dropping a line in the issue you are interested to fix, saying that you want to start working on it, if it is assigned to someone perhaps ping the person asking how far they progressed and if they want you to implement it (maybe they started, but are currently busy).

And thanks again for your help, it is very much appreciated!

As for this issue, I'll make sure to ping you for early testing.

@maximbaz maximbaz added the blocked Cannot progress without first resolving something else this depends on label Apr 30, 2019
@erayd erayd mentioned this pull request Jun 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Cannot progress without first resolving something else this depends on
Development

Successfully merging this pull request may close these issues.

2 participants