Skip to content

Improve oauth redirect #303

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

Merged
merged 3 commits into from
Jul 4, 2025
Merged

Conversation

k3-cat
Copy link
Contributor

@k3-cat k3-cat commented Jun 29, 2025

Background & Aim

I have migrated my existing instance from docker to podman and forget to set the RUSTDESK_API_RUSTAPI_API_SERVER. It still works fine until I tried to login via oauth and it brings me to 127.0.0.1:21114/xxxxx.

I think, accroading to web standers, the host part can be omited when redirect amount same host. So, my initial idead is just remove every host parts from the redirect url. However, I find it can be further improved when reading relavent codes.

Changes & Reasons

  • Align all redirects or endpoint in responses with hostname in requests header (if possible)
  • Remove RedirectURL from oauth config. As they are only checked, for security reasons, by the oauth provider for security reasons. Since the pervious change has already relaxed the needing of this field when generating responses, removing it can improve the overall robustness -- no need to chage a same setting multiple times at different configs.
  • And thus, increase the db version by 1 (263 now)

@k3-cat k3-cat force-pushed the fix-oauth-redirect branch from c17a0d2 to db03f59 Compare June 29, 2025 09:22
@IamTaoChen
Copy link
Contributor

If so, we also need to update the fronted web. The redirectUrl should display the reult(http(s)://xxx.xxx.xxx/_admin/xxxxx)

image

@k3-cat
Copy link
Contributor Author

k3-cat commented Jun 30, 2025

Yeah! You are right.
I totally forgot about the front end.

However, I will be quite busy this week... So maybe I'll fix it over the weekend if I get the work done.

@lejianwen lejianwen merged commit c52706e into lejianwen:master Jul 4, 2025
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.

3 participants