Skip to content

Staging #230

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.

Sign up for GitHub

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

Merged
gkorland merged 8 commits into from
Sep 7, 2025
Merged

Staging #230

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fdc5810
don't show logo
gkorland Sep 5, 2025
53b60db
fix email login
gkorland Sep 7, 2025
461060b
fix comments
gkorland Sep 7, 2025
5ab0d4e
fix comment
gkorland Sep 7, 2025
24c5ff3
fix comment
gkorland Sep 7, 2025
3549a12
fix name result
gkorland Sep 7, 2025
5182962
Merge pull request #229 from FalkorDB/email-login
gkorland Sep 7, 2025
a7ccd41
Merge branch 'main' into staging
gkorland Sep 7, 2025
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
102 changes: 58 additions & 44 deletions api/routes/auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import logging
import os
import re
import time
import secrets

from pathlib import Path
Expand Down Expand Up @@ -161,6 +160,17 @@ async def _set_mail_hash(email: str, password_hash: str) -> bool:
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="Internal server error"
)

def _is_request_secure(request: Request) -> bool:
"""Determine if the request is secure (HTTPS)."""

# Check X-Forwarded-Proto first (proxy-aware)
forwarded_proto = request.headers.get("x-forwarded-proto")
if forwarded_proto:
return forwarded_proto == "https"

# Fallback to request URL scheme
return request.url.scheme == "https"
Comment on lines +164 to +173

@coderabbitai coderabbitai Bot Sep 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Make proxy HTTPS detection robust (multi-proxy, casing).

Handle comma-separated X-Forwarded-Proto and case-insensitivity to avoid misclassifying HTTPS and failing to set Secure on cookies.

-def _is_request_secure(request: Request) -> bool:
-    """Determine if the request is secure (HTTPS)."""
-    
-    # Check X-Forwarded-Proto first (proxy-aware)
-    forwarded_proto = request.headers.get("x-forwarded-proto")
-    if forwarded_proto:
-        return forwarded_proto == "https"
-    
-    # Fallback to request URL scheme
-    return request.url.scheme == "https"
+def _is_request_secure(request: Request) -> bool:
+    """Determine if the request is secure (HTTPS), proxy-aware."""
+    forwarded_proto = request.headers.get("x-forwarded-proto", "")
+    if forwarded_proto:
+        first = forwarded_proto.split(",")[0].strip().lower()
+        return first == "https"
+    return request.url.scheme.lower() == "https"
📝 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
def _is_request_secure(request: Request) -> bool:
"""Determine if the request is secure (HTTPS)."""
# Check X-Forwarded-Proto first (proxy-aware)
forwarded_proto = request.headers.get("x-forwarded-proto")
if forwarded_proto:
return forwarded_proto == "https"
# Fallback to request URL scheme
return request.url.scheme == "https"
def _is_request_secure(request: Request) -> bool:
"""Determine if the request is secure (HTTPS), proxy-aware."""
forwarded_proto = request.headers.get("x-forwarded-proto", "")
if forwarded_proto:
first = forwarded_proto.split(",")[0].strip().lower()
return first == "https"
return request.url.scheme.lower() == "https"
🤖 Prompt for AI Agents 
In api/routes/auth.py around lines 164 to 173, the X-Forwarded-Proto check is
too strict: it doesn't handle comma-separated values from multiple proxies or
case differences, which can misclassify HTTPS requests; change the logic to
split the header on commas, strip whitespace, lower-case each value, and return
True if any of the entries equals "https" (fall back to request.url.scheme ==
"https" if header missing).


async def _authenticate_email_user(email: str, password: str):
"""Authenticate an email user."""
Expand Down Expand Up @@ -206,18 +216,18 @@ async def email_signup(request: Request, signup_data: EmailSignupRequest) -> JSO
"""Handle email/password user registration."""
try:
# Check if email authentication is enabled
if os.getenv("EMAIL_AUTH_ENABLED", "").lower() not in ["true", "1", "yes", "on"]:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="GEmail authentication is not enabled"
)
if not _is_email_auth_enabled():
return JSONResponse(
{"success": False, "error": "Email authentication is not enabled"},
status_code=status.HTTP_403_FORBIDDEN
)

# Validate required fields
if not all([signup_data.firstName, signup_data.lastName,
signup_data.email, signup_data.password]):
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail="All fields are required"
return JSONResponse(
{"success": False, "error": "All fields are required"},
status_code=status.HTTP_400_BAD_REQUEST
)

first_name = signup_data.firstName.strip()
Expand All @@ -227,16 +237,16 @@ async def email_signup(request: Request, signup_data: EmailSignupRequest) -> JSO

# Validate email format
if not _validate_email(email):
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail="Invalid email format"
return JSONResponse(
{"success": False, "error": "Invalid email format"},
status_code=status.HTTP_400_BAD_REQUEST
)

# Validate password strength
if len(password) < 8:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail="Password must be at least 8 characters long"
return JSONResponse(
{"success": False, "error": "Password must be at least 8 characters long"},
status_code=status.HTTP_400_BAD_REQUEST
)

api_token = secrets.token_urlsafe(32)
Expand Down Expand Up @@ -265,23 +275,23 @@ async def email_signup(request: Request, signup_data: EmailSignupRequest) -> JSO
key="api_token",
value=api_token,
httponly=True,
secure=True
secure=_is_request_secure(request)
)
return response

except Exception as e:
logging.error("Signup error: %s", e)
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="Registration failed"
return JSONResponse(
{"success": False, "error": "Registration failed"},
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR
)
Comment on lines 282 to 287

@coderabbitai coderabbitai Bot Sep 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Tighten exception handling and improve logging.

Avoid blind except Exception; re-raise HTTPException and use logging.exception for stack traces (matches Ruff hints).

-    except Exception as e:
-        logging.error("Signup error: %s", e)
+    except HTTPException:
+        raise
+    except Exception as e:
+        logging.exception("Signup error: %s", e)
         return JSONResponse(
             {"success": False, "error": "Registration failed"},
             status_code=status.HTTP_500_INTERNAL_SERVER_ERROR
         )
-    except Exception as e:
-        logging.error("Login error: %s", e)
+    except HTTPException:
+        raise
+    except Exception as e:
+        logging.exception("Login error: %s", e)
         return JSONResponse(
             {"success": False, "error": "Login failed"},
             status_code=status.HTTP_500_INTERNAL_SERVER_ERROR
         )

Also applies to: 364-369

🧰 Tools
🪛 Ruff (0.12.2)

282-282: Do not catch blind exception: Exception

(BLE001)

283-283: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

🤖 Prompt for AI Agents 
In api/routes/auth.py around lines 282-287 (and similarly 364-369), replace the
blind "except Exception" with explicit handling that re-raises HTTPException
instances and only catches unexpected exceptions; use logging.exception(...) to
record the full stack trace for unexpected errors and then return the
JSONResponse 500. Concretely: add an except HTTPException: raise to let FastAPI
handle known HTTP errors, and change the general catch to except Exception as e:
logging.exception("Signup error"); return the existing JSONResponse with status
500.


@auth_router.post("/login/email")
async def email_login(request: Request, login_data: EmailLoginRequest) -> JSONResponse:
"""Handle email/password user login."""
try:
# Check if email authentication is enabled
if os.getenv("EMAIL_AUTH_ENABLED", "").lower() not in ["true", "1", "yes", "on"]:
if not _is_email_auth_enabled():
return JSONResponse(
{"success": False, "error": "Email authentication is not enabled"},
status_code=status.HTTP_403_FORBIDDEN
Expand Down Expand Up @@ -309,48 +319,52 @@ async def email_login(request: Request, login_data: EmailLoginRequest) -> JSONRe

if not success:
return JSONResponse(
{"success": False, "error": result},
{"success": False, "error": result},
status_code=status.HTTP_401_UNAUTHORIZED
)

# Set session data - result is a dict when success is True
if isinstance(result, dict):
user_node = result.get("user")
identity_node = result.get("identity")

# Access node properties correctly
user_props = (
user_node.properties
if user_node and hasattr(user_node, "properties")
else {}
)
identity_props = (
identity_node.properties
if identity_node and hasattr(identity_node, "properties")
else {}
)

request.session["user_info"] = {
"id": identity_props.get("provider_user_id", email),
"name": user_props.get("name", ""),
"email": user_props.get("email", email),
"picture": user_props.get("picture", ""),
"provider": "email",

user_data = {
'id': identity_props.get("provider_user_id", email),
'email': identity_props.get('email', email),
'name': identity_props.get('name', ''),
'picture': identity_props.get('picture', ''),
}
request.session["email_authenticated"] = True
request.session["token_validated_at"] = time.time()

return JSONResponse({"success": True, "message": "Login successful"})
else:
return JSONResponse(
{"success": False, "error": "Authentication failed"},
status_code=status.HTTP_401_UNAUTHORIZED
)
# Call the registered Google callback handler if it exists to store user data.
handler = getattr(request.app.state, "callback_handler", None)
if handler:
api_token = secrets.token_urlsafe(32) # ~43 chars, hard to guess

# Call the registered handler (await if async)
await handler('email', user_data, api_token)
response = JSONResponse({"success": True}, status_code=200)

response.set_cookie(
key="api_token",
value=api_token,

Check failure

Code scanning / CodeQL

Clear-text storage of sensitive information

This expression stores [sensitive data (password)](1) as clear text.

@gkorland gkorland Sep 7, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

but we're using secure cookie, why do we need to encrypt it twice?

httponly=True,
secure=_is_request_secure(request)
)
return response

return JSONResponse(
{"success": False, "error": "Authentication failed"},
status_code=status.HTTP_401_UNAUTHORIZED
)
except Exception as e:
logging.error("Login error: %s", e)
return JSONResponse(
{"success": False, "error": "Login failed"},
{"success": False, "error": "Login failed"},
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR
)

Expand Down
8 changes: 8 additions & 0 deletions app/public/css/modals.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,14 @@
display: block;
}

.user-profile-initial {
display: flex;
align-items: center;
justify-content: center;
background-color: var(--falkor-quaternary);
color: var(--text-primary);
font-weight: bold;
}
.user-profile-info {
padding: 15px;
border-bottom: 1px solid var(--falkor-quaternary);
Expand Down
6 changes: 5 additions & 1 deletion app/templates/components/user_profile.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
{# User profile button and dropdown #}
<button id="user-profile-btn" class="user-profile-btn" title="{{ user_info.name }}"
aria-haspopup="true" aria-expanded="false" aria-controls="user-profile-dropdown">
<img src="{{ user_info.picture }}" alt="{{ user_info.name[0] | upper }}" class="user-profile-img">
{% if user_info.picture %}
<img src="{{ user_info.picture }}" alt="{{ user_info.name[0] | upper }}" class="user-profile-img">
{% else %}
<div class="user-profile-img user-profile-initial">{{ user_info.name[0] | upper }}</div>
{% endif %}
Comment on lines +4 to +8

@coderabbitai coderabbitai Bot Sep 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Guard against empty/missing name; improve alt text.

Indexing user_info.name[0] will error if name is empty/None. Also set alt to the full name (or email) for accessibility.

-    {% if user_info.picture %}
-        <img src="{{ user_info.picture }}" alt="{{ user_info.name[0] | upper }}" class="user-profile-img">
-    {% else %}
-        <div class="user-profile-img user-profile-initial">{{ user_info.name[0] | upper }}</div>
-    {% endif %}
+    {% set display_char = (user_info.name or user_info.email or '?')[:1] | upper %}
+    {% set alt_text = (user_info.name or user_info.email or 'User') %}
+    {% if user_info.picture %}
+        <img src="{{ user_info.picture }}" alt="{{ alt_text }}" class="user-profile-img">
+    {% else %}
+        <div class="user-profile-img user-profile-initial">{{ display_char }}</div>
+    {% endif %}
📝 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
{% if user_info.picture %}
<img src="{{ user_info.picture }}" alt="{{ user_info.name[0] | upper }}" class="user-profile-img">
{% else %}
<div class="user-profile-img user-profile-initial">{{ user_info.name[0] | upper }}</div>
{% endif %}
{% set display_char = (user_info.name or user_info.email or '?')[:1] | upper %}
{% set alt_text = (user_info.name or user_info.email or 'User') %}
{% if user_info.picture %}
<img src="{{ user_info.picture }}" alt="{{ alt_text }}" class="user-profile-img">
{% else %}
<div class="user-profile-img user-profile-initial">{{ display_char }}</div>
{% endif %}
🤖 Prompt for AI Agents 
In app/templates/components/user_profile.j2 around lines 4 to 8, the template
indexes user_info.name[0] which will raise if name is missing or empty and the
alt text uses only the first initial; update the template to first guard the
name (e.g., check that user_info.name is defined and non-empty) before accessing
[0], and for alt text use the full display name or fallback to user_info.email
or a localized default string; also ensure the initials fallback uses the first
character of that safe display name so no direct indexing of a possibly
None/empty value occurs.

</button>
<div id="user-profile-dropdown" class="user-profile-dropdown" role="menu" aria-labelledby="user-profile-btn">
<div class="user-profile-info">
Expand Down
12 changes: 2 additions & 10 deletions app/ts/modules/modals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,16 +172,8 @@ async function handleEmailSignup(e: Event) {
const data = await response.json();

if (data.success) {
alert('Account created successfully! Please sign in.');
// Switch to login modal
const signupModal = document.getElementById('signup-modal') as HTMLElement | null;
const loginModal = document.getElementById('login-modal') as HTMLElement | null;
if (signupModal && loginModal) {
signupModal.style.display = 'none';
loginModal.style.display = 'flex';
}
// Clear form
form.reset();
// Reload page to update authentication state
window.location.reload();
} else {
alert(data.error || 'Signup failed. Please try again.');
}
Expand Down
2 changes: 1 addition & 1 deletion start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ FALKORDB_HOST="${FALKORDB_HOST:-localhost}"
FALKORDB_PORT="${FALKORDB_PORT:-6379}"

# Start FalkorDB Redis server in background
redis-server --loadmodule /var/lib/falkordb/bin/falkordb.so &
redis-server --loadmodule /var/lib/falkordb/bin/falkordb.so | cat &

@coderabbitai coderabbitai Bot Sep 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Pipeline hides Redis startup failures; remove pipe or enable pipefail.

set -e won’t catch failures from the first command in a pipeline. If redis-server fails, cat still exits 0 and the script continues.

-set -e
+set -e
+set -o pipefail
@@
-redis-server --loadmodule /var/lib/falkordb/bin/falkordb.so | cat &
+redis-server --loadmodule /var/lib/falkordb/bin/falkordb.so &

If you need to tee logs, prefer >> /var/log/redis.log 2>&1 & over a pipe.

📝 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
redis-server --loadmodule /var/lib/falkordb/bin/falkordb.so | cat &
set -e
set -o pipefail
redis-server --loadmodule /var/lib/falkordb/bin/falkordb.so &
🤖 Prompt for AI Agents 
In start.sh around line 10, the redis-server startup is piped into cat which
masks failures; remove the pipe or enable pipefail and explicitly check the
command status. Replace the piped invocation with a direct background start that
appends both stdout and stderr to /var/log/redis.log (so redis-server runs in
background and logs to that file), or add "set -o pipefail" near the top and
capture the pipeline exit status after starting to fail the script on
redis-server errors.


# Wait until FalkorDB is ready
echo "Waiting for FalkorDB to start on $FALKORDB_HOST:$FALKORDB_PORT..."
Expand Down