Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ValidateRequestHeaderLayer): add assert() function #360

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Oliboy50
Copy link
Contributor

@Oliboy50 Oliboy50 commented Apr 17, 2023

Motivation

At my company, we use a CDN (AWS Cloudfront) in front of our Rust applications...

Then, the AWS recommendation to make sure that all requests come from our CDN (and not from a "hacker" that somehow managed to find the direct IP address of our Rust application) is to make the CDN set a "secret header" with a "secret value" and to assert, in the Rust application, that this "secret header" has been correctly set with the expected "secret value".

AWS documented this behavior here for classical Cloudfront origins and here for Lambda URLs.

However, since tower-http currently does not offer this kind of "header checker", instead of duplicating a ValidateRequestHeaderLayer::custom() closure in Rust application, we had to make our own internal Rust crate containing our own "tower http layer" that uses ValidateRequestHeaderLayer to do this check... It works fine, but I think it would be best for everyone to have this simple layer directly offered by tower-http (under the validate-request feature flag).

Solution

Add a ValidateRequestHeaderLayer::assert() function that acts like ValidateRequestHeaderLayer::accept() but for any header.

BTW, I'm really not sure about the function/struct naming, so better ones are more than welcome 馃檹

Comment on lines +101 to +114
//! #
//! # // Requests without the expected header also get a `403 Forbidden` response
//! # let request = Request::builder()
//! # .body(Body::empty())
//! # .unwrap();
//! #
//! # let response = service
//! # .ready()
//! # .await?
//! # .call(request)
//! # .await?;
//! #
//! # assert_eq!(StatusCode::FORBIDDEN, response.status());
//! #
Copy link
Contributor Author

@Oliboy50 Oliboy50 Apr 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

鈩癸笍 this has been hidden from the documentation because it feels redundant... but it has been kept because it remains a valuable test

@@ -112,6 +178,8 @@
//! # Ok(())
//! # }
//! ```
//!
//! [`Accept`]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept
Copy link
Contributor Author

@Oliboy50 Oliboy50 Apr 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not too sure about this one...
I copied/pasted it from the existing accept() function documentation when I added this line of documentation:

Validation of the Accept header can be made by using [ValidateRequestHeaderLayer::accept()]:

but I think that it is mostly useless because I didn't find any link to the MDN in the generated documentation page 馃し

just tell me if I should remove it... BTW I can also delete the one in the accept() function documentation if you want (same issue)

@Oliboy50 Oliboy50 force-pushed the validate-static-header-value branch from 46a73ce to 042bbff Compare April 18, 2023 07:12
@Oliboy50 Oliboy50 force-pushed the validate-static-header-value branch from 042bbff to bbd0d90 Compare April 18, 2023 18:56
//! # async fn main() -> Result<(), Box<dyn std::error::Error>> {
//! let mut service = ServiceBuilder::new()
//! // Require a `X-Custom-Header` header to have the value `random-value-1234567890` or reject with a `403 Forbidden` response
//! .layer(ValidateRequestHeaderLayer::assert("x-custom-header", "random-value-1234567890", StatusCode::FORBIDDEN))
Copy link
Contributor Author

@Oliboy50 Oliboy50 Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also thought of using a Builder pattern here

so instead of the assert function, the API would look like:

.layer(
    ValidateRequestHeaderLayer::has("x-custom-header")
        .with_value("random-value-1234567890")
        .or_reject_with(StatusCode::FORBIDDEN)
)

where or_reject_with would be the "build" method...

and we could say that with_value is optional, so this new layer could also be used by those who just want to make sure that a request has a specific header and they don't care about its value

.layer(
    ValidateRequestHeaderLayer::has("x-custom-header")
        .or_reject_with(StatusCode::FORBIDDEN)
)

WDYT?

@Oliboy50
Copy link
Contributor Author

Oliboy50 commented May 9, 2023

鈩癸笍 I don't think that the CI is failing because of this PR
Capture d鈥檈虂cran 2023-05-09 a虁 10 17 36

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.

None yet

1 participant