novnc
Support for .htpasswd Authentication (auth_plugin)
This is a draft PR that implements authentication using a .htpasswd file database hosted on the server.
Is it connected to #590. It also allows to authenticate multiple users.
Changes:
- Added a new
HtpasswdAuthclass that reads a.htpasswdfile database specified by--auth-source. - Implemented password verification using the passlib library to handle hashed passwords.
- Added bcrypt and passlib as a extra dependency in
setup.pyandtest-requirements.txt. - Added tests that use the new mechanism
Nice idea! I think it would be better to have a separate auth plugin for handling auth against a password-db-file, so that your code doesn't have to be intermingled with the code in the BasicHTTPAuth class.
Since you are indicating that this should/could use htpasswd as a db-backend, it would be nice if it could support htpasswd fully, and not just bcrypt.
A dependency to passlib would help implement all the different kinds of encryption that htpasswd can use. Which are plaintext, md5, sha1, sha256, sha512, bcrypt and crypt.
The check could also be implemented more manually.
I think in all cases (including bcrypt which I used so far) the requirements of their respective license may apply, I think they still applies even if it is just a requirement and not a redistribution but I far from an expert on the subject.
The package passlib use a custom license and bcrypt is using Apache2.0 for example.
A dependency to passlib would help implement all the different kinds of encryption that htpasswd can use. Which are plaintext, md5, sha1, sha256, sha512, bcrypt and crypt.
The check could also be implemented more manually.
I think in all cases (including bcrypt which I used so far) the requirements of their respective license may apply, I think they still applies even if it is just a requirement and not a redistribution but I far from an expert on the subject.
The package passlib use a custom license and bcrypt is using Apache2.0 for example.
Passlib sounds like a great alternative. The license is OK to use :+1:
Passlib sounds like a great alternative. The license is OK to use 👍
While looking around the documentation and implementing the change I found that passlib v1.7.4 with the sources available here, seems to have been deserted, and libpass (it's confusing because the python module v1.9.0 is named libpass, but the repository is still passlib) looks maintained, that said an issue about if it's a takeover is opened without real answer.
I checked the repo because I have a warning when using the module with the latest version:
(trapped) error reading bcrypt version
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/passlib/handlers/bcrypt.py", line 620, in _load_backend_mixin
version = _bcrypt.__about__.__version__
AttributeError: module 'bcrypt' has no attribute '__about__'
It can be fixed by pinning the bcrypt version to 4.0.1 or by using libpass.
While looking around the documentation and implementing the change I found that passlib v1.7.4 with the sources available here, seems to have been deserted.
Good finding! It looks like maybe the fork was renamed to avoid name collisions.
It can be fixed by pinning the bcrypt version to 4.0.1 or by using libpass.
Since the libpass project is alive, I think you can go with that!
@ThinLinc-Zeijlon
it would be nice if it could support htpasswd fully
Which method should be used to determine the type of encryption of the database file :
- An additional argument ?
- Determining the type at runtime ?
- Using a configuration file / environment variable ?
- Using the default ?
It seems that the default encryption scheme is md5 from this documentation
From what I gathered to stay inlined with the rest, it should be an additional argument, it entails to modify other files so I prefer to be sure of the proper method to follow.
Which method should be used to determine the type of encryption of the database file :
- An additional argument ?
- Determining the type at runtime ?
- Using a configuration file / environment variable ?
- Using the default ?
It seems that the default encryption scheme is
md5from this documentationFrom what I gathered to stay inlined with the rest, it should be an additional argument, it entails to modify other files so I prefer to be sure of the proper method to follow.
I don't think you have to do any check for which encryption is used. A htpasswd-file can contain multiple passwords with different encryption algorithms. Libpass (and htpasswd) determines which algorithm to use from a header string in the stored password hashes.
From your class, it should be enough to open the htpasswd-file with passlib.apache.HtpasswdFile() and then verify the check_password()-method. See passlib docs.
I think you need to install bcrypt extra here (libpass[bcrypt]), since I have removed built-in bcrypt backend because there's a well established implementation on pypi.
Alright, got side tracked a bit there! :smile:
Since the dependencies for your code are optional, I think it's fine to skip your tests for Python <= 3.8. You can look at an older test-requirements.txt for how to make libpass download depend on Python version.
Then you also need to skip your tests, e.g. calling self.skipTest("...") in your test class if libpass can't be imported.
Can you also look at squishing some of your commits to make the git history cleaner?
If you haven't done this before, lookup git's interactive rebase.
Can you also look at squishing some of your commits to make the git history cleaner?
If you haven't done this before, lookup git's interactive rebase.
Well it was a first indeed. I squashed all my commits into one but I don't know if it was what you wanted. I can split it back into something in between if needed.
Well it was a first indeed. I squashed all my commits into one, but I don't know if it was what you wanted. I can split it back into something in between if needed.
Thanks! In this case, I think squishing everything into one commit works well, since you are adding one new feature with a separate class.
When you get used to it, interactive rebases and force pushing the new and updated truth, is actually a quite nice way of working with PRs. :smile:
Since the dependencies for your code are optional, I think it's fine to skip your tests for Python <= 3.8. You can look at an older test-requirements.txt for how to make libpass download depend on Python version. Then you also need to skip your tests, e.g. calling self.skipTest("...") in your test class if libpass can't be imported.
Have you looked anything at this?