Skip to content

feat: Oauth design #269

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

Merged
merged 16 commits into from
Apr 2, 2025
Merged

Conversation

manisha1997
Copy link
Contributor

@manisha1997 manisha1997 commented Mar 23, 2025

Feature

This PR adds Oauth to twilio-go.
Changes added:

  1. Customers will now use the ClientCredentialProvider object and pass Client Credential parameters.
  2. The struct Client has Oauth object which contains the following:
    a. The IamService Object used --> This will act as an interface for any IAM service we plan to access
    b. The Credentials details
  3. Added test cases

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

@manisha1997 manisha1997 changed the title Oauth design feat: Oauth design Mar 23, 2025
@manisha1997 manisha1997 changed the base branch from main to oauth March 23, 2025 10:25
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces OAuth support by adding a new token authentication mechanism and updating corresponding API service and client methods. Key changes include:

  • A new package module (oauth.go) that implements token fetching and expiration using JWT.
  • Updates to REST API documentation, including new endpoints and model definitions for OAuth tokens.
  • Enhancements in the client and base client to support OAuth integration alongside basic authentication.

Reviewed Changes

Copilot reviewed 62 out of 62 changed files in this pull request and generated no comments.

File Description
oauth.go Adds TokenAuth and APIOAuth implementations for OAuth token handling.
oauth_test.go Introduces tests for token fetching and expiration checking.
rest/iam/v1/, rest/preview_iam/v1/ Updates and adds documentation and API service methods for OAuth.
client/client.go and client/base_client.go Enhances client functionality to support OAuth with new getters/setters.
Comments suppressed due to low confidence (2)

oauth.go:71

  • Setting the OAuth interface to nil before creating a token might lead to unintended behavior. Please review this line to ensure that the OAuth object is managed correctly.
a.iamService.RequestHandler().Client.SetOauth(nil)

client/client.go:235

  • [nitpick] Consider renaming 'SetOauth' to 'SetOAuth' for consistency with the 'GetOAuth' method and to improve readability.
func (c *Client) SetOauth(oauth OAuth) {

Copy link
Contributor

@kevinburkesegment kevinburkesegment left a comment

Choose a reason for hiding this comment

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

OK, we're getting closer. Still hoping for a test on expirations with UTC vs. other time zone.

@manisha1997
Copy link
Contributor Author

OK, we're getting closer. Still hoping for a test on expirations with UTC vs. other time zone.

I have now coverted all date to UTC. Do we still need these tests?

@manisha1997 manisha1997 merged commit f060659 into twilio:oauth Apr 2, 2025
1 check passed
manisha1997 added a commit that referenced this pull request Apr 7, 2025
* feat: Oauth design (#269)

* feat: oauth feature

* feat: auth feature

* refactor the interface

* feat: Oauth implementation

* feat: remove no auth client

* feat: Add test cases

* chore: address review comments

* chore: address review comments

* feat: Add docs

* feat: Add docs

* chore: address review comments

* feat: addressed review comments

* feat: addressed review comments

* feat: addressed review comments

* feat: addressed review comments

* feat: added public oauth examples

---------

Co-authored-by: kburke <[email protected]>

* chore: fix test cases

* chore: getter for oauth

* chore: getter for oauth

* chore: getter for oauth

* chore: add cluster tests

* chore: add cluster tests

* chore: add oauth test

* chore: add oauth test

* chore: oauth test

* chore: oauth test

* chore: oauth test

* chore: oauth test

* chore: oauth test

* chore: Update oauth.go

Co-authored-by: Kevin Burke <[email protected]>

* chore: oauth test

* chore: update cluster-test command with -trimpath flag

* chore: oauth test

* chore: refactor to use cached token

* chore: refactor to use cached token

* chore: rename client

* chore: handle token initializtion

* chore: handle token initializtion

* chore: add cluster tests

* chore: add cluster tests

* chore: add cluster tests

* chore: Fix incorrect assert.Nil parameters in test

* chore: Comment out TestTokenAuthFetchTokenException function

* chore: Remove error wrapping in OAuth token retrieval

* chore: add cluster tests

* chore: add cluster tests

* chore: add cluster tests

* chore: add cluster tests

* chore: add cluster tests

* chore: fix unix time stamp

* chore: refactor token params and expiration check

---------

Co-authored-by: kburke <[email protected]>
Co-authored-by: Kevin Burke <[email protected]>
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