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

Remove VITE_CALDERA_URL and use window.location.origin to route API requests #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daw1012345
Copy link

@daw1012345 daw1012345 commented Dec 26, 2024

Use window.location.origin to decide where to route API requests.

VITE_CALDERA_URL adds complexity with no clear benefit. It is very easy to misconfigure (especially when magma was built outside of container, the .env file was changed, and then the Docker container was built). When it is misconfigured, there is no error and the UI just seems unresponsive.

Due to CORS and tight coupling between caldera and magma, it is very unlikely that the URL is going to be anything other than the value held in window.location.origin. In all other scenarios, a reverse proxy should be used.

Use window.location.origin to decide where to route API requests. VITE_CALDERA_URL added more complexity with little benefit
@rfulwell
Copy link

Excellent changes! 🚀

@rfulwell
Copy link

I was able to confirm this fix by cloning these changes from @daw1012345 into my local caldera/plugins/magma repo.
I checked that the server allowed valid logins at both localhost and 127.0.0.1 with no changes to the server. This was the simplest case that I found to demonstrate the broken logins previously. I verified the fix both with and without --insecure on the command line (--fresh was required to reset the saved server state).

@elegantmoose These changes will resolve a number of issues and discussions in the caldera repo and generally make the project much easier to work with.

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.

2 participants