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

Readiness logic cannot access APIs when custom allowed origins are set #702

Closed
lukashornych opened this issue Oct 21, 2024 · 1 comment · Fixed by #704
Closed

Readiness logic cannot access APIs when custom allowed origins are set #702

lukashornych opened this issue Oct 21, 2024 · 1 comment · Fixed by #704
Assignees
Labels
breaking change Backward incompatible data model change bug Something isn't working

Comments

@lukashornych
Copy link
Collaborator

lukashornych commented Oct 21, 2024

When an API has custom allowedOrigins list defined in configuration, the internal readiness logic cannot access the API, because CorsFilter cancels the request because the readiness logic doesn't send any origin header, thus it is not allowed.

We need to send origin header from the readiness logic if any allowed origin is specified.

@lukashornych lukashornych self-assigned this Oct 21, 2024
@lukashornych lukashornych added the bug Something isn't working label Oct 21, 2024
lukashornych added a commit that referenced this issue Oct 22, 2024
…ess-apis-when-custom-allowed-origins-are-set

fix(#702): Readiness logic cannot access APIs when custom allowed origins are set
lukashornych added a commit that referenced this issue Oct 22, 2024
…ess-apis-when-custom-allowed-origins-are-set

 test(#702): readiness with custom origins tests
@lukashornych lukashornych added the breaking change Backward incompatible data model change label Oct 22, 2024
lukashornych added a commit that referenced this issue Oct 22, 2024
…orrect implementation, removed option to specify custom allowed origins
lukashornych added a commit that referenced this issue Oct 22, 2024
…ess-apis-when-custom-allowed-origins-are-set

fix(#702): reimplemented CORS filter and preflight handler for more correct implementation, removed option to specify custom allowed origins
@lukashornych
Copy link
Collaborator Author

lukashornych commented Oct 22, 2024

After lots of discussion, we've come to conclusion that custom allowed origins will no longer be supported by evita, instead * wildcard will be used for Access-Control-Allow-Origin at all times. This is because:

  • we support accessing all APIs through lab url and, thus we would have to dynamically merge allowedOrigins from all configs, which is lots of added complexity for feature that is not used
    • current implementation wasn't entirely correct, it required specifying same allowedOrigins for all APIs
  • evitaDB is database which will be exposed only in internal network most of the times, therefore it doesn't make sense to maintain such complexity for such edge case
  • more control can be added by placing nginx before evitaDB with custom CORS logic

Also the CorsFilter and CorsPreflightHandler were incorrectly implement (cause of the original issue) and were reimplemented into custom CorsService based on several resources on how a server should handle CORS for browser.

lukashornych added a commit that referenced this issue Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Backward incompatible data model change bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant