fix check for cors allow credentials #320
Conversation
|
Travis runs of tests generates fails in 2 environments py26 & py34 are unrelated. The py27 has a test fail that I believe is a bad test given it assumes the same thing the original code did i.e. that you can use Access-Control-Allow-Credentials as a request header, and so it necessarily fails my fix. I'm researching this further now. |
|
apologies for that broken commit message there. What I would add is that when eliminate the incorrect use of 'Access-Control-Allow-Credentials' as a request header in that one test, then it and the previous test can not both pass since they are the same and expect different results. Eliminated the test that is incorrect based on the issue that triggered this fix. |
| @@ -96,8 +96,7 @@ def ensure_origin(service, request, response=None): | |||
| for o in service.cors_origins_for(method)]): | |||
| request.errors.add('header', 'Origin', | |||
| '%s not allowed' % origin) | |||
| elif request.headers.get( | |||
| 'Access-Control-Allow-Credentials', False): | |||
| elif service.cors_support_credentials_for(method): | |||
There was a problem hiding this comment.
I believe this is not the correct way of handling the problem: here it prevents the response to contain the correct AC-Allow-Origin response header (as the tests showcase).
This is probably due to the fact this is in an elif where the else tests that the allowed origins contain a wildcard.
There was a problem hiding this comment.
I believe the test is testing for something that in fact should not be true and that's why I eliminated the test. Let me explain there on your other comment.
|
Okay, this makes more sense now. Thanks for waiting this long (I was on vacations). I believe we need to add tests still there and then we'll be good. |
|
Hope you enjoyed your vacation. Added the assertion to the fixed test. Thanks. |
fix check for cors allow credentials
|
Thanks! Merged. |
As per my submission of #319, I was hitting CORS issue when using credentials. In particular, I get the following error:
What I think should be happening is the server should be setting the 'Access-Control-Allow-Origin' response header to the origin provided in the credentialed request. I've replaced an incorrect check of a request header, Access-Control-Allow-Credentials, which is really a response header with a correct test on the service definition.