-
Notifications
You must be signed in to change notification settings - Fork 108
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
#1356 implements OAuth2 Two-legged flow #1357
base: devel
Are you sure you want to change the base?
Conversation
d68194c
to
a3b9a8b
Compare
✅ Deploy Preview for dlt-hub-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Questions:
Thank you! |
2cd8d72
to
5f4b8ec
Compare
self.token_expiry = pendulum.now().add(seconds=expires_in) | ||
|
||
|
||
class OAuth2Zoom(OAuth2ImplicitFlow): |
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.
Hey @willi-mueller I think once this out of the draft state this class would be too specific for this file. We could keep the code nonetheless and I would suggest moving this to the docs as an example of OAuth. (Right now there's nothing in the docs related to handling OAuth). What do you think?
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.
Yes, I agree that the version you reviewed was too specific and that any specific implementation, Zoom here, should not be part of the core code but part of the documentation.
I refactored the obtain_token()
to a template method. After this change, it works for many OAuth 2.0 flavors. Also, I did a couple of other refactorings and simplifications of the interface.
Example implementation for Zoom:
class OAuth2Zoom(OAuth2ImplicitFlow):
def build_access_token_request(self) -> Dict[str, Any]:
authentication: str = b64encode(f"{self.client_id}:{self.client_secret}".encode()).decode()
return {
"url": "https://zoom.us/oauth/token",
"headers": {
"Authorization": f"Basic {authentication}",
"Content-Type": "application/x-www-form-urlencoded",
},
"data": self.access_token_request_data,
}
auth = OAuth2Zoom(
access_token_request_data={
"grant_type": "account_credentials",
"account_id": dlt.secrets["sources.zoom.account_id"],
},
client_id=dlt.secrets["sources.zoom.client_id"],
client_secret=dlt.secrets["sources.zoom.client_secret"],
)
Implementation for LinkedIn:
class OAuth2LinkedIn(OAuth2ImplicitFlow):
def build_access_token_request(self) -> Dict[str, Any]:
return {
"url": "https://www.linkedin.com/oauth/v2/accessToken",
"headers": {
"Content-Type": "application/x-www-form-urlencoded",
},
"data": {
**self.access_token_request_data,
"client_id": dlt.secrets["sources.linkedin.client_id"],
"client_secret": dlt.secrets["sources.linkedin.client_secret"],
}
}
auth = OAuth2LinkedIn(
access_token_request_data={
"grant_type": "client_credentials",
},
client_id=dlt.secrets["sources.linkedin.client_id"],
client_secret=dlt.secrets["sources.linkedin.client_secret"],
),
What do you think about this direction? Do you have ideas on how to refine it further?
Thank you for your review and your comments!
…ith token refresh
…s Zoom implementation, that should go to the docs
…rom constructor since this is obtained by the class
cbeb3f2
to
755a680
Compare
for the record: one user in slack reported that he used this OAuth2 implementation successfully for Hivebrite |
Description
This PR implements OAuth2 implicit flow with access token refresh for server-to-server authorization.
Related Issues
Additional Context
Example usage: