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

fix: api external url should be for gotrue, not appflowy cloud #1128

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

khorshuheng
Copy link
Collaborator

@khorshuheng khorshuheng commented Jan 5, 2025

API_EXTERNAL_URL is meant for the external endpoint for gotrue, not appflowy cloud. Not having gotrue suffix will cause SAML integration to fail.

@khorshuheng khorshuheng force-pushed the fix-api-external-url branch 2 times, most recently from 85973c8 to adae05f Compare January 5, 2025 13:51
@speed2exe
Copy link
Collaborator

speed2exe commented Jan 6, 2025

If we want to remove gotrue prefix, we need also to modify nginx routing so that every supported path in gotrue service is routed properly. This will also break the dev env (AppFlowyCloud Develop) in appflowy flutter app if we also apply the changes to docker-compose-dev.yml.
In addition, documentations in doc/AUTHENTICATION.md will also need to be updated.

There might also be other issues, these are the concerns I can think of at the moment.

EDIT: did not see this line API_EXTERNAL_URL=http://your-host/gotrue, looks good to me

@khorshuheng khorshuheng force-pushed the fix-api-external-url branch from adae05f to 91a1fa9 Compare January 6, 2025 02:36
@khorshuheng khorshuheng force-pushed the fix-api-external-url branch from 91a1fa9 to f8e79cc Compare January 6, 2025 03:10
@khorshuheng khorshuheng merged commit cd10010 into main Jan 6, 2025
6 checks passed
@khorshuheng khorshuheng deleted the fix-api-external-url branch January 6, 2025 05:58
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