You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 optionNewAppsTransportWithAllOptions(trhttp.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.
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 theAppsTransport
struct such that there is anaud string
field in place ofappID int64
. This allows both APIs to be supported in a backwards compatible fashion.For example:
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
The text was updated successfully, but these errors were encountered: