matteo-convertino
commented
7 months ago I like the implementation with localstorage. Even if it is clear, we must take into account that Nextcloud authentication is present in the first place anyway. If any hacker manages to get to the extension there is probably another type of problem. I also really like the fact that if you log out of Nextcloud, the localStorage is also cleaned.
However the only thing I did was fix the syntax a little but otherwise it was perfect. Thank you so much for contributing, I hope for more PR too.
Fair warning: I've never worked with any modern frameworks or really written much code recently so it's entirely possible I've done something the wrong way. I just think this is a great project and I wanted to try and contribute. If this is no good I won't be offended!
This change adds a "Remember password" option to the password screen which writes the password to localStorage and loads it again when the page is refreshed. It would likely help with #16 as well as generally being useful.
In my mind this is an acceptable level of security as the password is only saved locally on the device and accessible by the same origin however it is in plaintext in localStorage. Presumably the password is also stored in the mobile apps in a similar way.
If it's unacceptable I could see an improvement being the server storing a unique key for the user which it sends and is used to encrypt/decrypt the password. That would mean that if not logged into Nextcloud or otherwise offline the password would be inaccessible. This seemed slightly beyond my initial attempt (working out DB migrations etc) but maybe is required?
Another option would be just saving in sessionStorage rather than localStorage.