UserManager race conditions

[johniez]

UserManager update() method reads and writes users.json file, but if two processes (for example two people will login at ~ same time) performs update() simultaneously, the later one can overwrite the data with some old version. Same applies for delete_user() method, but it is probably unused now.

And a note for auth: writing 'authenticated' flag into users.json is not necessary, as the user successfully loaded using loginmanager.user_loader has to be authenticated (or he was, just before he was written to the session after successfull login).

[alexanderjulo]

Very well possible, yeah. Unfortunately I am not really maintaining this project very much at the moment, so if you'd propose a fix (via PR) I'd happily merge it but I do not currently have the time to write one myself, sorry!

[johniez]

I can send a pull request, if adding python-fasteners dependency is not a problem :-)
It implements InterProcessLock for both windows and posix systems...

[alexanderjulo]

Whether it should be that library or another one is probably discussable but from what my quick research turned up this seems to be a difficult topic, so using an external library for locking stuff should be alright. We should probably implement it for other places aswell, not only the auth file.

[johniez]

In fact, Page.save() is not ready for concurrent usage at all. Current implementation is very simple, but does not cover situations where two or more people starts editing same page at some point, saving it far later (the last one will overwrite all others changes). So it needs locking, but it will make more sense with some "conflict guard" (at least file mtime or hash at the start of page editing).

[alexanderjulo]

Agreed, yes.