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

RFC: extend options implementations in appTransport, support App ClientID #118

Open
jamestelfer opened this issue May 10, 2024 · 0 comments

Comments

@jamestelfer
Copy link

jamestelfer commented May 10, 2024

GitHub recently announced changes to JWT creation for apps, such that the App ClientID is now preferred over the AppID.

In order to support this, I suggest extending the WithOptions API instead of adding more constructors, and changing the internal implementation of the AppsTransport struct such that there is an aud string field in place of appID int64. This allows both APIs to be supported in a backwards compatible fashion.

For example:

// name needs refinement
// transport could also be an option
NewAppsTransportWithAllOptions(tr http.RoundTripper, opts ...AppsTransportOption) {
    // impl
}

// obvs would only need one of Client or AppID
// validation would apply to make sure audience set after configuration applied.
NewAppsTransportWithAllOptions(tr, WithSigner(s), WithClientID(id), WithAppID(appID))

Optionally, I'd also like to pull the default auth implementation out of the transport itself, so I could use a different implementation that cached the JWT for a while.

Caching helps in situations where an external service is in use to sign requests (like AWS KMS): the JWT can have a 5 minute expiry and be reused a number of times. There is a twofold benefit here: at higher volume, the KMS Sign calls will start to become expensive, and it decreases the latency for the client making the requests.

IMO this can be fully backwards compatible with the existing API, and I'd be happy to create a candidate PR. LMK.

References

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

No branches or pull requests

1 participant