CORS support?

[jldiaz]

Hello.

Thank you very much for the effort of putting together this real-world example code. I was working on a project which uses morepath+pony at the backend and your example provides invaluable info for the configuration and deployment.

However, as far as I can see, your backend does not support CORS. When I try, for example, the Angular client and set https://conduit.yacoma.it/api as the api_url in environments/environment.ts, the page shows "Loading articles..." and no progress happens.

If I deploy my own local server (in a machine different than the one serving the client), I can see that the browser is sending OPTIONS requests (the CORS preflight), and the backend is answering "405 Method not allowed", instead of answering with "200 OK" and the appropiate CORS headers.

In my own project I had this problem also, and solved it by adding somewhere (in views.py) the following snippet:

@App.tween_factory()
def cors_tween(app, handler):
    def cors_handler(request):
        print(request)
        preflight = request.method == 'OPTIONS'
        if preflight:
            response = Response()
        else:
            response = handler(request)
        response.headers.add('Access-Control-Allow-Origin', '*')
        response.headers.add('Access-Control-Allow-Methods', 'GET, POST, PUT, DELETE')
        response.headers.add('Access-Control-Allow-Headers', 'authorization, content-type')
        return response
    return cors_handler

But I'm not sure if this is the proprer way to do it in "real world".

Could you please provide an example showing how CORS can be added to the backend?

[henri-hulski]

I have to release more.cors. But it's actually ready so I can release today.

[henri-hulski]

@jldiaz CORS support added in 6061a06.
Could you confirm that it's working now?

[jldiaz]

@henri-hulski Yes! It is working nicely now. Thank you very much.

[jldiaz]

@henri-hulski Hum... Actually something is wrong. All routes appear to work, except

OPTIONS /api/user

which returns 404 and no CORS headers. This makes impossible to update the user profile, because the browser rejects to do the PUT if the preflight response is not successful.

I think the problem can be related to the fact that the browser does not send the Authorization token as part of the preflight. Somehow, more.cors rejects the unauthorized attempt, but it does wrong, because, first, it returns 404 instead of 401, and second, no CORS headers are included in the response (which should be for OPTIONS verb).

This is the request:

OPTIONS /api/user HTTP/2.0
Host: <obfuscated server NAME>
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Encoding: gzip, deflate, br
Accept-Language: es-ES,es;q=0.8,en;q=0.5,en-US;q=0.3
Access-Control-Request-Headers: authorization,content-type
Access-Control-Request-Method: PUT
Origin: https://angular-realworld.stackblitz.io
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
X-Forwarded-For: <obfuscated IP>

This is the answer:

HTTP/1.1 404 Not Found
Connection: close
Content-Type: text/html; charset=UTF-8
Date: Fri, 08 Jun 2018 17:50:47 GMT
Server: gunicorn/19.8.1
Content-Length: 0

[henri-hulski]

Thanks for checking this
I will look into it asap.

[henri-hulski]

@jldiaz Could you open an issue in more.cors as it seems to belong there.
Maybe with some more background and a reproducible use case or test.
Thanks for finding this.

[henri-hulski]

I do not really understand why the route to /api/user doesn't work but the others are working.
Are you sure you haven't a cache problem?

[jldiaz]

No, it is not a cache problem. I have the same results if I perform the queries from CLI, using curl or httpie. But you are in part right. The problem is not with that route only. Other routes have the same problem.

Apparently, the browser never sends the Authentication header as part of OPTIONS, so any route which requires authentication (and should produce 401 if no auth is provided), causes a 404 error instead.

Routes which are not protected, such as /api/articles or /api/tags, or /article/id/comments have not this problem, so the page is almost functional and thus I did not notice the problems.

Interestinlgy, it is even posible to create a new article, because that is a POST on /api/articles, and this route allows GET (and thus OPTIONS) without auth.

[henri-hulski]

If you adjust the more.cors settings and set 'allow_credentials' to True and 'allowed_origin' to the specific server URL, does that fix it?

[henri-hulski]

For example by adding to conduit/settings/production.yml something like:

cors:
  allowed_origin: https://conduit.yacoma.it
  allow_credentials: True

[jldiaz]

No luck. Still the preflight answer does not contain cors headers.

I think the problem is related to line 94 in more.cors/main.py, because when the model is (tried to be) resolved, the exception HTTPUnauthorized is raised, and thus the context is set to app.

I think that causes trouble later, when at line 117 the context is used to find out the allowed verbs. The allowed_origin list is empty.

However I think that is a "philosophical" design flaw, difficult to be solved. I can build a (not so) minimal example using morepath, more.cors and more.jwt to show the problem, and post it as an issue at more.cors.

[jldiaz]

I'm not sure if more.cors is to be blamed... The problem is caused in fact by this line in morepath-realworld-example-app/conduit/auth/path.py, because raising an exception inside a path causes that no model can be associated with that path, and thus no introspection can be used to check the allowed verbs (which pertain to the views).

The problem is solved if permissions are managed in a standard way, i.e: allowing the path to return the appropiate model, and leaving the permission management to the view. I discovered this in the minimal app I was preparing to illustrate the issue.

[henri-hulski]

Hmm but that line should raise a HTTPUnauthorized 401 error not 404.

[henri-hulski]

If the path returns None like in https://github.com/yacoma/morepath-realworld-example-app/blob/master/conduit/auth/path.py#L32 it raises 404.

[henri-hulski]

In more.cors this line is raising a 404 exception:
https://github.com/morepath/more.cors/blob/master/more/cors/main.py#L109
You could change that to 401 on your site and check if the HTTP status code you get will also change.

[jldiaz]

The true problem is not wheter more.cors returns 401 or 404. The real problem is that the CORS headers are absent from the response (because not allowed-methods were found). Thus, when the browser receives this response, it aborts the request.

To summarize:

• The client wants to PUT /api/user some data, and includes the appropiate Authorization headers.
• The browser, before the PUT, performs a preflight with the OPTIONS verb.
• OPTIONS verb cannot include Authorization headers.
• The app route, without the Authorization header cannot determine the user identity, so it cannot return the appropiate User[] object, and raises 401 instead
• more.cors, without the appropiate model cannot introspect to get the list of allowed verbs, and thus get_cors_allowed_methods() returns an empty list of verbs
• When more.cors detects the empty list, it returns 404 (it could return 401, but that doesn't matter) and a response without CORS headers
• When the browser receives the response without CORS, it aborts the operation and the actual PUT is not performed. This PUT would be perfectly legal and include the appropriate Authorization headers, but it is never performed because more.cors did not provide the authorizing headers.

A solution could be to modify more.cors so that it returns App.settings.cors.allowed_verbs if the path raised HTTPUnauthorized. I made that patch and it is working ok, so far.

[henri-hulski]

Ok I got it. Thanks for the explanation.
Your solution sounds fine as the path in conduit seems to be valid regarding the Morepath logic.
So it would be good if more.cors would support it.
Could you create a PR for it in more.cors?

BTW As it seems you are into the CORS thingy maybe you could also review morepath/more.cors#2. A haven't found anyone who wanted to review it and at the end just merged it in. It looks good to me, but I'm not too experienced with CORS.

[henri-hulski]

Fixed in morepath/more.cors#4.