This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Use Pydantic to systematically validate a first batch of endpoints in synapse.rest.client.account
.
#13188
Merged
Merged
Use Pydantic to systematically validate a first batch of endpoints in synapse.rest.client.account
.
#13188
Changes from 27 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
c932ed5
Lock pydantic
c40edbd
Use pydantic mypy plugin
52b0ef3
Helper for validating a json body with pydantic
87a6e79
Validate `client/account/deactivate`
c7caf5b
Validate `/client/account/password`
3319732
Changelog
9a47994
Better errcode when pw reset emails are disabled
0a87861
Validate `/client/account/passsword/email/requestToken`
3e2d003
move out RequestToken body to models
ba71e02
Validate `/client/account/3pid/email/requestToken`
e4926c2
Ooops, use Strict* types for EmailRequestTokenBody
b81faf4
Fix linter whoops
dc1f188
Revert "Better errcode when pw reset emails are disabled"
61f8f25
Move the models into the servlet classes
296467e
Revert another status code change.
94ca8d3
Merge branch 'develop' into dmr/rest/client/account
a273870
Relax pydantic version Bound
07ba995
Fix bad merge?
74cceab
Argh, fix linter
ff80b3f
A few str -> StrictStr when TYPE_CHECKING
a256487
Merge remote-tracking branch 'origin/develop' into dmr/rest/client/ac…
7294fc2
Use Pydantic 1.7.4
f20226e
Update changelog
df1f840
Require access token if id server is provided
c4074aa
Ignore unused fields; create frozen instances
fefb0a8
Fix mypy
4dbafed
Fix lint
bd917d3
Update changelog
f8f6037
Add a missing `strict=True`.
c404f8f
Move the BaseModel subclass to synapse.rest
1597977
Try to break circular import?
22aee7a
Fix lint durr
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Improve validation of some account-related REST endpoints. | ||
This file contains 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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,9 +23,12 @@ | |
Optional, | ||
Sequence, | ||
Tuple, | ||
Type, | ||
TypeVar, | ||
overload, | ||
) | ||
|
||
from pydantic import BaseModel, ValidationError | ||
from typing_extensions import Literal | ||
|
||
from twisted.web.server import Request | ||
|
@@ -694,6 +697,28 @@ def parse_json_object_from_request( | |
return content | ||
|
||
|
||
Model = TypeVar("Model", bound=BaseModel) | ||
|
||
|
||
def parse_and_validate_json_object_from_request( | ||
request: Request, model_type: Type[Model] | ||
) -> Model: | ||
"""Parse a JSON object from the body of a twisted HTTP request, then deserialise and | ||
validate using the given pydantic model. | ||
|
||
Raises: | ||
SynapseError if the request body couldn't be decoded as JSON or | ||
if it wasn't a JSON object. | ||
""" | ||
content = parse_json_object_from_request(request, allow_empty_body=False) | ||
try: | ||
instance = model_type.parse_obj(content) | ||
except ValidationError as e: | ||
raise SynapseError(HTTPStatus.BAD_REQUEST, str(e), errcode=Codes.BAD_JSON) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be nice to define a new subclass of SynapseError for this. Not going to block this PR on it though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, so we could catch a validation failure specifically? Happy to revisit this i the future if it turns out we need it
DMRobertson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return instance | ||
|
||
|
||
def assert_params_in_dict(body: JsonDict, required: Iterable[str]) -> None: | ||
absent = [] | ||
for k in required: | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a
feature
, and we should name the endpoints in question.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, bd917d3