Allow OPTIONS requests through domain proxy

[helgehatt]

Fixes #7283

I believe the fix is as simple as allowing OPTIONS requests to pass through the proxy without authentication. Then the service behind the proxy should respond with 200 or 204.

[code-asher]

This is probably OK, but something about letting things through the proxy to the app without auth is making me nervous. Would it work if we just respond with a 204 here?

[code-asher]

Also thank you for working on this!

[code-asher]

And I suppose we will need to do the same for the path-based proxy.

[code-asher]

Actually, I suppose we have to let it through because it could be a 404 or something else, no way to know for sure that particular preflight will actually be a 204.

Will approve once we add it to the path proxy as well!

[code-asher]

Sorry to keep going back and forth :sweat_smile:

But I was thinking if we do pass it through, that exposes information (which ports have things on them) and maybe we should just hardcode a 204 after all...trying to see what other things do like oauth2-proxy to see if there is some standard. edit seems they have a skip-auth-preflight flag?

[helgehatt]

What do you think is the right direction here? A skip-auth-preflight flag is definitely an option, but hardcoding adds a layer of security by obscurity. However, hardcoding 204 kind of defeats the purpose of preflighting requests.

[code-asher]

hardcoding 204 kind of defeats the purpose of preflighting requests

Yeah, you are right, but the attack surface is large since we automatically forward every port, so it makes me nervous. A hardcoded response would eliminate the attack vector, and would probably be good enough for development?

But, I think for now my vote would be to gate this behind a skip-auth-preflight flag set to false by default. Or I suppose it could take a list of port numbers.

What do you think?

[helgehatt]

We can start off with a boolean skip-auth-preflight flag which can later be extended with a list of ports or a hardcoded mode?

[code-asher]

That sounds good to me!

[helgehatt]

@code-asher Have a look and see if these changes make sense to you.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 73.01%. Comparing base (4b7bca3) to head (88ff214). Report is 17 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node/routes/domainProxy.ts	0.00%	1 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7284      +/-   ##
==========================================
+ Coverage   72.94%   73.01%   +0.07%     
==========================================
  Files          29       29              
  Lines        1785     1790       +5     
  Branches      380      383       +3     
==========================================
+ Hits         1302     1307       +5     
  Misses        408      408              
  Partials       75       75              

Files with missing lines	Coverage Δ
src/node/cli.ts	90.90% <ø> (ø)
src/node/main.ts	49.10% <100.00%> (+0.92%)	:arrow_up:
src/node/routes/pathProxy.ts	89.65% <100.00%> (+7.51%)	:arrow_up:
src/node/routes/domainProxy.ts	47.27% <0.00%> (-1.79%)	:arrow_down:

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e2c489d...88ff214. Read the comment docs.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.