Provide request input sanitization #358
Conversation
|
|
||
|
|
||
| async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: | ||
| if "method" not in scope or scope["method"] in ("GET", "HEAD", "OPTIONS"): |
There was a problem hiding this comment.
I just went with those methods. Please adjust this line, if we want to make this middleware more specific to special header configurations.
There was a problem hiding this comment.
This conditional asserts all exclude options as true. Since we like to sanitize POST, it's not listed here. Or do I misunderstood something?
There was a problem hiding this comment.
oh by returning none it will move forward with the middleware? if so can you explicitly return none.
if thats not it, i need to look at the starlette docs again 😅
There was a problem hiding this comment.
ohh. the sanitize functions are in call. okay i just cant read indents today. my b.
We might want to sanitize GETs though. we can coms back to that
There was a problem hiding this comment.
Only the sanitize_request_body def is in __call__, sorry if this is confusing 🙈 When I implemented it, it made perfectly sense 😂 Always open for suggestions.
| if isinstance(value, dict): | ||
| json_body[key] = __class__.sanitize_dict(value) | ||
| elif isinstance(value, list): | ||
| json_body[key] = __class__.sanitize_list(value) | ||
| else: | ||
| json_body[key] = __class__.sanitize_str(value) |
There was a problem hiding this comment.
I thought handling dicts, lists and strings would be sufficient for now. But let me know, if we need more checks here.
|
|
||
|
|
||
| async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: | ||
| if "method" not in scope or scope["method"] in ("GET", "HEAD", "OPTIONS"): |
| return message | ||
| if not isinstance(body, bytes) : | ||
| return message | ||
| json_body = json.loads(body) |
There was a problem hiding this comment.
I think we use formData for login/password form. So this might not work here. 🤔
I guess you could catch the exception and sanitize body input itself.
There was a problem hiding this comment.
Good point, will add that
There was a problem hiding this comment.
I provided a is_json() utility function now.
|
@MelissaAutumn This PR should be ready now, I added an example test case for schedule updates. We can add more tests for different endpoints in the future, if we need them. |
|
Since we're only using formData for temporary dev login for now, let's take a look at that again if we need it somewhere else. |
* ➕ Provide request input sanitization * 🔨 Check for json request body * ➕ Add test for input sanitization
Description of the Change
This PR provides a new middleware to strip all html tags from data changing requests using nh3.
I added one test case for schedule updates, we can add more if we need them at other places.
Benefits
Prevent XSS.
Applicable Issues
Closes #263