facebookarchive / CommunityCellularManager

Tool for deploying, managing and controlling your Community Cellular Networks
Other
87 stars 36 forks source link

nginx incorrectly redirects POST traffic #23

Closed kheimerl closed 7 years ago

kheimerl commented 7 years ago

nginx is currently redirecting http POST traffic, but that is not allowed (the browsers resend as GET). I have a fix in our UW branch.

shaddi commented 7 years ago

Can you provide an example of a request that triggers this behavior? The stock ngnix configuration doesn't do any redirects afaik. A pointer to your fix would also be helpful.

shaddi commented 7 years ago

Ah, my bad -- I see where we're doing that. Are you deploying on AWS behind an ELB? Since that's how we've always deployed I wonder if that's masked this issue.

kheimerl commented 7 years ago

I think the current arch redirects:

# Force HTTPS
if ($http_x_forwarded_proto != "https") {
  rewrite ^(.*)$ https://$host$request_uri permanent;
}

This command was failing anyhow

kheimerl commented 7 years ago

Gha too many bad buttons. Yes, i think my AWS config might be incorrect and thus not masking the issue. The current state of the fix is here: https://github.com/uw-ictd/CommunityCellularManager/blob/regionchange/cloud/configs/nginx.conf

kheimerl commented 7 years ago

I think, moreover, we may have never been sending POSTS to http addresses to be redirected anyhow, as that was a bad registry on client

shaddi commented 7 years ago

Got it. Your fix looks good, feel free to submit it as a PR when ready. Thanks!

We should note that this will disallow non-HTTPS POST requests by default in deployments using nginx but not HTTPS. I think this is the right default behavior.

9muir commented 7 years ago

it should be pretty easy to create a small PR from your fork, you can just choose this repo as master instead of your fork's master branch.

9muir commented 7 years ago

This was closed per the above (PR #25)