Skip to content

TDL-27031: Add oauth option for authorization - #79

Merged
prijendev merged 7 commits into
masterfrom
TDL-27031-implement-oauth
Feb 25, 2025
Merged

prijendev merged 7 commits into
masterfrom
TDL-27031-implement-oauth

Conversation

@prijendev

@prijendev prijendev commented Feb 14, 2025 •

Copy link
Copy Markdown
Contributor

Description of change

  • Add option of oauth for authorization.
  • If auth_type is Basic then consider username/password.
  • If auth_type is Oauth, then generate access_token from client_id/secret and consider it.

Manual QA steps

Risks

Rollback steps

  • revert this branch

AI generated code

https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code

  • this PR has been written with the help of GitHub Copilot or another generative AI tool

@prijendev prijendev changed the title Tdl 27031 implement oauth TDL-27031: Add oauth option for authorization Feb 17, 2025
@prijendev
prijendev marked this pull request as ready for review February 17, 2025 06:35
Comment thread tests/unittests/test_authentication.py Outdated
Comment thread tap_zuora/client.py Outdated
Comment thread tests/unittests/test_authentication.py Outdated
Comment thread tap_zuora/client.py Outdated
Comment on lines +99 to +102
if self.auth_type == "Basic":
self.access_token = None
else:
self.access_token = get_access_token(self.username, self.password, URLS[(self.sandbox, self.european)])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if self.auth_type == "Basic":
self.access_token = None
else:
self.access_token = get_access_token(self.username, self.password, URLS[(self.sandbox, self.european)])
if self.auth_type == "Basic":
self.access_token = None
elif self.auth_type == "OAuth":
self.access_token = get_access_token(self.username, self.password, URLS[(self.sandbox, self.european)])
else:
raise Exception("...")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, this is not require as we are already checking any extra values over here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have auth_type validation in the constructor as well to make it more fail safe in case we don't invoke it from here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Add validation in constructor as well

@prijendev
prijendev requested a review from RushiT0122 February 20, 2025 12:30
@prijendev
prijendev merged commit 9867f1f into master Feb 25, 2025
@prijendev prijendev mentioned this pull request Feb 25, 2025
1 task
@dsprayberry
dsprayberry deleted the TDL-27031-implement-oauth branch April 10, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants