feat: Allow multiple options in request_ip_source #1985

Open
Omar007 wants to merge 2 commits from Omar007/continuwuity:feature/ip-src-fallback into main
Contributor

Only allowing a single option results in requests being denied if the singular selected source option isn't used for every single request flow.
This means you need to choose between selecting the right IP or properly separating your infrastructure traffic. You can't have both; running edge traffic through a reverse proxy with proper IP resolution, as well as appservices isolated internally is not possible at the same time.

By allowing multiple options to be set, this limitation is resolved and it becomes possible to have client IP resolution behind reverse proxies, as well as allow for direct internal connections for appservices.

Options are evaluated in order. If all fail, the request still fails.

Pull request checklist:

  • This pull request targets the main branch, and the branch is named something other than
    main.
  • I have written an appropriate pull request title and my description is clear.
  • I understand I am responsible for the contents of this pull request.
  • I have followed the contributing guidelines:
Only allowing a single option results in requests being denied if the singular selected source option isn't used for every single request flow. This means you need to choose between selecting the right IP or properly separating your infrastructure traffic. You can't have both; running edge traffic through a reverse proxy with proper IP resolution, as well as appservices isolated internally is not possible at the same time. By allowing multiple options to be set, this limitation is resolved and it becomes possible to have client IP resolution behind reverse proxies, as well as allow for direct internal connections for appservices. Options are evaluated in order. If all fail, the request still fails. **Pull request checklist:** <!-- You need to complete these before your PR can be considered. If you aren't sure about some, feel free to ask for clarification in #dev:continuwuity.org. --> - [x] This pull request targets the `main` branch, and the branch is named something other than `main`. - [x] I have written an appropriate pull request title and my description is clear. - [x] I understand I am responsible for the contents of this pull request. - I have followed the [contributing guidelines][c1]: - [x] My contribution follows the [code style][c2], if applicable. - [x] I ran [pre-commit checks][c1pc] before opening/drafting this pull request. - [x] I have [tested my contribution][c1t] (or proof-read it for documentation-only changes) myself, if applicable. This includes ensuring code compiles. - [x] My commit messages follow the [commit message format][c1cm] and are descriptive. [c1]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md [c2]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/docs/development/code_style.mdx [c1pc]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#pre-commit-checks [c1t]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#running-tests-locally [c1cm]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#commit-messages
feat: Allow multiple options in request_ip_source
Some checks failed
Auto Labeler / Apply labels based on changed files (pull_request_target) Successful in 3s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 5s
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m9s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m19s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
4f0a8febf9
Only allowing a single option prevents results in requests being denied if
the singular selected source option isn't used for every single request flow.
This means you need to choose between selecting the right IP or properly
separating your infrastructure traffic. You can't have both;
running edge traffic through a reverse proxy with proper IP resolution,
as well as appservices isolated internally is not possible at the same time.

By allowing multiple options to be set, this limitation is resolved and
it becomes possible to have client IP resolution behind reverse proxies,
as well as allow for direct internal connections for appservices.

Options are evaluated in order. If all fail, the request still fails.
@ -21,0 +89,4 @@
.iter()
.filter_map(|s| ClientIp::for_source(s))
.map(|f| f(&parts))
.reduce(|a, b| a.or_else(|_| b))
Author
Contributor

I thought about maybe doing the mapping early and recording it as an extension, then only grab the extension and execute it here but placing the type in the top-level src/router crate would not allow it to be used in here and I don't know if some other common location would make sense 🤔

I thought about maybe doing the mapping early and recording it as an extension, then only grab the extension and execute it here but placing the type in the top-level src/router crate would not allow it to be used in here and I don't know if some other common location would make sense 🤔
chore: Add towncrier news fragment
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m11s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
24b90164ef
ginger requested changes 2026-07-14 15:30:45 +00:00
Dismissed
Cargo.toml Outdated
@ -117,3 +117,2 @@
[workspace.dependencies.axum-client-ip]
version = "1.3"
[workspace.dependencies.client-ip]
Owner

where is this dependency used?

where is this dependency used?
Author
Contributor

In src/api/router. It's the actual functionality that was behind axum-client-ip. I guess it can be moved to a lower level maybe?

Can't seem to link to the line within the PR, it's in the for_source function starting on line 62.

In `src/api/router`. It's the actual functionality that was behind `axum-client-ip`. I guess it can be moved to a lower level maybe? Can't seem to link to the line within the PR, it's in the `for_source` function starting on line 62.
ginger marked this conversation as resolved
ginger approved these changes 2026-07-14 15:37:59 +00:00
Omar007 force-pushed feature/ip-src-fallback from 24b90164ef
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m11s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
to 594f5ea32a
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m14s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 5m40s
Deploy Element Web / 🏗️ Build and Deploy (pull_request) Failing after 6m30s
2026-07-14 15:42:21 +00:00
Compare
ginger added this to the 26.7.0 milestone 2026-07-14 17:33:26 +00:00
Owner

@Omar007 this fails clippy right now

@Omar007 this fails clippy right now
Author
Contributor

Weird, I'm fairly sure I ran it earlier. I'll do the rebase and run it again 🤔

Weird, I'm fairly sure I ran it earlier. I'll do the rebase and run it again 🤔
Omar007 force-pushed feature/ip-src-fallback from 594f5ea32a
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m14s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 5m40s
Deploy Element Web / 🏗️ Build and Deploy (pull_request) Failing after 6m30s
to a12805e0c9
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 58s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m12s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 11m8s
2026-07-14 18:29:03 +00:00
Compare
Author
Contributor

Ok turns out after my last set of changes I only ended up doing the +nightly format before I committed, I forgot the clippy run at the very end. Fixed, apologies 🙈

Ok turns out after my last set of changes I only ended up doing the `+nightly` format before I committed, I forgot the clippy run at the very end. Fixed, apologies 🙈
@ -391,2 +395,3 @@
/// default: ["direct"]
#[serde(default)]
pub request_ip_source: Option<String>,
pub request_ip_source: Vec<String>,
Owner

This is a breaking change for people that have already set this to something

This is a breaking change for people that have already set this to something
Owner

Seems to have overlapped with !2003 (cc @nex)

Seems to have overlapped with !2003 (cc @nex)
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Required
Details
Checks / Prek / Check changed files (pull_request) Successful in 4s
Required
Details
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 58s
Required
Details
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m12s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 11m8s
Required
Details
This pull request has changes conflicting with the target branch.
  • src/api/client/account/mod.rs
  • src/api/client/account/register.rs
  • src/api/client/dehydrated_device.rs
  • src/api/client/device.rs
  • src/api/client/directory.rs
  • src/api/client/media.rs
  • src/api/client/media_legacy.rs
  • src/api/client/membership/invite.rs
  • src/api/client/membership/join.rs
  • src/api/client/membership/knock.rs
  • src/api/client/message.rs
  • src/api/client/read_marker.rs
  • src/api/client/redact.rs
  • src/api/client/report.rs
  • src/api/client/room/summary.rs
  • src/api/client/send.rs
  • src/api/client/session.rs
  • src/api/client/state.rs
  • src/api/client/sync/v3/mod.rs
  • src/api/client/sync/v5.rs
  • src/api/client/typing.rs
  • src/api/server/invite.rs
  • src/api/server/media.rs
  • src/api/server/publicrooms.rs
  • src/api/server/send.rs
  • src/router/layers.rs
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u feature/ip-src-fallback:Omar007-feature/ip-src-fallback
git switch Omar007-feature/ip-src-fallback
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
continuwuation/continuwuity!1985
No description provided.