-
Notifications
You must be signed in to change notification settings - Fork 1
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
Utility for printing JWT #9
Conversation
@@ -39,7 +39,7 @@ type jwtBearer struct { | |||
errMutex *sync.RWMutex | |||
} | |||
|
|||
func NewClientWithJWTBearer(sandbox bool, instanceURL, consumerKey, username string, privateKey []byte, tokenDuration time.Duration, httpClient http.Client) (Client, error) { | |||
func NewClientWithJWTBearer(sandbox bool, instanceURL, consumerKey, username string, privateKey []byte, tokenDuration time.Duration, httpClient http.Client) (*jwtBearer, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this return type from an interface type (Client
) to a struct type (*jwtBearer
) to comply with Go client libraries.
I didn't think it was particularly valuable to also make sure that the returned struct type was exported, as I'm not sure what the use would be for other modules to be able to reference the jwtBearer type directly. Let me know if you think I'm missing something and should convert this to an exported struct type!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems reasonable to me! I agree that ohter modules are unlikely to have a need to reference the jwtBearer type directly.
) | ||
// Sign JWT with the private key | ||
signedJWT, err := token.SignedString(c.rsaPrivateKey) | ||
signedJWT, err := c.JWT() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the way this refactor worked out! 🚀 Makes sense to make this its own separate method and call it here.
sandbox, // whether the instance the client connects to, is a sandbox or not | ||
salesforceInstanceID, // the salesforce instance URL | ||
consumerKey, // the connected app consumer key | ||
username, // the username using the connected app | ||
privateKeyBytes, | ||
3*time.Second, // request timeout for the OAuth new token HTTP request (3 minute max) | ||
http.Client{ // underlying HTTP client making all HTTP calls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good comments. A couple of them felt redundant with the variable names on first look, but I think they all add a little bit information that is not in the variable name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nifty! Looks good to me!
Description
In #8, we would like to replace the deprecated dgrijalva/jwt-go library with the maintained, drop-in replacement golang-jwt/jwt.
As part of the testing considerations for that PR, I would like to compare the JWTs produced using each JWT library. To make that easier, I added a small Go cmd/print-jwt CLI tool to this repository. This tool accepts a series of flags dictating the Salesforce connection parameters you would like to use, performs a login to the Salesforce org specified using the
sfdcclient
package, then simply prints out the JWT used to the console.I think this is a useful tool to have available for debugging and testing purposes more broadly, too.
This PR also removes the
Client
interface from this package thatNewClientWithJWTBearer
used to return to follow our Go client libraries best practices. This necessitates a major version release, as it is not technically fully backwards compatible to change the return type of this constructor.Testing Considerations
Run
go run ./cmd/print-jwt
and verify that the string that is printed to the console is a valid JWT prepped to be sent to Salesforce.Deployment
None -- non-functional service library update.
Versioning
Major