[FEATURE] Support multithreading in the OAuth client

[BenZickel]

This issue is identical to another issue which I opened here.
The problem is that simultaneous authorization requests overwrite the same token attribute of the client (see this line).

[algal]

I had a look at this, and a detailed conversation with @johnowhitaker this morning.

Here's my understanding of the particular issue described here, and more broadly what might be done to support secure oauth correctly and comfortably. I believe I'm also summarizing Johno's view but I'll let him chime in to correct me!

• This issue (and the related issue Support multithreading in the OAuth client  fasthtml-example#54) point out that the FasthHTML (FH) does not "support multithreading" in the following specific sense. If the FH library receives two near simultaneous requests, the following could happen:

  1. Request one could store the first user's self.token in the underlying oauthlib Client object, which is in the application scope (not a user scope).
  2. Request two would then store second user's token in the same object, clobbering the first user's token.
  3. In a request three from the first user, the first user would not be able to authenticate because his token would be gone.

• The underlying library oauthlib has no particular support for async, multithreading, or multiprocessing. I don't think FH aims to add support for this. I also don't think we add any new multi-threading specific vulnerabilities to it. In particular since the FH oauth code will run single-threaded, afaict there's no need or benefit to mark out critical sections within an individual function body. This is because there is no situation where distinct HTTP requests will result in concurrent execution of the same Python function in the same execution environment.

• However, I do think there is potentially an issue if the FH library encourages clients to use the library in such a way that they receive user- or session-scoped information, such as a token, and save it into a global application scope. I confess I'm not 100% sure if the current minimal example is showing the wrong thing -- but again, not because of a potential for multithreading to interrupt a critical section within a function body, but because of the way the oauth flow itself is broken up into distinct requests, which means distinct function calls. (What do you think, @johnowhitaker ?) If nothing else, we need to document the right way to use this better.

• On that topic, I believe it is now easier to use it in the right way thanks to some recent simplifications in the library, such as commits e9451...1239708. But I don't think they eliminate the possible for a client to use the library incorrectly, and I think that's possible to do in an absolute way, because it's ultimately going to be up to the application to handle user-scoped information correctly and not store it in an application-scoped context where requests from another user might interact with it.

[johnowhitaker]

Yup, as Alexis says, we use WebApplicationClient which has this behavior of setting .token on the client.

If you're using the OAuth class this isn't a problem since all the logic happens in the redirect function, which is not async, so there won't be multiple versions running simultaneously and fighting over client.token. At the end of that redirect function the only place any user-specific data will persist is in the session (signed and scoped per user), in any database things the developer sets up in their get_auth function (which hopefully they are managing to keep it per-user) and in that lingering .token attribute of the client (which will get overwritten by the next login).

So unless I'm missing something the current behaviour is OK but the request is for a thread-safe alternative (which might require modifying or switching away from oauthlib (or maybe making one ephemeral client per request?). I don't think we have plans for this for now.

PS: closing the other issue for now, to keep the discussion here.

[algal]

@fyi @pydanny , please feel free to share any thoughts.

[jph00]

I think this should be fine now.