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

removing incorrect CORS headers #4005

Merged
merged 1 commit into from
Apr 12, 2023
Merged

Conversation

DenysGonchar
Copy link
Collaborator

There are multiple things that are wrong with that admin API Access-Control-Allow-* headers, also we do not support it for GraphQL nor client REST API.

Removing them for now, if we want to add CORS support in the future it must be done properly and for GraphQL and
client REST API as well admin REST API.

Issue can be easily demonstrated with curl, below is the corresponding part of the config:

[[listen.http]]
  port = 8088
  transport.num_acceptors = 10
  transport.max_connections = 1024
  tls.verify_mode = "none"
  tls.certfile = "priv/ssl/fake_cert.pem"
  tls.keyfile = "priv/ssl/fake_key.pem"
  
  [[listen.http.handlers.mongoose_admin_api]]
    host = "_"
    path = "/api"

and here are 2 curl commands:

  • HTTP1.1 request: curl -v --http1.1 --insecure -X OPTIONS https://localhost:8088/api/users/localhost
  • HTTP2 request: curl -v --http2 --insecure -X OPTIONS https://localhost:8088/api/users/localhost

Notice that HTTP2 query fails with existing CORS headers.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 9, 2023

small_tests_24 / small_tests / 6732535
Reports root / small


small_tests_25 / small_tests / 6732535
Reports root / small


small_tests_25_arm64 / small_tests / 6732535
Reports root / small


ldap_mnesia_24 / ldap_mnesia / 6732535
Reports root/ big
OK: 2219 / Failed: 0 / User-skipped: 827 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 6732535
Reports root/ big
OK: 4180 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


ldap_mnesia_25 / ldap_mnesia / 6732535
Reports root/ big
OK: 2219 / Failed: 0 / User-skipped: 827 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 6732535
Reports root/ big
OK: 4180 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 6732535
Reports root/ big
OK: 4553 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


dynamic_domains_mysql_redis_25 / mysql_redis / 6732535
Reports root/ big
OK: 4154 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 6732535
Reports root/ big
OK: 4177 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0


internal_mnesia_25 / internal_mnesia / 6732535
Reports root/ big
OK: 2361 / Failed: 0 / User-skipped: 685 / Auto-skipped: 0


elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 6732535
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 664 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / 6732535
Reports root/ big
OK: 4539 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / 6732535
Reports root/ big
OK: 4550 / Failed: 0 / User-skipped: 100 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / 6732535
Reports root/ big
OK: 4553 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

Copy link
Contributor

@arcusfelis arcusfelis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@arcusfelis arcusfelis merged commit be964c5 into master Apr 12, 2023
2 checks passed
@arcusfelis arcusfelis deleted the removing-incorrect-CORS-headers branch April 12, 2023 09:55
@jacekwegr jacekwegr added this to the 6.1.0 milestone Apr 26, 2023
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 this pull request may close these issues.

None yet

4 participants