Implement a Django authentication backend for NAV-style LDAP login flows #3624
+315
−18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lunkwill42
added
nonews
hmpf
force-pushed
the
towards-django-auth
branch
from
12fddee to
720763d
Compare
Test results 27 files 27 suites 45m 43s ⏱️ Results for commit 8619aef. ♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3624 +/- ##
==========================================
+ Coverage 62.49% 62.54% +0.04%
==========================================
Files 611 612 +1
Lines 45103 45153 +50
Branches 43 43
==========================================
+ Hits 28188 28241 +53
+ Misses 16905 16902 -3
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lunkwill42
force-pushed
the
feature/nav-django-ldap-backend
branch
from
129f823 to
c45f372
Compare
hmpf
force-pushed
the
towards-django-auth
branch
from
720763d to
55c9d73
Compare
lunkwill42
force-pushed
the
feature/nav-django-ldap-backend
branch
from
79e2568 to
1946a3c
Compare
lunkwill42
removed
the
nonews
Preparing a new LDAP authentication backend module for integrating NAV's LDAP authentication with Django. When finished, there should be no LDAP-specific stuff in the generic auth module.
lunkwill42
force-pushed
the
feature/nav-django-ldap-backend
branch
from
a0b4085 to
78c2967
Compare
This `LdapBackend` can replace the existing legacy LDAP login flow in NAV, by re-using the `nav.web.auth.ldap` module, and without unnecessarily leaking LDAP abstractions and errors to the rest of the login system.
Grokking what this function did from its name wasn't all too easy. Also, given all the other methods of the LdapBackend class, it fit better as an extra method of that class. Additionally, added a more explicit docstring.
This ensures basic OpenLDAP library configuration is installed in the devcontainer. These configuration files are necessary in order to point the OpenLDAP library to the correct set of CA certificates to consider when verifying LDAP server connections. Without this, setting up NAV to authenticate against an SSL-protected LDAP server will just fail.
lunkwill42
force-pushed
the
feature/nav-django-ldap-backend
branch
from
78c2967 to
27ba495
Compare
10 tasks
|
hmpf
approved these changes
Oct 30, 2025
lunkwill42
commented
Oct 31, 2025
| def test_given_invalid_ldap_user_info_raise_permission_denied_exception( | ||
| self, mock_authenticate, db, ldap_synced_account | ||
| ): | ||
| mock_authenticate.side_effect = PermissionDenied |
There was a problem hiding this comment.
This makes no sense. The low-level LDAP authenticate() function would never raise a Django specific exception.
There was a problem hiding this comment.
(and I can't request changes when reviewing, since I originated this PR 😆 )
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Scope and purpose
Fixes #3498.
This adds a new Django authentication backend implementation that reframes the legacy NAV LDAP authentication into Django's way of doing authentication.
The back-end is only implemented, not activated. Se subsequent PRs for this.
Contributor Checklist
Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to NAV can be found in the
Hacker's guide to NAV.
<major>.<minor>.x). For a new feature or other additions, it should be based onmaster.If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be doneIf it's not obvious from a linked issue, described how to interact with NAV in order for a reviewer to observe the effects of this change first-hand (commands, URLs, UI interactions)If this results in changes in the UI: Added screenshots of the before and after