🎉 Source Drift: add `OAuth 2.0` authentication option #7337

bazarnov · 2021-10-25T14:42:58Z

What

#6261 - Source Drift: support oauth

How

changed the authentication logic of client.py
changed spec.json
corrected unit_tests

Pre-merge Checklist

Expand the relevant checklist and delete the others.

Updating a connector

Community member or Airbyter

Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions
Connector version bumped like described here

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing.
New Connector version released on Dockerhub by running the /publish command described here

bazarnov · 2021-10-28T11:29:27Z

/test connector=connectors/source-drift

🕑 connectors/source-drift https://github.com/airbytehq/airbyte/actions/runs/1394373636
✅ connectors/source-drift https://github.com/airbytehq/airbyte/actions/runs/1394373636
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                 Stmts   Miss  Cover
	 ------------------------------------------------------------------------
	 source_acceptance_test/__init__.py                       2      0   100%
	 source_acceptance_test/base.py                          10      4    60%
	 source_acceptance_test/config.py                        74      8    89%
	 source_acceptance_test/conftest.py                     108    108     0%
	 source_acceptance_test/plugin.py                        47     47     0%
	 source_acceptance_test/tests/__init__.py                 4      0   100%
	 source_acceptance_test/tests/test_core.py              200     94    53%
	 source_acceptance_test/tests/test_full_refresh.py       18     11    39%
	 source_acceptance_test/tests/test_incremental.py        69     38    45%
	 source_acceptance_test/utils/__init__.py                 6      0   100%
	 source_acceptance_test/utils/asserts.py                 37      2    95%
	 source_acceptance_test/utils/common.py                  41     24    41%
	 source_acceptance_test/utils/compare.py                 47     20    57%
	 source_acceptance_test/utils/connector_runner.py        82     49    40%
	 source_acceptance_test/utils/json_schema_helper.py     115     14    88%
	 ------------------------------------------------------------------------
	 TOTAL                                                  860    419    51%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                              Stmts   Miss  Cover
	 -----------------------------------------------------
	 source_drift/__init__.py              3      0   100%
	 source_drift/client/__init__.py       3      0   100%
	 source_drift/client/api.py           97     28    71%
	 source_drift/client/client.py        33      4    88%
	 source_drift/client/common.py        56     27    52%
	 source_drift/client/fixture.py       24     24     0%
	 source_drift/source.py                4      0   100%
	 -----------------------------------------------------
	 TOTAL                               220     83    62%

airbyte-integrations/connectors/source-drift/source_drift/spec.json

alexandr-shegeda

java part looks good

yurii-bidiuk · 2021-11-03T16:11:00Z

Hello @sherifnada
There is some issue with Drift authentication.
When we set up a new Drift source with OAuth2 authentication, we are redirected to the Drift authentication window

After clicking on 'Authorize App' we should be redirected back to Airbyte 'New connection' window and see that authentication succeeded. But instead of that, we get a CORS error:

When I disable CORS in my web browser, then I can pass the authentication scenario and get a success message:

Could you please advise something?

sherifnada · 2021-11-04T05:02:40Z

@bazarnov to which domain is the erroneous request being made?

sherifnada · 2021-11-04T05:03:12Z

airbyte-integrations/connectors/source-drift/source_drift/spec.json

+          {
+            "type": "object",
+            "title": "OAuth2.0",
+            "required": ["client_id", "client_secret", "access_token", "refresh_token"],


access token is not required since we can use the other params to generate it

yurii-bidiuk · 2021-11-04T14:37:29Z

@bazarnov to which domain is the erroneous request being made?

@sherifnada domain is customer2.api.drift.com
It fails on the case when we set redirect URL as localhost. When we set any other URL (eg https://dev-cloud.airbyte.io/auth_flow) then everything is going well and authentication is successful

sherifnada · 2021-11-04T16:18:20Z

@yurii-bidiuk oops, tagged bazarnov by accident. It seems that local auth is the problem then. I think we should not block this PR on fixing localhost support. We should move ahead w this PR but also ask Drift support of there is any way to support localhost.

bazarnov · 2021-11-04T17:33:59Z

They have the support of localhost, but it should be using https://... instead of http://...

bazarnov · 2021-11-10T14:35:38Z

@sherifnada can we plan to marge this PR? I think we passed all the tests at this point.

…-oauth

bazarnov · 2021-11-16T10:52:33Z

/test connector=connectors/source-drift

🕑 connectors/source-drift https://github.com/airbytehq/airbyte/actions/runs/1466482339
✅ connectors/source-drift https://github.com/airbytehq/airbyte/actions/runs/1466482339
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                 Stmts   Miss  Cover
	 ------------------------------------------------------------------------
	 source_acceptance_test/__init__.py                       2      0   100%
	 source_acceptance_test/base.py                          10      4    60%
	 source_acceptance_test/config.py                        75      8    89%
	 source_acceptance_test/conftest.py                     108    108     0%
	 source_acceptance_test/plugin.py                        47     47     0%
	 source_acceptance_test/tests/__init__.py                 4      0   100%
	 source_acceptance_test/tests/test_core.py              200     94    53%
	 source_acceptance_test/tests/test_full_refresh.py       38     27    29%
	 source_acceptance_test/tests/test_incremental.py        69     38    45%
	 source_acceptance_test/utils/__init__.py                 6      0   100%
	 source_acceptance_test/utils/asserts.py                 37      2    95%
	 source_acceptance_test/utils/common.py                  41     24    41%
	 source_acceptance_test/utils/compare.py                 62     25    60%
	 source_acceptance_test/utils/connector_runner.py        82     49    40%
	 source_acceptance_test/utils/json_schema_helper.py     115     14    88%
	 ------------------------------------------------------------------------
	 TOTAL                                                  896    440    51%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                              Stmts   Miss  Cover
	 -----------------------------------------------------
	 source_drift/__init__.py              3      0   100%
	 source_drift/client/__init__.py       3      0   100%
	 source_drift/client/api.py           97     28    71%
	 source_drift/client/client.py        33      4    88%
	 source_drift/client/common.py        56     27    52%
	 source_drift/client/fixture.py       24     24     0%
	 source_drift/source.py                4      0   100%
	 -----------------------------------------------------
	 TOTAL                               220     83    62%

bazarnov · 2021-11-16T11:01:56Z

/publish connector=connectors/source-drift

🕑 connectors/source-drift https://github.com/airbytehq/airbyte/actions/runs/1466516803
✅ connectors/source-drift https://github.com/airbytehq/airbyte/actions/runs/1466516803

…-oauth

github-actions bot added the area/connectors Connector related issues label Oct 25, 2021

bazarnov linked an issue Oct 25, 2021 that may be closed by this pull request

Source Drift: support oauth #6261

Closed

bazarnov self-assigned this Oct 25, 2021

github-actions bot added the area/documentation Improvements or additions to documentation label Oct 25, 2021

bazarnov temporarily deployed to more-secrets October 25, 2021 18:25 Inactive

bazarnov mentioned this pull request Oct 25, 2021

Source Drift: implement oauth2 java-part #7353

Closed

bazarnov requested review from midavadim and antixar October 25, 2021 18:36

bazarnov temporarily deployed to more-secrets October 28, 2021 11:12 Inactive

jrhizor temporarily deployed to more-secrets October 28, 2021 11:31 Inactive

midavadim suggested changes Oct 29, 2021

View reviewed changes

airbyte-integrations/connectors/source-drift/source_drift/spec.json Show resolved Hide resolved

bazarnov requested a review from midavadim November 1, 2021 09:52

bazarnov temporarily deployed to more-secrets November 1, 2021 09:54 Inactive

yurii-bidiuk linked an issue Nov 3, 2021 that may be closed by this pull request

Source Drift: implement oauth2 java-part #7353

Closed

yurii-bidiuk temporarily deployed to more-secrets November 3, 2021 12:44 Inactive

alexandr-shegeda self-requested a review November 3, 2021 13:25

alexandr-shegeda approved these changes Nov 3, 2021

View reviewed changes

bazarnov temporarily deployed to more-secrets November 3, 2021 14:53 Inactive

yurii-bidiuk requested a review from sherifnada November 3, 2021 16:12

bazarnov force-pushed the bazarnov/6261-drift-oauth branch from 3b1275b to 4176cbd Compare November 3, 2021 16:15

bazarnov temporarily deployed to more-secrets November 3, 2021 16:17 Inactive

yurii-bidiuk temporarily deployed to more-secrets November 3, 2021 16:27 Inactive

sherifnada reviewed Nov 4, 2021

View reviewed changes

bazarnov added 2 commits November 16, 2021 11:27

Merge remote-tracking branch 'origin/master' into bazarnov/6261-drift…

e053589

…-oauth

updated after review

73b3b85

bazarnov temporarily deployed to more-secrets November 16, 2021 09:39 Inactive

fixed readme

737e840

bazarnov temporarily deployed to more-secrets November 16, 2021 10:00 Inactive

midavadim approved these changes Nov 16, 2021

View reviewed changes

jrhizor temporarily deployed to more-secrets November 16, 2021 10:55 Inactive

jrhizor temporarily deployed to more-secrets November 16, 2021 11:04 Inactive

bazarnov temporarily deployed to more-secrets November 16, 2021 11:14 Inactive

use BaseOAuth2Flow instead of BaseOAuthFlow

28d6f3b

yurii-bidiuk temporarily deployed to more-secrets November 16, 2021 12:21 Inactive

yurii-bidiuk force-pushed the bazarnov/6261-drift-oauth branch from 28d6f3b to 3c3180c Compare November 18, 2021 12:33

yurii-bidiuk temporarily deployed to more-secrets November 18, 2021 12:34 Inactive

yurii-bidiuk temporarily deployed to more-secrets November 18, 2021 15:29 Inactive

bazarnov added 3 commits November 18, 2021 17:45

Merge remote-tracking branch 'origin/master' into bazarnov/6261-drift…

e795a2a

…-oauth

Merge remote-tracking branch 'origin/master' into bazarnov/6261-drift…

cfc871a

…-oauth

updated flow

bcbb385

bazarnov force-pushed the bazarnov/6261-drift-oauth branch from 3c3180c to bcbb385 Compare November 22, 2021 11:53

bazarnov temporarily deployed to more-secrets November 22, 2021 11:55 Inactive

fixed oauth flow and unit tests

99ecbad

yurii-bidiuk temporarily deployed to more-secrets November 22, 2021 14:08 Inactive

bazarnov merged commit c790a7f into master Nov 22, 2021

bazarnov deleted the bazarnov/6261-drift-oauth branch November 22, 2021 15:28

jrhizor mentioned this pull request Nov 23, 2021

Bump Airbyte version from 0.32.5-alpha to 0.32.6-alpha #8192

Merged

schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022

🎉 Source Drift: add OAuth 2.0 authentication option (airbytehq#7337)

19d086d

igrankova added connectors/sources-api connectors/source/drift labels Jan 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Source Drift: add `OAuth 2.0` authentication option #7337

🎉 Source Drift: add `OAuth 2.0` authentication option #7337

bazarnov commented Oct 25, 2021

bazarnov commented Oct 28, 2021 •

edited by github-actions bot

Loading

alexandr-shegeda left a comment

yurii-bidiuk commented Nov 3, 2021

sherifnada commented Nov 4, 2021

sherifnada Nov 4, 2021

yurii-bidiuk commented Nov 4, 2021

sherifnada commented Nov 4, 2021

bazarnov commented Nov 4, 2021

bazarnov commented Nov 10, 2021

bazarnov commented Nov 16, 2021 •

edited by github-actions bot

Loading

bazarnov commented Nov 16, 2021 •

edited by github-actions bot

Loading

🎉 Source Drift: add OAuth 2.0 authentication option #7337

🎉 Source Drift: add OAuth 2.0 authentication option #7337

Conversation

bazarnov commented Oct 25, 2021

What

How

Pre-merge Checklist

Community member or Airbyter

Airbyter

bazarnov commented Oct 28, 2021 • edited by github-actions bot Loading

alexandr-shegeda left a comment

Choose a reason for hiding this comment

yurii-bidiuk commented Nov 3, 2021

sherifnada commented Nov 4, 2021

sherifnada Nov 4, 2021

Choose a reason for hiding this comment

yurii-bidiuk commented Nov 4, 2021

sherifnada commented Nov 4, 2021

bazarnov commented Nov 4, 2021

bazarnov commented Nov 10, 2021

bazarnov commented Nov 16, 2021 • edited by github-actions bot Loading

bazarnov commented Nov 16, 2021 • edited by github-actions bot Loading

🎉 Source Drift: add `OAuth 2.0` authentication option #7337

🎉 Source Drift: add `OAuth 2.0` authentication option #7337

bazarnov commented Oct 28, 2021 •

edited by github-actions bot

Loading

bazarnov commented Nov 16, 2021 •

edited by github-actions bot

Loading

bazarnov commented Nov 16, 2021 •

edited by github-actions bot

Loading