Skip to content

Implement a Django authentication backend for NAV-style LDAP login flows#3624

Merged
johannaengland merged 6 commits intomasterfrom
feature/nav-django-ldap-backend
Dec 8, 2025
Merged

Implement a Django authentication backend for NAV-style LDAP login flows#3624
johannaengland merged 6 commits intomasterfrom
feature/nav-django-ldap-backend

Conversation

@lunkwill42
Copy link
Copy Markdown
Member

@lunkwill42 lunkwill42 commented Oct 22, 2025 •
edited by johannaengland
Loading

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.

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Added/changed documentation
  • Linted/formatted the code with ruff, easiest by using pre-commit
  • Wrote the commit message so that the first line continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See https://cbea.ms/git-commit/
  • Based this pull request on the correct upstream branch: For a patch/bugfix affecting the latest stable version, it should be based on that version's branch (<major>.<minor>.x). For a new feature or other additions, it should be based on master.
  • If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done
  • If 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
  • If this adds a new Python source code file: Added the boilerplate header to that file

@lunkwill42 lunkwill42 added nonews No news fragment is necessary for this PR (e.g. refactoring, cleanups, workflow/development changes) ldap authentication labels Oct 22, 2025
@hmpf hmpf force-pushed the towards-django-auth branch from 12fddee to 720763d Compare October 22, 2025 14:18
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 22, 2025 •
edited
Loading

Test results

    27 files      27 suites   45m 59s ⏱️
 2 718 tests  2 718 ✅ 0 💤 0 ❌
20 134 runs  20 134 ✅ 0 💤 0 ❌

Results for commit fdff073.

♻️ This comment has been updated with latest results.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 22, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 98.36066% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.00%. Comparing base (830b9a4) to head (fdff073).
⚠️ Report is 380 commits behind head on master.

Files with missing lines Patch % Lines
python/nav/web/auth/__init__.py 75.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3624      +/-   ##
==========================================
+ Coverage   62.95%   63.00%   +0.04%     
==========================================
  Files         611      612       +1     
  Lines       45163    45213      +50     
  Branches       43       43              
==========================================
+ Hits        28431    28485      +54     
+ Misses      16722    16718       -4     
  Partials       10       10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lunkwill42 lunkwill42 force-pushed the feature/nav-django-ldap-backend branch from 129f823 to c45f372 Compare October 22, 2025 14:30
johannaengland
johannaengland reviewed Oct 22, 2025
View reviewed changes
@hmpf hmpf force-pushed the towards-django-auth branch from 720763d to 55c9d73 Compare October 23, 2025 07:51
@lunkwill42 lunkwill42 force-pushed the feature/nav-django-ldap-backend branch from 79e2568 to 1946a3c Compare October 23, 2025 08:34
Base automatically changed from towards-django-auth to master October 23, 2025 08:38
@lunkwill42 lunkwill42 removed the nonews No news fragment is necessary for this PR (e.g. refactoring, cleanups, workflow/development changes) label Oct 23, 2025
@lunkwill42 lunkwill42 force-pushed the feature/nav-django-ldap-backend branch 2 times, most recently from 78c2967 to 27ba495 Compare October 23, 2025 08:47
@lunkwill42 lunkwill42 mentioned this pull request Oct 23, 2025
Merged
10 tasks
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Oct 23, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@johannaengland johannaengland requested review from a team and hmpf October 23, 2025 14:57
@johannaengland johannaengland marked this pull request as ready for review October 23, 2025 15:22
@hmpf hmpf requested a review from johannaengland October 30, 2025 13:19
hmpf
hmpf approved these changes Oct 30, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this can safely be merged to master.

Then poc/allauth needs to move to the new master, and then rebase the dependents. (poc/django-auth-2, poc/allauth-dataporten, feature/enable-django-auth.. I think that's all of them.)

lunkwill42
lunkwill42 commented Oct 31, 2025
View reviewed changes
@johannaengland johannaengland requested review from a team and removed request for johannaengland December 1, 2025 13:58
lunkwill42
lunkwill42 commented Dec 8, 2025
View reviewed changes
Copy link
Copy Markdown
Member Author

@lunkwill42 lunkwill42 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, my only complaint was fixed, so I guess this is good to go :)

Still can't approve my self-initiated PR, so I'm just moving it in our board (just don't forget to squash the fixups)

@lunkwill42 @johannaengland
Move _handle_ldap_admin_status() to new module
14ba4d2 
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 @johannaengland
Implement a NAV-specific LdapBackend
3bdbe1a 
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.
@lunkwill42 @johannaengland
Move/rename _handle_ldap_admin_status
f4cf2a7 
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.
lunkwill42 and others added 3 commits December 8, 2025 14:07
@lunkwill42 @johannaengland
Add libldap-common to devcontainer image
a503b3e 
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 @johannaengland
Add news fragment
c5abe81
@johannaengland
Add tests for LDAP auth backend
fdff073
@johannaengland johannaengland force-pushed the feature/nav-django-ldap-backend branch from cca7923 to fdff073 Compare December 8, 2025 13:07
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Dec 8, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@johannaengland johannaengland merged commit 5db2915 into master Dec 8, 2025
18 checks passed
@johannaengland johannaengland deleted the feature/nav-django-ldap-backend branch December 8, 2025 13:39
johannaengland added a commit that referenced this pull request Dec 11, 2025
@johannaengland
Remove tests for authenticate
7613288 
These are covered by the new ldap tests from #3624
johannaengland added a commit that referenced this pull request Dec 11, 2025
@johannaengland
Remove tests for authenticate
d992937 
These are covered by the new ldap tests from #3624
johannaengland added a commit that referenced this pull request Dec 19, 2025
@johannaengland
Remove tests for authenticate
45f4c50 
These are covered by the new ldap tests from #3624
@hmpf hmpf mentioned this pull request Feb 11, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johannaengland johannaengland johannaengland left review comments

@hmpf hmpf hmpf approved these changes

Assignees

@lunkwill42 lunkwill42

@johannaengland johannaengland

Labels

authentication ldap

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make a Django auth backend for logging into NAV with LDAP, NAV-style

3 participants

@lunkwill42 @hmpf @johannaengland