search

paulgreg / UniquePasswordBuilder

A bookmarklet to generate strong password per site
https://paulgreg.me/UniquePasswordBuilder/
MIT License
8 stars 4 forks source link

Add support for Argon2 algorithm #6

Closed pmiossec closed 6 years ago

pmiossec commented 6 years ago

It begins to take good shape. I create this PR on work in progress to be able to discuss about it.

Things done:

Tell me if you are aware of something missing...or have some remarks!

paulgreg commented 6 years ago

Also, there should be an option on the page and the extension to select which algorith (and so, show one or another difficulty).

paulgreg commented 6 years ago

Also, it would be nice to update tests to be sure default algorithm is still scrypt while adding some testfor argon processing.

pmiossec commented 6 years ago

I have finished the development from my point of view and cleaned the history as much as I can to make the review easier.

pmiossec commented 6 years ago

I think I have fixed everything we told about... I have tested the extension on Chrome and Firefox without problem. I have also made a quick test and the passwords generated with scrypt are the same, so it seems that there is no regression 😄

paulgreg commented 6 years ago

Thank you. I noticed a bug on the firefox addon : the url and the password is not computed until option have been opened. Do you have any idea why ?

pmiossec commented 6 years ago

I don't have this problem. But I have made the setTimeout() for more display (especially in addon).

Could you test again?

pmiossec commented 6 years ago

I have also added a shortcut to open the extension (ctrl + shift + L) It works well \o/

paulgreg commented 6 years ago

The shortcut is very nice ! Also, refactoring the common control is very nice too. Thanks.

I’m still getting the problem on the firefox addon : right after installing the addon, the URL input is never filled : bug It’s not until option are clicked that it begins to work. If you don’t have the problem, I’ll try to debug it, maybe this evening.

paulgreg commented 6 years ago

By the way, the to_ghpages.sh script is working great. I just add to change line ending to make it work under linux.

paulgreg commented 6 years ago

The addon problem is fixed in https://github.com/paulgreg/UniquePasswordBuilder/commit/c7a3a1a4be664c565b4a17dae49364ff26a7014c But I have another problem : I’ve got different result from both addon. Investigating.

paulgreg commented 6 years ago

I’ve also fixed line ending in to_ghpages.sh script in https://github.com/paulgreg/UniquePasswordBuilder/commit/80f7202ef5395e0a00baa387781c84ed79b26c8c and removed commented code. Hope it still works on your end.

paulgreg commented 6 years ago

I’ve got different results for about:debugging pages but there was a bug before : https://github.com/paulgreg/UniquePasswordBuilder/commit/b4c57c19de78cf3dbe550afff5d64a7e53453ab1#diff-db67ee7f7f9d95a95b6341bcc6f66576R509 Since about pages are not very relevant, no need to handle it like before.

paulgreg commented 6 years ago

I’ve made some little changes in my branch https://github.com/paulgreg/UniquePasswordBuilder/commits/argon2 . I will now try the new addon / page for some days, just to be sure before accepting the PR. Thanks again for you great work @pmiossec !

pmiossec commented 6 years ago

Well done especially for the fix and the unit tests on getSaltOnLocation(). I wanted to add it but forgot...