Skip to content

Allowed HTTP Methods for Actions #10

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 4, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions design-documents/allowed-http-methods-for-actions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
### Overview

To improve security and logistics we need to allow limiting Actions to processing only requests with certain HTTP methods and add those limitations to as many existing Actions as possible.
There are many vulnerabilities caused by actions processing both GET and POST requests and thus allowing bypassing security validations like form key validation. Also limiting actions to processing only requests with certain methods would serve as self-documentation for Action classes and improve consistency of server side for client code and functional tests.

### Theory of Operation

Since we use by-convention routing it would be inconsistent to defined actions’ allowed HTTP methods in a config file, so it’s better to create marker interfaces like these:

>interface HttpGetActionInterface extends ActionInterface
>
>{
>
>}
>
>
>interface HttpPostActionInterface extends ActionInterface
>
>{
>
>}
>
>
>interface HttpPutActionInterface extends ActionInterface
>
>{
>
>}

etc.

One interface per each HTTP method (using list of HTTP methods from \Zend\Http\Request::METHOD_* constants). We will have an exentable map *HTTP method* => *Interface* passed to the validator to allow custom HTTP methods to be processed.

Request method will be validated in the FrontController before executing a matched action, somewhat like this:

>if (!(array_key_exists($request->getMethod(), $interfaceMap) && $actionInstance instanceof $interfaceMap[$request->getMethod()])) {
>
>    throw new NotFoundException(__(‘Page not found.’));
>
>}


To limit already existing Actions and get developers to know these new interfaces a temporary event listener will be added for controller_action_postdispatch event to log HTTP methods used with actions and then the full suite of functional tests (MTF and MFTF) would be ran to get information on as much actions as possible.
Copy link
Member

Choose a reason for hiding this comment

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

Can we leave logging in place to let extension developers know that they need to update their extensions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remember we discussed this, I just forgot to mention it - we can add a debug message each time an action without HTTP method declared s processed

Copy link
Member

Choose a reason for hiding this comment

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

I'm ok with not having this if we going to have a mess detector rule.


After corresponding Http*Method*ActionInterface will be added to every Action class we have information on. If an action is logged to be accessed only by GET methods it will allow only GET, if only by POST – allowing only POST etc. If by more than one methods – the Action will be investigated and either updated to implement multiple interfaces or left alone if proven too difficult to determine HTTP methods.

To sway developers to specify HTTP methods for actions a new rule for code mess detector will be added that will require implementations of ActionInterface to implement also at least one of Http*Method*ActionInterface interfaces.

### Work Breakdown
* Add Http*Method*ActionInterface and implement request validation in the FrontController
* Add a rule to code mess detector
* Create logging mechanism for Actions
* Log HTTP methods used when running full functional tests’ suite
* Create script to add Http*Method*ActionInterface to Action classes we’ve collection information about
* Update Action Classes