Skip to content

Change the default port to 8090#89

Closed
erickoledadevrel wants to merge 1 commit intomasterfrom
port
Closed

Change the default port to 8090#89
erickoledadevrel wants to merge 1 commit intomasterfrom
port

Conversation

@erickoledadevrel
Copy link
Copy Markdown

The default port 8080 is more likely to be in use, and the library doesn't handle that case gracefully. Fixes #86.

@erickoledadevrel erickoledadevrel requested a review from sqrrrl March 7, 2019 02:12
@sqrrrl
Copy link
Copy Markdown
Member

sqrrrl commented Mar 7, 2019

This doesn't fix anything. Port conflicts will still happen any time a port is hardcoded like that.

The correct fix is to make a fix in the auth library. You can pass in port 0 which instructs the OS to pick an unused ephemeral port. That should be the default, and for localhost redirect URIs the port is always ignored for this reason -- to allow random ports to be used at runtime.

Anyway, it looks like you can pass port 0 in. The only bug is the auth library won't create the correct redirect URL since it creates the URL before the server port is known.

https://github.com/GoogleCloudPlatform/google-auth-library-python-oauthlib/blob/master/google_auth_oauthlib/flow.py#L401

Switching the order of operations and using local_server.port as the port value would fix this for real.

@sqrrrl
Copy link
Copy Markdown
Member

sqrrrl commented Mar 7, 2019

Sent a PR to fix this in the auth lib. Once that's in, and assuming we don't make it the default, then the fix here would be to upgrade to that version and then use port 0.

@erickoledadevrel
Copy link
Copy Markdown
Author

Agreed this change doesn't fix the problem in all cases, just made the situation somewhat more rare. Your solution is much better, and not too far off, so I'll close this PR.

@erickoledadevrel erickoledadevrel deleted the port branch March 8, 2019 02:16
@sqrrrl sqrrrl restored the port branch March 8, 2019 02:16
@sqrrrl
Copy link
Copy Markdown
Member

sqrrrl commented Mar 8, 2019

Let's keep open. Even with my fix, we still probably need to change the port number to take advantage of it.

@erickoledadevrel
Copy link
Copy Markdown
Author

This PR was just a simple find and replace over the codebase. Changing the port to 0 is just as much work as starting from scratch.

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