This repository has been archived by the owner on Feb 12, 2023. It is now read-only.
Security fixes #303
Merged
Security fixes #303
Conversation
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 will let us use lambdas and functional objects to reduce code verbosity
|
So far this looks good, but as someone who is only marginally able to run Aggregate (and even then with tons of errors on startup that don’t seem to break the features I need for running Briefcase), I can’t vet this change with confidence in my ability to understand it. |
|
Thanks, @dcbriccetti! |
dcbriccetti
reviewed
Aug 26, 2018
| }; | ||
| } | ||
|
|
||
| ; |
There was a problem hiding this comment.
By reformatting the code and exposing these unnecessary semicolons—I count ten—you acquire the responsibility of deleting them. 😀
There was a problem hiding this comment.
I wonder how on earth did I miss those! Removing them :)
- Reformat and optimize imports - Remove from method signatures all the exceptions that weren't being thrown - Use diamond operator where available - Remove unnecessary initialization of variables - Remove empty doc blocks - Collapse similar catch blocks - Apply some static imports to reduce verbosity
- Suggested by IntelliJ
|
I am not sure about purging process so I reported a new issue #307. Confirm that other process works. |
Merged
lognaturel
approved these changes
Sep 11, 2018
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 adds extra security protections to some RPC calls:
What has been done to verify that this works as intended?
First, I've manually verified that all these operations still work in my local Aggregate instance
Second, I've inspected the network interchange of the delete form action and I've resent it using curl from the command line to verify that it won't work since the CSRF token wasn't valid anymore.
Why is this the best possible solution? Were any other approaches considered?
This solution implements the official instructions to add CSRF protection to GWT apps.
To reduce code duplication and complexity, I've created a wrapper that handles client-side CSRF requests.
Are there any risks to merging this code? If so, what are they?
Any third party hitting these RPCs will have to request a CSRF token before making them.
Do we need any specific form for testing your changes? If so, please attach one
No.
Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.
No.