-
Notifications
You must be signed in to change notification settings - Fork 349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A more secure and generalized approach for PDO Basic Auth Backend #1284
A more secure and generalized approach for PDO Basic Auth Backend #1284
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a unit test - THX
@DeepDiver1975 added the requested Unit Tests. All but one test failed and i can not pinpoint why this happens, because i cannot recreate this failed test locally. https://travis-ci.org/github/sabre-io/dav/jobs/709463652 Do you have any idea why this might be happening? |
Codecov Report
@@ Coverage Diff @@
## master #1284 +/- ##
============================================
+ Coverage 97.12% 97.15% +0.02%
- Complexity 2774 2801 +27
============================================
Files 174 175 +1
Lines 8028 8142 +114
============================================
+ Hits 7797 7910 +113
- Misses 231 232 +1
Continue to review full report at Codecov.
|
Unit Test are now passing. |
Hey @DeepDiver1975, i just wanted to check on the progress of this PR and if by any chance it will be merged soon. Best regards. |
@staabm, @ByteHamster, could any one of you guys review this PR or give me some feedback? I think @DeepDiver1975 is a bit too busy atm. |
Are there any chance, to include this into the release? |
@phil-davis @DeepDiver1975 any opinions? |
/** | ||
* Creates the backend object. | ||
* | ||
* If the filename argument is passed in, it will parse out the specified file fist. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment does not make sense to me. I will make a comments-only PR to sort it out.
Nice work and many thx for the new release. Some hints for setting this up: First you should define a $options array in your server.php file like: $options = array(
'digestColumn' => 'digest',
'uuidColumn' => 'username',
'tableName' => 'users',
'digestPrefix' => ''
); After that, you can use it with The $options assume, that you have a table 'users' with a unique 'username' column and a column 'digest'. In 'digest', you can save your hashed passords, generated by the php function passord_hash(). Optionally, you can use a salt to hash your passord and/or use a digestPrefix. I dont know if this should be go up sometime in a small documentation. If not, the info is here. Feel free to correct the info i have found. |
Hello,
motivated by the currently insecure password hashing in sabre/dav (as discussed in Baikal #514), I developed this PR. It allows administrators to choose any password hashing function supported by
password_verify()
(among others, this includes the state of the art hashes bcrypt and Argon).Furthermore this PR is about generalized approach on using the PDO Backend with Basic Authentication. The supplied Backend allows the customization of:
I think this covers a large part of use cases and would benefit a lot of sabre/dav users.
Best Regards.