Skip to content

Closes #23335: Adopt Django's LoginRequiredMiddleware - #23663

Merged
jnovinger merged 1 commit into
featurefrom
23335-adopt-login-required-middleware
Nov 10, 2026
Merged

jnovinger merged 1 commit into
featurefrom
23335-adopt-login-required-middleware

Conversation

@jeremystretch

@jeremystretch jeremystretch commented Nov 10, 2026 •

Copy link
Copy Markdown
Member

Closes: #23335

Adds netbox.middleware.LoginRequiredMiddleware, a subclass of Django's LoginRequiredMiddleware. When LOGIN_REQUIRED is true, it requires authentication for every UI view unless the view is marked @login_not_required. When LOGIN_REQUIRED is false, it does nothing. DRF views (the REST API, including plugin APIs) are exempt, since DRF authenticates requests itself. So is the Prometheus metrics endpoint, so that existing scrapers keep working.

  • Exempted LoginView, LogoutView, SocialAuthBeginView, StaticMediaFailureView, MediaView and NetBoxGraphQLView. The last two keep their own token auth and LOGIN_REQUIRED checks.
  • Removed ConditionalLoginRequiredMixin from HomeView, SearchView, MACAddressSetPrimaryView and the generic feature views. Also removed the dead inline check in HomeView.get(). ContentTypePermissionRequiredMixin and ObjectPermissionRequiredMixin now inherit directly from AccessMixin.
  • ConditionalLoginRequiredMixin and TokenConditionalLoginRequiredMixin are unchanged, but marked deprecated for removal in v5.0.
  • Added an "Authentication" section to the plugin views docs, covering login_not_required (and csrf_exempt for views that accept POST requests from external clients).

Departures from the proposal in the issue:

  • No social/djdt namespace exemptions. The pinned social-auth-app-django (6.1.0) and django-debug-toolbar (8.0.0) already decorate their public views with @login_not_required. social:disconnect keeps its own @login_required.
  • DRF views are exempted by view class, not by the /api/ path. With the login check gone from the permission mixins, a path-based exemption would leave a non-DRF view mounted under /api/ (e.g. a plugin ObjectView in api_urls) open to anonymous users whenever EXEMPT_VIEW_PERMISSIONS covers it. The middleware checks the cls attribute that DRF's as_view() sets instead. All 665 current /api/ routes are DRF views, including the cache_page-wrapped schema view.

Behavior change: the only core view that becomes newly login-protected is GraphiQLWorkerView, and only when LOGIN_REQUIRED is true. The GraphiQL page that loads those workers already requires login, and a headless Chrome check confirmed the workers start for a logged-in session. Plugin views built on a plain View now require login when LOGIN_REQUIRED is true. Plugins that serve intentionally public UI views will need login_not_required, so this needs a callout in the v4.8 release notes (not included here, per convention).

Testing:

  • New LoginRequiredMiddlewareTestCase (netbox/tests/test_middleware.py). It checks that anonymous UI requests, including a plain plugin view, redirect to login; that authenticated and LOGIN_REQUIRED=False requests are allowed; that each exempt endpoint answers for itself rather than redirecting; and that a non-DRF view under /api/ still requires login while the metrics URL name is exempt.
  • New MediaViewTestCase.test_media_token_authentication: token-authenticated media requests still reach the view with LOGIN_REQUIRED=True.
  • Full test suite and ruff check pass.
  • Checked by hand against a dev server with LOGIN_REQUIRED on and off, using curl for anonymous, session and token requests, including /metrics with metrics enabled.

Enforce LOGIN_REQUIRED for all UI views via a subclass of Django's
LoginRequiredMiddleware, rather than relying on each view to inherit a
login mixin. DRF views (which authenticate requests themselves) and the
Prometheus metrics endpoint are exempt, and views which handle
authentication themselves are marked with @login_not_required.
ConditionalLoginRequiredMixin and TokenConditionalLoginRequiredMixin are
deprecated for removal in v5.0.
@jeremystretch
jeremystretch force-pushed the 23335-adopt-login-required-middleware branch from c7c805b to 4a28cbd Compare November 10, 2026 20:31
@jeremystretch
jeremystretch marked this pull request as ready for review November 10, 2026 20:33
@jeremystretch
jeremystretch requested review from a team and jnovinger and removed request for a team November 10, 2026 20:39
@jnovinger
jnovinger merged commit e243c71 into feature Nov 10, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants