Skip to content
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

After modifying the password, need to disable the token of other clients. #14

Closed
dongfengweixiao opened this issue Oct 6, 2022 · 10 comments

Comments

@dongfengweixiao
Copy link
Contributor

dongfengweixiao commented Oct 6, 2022

def password_changed():
  ......
  access_token_key = "user:{}:access_token".format(user_id)
  redis.sadd(access_token_key, new_access_token)
  redis.expore(access_token_key, ACCESS_TOKEN_EXPIRE_MINUTES)

  refresh_token_key = "user:{}:refresh_token".format(user_id)
  redis.sadd(refresh_token_key, new_refresh_token)
  redis.expore(refresh_token_key, REFRESH_TOKEN_EXPIRE_MINUTES)
  ......
def get_current_user(required_roles: List[str] = None) -> User:
    async def current_user(token: str = Depends(reusable_oauth2)) -> User:        
        try:
            payload = jwt.decode(token, settings.SECRET_KEY, algorithms=[security.ALGORITHM])
        except (jwt.JWTError, ValidationError):
            raise HTTPException(
                status_code=status.HTTP_403_FORBIDDEN,
                detail="Could not validate credentials",
            )
        id = payload["sub"]
        access_token_key = "user:{}:access_token".format(id)
        valid_access_tokens = redis.smembers(access_token_key, token)
        if valid_access_tokens and token not in valid_access_tokens:
            raise HTTPException(
                status_code=status.HTTP_403_FORBIDDEN,
                detail="Could not validate credentials",
            )

        user: User = await crud.user.get(id=payload["sub"])
        if not user:
            raise HTTPException(status_code=404, detail="User not found")
......
@jonra1993
Copy link
Owner

Hello, @dongfengweixiao yes this is a really important feature. I am going to add it.

@dongfengweixiao
Copy link
Contributor Author

Hello, @jonra1993 The above code may help you.

@jonra1993
Copy link
Owner

Hi @dongfengweixiao this feature has been implemented in this commit c7c38d5 Please check it.

@dongfengweixiao
Copy link
Contributor Author

Hi @dongfengweixiao this feature has been implemented in this commit c7c38d5 Please check it.

The password change function is available, but there is a problem: the old refresh token is still valid.
无标题流程图 drawio

@dongfengweixiao
Copy link
Contributor Author

diff --git a/fastapi-alembic-sqlmodel-async/app/api/v1/endpoints/login.py b/fastapi-alembic-sqlmodel-async/app/api/v1/endpoints/login.py
index 0d929c1..50491aa 100644
--- a/fastapi-alembic-sqlmodel-async/app/api/v1/endpoints/login.py
+++ b/fastapi-alembic-sqlmodel-async/app/api/v1/endpoints/login.py
@@ -1,5 +1,5 @@
 from datetime import timedelta
-from typing import Any
+from typing import Any, Optional
 from fastapi import APIRouter, Body, Depends, HTTPException
 from app.core.security import get_password_hash
 from app.core.security import verify_password
@@ -27,12 +27,13 @@ class TokenType(str, Enum):
 
 
 async def add_token_to_redis(
-    redis_client: Redis, user: User, token: str, token_type: TokenType, expire_time: int
+    redis_client: Redis, user: User, token: str, token_type: TokenType, expire_time: Optional[int] = None
 ):
     token_key = f"user:{user.id}:{token_type}"
     print("token_key", token_key)
     await redis_client.sadd(token_key, token)
-    await redis_client.expire(token_key, expire_time)
+    if expire_time:
+        await redis_client.expire(token_key, expire_time)
     print("done")
 
 
@@ -70,20 +71,24 @@ async def login(
         refresh_token=refresh_token,
         user=user,
     )
-    await add_token_to_redis(
-        redis_client,
-        user,
-        access_token,
-        TokenType.ACCESS,
-        settings.ACCESS_TOKEN_EXPIRE_MINUTES,
-    )
-    await add_token_to_redis(
-        redis_client,
-        user,
-        refresh_token,
-        TokenType.REFRESH,
-        settings.REFRESH_TOKEN_EXPIRE_MINUTES,
-    )
+    access_token_key = f"user:{user.id}:{TokenType.ACCESS}"
+    valid_access_tokens = await redis_client.smembers(access_token_key)
+    if valid_access_tokens:
+        await add_token_to_redis(
+            redis_client,
+            user,
+            access_token,
+            TokenType.ACCESS,
+        )
+    refresh_token_key = f"user:{user.id}:{TokenType.REFRESH}"
+    valid_refresh_tokens = await redis_client.smembers(refresh_token_key)
+    if valid_refresh_tokens:
+        await add_token_to_redis(
+            redis_client,
+            user,
+            refresh_token,
+            TokenType.REFRESH,
+        )
 
     return create_response(meta=meta_data, data=data, message="Login correctly")
 
@@ -159,6 +164,12 @@ async def get_refresh_token(
     except (jwt.JWTError, ValidationError):
         raise HTTPException(status_code=403, detail="Refresh token invalid")
 
+    user_id = payload["sub"]
+    refresh_token_key = f"user:{user_id}:{TokenType.REFRESH}"
+    valid_refresh_tokens = await redis_client.smembers(refresh_token_key)
+    if valid_refresh_tokens and body.refresh_token not in valid_refresh_tokens:
+        raise HTTPException(status_code=403, detail="Refresh token invalid")
+
     if payload["type"] == "refresh":
         access_token_expires = timedelta(minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES)
         user = await crud.user.get(id=payload["sub"])

@jonra1993
Copy link
Owner

jonra1993 commented Oct 7, 2022

Sure you are right @dongfengweixiao this commit validates refresh token 33862a1 2c9ac33

@dongfengweixiao
Copy link
Contributor Author

Sure you are right @dongfengweixiao this commit validates refresh token 33862a1 2c9ac33

Fantastic job.

@dongfengweixiao
Copy link
Contributor Author

It is unnecessary to record the token in Redis for normal login action. When the password is changed, the new token will be record to Redis. There are two cases when other clients log in.

Case 1: There are records in Redis. At this time, when other clients log in, they need to check whether there is a token in Redis. If yes, they need to put token to Redis together.
Case 2: There are no records in Redis. At this time, all the existing tokens must have expired. When you log in again, you do not need to save new tokens to Redis.

@jonra1993
Copy link
Owner

jonra1993 commented Oct 8, 2022

Hello @dongfengweixiao I caught your logic now. Please check this commit b9ff90c and let me know what do you think

@dongfengweixiao
Copy link
Contributor Author

Hello @dongfengweixiao I caught your logic now. Please check this commit b9ff90c and let me know what do you think

logic is right, but in login function, you call redis three times: read token from redis(get_valid_tokens) -> read token from redis again(add_token_to_redis) -> write token to redis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants