-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
add auth verification to fastapi #11704
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
base: master
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
cdrini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm! We tested all the helper endpoints and they did the correct thing (logout excepting). Will bring up with @mekarpeles to do a quick pass of the cookie handling stuff since he's more familiar with authentication.
|
|
||
|
|
||
| @router.post("/account/logout") | ||
| async def logout(request: Request) -> Response: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This endpoint wasn't working correctly ; we think it might be something related to webpy / fastapi storing the session cookie slightly differently?
Here's the set-cookie header from the two systems:
fastapi:
set-cookie
session=""; expires=Wed, 21 Jan 2026 18:38:46 GMT; Max-Age=0; Path=/; SameSite=lax
set-cookie
pd=""; expires=Wed, 21 Jan 2026 18:38:46 GMT; Max-Age=0; Path=/; SameSite=lax
set-cookie
sfw=""; expires=Wed, 21 Jan 2026 18:38:46 GMT; Max-Age=0; Path=/; SameSite=lax
webpy:
set-cookie
pd=; expires=Sun, 15 May 1994 16:50:45 GMT; Path=/
set-cookie
sfw=; expires=Sun, 15 May 1994 16:50:45 GMT; Path=/
set-cookie
session=; expires=Sun, 15 May 1994 16:50:45 GMT; Path=/
cdrini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mekarpeles and I reviewed and this looks good! Marking as need changes to delete those files we talked about.
|
@cdrini all changes are addressed |
This pull request adds FastAPI-based authentication endpoints to Open Library, enabling login, logout, and authentication checks using the same session cookie format as the legacy system. It introduces new authentication dependencies, refactors cookie generation logic for reuse, and includes a test script for local validation. These changes lay the groundwork for a modern API-based authentication flow while maintaining compatibility with existing web.py logic.
FastAPI Authentication Endpoints:
openlibrary/fastapi/account.pywith endpoints for login, logout, and authentication status checks, reusing existing account auditing and session cookie logic for compatibility with the legacy system.openlibrary/asgi_app.py, making the endpoints available.Authentication Logic and Utilities:
openlibrary/fastapi/auth.py, providing dependencies for extracting and verifying authenticated users from session cookies, including models and helper functions for authentication in FastAPI routes.generate_login_code_for_userinopenlibrary/accounts/model.pyand updated theAccount.generate_login_codemethod to use it, ensuring consistent cookie format between legacy and new endpoints. [1] [2]Testing and Validation:
test_fastapi_auth.sh, a shell script for local testing of FastAPI authentication endpoints, covering login, logout, session cookie handling, and compatibility checks.Related to #11133