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

Wrapper for OAuth URLs #242

Merged
merged 9 commits into from
Apr 16, 2024
Merged

Wrapper for OAuth URLs #242

merged 9 commits into from
Apr 16, 2024

Conversation

bhavyay
Copy link
Contributor

@bhavyay bhavyay commented Feb 22, 2024

Note :- Please follow the below points while attaching test cases document link below:

- If label Tested is added then test cases document URL is mandatory.

- Link added should be a valid URL and accessible throughout the org.

- If the branch name contains hotfix / revert by default the BVT workflow check will pass.

Test Case Document URL
https://docs.google.com/spreadsheets/d/1VlHhQWwJBN0y3WMumA5upH2h52WYi61SxoTKr4XyXy8/edit?usp=sharing

PR Description: Adding support for OAuth URLS(fetch authorize URL, access token, refresh token & revoke token APIs) in SDK. Also, adding support to access Razorpay Public APIs with access token
Solution doc: https://docs.google.com/document/d/1mW1Ql08vndWIB_PjS-HWM_R2Nr63isW3jPsewYbTycE/edit

@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2024

Codecov Report

Attention: Patch coverage is 96.94656% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 96.83%. Comparing base (9423e2b) to head (bfb0bde).

Files Patch % Lines
lib/razorpay/payload_validator.rb 93.47% 3 Missing ⚠️
lib/razorpay/oauth_token.rb 97.91% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master     #242    +/-   ##
========================================
  Coverage   96.83%   96.83%            
========================================
  Files          35       38     +3     
  Lines         631      758   +127     
========================================
+ Hits          611      734   +123     
- Misses         20       24     +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bhavyay bhavyay changed the title Oauth sdk Wrapper for OAuth URLs Feb 22, 2024
Copy link
Member

@Swati707 Swati707 left a comment

Choose a reason for hiding this comment

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

Can we add proper documentations for all newly added methods? Atleast the public methods should be documented.

Comment on lines 25 to 32
**Parameters:**
| Name | Type | Description |
|----------------------|--------|---------------------------------------------------------------------------------------------------------------------------------------------------------|
| client_id* | string | Unique client identifier. |
| redirect_uri* | string | Callback URL used by Razorpay to redirect after the user approves or denies the authorisation request. The client should whitelist the 'redirect_uri'. |
| scopes* | array | Defines what access your application is requesting from the user. You can request one or multiple scopes by adding them to an array as indicated above. |
| state* | string | A random string generated by your service. This parameter helps prevent cross-site request forgery (CSRF) attacks. |
| onboarding_signature | string | A cryptographic string generated by your service using generateOnboardingSignature method in Utils class. Only applicable for accounts created with pre-fill KYC |
Copy link
Member

Choose a reason for hiding this comment

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

Does this formatting needs to be improved?
This is how it looks currently.
Screenshot 2024-02-28 at 11 48 50 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Formatting was done. Had to add additional line after parameters which was causing this issue.

oauth_token = Razorpay::OAuthToken.get_access_token(options)
```

**Parameters:**
Copy link
Member

Choose a reason for hiding this comment

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

Same with this format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Formatting was done. Had to add additional line after parameters which was causing this issue.

Comment on lines +69 to +70
"access_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImp0aSI6IkY1Z0NQYkhhRzRjcUpnIn0.eyJhdWQiOiJGNFNNeEgxanMxbkpPZiIsImp0aSI6IkY1Z0NQYkhhRzRjcUpnIiwiaWF0IjoxNTkyODMxMDExLCJuYmYiOjE1OTI4MzEwMTEsInN1YiI6IiIsImV4cCI6MTYwMDc3OTgxMSwidXNlcl9pZCI6IkYycVBpejJEdzRPRVFwIiwibWVyY2hhbnRfaWQiOiJGMnFQaVZ3N0lNV01GSyIsInNjb3BlcyI6WyJyZWFkX29ubHkiXX0.Wwqt5czhoWpVzP5_aoiymKXoGj-ydo-4A_X2jf_7rrSvk4pXdqzbA5BMrHxPdPbeFQWV6vsnsgbf99Q3g-W4kalHyH67LfAzc3qnJ-mkYDkFY93tkeG-MCco6GJW-Jm8xhaV9EPUak7z9J9jcdluu9rNXYMtd5qxD8auyRYhEgs",
"refresh_token": "def50200f42e07aded65a323f6c53181d802cc797b62cc5e78dd8038d6dff253e5877da9ad32f463a4da0ad895e3de298cbce40e162202170e763754122a6cb97910a1f58e2378ee3492dc295e1525009cccc45635308cce8575bdf373606c453ebb5eb2bec062ca197ac23810cf9d6cf31fbb9fcf5b7d4de9bf524c89a4aa90599b0151c9e4e2fa08acb6d2fe17f30a6cfecdfd671f090787e821f844e5d36f5eacb7dfb33d91e83b18216ad0ebeba2bef7721e10d436c3984daafd8654ed881c581d6be0bdc9ebfaee0dc5f9374d7184d60aae5aa85385690220690e21bc93209fb8a8cc25a6abf1108d8277f7c3d38217b47744d7",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"access_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImp0aSI6IkY1Z0NQYkhhRzRjcUpnIn0.eyJhdWQiOiJGNFNNeEgxanMxbkpPZiIsImp0aSI6IkY1Z0NQYkhhRzRjcUpnIiwiaWF0IjoxNTkyODMxMDExLCJuYmYiOjE1OTI4MzEwMTEsInN1YiI6IiIsImV4cCI6MTYwMDc3OTgxMSwidXNlcl9pZCI6IkYycVBpejJEdzRPRVFwIiwibWVyY2hhbnRfaWQiOiJGMnFQaVZ3N0lNV01GSyIsInNjb3BlcyI6WyJyZWFkX29ubHkiXX0.Wwqt5czhoWpVzP5_aoiymKXoGj-ydo-4A_X2jf_7rrSvk4pXdqzbA5BMrHxPdPbeFQWV6vsnsgbf99Q3g-W4kalHyH67LfAzc3qnJ-mkYDkFY93tkeG-MCco6GJW-Jm8xhaV9EPUak7z9J9jcdluu9rNXYMtd5qxD8auyRYhEgs",
"refresh_token": "def50200f42e07aded65a323f6c53181d802cc797b62cc5e78dd8038d6dff253e5877da9ad32f463a4da0ad895e3de298cbce40e162202170e763754122a6cb97910a1f58e2378ee3492dc295e1525009cccc45635308cce8575bdf373606c453ebb5eb2bec062ca197ac23810cf9d6cf31fbb9fcf5b7d4de9bf524c89a4aa90599b0151c9e4e2fa08acb6d2fe17f30a6cfecdfd671f090787e821f844e5d36f5eacb7dfb33d91e83b18216ad0ebeba2bef7721e10d436c3984daafd8654ed881c581d6be0bdc9ebfaee0dc5f9374d7184d60aae5aa85385690220690e21bc93209fb8a8cc25a6abf1108d8277f7c3d38217b47744d7",
"access_token": "eyJ0e...",
"refresh_token": "def50...",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The response was taken from razorpay public docs. This might easier for partners to understand

@@ -0,0 +1,19 @@
module Razorpay
ValidationType = {
Copy link
Member

Choose a reason for hiding this comment

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

Where is this getting used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when ValidationType[:non_null]

Copy link
Member

@Swati707 Swati707 left a comment

Choose a reason for hiding this comment

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

Discussed the documentation comment on call. There is no such method and class level code documentation added for existing files in this repo. And these methods are direct calls to Rzp public APIs. Hence won’t be adding for the new methods.

Rest LGTM.

@bhavyay bhavyay added the Tested label Apr 16, 2024
@bhavyay bhavyay merged commit 45718c1 into master Apr 16, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants