Skip to content

Commit 7054d62

Browse files
authored
fix: Clean up local server socket on error (#339)
This resolves https://togithub.com/googleapis/google-auth-library-python-oauthlib/issues/338. Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/google-auth-library-python-oauthlib/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) Fixes #<issue_number_goes_here> 🦕
1 parent c4d754c commit 7054d62

File tree

2 files changed

+42
-25
lines changed

2 files changed

+42
-25
lines changed

google_auth_oauthlib/flow.py

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -433,31 +433,33 @@ def run_local_server(
433433
bind_addr or host, port, wsgi_app, handler_class=_WSGIRequestHandler
434434
)
435435

436-
redirect_uri_format = (
437-
"http://{}:{}/" if redirect_uri_trailing_slash else "http://{}:{}"
438-
)
439-
self.redirect_uri = redirect_uri_format.format(host, local_server.server_port)
440-
auth_url, _ = self.authorization_url(**kwargs)
441-
442-
if open_browser:
443-
# if browser is None it defaults to default browser
444-
webbrowser.get(browser).open(auth_url, new=1, autoraise=True)
445-
446-
if authorization_prompt_message:
447-
print(authorization_prompt_message.format(url=auth_url))
448-
449-
local_server.timeout = timeout_seconds
450-
local_server.handle_request()
451-
452-
# Note: using https here because oauthlib is very picky that
453-
# OAuth 2.0 should only occur over https.
454-
authorization_response = wsgi_app.last_request_uri.replace("http", "https")
455-
self.fetch_token(
456-
authorization_response=authorization_response, audience=token_audience
457-
)
458-
459-
# This closes the socket
460-
local_server.server_close()
436+
try:
437+
redirect_uri_format = (
438+
"http://{}:{}/" if redirect_uri_trailing_slash else "http://{}:{}"
439+
)
440+
self.redirect_uri = redirect_uri_format.format(
441+
host, local_server.server_port
442+
)
443+
auth_url, _ = self.authorization_url(**kwargs)
444+
445+
if open_browser:
446+
# if browser is None it defaults to default browser
447+
webbrowser.get(browser).open(auth_url, new=1, autoraise=True)
448+
449+
if authorization_prompt_message:
450+
print(authorization_prompt_message.format(url=auth_url))
451+
452+
local_server.timeout = timeout_seconds
453+
local_server.handle_request()
454+
455+
# Note: using https here because oauthlib is very picky that
456+
# OAuth 2.0 should only occur over https.
457+
authorization_response = wsgi_app.last_request_uri.replace("http", "https")
458+
self.fetch_token(
459+
authorization_response=authorization_response, audience=token_audience
460+
)
461+
finally:
462+
local_server.server_close()
461463

462464
return self.credentials
463465

tests/unit/test_flow.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import pytest
2626
import requests
2727
import urllib
28+
import webbrowser
2829

2930
from google_auth_oauthlib import flow
3031

@@ -440,3 +441,17 @@ def test_run_local_server_occupied_port(
440441
with pytest.raises(OSError) as exc:
441442
instance.run_local_server(port=port)
442443
assert "address already in use" in exc.strerror.lower()
444+
445+
@mock.patch("google_auth_oauthlib.flow.webbrowser.get", autospec=True)
446+
@mock.patch("wsgiref.simple_server.make_server", autospec=True)
447+
def test_local_server_socket_cleanup(
448+
self, make_server_mock, webbrowser_mock, instance
449+
):
450+
server_mock = mock.MagicMock()
451+
make_server_mock.return_value = server_mock
452+
webbrowser_mock.side_effect = webbrowser.Error("Browser not found")
453+
454+
with pytest.raises(webbrowser.Error):
455+
instance.run_local_server()
456+
457+
server_mock.server_close.assert_called_once()

0 commit comments

Comments
 (0)