JWT -> Redis #3574
JWT -> Redis #3574
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
pablonyx
force-pushed
the
jwt_to_redis_for_multitenant
branch
from
57a94c9 to
639d82f
Compare
pablonyx
force-pushed
the
jwt_to_redis_for_multitenant
branch
from
99d3284 to
959ca25
Compare
| logger.debug( | ||
| "Token data not found or expired in Redis, defaulting to POSTGRES_DEFAULT_SCHEMA" | ||
| ) | ||
| return POSTGRES_DEFAULT_SCHEMA |
There was a problem hiding this comment.
What are the implications of returning POSTGRES_DEFAULT_SCHEMA in the multi tenant case?
There was a problem hiding this comment.
Auth error (which is what we'd want!)
backend/onyx/auth/users.py
Outdated
|
|
||
| def __init__( | ||
| self, | ||
| lifetime_seconds: Optional[int] = None, |
There was a problem hiding this comment.
Do we want a default of none if the whole point of this was to enforce expiration?
There was a problem hiding this comment.
Went along with FastAPI User's convention: https://github.com/fastapi-users/fastapi-users/blob/9d78b2a35dc7f35c2ffca67232c11f4d27a5db00/fastapi_users/authentication/strategy/redis.py#L11
Purpose of the PR was more so to enforce logout functionality-related expiration, but I think we should probably add a constant or config related to this! Will do so
backend/onyx/auth/users.py
Outdated
| async def read_token( | ||
| self, token: Optional[str], user_manager: BaseUserManager[User, uuid.UUID] | ||
| ) -> Optional[User]: | ||
| if not token: |
There was a problem hiding this comment.
Should token be optional if we just return None immediately when it is None?
There was a problem hiding this comment.
Similar to above, but agree! Unnecessary
| {!NEXT_PUBLIC_CLOUD_ENABLED && ( | ||
| <Checkbox | ||
| label="Anonymous Users" | ||
| sublabel="If set, users will not be required to sign in to use Danswer." |
Description
[Provide a brief description of the changes in this PR]
How Has This Been Tested?
[Describe the tests you ran to verify your changes]
Backporting (check the box to trigger backport action)
Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.