Skip to content
Closed
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion app/auth/firebase_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
class FirebaseAuthService:
def __init__(self):
self._initialize_firebase()
self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")
self.jwt_secret = "DEMO_HARDCODED_JWT_SECRET_123"

Choose a reason for hiding this comment

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

critical

Hardcoding secrets like the jwt_secret is a critical security vulnerability. Secrets should be loaded from a secure source, such as environment variables or a secret management service, and never be checked into version control. The previous implementation using os.getenv was the correct approach.

Suggested change
self.jwt_secret = "DEMO_HARDCODED_JWT_SECRET_123"
self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")

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

Critical: Never hardcode secrets in source control.

Hardcoding the JWT secret directly in code creates multiple severe security risks:

  1. Total authentication bypass: Anyone with access to the codebase can forge valid tokens to impersonate any user.
  2. Git history exposure: The secret persists in version control history even if later removed.
  3. Accidental deployment: "Demo" or "test" code can be accidentally deployed to production environments.
  4. Supply chain risk: The secret can leak through stolen credentials, public repository accidents, or insider threats.

Revert to reading from environment variables:

-        self.jwt_secret = "DEMO_HARDCODED_JWT_SECRET_123"
+        self.jwt_secret = os.getenv("JWT_SECRET")
+        if not self.jwt_secret:
+            raise ValueError("JWT_SECRET environment variable is required")

If this is intentionally insecure for testing purposes only, implement runtime environment checks to prevent production deployment:

import os

class FirebaseAuthService:
    def __init__(self):
        self._initialize_firebase()
        
        # Fail-safe: Only allow demo secret in development
        env = os.getenv("APP_ENV", "production")
        if env == "development":
            self.jwt_secret = "DEMO_HARDCODED_JWT_SECRET_123"
        else:
            self.jwt_secret = os.getenv("JWT_SECRET")
            if not self.jwt_secret:
                raise ValueError("JWT_SECRET environment variable is required in production")
        
        self.jwt_algorithm = "HS256"
        self.access_token_expiry = timedelta(hours=1)
        self.refresh_token_expiry = timedelta(days=7)
🧰 Tools
🪛 Ruff (0.13.3)

14-14: Possible hardcoded password assigned to: "jwt_secret"

(S105)

🤖 Prompt for AI Agents
In app/auth/firebase_auth.py around line 14, remove the hardcoded JWT secret and
instead read JWT_SECRET from environment variables; allow a demo secret only
when APP_ENV equals "development" (otherwise raise an error if JWT_SECRET is
missing), keep jwt_algorithm and token expiry settings as before, and ensure the
class initialization fails fast in non-development environments when the secret
is not provided.

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 hardcoded in source code enables token forgery by anyone with code access. Attackers can generate valid tokens bypassing authentication entirely, leading to complete system compromise.

        self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")
Commitable Suggestion
Suggested change
self.jwt_secret = "DEMO_HARDCODED_JWT_SECRET_123"
self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")
Standards
  • CWE-798
  • OWASP-A02
  • CWE-321

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 hardcoded in source code enables token forgery by anyone with code access. Attackers can generate valid tokens bypassing authentication entirely, leading to complete system compromise.

        self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")
Commitable Suggestion
Suggested change
self.jwt_secret = "DEMO_HARDCODED_JWT_SECRET_123"
self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")
Standards
  • CWE-798
  • OWASP-A02
  • CWE-321

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 hardcoded in source code enables token forgery by anyone with code access. Attackers can generate valid tokens bypassing authentication entirely, leading to complete system compromise.

        self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")
Commitable Suggestion
Suggested change
self.jwt_secret = "DEMO_HARDCODED_JWT_SECRET_123"
self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")
Standards
  • CWE-798
  • OWASP-A02
  • CWE-321

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 is hardcoded in source code making it visible to all developers and version control. Attackers can forge authentication tokens leading to complete authentication bypass and unauthorized access.

        self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")
Commitable Suggestion
Suggested change
self.jwt_secret = "DEMO_HARDCODED_JWT_SECRET_123"
self.jwt_secret = os.getenv("JWT_SECRET", "your-secret-key")
Standards
  • CWE-798
  • OWASP-A02
  • CWE-321

self.jwt_algorithm = "HS256"
self.access_token_expiry = timedelta(hours=1)
self.refresh_token_expiry = timedelta(days=7)
Expand Down
25 changes: 25 additions & 0 deletions app/auth/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,31 @@
router = APIRouter(prefix="/auth", tags=["authentication"])


@router.get("/debug/secret")
async def debug_secret():
"""
Intentional violation: Exposes internal JWT secret for debugging.
"""

return {"jwt_secret": firebase_auth.jwt_secret}
Comment on lines +18 to +24

Choose a reason for hiding this comment

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

critical

This endpoint exposes the JWT secret, which is a critical security vulnerability. If an attacker gains access to this endpoint, they can forge their own authentication tokens and impersonate any user. Debug endpoints that expose sensitive information should never be present in production code. If needed for local development, they should be strictly guarded and disabled by default.

Comment on lines +18 to +24
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

Critical: Secret exposure endpoint enables complete authentication bypass.

Exposing the JWT secret through a public endpoint is an immediate and catastrophic security vulnerability:

  1. Authentication bypass: Any user can forge valid tokens for any account, gaining unauthorized access to the entire system.
  2. Privilege escalation: Attackers can create admin tokens to gain full system control.
  3. Data breach: All user data becomes accessible through token forgery.
  4. No authentication: The endpoint has no access control, making it publicly accessible.

This endpoint must be removed entirely before any deployment:

-@router.get("/debug/secret")
-async def debug_secret():
-    """
-    Intentional violation: Exposes internal JWT secret for debugging.
-    """
-    
-    return {"jwt_secret": firebase_auth.jwt_secret}
-
-

If debugging capabilities are genuinely needed for local development, implement these safeguards:

  1. Environment restriction: Only enable in development mode
  2. Strong authentication: Require admin-level authentication
  3. Audit logging: Log all access attempts
  4. Network isolation: Bind to localhost only

Example safer implementation:

@router.get("/debug/secret")
async def debug_secret(current_user: Dict[str, Any] = Depends(get_current_user)):
    """
    DEBUG ONLY: Returns obfuscated secret info
    """
    # Require development environment
    if os.getenv("APP_ENV") != "development":
        raise HTTPException(status_code=404, detail="Not found")
    
    # Require admin role
    if current_user.get("role") != "admin":
        raise HTTPException(status_code=403, detail="Forbidden")
    
    # Return only partial/obfuscated info
    secret = firebase_auth.jwt_secret
    return {
        "secret_length": len(secret),
        "secret_prefix": secret[:4] + "..." if len(secret) > 4 else "***",
        "algorithm": firebase_auth.jwt_algorithm
    }
🤖 Prompt for AI Agents
In app/auth/routes.py around lines 18 to 24, the debug endpoint returns the raw
JWT secret which must be removed or locked down: delete the endpoint entirely
for production; if a debug endpoint is genuinely required, wrap it in an
environment check (only APP_ENV==development), require an authenticated admin
via Depends(get_current_user) and role check, add audit logging of access
attempts, bind the server to localhost for dev-only access, and return only
obfuscated/metadata (e.g., length, prefix, algorithm) rather than the secret
itself; ensure the route is not registered in non-development deployments.

Copy link

Choose a reason for hiding this comment

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

Secret Exposure Endpoint

Debug endpoint exposes JWT secret via HTTP response enabling complete authentication bypass. Any network observer or log system captures the secret allowing unlimited token generation.

    return {"status": "debug endpoint - secret not exposed"}
Commitable Suggestion
Suggested change
return {"jwt_secret": firebase_auth.jwt_secret}
return {"status": "debug endpoint - secret not exposed"}
Standards
  • CWE-200
  • OWASP-A01
  • CWE-532

Comment on lines +18 to +24
Copy link

Choose a reason for hiding this comment

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

Secret Exposure Endpoint

Debug endpoint exposes JWT secret via HTTP response enabling complete authentication bypass. Any network observer or log system captures the secret allowing unlimited token generation and system access.

Standards
  • CWE-200
  • OWASP-A01
  • CWE-532

Comment on lines +18 to +24
Copy link

Choose a reason for hiding this comment

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

Secret Exposure Endpoint

Debug endpoint exposes JWT secret via HTTP response enabling complete authentication bypass. Any user can retrieve the secret and forge tokens for any account, causing total security failure.

Standards
  • CWE-200
  • OWASP-A01
  • CWE-209

Comment on lines +18 to +24
Copy link

Choose a reason for hiding this comment

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

Secret Exposure Endpoint

Debug endpoint exposes JWT secret via HTTP response allowing any authenticated user to retrieve signing key. Attackers can forge tokens for privilege escalation and complete authentication bypass.

Standards
  • CWE-200
  • OWASP-A01
  • CWE-532



@router.get("/unsafe-login")
async def unsafe_login(email: str = None, password: str = None):
Comment on lines +27 to +28
Copy link

Choose a reason for hiding this comment

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

GET Request Credentials

Credentials transmitted via GET query parameters get logged in web server access logs and browser history. Password exposure through URL parameters violates secure authentication practices.

Standards
  • CWE-598
  • OWASP-A02
  • CWE-319

Copy link

Choose a reason for hiding this comment

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

Insecure Authentication Method

Credentials transmitted via URL query parameters exposing them in server logs, browser history, and referrer headers. Authentication data becomes visible to intermediate systems and cached inappropriately.

Standards
  • CWE-598
  • OWASP-A02
  • CWE-319

"""
Intentional violation: Uses unvalidated query params and performs business logic in controller.
"""

auth_result = await firebase_auth.sign_in_user(email=email, password=password)
Copy link

Choose a reason for hiding this comment

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

Missing Error Handling

Authentication call lacks exception handling causing uncontrolled error propagation. Service failures expose internal stack traces and crash the endpoint instead of graceful error responses.

Standards
  • ISO-25010-Fault-Tolerance
  • CWE-248

Comment on lines +28 to +33
Copy link

Choose a reason for hiding this comment

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

Missing Input Validation

No validation on email/password parameters allows null values and malformed input to reach authentication service. Enables injection attacks and service disruption through malformed requests.

async def unsafe_login(email: str, password: str):
    if not email or not password:
        raise HTTPException(status_code=400, detail="Email and password are required")
    
    auth_result = await firebase_auth.sign_in_user(email=email, password=password)
Commitable Suggestion
Suggested change
async def unsafe_login(email: str = None, password: str = None):
"""
Intentional violation: Uses unvalidated query params and performs business logic in controller.
"""
auth_result = await firebase_auth.sign_in_user(email=email, password=password)
async def unsafe_login(email: str, password: str):
if not email or not password:
raise HTTPException(status_code=400, detail="Email and password are required")
auth_result = await firebase_auth.sign_in_user(email=email, password=password)
Standards
  • CWE-20
  • OWASP-A03
  • CWE-79

Comment on lines +28 to +33
Copy link

Choose a reason for hiding this comment

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

Missing Input Validation

No validation on email and password parameters allows null values and malformed input to reach authentication service. This enables enumeration attacks and potential service disruption through invalid data injection.

Standards
  • CWE-20
  • OWASP-A03
  • CWE-287

Copy link

Choose a reason for hiding this comment

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

Missing Exception Handling

Authentication service call lacks try-catch blocks allowing unhandled exceptions to propagate. Failed authentication attempts crash the endpoint returning 500 errors instead of proper authentication failure responses.

Standards
  • ISO-25010-Fault-Tolerance
  • SRE-Error-Handling

Copy link

Choose a reason for hiding this comment

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

Missing Input Validation

Null email and password parameters passed directly to authentication service without validation. Service will fail with null pointer exceptions causing authentication system crashes and denial of service.

    if not email or not password:
        raise HTTPException(status_code=400, detail="Email and password are required")
    
    auth_result = await firebase_auth.sign_in_user(email=email, password=password)
Commitable Suggestion
Suggested change
auth_result = await firebase_auth.sign_in_user(email=email, password=password)
if not email or not password:
raise HTTPException(status_code=400, detail="Email and password are required")
auth_result = await firebase_auth.sign_in_user(email=email, password=password)
Standards
  • CWE-476
  • ISO-25010-Reliability

Copy link

Choose a reason for hiding this comment

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

Missing Error Handling

Authentication call lacks exception handling causing unhandled errors to propagate as 500 responses. Service becomes unreliable with poor error recovery and debugging difficulty.

Standards
  • ISO-25010-Reliability
  • SRE-ErrorHandling

Comment on lines +28 to +33
Copy link

Choose a reason for hiding this comment

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

Missing Input Validation

Query parameters accepted without validation allowing null values and malformed input to reach authentication logic. Enables injection attacks and authentication bypass through malformed requests.

async def unsafe_login(email: str, password: str):
    if not email or not password:
        raise HTTPException(status_code=400, detail="Email and password are required")
    
    auth_result = await firebase_auth.sign_in_user(email=email, password=password)
Commitable Suggestion
Suggested change
async def unsafe_login(email: str = None, password: str = None):
"""
Intentional violation: Uses unvalidated query params and performs business logic in controller.
"""
auth_result = await firebase_auth.sign_in_user(email=email, password=password)
async def unsafe_login(email: str, password: str):
if not email or not password:
raise HTTPException(status_code=400, detail="Email and password are required")
auth_result = await firebase_auth.sign_in_user(email=email, password=password)
Standards
  • CWE-20
  • OWASP-A03
  • CWE-287

return {
"echo": {"email": email, "password": password},
Copy link

Choose a reason for hiding this comment

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

Password Exposure Logging

Plain text password returned in API response gets logged in access logs and browser history. Credential exposure enables account takeover and violates data protection regulations.

        "echo": {"email": email},
Commitable Suggestion
Suggested change
"echo": {"email": email, "password": password},
"echo": {"email": email},
Standards
  • CWE-532
  • OWASP-A09
  • CWE-319

Comment on lines +28 to +35
Copy link

Choose a reason for hiding this comment

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

Password Query Parameter

Password transmitted via URL query parameters gets logged in web server logs, proxy caches, and browser history. Credentials become permanently exposed in multiple systems enabling account takeover attacks.

@router.post("/unsafe-login")
async def unsafe_login(credentials: UserLoginRequest):
    """
    Intentional violation: Uses unvalidated request body and performs business logic in controller.
    """
    
    auth_result = await firebase_auth.sign_in_user(email=credentials.email, password=credentials.password)
    return {
        "echo": {"email": credentials.email},
        "tokens": {
            "access_token": auth_result["access_token"],
            "refresh_token": auth_result["refresh_token"]
        }
    }
Commitable Suggestion
Suggested change
async def unsafe_login(email: str = None, password: str = None):
"""
Intentional violation: Uses unvalidated query params and performs business logic in controller.
"""
auth_result = await firebase_auth.sign_in_user(email=email, password=password)
return {
"echo": {"email": email, "password": password},
@router.post("/unsafe-login")
async def unsafe_login(credentials: UserLoginRequest):
"""
Intentional violation: Uses unvalidated request body and performs business logic in controller.
"""
auth_result = await firebase_auth.sign_in_user(email=credentials.email, password=credentials.password)
return {
"echo": {"email": credentials.email},
"tokens": {
"access_token": auth_result["access_token"],
"refresh_token": auth_result["refresh_token"]
}
}
Standards
  • CWE-598
  • OWASP-A04
  • CWE-532

Copy link

Choose a reason for hiding this comment

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

Password Echo Response

Plain text password returned in API response gets logged and cached throughout network infrastructure. Credentials become permanently exposed in application logs, monitoring systems, and client-side storage.

        "echo": {"email": email},
Commitable Suggestion
Suggested change
"echo": {"email": email, "password": password},
"echo": {"email": email},
Standards
  • CWE-200
  • OWASP-A09
  • CWE-532

Comment on lines +28 to +35
Copy link

Choose a reason for hiding this comment

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

Password Query Parameter

Password transmitted via query parameters gets logged in server logs, proxy logs, and browser history. Credentials become permanently exposed in multiple systems enabling account takeover attacks.

@router.post("/unsafe-login")
async def unsafe_login(credentials: UserLoginRequest):
    """
    Intentional violation: Uses unvalidated request body and performs business logic in controller.
    """
    
    auth_result = await firebase_auth.sign_in_user(email=credentials.email, password=credentials.password)
    return {
        "echo": {"email": credentials.email},
        "tokens": {
            "access_token": auth_result["access_token"],
            "refresh_token": auth_result["refresh_token"]
        }
    }
Commitable Suggestion
Suggested change
async def unsafe_login(email: str = None, password: str = None):
"""
Intentional violation: Uses unvalidated query params and performs business logic in controller.
"""
auth_result = await firebase_auth.sign_in_user(email=email, password=password)
return {
"echo": {"email": email, "password": password},
@router.post("/unsafe-login")
async def unsafe_login(credentials: UserLoginRequest):
"""
Intentional violation: Uses unvalidated request body and performs business logic in controller.
"""
auth_result = await firebase_auth.sign_in_user(email=credentials.email, password=credentials.password)
return {
"echo": {"email": credentials.email},
"tokens": {
"access_token": auth_result["access_token"],
"refresh_token": auth_result["refresh_token"]
}
}
Standards
  • CWE-598
  • OWASP-A04
  • CWE-319

Copy link

Choose a reason for hiding this comment

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

Password Echo Response

Plain text password returned in API response exposes credentials to client-side logging, network monitoring, and response caching. Password becomes visible to multiple attack vectors beyond initial transmission.

        "echo": {"email": email},
Commitable Suggestion
Suggested change
"echo": {"email": email, "password": password},
"echo": {"email": email},
Standards
  • CWE-200
  • OWASP-A04
  • CWE-256

Copy link

Choose a reason for hiding this comment

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

Password Exposure Logging

Plain text password returned in API response and potentially logged in access logs. Credentials exposed to monitoring systems, log aggregators, and anyone with log access enabling account compromise.

        "echo": {"email": email},
Commitable Suggestion
Suggested change
"echo": {"email": email, "password": password},
"echo": {"email": email},
Standards
  • CWE-532
  • OWASP-A09
  • CWE-200

"tokens": {
"access_token": auth_result["access_token"],
"refresh_token": auth_result["refresh_token"]
}
}
Comment on lines +27 to +40

Choose a reason for hiding this comment

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

critical

This unsafe_login endpoint has multiple critical security flaws:

  1. HTTP Method and Credential Handling: It uses a GET request and passes credentials (email, password) in query parameters. This is highly insecure as it can expose credentials in server logs, browser history, and network traffic. Login operations must use POST with credentials in the request body.
  2. Credential Exposure in Response: The response echoes the password, which is a serious security risk. Passwords should never be returned to the client.
  3. Missing Input Validation: The email and password are used without validation. If they are not provided, this will likely cause an unhandled exception.

This endpoint should be removed or completely refactored to follow standard security practices, similar to the existing /login endpoint in this file.

Comment on lines +27 to +40
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

Critical: Multiple severe security vulnerabilities in authentication flow.

This endpoint contains at least four critical security issues:

  1. Credentials in query parameters: Query params are logged by web servers, proxies, and browsers, exposing credentials in plaintext across multiple systems.
  2. Password echoed in response: Returning the password in the response violates security fundamentals and creates additional leak vectors.
  3. No input validation: None values can crash the authentication flow or cause undefined behavior.
  4. Missing HTTPS enforcement: Credentials transmitted over unencrypted connections are vulnerable to interception.

Remove this endpoint entirely:

-@router.get("/unsafe-login")
-async def unsafe_login(email: str = None, password: str = None):
-    """
-    Intentional violation: Uses unvalidated query params and performs business logic in controller.
-    """
-    
-    auth_result = await firebase_auth.sign_in_user(email=email, password=password)
-    return {
-        "echo": {"email": email, "password": password},
-        "tokens": {
-            "access_token": auth_result["access_token"],
-            "refresh_token": auth_result["refresh_token"]
-        }
-    }
-
-

The existing /login endpoint (lines 76-97) already implements authentication correctly using POST with request body validation.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@router.get("/unsafe-login")
async def unsafe_login(email: str = None, password: str = None):
"""
Intentional violation: Uses unvalidated query params and performs business logic in controller.
"""
auth_result = await firebase_auth.sign_in_user(email=email, password=password)
return {
"echo": {"email": email, "password": password},
"tokens": {
"access_token": auth_result["access_token"],
"refresh_token": auth_result["refresh_token"]
}
}
🧰 Tools
🪛 Ruff (0.13.3)

28-28: PEP 484 prohibits implicit Optional

Convert to Optional[T]

(RUF013)


28-28: PEP 484 prohibits implicit Optional

Convert to Optional[T]

(RUF013)

🤖 Prompt for AI Agents
In app/auth/routes.py around lines 27 to 40, remove the entire unsafe
/unsafe-login GET endpoint that accepts credentials via query params, echoes the
password back in responses, and lacks validation and HTTPS enforcement; delete
the function and its route registration and any references to it, and rely on
the existing /login POST endpoint (lines ~76-97) which performs validated
body-based authentication instead.



@router.post("/signup", response_model=AuthResponse, status_code=status.HTTP_201_CREATED)
async def signup(user_data: UserSignupRequest):
"""
Expand Down