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

feat: Oauth design #269

Open
wants to merge 15 commits into
base: oauth
Choose a base branch
from
Open

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

@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?

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.

None yet

2 participants