ghinstallation
ghinstallation copied to clipboard
RFC: extend options implementations in appTransport, support App ClientID
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.