feat: add auth_port configuration option #1657
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hrkfdn
reviewed
Jun 15, 2025
There was a problem hiding this comment.
Thanks! I was wondering whether the port selection should happen automatically? Another thing I was wondering about whether freely choosing a port is OK? When registering an API client, the user needs to specify a callback URL and I'm uncertain if we can freely choose the port. Did you test this?
src/authentication.rs
Outdated
Comment on lines
37
to
41
| pub fn get_client_redirect_uri(configuration: &Config) -> Result<String, String> { | ||
| let auth_port = configuration.values().auth_port.unwrap_or(8989).to_string(); | ||
| let redirect_url = format!("http://127.0.0.1:{auth_port}/login"); | ||
| Ok(redirect_url) | ||
| } |
There was a problem hiding this comment.
Ins 8000 tead of configuring the port, should ncspot pick a free port automatically maybe?
There was a problem hiding this comment.
I tried that, if we remove the port it gives back a prompt where you enter the URL manually:
let redirect_url = format!("http://127.0.0.1/login");Browse to: https://accounts.spotify.com/authorize?response_type=code&client_id=65b708073fc0480ea92a077233ca87bd&state=TiFVv25QeINX7InxhQjIqA&code_challenge=QLMUWfMSejnKrBxkpqIZu-2RA3EScy25c8xZ1OXmp10&code_challenge_method=S256&redirect_uri=http%3A%2F%2F127.0.0.1%2Flogin&scope=playlist-modify+playlist-modify-private+playlist-modify-public+playlist-read+playlist-read-collaborative+playlist-read-private+streaming+user-follow-modify+user-follow-read+user-library-modify+user-library-read+user-modify+user-modify-playback-state+user-modify-private+user-personalized+user-read-currently-playing+user-read-email+user-read-play-history+user-read-playback-position+user-read-playback-state+user-read-private+user-read-recently-played+user-top-read
Provide redirect URL
hi
Unable to parse redirect URI hi (relative URL without a base)There was a problem hiding this comment.
Ah, what I meant was have ncspot pick a value for auth_port automatically.
Signed-off-by: Elias Assaf <elyas51000@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Add a configuration option to set the authenticate port, by default its 8989
Issue ticket number and link
#1637
Checklist before requesting a review
not performance improvements, etc.)