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

Use new context for refreshing the key in background #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ejdem86grx
Copy link

In its current shape the background refresh will never run, if the ctx passed to the go-jwks client will be canceled or expires before the library gets the new key.

Example code:

func External() {
  // 1. create a context with cancel
  ctx, cancel := context.WithCancel(context.Background())
  // 3. cancel is called before the method has finished, and this is causing the refreshKey method to fail because the context is cancelled at this point
  defer cancel()
  key, err := jwks.GetKey(ctx)
  // 2. the key is in the cache, but it should be refreshed, so GetKey starts a goroutine with the ctx passed to the refreshKey method
  return
}

Copy link

@andresvia andresvia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is an old PR, I just created #16

Is anyone reading this?

ping: @s12v , @bigkraig

@bigkraig
Copy link
Contributor

@andresvia yep, it turned out to be a problem for us and we opened this PR

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

Successfully merging this pull request may close these issues.

None yet

3 participants