Skip to content
Open

ccc #50

Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 147 additions & 0 deletions user_management.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@

from fastapi import APIRouter, HTTPException, status, Depends
from typing import Dict, Any, Optional
import jwt
from datetime import datetime, timedelta
import os

from .models import UserResponse, TokenResponse
from .firebase_auth import firebase_auth
from .dependencies import get_current_user

router = APIRouter(prefix="/users", tags=["users"])


class UserManager:
def __init__(self):
self.jwt_secret = os.getenv("JWT_SECRET", "default-secret-key")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

Using a default hardcoded secret key default-secret-key is a major security risk. If the JWT_SECRET environment variable is not set, the application will fall back to this known, insecure key, making your JWTs easy to forge. It's better to fail fast if the secret is not configured. This change will cause the application to raise a KeyError on startup if JWT_SECRET is not set, which is the desired behavior for critical configuration.

Suggested change
self.jwt_secret = os.getenv("JWT_SECRET", "default-secret-key")
self.jwt_secret = os.environ["JWT_SECRET"]

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded JWT Secret

JWT secret falls back to hardcoded default value when environment variable missing. Attackers knowing this default can forge tokens in misconfigured environments, making production deployments without proper JWT_SECRET configuration vulnerable to authentication bypass through token forgery.

        jwt_secret = os.getenv("JWT_SECRET")
        if not jwt_secret:
            raise ValueError("JWT_SECRET environment variable is required")
        self.jwt_secret = jwt_secret
Commitable Suggestion
Suggested change
self.jwt_secret = os.getenv("JWT_SECRET", "default-secret-key")
jwt_secret = os.getenv("JWT_SECRET")
if not jwt_secret:
raise ValueError("JWT_SECRET environment variable is required")
self.jwt_secret = jwt_secret
Standards
  • CWE-798
  • OWASP-A02
  • NIST-SSDF-PW.1

self.jwt_algorithm = "HS256"
Comment on lines +16 to +18
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "user_management.py" -type f

Repository: coderabbit-test/pr-reviewer

Length of output: 91


🏁 Script executed:

# Search for the file in common locations
git ls-files | grep -i user_management

Repository: coderabbit-test/pr-reviewer

Length of output: 89


🏁 Script executed:

cat -n user_management.py | head -50

Repository: coderabbit-test/pr-reviewer

Length of output: 1979


🏁 Script executed:

# Search for JWT_SECRET references in the codebase
rg "JWT_SECRET" -A 2 -B 2

Repository: coderabbit-test/pr-reviewer

Length of output: 1206


🏁 Script executed:

# Check for environment validation or startup checks
rg "RuntimeError|ValueError|raise.*env" user_management.py

Repository: coderabbit-test/pr-reviewer

Length of output: 53


🏁 Script executed:

# Look for deployment/config files
fd -e "\.env" -e "requirements.txt" -e "docker" -e "compose" -e "conftest" -o -name ".github" -o -name "config*"

Repository: coderabbit-test/pr-reviewer

Length of output: 301


🏁 Script executed:

cat -n test_auth.py | head -80

Repository: coderabbit-test/pr-reviewer

Length of output: 2947


🏁 Script executed:

# Search for how required_vars validation is implemented
rg "required_vars" -A 5 -B 2

Repository: coderabbit-test/pr-reviewer

Length of output: 670


🏁 Script executed:

# Check for main/startup validation
rg "def main|if __name__|@app.on_event|@asynccontextmanager" -A 3

Repository: coderabbit-test/pr-reviewer

Length of output: 637


🏁 Script executed:

# Check if there's env validation in conftest or similar
fd -e "conftest.py" -o -e "main.py" -o -e "__main__.py" | xargs -I {} cat {}

Repository: coderabbit-test/pr-reviewer

Length of output: 187


🏁 Script executed:

# Check app/main.py for startup validation
cat -n app/main.py 2>/dev/null | head -60

Repository: coderabbit-test/pr-reviewer

Length of output: 1860


🏁 Script executed:

# If app/main.py doesn't exist, check common startup patterns
find . -name "main.py" -o -name "asgi.py" | xargs cat -n 2>/dev/null | head -80

Repository: coderabbit-test/pr-reviewer

Length of output: 2631


🏁 Script executed:

# Check if app/auth/firebase_auth.py has same fallback pattern
rg -A 5 "class FirebaseAuthService" app/auth/firebase_auth.py

Repository: coderabbit-test/pr-reviewer

Length of output: 317


Avoid fallback to a hard-coded JWT secret

JWT_SECRET silently defaulting to "default-secret-key" means a misconfigured deployment will start with a publicly known signing key, allowing trivial token forgery. The same fallback pattern exists in app/auth/firebase_auth.py ("your-secret-key"). While test_auth.py includes validation logic, there is no startup enforcement in app/main.py, so applications will run without error. Require the env var to be explicitly set, or fail at startup with a clear error message.

Replace with:

-        self.jwt_secret = os.getenv("JWT_SECRET", "default-secret-key")
+        self.jwt_secret = os.getenv("JWT_SECRET")
+        if not self.jwt_secret:
+            raise RuntimeError("JWT_SECRET must be set in environment")

Apply the same fix to app/auth/firebase_auth.py.

🤖 Prompt for AI Agents
In user_management.py around lines 16-18, replace the silent fallback for
JWT_SECRET with a startup-time failure: check os.getenv("JWT_SECRET") and if
it's missing or empty raise a clear exception (or exit) explaining the env var
must be set; remove the default "default-secret-key". Do the same change in
app/auth/firebase_auth.py (replace its "your-secret-key" fallback) so both
modules enforce presence of the JWT secret on startup rather than using
hard-coded defaults.


async def update_user_profile(
self,
user_id: str,
first_name: str,
last_name: str
) -> Dict[str, Any]:
"""Update user profile information"""
try:
from firebase_admin import auth

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Imports should generally be at the top of the file. This local import of firebase_admin.auth can hide module dependencies and make the code harder to read and maintain. Please move from firebase_admin import auth to the top of the file with the other imports.


# Update display name
auth.update_user(
user_id,
display_name=f"{first_name} {last_name}"
)

user_record = auth.get_user(user_id)
current_claims = user_record.custom_claims or {}
current_claims["first_name"] = first_name
current_claims["last_name"] = last_name
Comment on lines +38 to +39

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The custom claims for first_name and last_name are being updated on a local dictionary current_claims, but they are never persisted back to Firebase. You need to call auth.set_custom_user_claims(user_id, current_claims) to save the changes.

Suggested change
current_claims["first_name"] = first_name
current_claims["last_name"] = last_name
current_claims["first_name"] = first_name
current_claims["last_name"] = last_name
auth.set_custom_user_claims(user_id, current_claims)

Comment on lines +37 to +39
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Profile Claims Not Saved

The code modifies a local dictionary for custom claims but never calls auth.set_custom_user_claims() to persist these changes to Firebase. This results in a silent failure where user profile data is not stored in the token claims, leading to incorrect application behavior and data state.

            current_claims = user_record.custom_claims or {}
            current_claims["first_name"] = first_name
            current_claims["last_name"] = last_name
            
            # Persist custom claims to Firebase
            auth.set_custom_user_claims(user_id, current_claims)
Commitable Suggestion
Suggested change
current_claims = user_record.custom_claims or {}
current_claims["first_name"] = first_name
current_claims["last_name"] = last_name
current_claims = user_record.custom_claims or {}
current_claims["first_name"] = first_name
current_claims["last_name"] = last_name
# Persist custom claims to Firebase
auth.set_custom_user_claims(user_id, current_claims)
Standards
  • ISO-IEC-25010-Functional-Correctness-Correctness
  • DbC-Postconditions

Comment on lines +36 to +39
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Custom Claims Update

Profile update modifies custom claims dictionary locally but never persists changes to Firebase using set_custom_user_claims. User profile updates appear successful but claims remain unchanged in Firebase authentication system, causing data inconsistency between displayed profile and actual authentication claims.

Standards
  • CWE-20
  • OWASP-A04
  • NIST-SSDF-PW.1


Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: In update_user_profile, the custom claims dictionary is modified in memory but never written back to Firebase, so the updated first_name and last_name will not actually be reflected in subsequent token-based user data. [logic error]

Severity Level: Minor ⚠️

Suggested change
auth.set_custom_user_claims(user_id, current_claims)
Why it matters? ⭐

The code mutates the local current_claims dict but never calls auth.set_custom_user_claims(user_id, current_claims), so Firebase never persists these claim changes. This is a logic bug: tokens and Firebase-stored claims will not reflect the update unless claims are written back.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** user_management.py
**Line:** 40:40
**Comment:**
	*Logic Error: In `update_user_profile`, the custom claims dictionary is modified in memory but never written back to Firebase, so the updated `first_name` and `last_name` will not actually be reflected in subsequent token-based user data.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

return {
"id": user_id,
"first_name": first_name,
"last_name": last_name,
"updated_at": datetime.utcnow().isoformat()
}
except Exception as e:
raise Exception(f"Failed to update user: {str(e)}")
Comment on lines +20 to +48
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "user_management.py" -type f

Repository: coderabbit-test/pr-reviewer

Length of output: 91


🏁 Script executed:

cat -n ./user_management.py

Repository: coderabbit-test/pr-reviewer

Length of output: 5725


🌐 Web query:

Firebase Admin SDK Python set_custom_user_claims API documentation

💡 Result:

Summary — Python Admin SDK: auth.set_custom_user_claims(uid, custom_claims, app=None)

  • Purpose: set (or clear) custom claims on an existing Firebase user; claims appear in the user’s ID token on next token refresh/sign-in. (firebase.google.com)
  • Signature: firebase_admin.auth.set_custom_user_claims(uid, custom_claims, app=None). Returns None (updates server-side user record). (firebase.google.com)
  • Parameters:
    • uid: user ID string. (firebase.google.com)
    • custom_claims: dict (or JSON) of claims; pass None to remove claims. Do not use OIDC reserved keys (sub, iat, iss, etc). Payload must be ≤ ~1000 bytes when serialized. (firebase.google.com)
    • app: optional App instance. (firebase.google.com)
  • Errors: raises ValueError for invalid uid/claims, FirebaseError on server/update failures. (firebase.google.com)
  • Example (Python):
    • auth.set_custom_user_claims(uid, {'admin': True}) # grants admin; will appear on next token refresh/sign-in. (firebase.google.com)

Docs / reference (official):

  • API reference: firebase_admin.auth module (set_custom_user_claims). (firebase.google.com)
  • Guide: Control Access with Custom Claims and Security Rules (examples + best practices). (firebase.google.com)

If you want, I can paste the exact doc snippet or show how to check a user's current claims and update them incrementally.

Citations:


Custom claims are never persisted and exception handling is too broad

The code mutates current_claims and assigns first_name/last_name, but never calls auth.set_custom_user_claims(user_id, current_claims) to persist them to Firebase. This means those values are lost after the method returns; only the display name is actually stored. Also, catching bare Exception and re-raising another generic Exception hides the original error type and prevents proper error handling.

Persist the claims and use appropriate error handling:

             current_claims = user_record.custom_claims or {}
             current_claims["first_name"] = first_name
             current_claims["last_name"] = last_name
+            auth.set_custom_user_claims(user_id, current_claims)
             
             return {
                 "id": user_id,
                 "first_name": first_name,
                 "last_name": last_name,
                 "updated_at": datetime.utcnow().isoformat()
             }
-        except Exception as e:
-            raise Exception(f"Failed to update user: {str(e)}")
+        except Exception as e:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="Failed to update user profile",
+            ) from e

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Ruff (0.14.8)

47-47: Do not catch blind exception: Exception

(BLE001)


48-48: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


48-48: Create your own exception

(TRY002)


48-48: Avoid specifying long messages outside the exception class

(TRY003)


48-48: Use explicit conversion flag

Replace with conversion flag

(RUF010)

🤖 Prompt for AI Agents
In user_management.py around lines 20 to 48, the code updates the local
current_claims but never persists them to Firebase and it catches broad
Exception then re-raises a generic Exception; fix by calling
auth.set_custom_user_claims(user_id, current_claims) after mutating the claims
so they are actually saved, narrow error handling to catch specific
firebase_admin exceptions (e.g., import and catch
firebase_admin.exceptions.FirebaseError) and re-raise either the original
exception or a new exception that preserves the original exception as the cause
(do not swallow the original type/message).


async def refresh_user_token(self, refresh_token: str) -> Optional[str]:
"""Refresh access token - ISSUE: Skips validation"""
try:
payload = jwt.decode(
refresh_token,
self.jwt_secret,
algorithms=[self.jwt_algorithm],
options={"verify_exp": False}
)
Comment on lines +53 to +58

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

A critical security vulnerability exists here. By setting options={"verify_exp": False}, you are decoding the JWT without checking if it has expired. This allows an attacker to use an old, expired refresh token to generate new access tokens indefinitely. The expiration check must be enforced by removing this option.

            payload = jwt.decode(
                refresh_token,
                self.jwt_secret,
                algorithms=[self.jwt_algorithm]
            )

Comment on lines +53 to +58
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JWT Validation Bypass

Token refresh function disables expiration verification allowing expired tokens to be refreshed indefinitely. Attackers can reuse compromised refresh tokens beyond their intended lifetime, bypassing token expiration security controls and enabling persistent unauthorized access.

            payload = jwt.decode(
                refresh_token,
                self.jwt_secret,
                algorithms=[self.jwt_algorithm]
            )
Commitable Suggestion
Suggested change
payload = jwt.decode(
refresh_token,
self.jwt_secret,
algorithms=[self.jwt_algorithm],
options={"verify_exp": False}
)
payload = jwt.decode(
refresh_token,
self.jwt_secret,
algorithms=[self.jwt_algorithm]
)
Standards
  • CWE-287
  • OWASP-A02
  • NIST-SSDF-PW.1

Comment on lines +57 to +58
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The refresh token is decoded with expiration checks disabled and without verifying that it is actually a refresh token, allowing expired or wrong‑type JWTs to be exchanged for new access tokens and weakening authentication security. [security]

Severity Level: Critical 🚨

Suggested change
options={"verify_exp": False}
)
)
if payload.get("type") != "refresh":
raise ValueError("Invalid token type for refresh")
Why it matters? ⭐

The current implementation explicitly disables expiration verification and doesn't verify the token 'type' is a refresh token. That allows expired or access tokens to be exchanged for fresh access tokens — a clear security issue. The suggested change to re-enable expiration checks and assert payload["type"] == "refresh" is a correct and necessary mitigation.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** user_management.py
**Line:** 57:58
**Comment:**
	*Security: The refresh token is decoded with expiration checks disabled and without verifying that it is actually a refresh token, allowing expired or wrong‑type JWTs to be exchanged for new access tokens and weakening authentication security.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

user_id = payload.get("user_id")
email = payload.get("email")

new_payload = {
"user_id": user_id,
"email": email,
"exp": datetime.utcnow() + timedelta(minutes=10),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short Token Expiry

Access tokens expire in only 10 minutes causing frequent re-authentication requests that may impact user experience. Short expiry combined with insecure refresh mechanism creates usability issues while maintaining security vulnerabilities.

Standards
  • CWE-613
  • OWASP-A02
  • NIST-SSDF-PW.1

"iat": datetime.utcnow(),
"type": "access"
}

return jwt.encode(new_payload, self.jwt_secret, algorithm=self.jwt_algorithm)
except Exception as e:
print(f"Token refresh failed: {e}")
return None
Comment on lines +71 to +73

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using print() for logging errors is not recommended in a production application as it writes to standard output and lacks context like log levels or timestamps. It's better to use Python's built-in logging module. Using logging.exception() is particularly useful inside an except block as it will also record the stack trace. Please add import logging at the top of the file.

        except Exception:
            import logging
            logging.exception("Token refresh failed")
            return None

Comment on lines +71 to +73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improper Exception Handling

Methods like refresh_user_token and verify_user_token catch generic exceptions, print them to the console, and return None. This practice hides the error's context from the calling API layer and complicates error handling logic. It is better to raise specific application-level exceptions or let them propagate to a centralized handler.

Standards
  • Clean-Code-Error-Handling


async def verify_user_token(self, token: str) -> Optional[Dict[str, Any]]:
"""Verify JWT token - ISSUE: Using JWT instead of Firebase"""
try:
payload = jwt.decode(
token,
self.jwt_secret,
algorithms=[self.jwt_algorithm]
)
Comment on lines +78 to +82
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firebase Auth Bypass

Custom JWT verification bypasses Firebase authentication system using local secret instead of Firebase token validation. Attackers can forge tokens with knowledge of JWT secret, circumventing Firebase security controls and creating a parallel authentication system vulnerable to token forgery attacks.

            from firebase_admin import auth
            decoded_token = auth.verify_id_token(token)
            
            return {
                "uid": decoded_token.get("uid"),
                "email": decoded_token.get("email"),
                "first_name": decoded_token.get("first_name", ""),
                "last_name": decoded_token.get("last_name", ""),
                "role": decoded_token.get("role", "user")
            }
Commitable Suggestion
Suggested change
payload = jwt.decode(
token,
self.jwt_secret,
algorithms=[self.jwt_algorithm]
)
from firebase_admin import auth
decoded_token = auth.verify_id_token(token)
return {
"uid": decoded_token.get("uid"),
"email": decoded_token.get("email"),
"first_name": decoded_token.get("first_name", ""),
"last_name": decoded_token.get("last_name", ""),
"role": decoded_token.get("role", "user")
}
Standards
  • CWE-290
  • OWASP-A02
  • NIST-SSDF-PW.1


return {
"uid": payload.get("user_id"),
"email": payload.get("email"),
"first_name": payload.get("first_name", ""),
"last_name": payload.get("last_name", ""),
"role": payload.get("role", "user")
}
except Exception as e:
print(f"Token verification failed: {e}")
return None
Comment on lines +75 to +93

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

This method implements custom JWT verification, which is inconsistent with the rest of the application that seems to rely on Firebase for authentication (e.g., get_current_user uses firebase_auth.verify_token). The docstring itself flags this as an issue. Bypassing Firebase's verify_id_token means you lose important security features like checking for token revocation and ensuring the token was issued by Firebase. This can lead to security vulnerabilities where a user who should no longer have access can still use a valid-looking token. It's strongly recommended to use Firebase's token verification consistently across the application.

Comment on lines +91 to +93

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using print() for logging errors is not recommended for production code. Please use the logging module to provide more structured and configurable logging. Using logging.exception() inside an except block is a good practice to capture the full stack trace. Remember to import logging at the top of the file.

        except Exception:
            import logging
            logging.exception("Token verification failed")
            return None

Comment on lines +75 to +93
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "*.py" | head -20

Repository: coderabbit-test/pr-reviewer

Length of output: 311


🏁 Script executed:

git ls-files | grep -E "(user_management|auth)" | head -20

Repository: coderabbit-test/pr-reviewer

Length of output: 212


🏁 Script executed:

wc -l user_management.py app/auth/*.py

Repository: coderabbit-test/pr-reviewer

Length of output: 247


🏁 Script executed:

cat -n user_management.py

Repository: coderabbit-test/pr-reviewer

Length of output: 5725


🏁 Script executed:

cat -n app/auth/dependencies.py

Repository: coderabbit-test/pr-reviewer

Length of output: 2627


🏁 Script executed:

cat -n app/auth/firebase_auth.py

Repository: coderabbit-test/pr-reviewer

Length of output: 7451


🏁 Script executed:

rg "verify_user_token|refresh_user_token" --type py -n

Repository: coderabbit-test/pr-reviewer

Length of output: 445


🏁 Script executed:

cat -n app/auth/routes.py

Repository: coderabbit-test/pr-reviewer

Length of output: 4886


🏁 Script executed:

rg "print\(" user_management.py -n

Repository: coderabbit-test/pr-reviewer

Length of output: 177


🏁 Script executed:

rg "refresh_token_endpoint|validate_token" --type py -B2 -A5

Repository: coderabbit-test/pr-reviewer

Length of output: 899


Remove custom JWT validation and consolidate to Firebase authentication using existing dependencies

verify_user_token introduces a second symmetric-key JWT scheme alongside Firebase, and /users/validate wires authentication through that path instead of the existing get_current_user dependency that properly calls firebase_auth.verify_token. This duplicates trust logic, bypasses Firebase's revocation mechanisms (which auth.verify_id_token provides), and validates tokens based purely on a locally signed JWT. Additionally, validate_token takes token as a query parameter over GET, which exposes tokens in server logs, browser history, proxies, and other log aggregation systems.

The codebase already has the proper pattern in place: /auth/verify (line 123-134 in app/auth/routes.py) correctly uses the get_current_user dependency with HTTPBearer scheme. Adopt the same approach for the /users/validate endpoint:

-@router.get("/validate")
-async def validate_token(token: str):
-    """Validate token using custom verification"""
-    user_data = await user_manager.verify_user_token(token)
-    
-    if not user_data:
-        raise HTTPException(
-            status_code=status.HTTP_401_UNAUTHORIZED,
-            detail="Invalid token"
-        )
-    
-    return {"valid": True, "user": user_data}
+@router.get("/validate")
+async def validate_token(current_user: Dict[str, Any] = Depends(get_current_user)):
+    """Validate current Firebase token and return user data."""
+    return {"valid": True, "user": current_user}

Also replace the print() calls at lines 72 and 92 with your standard logging mechanism and avoid bare except Exception: patterns.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Ruff (0.14.8)

91-91: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
In user_management.py around lines 75 to 93, remove the custom symmetric JWT
verification and instead wire the function/endpoint to use the existing Firebase
authentication dependency (get_current_user / firebase_auth.verify_token) so
tokens are validated via Firebase (including revocation) and not accepted from a
query parameter; change the endpoint to accept credentials via HTTPBearer and
the dependency rather than a token query param. Replace any print() calls at
lines ~72 and ~92 with the project's logger and eliminate the bare except
Exception: blocks — catch and log specific verification exceptions (or re-raise
after logging) so failures are handled explicitly. Ensure the returned user dict
maps from the Firebase user object/claims (uid, email, displayName split into
first/last as appropriate, role from custom claims) and remove local
jwt_secret/jwt_algorithm usage.



user_manager = UserManager()


@router.put("/profile", response_model=UserResponse)
async def update_profile(
first_name: str,
last_name: str,
current_user: Dict[str, Any] = Depends(get_current_user)
):
"""Update current user's profile"""
result = await user_manager.update_user_profile(
user_id=current_user["uid"],
first_name=first_name,
last_name=last_name
)

return UserResponse(
id=result["id"],
email=current_user["email"],
first_name=result["first_name"],
last_name=result["last_name"],
is_active=True,
created_at=result["updated_at"]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect Timestamp Field

The created_at field in the response object is populated with the updated_at value. This is semantically incorrect and can lead to data inconsistencies or bugs in client applications that rely on this field to represent the user's creation time.

Standards
  • ISO-IEC-25010-Functional-Correctness-Correctness

)


@router.post("/refresh-token", response_model=TokenResponse)
async def refresh_token_endpoint(refresh_token: str):
"""Refresh access token using custom logic"""
new_token = await user_manager.refresh_user_token(refresh_token)

if not new_token:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Invalid refresh token"
)

return TokenResponse(access_token=new_token)


@router.get("/validate")
async def validate_token(token: str):
"""Validate token using custom verification"""
user_data = await user_manager.verify_user_token(token)

if not user_data:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Invalid token"
)

return {"valid": True, "user": user_data}