🎉 Source Slack: Implement OAuth support with OAuth authenticator #6570
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
area/connectors
|
/test connector=connectors/source-slack
|
avida
reviewed
Sep 30, 2021
avida
approved these changes
Sep 30, 2021
htrueman
approved these changes
Sep 30, 2021
docs/integrations/sources/slack.md
Outdated
| @@ -101,6 +108,7 @@ We recommend creating a restricted, read-only key specifically for Airbyte acces | |||
|
|
|||
| | Version | Date | Pull Request | Subject | | |||
| | :------ | :-------- | :----- | :------ | | |||
| | 0.1.12 | 2021-08-?? | [????](https://github.com/airbytehq/airbyte/pull/????) | Implement OAuth support with OAuth authenticator | | |||
There was a problem hiding this comment.
don't forget to replace it
yevhenii-ldv
changed the title
|
/test connector=connectors/source-slack
|
Zirochkaa
approved these changes
Oct 1, 2021
|
/test connector=connectors/source-slack
|
sherifnada
reviewed
Oct 6, 2021
airbyte-integrations/connectors/source-slack/source_slack/source.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-slack/source_slack/spec.json
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-slack/source_slack/spec.json
Outdated
Show resolved
Hide resolved
| if credentials_title == "Default OAuth2.0 authorization": | ||
| # We can get `refresh_token` only if the token rotation function is enabled for the Slack Application. | ||
| # If it is disabled, then we use the generated `access_token`, which acts without expiration. | ||
| if credentials.get("refresh_token", "").strip(): |
There was a problem hiding this comment.
we should use the option name, as that is the only thing guaranteed to be uniquely present
There was a problem hiding this comment.
There is a fork here that needs to be handled. If Token rotation is enabled for the application, then in the response to the oauth.v2.access request we will receive both Access token(which expires after some time) and Refresh token(with which we can create a new Access token).
However, if the Token rotation function is disabled for the application, then in this case we will receive only Access token in the response to oauth.v2.access and it will be unlimited.
For this reason, we cannot indicate in the specification that Refresh Token is a required field (since it simply may not exist) and we cannot remove Access Token from required (since it may be unlimited and we will need to use it).
| "type": "object", | ||
| "title": "OAuth2.0 authorization", | ||
| "required": [ | ||
| "access_token", |
There was a problem hiding this comment.
refresh token is the required one since it can be used to generate access tokens
Suggested change
| "access_token", | |
| "refresh_token", |
| "examples": ["slack-client-secret-example"], | ||
| "airbyte_secret": true | ||
| }, | ||
| "access_token": { |
There was a problem hiding this comment.
i suggest we remove the access token since it expires anyways, and just rely on refresh token
airbyte-integrations/connectors/source-slack/source_slack/spec.json
Outdated
Show resolved
Hide resolved
| @@ -285,16 +284,36 @@ def request_body_json(self, stream_slice: Mapping = None, **kwargs) -> Optional[ | |||
|
|
|||
|
|
|||
| class SourceSlack(AbstractSource): | |||
| def _get_authenticator(self, config: Mapping[str, Any]): | |||
There was a problem hiding this comment.
this should be backwards compatible with the previous config format to prevent a breaking change for current users
|
/test connector=connectors/source-slack
|
sherifnada
approved these changes
Oct 6, 2021
| credentials = config.get("credentials") | ||
| credentials_title = credentials.get("option_title") | ||
| if credentials_title == "Default OAuth2.0 authorization": | ||
| # We can get `refresh_token` only if the token rotation function is enabled for the Slack Application. |
There was a problem hiding this comment.
Suggested change
| # We can get `refresh_token` only if the token rotation function is enabled for the Slack Application. | |
| # We can get `refresh_token` only if the token rotation function is enabled for the Slack Oauth Application. |
|
/publish connector=connectors/source-slack
|
…urochkin/slack-oauth-support
|
/publish connector=connectors/source-slack
|
schlattk
pushed a commit
to schlattk/airbyte
that referenced
this pull request
Jan 4, 2022
…bytehq#6570) * Source Slack: Implement OAuth support with OAuth authenticator
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
closes #6256 .
How
Describe the solution
Recommended reading order
x.javay.pythonPre-merge Checklist
Expand the relevant checklist and delete the others.
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing./publishcommand described here