Skip to content

feat(config): add server.external_url for reverse proxy WebAuthn#785

Merged
penso merged 2 commits intomainfrom
stone-wood
Apr 18, 2026
Merged

feat(config): add server.external_url for reverse proxy WebAuthn#785
penso merged 2 commits intomainfrom
stone-wood

Conversation

@penso
Copy link
Copy Markdown
Collaborator

@penso penso commented Apr 18, 2026

Summary

  • Add server.external_url config field so users behind a reverse proxy can declaratively set their public URL for WebAuthn passkey auth
  • The MOLTIS_EXTERNAL_URL env var takes precedence over the config field
  • Derives WebAuthn RP ID (host) and origin automatically from the URL, falling through to existing env var chain if unset
  • Includes schema validation (http/https scheme required, trailing slash warning), config template documentation, and security docs update

Closes #782

Validation

Completed

  • cargo test -p moltis-config — 239 tests pass (13 new)
  • cargo check -p moltis-gateway — compiles clean
  • cargo clippy -p moltis-config -- -D warnings — no warnings
  • cargo clippy -p moltis-gateway -- -D warnings — no warnings
  • cargo +nightly-2025-11-30 fmt --all -- --check — no new formatting issues

Remaining

  • ./scripts/local-validate.sh — full CI validation
  • Manual: set external_url = "https://test.example.com" in moltis.toml, start gateway, verify log shows WebAuthn RP registered with the correct origin
  • Manual: set MOLTIS_EXTERNAL_URL=https://env.example.com, verify it takes priority over config value

Manual QA

  1. Start gateway with [server] external_url = "https://test.example.com" — verify startup log shows WebAuthn RP registered with RP ID test.example.com and origin https://test.example.com
  2. Set MOLTIS_EXTERNAL_URL=https://env.example.com env var alongside the config field — verify the env var wins (RP ID = env.example.com)
  3. Remove both config field and env var — verify localhost/hostname auto-detection still works as before
  4. Set external_url = "ftp://bad.example.com" — verify moltis config validate reports an error
  5. Set external_url = "https://example.com/" — verify validation warns about trailing slash

Users running moltis behind a reverse proxy with SSL termination
(e.g. https://moltis.example.com) had no discoverable way to tell
moltis its public URL. The only workaround was setting env vars
(MOLTIS_WEBAUTHN_RP_ID + MOLTIS_WEBAUTHN_ORIGIN), which wasn't
documented in the config file.

Add a first-class `server.external_url` config field that:
- Derives WebAuthn RP ID (host) and origin from the URL
- Supports MOLTIS_EXTERNAL_URL env var override (highest priority)
- Falls through to existing env var chain if unset
- Validates scheme (http/https only) and warns on trailing slashes
- Is excluded from the generic MOLTIS_ env override mechanism

Priority order for WebAuthn RP registration:
1. server.external_url / MOLTIS_EXTERNAL_URL (new)
2. MOLTIS_WEBAUTHN_RP_ID + MOLTIS_WEBAUTHN_ORIGIN (existing)
3. Cloud platform env vars: APP_DOMAIN, RENDER_*, FLY_*, RAILWAY_*
4. Localhost/hostname auto-detection

Closes #782

Entire-Checkpoint: 3ae551084ef1
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 18, 2026

Greptile Summary

This PR adds server.external_url to ServerConfig so users behind a reverse proxy can declaratively set their public URL for WebAuthn passkey registration. It derives RP ID and origin from the URL automatically, with MOLTIS_EXTERNAL_URL taking precedence over the config field. The second commit (7f4c46e8) correctly addresses the previously-noted silent passkey-loss bug when a URL parses successfully but has no hostname. Schema validation, config template documentation, schema-map registration, and tests are all included.

Confidence Score: 5/5

Safe to merge; the only finding is a documentation wording issue that does not affect runtime behaviour.

The empty-host bug from the previous review thread is correctly fixed. All validation, schema-map, template, and test changes are sound. The single remaining finding is a P2 doc clarification in security.md — the fine-grained env-var precedence paragraph describes a use-case that the code does not support, but no runtime breakage results from the wording.

docs/src/security.md — the 'take precedence after' phrasing should be corrected to make clear that the per-field WebAuthn env vars are fallbacks used only when external_url is unset.

Important Files Changed

Filename Overview
crates/config/src/schema/system.rs Adds external_url: Option<String> to ServerConfig, effective_external_url() (reads env > config), and pure resolve_external_url() helper for testability — all well-structured
crates/gateway/src/server/prepare_core/post_state.rs Integrates effective_external_url() into build_webauthn_registry; the empty-host guard added in the fix commit correctly prevents silent passkey loss for hostless URLs
crates/config/src/validate/semantic.rs Adds scheme validation (Error for non-http/https) and trailing-slash warning for server.external_url; consistent with existing validation patterns
docs/src/security.md Adds reverse-proxy passkey section, but the precedence description for fine-grained env vars is backwards relative to the code
crates/config/src/schema/tests.rs Adds 9 tests for external_url field parsing, effective_external_url, and the pure resolve_external_url helper covering precedence, fallback, trailing-slash stripping, and both-unset
crates/config/src/validate/schema_map.rs Correctly adds external_url as a Leaf entry to the server struct in build_schema_map
crates/config/src/template.rs Adds commented-out external_url example with env-var precedence note to the default config template
crates/config/src/validate/tests/security.rs Adds four tests covering bad scheme (error), trailing slash (warning), valid https, and valid http — good coverage of the new validation paths
crates/config/src/loader/tests.rs Adds apply_env_overrides_ignores_external_url test confirming generic env override skips MOLTIS_EXTERNAL_URL, leaving it to effective_external_url()

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build_webauthn_registry] --> B{effective_external_url\nreturns Some?}
    B -- No --> C{Fine-grained env vars\nMOLTIS_WEBAUTHN_RP_ID\nAPP_DOMAIN etc.}
    B -- Yes --> D{url::Url::parse OK?}
    D -- No --> E[warn! invalid URL\nreturn None,None]
    E --> C
    D -- Yes --> F{host_str empty?}
    F -- Yes --> G[warn! no hostname\nreturn None,None]
    G --> C
    F -- No --> H[external_rp_id = host\nexternal_origin = url]
    H --> I[explicit_rp_id = external_rp_id\nFine-grained env vars skipped]
    C --> J{Any RP ID found?}
    I --> J
    J -- No --> K[Localhost / hostname fallback]
    J -- Yes --> L[try_add rp_id + origin\nWebAuthn RP registered]
    K --> L
    L --> M{any_ok?}
    M -- Yes --> N[return SharedWebAuthnRegistry]
    M -- No --> O[return None\npasskeys disabled]
Loading

Reviews (2): Last reviewed commit: "fix(config): guard empty-host URL and ad..." | Re-trigger Greptile

Comment thread crates/gateway/src/server/prepare_core/post_state.rs Outdated
Comment thread crates/config/src/schema/system.rs
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

❌ Patch coverage is 55.00000% with 27 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ates/gateway/src/server/prepare_core/post_state.rs 0.00% 27 Missing ⚠️

📢 Thoughts on this report? Let us know!

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented Apr 18, 2026

Merging this PR will not alter performance

✅ 39 untouched benchmarks
⏩ 5 skipped benchmarks1


Comparing stone-wood (7f4c46e) with main (9ab3a05)

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Address PR review feedback:

- Guard against URLs that parse but have no host (e.g. file:///path).
  Without this, unwrap_or_default() produced "", which propagated as
  Some("") and silently disabled passkey auth by skipping the localhost
  fallback. Now emits a warning and returns (None, None) to fall through.

- Extract resolve_external_url() as a pure function testable without
  env var mutation. Add 5 unit tests covering: env precedence over
  config, empty env fallback, trailing slash stripping from both
  sources, and both-unset returns None.

Entire-Checkpoint: e69b85acb7e1
@penso
Copy link
Copy Markdown
Collaborator Author

penso commented Apr 18, 2026

Review feedback addressed in 7f4c46e

P1 — Silent passkey loss when URL has no host: Added empty-host guard. URLs like file:///path that parse but have no hostname now emit a warning and fall through to the existing env var / localhost detection chain instead of silently disabling passkey auth.

P2 — Missing unit tests for effective_external_url: Extracted resolve_external_url(env_val, config_val) as a pure function testable without set_var (which is unsafe and denied by workspace lints). Added 5 unit tests covering all three behaviors: env precedence over config, empty-env fallback to config, and trailing slash stripping from both sources.

@penso
Copy link
Copy Markdown
Collaborator Author

penso commented Apr 18, 2026

@greptile review

@penso penso merged commit 2efe1a3 into main Apr 18, 2026
24 of 34 checks passed
@penso penso deleted the stone-wood branch April 18, 2026 12:50
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.

1 participant