[Ops/VSP] 526: File Upload Network Error Issues

[luke-gcio]

Issue Description

When I make a request from vets-website that fails at the revproxy rather than vets-api from vets-website I get see a CORS error rather than the actual error sent from the revproxy because the revproxy does not set the Access-Control-Allow-Origin header. If I were to make the same request as a non-ajax request (through command line or postman or something) I would successfully see the revproxy error.

So, for example, if I sent a too-large file (>151mb) from vets-website the revproxy would return 413 Request Entity Too Large but it would appear in the browser as being blocked by the CORS policy.

Placeholder for followup to https://github.com/department-of-veterans-affairs/va.gov-team/issues/14427

I'm thinking that the correct fix is to set the Access-Control-Allow-Origin header in the nginx config

to the same values we use in vets-api

[jason-gcio]

Thank you Anna!

[omgitsbillryan]

1. You mention "fwdproxy" - did you mean the revproxy?

   the fwdproxy would return 413 Request Entity Too Large

2. The vets-api rails app has a middleware that responds with it's own CORS headers. Is the assumption that the revproxy is rejecting the request immediately, such that, it never gets to the rails app?

3. Would you mind posting the full curl (or equivalent) command to reproduce the issue?

4. Since the rails app already returns CORS response headers, there might need to be some additional logic to prevent us from accidentally returning multiple / duplicate CORS response headers from rails & nginx.

[omgitsbillryan]

WRT point 4 ☝️ , looks like we may have already experienced & solved - link to code

[annaswims]

You mention "fwdproxy" - did you mean the revproxy?

yeah, I fixed it above. This is all about the proxy between the browser and rails.

[annaswims]

Would you mind posting the full curl (or equivalent) command to reproduce the issue?

I think this sort of gets you what you're looking for. I'd want the response to have a Access-Control-Allow-Origin header. We first saw this before we uploaded files that were larger than what the proxy allowed, but since then we've upped the proxy limit.

small file - response from rails (disregard that it's an error response, we just want to look at the headers)

curl -i  --location --request POST 'https://staging-api.va.gov/v0/vic/supporting_documentation_attachments' \
> --header 'Origin: https://staging.va.gov' \
> --form 'file=@"/Users/annacarey/Downloads/orgchart.pdf"'
HTTP/1.1 100 Continue

HTTP/1.1 403 Forbidden
Date: Wed, 17 Feb 2021 01:11:12 GMT
Content-Type: application/json; charset=utf-8
Content-Length: 100
Connection: keep-alive
Access-Control-Allow-Credentials: true
Access-Control-Allow-Methods: GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS
Access-Control-Allow-Origin: https://staging.va.gov
Access-Control-Expose-Headers: X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-Session-Expiration, X-CSRF-Token
Access-Control-Max-Age: 7200
Referrer-Policy: strict-origin-when-cross-origin
Vary: Origin
X-Content-Type-Options: nosniff
X-Download-Options: noopen
X-Frame-Options: SAMEORIGIN
X-Git-SHA: 4af6ea46153d6610a4d6dcb7816907dc816e9d87
X-GitHub-Repository: https://github.com/department-of-veterans-affairs/vets-api
X-Permitted-Cross-Domain-Policies: none
X-Request-Id: a8b07a6b-e163-43a2-b6cf-e5164ad60e12
X-Runtime: 0.044629
X-XSS-Protection: 1; mode=block
Strict-Transport-Security: max-age=31536000; includeSubDomains; preload
Set-Cookie: TS016f4012=01c8917e48de31a26e2391d2fe9ed889fe2a504883c3ffb2bee96c6e08bb48d3ae1557dfe01e21b9bb0f642beef3bbfc57ee816bbf; Max-Age=900; Path=/

{"errors":[{"title":"Forbidden","detail":"Invalid Authenticity Token","code":"403","status":"403"}]}

same request, but with a file that's > 150 mb and therefore gets the response from nginx

curl -i  --location --request POST 'https://staging-api.va.gov/v0/vic/supporting_documentation_attachments' \
> --header 'Origin: https://staging.va.gov' \
> --form 'file=@"/Users/annacarey/Downloads/oldhouse/DJI_1122.MP4"'
HTTP/1.1 100 Continue

HTTP/1.1 413 Request Entity Too Large
Date: Wed, 17 Feb 2021 01:13:37 GMT
Content-Type: text/html
Content-Length: 180
Connection: close
Cache-Control:
Strict-Transport-Security: max-age=31536000; includeSubDomains; preload

<html>
<head><title>413 Request Entity Too Large</title></head>
<body>
<center><h1>413 Request Entity Too Large</h1></center>
<hr><center>openresty</center>
</body>
</html>

[omgitsbillryan]

Thanks for reproducing.

WRT adding the CORS response headers via nginx - isn't the real problem that nginx is denying these requests in the first place? If nginx allowed the request through, rails would tack on the appropriate CORS response headers.

It looks like you already addressed this problem by setting client_max_body_size 151m; for any directive that include base-shared.conf which includes the entire api server directive itself. Do you have any idea why nginx is still denying this request?

[annaswims]

Without the headers it's a timesuck of troubleshooting. The nginx error appears to be a cord error and fe devs never see the real error. But yeah, it's a secondary problem.

https://dsva.slack.com/archives/CBU0KDSB1/p1612826771412600

[annaswims]

I think for a long time we've assumed that Network request failed errors were outside of our control, but they're not. At least some of them are CORS errors that block the original error message.

[omgitsbillryan]

I started deep-diving on this one, and so far, I hate all my solutions 😄

• disabling the nginx check altogether --- exposes us to some DOS attacks
• conditionally adding Access-Control-Allow-Origin response header if it doesnt exist --- seems rather clunky since these CORS headers are more intricate than that and I don't pretend to know all the subtle nuances of how these headers work

I think we might be able to disable the nginx max_size check in conjunction with a vets-api change that enforces a max_size check. I think this enforcement can be done with Carrierwave which vets-api already uses. That way, Rack::Cors would gracefully handle all response CORS headers and as an added bonus - the response body would actually be in JSON instead of HTML (which is what nginx returns with its 413)

Another possibility that I know a lot less about, would be to have the front-end JS implement some kind of uploading chunking/slicing to keep each upload under the limit. That's a bit outside my expertise and would need some additional input.

[omgitsbillryan]

I hacked this together ☝️ which really only fixes the error messaging. I think we need a more comprehensive solution to allow for large uploads while also not exposing us to DOS attacks.

[omgitsbillryan]

Today's my last day - we should probably just close this as "Will not fix".

I wasn't confident enough in my bandaid PR that was never merged. It may have ended up causing more problems than it solved. The real solution to this problem is the Epic that I linked to where we have a centralized / global solution for handling file uploads.