search

cofacts / rumors-api

GraphQL API server for clients like rumors-site and rumors-line-bot
https://api.cofacts.tw
MIT License
109 stars 26 forks source link

Redirect user to domains in RUMORS_SITE_REDIRECT_ORIGIN to fix login issue #251

Closed MrOrz closed 3 years ago

MrOrz commented 3 years ago

This PR fixes #250

Current situation

On Mac & iOS safari, users cannot login when they visit https://cofacts.org.

This is because the current login mechanism will write cookie to https://cofacts-api.g0v.tw.

Even though https://cofacts-api.g0v.tw have CORS enabled and also uses sames-site: lax, it is possibly that browsers are blocking all third-party cookies anyway.

What this PR do

We redirect users to domains listed in RUMORS_SITE_REDIRECT_ORIGIN.

After we apply corresponding changes in rumors-deploy ( cofacts/rumors-deploy#18 ), we can always redirect users to https://cofacts.tw or https://en.cofacts.tw, depending on which site the user is visiting.

We also have better error handing for URL concatenation errors. Since the website always connects to https://api.cofacts.tw, and https://cofacts.tw & https://api.cofacts.tw are considered same-site, the browser can pickup the login session cookie and shows as logged-in.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.4%) to 86.295% when pulling 67bf7edaf6bb35fc6e552e54ad3cd19b7be65f48 on redirect into b887a4fb3ae968e0b1e248cc32099f80ad812353 on master.