feat: add foundation for LDAP in core repository #2923
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Uffizzi Preview |
There was a problem hiding this comment.
I'm a little confused as to what should happen if some of the settings are left unset. Presumably the LDAP functionality would just fail somewhere, no? If it would, would it be better to ensure that the settings are present if the top LDAP configuration is set?
Since you've probably got a good answer to this question I'm approving the PR since I didn't spot any code errors. Just curious as to the approach.
| if LDAP_AUTH_URL: | ||
| AUTHENTICATION_BACKENDS.insert(0, "django_python3_ldap.auth.LDAPBackend") |
There was a problem hiding this comment.
Should this throw an exception if a URL is not set?
There was a problem hiding this comment.
I think it's because we will include LDAP in every private cloud image... but it's up to the client to enable it (by setting LDAP_AUTH_URL). Is that correct @matthewelwell
There was a problem hiding this comment.
Yes, that's correct.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2923 +/- ##
==========================================
- Coverage 95.63% 95.58% -0.05%
==========================================
Files 1011 1013 +2
Lines 29006 29206 +200
==========================================
+ Hits 27741 27918 +177
- Misses 1265 1288 +23
☔ View full report in Codecov by Sentry. |
Changes
Adds the foundation for LDAP functionality in the core repository.
Ideally, the
ldap_dnattribute would be part of a separate model contained in the LDAP repository but, to reduce friction in moving from the EE image to the private-cloud image in the future, I've added the attribute directly to the existing model as in the EE codebase.How did you test this code?
This PR just adds the settings and model attribute, no logic to be tested.