pysnippet / fastapi-oauth2

Easy to integrate OAuth2 authentication with support for several identity providers.
https://docs.pysnippet.org/fastapi-oauth2
MIT License
58 stars 12 forks source link

🐛 Bug Report - `_access_token` cache leads to wrong user being logged in #47

Closed mrharpo closed 2 months ago

mrharpo commented 2 months ago

Bug description

First off, thanks for the great project! We are trying to see if we can use this as part of our FastAPI application, but found a major bug in the implementation:

TLDR

Storing the OAuth2Core._access_token means the 2nd person to login to the server (and each subsequent user) gets the 1st person's user_data in their token.

core.py

https://github.com/pysnippet/fastapi-oauth2/blob/53973d67472f43b0f3a5ad157465970071ff4206/src/fastapi_oauth2/core.py#L72-L76

Solution

Returning the access_token directly from the _oauth_client works correctly, as far as I can tell.

    @property
    def access_token(self) -> str:
        return self._oauth_client.access_token

Reproduction URL

https://github.com/WGBH-MLA/organ/pull/3

Reproduction steps

Server

from os import getenv

from fastapi import FastAPI
from fastapi_oauth2.claims import Claims
from fastapi_oauth2.client import OAuth2Client
from fastapi_oauth2.config import OAuth2Config
from fastapi_oauth2.middleware import OAuth2Middleware
from fastapi_oauth2.router import router as oauth2_router
from social_core.backends.github import GithubOAuth2

github_client = OAuth2Client(
    backend=GithubOAuth2,
    client_id=getenv('OAUTH2_GITHUB_CLIENT_ID'),
    client_secret=getenv('OAUTH2_GITHUB_CLIENT_SECRET'),
    scope=['user:email'],
    claims=Claims(
        picture='avatar_url',
        identity=lambda user: f"{user.provider}:{user.sub}",
    ),
)
oauth_config = OAuth2Config(
    allow_http=True,
    jwt_secret=getenv('JWT_SECRET'),
    jwt_expires=getenv('JWT_EXPIRES'),
    jwt_algorithm=getenv('JWT_ALGORITHM'),
    clients=[
        github_client,
    ],
)

app = FastAPI()
app.include_router(oauth2_router)
app.add_middleware(
    OAuth2Middleware,
    config=oauth_config,
    callback=lambda auth, user: print(auth, user),
)

Env

JWT_SECRET=superdupersecret
JWT_ALGORITHM=HS256
JWT_EXPIRES=900
OAUTH2_GITHUB_CLIENT_ID=...
OAUTH2_GITHUB_CLIENT_SECRET=...

Steps

  1. Configure GitHub OAuth2 application with:
  2. Run server: uvicorn server:app
  3. Browser 1: Make login authorization request: http://localhost:8000/oauth2/github/authorize
  4. Browser 1 gets correct Authorization cookie
  5. Browser 2: Make login authorization request with different GitHub user
  6. Browser 2: gets Authorization cookie with Browser 1's user_data

Screenshots

DESCRIPTION

Logs

No response

Browsers

No response

OS

No response

ArtyomVancyan commented 2 months ago

@mrharpo, thanks for reporting the issue. I have tested with different OAuth clients but never tested with different accounts of the same OAuth client. I was able to reproduce the issue on the playground application and reviewed your pull request (#48). The thing is that self._oauth_client.access_token is accessible only once, which is why I implemented caching on it. I suggest just setting the self._access_token to None on logout or making it an instance variable.