Added caching of tokens

[xgryph]

No description provided.

[addaleax]

Hi @xgryph, thank you for the PR! This is great to see 🙂

I think there are a few conceptual details we'll want to work out here, but I don't think we'd be fundamentally opposed to adding this feature to the plugin.

First of all, just going to provide some context, we've been in a similar situation before ourselves, where we want to coordinate access to the same OIDC authentication session across multiple Node.js processes. At that point, we decided to use a single "main" instance of the plugin in one process, then share that with other processes via local RPC – you can find our implementation of that here, if you are curious.

This avoids issues like the one I've hinted at here – coordinating communication with outside entities (the IdP and the human being in front of the machine, for example) is something we need.

Similarly, we've also had the need to share state across multiple processes that were running at different times, rather than the same time (i.e. a "stay logged in" type of logic). That is implemented through the serializedState/serialize() API here, essentially allowing a plugin instance to be saved to disk and restored later.

So, even though this is a great PR code-wise, I think it's worth thinking about how to address the use case in question without these issues. Maybe adding some sort of locking API is already enough – whether that's easy enough to implement for you as a consumer is another question, though?

I'd also be curious to hear more about your specific use cases and what you're building with this plugin in general – you're mentioning Jest workers, which I would assume refers to an automated test environment, where we wouldn't have typically expected this plugin to be used.

And lastly, I have to mention that we have a CLA, at https://www.mongodb.com/legal/contributor-agreement, and for legal reasons we can only accept external contributions if it has been signed. The code obviously remains open source and Apache-2.0-licensed, of course.

[addaleax]

I think one particular scenario we'd want to avoid is that, if two plugin instances reach this point at the same time and read an expired token from the cache, they both start the full flow, including potential human-facing interactions such as spawning a browser or both attempting to perform a token refresh at the same time

[xgryph]

Hi @addaleax ,

Thanks for your detailed comment, sorry my PR seems to be premature, I thought I was raising against my private fork so I could do some more testing locally before signing the CLA and writing up the PR.

My use case that I'm migrating how our developers connect to MongoDB to be via SSO and part of this task involves refactoring our unit tests. These tests spawn 10 workers and currently this means the developer running the tests locally would see 10 auth flow windows load up which isn't great for their quality of life.

With this PR I was hoping to bootstrap an initial connection when the test first fires up thus eliminating the duplicated auth flows. But it is still in the early phase of work. And maybe there's a better way...

Andy

[addaleax]

sorry my PR seems to be premature, I thought I was raising against my private fork so I could do some more testing locally before signing the CLA and writing up the PR.

No worries at all! It's a good PR for sure, but I do think there's at least this one core issue we'd want to figure out. And unfortunately, I don't immediately have a good solution to it either.

That being said, I would love for something like what this PR proposes to actually happen. I think the fact that we've implemented two APIs ourselves for sharing OIDC auth state between processes, either temporally concurrent or temporally separate, shows that there's a need for this type of logic, and if we can add it to the plugin itself in a clean way that's more generic than what's currently available, I'm all ears.

With this PR I was hoping to bootstrap an initial connection when the test first fires up thus eliminating the duplicated auth flows. But it is still in the early phase of work. And maybe there's a better way...

Depending on your setup (essentially, if it's reasonable to assume that the tokens will be valid longer than the tests run), you may be able to work around this by spinning up an instance of oidc-plugin, doing the regular auth flow, then serializing using .serialize() and subsequently pass this as serializedState to the individual tests. That won't lead to any actual synchronization between the different plugin instances, but it might just work.