fix: Remove 3PI config url validation (#1220)

* fix: Remove 3PI config url validation * add security consideration in doc * Break the security consideration into workload and woorkforce section --------- Co-authored-by: Leo <[email protected]>
googleapis · Feb 9, 2023 · 8b95515 · 8b95515
1 parent 462da2c
commit 8b95515
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 321 deletions.
diff --git a/docs/user-guide.rst b/docs/user-guide.rst
@@ -548,6 +548,16 @@ For AWS providers, use :meth:`aws.Credentials.from_info
         ['https://www.googleapis.com/auth/cloud-platform'])
 
 
+Security considerations
+~~~~~~~~~~~~~~~~~~~~~~~
+
+Note that this library does not perform any validation on the token_url,
+token_info_url, or service_account_impersonation_url fields of the credential
+configuration. It is not recommended to use a credential configuration that you
+did not generate with the gcloud CLI unless you verify that the URL fields point
+to a googleapis.com domain.
+
+
 External credentials (Workforce identity federation)
 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 
@@ -793,6 +803,13 @@ Cloud resources from an OIDC or SAML provider.
     https://cloud.google.com/iam/docs/workforce-identity-federation#workforce-pools-user-project
 
 
+Note that this library does not perform any validation on the token_url,
+token_info_url, or service_account_impersonation_url fields of the credential
+configuration. It is not recommended to use a credential configuration that you
+did not generate with the gcloud CLI unless you verify that the URL fields point
+to a googleapis.com domain.
+
+
 Impersonated credentials
 ++++++++++++++++++++++++
 

diff --git a/google/auth/external_account.py b/google/auth/external_account.py
@@ -35,7 +35,6 @@
 import re
 
 import six
-from urllib3.util import parse_url
 
 from google.auth import _helpers
 from google.auth import credentials
@@ -127,14 +126,6 @@ def __init__(
         self._default_scopes = default_scopes
         self._workforce_pool_user_project = workforce_pool_user_project
 
-        Credentials.validate_token_url(token_url)
-        if token_info_url:
-            Credentials.validate_token_url(token_info_url, url_type="token info")
-        if service_account_impersonation_url:
-            Credentials.validate_service_account_impersonation_url(
-                service_account_impersonation_url
-            )
-
         if self._client_id:
             self._client_auth = utils.ClientAuthentication(
                 utils.ClientAuthType.basic, self._client_id, self._client_secret
@@ -434,58 +425,6 @@ def _initialize_impersonated_credentials(self):
             ),
         )
 
-    @staticmethod
-    def validate_token_url(token_url, url_type="token"):
-        _TOKEN_URL_PATTERNS = [
-            "^[^\\.\\s\\/\\\\]+\\.sts(?:\\.mtls)?\\.googleapis\\.com$",
-            "^sts(?:\\.mtls)?\\.googleapis\\.com$",
-            "^sts\\.[^\\.\\s\\/\\\\]+(?:\\.mtls)?\\.googleapis\\.com$",
-            "^[^\\.\\s\\/\\\\]+\\-sts(?:\\.mtls)?\\.googleapis\\.com$",
-            "^sts\\-[^\\.\\s\\/\\\\]+\\.p(?:\\.mtls)?\\.googleapis\\.com$",
-        ]
-
-        if not Credentials.is_valid_url(_TOKEN_URL_PATTERNS, token_url):
-            raise exceptions.InvalidResource(
-                "The provided {} URL is invalid.".format(url_type)
-            )
-
-    @staticmethod
-    def validate_service_account_impersonation_url(url):
-        _SERVICE_ACCOUNT_IMPERSONATION_URL_PATTERNS = [
-            "^[^\\.\\s\\/\\\\]+\\.iamcredentials\\.googleapis\\.com$",
-            "^iamcredentials\\.googleapis\\.com$",
-            "^iamcredentials\\.[^\\.\\s\\/\\\\]+\\.googleapis\\.com$",
-            "^[^\\.\\s\\/\\\\]+\\-iamcredentials\\.googleapis\\.com$",
-            "^iamcredentials\\-[^\\.\\s\\/\\\\]+\\.p\\.googleapis\\.com$",
-        ]
-
-        if not Credentials.is_valid_url(
-            _SERVICE_ACCOUNT_IMPERSONATION_URL_PATTERNS, url
-        ):
-            raise exceptions.InvalidResource(
-                "The provided service account impersonation URL is invalid."
-            )
-
-    @staticmethod
-    def is_valid_url(patterns, url):
-        """
-        Returns True if the provided URL's scheme is HTTPS and the host comforms to at least one of the provided patterns.
-        """
-        # Check specifically for whitespcaces:
-        #   Some python3.6 will parse the space character into %20 and pass the regex check which shouldn't be passed
-        if not url or len(str(url).split()) > 1:
-            return False
-
-        try:
-            uri = parse_url(url)
-        except Exception:
-            return False
-
-        if not uri.scheme or uri.scheme != "https" or not uri.hostname:
-            return False
-
-        return any(re.compile(p).match(uri.hostname.lower()) for p in patterns)
-
     @classmethod
     def from_info(cls, info, **kwargs):
         """Creates a Credentials instance from parsed external account info.

diff --git a/tests/test_aws.py b/tests/test_aws.py
@@ -1085,16 +1085,6 @@ def test_token_info_url_custom(self):
 
             assert credentials.token_info_url == (url + "/introspect")
 
-    def test_token_info_url_bad(self):
-        for url in INVALID_TOKEN_URLS:
-            with pytest.raises(ValueError) as excinfo:
-                self.make_credentials(
-                    credential_source=self.CREDENTIAL_SOURCE.copy(),
-                    token_info_url=(url + "/introspect"),
-                )
-
-            assert excinfo.match(r"The provided token info URL is invalid\.")
-
     def test_token_info_url_negative(self):
         credentials = self.make_credentials(
             credential_source=self.CREDENTIAL_SOURCE.copy(), token_info_url=None
@@ -1111,16 +1101,6 @@ def test_token_url_custom(self):
 
             assert credentials._token_url == (url + "/token")
 
-    def test_token_url_bad(self):
-        for url in INVALID_TOKEN_URLS:
-            with pytest.raises(ValueError) as excinfo:
-                self.make_credentials(
-                    credential_source=self.CREDENTIAL_SOURCE.copy(),
-                    token_url=(url + "/token"),
-                )
-
-            assert excinfo.match(r"The provided token URL is invalid\.")
-
     def test_service_account_impersonation_url_custom(self):
         for url in VALID_SERVICE_ACCOUNT_IMPERSONATION_URLS:
             credentials = self.make_credentials(
@@ -1134,20 +1114,6 @@ def test_service_account_impersonation_url_custom(self):
                 url + SERVICE_ACCOUNT_IMPERSONATION_URL_ROUTE
             )
 
-    def test_service_account_impersonation_url_bad(self):
-        for url in INVALID_SERVICE_ACCOUNT_IMPERSONATION_URLS:
-            with pytest.raises(ValueError) as excinfo:
-                self.make_credentials(
-                    credential_source=self.CREDENTIAL_SOURCE.copy(),
-                    service_account_impersonation_url=(
-                        url + SERVICE_ACCOUNT_IMPERSONATION_URL_ROUTE
-                    ),
-                )
-
-            assert excinfo.match(
-                r"The provided service account impersonation URL is invalid\."
-            )
-
     def test_retrieve_subject_token_missing_region_url(self):
         # When AWS_REGION envvar is not available, region_url is required for
         # determining the current AWS region.

diff --git a/tests/test_external_account.py b/tests/test_external_account.py
@@ -65,101 +65,6 @@
     "//iam.googleapis.com/locations//workforcePool/pool-id/providers/provider-id",
 ]
 
-VALID_TOKEN_URLS = [
-    "https://sts.googleapis.com",
-    "https://sts.mtls.googleapis.com",
-    "https://us-east-1.sts.googleapis.com",
-    "https://us-east-1.sts.mtls.googleapis.com",
-    "https://US-EAST-1.sts.googleapis.com",
-    "https://sts.us-east-1.googleapis.com",
-    "https://sts.US-WEST-1.googleapis.com",
-    "https://us-east-1-sts.googleapis.com",
-    "https://US-WEST-1-sts.googleapis.com",
-    "https://US-WEST-1-sts.mtls.googleapis.com",
-    "https://us-west-1-sts.googleapis.com/path?query",
-    "https://sts-us-east-1.p.googleapis.com",
-    "https://sts-us-east-1.p.mtls.googleapis.com",
-]
-INVALID_TOKEN_URLS = [
-    "https://iamcredentials.googleapis.com",
-    "https://mtls.iamcredentials.googleapis.com",
-    "sts.googleapis.com",
-    "mtls.sts.googleapis.com",
-    "mtls.googleapis.com",
-    "https://",
-    "http://sts.googleapis.com",
-    "https://st.s.googleapis.com",
-    "https://us-eas\t-1.sts.googleapis.com",
-    "https:/us-east-1.sts.googleapis.com",
-    "https:/us-east-1.mtls.sts.googleapis.com",
-    "https://US-WE/ST-1-sts.googleapis.com",
-    "https://sts-us-east-1.googleapis.com",
-    "https://sts-US-WEST-1.googleapis.com",
-    "testhttps://us-east-1.sts.googleapis.com",
-    "https://us-east-1.sts.googleapis.comevil.com",
-    "https://us-east-1.us-east-1.sts.googleapis.com",
-    "https://us-ea.s.t.sts.googleapis.com",
-    "https://sts.googleapis.comevil.com",
-    "hhttps://us-east-1.sts.googleapis.com",
-    "https://us- -1.sts.googleapis.com",
-    "https://-sts.googleapis.com",
-    "https://-mtls.googleapis.com",
-    "https://us-east-1.sts.googleapis.com.evil.com",
-    "https://sts.pgoogleapis.com",
-    "https://p.googleapis.com",
-    "https://sts.p.com",
-    "https://sts.p.mtls.com",
-    "http://sts.p.googleapis.com",
-    "https://xyz-sts.p.googleapis.com",
-    "https://sts-xyz.123.p.googleapis.com",
-    "https://sts-xyz.p1.googleapis.com",
-    "https://sts-xyz.p.foo.com",
-    "https://sts-xyz.p.foo.googleapis.com",
-    "https://sts-xyz.mtls.p.foo.googleapis.com",
-    "https://sts-xyz.p.mtls.foo.googleapis.com",
-]
-VALID_SERVICE_ACCOUNT_IMPERSONATION_URLS = [
-    "https://iamcredentials.googleapis.com",
-    "https://us-east-1.iamcredentials.googleapis.com",
-    "https://US-EAST-1.iamcredentials.googleapis.com",
-    "https://iamcredentials.us-east-1.googleapis.com",
-    "https://iamcredentials.US-WEST-1.googleapis.com",
-    "https://us-east-1-iamcredentials.googleapis.com",
-    "https://US-WEST-1-iamcredentials.googleapis.com",
-    "https://us-west-1-iamcredentials.googleapis.com/path?query",
-    "https://iamcredentials-us-east-1.p.googleapis.com",
-]
-INVALID_SERVICE_ACCOUNT_IMPERSONATION_URLS = [
-    "https://sts.googleapis.com",
-    "iamcredentials.googleapis.com",
-    "https://",
-    "http://iamcredentials.googleapis.com",
-    "https://iamcre.dentials.googleapis.com",
-    "https://us-eas\t-1.iamcredentials.googleapis.com",
-    "https:/us-east-1.iamcredentials.googleapis.com",
-    "https://US-WE/ST-1-iamcredentials.googleapis.com",
-    "https://iamcredentials-us-east-1.googleapis.com",
-    "https://iamcredentials-US-WEST-1.googleapis.com",
-    "testhttps://us-east-1.iamcredentials.googleapis.com",
-    "https://us-east-1.iamcredentials.googleapis.comevil.com",
-    "https://us-east-1.us-east-1.iamcredentials.googleapis.com",
-    "https://us-ea.s.t.iamcredentials.googleapis.com",
-    "https://iamcredentials.googleapis.comevil.com",
-    "hhttps://us-east-1.iamcredentials.googleapis.com",
-    "https://us- -1.iamcredentials.googleapis.com",
-    "https://-iamcredentials.googleapis.com",
-    "https://us-east-1.iamcredentials.googleapis.com.evil.com",
-    "https://iamcredentials.pgoogleapis.com",
-    "https://p.googleapis.com",
-    "https://iamcredentials.p.com",
-    "http://iamcredentials.p.googleapis.com",
-    "https://xyz-iamcredentials.p.googleapis.com",
-    "https://iamcredentials-xyz.123.p.googleapis.com",
-    "https://iamcredentials-xyz.p1.googleapis.com",
-    "https://iamcredentials-xyz.p.foo.com",
-    "https://iamcredentials-xyz.p.foo.googleapis.com",
-]
-
 
 class CredentialsImpl(external_account.Credentials):
     def __init__(self, **kwargs):
@@ -350,44 +255,6 @@ def assert_resource_manager_request_kwargs(
         assert request_kwargs["headers"] == headers
         assert "body" not in request_kwargs
 
-    def test_valid_token_url_shall_pass_validation(self):
-        valid_urls = VALID_TOKEN_URLS
-
-        for url in valid_urls:
-            # A valid url shouldn't throw exception and a None value should be returned
-            external_account.Credentials.validate_token_url(url)
-
-    def test_invalid_token_url_shall_throw_exceptions(self):
-        invalid_urls = INVALID_TOKEN_URLS
-
-        for url in invalid_urls:
-            # An invalid url should throw a ValueError exception
-            with pytest.raises(ValueError) as excinfo:
-                external_account.Credentials.validate_token_url(url)
-
-            assert excinfo.match("The provided token URL is invalid.")
-
-    def test_valid_service_account_impersonation_url_shall_pass_validation(self):
-        valid_urls = VALID_SERVICE_ACCOUNT_IMPERSONATION_URLS
-
-        for url in valid_urls:
-            # A valid url shouldn't throw exception and a None value should be returned
-            external_account.Credentials.validate_service_account_impersonation_url(url)
-
-    def test_invalid_service_account_impersonate_url_shall_throw_exceptions(self):
-        invalid_urls = INVALID_SERVICE_ACCOUNT_IMPERSONATION_URLS
-
-        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
-                )
-
-            assert excinfo.match(
-                "The provided service account impersonation URL is invalid."
-            )
-
     def test_default_state(self):
         credentials = self.make_credentials(
             service_account_impersonation_url=self.SERVICE_ACCOUNT_IMPERSONATION_URL
@@ -409,31 +276,6 @@ def test_default_state(self):
         # Token info url not set yet
         assert not credentials.token_info_url
 
-    def test_invalid_token_url(self):
-        with pytest.raises(ValueError) as excinfo:
-            CredentialsImpl(
-                audience=self.AUDIENCE,
-                subject_token_type=self.SUBJECT_TOKEN_TYPE,
-                token_url="https:///v1/token",
-                credential_source=self.CREDENTIAL_SOURCE,
-            )
-
-        assert excinfo.match("The provided token URL is invalid.")
-
-    def test_invalid_service_account_impersonate_url(self):
-        with pytest.raises(ValueError) as excinfo:
-            CredentialsImpl(
-                audience=self.AUDIENCE,
-                subject_token_type=self.SUBJECT_TOKEN_TYPE,
-                token_url=self.TOKEN_URL,
-                credential_source=self.CREDENTIAL_SOURCE,
-                service_account_impersonation_url=12345,  # create an exception by sending to parse url
-            )
-
-        assert excinfo.match(
-            "The provided service account impersonation URL is invalid."
-        )
-
     def test_nonworkforce_with_workforce_pool_user_project(self):
         with pytest.raises(ValueError) as excinfo:
             CredentialsImpl(