LDAP authentication, API key and some CLI improvements
The LDAP authentication works by:
Look up the user in the LDAP server If the user exists try to authenticate (bind) If it succeeds and the user doesn't exist in the database a new one is created (with ldap=1) If the user does not exist try the supysonic database (with ldap=0)
Other things considered:
It requires the ldap3 module but if it's not installed it just throws a warning in the log. To disable LDAP authentication just comment the ldap section in the config. The mysql fields "password" and "salt" are now nullable because they are null if the user is from LDAP. A new mysql field was added "ldap" with default 0. It adds an icon to the user profile if they are a LDAP user. Some features like changing email and password are disabled because it doesn't make sense to change them manually. Changing username is allowed.
Regarding the API key, I think its safer to use a key for API authentication since it is just sent encoded and not encrypted, so since I was modifying the code for LDAP authentication I also joined these changes because it would then be too confusing.
The API key authentication works by:
If require_api_key is enabled: try to login with the API key If require_api_key is disabled: try API key first then password if it fails
It adds a form to the user profile to generate a new key or delete the existing one. It adds new option group to the cli: "user api key" with commands: show, new and delete A new mysql field was added "api_key" with default 0.
I also adapted the cli to the LDAP authentication and ended up making some improvements:
Added a new option group "user edit" Moved "changepass" to "user edit password" Moved "rename" to "user edit username" Added a new option "user edit email"
Codecov Report
Merging #250 (b2f0c99) into master (8e2adf8) will decrease coverage by
24.38%. The diff coverage is49.37%.
@@ Coverage Diff @@
## master #250 +/- ##
===========================================
- Coverage 86.41% 62.04% -24.38%
===========================================
Files 46 47 +1
Lines 3776 3915 +139
===========================================
- Hits 3263 2429 -834
- Misses 513 1486 +973
| Impacted Files | Coverage Δ | |
|---|---|---|
| supysonic/cli.py | 71.13% <38.33%> (-19.68%) |
:arrow_down: |
| supysonic/frontend/user.py | 93.30% <50.00%> (-6.70%) |
:arrow_down: |
| supysonic/ldap.py | 50.00% <50.00%> (ø) |
|
| supysonic/managers/user.py | 82.50% <56.00%> (-17.50%) |
:arrow_down: |
| supysonic/api/__init__.py | 57.94% <75.00%> (-39.09%) |
:arrow_down: |
| supysonic/config.py | 95.83% <100.00%> (+0.08%) |
:arrow_up: |
| supysonic/db.py | 86.36% <100.00%> (-6.28%) |
:arrow_down: |
| supysonic/api/albums_songs.py | 14.78% <0.00%> (-84.51%) |
:arrow_down: |
| supysonic/api/search.py | 15.58% <0.00%> (-84.42%) |
:arrow_down: |
| supysonic/api/annotation.py | 16.37% <0.00%> (-83.63%) |
:arrow_down: |
| ... and 22 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Hello again.
What do you mean by API key? Please add description to your PR so I know what I'm reviewing. Actually if you're adding several new authentication methods, could you please split this PR (a PR per auth method)? And give a proper title to your commits please, once merged we'll only have an "Apply changes" in the history and that makes it hard to browse later (especially when building changelogs for releases).
I haven't reviewed the code yet, but just looking at the list of changed files I can tell than this PR is missing:
- migrations
- documentation
- and some tests would be nice too, especially if we're talking about authentication
Hi, thanks for the quick response. Also thanks for your software I tried many servers but yours have been working great for some years now. Indeed I should have added more details, I edited the original post. The commit history is just "Apply changes" because I merged a patch from a local repository (sorry I'm a bit noob to git lol). I have been using these changes (and the others from the other branches) for almost an year and it has been fine. Regarding the missing stuff I can have a look at what's missing from the documentation but if you could help me with the migrations/tests or add those later on it would be nice. Cheers