Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch unauthenticated POST requests to the proxy and return an error instead of redirecting to vs code #2128

Closed
janus-reith opened this issue Sep 22, 2020 · 14 comments
Assignees
Milestone

Comments

@janus-reith
Copy link

I'm trying to proxy POST calls for a graphql api running on my dev instance, but when I do POST requests, I always recieve:
{"error":"Unsupported method POST"}
Visiting pages / GET requests work fine.

I assume this to be a restriction in the proxy, but didn't take a deeper look yet.

@marissa999
Copy link

You could maybe try the same thing by accessing code-server directly and not through a reverse proxy.

What do the browser devtools (the network tab especially) say about the POST requests?

https://github.com/cdr/code-server/blob/v3.5.0/doc/guide.md#nginx
I just quickly looked at the reverse proxy example. And I am by far not an expert when it comes to nginx configuration/reverse proxy stuff. In fact I just spent way too much time with a reverse proxy for synclounge a few days ago and had problems with websockets/socket.io. For me the problem was the / at the end of the proxy_pass right before the semicolon. Removing the / apparently changes how the nginx reverse proxy forwards requests?! But the / seems to exist in the example config here, maybe it might help to remove it?

@janus-reith
Copy link
Author

Thanks for you input on this, I just tried to leave away my reverse proxy, and directly access http://INTERNALIP:8080/proxy/3000/graphql so no other reverse proxy should be involved, except the one provided as part of code-server.
Result is the same, I recieve a Status 400 and {"error":"Unsupported method POST"}

I know for sure that the express + apollo server listening on port 3000 on that host machine wouldn't have any issues to parse POST requests.
I also had the thought if authorization might get in the way, but I checked and my POST requests all include the key Cookie from signin, so that should not be an issue.

I just looked it up in the code, and it's probably related to this:
https://github.com/cdr/code-server/blob/b509063e143bbf74b74ec295260c4fd5f6332f71/src/node/http.ts#L282L287
So far I didn't dive deeper to find the connection between ProxyHttpProvider and ensureMethod.

All ProxyHttpProvider seems to do is to check authentication and make sure there is a trailing slash, or in successful cases, return that object:

proxy: {
        strip: `${route.providerBase}/${port}`,
        port,
},

An external library, http-proxy seems to be used to intercept those returns in HttpProvider: https://github.com/cdr/code-server/blob/b509063e143bbf74b74ec295260c4fd5f6332f71/src/node/http.ts

ensureMethod seems to be only used in login.ts, static.ts, update.ts and vscode.ts, but I didn't find out yet how the proxy relates to one of these.

@code-asher
Copy link
Member

What request is giving the 400 and unsupported method? My guess is it's trying to make a request against the root instead of against /proxy/3000, maybe because it's not aware it's underneath a sub-path.

@janus-reith
Copy link
Author

@code-asher I don't think so, I'm using the proxied subdomain feature,
so Codeserver runs on e..g some.host.com

So in my case, running GraphQL Playground works fine and comes up on:
https://3000.some.host.com/graphql (Comes up on GET)
but I'm unable to send POSTs to it as the code server proxy seems to intercept that.

Inspecting the network tab, they seem to go right where they are supposed to,
So the subpath doesn't seem to be the issue here.

@code-asher
Copy link
Member

Aaahh I only tested the sub-path proxy method. I'll test the sub-domain method and see if I can reproduce.

@code-asher
Copy link
Member

Hmm I haven't had luck replicating so far. Here's what I did to test (all local):

Ran httpbin which lets you test various HTTP requests:

$ docker run -p 8081:80 kennethreitz/httpbin

Started up code-server:

$ code-server --auth none --proxy-domain <domain>

In my case <domain> is localhost.

Then ran a POST request through code-server's proxy (this can also be done from httpbin's web interface):

$ curl -X POST "http://8081.<domain>/post"  -d '{test: 1}' -H "Content-Type: application/json"
{
  ...
  "data": "{test: 1}", 
  ...
}

I'll give graphql a try as well, just need to get familiar with it a bit so I can get it running.

@code-asher
Copy link
Member

I loaded up some data and ran a query and it appears to be working. I tried both the playground interface and cURL:

$ curl 'http://8082.<domain>/' -H 'content-type: application/json' -X POST --data-raw '{"operationName":null,"variables":{},"query":"{ users { id } }"}'
{"data":{"users":[{"id":"1"}]}}

I used this example: https://codesandbox.io/s/node-apollo-graphql-7sgx4 and ran it locally on port 8082 then accessed it through code-server's proxy. Any idea on how this might be different from your setup?

@janus-reith
Copy link
Author

@code-asher Thanks for testing, I‘ll do some more testing aswell with other containers maybe.
One difference I see is, that I‘m running with authentication enabled.

@code-asher
Copy link
Member

Ah yeah good point, I'll give it a go with auth on.

@code-asher
Copy link
Member

I'm still able to POST with httpbin but when I load up the graphql playground I get the unspported method message!

When unauthenticated we skip the proxy so it'll try to load VS Code instead which doesn't support POST. We should make this return an unauthorized error.

As for why it fails, it looks like the default for credentials is omit so I was able to fix it by configuring the Apollo server:

const server = new ApolloServer({
  typeDefs,
  resolvers,
  playground: {
    settings: {
      "request.credentials": "same-origin"
    }
  }
});

Relevant docs: https://www.apollographql.com/docs/apollo-server/testing/graphql-playground/#configuring-playground

@code-asher
Copy link
Member

That may or may not be helpful for you though since you mentioned your POST requests are already including the cookie. In that case I think there must be some other reason code-server isn't properly authenticating these requests.

@janus-reith janus-reith changed the title POST support for proxy Catch unauthenticated POST requests to the proxy and return an error instead of redirecting to vs code Oct 5, 2020
@janus-reith
Copy link
Author

janus-reith commented Oct 5, 2020

@code-asher Thanks for taking the time to investigate this!
I had another look, and the issue indeed was the missing cookie on these POST request, sorry for the confusion.
(I wonder what went wrong when I checked the Network requests, I guess I looked at the same-named GET for the graphql document, which certainly has the Cookie with key included, probably out of habit as usually I look out for "graphql" as a POST and rarely debug network calls from inside GraphQL Playground ).

So the only issue seems to be the wrong message that is returned, I updated the title to reflect that.
Again, thanks for having a look!

@code-asher code-asher added this to the v3.6.0 milestone Oct 7, 2020
@code-asher
Copy link
Member

Huh, turns out I had already fixed this in master. It'll be in the 3.6.0 release!!

@code-asher
Copy link
Member

code-asher commented Oct 7, 2020

Actually although it properly shows the authentication error now it still needs to be a bit smarter and redirect to the login page when the root page is requested if necessary.

@code-asher code-asher reopened this Oct 7, 2020
@code-asher code-asher self-assigned this Oct 7, 2020
@code-asher code-asher linked a pull request Oct 9, 2020 that will close this issue
@nhooyr nhooyr modified the milestones: v3.6.0, v3.6.1 Oct 12, 2020
@nhooyr nhooyr modified the milestones: v3.6.1, v3.6.2 Oct 21, 2020
@nhooyr nhooyr mentioned this issue Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants