Skip to content
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

fix: fix socket leak in impersonated_credentials #1123

Conversation

stewartmiles
Copy link
Contributor

impersonated_credentials.Credentials.sign_bytes() created
a session that wasn't closed leaking a socket. This fixes
the issue by always closing the requests session after
a signing request is complete.

Fixes #1122

impersonated_credentials.Credentials.sign_bytes() created
a session that wasn't closed leaking a socket. This fixes
the issue by always closing the requests session after
a signing request is complete.

Fixes googleapis#1122
@stewartmiles stewartmiles force-pushed the fix_socket_leak_in_impersonated_credentials branch from ac64264 to 2dcb6d3 Compare August 26, 2022 20:55
@parthea parthea added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Aug 27, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 27, 2022
@parthea parthea changed the title Fix socket leak in impersonated_credentials fix: fix socket leak in impersonated_credentials Aug 27, 2022
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 27, 2022
@stewartmiles
Copy link
Contributor Author

The failing presubmit looks like nothing to do with my change https://source.cloud.google.com/results/invocations/29f9a2fd-8d3a-4251-88f0-4598f6910f6b/targets/github%2Fgoogle-auth-library-python%2Fsystem_tests/tests

verify_refresh = <function _verify_refresh at 0x7fb0f2b48bd0>

    def test_application_default_credentials(verify_refresh):
        credentials, project_id = google.auth.default()

        if EXPECT_PROJECT_ID is not None:
            assert project_id is not None

>       verify_refresh(credentials)

system_tests_sync/test_default.py:28:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
system_tests_sync/conftest.py:122: in _verify_refresh
    credentials.refresh(http_request)
.nox/default_explicit_authorized_user-2-7/lib/python2.7/site-packages/google/oauth2/credentials.py:310: in refresh
    enable_reauth_refresh=self._enable_reauth_refresh,
.nox/default_explicit_authorized_user-2-7/lib/python2.7/site-packages/google/oauth2/reauth.py:347: in refresh_grant
    _client._handle_error_response(response_data)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

response_data = {'error': 'invalid_grant', 'error_description': 'Token has been expired or revoked.'}

    def _handle_error_response(response_data):
        """Translates an error response into an exception.

        Args:
            response_data (Mapping | str): The decoded response data.

        Raises:
            google.auth.exceptions.RefreshError: The errors contained in response_data.
        """
        if isinstance(response_data, six.string_types):
            raise exceptions.RefreshError(response_data)
        try:
            error_details = "{}: {}".format(
                response_data["error"], response_data.get("error_description")
            )
        # If no details could be extracted, use the response data.
        except (KeyError, ValueError):
            error_details = json.dumps(response_data)

>       raise exceptions.RefreshError(error_details, response_data)
E       RefreshError: ('invalid_grant: Token has been expired or revoked.', {u'error_description': u'Token has been expired or revoked.', u'error': u'invalid_grant'})

.nox/default_explicit_authorized_user-2-7/lib/python2.7/site-packages/google/oauth2/_client.py:62: RefreshError

@clundin25 clundin25 added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Aug 29, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 29, 2022
@clundin25
Copy link
Contributor

The failing presubmit looks like nothing to do with my change https://source.cloud.google.com/results/invocations/29f9a2fd-8d3a-4251-88f0-4598f6910f6b/targets/github%2Fgoogle-auth-library-python%2Fsystem_tests/tests

I've pushed a new test token to your branch. The tests will pass now.

@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 29, 2022
@stewartmiles
Copy link
Contributor Author

@clundin25 thanks, looks like everything is green now :) I don't have a merge button so I assume you'll merge this patch when you're ready?

@clundin25
Copy link
Contributor

@stewartmiles Thanks for the contribution!

@clundin25 clundin25 merged commit b1eb467 into googleapis:main Aug 29, 2022
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.

impersonated_credentials.Credentials.sign_bytes leaks socket
4 participants