ghinstallation icon indicating copy to clipboard operation
ghinstallation copied to clipboard

RFC: add a `SignerWithContext` interface for custom sign implementations

Open jamestelfer opened this issue 9 months ago • 2 comments

The current sign interface has the single Sign method:

Sign(claims jwt.Claims) (string, error)

When implementing an AWS KMS signer, the lack of the context means the remote operation doesn't participate in the main context, so it's unaware of request timeouts or server shutdown.

What about adding:

type SignerWithContext interface {
    Sign(ctx context.Context, claims jwt.Claims) (string, error)
}

An existing Signer implementation can be supported through a simple wrapper function:

type SignerWithContextFunc func Sign(claims jwt.Claims) (string, error)

func (s SignerWithContextFunc) (_ context.Context, claims jwt.Claims) (string, error) {
    return s(claims)
}

// ... 

signer := // Signer implementation
contextSigner := SignerWithContextFunc(signer.Sign)

Then a configuration item for WithContextSigner or similar. Thoughts?

I am able to raise a PR for this, or some more acceptable variation.

jamestelfer avatar May 09 '24 10:05 jamestelfer

I've created a PR fleshing out the idea if you're interested @bradleyfalzon. Feedback welcome!

jamestelfer avatar May 19 '24 14:05 jamestelfer

Another valuable use case for the context is when using telemetry. The current trace is generally carried on the request context, so including the context allows actions taken during signing to be included in the wider request trace.

jamestelfer avatar Jun 10 '24 04:06 jamestelfer