-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
🎉 Source Google Directory: support oauth #7409
🎉 Source Google Directory: support oauth #7409
Conversation
/test connector=source-google-directory
|
/test connector=source-google-directory
|
@@ -4,18 +4,97 @@ | |||
"$schema": "http://json-schema.org/draft-07/schema#", | |||
"title": "Google Directory Spec", | |||
"type": "object", | |||
"required": ["credentials_json", "email"], | |||
"required": ["credentials"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would it work w/ old config format with email / credentials_json fields in the root? You need to remove required field and set additionalProperties to true if you want make it backward compatible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also use OneOf in the required field
"description": "The token for obtaining new access token", | ||
"airbyte_secret": true | ||
}, | ||
"scopes": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you dont need scopes, just client_id, client_secret and refresh_token
"oauth2Specification": { | ||
"rootObject": ["credentials", 0], | ||
"oauthFlowInitParameters": [["client_id"], ["client_secret"]], | ||
"oauthFlowOutputParameters": [["access_token"], ["refresh_token"]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove access_token
/test connector=source-google-directory
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't scrutinize the implementation, mostly looked at docs/UX. Do we need unit tests for auth?
@@ -37,7 +37,9 @@ This connector attempts to back off gracefully when it hits Directory API's rate | |||
|
|||
## Getting started | |||
|
|||
### Requirements | |||
Google APIs use the OAuth 2.0 protocol for authentication and authorization. The Source supports [Web server application](https://developers.google.com/identity/protocols/oauth2#webserver) and [Service accounts](https://developers.google.com/identity/protocols/oauth2#serviceaccount) scenarios. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Google APIs use the OAuth 2.0 protocol for authentication and authorization. The Source supports [Web server application](https://developers.google.com/identity/protocols/oauth2#webserver) and [Service accounts](https://developers.google.com/identity/protocols/oauth2#serviceaccount) scenarios. | |
Google APIs use the OAuth 2.0 protocol for authentication and authorization. This connector supports [Web server application](https://developers.google.com/identity/protocols/oauth2#webserver) and [Service accounts](https://developers.google.com/identity/protocols/oauth2#serviceaccount) scenarios. |
Can you also add a section explaining the difference in setup between OSS and Cloud? Similar to this one https://docs.airbyte.io/integrations/sources/google-analytics-v4#getting-started-airbyte-cloud
"type": "object", | ||
"oneOf": [ | ||
{ | ||
"title": "Web server application", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
making this suggestion because the non-technical personas explained in the UX handbook e.g: data analyst won't know what a "Web Server Application" means
"title": "Web server application", | |
"title": "Sign in via Google (Oauth)", |
} | ||
}, | ||
{ | ||
"title": "Service accounts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"title": "Service accounts", | |
"title": "Service account Key", |
@@ -82,6 +82,7 @@ write_standard_creds source-google-analytics-v4 "$GOOGLE_ANALYTICS_V4_TEST_CREDS | |||
write_standard_creds source-google-analytics-v4 "$GOOGLE_ANALYTICS_V4_TEST_CREDS_SRV_ACC" "service_config.json" | |||
write_standard_creds source-google-analytics-v4 "$GOOGLE_ANALYTICS_V4_TEST_CREDS_OLD" "old_config.json" | |||
write_standard_creds source-google-directory "$GOOGLE_DIRECTORY_TEST_CREDS" | |||
write_standard_creds source-google-directory "$GOOGLE_DIRECTORY_TEST_CREDS_OAUTH" "config_oauth.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does SAT use this config?
is there any tests for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Conflicts: # airbyte-integrations/connectors/source-google-directory/source_google_directory/api.py
/test connector=source-google-directory
|
/publish connector=connectors/source-google-directory
|
/publish connector=connectors/source-google-directory
|
/publish connector=connectors/source-google-directory
|
What
Supporting oauth
How
Source Google Directory already supports authentication via Service account. So oauth webflow support was added. And specification was updated.
Closes #6265
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog example