feat: automatic OIDC protocol detection with proxy header validation #180
Conversation
Replaces manual oidc_redirect_https setting with automatic detection via Uvicorn's --proxy-headers flag. OIDC now works for both local (http) and external (https) access simultaneously. Changes: - Add --proxy-headers and --forwarded-allow-ips flags to Dockerfile - Remove oidc_redirect_https setting from UI and backend - Add database migration to clean up deprecated setting - Add proxy header validation to auth routes Security: - Validates proxy headers come from trusted IPs - Returns 403 with error ID if headers from untrusted source - Logs details at INFO level for troubleshooting - Detailed debug info available at DEBUG log level
markbeep
left a comment
markbeep
left a comment
There was a problem hiding this comment.
Good idea. Haven't thought about the use-case of requiring both http and https redirects interchangeably. As this is strongly dependent on your proxy setup, I believe there still should be an option to force the redirect to use http or https. It's possible for a proxy to not correctly relay the scheme used. While this can sometimes be chalked up to incorrect configuration, I find it doesn't hurt to have an option to aid with whatever setup you have.
I suggest instead of completely removing the oidc_redirect_https option, to instead rename it into a force_oidc_redirect_https which is an optional boolean. If you toggle it off, it will try to determine the scheme automatically like you have done, but if enabled it will use the set option. Then create a database migration that simply moves the old value to this new key, to make any transition seamless. Any new setups will then default to using the dynamic approach, while old setups will continue working the exact same as before.
At the end, also add any new env variable to the env variables section in the readme and in the docs (docs/content/docs/concepts/environment-variables.md)
| # ===== SECURITY WARNING: Proxy Headers with Permissive Config ===== | ||
| # Warn if proxy headers are detected but FORWARDED_ALLOW_IPS allows all IPs | ||
| if request.headers.get("X-Forwarded-Proto"): | ||
| forwarded_allow_ips = os.getenv("FORWARDED_ALLOW_IPS", "0.0.0.0/0") |
There was a problem hiding this comment.
Don't use os.getenv. Add env variables to app/internal/env_settings.py. They get properly type checked that way and are all in one place.
| "FORWARDED_ALLOW_IPS is set to allow all IPs (0.0.0.0/0). " | ||
| "For production use, set FORWARDED_ALLOW_IPS to your reverse proxy IP(s) only. " | ||
| "This prevents header spoofing attacks. " | ||
| "See documentation: https://github.com/markbeep/AudioBookRequest/docs/oidc" |
There was a problem hiding this comment.
This warning is a bit odd. For one, https://github.com/markbeep/AudioBookRequest/docs/oidc doesn't exist.
But if any validation is required for the forwarded_allow_ips, you can add it to app/internal/env_settings.py by for example adding a check at the bottom under the class definitions that does something like the following:
class Settings(BaseSettings): ...
# validate forwarded_allow_ips
if Settings().app.forwaded_allow_ips in ("0.0.0.0/0", "*"):
raise ValueError(...)Then the env error/warning is shown on startup.
|
|
||
| # DEBUG: Show the redirect URI being sent to token endpoint | ||
| logger.debug( | ||
| "OIDC_CALLBACK_REDIRECT_URI_DEBUG", |
There was a problem hiding this comment.
No need to capitalize, combine strings with underscore, nor add "DEBUG" at the end.
Instead, indicate what is going on or has happened in the message. For example, "Validating code on token endpoint" or similar.
Same goes for the above log messages.
| # docker-compose.yml | ||
| services: | ||
| abr: | ||
| image: ghcr.io/markbeep/audiobookrequest:latest |
There was a problem hiding this comment.
- Audiobookrequest doesn't exist on ghcr.io
- Don't show
latestin the docs. Instead put a major version like:1
| - ✅ Works out-of-the-box for all reverse proxies | ||
| - ✅ Great for home labs and self-hosted setups | ||
| - ⚠️ Less secure on shared infrastructure (susceptible to header spoofing) | ||
|
|
||
| If you see this warning in your logs: | ||
| ``` | ||
| 🔐 SECURITY WARNING: Proxy headers detected (X-Forwarded-Proto) but | ||
| FORWARDED_ALLOW_IPS is set to allow all IPs (0.0.0.0/0). |
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash(fastapi --help:*)", | ||
| "Bash(uv run fastapi run --help:*)" | ||
| ] | ||
| } | ||
| } |
| ENV ABR_APP__VERSION=$VERSION | ||
|
|
||
| CMD /app/.venv/bin/alembic upgrade heads && /app/.venv/bin/fastapi run --port $ABR_APP__PORT | ||
| CMD ["/bin/sh", "-c", "/app/.venv/bin/alembic upgrade heads && /app/.venv/bin/fastapi run --port $ABR_APP__PORT --proxy-headers --forwarded-allow-ips=\"${FORWARDED_ALLOW_IPS:-0.0.0.0/0}\""] |
There was a problem hiding this comment.
After adding the env var to the env_settings.py file, you'll have to adjust it here as well.
2 participants
Summary
Replaces manual
oidc_redirect_httpssetting with automatic HTTP/HTTPS protocol detection using Uvicorn's--proxy-headersflag. OIDC now worksseamlessly for both local (http) and external (https) access simultaneously.
Problem
Users accessing AudioBookRequest through multiple methods (local HTTP via VPN, external HTTPS via reverse proxy) couldn't use OIDC because the manual setting forced a single protocol for all access paths.
Solution
Leverages Uvicorn's built-in
--proxy-headersflag to automatically detect protocol from incoming requests:Changes
--proxy-headersflag to Dockerfile with configurableFORWARDED_ALLOW_IPSoidc_redirect_httpssetting from UI, API, and databaseConfiguration
Default: Works out-of-the-box, trusts all proxy IPs (0.0.0.0/0)
Production (Optional): Set
FORWARDED_ALLOW_IPSenv var to trust specific proxy IPs onlyTesting
oidc_redirect_httpssetting in UI ✓Notes
FORWARDED_ALLOW_IPSfor additional securityDisclaimer: Encountered this issue on my personal setup - used Claude Code to speed up implementation but thoroughly reviewed and tested all changes.
Hope this helps others facing the same OIDC + reverse proxy challenge!