Merged
Conversation
Member
Author
|
Updated a few things:
|
| pub(crate) async fn validate_request_optionally(&self, req: Request) -> Request { | ||
| let (mut parts, body) = req.into_parts(); | ||
| if parts.headers.contains_key(MAUTH_V2_SIGNATURE_HEADER) | ||
| || parts.headers.contains_key(MAUTH_V1_SIGNATURE_HEADER) |
There was a problem hiding this comment.
is this check case-insensitive?
https://www.rfc-editor.org/rfc/rfc7230#section-3.2
Each header field consists of a case-insensitive field name
Member
Author
There was a problem hiding this comment.
Yes, the headers field here is a http::header::HeaderMap, not a standard HashMap. It already fully accounts for the case-insensitive nature of HTTP headers.
ejinotti-mdsol
approved these changes
Jan 7, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR redoes the middleware system. The previous code had one middleware which always rejected the request entirely if it could not find or validate a MAuth signature, not allowing it to reach the lower layers at all. This may be desirable in some cases, and is possible to use reasonably flexibly with the way most server routers support dynamically attaching layers to only some of the routes. After some work with it though, I decided that it is not sufficiently flexible for many reasonable uses. So here, I renamed the old layer and service to be
RequiredMAuthValidationLayerand created a new one namedOptionalMAuthValidationLayer. I also decided that a configuration option for this was insufficiently clear as to the intent of the user. This is a breaking change for any users of the old layer, but I don't think there are any right now, aside from the demo service I deployed. Making this not a breaking change would also require a default setting for the original layer, which in my opinion is insufficiently clear to users and readers of code using it regarding which mode the layer is operating in.The Optional layer will attempt to check for a mauth signature, attach an extension to the request if it finds a valid one. However, it will always forward the request down the layers, whether or not it finds a valid signature. If this is used, it is the responsibility of the handler to specify that the extension is required, or use some other algorithm to validate requests.
To make this smoother, the
ValidatedRequestDetailsnow also impls Axum'sFromRequestParts, so you can include it in a handler function, and it will validate that it is present, make it available to the function if desired, and also return a 401 Unauthorized response if it is not present. The return code and body of the default Axum Extension extractor is not very good for this use case.Also updated to use Yohei's updated mauth-core.