Skip to content
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

[EPIC] Support single use refresh tokens (connectors can update their configs during a sync) #3990

Closed
davinchia opened this issue Jun 9, 2021 · 31 comments
Assignees
Labels
area/connectors Connector related issues area/oauth autoteam CDK Connector Development Kit Epic from/connector-ops type/enhancement New feature or request

Comments

@davinchia
Copy link
Contributor

davinchia commented Jun 9, 2021

Tell us about the problem you're trying to solve

Today we do not handle the case where a new refresh token is returned when a new access token is granted.

This is because there doesn't exist a Protocol message that updates configs. This will probably involve adding a message similar to STATE but updates the config.

Describe the solution you’d like

We should provide a way to save a returned refresh token.

@davinchia davinchia added the type/enhancement New feature or request label Jun 9, 2021
@davinchia davinchia changed the title Python HTTP CDK Authenticator should expose a manner to save refresh token. Python HTTP CDK OAuth Authenticator should expose a manner to save refresh token. Jun 9, 2021
@davinchia davinchia added the CDK Connector Development Kit label Jun 9, 2021
@santels
Copy link

santels commented Jun 9, 2021

To add to the context,

I'm currently working on a source connector that's implementing "short-lived access tokens and long-lived refresh tokens".

Based on the documentation of the source I'm working on, they return a new refresh_token after the renewal (of the access token) and they require the new refresh_token to be used in the succeeding token refresh calls.

From their docs:

Each time you perform a token refresh, you should save the new refresh token returned in the response

From this specification, I need to have the most updated refresh_token for the succeeding token refresh calls to be valid.

@sherifnada sherifnada changed the title Python HTTP CDK OAuth Authenticator should expose a manner to save refresh token. Allow a connector to update its config Jun 9, 2021
@sherifnada sherifnada added area/platform issues related to the platform area/connectors Connector related issues labels Jun 9, 2021
@avaidyanatha
Copy link
Contributor

@sherifnada @davinchia

Currently, @santels is blocked on contributing their Xero connector because of this issue. How big of a change would this be to implement?

@davinchia
Copy link
Contributor Author

To me, the task that takes more time here is extending the protocol. We have to make sure we do it correctly. Making the CDK modification should be a matter of prioritisation after. Do you agree @sherifnada ?

@avaidyanatha is there are preferred deadline for this or is it asap?

@wissevrowl
Copy link
Contributor

I want to write two source connectors which both require the use of Authorization grant followed by rotating refresh tokens.

In the meantime I am thinking of using external storage (like azure blob or ftp) to store this azure token outside of the connector until this feature is complete. Or is there another way to more elegantly solve this?

@sherifnada
Copy link
Contributor

@wissevrowl until we fix this issue that's probably your best bet

@akvadrako
Copy link

I have this problem with the Intuit Quickbooks source. The old refresh token is expired when a new one is granted (or used, possibly). The singer tap itself does handle it in a fashion ­– it overwrites the config.json file with the new token. However this doesn't work with the check command.

Is the protocol under discussion? It seems something simple like this would be sufficient?

{"type": "CONFIG", "config": {"refresh_token": "...", "access_token": "..."}}

@RickRen7575
Copy link
Contributor

Has anyone come up with a way around this for now? @akvadrako ?

@akvadrako
Copy link

I spend a day on this and gave up. The singer tap could handle it, but the airbyte adapter would delete the updated config file right away, throwing away the refresh token. I tried to patch the tap and the connector, but there were plenty of issues with incompatible versions since none of the dependencies are versioned.

@RickRen7575
Copy link
Contributor

@akvadrako so the solution was just to do this outside of Airbyte?

@akvadrako
Copy link

akvadrako commented Dec 20, 2021

Yes, in the end we didn't use the airbyte connector.

@thomas-vl
Copy link
Contributor

@sherifnada can you provide an ETA for this to become available? This is a serious blocker for a while now.

@sherifnada sherifnada changed the title Allow a connector to update its config Support single use refresh tokens Jan 17, 2022
@RickRen7575
Copy link
Contributor

Any update on this? How can we help?

@evantahler
Copy link
Contributor

I happen to know that Hubspot has rather short-lived oAuth refresh tokens. Is what we do within that connector generalizable?

@evantahler
Copy link
Contributor

@RickRen7575 this isn't yet prioritized... we will comment here when it is!

@RickRen7575
Copy link
Contributor

Just checking in here!

@evantahler
Copy link
Contributor

evantahler commented Oct 11, 2022

We are going to start looking into this in Q4! This is "Support single use refresh tokens" in our roadmap.

@supertopher
Copy link
Contributor

supertopher commented Nov 9, 2022

I talked to @davinchia and @pmossman to add context on customers effected by this story
TCS believes this story will still be done this quarter.
turns out @evantahler 's team is on this one.. but we still are expecting this this quarter to the best of my knowledge

@RickRen7575
Copy link
Contributor

RickRen7575 commented Nov 9, 2022 via email

@evantahler
Copy link
Contributor

Noting that this work will help troubleshoot:

@RickRen7575
Copy link
Contributor

RickRen7575 commented Nov 10, 2022 via email

@evantahler
Copy link
Contributor

Noting that this work will help troubleshoot:

Those are additional issues we see that solving this will /also/ help!

@midavadim
Copy link
Contributor

#8019
Source Typeform: support oauth:
ISSIE: #7814
PR: #8019
blocked since the connector uses single-use refresh token, which requires proper processing from platform side

@evantahler evantahler changed the title Support single use refresh tokens (connectors can update their configs during a sync) [EPIC] Support single use refresh tokens (connectors can update their configs during a sync) Jan 25, 2023
@evantahler evantahler assigned pedroslopez and unassigned evantahler Feb 2, 2023
@evantahler
Copy link
Contributor

@pedroslopez I think that it is time to close this epic! The platform and CDK now fully support AirbyteControlMessages to allow connectors to change their configuration. Future work will be done in individual connectors.

@leo-schick
Copy link
Contributor

@evantahler is that true as well for the low-code CDK? What authentication do I need to use there to support changing refresh tokens?

@girarda
Copy link
Contributor

girarda commented Mar 23, 2023

@leo-schick, this isn't available yet with the low-code CDK. You can track the issue here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/oauth autoteam CDK Connector Development Kit Epic from/connector-ops type/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests