fix(auth): disable ApisixUserMiddleware by default in local dev/codespaces by shaidar · Pull Request #4002 · mitodl/mit-learn

shaidar · 2026-09-28T19:05:08Z

What are the relevant tickets?

Fixes https://github.com/mitodl/hq/issues/13471

Description (What does it do?)

ApisixUserMiddleware decodes and trusts the X-Userinfo header (base64-encoded JSON identity claims) unconditionally, with nothing tying it to a real Keycloak login through APISIX -- it just assumes the header could only be there because APISIX put it there.

The default local dev profile (COMPOSE_PROFILES=backend,frontend) never runs APISIX/Keycloak, and Codespaces doesn't either -- while also exposing forwardable, network-reachable ports. In both cases nothing verifies that header before Django trusts it: a request straight to Django with a forged X-Userinfo header logs in as (and creates, if the identity doesn't exist) whoever the header claims.

DISABLE_APISIX_USER_MIDDLEWARE is already the purpose-built lever for exactly this, already documented in README-keycloak.md as the supported way to run without Keycloak/APISIX. This sets it to True by default.

How can this be tested?

Verified live, end to end, using the issue's own repro steps:

Confirmed vulnerable on a clean checkout of main: docker compose up (no APISIX/Keycloak profile), then a forged X-Userinfo header against /api/v0/users/me/ returned "is_authenticated":true,"username":"totally_fake_user" -- a brand-new account, no login required.
Confirmed fixed on this branch: the identical request now returns "is_authenticated":false" -- fully rejected, no account created.

Also:

Confirmed via code trace (Django's RemoteUserMiddleware base class + AUTHENTICATION_BACKENDS, which has no remote_user-capable backend) that disabling this middleware is a safe no-op for real Django session-based logins -- doesn't affect ordinary email/password auth at all.
env/codespaces.env needed its own copy of the setting, since it doesn't inherit from env/backend.env at all (extends replaces a service's env_file list rather than merging it).
Confirmed QA/Production are unaffected: neither sets this anywhere in ol-infrastructure, and APISIX is genuinely the only path into Django there (per the issue's own live verification).
Ran main/middleware/apisix_user_test.py and website_content/admin_test.py (42 tests) -- both set this setting explicitly per-test rather than relying on the env default, so unaffected either way, confirmed by actually running them.
Updated README-keycloak.md's opt-into-Keycloak instructions to add the now-necessary DISABLE_APISIX_USER_MIDDLEWARE=False override, and corrected its now-inaccurate "Keycloak is enabled by default" claim.
pre-commit (detect-secrets, trailing-whitespace, end-of-file-fixer) passes on all changed files.

Additional Context

None.

🤖 Generated with Claude Code

…paces ApisixUserMiddleware trusts the X-Userinfo header (base64-encoded JSON identity claims) unconditionally, with no signature or secret tying it to a real Keycloak login through APISIX -- it just assumes the header could only be there because APISIX put it there. The default local dev profile (COMPOSE_PROFILES=backend,frontend) never runs APISIX/Keycloak, and Codespaces doesn't either while also exposing forwardable, network-reachable ports -- so in both cases nothing verifies that header before Django trusts it. A request straight to Django with a forged X-Userinfo header logs in (and creates, if the identity doesn't exist) as whoever the header claims. Verified live, end to end: reproduced the exact repro from the linked issue against a clean checkout (forged header -> is_authenticated: true, brand-new account) with docker compose up (no APISIX/Keycloak profile), then confirmed the same request is fully rejected (is_authenticated: false, no account created) once DISABLE_APISIX_USER_MIDDLEWARE=True is set. DISABLE_APISIX_USER_MIDDLEWARE was already the purpose-built lever for this -- traced through Django's RemoteUserMiddleware base class and confirmed AUTHENTICATION_BACKENDS has no remote_user-capable backend, so this is a safe no-op for real session-based Django logins. QA/ Production are unaffected (they don't set this anywhere; APISIX is the only path in there, confirmed separately in the issue). Existing middleware/admin tests set this explicitly per-test rather than relying on the env default, and aren't affected either (42 passed). Set the default in both env/backend.env and env/codespaces.env, since codespaces.env doesn't inherit from backend.env (extends replaces its env_file list rather than merging). Updated README-keycloak.md's opt-in-to-Keycloak instructions to add the now- necessary DISABLE_APISIX_USER_MIDDLEWARE=False override, and corrected its now-inaccurate "Keycloak is enabled by default" claim. Fixes mitodl/hq#13471 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

github-actions · 2026-09-28T19:05:34Z

OpenAPI Changes

No changes detected

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

Copilot

Copilot review overview

🟡 Changes recommended

The new backend default causes existing middleware tests to run with APISIX handling disabled and fail in a fresh Docker Compose environment.

Review effort: Balanced
Findings: 1

Open (1)

Default APISIX disablement breaks middleware tests · New

What changed in this PR

This PR secures local and Codespaces authentication by disabling APISIX header-based authentication unless explicitly enabled.

Changes:

Disables ApisixUserMiddleware in default local and Codespaces environments.
Documents how to enable APISIX/Keycloak authentication explicitly.

File	Description
`README-keycloak.md`	Documents secure defaults and opt-in configuration.
`env/codespaces.env`	Disables APISIX authentication in Codespaces.
`env/backend.env`	Disables APISIX authentication in local development.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…abled-path case Copilot review feedback on this PR: env/backend.env's new DISABLE_APISIX_USER_MIDDLEWARE=True default is injected into the web container, so the documented `docker compose run --rm web ... pytest main/middleware/apisix_user_test.py` path now runs those tests with the middleware disabled. Confirmed live: on a fresh setup (no personal backend.local.env override), that command now fails 12 tests and errors 6 more with unrelated-looking TypeErrors -- process_request() takes the disabled-path branch (Django's base RemoteUserMiddleware), which touches real request attributes the test mocks don't provide. The existing userinfo_flag_defaults autouse fixture already exists specifically to pin settings regardless of backend.local.env drift (its own docstring says so, for the userinfo create/update flags) -- extended it to also force DISABLE_APISIX_USER_MIDDLEWARE = False, so every test in this file keeps exercising the enabled/trusting code path regardless of the new env default. Added test_disabled_middleware_ignores_forged_header as the requested regression case for the actual disabled path: sets the flag to True and asserts a forged X-Userinfo header is fully ignored through a real request (client.get, not a bare Mock, since the disabled path needs real request attributes) -- the same property already verified live against a real docker compose setup in this PR's earlier commit, now covered at the test level too. Verified via the exact cited repro: `docker compose run --rm web pytest main/middleware/apisix_user_test.py` now passes 19/19 (was 12 failed, 6 errored). Also re-ran via the normal host pytest path (43 passed, unaffected either way). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

shaidar requested a review from a team as a code owner September 28, 2026 19:05

Copilot AI balanced review requested due to automatic review settings September 28, 2026 19:05

Copilot started reviewing on behalf of shaidar September 28, 2026 19:05 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

Comment thread env/backend.env

shaidar added the Needs Review An open Pull Request that is ready for review label Sep 28, 2026

alexfigtree self-requested a review September 29, 2026 04:03

Navigation Menu

fix(auth): disable ApisixUserMiddleware by default in local dev/codespaces - #4002

fix(auth): disable ApisixUserMiddleware by default in local dev/codespaces#4002
shaidar wants to merge 2 commits into
mainfrom
apisix-header-trust-default

shaidar commented Sep 28, 2026

Uh oh!

github-actions Bot commented Sep 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

shaidar commented Sep 28, 2026

What are the relevant tickets?

Description (What does it do?)

How can this be tested?

Additional Context

Uh oh!

github-actions Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

OpenAPI Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Sep 28, 2026 •

edited

Loading