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: validate urls for external accounts #1031

Merged
merged 13 commits into from
May 3, 2022
Merged

fix: validate urls for external accounts #1031

merged 13 commits into from
May 3, 2022

Conversation

BigTailWolf
Copy link
Contributor

@BigTailWolf BigTailWolf commented Apr 22, 2022

[b-193694420] adding url validation for token url and service account impersonation url

  • adding coverage of empty hostname to invalid token url
  • Add more tests to enlarge the coverage
    Co-authored-by: Jin Qin qinjin@google.com

Blacken successful

❯ python -m nox -s blacken
nox > Running session blacken
nox > Creating virtual environment (virtualenv) using python3.8 in .nox/blacken
nox > python -m pip install click==8.0.4 black==19.3b0
nox > black google tests tests_async noxfile.py setup.py docs/conf.py
All done! ✨ 🍰 ✨
111 files left unchanged.
nox > Session blacken was successful.

Lint successful

❯ python -m nox -s lint
nox > Running session lint
nox > Creating virtual environment (virtualenv) using python3.8 in .nox/lint
nox > python -m pip install flake8 flake8-import-order docutils click==8.0.4 black==19.3b0
nox > python -m pip install -e .
nox > black --check google tests tests_async noxfile.py setup.py docs/conf.py
All done! ✨ 🍰 ✨
111 files would be left unchanged.
nox > flake8 --import-order-style=google --application-import-names=google,tests,system_tests google tests tests_async
nox > python setup.py check --metadata --restructuredtext --strict
running check
nox > Session lint was successful.

nox test added and 100% covered

----------------------------------------------------------------------------------------------
TOTAL                                              11158      0   1204      0   100%
nox > Session cover was successful.

…personation url (#1)

[b-193694420] adding url validation for token url and service account impersonation url
  * adding coverage of empty hostname to invalid token url
  * Add more tests to enlarge the coverage
Co-authored-by: Jin Qin <qinjin@google.com>
tests/test_external_account.py Outdated Show resolved Hide resolved
google/auth/external_account.py Outdated Show resolved Hide resolved
@arithmetic1728 arithmetic1728 changed the title [Bug 193694420] adding validation of token url and service account im… fix: validate urls for external accounts Apr 22, 2022
@arithmetic1728
Copy link
Contributor

Please change the commit message to "fix: ". The commit has to start with fix: prefix, otherwise the presubmit check will fail.

@arithmetic1728 arithmetic1728 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 22, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 22, 2022
@arithmetic1728 arithmetic1728 requested a review from lsirac April 22, 2022 22:38
@arithmetic1728
Copy link
Contributor

arithmetic1728 commented Apr 22, 2022

Unit test failed:

ImportError while importing test module '/tmpfs/src/github/google-auth-library-python/tests/test_identity_pool.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
tests/test_identity_pool.py:26: in <module>
    from google.auth import identity_pool
google/auth/identity_pool.py:47: in <module>
    from google.auth import external_account
google/auth/external_account.py:37: in <module>
    from urllib.parse import urlparse
E   ImportError: No module named parse

Lint nox session also failed. Please run python -m nox -s blacken to format the files, then python -m nox -s lint to make sure it can pass.

@arithmetic1728 arithmetic1728 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 25, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 25, 2022
google/auth/external_account.py Outdated Show resolved Hide resolved
google/auth/external_account.py Outdated Show resolved Hide resolved
tests/test_external_account.py Outdated Show resolved Hide resolved
tests/test_external_account.py Outdated Show resolved Hide resolved
google/auth/external_account.py Outdated Show resolved Hide resolved
@lsirac
Copy link
Contributor

lsirac commented Apr 27, 2022

LGTM, thanks Jin!

tests/test_external_account.py Outdated Show resolved Hide resolved
tests/test_external_account.py Outdated Show resolved Hide resolved
google/auth/external_account.py Show resolved Hide resolved
@arithmetic1728 arithmetic1728 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 28, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 28, 2022
@arithmetic1728 arithmetic1728 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 28, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 28, 2022
@arithmetic1728
Copy link
Contributor

Session unit-3.6 failed
FAILED tests/test_external_account.py::TestCredentials::test_invalid_token_url_shall_throw_exceptions
FAILED tests/test_external_account.py::TestCredentials::test_invalid_service_account_impersonate_url_shall_throw_exceptions

for url in invalid_urls:
            # An invalid url should throw a ValueError exception
            with pytest.raises(ValueError) as excinfo:
                external_account.Credentials.validate_service_account_impersonation_url(
>                   url
                )
E               Failed: DID NOT RAISE <class 'ValueError'>

@arithmetic1728 arithmetic1728 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 29, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 29, 2022
@arithmetic1728 arithmetic1728 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 3, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 3, 2022
@arithmetic1728 arithmetic1728 added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels May 3, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 3, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 3, 2022
@arithmetic1728 arithmetic1728 merged commit 61b1f15 into googleapis:main May 3, 2022
gcf-merge-on-green bot pushed a commit that referenced this pull request Jun 7, 2022
🤖 I have created a release *beep* *boop*
---


## [2.7.0](v2.6.6...v2.7.0) (2022-06-07)


### Features

* add experimental enterprise cert support ([#1052](#1052)) ([dda7dda](dda7dda))
* add experimental GDCH support ([#1022](#1022)) ([5367aac](5367aac))
* Pluggable auth support ([#995](#995)) ([62daa73](62daa73))


### Bug Fixes

* validate urls for external accounts ([#1031](#1031)) ([61b1f15](61b1f15))


### Reverts

* pluggable auth support [#995](#995) ([#1039](#1039)) ([513d999](513d999))
* revert experimental GDCH support ([#1022](#1022)) ([#1042](#1042)) ([c720995](c720995))


### Documentation

* fix changelog header to consistent size ([#1046](#1046)) ([e64d084](e64d084))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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.

4 participants