Skip to content

chore: add public key client validation #1088

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

tiwarishubham635
Copy link
Contributor

@tiwarishubham635 tiwarishubham635 commented May 12, 2025

Fixes

Adds support for PKCV in Node. Reference doc.

  1. Added ValidationClient to BaseTwilio Client
  2. When ValidationClient is set, add request interceptor for adding Twilio client Validation header
  3. ValidationToken creates the JWT token required to be added for Request Validation using keys and credentials.
  4. RequestCanonicalizer canonicalizes the request as per the examples/pkcv.js given in the PKCV doc
  5. Added unit tests for the ValidationToken and RequestCanonicalizer
  6. Added examples/pkcv.js file for example use case

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.

Copy link

@@ -382,6 +412,18 @@ namespace RequestClient {
* Maximum number of request retries for 429 Error responses. Defaults to 3.
*/
maxRetries?: number;
/**
* Validation client for PKCV
Copy link
Contributor

Choose a reason for hiding this comment

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

Elaborate a bit further the use case for introducing ValidationClient. Give some doc links.


getCanonicalizedRequestString(): string {
let canonicalizedRequest = "";
canonicalizedRequest += this.getCanonicalizedMethod() + "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Use template literals to do concatenation

class RequestCanonicalizer {
method: string;
uri: string;
queryParams: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested changes
queryParams: Record<string, string>
headers: Record<string, string>
Since they are a map of String to String.

return "";
}
// sort query params on the basis of '{key}={value}'
const sortedQueryParams = Object.entries(this.queryParams)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we make queryParams of format Racord<String, String>, it will be easier to serialize

class ValidationToken implements ValidationToken.ValidationTokenOptions {
static DEFAULT_ALGORITHM: "RS256" = "RS256";
static ALGORITHMS = ["RS256", "PS256"];
accountSid: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the visibility of these fields?
Please mark them private if it is not be default


/**
* @constructor
* @param opts - The options for the ValidationToken
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param opts - The options for the ValidationToken
* @param opts - The Options used to configure the ValidationToken

this.algorithm = algorithm;
this.ttl = 300;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/**
* Generates a `RequestCanonicalizer` instance for the given HTTP request.
*
* @param request - The HTTP request object containing details such as headers, URL, method, query parameters, and body.
* @throws {Error} If the request URL or method is missing.
* @returns {RequestCanonicalizer} - An instance of `RequestCanonicalizer` initialized with the canonicalized request details.
*/

}

/**
* Create JWT token to be added in the request header for PKCV
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Create JWT token to be added in the request header for PKCV
* Generate a JWT token to include in the request header for PKCV

function ValidationInterceptor(
validationClient: RequestClient.ValidationClient
) {
return function (config: InternalAxiosRequestConfig) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are we doing the exception handling?

* @param request - The request object
* @returns {string} - The JWT token
*/
fromHttpRequest(request: any): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

contains no exception handling

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