Skip to content

"Method Not Allowed" on POST request #56

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

Closed
GregPeden opened this issue Mar 6, 2017 · 5 comments
Closed

"Method Not Allowed" on POST request #56

GregPeden opened this issue Mar 6, 2017 · 5 comments

Comments

@GregPeden
Copy link
Contributor

I am really struggling to get a POST request to create a new model entry to work. I get "405 Method Not Allowed" no matter what I do.

Note I am using Laravel 5.3 and trying to use Passport with v0.6.2 of this API package on a Homestead development environment. When using Postman to test, if I have the Passport auth API disabled then a GET request works but with Passport on (and no auth effort attempted in Postman) it refuses. This is expected and appears to show that Passport is working as expected.

So now I am trying to consume the API within my website itself per one of the features of Passport. Here, a GET request works but a POST request to the same resource gets "method not allowed". The "X-CSRF-TOKEN" and "X-XSRF-TOKEN" is being included with every request.

Laravel docs mention a "laravel_token" baked in to the session cookie I can see it is provided with each session but is not included in a request cookie or the headers, and for the life of me I cannot figure out how or if I am meant to return this on a request. I can find almost nothing about "laravel_token" online.

https://laravel.com/docs/5.3/passport#consuming-your-api-with-javascript

Any thoughts on where to start?

Code references will follow.

@GregPeden
Copy link
Contributor Author

GregPeden commented Mar 6, 2017

Here is my test page to play with the API within Laravel's environment. I am using VueJS and Axios.

@extends('layouts.master')

@section('title')
    @parent :: Sandbox
@stop

@section('content')
<div class="container">
    <div class="row">
        <div class="col-12 my-2">
            CSRF: {{ csrf_token() }}
        </div>
        <div class="col-12 my-2">
            <button class="btn btn-primary" v-on:click="apiTest" :disabled="running">Run <span v-show="running" class="fa fa-spinner fa-spin"></span></button>
        </div>
        <div v-if="response.status" class="col-12 my-2">
            <table class="table table-striped">
                <tr>
                    <td>Status</td>
                    <td>@{{ response.status }} @{{ response.statusText }}</td>
                </tr>
                <tr>
                    <td>Headers</td>
                    <td><pre>@{{ headersPretty }}</pre></td>
                </tr>
                <tr>
                    <td>Data</td>
                    <td><pre>@{{ dataPretty }}</pre></td>
                </tr>
                <tr>
                    <td>Config</td>
                    <td><pre>@{{ configPretty }}</pre></td>
                </tr>
            </table>
        </div>
    </div>
</div>
@stop

@section('vue')
    <script>
        new Vue({
            el: '#app',

            data: {
                running: false,
                response: {
                    data: null,
                    status: null,
                    statusText: null,
                    headers: null,
                    config: null
                }
            },

            computed: {
                dataPretty: function() {
                    return this.response.data ? JSON.stringify(this.response.data, null, 2) : '';
                },

                headersPretty: function() {
                    return this.response.headers ? JSON.stringify(this.response.headers, null, 2) : '';
                },

                configPretty: function() {
                    return this.response.config ? JSON.stringify(this.response.config, null, 2) : '';
                }
            },

            methods: {
                setResponse: function(response) {
                    this.running = false;
                    this.response.data = response.data;
                    this.response.status = response.status;
                    this.response.statusText = response.statusText;
                    this.response.headers = response.headers;
                    this.response.config = response.config;
                },

                resetResponse: function() {
                    this.running = true;
                    this.response.data = null;
                    this.response.status = null;
                    this.response.statusText = null;
                    this.response.headers = null;
                    this.response.config = null;
                },

                apiTest: function() {
                    this.resetResponse();
                    axios
                        .post('{!! route('v1::client-companies.index') !!}',
                            {
                                data: {
                                    type: "client-companies",
                                    attributes: {
                                        name: "API Corp"
                                    }
                                }
                            })
                        .then(function(response) {
                            this.setResponse(response);
                        }.bind(this))
                        .catch(function(error) {
                            this.setResponse(error.response);
                        }.bind(this));
                }
            }
        })
    </script>
@stop

The CSRF token is embedded within all Axios requests like so:

window.axios.defaults.headers.common = {
    'X-CSRF-TOKEN': window.Laravel.csrfToken,
    'X-Requested-With': 'XMLHttpRequest'
};

window.axios.defaults.headers['Content-Type'] = 'application/vnd.api+json';

Every page request includes this for the CSRF token lookup by the client:

<!-- Scripts -->
<script>
    window.Laravel = {!! json_encode([
            'csrfToken' => csrf_token(),
    ]) !!}
</script>

Running the button queries the API and displays the result. The post-execution result is here:
http://codepen.io/gregpeden/pen/BWLGoj

If I run a GET request, it works (I cut out the result in the below result for privacy reasons).
http://codepen.io/gregpeden/pen/yMaQyo

@GregPeden
Copy link
Contributor Author

Okay I figured it out.

I created the model and then later chose to allow write actions, so I created the Hydrator but failed to pass it in to the controller constructor as required. The API package checks for this, and in this case it returns the "405 Method Not Allowed" response here:

protected function methodNotAllowed()

I suspect this will happen to others as well, and I propose that "Method Not Allowed" is not the correct response for this error, as this implies the POST/PUT/PATCH methods are intended to be forbidden on the model when the issue is in truth a program error. This code is detecting the exact problem and then returning a misleading response. Furthermore, it does not make use of Laravel's exception handling system, which I spent hours trying to hook to log the event. I only figured this out after searching "methodnotallowed" on the entire package out of suspicion that it's not actually throwing the "MethodNotAllowed" exception.

@GregPeden
Copy link
Contributor Author

GregPeden commented Mar 8, 2017

Actually, I see that the absence of a hydrator is meant to imply that the route is forbidden. It probably should occur earlier in the process, since a lot of code executed, including validation, before it gets to the hydrator check. Probably it should be expected that the designer applies 'only' or 'except' options to the route 'resource()' method, except the $options array is never parsed and so this can't be done at present. This would also improve performance a little bit since Laravel allows the routing table to be pre-compiled and this would reduce the number of routing options it is handling during the URI parsing.

Of course, I am aware that you guys are also exploring refactoring the request/routing design for this package, so this might be moot.

Overall thanks a lot, this is developing in to a great package and it's great to be able to say "our API is JSON spec-compliant so just follow that please".

@lindyhopchris
Copy link
Member

Hi @SirLamer

Sorry for not replying earlier, have been off work. Glad you figured this out and sorry it took a while to discover it.

The 405 is intentional - i.e. if you don't inject a hydrator, the methods that create/update a resource do not effectively exist, which is why I decided to return a 405.

I totally concur though with your point that this actually should be discovered higher up the stack (i.e. don't register the route if it's not allowed), which would then mean that the controller can throw a RuntimeException if it gets hit with a create/update request but hasn't got a hydrator injected. That would have made it a lot easier for you to track down.

I'll link this issue to the routing refactoring issue so that we can incorporate not registering routes into that.

The secondary point is that better documentation would also help. I definitely want to see comprehensive documentation but I think the package needs to settle a bit more before investing the effort in documenting everything fully.

@lindyhopchris
Copy link
Member

The new routing design has been merged into develop and will be released in v0.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants