feat: Support for token based authentication and addition of helpers for organisation apis #797

AsabuHere · 2024-05-29T03:36:46Z

Fixes

A short description of what this PR does.

The Page class in bearertoken section is responsible for handling the pagination. This fetch "meta" information from the root level and use this information for pagination. For /scim/Users endpoint, this information is not present at root level which is causing the sdk to break. To avoid this added a null check after fetching "meta" from root level and proceed only if its not null

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.

sbansla · 2024-05-29T04:32:50Z

@AsabuHere
Going forward all APIs will be moving towards Bearer Token Authentication, So we don't want to change pagination just for one API.
[Important] Make sure whatever pagination you are using for Orgs API is compatible with other APIs as well.
Add summary to PR.

Rest Proxy supports 2 types of pagination.
I will message you what are those pagination and we don't want to support any 3rd pagination technique.

AsabuHere · 2024-06-03T04:08:28Z

@AsabuHere Going forward all APIs will be moving towards Bearer Token Authentication, So we don't want to change pagination just for one API. [Important] Make sure whatever pagination you are using for Orgs API is compatible with other APIs as well. Add summary to PR.

Rest Proxy supports 2 types of pagination. I will message you what are those pagination and we don't want to support any 3rd pagination technique.

This is not addition of a new pagination mechanism. This is just handling of a null pointer issue. When meta is not present, the sdk is breaking, to prevent that added a check to verify if meta is null or not before actually using it

@sbansla @kridai Can you please approve?

sbansla · 2024-06-10T05:39:25Z

src/main/java/com/twilio/base/noauth/Fetcher.java

+import com.twilio.Twilio;
+
+import com.twilio.TwilioNoAuth;
+import com.twilio.base.Resource;


Create separate Resource for noauth and create all api files

kridai · 2024-06-13T07:34:50Z

@AsabuHere We also need an example of how the auth token based authentication apis will be called via the client /customer. Could you please add an example here

kridai · 2024-06-13T07:40:20Z

src/main/java/com/twilio/TwilioBearerTokenAuth.java


    private static volatile ExecutorService executorService;

    private TwilioBearerTokenAuth() {
    }

-    public static synchronized void init(final String accessToken) {


are we expecting the client to first create a token manager and then pass it here. We should just collect all the info from the client need to initialize and internally create/set the tokenManger if needed

AsabuHere · 2024-06-14T03:27:07Z

@AsabuHere We also need an example of how the auth token based authentication apis will be called via the client /customer. Could you please add an example here

Added

AsabuHere · 2024-06-14T03:49:19Z

@AsabuHere We also need an example of how the auth token based authentication apis will be called via the client /customer. Could you please add an example here

added

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java

sbansla · 2024-06-14T03:59:41Z

src/main/java/com/twilio/http/bearertoken/TokenManagerImpl.java

+    @Setter
+    private final String grantType;
+    private final String clientId;
+    private final String clientSecret;


Lets add all fields to this class which /v1/token api has.

sbansla · 2024-06-14T04:00:12Z

src/main/java/com/twilio/http/bearertoken/TokenManagerImpl.java

+
+    @Override
+    public String refreshAccessToken(String refreshToken){
+        Token token = Token.creator(grantType, clientId).setClientSecret(refreshToken).create();


There is no concept of refresh token now for client credential. Remove this.

sbansla · 2024-06-14T04:02:26Z

examples/BearerTokenAuthentication.md

+    class BearerTokenAuthenticationExamples {
+        public static void main {
+            //Getting access token
+            TokenManager tokenManager = new TokenManagerImpl(GRANT_TYPE, CLIENT_ID, CLIENT_SECRET);


As we discussion on call, Customer don't need to initialise 'TokenManagerImpl', SDK will do it without customer intervention.

sbansla · 2024-06-14T04:06:14Z

src/main/java/com/twilio/TwilioBearerTokenAuth.java

@@ -69,6 +69,7 @@ private static BearerTokenTwilioRestClient buildOAuthRestClient() {

        builder.region(TwilioBearerTokenAuth.region);
        builder.edge(TwilioBearerTokenAuth.edge);
+        builder.tokenManager(TwilioBearerTokenAuth.tokenManager);


We should not be taking tokenManager from customer, SDK should be taking following inputs from customer clientId, clientSecret, grantType from customer.
Above are mandatory.
So there would be 2 constructors

clientId, clientSecret, grantType

clientId, clientSecret, grantType and other variables which are not mandatory.

src/main/java/com/twilio/base/bearertoken/Page.java

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java

sbansla · 2024-06-14T04:11:13Z

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java

@@ -119,9 +128,15 @@ public Response request(BearerTokenRequest request) {
        }
        logRequest(request);
        Response response = httpClient.reliableRequest(request);
+        int statusCode = response.getStatusCode();
+        if(statusCode == 401){
+            this.accessToken = tokenManager.fetchAccessToken();


Can you test a scenario where fetching of access token is failed.
In my opinion TokenCreator will throw the exception and that will be propagated to orgs api, will that be good, propagating one api exception to another api ?

We should do some handling in TokenManager implementation class.
Logging need to be done otherwise during debugging we won't know which API call failed.

You can think and then we can also discuss on this.

src/main/java/com/twilio/rest/previewiam/organizations/TokenCreator.java

sbansla · 2024-06-14T04:40:55Z

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java

@@ -119,9 +128,15 @@ public Response request(BearerTokenRequest request) {
        }
        logRequest(request);
        Response response = httpClient.reliableRequest(request);
+        int statusCode = response.getStatusCode();


there will be scenario where response is null, we have to handle that as well.

sbansla · 2024-06-14T04:54:33Z

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java

@@ -119,9 +128,15 @@ public Response request(BearerTokenRequest request) {
        }
        logRequest(request);


During request logging SDK is printing key of authorization which will be equal to "Bearer actual_token_value"
So in short token is getting logged, which shouldn't be logged.
Can you make sure that clientSecret and token is not logged anywhere.

You might need to override logRequest method in accordance to bearer token request.

sbansla · 2024-06-14T08:07:14Z

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java

@@ -61,6 +64,7 @@ public static class Builder {
        private String edge;
        private HttpClient httpClient;
        private List<String> userAgentExtensions;
+        private TokenManager tokenManager;

        public Builder(String accessToken) {
            this.accessToken = accessToken;


access token setting needs to be removed from here.

sbansla · 2024-06-14T08:08:06Z

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java

@@ -47,6 +49,7 @@ private BearerTokenTwilioRestClient(BearerTokenTwilioRestClient.Builder b) {
        this.httpClient = b.httpClient;


access token needs to be removed from here as well.

… on auto generation

sbansla · 2024-06-20T04:14:51Z

src/main/java/com/twilio/TwilioBearerTokenAuth.java


        if (userAgentExtensions != null) {
            builder.userAgentExtensions(TwilioBearerTokenAuth.userAgentExtensions);
        }

        builder.region(TwilioBearerTokenAuth.region);
        builder.edge(TwilioBearerTokenAuth.edge);
+        builder.tokenManager(TwilioBearerTokenAuth.tokenManager);
+        builder.tokenManager(TwilioBearerTokenAuth.tokenManager);


This line of code is written twice, one line can be removed.

sbansla · 2024-06-20T04:17:30Z

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java

-            throw new AuthenticationException("Access Token is expired.");
+
+        if (accessToken == null || accessToken.isEmpty() || isTokenExpired(this.accessToken)) {
+            this.accessToken = tokenManager.fetchAccessToken();


This code has to be thread safe.

sbansla · 2024-06-20T04:19:30Z

src/main/java/com/twilio/http/bearertoken/OrgsTokenManager.java

+        } catch(Exception e){
+            throw new ApiException("Token creation failed");
+        }
+        return token.getAccessToken();


Add null check for token.

sbansla · 2024-06-20T04:40:16Z

src/main/java/com/twilio/TwilioBearerTokenAuth.java

+    }
+    public static synchronized void init(String grantType, String clientId, String clientSecret, String code, String redirectUri, String audience, String refreshToken, String scope) {
+        validateAuthCredentials(grantType, clientId, clientSecret);
+        tokenManager = new OrgsTokenManager(grantType, clientId, clientSecret, code, redirectUri, audience, refreshToken, scope);


Bug:
Follow below steps:
Case1:

CallOrgs API
If orgs api is called without init, then it will throw null pointer exception. This exception need to be handled.

Case2:

TwilioBearerTokenAuth.init() // Initialize

TwilioBearerTokenAuth.invalidate() // invalidate

CallOrgs API
In case2, previous invalidated token manager will be used, so during invalidate, tokenManager needs to be invalidated.

Once done, test above scenarios.

…nd PUT method in REST calls (#803) * feat: Support for token based authentication and addition of helpers for organisation apis (#797)

AsabuHere requested review from kridai and sbansla May 29, 2024 03:36

Handling null pointers when meta info is null in a response

af54ebd

AsabuHere changed the title ~~Fix: Handling Null pointers when meta information is null in a rest response~~ fix: Handling Null pointers when meta information is null in a rest response Jun 3, 2024

sbansla reviewed Jun 10, 2024

View reviewed changes

Add fetcher class under noauth package

12d20da

kridai reviewed Jun 13, 2024

View reviewed changes

AsabuHere added 4 commits June 13, 2024 09:49

Implementation of token re fetch logic

7c5cd6e

Implementation of token re fetch logic

f89ba9f

Implementation of token re fetch logic

ba5c794

Making token fetch thread safe

a91fb70

sbansla reviewed Jun 14, 2024

View reviewed changes

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java Outdated Show resolved Hide resolved

sbansla reviewed Jun 14, 2024

View reviewed changes

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java Outdated Show resolved Hide resolved

sbansla reviewed Jun 14, 2024

View reviewed changes

src/main/java/com/twilio/base/bearertoken/Page.java Show resolved Hide resolved

sbansla reviewed Jun 14, 2024

View reviewed changes

src/main/java/com/twilio/http/bearertoken/BearerTokenTwilioRestClient.java Outdated Show resolved Hide resolved

sbansla reviewed Jun 14, 2024

View reviewed changes

src/main/java/com/twilio/rest/previewiam/organizations/TokenCreator.java Show resolved Hide resolved

sbansla reviewed Jun 14, 2024

View reviewed changes

AsabuHere added 8 commits June 14, 2024 08:51

Token re fetch logic implementation

93f857b

Token re fetch logic implementation

772f633

Adding dummy file to avoid compilation errors. This will b e replaced…

a88283b

… on auto generation

Adding dummy file to avoid compilation errors. This will b e replaced…

fd9a2ba

… on auto generation

adding token class files

f690533

adding token class files

9acfcaa

Adding null check for response

4171512

uptaking review comments

8716eb1

sbansla reviewed Jun 20, 2024

View reviewed changes

AsabuHere changed the title ~~fix: Handling Null pointers when meta information is null in a rest response~~ feat: Support for token based authentication and addition of helpers for organisation apis Jun 21, 2024

AsabuHere added 2 commits June 21, 2024 03:08

Uptaking review comments

a4ab362

making token fetch thread safe

c046ba6

sbansla approved these changes Jun 21, 2024

View reviewed changes

AsabuHere merged commit b5d7622 into auth-rest-client Jun 21, 2024
7 of 8 checks passed

AsabuHere deleted the auth_nullpointer_fix branch June 21, 2024 14:33

Making token fetch thread safe

fd9731a

AsabuHere added a commit that referenced this pull request Jun 25, 2024

feat: Support for bearer token authentication and no authentication a…

2b32a89

…nd PUT method in REST calls (#803) * feat: Support for token based authentication and addition of helpers for organisation apis (#797)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support for token based authentication and addition of helpers for organisation apis #797

feat: Support for token based authentication and addition of helpers for organisation apis #797

AsabuHere commented May 29, 2024 •

edited

Loading

sbansla commented May 29, 2024 •

edited

Loading

AsabuHere commented Jun 3, 2024

sbansla Jun 10, 2024

kridai commented Jun 13, 2024

kridai Jun 13, 2024

AsabuHere commented Jun 14, 2024

AsabuHere commented Jun 14, 2024

sbansla Jun 14, 2024

sbansla Jun 14, 2024 •

edited

Loading

sbansla Jun 14, 2024 •

edited

Loading

sbansla Jun 14, 2024

sbansla Jun 14, 2024 •

edited

Loading

sbansla Jun 14, 2024

sbansla Jun 14, 2024 •

edited

Loading

sbansla Jun 14, 2024

sbansla Jun 14, 2024

sbansla Jun 14, 2024

sbansla Jun 20, 2024

sbansla Jun 20, 2024

sbansla Jun 20, 2024

sbansla Jun 20, 2024 •

edited

Loading

		@@ -119,9 +128,15 @@ public Response request(BearerTokenRequest request) {
		}
		logRequest(request);

		@@ -47,6 +49,7 @@ private BearerTokenTwilioRestClient(BearerTokenTwilioRestClient.Builder b) {
		this.httpClient = b.httpClient;

feat: Support for token based authentication and addition of helpers for organisation apis #797

feat: Support for token based authentication and addition of helpers for organisation apis #797

Conversation

AsabuHere commented May 29, 2024 • edited Loading

Fixes

Checklist

sbansla commented May 29, 2024 • edited Loading

AsabuHere commented Jun 3, 2024

Choose a reason for hiding this comment

kridai commented Jun 13, 2024

Choose a reason for hiding this comment

AsabuHere commented Jun 14, 2024

AsabuHere commented Jun 14, 2024

Choose a reason for hiding this comment

sbansla Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

sbansla Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbansla Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbansla Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbansla Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

AsabuHere commented May 29, 2024 •

edited

Loading

sbansla commented May 29, 2024 •

edited

Loading

sbansla Jun 14, 2024 •

edited

Loading

sbansla Jun 14, 2024 •

edited

Loading

sbansla Jun 14, 2024 •

edited

Loading

sbansla Jun 14, 2024 •

edited

Loading

sbansla Jun 20, 2024 •

edited

Loading