From 89cfb39dbffdb31fb496235333aa1044cc3e4490 Mon Sep 17 00:00:00 2001 From: Daryna Ishchenko <80129833+darynaishchenko@users.noreply.github.com> Date: Thu, 25 Jan 2024 16:37:06 +0200 Subject: [PATCH] CAT: Add scenario that OAuth is default method and fields are marked as airbyte_secret (#34178) --- .../connector-acceptance-test/CHANGELOG.md | 3 + .../connector_acceptance_test/config.py | 12 + .../tests/test_core.py | 29 ++ .../connector-acceptance-test/pyproject.toml | 2 +- .../unit_tests/test_spec.py | 379 +++++++++++++++++- .../connector-acceptance-tests-reference.md | 15 +- 6 files changed, 431 insertions(+), 9 deletions(-) diff --git a/airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md index 407a1754e01c..73e2bd75aaba 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md +++ b/airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 3.1.0 +Add TestSpec.test_oauth_is_default_method test with OAuth is default option validation. + ## 3.0.1 Upgrade to Dagger 0.9.6 diff --git a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/config.py b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/config.py index 045e23671b74..af0993135dcd 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/config.py +++ b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/config.py @@ -42,6 +42,17 @@ class BackwardCompatibilityTestsConfig(BaseConfig): ) +class OAuthTestConfig(BaseConfig): + oauth = Field(True, description="Allow source to have another default method that OAuth.") + bypass_reason: Optional[str] = Field(description="Reason why OAuth is not default method.") + + @validator("oauth", always=True) + def validate_oauth(cls, oauth, values): + if oauth is False and not values.get("bypass_reason"): + raise ValueError("Please provide a bypass reason for Auth default method") + return oauth + + class SpecTestConfig(BaseConfig): spec_path: str = spec_path config_path: str = config_path @@ -50,6 +61,7 @@ class SpecTestConfig(BaseConfig): backward_compatibility_tests_config: BackwardCompatibilityTestsConfig = Field( description="Configuration for the backward compatibility tests.", default=BackwardCompatibilityTestsConfig() ) + auth_default_method: Optional[OAuthTestConfig] = Field(description="Auth default method details.") class ConnectionTestConfig(BaseConfig): diff --git a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py index 7ced702152db..91aee970a97b 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py +++ b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py @@ -120,6 +120,12 @@ async def skip_backward_compatibility_tests_fixture( pytest.skip(f"Backward compatibility tests are disabled for version {previous_connector_version}.") return False + @pytest.fixture(name="skip_oauth_default_method_test") + def skip_oauth_default_method_test_fixture(self, inputs: SpecTestConfig): + if inputs.auth_default_method and not inputs.auth_default_method.oauth: + pytest.skip(f"Skipping OAuth is default method test: {inputs.auth_default_method.bypass_reason}") + return False + def test_config_match_spec(self, actual_connector_spec: ConnectorSpecification, connector_config: SecretDict): """Check that config matches the actual schema from the spec call""" # Getting rid of technical variables that start with an underscore @@ -521,6 +527,29 @@ def test_oauth_flow_parameters(self, actual_connector_spec: ConnectorSpecificati diff = paths_to_validate - set(get_expected_schema_structure(spec_schema)) assert diff == set(), f"Specified oauth fields are missed from spec schema: {diff}" + def test_oauth_is_default_method(self, skip_oauth_default_method_test: bool, actual_connector_spec: ConnectorSpecification): + """ + OAuth is default check. + If credentials do have oneOf: we check that the OAuth is listed at first. + If there is no oneOf and Oauth: OAuth is only option to authenticate the source and no check is needed. + """ + advanced_auth = actual_connector_spec.advanced_auth + if not advanced_auth: + pytest.skip("Source does not have OAuth method.") + + spec_schema = actual_connector_spec.connectionSpecification + credentials = advanced_auth.predicate_key[0] + try: + one_of_default_method = dpath.util.get(spec_schema, f"/**/{credentials}/oneOf/0") + except KeyError as e: # Key Error when oneOf is not in credentials object + pytest.skip("Credentials object does not have oneOf option.") + + path_in_credentials = "/".join(advanced_auth.predicate_key[1:]) + auth_method_predicate_const = dpath.util.get(one_of_default_method, f"/**/{path_in_credentials}/const") + assert ( + auth_method_predicate_const == advanced_auth.predicate_value + ), f"Oauth method should be a default option. Current default method is {auth_method_predicate_const}." + @pytest.mark.default_timeout(ONE_MINUTE) @pytest.mark.backward_compatibility def test_backward_compatibility( diff --git a/airbyte-integrations/bases/connector-acceptance-test/pyproject.toml b/airbyte-integrations/bases/connector-acceptance-test/pyproject.toml index 12f3060287b3..33e8154a63c1 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/pyproject.toml +++ b/airbyte-integrations/bases/connector-acceptance-test/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api" [tool.poetry] name = "connector-acceptance-test" -version = "3.0.1" +version = "3.1.0" description = "Contains acceptance tests for connectors." authors = ["Airbyte "] license = "MIT" diff --git a/airbyte-integrations/bases/connector-acceptance-test/unit_tests/test_spec.py b/airbyte-integrations/bases/connector-acceptance-test/unit_tests/test_spec.py index 3303c2650d1e..e4651b200404 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/unit_tests/test_spec.py +++ b/airbyte-integrations/bases/connector-acceptance-test/unit_tests/test_spec.py @@ -681,7 +681,7 @@ def test_enum_usage(connector_spec, should_fail): }, ), "", - ), + ) ], ) def test_validate_oauth_flow(connector_spec, expected_error): @@ -693,6 +693,314 @@ def test_validate_oauth_flow(connector_spec, expected_error): t.test_oauth_flow_parameters(connector_spec) +@pytest.mark.parametrize( + "connector_spec, expected_error", + [ + # FAIL: OAuth is not default + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "api_url": { + "type": "string" + }, + "credentials": { + "type": "object", + "oneOf": [ + { + "type": "object", + "properties": { + "auth_type": { + "type": "string", + "const": "access_token" + }, + "access_token": { + "type": "string", + } + } + }, + { + "type": "object", + "properties": { + "auth_type": { + "type": "string", + "const": "oauth2.0" + }, + "client_id": { + "type": "string" + }, + "client_secret": { + "type": "string" + }, + "access_token": { + "type": "string" + }, + "token_expiry_date": { + "type": "string", + }, + "refresh_token": { + "type": "string", + } + } + }, + ] + } + } + }, + advanced_auth={ + "auth_flow_type": "oauth2.0", + "predicate_key": ["credentials", "auth_type"], + "predicate_value": "oauth2.0", + "oauth_config_specification": { + "oauth_user_input_from_connector_config_specification": { + "type": "object", + "properties": { + "domain": { + "type": "string", + "path_in_connector_config": ["api_url"] + } + } + }, + "complete_oauth_output_specification": { + "type": "object", + "properties": { + "access_token": { + "type": "string", + "path_in_connector_config": ["credentials", "access_token"] + }, + "refresh_token": { + "type": "string", + "path_in_connector_config": ["credentials", "refresh_token"] + }, + "token_expiry_date": { + "type": "string", + "format": "date-time", + "path_in_connector_config": ["credentials", "token_expiry_date"] + } + } + }, + "complete_oauth_server_input_specification": { + "type": "object", + "properties": { + "client_id": { + "type": "string" + }, + "client_secret": { + "type": "string" + } + } + }, + "complete_oauth_server_output_specification": { + "type": "object", + "properties": { + "client_id": { + "type": "string", + "path_in_connector_config": ["credentials", "client_id"] + }, + "client_secret": { + "type": "string", + "path_in_connector_config": ["credentials", "client_secret"] + } + } + } + } + } + ), "Oauth method should be a default option. Current default method is access_token." + ), + # SUCCESS: Oauth is default + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "api_url": { + "type": "string" + }, + "credentials": { + "type": "object", + "oneOf": [ + { + "type": "object", + "properties": { + "auth_type": { + "type": "string", + "const": "oauth2.0" + }, + "client_id": { + "type": "string" + }, + "client_secret": { + "type": "string" + }, + "access_token": { + "type": "string" + }, + "token_expiry_date": { + "type": "string", + }, + "refresh_token": { + "type": "string", + } + } + }, + { + "type": "object", + "properties": { + "auth_type": { + "type": "string", + "const": "access_token" + }, + "access_token": { + "type": "string", + } + } + } + ] + } + } + }, + advanced_auth={ + "auth_flow_type": "oauth2.0", + "predicate_key": ["credentials", "auth_type"], + "predicate_value": "oauth2.0", + "oauth_config_specification": { + "oauth_user_input_from_connector_config_specification": { + "type": "object", + "properties": { + "domain": { + "type": "string", + "path_in_connector_config": ["api_url"] + } + } + }, + "complete_oauth_output_specification": { + "type": "object", + "properties": { + "access_token": { + "type": "string", + "path_in_connector_config": ["credentials", "access_token"] + }, + "refresh_token": { + "type": "string", + "path_in_connector_config": ["credentials", "refresh_token"] + }, + "token_expiry_date": { + "type": "string", + "format": "date-time", + "path_in_connector_config": ["credentials", "token_expiry_date"] + } + } + }, + "complete_oauth_server_input_specification": { + "type": "object", + "properties": { + "client_id": { + "type": "string" + }, + "client_secret": { + "type": "string" + } + } + }, + "complete_oauth_server_output_specification": { + "type": "object", + "properties": { + "client_id": { + "type": "string", + "path_in_connector_config": ["credentials", "client_id"] + }, + "client_secret": { + "type": "string", + "path_in_connector_config": ["credentials", "client_secret"] + } + } + } + } + } + ), "" + ), + # SUCCESS: no advancedAuth specified + (ConnectorSpecification(connectionSpecification={}), ""), + # SUCCESS: only OAuth option to auth + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "api_url": {"type": "object"}, + "credentials": { + "type": "object", + "properties": { + "auth_type": {"type": "string", "const": "oauth2.0"}, + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + "refresh_token": {"type": "string"}, + "token_expiry_date": {"type": "string", "format": "date-time"}, + }, + }, + }, + }, + advanced_auth={ + "auth_flow_type": "oauth2.0", + "predicate_key": ["credentials", "auth_type"], + "predicate_value": "oauth2.0", + "oauth_config_specification": { + "oauth_user_input_from_connector_config_specification": { + "type": "object", + "properties": {"domain": {"type": "string", "path_in_connector_config": ["api_url"]}}, + }, + "complete_oauth_output_specification": { + "type": "object", + "properties": { + "access_token": {"type": "string", "path_in_connector_config": ["credentials", "access_token"]}, + "refresh_token": {"type": "string", "path_in_connector_config": ["credentials", "refresh_token"]}, + "token_expiry_date": { + "type": "string", + "format": "date-time", + "path_in_connector_config": ["credentials", "token_expiry_date"], + }, + }, + }, + "complete_oauth_server_input_specification": { + "type": "object", + "properties": {"client_id": {"type": "string"}, "client_secret": {"type": "string"}}, + }, + "complete_oauth_server_output_specification": { + "type": "object", + "properties": { + "client_id": {"type": "string", "path_in_connector_config": ["credentials", "client_id"]}, + "client_secret": {"type": "string", "path_in_connector_config": ["credentials", "client_secret"]}, + }, + }, + }, + }, + ), + "", + ), + # SUCCESS: Credentials object does not have oneOf option. + ( + ConnectorSpecification( + connectionSpecification={"type": "object"}, + advanced_auth={ + "auth_type": "oauth2.0", + "predicate_key": ["credentials", "auth_type"], + }, + ), + "Credentials object does not have oneOf option.", + ), + ] +) +def test_validate_auth_default_method(connector_spec, expected_error): + t = _TestSpec() + if expected_error: + with pytest.raises(AssertionError, match=expected_error): + t.test_oauth_is_default_method(skip_oauth_default_method_test=False, actual_connector_spec=connector_spec) + else: + t.test_oauth_is_default_method(skip_oauth_default_method_test=False, actual_connector_spec=connector_spec) + + @pytest.mark.parametrize( "connector_spec, expectation", [ @@ -740,6 +1048,73 @@ def test_additional_properties_is_true(connector_spec, expectation): ({"type": "object", "properties": {"refresh_token": {"type": "boolean", "airbyte_secret": True}}}, True), ({"type": "object", "properties": {"refresh_token": {"type": ["null", "string"]}}}, True), ({"type": "object", "properties": {"credentials": {"type": "array", "items": {"type": "string"}}}}, True), + ( + { + "type": "object", + "properties": {"credentials": {"type": "object", "properties": { + "client_secret": {"type": "string"}, + "access_token": {"type": "string", "airbyte_secret": True}}}} + }, + True + ), + ( + { + "type": "object", + "properties": {"credentials": {"type": "object", "properties": { + "client_secret": {"type": "string", "airbyte_secret": True}, + "access_token": {"type": "string", "airbyte_secret": True}}}} + }, + False + ), + ( + { + "type": "object", + "properties": { + "credentials": { + "type": "object", + "oneOf": [ + { + "type": "object", + "properties": { + "auth_type": { + "type": "string", + "const": "access_token" + }, + "access_token": { + "type": "string", + } + } + }, + { + "type": "object", + "properties": { + "auth_type": { + "type": "string", + "const": "oauth2.0" + }, + "client_id": { + "type": "string" + }, + "client_secret": { + "type": "string" + }, + "access_token": { + "type": "string" + }, + "token_expiry_date": { + "type": "string", + }, + "refresh_token": { + "type": "string", + } + } + }, + ] + } + } + }, + True + ), ({"type": "object", "properties": {"auth": {"oneOf": [{"api_token": {"type": "string"}}]}}}, True), ( { @@ -757,7 +1132,7 @@ def test_airbyte_secret(mocker, connector_spec, should_fail): t = _TestSpec() logger = mocker.Mock() t.test_secret_is_properly_marked( - {"connectionSpecification": connector_spec}, logger, ("api_key", "api_token", "refresh_token", "jwt", "credentials") + {"connectionSpecification": connector_spec}, logger, ("api_key", "api_token", "refresh_token", "jwt", "credentials", "access_token", "client_secret") ) if should_fail: conftest.pytest.fail.assert_called_once() diff --git a/docs/connector-development/testing-connectors/connector-acceptance-tests-reference.md b/docs/connector-development/testing-connectors/connector-acceptance-tests-reference.md index 7d650d089415..a0e2287a3d58 100644 --- a/docs/connector-development/testing-connectors/connector-acceptance-tests-reference.md +++ b/docs/connector-development/testing-connectors/connector-acceptance-tests-reference.md @@ -144,12 +144,15 @@ Additional tests are validating the backward compatibility of the current specif These backward compatibility tests can be bypassed by changing the value of the `backward_compatibility_tests_config.disable_for_version` input in `acceptance-test-config.yml` (see below). One more test validates the specification against containing exposed secrets. This means fields that potentially could hold a secret value should be explicitly marked with `"airbyte_secret": true`. If an input field like `api_key` / `password` / `client_secret` / etc. is exposed, the test will fail. -| Input | Type | Default | Note | -| :--------------------------------------------------------------- | :----- | :------------------ | :-------------------------------------------------------------------------------------------------------------------- | -| `spec_path` | string | `secrets/spec.json` | Path to a YAML or JSON file representing the spec expected to be output by this connector | -| `backward_compatibility_tests_config.previous_connector_version` | string | `latest` | Previous connector version to use for backward compatibility tests (expects a version following semantic versioning). | -| `backward_compatibility_tests_config.disable_for_version` | string | None | Disable the backward compatibility test for a specific version (expects a version following semantic versioning). | -| `timeout_seconds` | int | 10 | Test execution timeout in seconds | +| Input | Type | Default | Note | +|:-----------------------------------------------------------------|:--------|:--------------------|:----------------------------------------------------------------------------------------------------------------------| +| `spec_path` | string | `secrets/spec.json` | Path to a YAML or JSON file representing the spec expected to be output by this connector | +| `backward_compatibility_tests_config.previous_connector_version` | string | `latest` | Previous connector version to use for backward compatibility tests (expects a version following semantic versioning). | +| `backward_compatibility_tests_config.disable_for_version` | string | None | Disable the backward compatibility test for a specific version (expects a version following semantic versioning). | +| `timeout_seconds` | int | 10 | Test execution timeout in seconds | +| `auth_default_method` | object | None | Ensure that OAuth is default method, if OAuth uses by source | +| `auth_default_method.oauth` | boolean | True | Validate that OAuth is default method if set to True | +| `auth_default_method.bypass_reason` | string | | Reason why OAuth is not default method | ## Test Connection