user_backend_sql_raw
user_backend_sql_raw copied to clipboard
PanCakeConnaisseur
feat: Allow to use custom password hash
Hi, what is the purpose of a custom password verification class? It seems to me that the available hashing algorithms should be sufficient for 99,999% of users? I am skeptical about adding more code without a good reason.
I made integration with a software that the hash of password is made by md5(md5($passowd)). Unfortunally isn't all systems that use functions like password_hash and password_verify using the default algorithms.
Would adding an option to not hash the password at all solve your problem? Then you could use a DB trigger or stored procedure to intercept the password INSERT and apply the double md5 hash this way. PostgreSQL and probably MariaDB support md5 hashing out-of-the-box.
The system that I made the integration is a software with more than 20 years of development. Is an open source ERP made to cities of Brazil. Already is used by a lot of cities.
The stored password at database already have douple md5 and isn't possible to change without touching the ERP code. I only wish to make to work the integration of login between this ERP and Nextcloud.
My proposed solution would only work for new passwords but not for checking existing ones, you're right.
My proposed solution would only work for new passwords
Yes, this is the critical point. Is necessary to be possible login using the stored hash password. Unfortunately have applications that don't use password_hash to store password and that implement different ways.
I send a push fixing the conflicts.
The class lib/HashPassword.php is only by example.
Follow the implementation that I made with a custom class:
https://github.com/e-cidade/ecidade_login/blob/ef284e244c63810c4a46151b188dbbb7fb97396b/lib/HashPassword.php#L5-L16
Maybe also could be nice to have a real example at README with a link to this implementation. What do you think about?
@vitormattos I thought about this solution more. AFAIU you still need to implement your own class even if this code is merged, since HashPassword.php is only an example and not really what your DB does with double hashing with md5, right? In that case, you would need to keep your own forked version anyway, right?
Regardless, I am very hesitant to add this to the repository, because it seems to be a very corner case which would complicate the code base. "Injecting" classes by configuration is really on another level than just changing SQL configuration. I would like the users to remain administrators and not developers.
What about this, instead: You could add a class MyUserBackend that extends UserBackend and then only override checkPassword() in it. Then in Application.php you add a one-line patch that changes boot() and registers your class MyUserBackend instead of UserBackend. This still requires some code changes on your side that you would have to maintain in a fork, but AFAICS it will be much less than in this PR. Do you think this makes sense?