Skip to content

Commit

Permalink
feat!: utilize async lock to overcome dangerous race conditions (#189)
Browse files Browse the repository at this point in the history
Brought into light by @parikls in !186
  • Loading branch information
tomasvotava authored Nov 4, 2024
1 parent ff4438a commit 49fa33a
Show file tree
Hide file tree
Showing 32 changed files with 774 additions and 613 deletions.
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
# We need requirements.txt for tox, but it may not be updated properly, so let's keep it all in pyproject.toml
requirements.txt

.python-version

.vscode/
main.py

Expand Down
6 changes: 0 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,6 @@ Run the tests by calling:
poe test
```

Or using `tox`:

```console
tox
```

## Documentation

Please try to provide documentation for your code. I use `mkdocs` to generate the documentation.
Expand Down
51 changes: 45 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,53 @@ Quick links for the eager ones:
- [Demo site](https://fastapi-sso-example.vercel.app/)
- [Medium articles](https://medium.com/@christos.karvouniaris247)

## Security warning
## Security Notice

Please note that versions preceding `0.7.0` had a security vulnerability.
The SSO instance could share state between requests, which could lead to security issues.
**Please update to `0.7.0` or newer**.
### Version `0.16.0` Update: Race Condition Bug Fix & Context Manager Change

Also, the preferred way of using the SSO instances is to use `with` statement, which will ensure the state is cleared.
See example below.
A race condition bug in the login flow that could, in rare cases, allow one user
to assume the identity of another due to concurrent login requests was recently discovered
by [@parikls](https://github.com/parikls).
This issue was reported in [#186](https://github.com/tomasvotava/fastapi-sso/issues/186) and has been resolved
in version `0.16.0`.

**Details of the Fix:**

The bug was mitigated by introducing an async lock mechanism that ensures only one user can attempt the login
process at any given time. This prevents race conditions that could lead to unintended user identity crossover.

**Important Change:**

To fully support this fix, **users must now use the SSO instance within an `async with`
context manager**. This adjustment is necessary for proper handling of asynchronous operations.

The synchronous `with` context manager is now deprecated and will produce a warning.
It will be removed in future versions to ensure best practices for async handling.

**Impact:**

This bug could potentially affect deployments with high concurrency or scenarios where multiple users initiate
login requests simultaneously. To prevent potential issues and deprecation warnings, **update to
version `0.16.0` or later and modify your code to use the async with context**.

Code Example Update:

```python
# Before (deprecated)
with sso:
openid = await sso.verify_and_process(request)

# After (recommended)
async with sso:
openid = await sso.verify_and_process(request)
```

Thanks to both [@parikls](https://github.com/parikls) and the community for helping me identify and improve the
security of `fastapi-sso`. If you encounter any issues or potential vulnerabilities, please report them
immediately so they can be addressed.

For more details, refer to Issue [#186](https://github.com/tomasvotava/fastapi-sso/issues/186)
and PR [#189](https://github.com/tomasvotava/fastapi-sso/pull/189).

## Support this project

Expand Down
4 changes: 2 additions & 2 deletions docs/how-to-guides/additional-query-params.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ E.g. sometimes you want to specify `access_type=offline` or `prompt=consent` in

@app.get("/google/login")
async def google_login(request: Request):
with google_sso:
async with google_sso:
return await google_sso.get_login_redirect(
redirect_uri=request.url_for("google_callback"),
params={"prompt": "consent", "access_type": "offline"}
)

@app.get("/google/callback")
async def google_callback(request: Request):
with google_sso:
async with google_sso:
user = await google_sso.verify_and_process(request)
# you may now use google_sso.refresh_token to refresh the access token
```
4 changes: 2 additions & 2 deletions docs/how-to-guides/additional-scopes.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ sso = GoogleSSO(client_id="client-id", client_secret="client-secret", scope=["op

@app.get("/google/login")
async def google_login():
with sso:
async with sso:
return await sso.get_login_redirect(redirect_uri=request.url_for("google_callback"))

@app.get("/google/callback")
async def google_callback(request: Request):
with sso:
async with sso:
await sso.verify_and_process(request)
# you may now use sso.access_token to access user's Google calendar
async with httpx.AsyncClient() as client:
Expand Down
2 changes: 1 addition & 1 deletion docs/how-to-guides/redirect-uri-request-time.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ google_sso = GoogleSSO("my-client-id", "my-client-secret")
@app.get("/google/login")
async def google_login(request: Request):
"""Dynamically generate login url and return redirect"""
with google_sso:
async with google_sso:
return await google_sso.get_login_redirect(redirect_uri=request.url_for("google_callback"))

@app.get("/google/callback")
Expand Down
4 changes: 2 additions & 2 deletions docs/how-to-guides/state-return-url.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ google_sso = GoogleSSO("client-id", "client-secret")

# E.g. https://example.com/auth/login?return_url=https://example.com/welcome
async def google_login(return_url: str):
with google_sso:
async with google_sso:
# Send return_url to Google as a state so that Google knows to return it back to us
return await google_sso.get_login_redirect(redirect_uri=request.url_for("google_callback"), state=return_url)

async def google_callback(request: Request):
with google_sso:
async with google_sso:
user = await google_sso.verify_and_process(request)
# google_sso.state now holds your return_url (https://example.com/welcome)
return RedirectResponse(google_sso.state)
Expand Down
4 changes: 2 additions & 2 deletions docs/how-to-guides/use-with-fastapi-security.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ async def protected_endpoint(user: OpenID = Depends(get_logged_user)):
@app.get("/auth/login")
async def login():
"""Redirect the user to the Google login page."""
with sso:
async with sso:
return await sso.get_login_redirect()


Expand All @@ -86,7 +86,7 @@ async def logout():
@app.get("/auth/callback")
async def login_callback(request: Request):
"""Process login and redirect the user to the protected endpoint."""
with sso:
async with sso:
openid = await sso.verify_and_process(request)
if not openid:
raise HTTPException(status_code=401, detail="Authentication failed")
Expand Down
4 changes: 2 additions & 2 deletions docs/tutorials.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ google_sso = GoogleSSO(CLIENT_ID, CLIENT_SECRET, "http://localhost:3000/google/c

@app.get("/google/login")
async def google_login():
with google_sso:
async with google_sso:
return await google_sso.get_login_redirect()

@app.get("/google/callback")
async def google_callback(request: Request):
with google_sso:
async with google_sso:
user = await google_sso.verify_and_process(request)
return user
```
Expand Down
4 changes: 2 additions & 2 deletions examples/facebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@
@app.get("/auth/login")
async def auth_init():
"""Initialize auth and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect(params={"prompt": "consent", "access_type": "offline"})


@app.get("/auth/callback")
async def auth_callback(request: Request):
"""Verify login"""
with sso:
async with sso:
user = await sso.verify_and_process(request)
return user

Expand Down
7 changes: 3 additions & 4 deletions examples/fitbit.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Fitbit Login Example
"""
"""Fitbit Login Example"""

import os
import uvicorn
Expand All @@ -22,14 +21,14 @@
@app.get("/auth/login")
async def auth_init():
"""Initialize auth and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect()


@app.get("/auth/callback")
async def auth_callback(request: Request):
"""Verify login"""
with sso:
async with sso:
return await sso.verify_and_process(request)


Expand Down
7 changes: 3 additions & 4 deletions examples/generic.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""This is an example usage of fastapi-sso.
"""
"""This is an example usage of fastapi-sso."""

from typing import Any, Dict, Union
from httpx import AsyncClient
Expand Down Expand Up @@ -46,14 +45,14 @@ def convert_openid(response: Dict[str, Any], _client: Union[AsyncClient, None])
@app.get("/login")
async def sso_login():
"""Generate login url and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect()


@app.get("/callback")
async def sso_callback(request: Request):
"""Process login response from OIDC and return user info"""
with sso:
async with sso:
user = await sso.verify_and_process(request)
if user is None:
raise HTTPException(401, "Failed to fetch user information")
Expand Down
7 changes: 3 additions & 4 deletions examples/github.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Github Login Example
"""
"""Github Login Example"""

import os
import uvicorn
Expand All @@ -22,14 +21,14 @@
@app.get("/auth/login")
async def auth_init():
"""Initialize auth and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect()


@app.get("/auth/callback")
async def auth_callback(request: Request):
"""Verify login"""
with sso:
async with sso:
user = await sso.verify_and_process(request)
return user

Expand Down
7 changes: 3 additions & 4 deletions examples/gitlab.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Github Login Example
"""
"""Github Login Example"""

import os
import uvicorn
Expand All @@ -24,14 +23,14 @@
@app.get("/auth/login")
async def auth_init():
"""Initialize auth and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect()


@app.get("/auth/callback")
async def auth_callback(request: Request):
"""Verify login"""
with sso:
async with sso:
user = await sso.verify_and_process(request)
return user

Expand Down
4 changes: 2 additions & 2 deletions examples/google.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
@app.get("/auth/login")
async def auth_init():
"""Initialize auth and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect(params={"prompt": "consent", "access_type": "offline"})


@app.get("/auth/callback")
async def auth_callback(request: Request):
"""Verify login"""
with sso:
async with sso:
user = await sso.verify_and_process(request)
return user

Expand Down
7 changes: 3 additions & 4 deletions examples/kakao.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Kakao Login Example
"""
"""Kakao Login Example"""

import os
import uvicorn
Expand All @@ -22,14 +21,14 @@
@app.get("/auth/login")
async def auth_init():
"""Initialize auth and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect()


@app.get("/auth/callback")
async def auth_callback(request: Request):
"""Verify login"""
with sso:
async with sso:
return await sso.verify_and_process(request, params={"client_secret": CLIENT_SECRET})


Expand Down
8 changes: 4 additions & 4 deletions examples/line.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Line Login Example
"""
"""Line Login Example"""

import os
import uvicorn
Expand All @@ -22,13 +21,14 @@
@app.get("/auth/login")
async def auth_init():
"""Initialize auth and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect(state="randomstate")


@app.get("/auth/callback")
async def auth_callback(request: Request):
"""Verify login"""
with sso:
async with sso:
user = await sso.verify_and_process(request)
return user

Expand Down
7 changes: 3 additions & 4 deletions examples/linkedin.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Github Login Example
"""
"""Github Login Example"""

import os
import uvicorn
Expand All @@ -22,14 +21,14 @@
@app.get("/auth/login")
async def auth_init():
"""Initialize auth and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect()


@app.get("/auth/callback")
async def auth_callback(request: Request):
"""Verify login"""
with sso:
async with sso:
user = await sso.verify_and_process(request)
return user

Expand Down
7 changes: 3 additions & 4 deletions examples/microsoft.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Microsoft Login Example
"""
"""Microsoft Login Example"""

import os
import uvicorn
Expand All @@ -24,14 +23,14 @@
@app.get("/auth/login")
async def auth_init():
"""Initialize auth and redirect"""
with sso:
async with sso:
return await sso.get_login_redirect()


@app.get("/auth/callback")
async def auth_callback(request: Request):
"""Verify login"""
with sso:
async with sso:
return await sso.verify_and_process(request)


Expand Down
Loading

0 comments on commit 49fa33a

Please sign in to comment.