ViSolve,
Thanks taking a look at the code.
With just those changes, a user migrating from the old password storage mechanism to the new will get kicked out without knowing why as will a user changing their password for themselves as the SESSION[‘authPass’] won’t match. So we either need to update the session variable properly in those cases too, or provide a warning.
Additionally I think it’s cleaner to turn the existing query in authCheckSession into a join of users and users_secure rather than doing two queries.
Code to be reviewed for active user check and password hash match.
Testcases I tried:
A. User with password in old format.
Executed this query through MySqlWorkbench
update users set password=sha1(“pass”) where username=‘admin’ and id=‘1’;
delete from users_secure where username=‘admin’;
Then logged in successfully.
B. Update user’s own password with Misc>password
C. Update user’s own password with Administration > Users
Not kicked out in either case (correctly)
(Prior to addition modification ins password_change, was getting kicked out as expected.)
D. Update other user’s password with Administration > Users
Admin not kicked out, but other user kicked out on next operation
E. Set other user as inactive with Administration > Users
Other user kicked out on next operation.
The parent commit in this branch is the oemr_ prefixing of the password_hash and password_salt function for PHP 5.5. compatibility if anybody wants to do a quick sanity check.
Brady,
After we’ve heard from you and if there are no other objections I can push all of these changes to the master repository. (your site ID fix, plus these two commits.)
P.S. I lettered the testcases here rather than numbering, because SF seems to mess with them through some kind of autonumbering scheme.
I have faith in your password/user stuff. I’ll add the site id check stuff after some testing to ensure not breaking patient portal. (Something that will be cool to test is to also ensure the across site access vulnerability is shut down with your above password check, which I plan to try out; then with 2 layers of protection against this via the hash check and the site check).
Will plan to get all this stuff in the next 4.1.2 patch, so the earlier it gets into the master branch for testing the better.
Committed above multisite check commit to codebase. After some testing time in master, will plan to place these security/php5.5 related commits into a 4.1.2 patch.