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

[node] Using an authenticated session across multiple processes eg. Service + MQ Worker #2348

Open
elf-pavlik opened this issue Sep 23, 2022 · 8 comments

Comments

@elf-pavlik
Copy link

elf-pavlik commented Sep 23, 2022

We are planning to add a message queue with a worker to https://github.com/janeirodigital/sai-impl-service/
It will handle things like subscribing to Solid Notifications - Webhook and other background jobs that will require an authenticated session.

I wanted to ask if we should expect any issues when the authenticated session gets used by more than one node process. I haven't dug into how refreshing of the tokens is being scheduled but I can imagine some potential issues if two processes would try refreshing tokens without coordinating it via storage. Especially when the refresh token is rotated.

Not sure how related it is to #308 but it might be 🤔

@NSeydoux
Copy link
Contributor

This library has not been designed for multiple processes being authenticated using the same session concurrently. In order for this to be successful, you'll have to be careful about how the Storage implementation that gets passed to the library handles the refresh token. One approach that would probably work (but that's just me thinking out loud) is if each process gets the session from storage before performing an authenticated request, which will enforce the latest refresh token being used (since the refresh token may be rotated on each usage). Concurrency will need to be managed very carefully.

@elf-pavlik
Copy link
Author

I think each process still needs an instance of Session which gets created from serialized data in Storage. Probably there might need to be a way to disable refreshing tokens by each session and instead enable re-hydrating the session from the storage instead. In that case, there would be one instance of the Session which would still be refreshing the token, would it keep serialized tokens in the Storage valid? (also thinking out loud)

@NSeydoux
Copy link
Contributor

Re-hydrating the session from storage is actually based on the refresh token being present: the access token is short-lived, and it is a sensitive piece of data, so it isn't persisted in storage. This means re-hydrating the session necessarily refreshes the token (and rotates the refresh token).

@elf-pavlik
Copy link
Author

If each process has to use refresh-token (which results in it being rotated). I don't really see how the session could be used across multiple processes. At least not without some pesky locking.

Looking at CSS' OP config

It sets the Access Token expiry to 1h and the Refresh token to 24h. Having an option that opts-in to store the Access Token in the storage would make it easier to share the session across processes. Only one process would be responsible for refreshing it, possibly using a scheduler.

BTW we should be already talking about storing the ID Token as in the latest Solid-OIDC. We can already treat the Access Token as it was an ID Token for the sake of converstaion.

@NSeydoux
Copy link
Contributor

That's absolutely true, we are planning on a big refactor of this library to stop relying on the Access Token being issued by the OpenID Provider. When we do it, the way things are stored will evolve to rely on the Credential Management API.

@ThisIsMissEm
Copy link
Contributor

@elf-pavlik in the meantime, if you want to go outside of in-memory storage, you'd need to implement this interface/file, but backed by some sort of database (e.g., redis) — keep in mind that you'll almost definitely want to encrypt whatever you store in there, as the data stored is equivalent to a password.

But as @NSeydoux suggests, all this is planned for a major upheaval.

@elf-pavlik
Copy link
Author

elf-pavlik commented Jan 13, 2023

Thank you @ThisIsMissEm. We do have simple redis backed implementation of IStorage. We also use BullMQ for a job queue. The problem comes when different workers use getSessionFromStorage and each returned instances of Session in each worker will try to refresh tokens.

I imagine that following approach could work in the future (with latest Solid-OIDC):

  • One dedicated worker which schedules jobs to refresh ID Token with OP and stores latest in the redis storage
  • The main service and all other workers rely on latest ID Token being available in redis and get it from there instead of trying to refresh it with OP. Each worker would still have the session do the dance with AS to push the ID Token (as UMA claim) and get an access token based on it.

@ThisIsMissEm
Copy link
Contributor

@elf-pavlik yeah, that'd make sense to me. That or have some sort of "refresh token lock", where by there's a race to the first worker to start the refresh, with an expiry in case it fails/crashes, and then perhaps go from there; But actually having a dedicated worker (not horizontally scalable, but potentially shard-able), which tries to semi-eagerly refresh the token such that the other workers all just grab the latest token & don't try refreshes. Or you could expire the access tokens in redis in accordance with the expiry information (I don't think the SDK exposes that at the moment, see: #2122

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

No branches or pull requests

3 participants