FoxxMD / multi-scrobbler

Scrobble plays from multiple sources to multiple clients
https://foxxmd.github.io/multi-scrobbler
MIT License
346 stars 16 forks source link

Incorrect port when BASE_URL contains protocol's default port #155

Closed pedorich-n closed 2 months ago

pedorich-n commented 3 months ago

Describe the bug I am running the multi-scrobbler behind the reverse proxy on the address http://multi-scrobbler.server.lan and specify the same address in BASE_URL (also tried http://multi-scrobbler.server.lan:80).

Two issues arise:

  1. Because URL trims the default protocol's port (80 for HTTP and 443 for HTTPS), MS assumes I didn't specify the port and tries to use its default port for the base url instead. Which is not correct in my case: https://github.com/FoxxMD/multi-scrobbler/blob/0dcd049453fcbb0e8cadfe8978f5d822b069ce1d/src/backend/ioc.ts#L74-L79
  2. The items.mainPort in the code mentioned above doesn't exist, so I ended up with http://multi-scrobbler.server.lan:undefined as a base url for redirects. Looks like this commit caused this issue.

Expected behavior Use the default port when specified.

Logs

[App] [API] User-defined base URL for UI and redirect URLs (spotify, deezer, lastfm): http://multi-scrobbler.server.lan:undefined

Versions (please complete the following information):

Additional context The port gets lost after normalization because the normalize-url also uses URL.

I think the code should either assume the port is 80 when not specified, as it seems to be the behavior of URL, or at least respect the explicitly specified port even when it's the default.

The current workaround for me is to explicitly specify the redirect URLs for sources.

FoxxMD commented 3 months ago

Thanks for catching this and the detailed write up!

FoxxMD commented 3 months ago

@pedorich-n please try foxxmd/multi-scrobbler:develop docker image and let me know if that fixes URL issues for you

pedorich-n commented 3 months ago

Thanks for such a quick fix! The URL does appear to be correct now, with the BASE_URL=http://multi-scrobbler.server.lan:80:

[App] [API] User-defined base URL for UI and redirect URLs (spotify, deezer, lastfm): http://multi-scrobbler.server.lan/
...
[App] [Sources] [Spotify - spotify] Redirect URL that will be used on auth callback: 'http://multi-scrobbler.server.lan/callback'

But I am getting a 400 HTTP code from MS when trying to authorize Spotify now. I don't know if that's something on MS's side or my reverse proxy or what... The URL I see is http://multi-scrobbler.server.lan/api/callback?code=<code>&state=spotify. Where did the /api/ come from? 🤔

In the logs, I can see

[2024-06-05 20:07:04.265 +0900] INFO   : [App] [API] Received auth code callback from Spotify
[2024-06-05 20:07:02.364 +0900] INFO   : [App] [API] Redirecting to spotify authorization URL

But the page still shows "Auth Interaction Required" and no currentCreds-spotify.json file is created.


Edit: Upon rolling back to 0.7.1 and using the explicit redirectUri everything works again. So I assume something changed in the dev branch. 🤔 Also I can see /api/ in URL too, so I guess that's not the problem.

FoxxMD commented 3 months ago

Oops! Missed a string conversion of the base url when passing to the spotify api. Try the newest develop tag, it should be fixed now.

pedorich-n commented 3 months ago

Yes, it works now! Thank you for the fix!

FoxxMD commented 2 months ago

Released in v0.8.0