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

Missing CORS headers for DELETE in API v2 #3165

Open
ribetm opened this issue Dec 6, 2022 · 2 comments
Open

Missing CORS headers for DELETE in API v2 #3165

ribetm opened this issue Dec 6, 2022 · 2 comments

Comments

@ribetm
Copy link

ribetm commented Dec 6, 2022

What did you do?

  • Setup Karma and AlertManager v0.24.0 over SSL on different domains
  • Create a silence in Karma
  • Try to delete it from Karma

What did you expect to see?

  • A successful response from AlertManager
  • The deletion of the desired silence

What did you see instead? Under which circumstances?

  • No CORS headers in the responses to either the preflight request OPTIONS and the actual DELETE
  • Network failure as the browser blocks the DELETE request, since the preflight request failed
  • Silence not deleted

It seems like the CORS policy of API v1 allowed GET, POST, DELETE, OPTIONS, while API v2 uses rs/cors default configuration, which only allows GET, POST, HEAD.

Workaround
Add missing headers in Nginx

  location /api/v2 {
    proxy_pass http://alertmanager;
    if ($request_method = DELETE ) {
      add_header Access-Control-Allow-Methods "DELETE";
      add_header Access-Control-Allow-Origin "https://karma";
    }
    if ($http_access_control_request_method = DELETE ) {
      add_header Access-Control-Allow-Methods "DELETE";
      add_header Access-Control-Allow-Origin "https://karma";
    }
  }

Environment

  • System information:

    Linux 5.4.0-48-generic x86_6

  • Alertmanager version:

alertmanager, version 0.24.0 (branch: HEAD, revision: f484b17fa3c583ed1b2c8bbcec20ba1db2aa5f11)
  build user:       root@265f14f5c6fc
  build date:       20220325-09:31:33
  go version:       go1.17.8
  platform:         linux/amd64
  • Prometheus version:
prometheus, version 2.29.2 (branch: HEAD, revision: 752c4f11ae86effa9a46f017f2feb66730c67ed8)
  build user:       root@61bcc9848ade
  build date:       20210827-09:44:22
  go version:       go1.16.7
  platform:         linux/amd64
@alexweav
Copy link
Contributor

Unlike the v1 API, v2 instead uses rs/cors to manage CORS headers. V1 blindly sends all CORS headers all the time, where v2 only sends the minimum required set of headers.

When I set an Origin header in my request, I indeed get the (minimum) subset of required headers and things work correctly.

└──>curl -s -v -H "Origin: http://localhost:9093" localhost:9093/api/v2/silences > /dev/null
*   Trying 127.0.0.1:9093...
* Connected to localhost (127.0.0.1) port 9093 (#0)
> GET /api/v2/silences HTTP/1.1
> Host: localhost:9093
> User-Agent: curl/7.87.0
> Accept: */*
> Origin: http://localhost:9093
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Access-Control-Allow-Origin: *
< Content-Type: application/json
< Vary: Origin
< Date: Wed, 28 Dec 2022 06:13:20 GMT
< Content-Length: 3
<
{ [3 bytes data]
* Connection #0 to host localhost left intact

It appears that the browser must send the Origin header for CORS to work properly. I am not an expert in this area, but most documentation that I can find indicates that the expectation is on the browser to include that header. See: rs/cors#93 (comment)

I believe your client-side application needs to send that header, and it seems that the server's behavior is correct. It only happened to work in v1 by chance, because v1 blindly sent all CORS headers on all responses.

@bramotten
Copy link

bramotten commented Oct 9, 2023

What is the status of this? As strongly hinted at by ribetm and hloeung in issue #3175, a naive fix is to replace https://github.com/prometheus/alertmanager/blob/main/api/v2/api.go#L132

handleCORS := cors.Default().Handler

by

handleCORS := cors.New(cors.Options{
	AllowedMethods: []string{http.MethodGet, http.MethodPost, http.MethodHead, http.MethodDelete},
}).Handler

since cors.Default() is just cors.New(Options{}) and implicitly lacks http.MethodDelete. To be clear, that is a problem because api/v2 allows deleting a silence by ID via the DELETE method.

As for whether there is a (different?) problem client-side that has to do with an Origin header... explicitly including this header does not seem to be enough to make DELETEs work through the Fetch API in browsers, where GETs just work. Everything also works with this browser client after a make build test with the little cors.New(...) fix above.

But it might be possible to pull some kind of trick, since the following somehow works even with cors.Default():

$ curl -v -X DELETE http://xxx:9093/api/v2/silence/5447de2d-c213-
4b8e-8d9e-12dbfc0b1106
*   Trying xxx:9093...
* Connected to xxx (xxx) port 9093
> DELETE /api/v2/silence/5447de2d-c213-4b8e-8d9e-12dbfc0b1106 HTTP/1.1
> Host: xxx
> User-Agent: curl/8.3.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Vary: Origin
< Date: Mon, 09 Oct 2023 09:26:53 GMT
< Content-Length: 0
< 
* Connection #0 to host xxx left intact

where xxx is a "remote" IPv4 and 5447de... the ID of an active silence. Note that curl does not require an explicit Origin header either.

bramotten pushed a commit to bramotten/alertmanager that referenced this issue Oct 12, 2023
bramotten pushed a commit to bramotten/alertmanager that referenced this issue Oct 12, 2023
Signed-off-by: Bram <bram@ii/nl>
bramotten pushed a commit to bramotten/alertmanager that referenced this issue Oct 12, 2023
Signed-off-by: Bram Otten <hcotten@pm.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants