Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
25 changes: 19 additions & 6 deletions api/routes/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,18 @@ def _verify_password(password: str, stored_password_hex: str) -> bool:
return False

def _sanitize_for_log(value: str) -> str:
"""Sanitize user input for logging by removing newlines and carriage returns."""
"""Sanitize user input for logging by removing all newlines and separator characters."""
if not isinstance(value, str):
return str(value)
return value.replace('\r\n', '').replace('\n', '').replace('\r', '')
value = str(value)
# Remove CR, LF, CRLF, and Unicode line/paragraph separators
return (
value
.replace('\r\n', '')
.replace('\n', '')
.replace('\r', '')
.replace('\u2028', '')
.replace('\u2029', '')
)

def _validate_email(email: str) -> bool:
"""Basic email validation."""
Expand Down Expand Up @@ -257,7 +265,7 @@ async def email_signup(request: Request, signup_data: EmailSignupRequest) -> JSO
f"{first_name} {last_name}", "email", api_token)

if success and user_info and user_info["new_identity"]:
logging.info("New user created: %s", _sanitize_for_log(email))
logging.info("New user created: [%s]", _sanitize_for_log(email))

# Hash password
password_hash = _hash_password(password)
Expand All @@ -266,9 +274,14 @@ async def email_signup(request: Request, signup_data: EmailSignupRequest) -> JSO
await _set_mail_hash(email, password_hash)

else:
logging.info("User already exists: %s", _sanitize_for_log(email))
# User already exists - return error instead of success
logging.info("Signup attempt for existing user: [%s]", _sanitize_for_log(email))
return JSONResponse(
Copy link

Choose a reason for hiding this comment

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

[major] This new branch now treats any failure from ensure_user_in_organizations() as “user already exists” and forces a 409. When the helper returns success = False because the graph DB or callback handler failed, we’ll still execute this block, tell the user their email is taken, and hide the real 5xx error. That makes triage impossible and forces users to retry endlessly even though the backend is down. We need to distinguish between success is False (return a 500/"Registration failed") and the legit success=True/new_identity=False case before emitting this response.

{"success": False, "error": "An account with this email already exists"},
Copy link

Choose a reason for hiding this comment

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

[major] Returning "An account with this email already exists" along with a 409 leaks whether a particular email has ever registered. Attackers can now enumerate valid accounts just by hitting /signup/email. The earlier implementation always returned a generic success payload, so this is a new information disclosure. Please respond with a generic error (e.g. “Registration failed”/400) even when the identity already exists, and surface the specific reason only in logs.

status_code=status.HTTP_409_CONFLICT
)

logging.info("User registration successful: %s", _sanitize_for_log(email))
logging.info("User registration successful: [%s]", _sanitize_for_log(email))

response = JSONResponse({
"success": True,
Expand Down
307 changes: 251 additions & 56 deletions app/src/components/modals/LoginModal.tsx

Large diffs are not rendered by default.

327 changes: 327 additions & 0 deletions app/src/components/modals/SignupModal.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,327 @@
import { useState, useEffect } from 'react';
import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription } from "@/components/ui/dialog";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";
import { useAuth } from "@/contexts/AuthContext";
import { buildApiUrl, API_CONFIG } from "@/config/api";

interface SignupModalProps {
open: boolean;
onOpenChange: (open: boolean) => void;
onSwitchToLogin?: () => void;
canClose?: boolean;
startWithEmailForm?: boolean; // Start with email form already open
}

const SignupModal = ({ open, onOpenChange, onSwitchToLogin, canClose = true, startWithEmailForm = false }: SignupModalProps) => {
const { signup, refreshAuth } = useAuth();
const [firstName, setFirstName] = useState('');
const [lastName, setLastName] = useState('');
const [email, setEmail] = useState('');
const [password, setPassword] = useState('');
const [confirmPassword, setConfirmPassword] = useState('');
const [error, setError] = useState('');
const [isLoading, setIsLoading] = useState(false);
const [showEmailForm, setShowEmailForm] = useState(startWithEmailForm);

// Reset form when modal opens/closes or startWithEmailForm changes
useEffect(() => {
if (open) {
setShowEmailForm(startWithEmailForm);
setError('');
setFirstName('');
setLastName('');
setEmail('');
setPassword('');
setConfirmPassword('');
} else {
// Reset when closing
setShowEmailForm(false);
setError('');
setFirstName('');
setLastName('');
setEmail('');
setPassword('');
setConfirmPassword('');
}
}, [open, startWithEmailForm]);

const handleGoogleSignup = () => {
window.location.href = buildApiUrl(API_CONFIG.ENDPOINTS.LOGIN_GOOGLE);
};

const handleGithubSignup = () => {
window.location.href = buildApiUrl(API_CONFIG.ENDPOINTS.LOGIN_GITHUB);
};

const handleEmailSignup = async (e: React.FormEvent) => {
e.preventDefault();
setError('');

// Validation
if (!firstName.trim() || !lastName.trim() || !email.trim() || !password) {
setError('All fields are required');
return;
}

if (password.length < 8) {
setError('Password must be at least 8 characters long');
return;
}

if (password !== confirmPassword) {
setError('Passwords do not match');
return;
}

setIsLoading(true);
try {
const result = await signup.email(firstName, lastName, email, password);

if (result.success) {
// Refresh auth to get user data
await refreshAuth();
onOpenChange(false);
// Reset form
setFirstName('');
setLastName('');
setEmail('');
setPassword('');
setConfirmPassword('');
setShowEmailForm(false);
} else {
setError(result.error || 'Signup failed');
}
} catch (err) {
setError('An unexpected error occurred');
} finally {
setIsLoading(false);
}
};

return (
<Dialog
open={open}
onOpenChange={canClose ? onOpenChange : undefined}
>
<DialogContent
className="sm:max-w-[425px] bg-card border-border"
onInteractOutside={(e) => {
if (!canClose) {
e.preventDefault();
}
}}
onEscapeKeyDown={(e) => {
if (!canClose) {
e.preventDefault();
}
}}
>
<DialogHeader>
<DialogTitle className="text-2xl font-semibold text-center text-card-foreground">
Create Your Account
</DialogTitle>
<DialogDescription className="text-center text-muted-foreground pt-2">
Sign up to start using QueryWeaver
</DialogDescription>
</DialogHeader>

{!showEmailForm ? (
<div className="space-y-4 py-6">
<Button
onClick={handleGoogleSignup}
className="w-full bg-white hover:bg-gray-50 text-gray-900 hover:text-gray-900 border-2 border-gray-300 hover:border-gray-400 font-medium py-6 text-base flex items-center justify-center gap-3 shadow-sm hover:shadow transition-all"
variant="outline"
>
<svg className="w-5 h-5" viewBox="0 0 24 24">
<path
fill="currentColor"
d="M22.56 12.25c0-.78-.07-1.53-.2-2.25H12v4.26h5.92c-.26 1.37-1.04 2.53-2.21 3.31v2.77h3.57c2.08-1.92 3.28-4.74 3.28-8.09z"
/>
<path
fill="currentColor"
d="M12 23c2.97 0 5.46-.98 7.28-2.66l-3.57-2.77c-.98.66-2.23 1.06-3.71 1.06-2.86 0-5.29-1.93-6.16-4.53H2.18v2.84C3.99 20.53 7.7 23 12 23z"
/>
<path
fill="currentColor"
d="M5.84 14.09c-.22-.66-.35-1.36-.35-2.09s.13-1.43.35-2.09V7.07H2.18C1.43 8.55 1 10.22 1 12s.43 3.45 1.18 4.93l2.85-2.22.81-.62z"
/>
<path
fill="currentColor"
d="M12 5.38c1.62 0 3.06.56 4.21 1.64l3.15-3.15C17.45 2.09 14.97 1 12 1 7.7 1 3.99 3.47 2.18 7.07l3.66 2.84c.87-2.6 3.3-4.53 6.16-4.53z"
/>
</svg>
Sign up with Google
</Button>

<Button
onClick={handleGithubSignup}
className="w-full bg-gradient-to-r from-gray-200 to-gray-300 hover:from-gray-300 hover:to-gray-400 dark:from-[#24292e] dark:to-[#1a1e22] dark:hover:from-[#1b1f23] dark:hover:to-[#161a1d] text-gray-900 dark:text-white font-medium py-6 text-base flex items-center justify-center gap-3 shadow-md hover:shadow-lg transition-all border-2 border-gray-400 hover:border-gray-500 dark:border-gray-600"
>
<svg className="w-5 h-5" fill="currentColor" viewBox="0 0 24 24">
<path d="M12 0c-6.626 0-12 5.373-12 12 0 5.302 3.438 9.8 8.207 11.387.599.111.793-.261.793-.577v-2.234c-3.338.726-4.033-1.416-4.033-1.416-.546-1.387-1.333-1.756-1.333-1.756-1.089-.745.083-.729.083-.729 1.205.084 1.839 1.237 1.839 1.237 1.07 1.834 2.807 1.304 3.492.997.107-.775.418-1.305.762-1.604-2.665-.305-5.467-1.334-5.467-5.931 0-1.311.469-2.381 1.236-3.221-.124-.303-.535-1.524.117-3.176 0 0 1.008-.322 3.301 1.23.957-.266 1.983-.399 3.003-.404 1.02.005 2.047.138 3.006.404 2.291-1.552 3.297-1.23 3.297-1.23.653 1.653.242 2.874.118 3.176.77.84 1.235 1.911 1.235 3.221 0 4.609-2.807 5.624-5.479 5.921.43.372.823 1.102.823 2.222v3.293c0 .319.192.694.801.576 4.765-1.589 8.199-6.086 8.199-11.386 0-6.627-5.373-12-12-12z"/>
</svg>
Sign up with GitHub
</Button>

<div className="relative">
<div className="absolute inset-0 flex items-center">
<span className="w-full border-t border-border" />
</div>
<div className="relative flex justify-center text-xs uppercase">
<span className="bg-card px-2 text-muted-foreground">Or</span>
</div>
</div>

<Button
onClick={() => setShowEmailForm(true)}
className="w-full"
variant="outline"
>
Sign up with Email
</Button>

{onSwitchToLogin && (
<div className="text-center text-sm text-muted-foreground pt-4">
Already have an account?{' '}
<button
onClick={onSwitchToLogin}
className="text-primary hover:underline font-medium"
>
Sign in
</button>
</div>
)}
</div>
) : (
<form onSubmit={handleEmailSignup} className="space-y-4 py-6">
<div className="grid grid-cols-2 gap-4">
<div className="space-y-2">
<Label htmlFor="firstName">First Name</Label>
<Input
id="firstName"
type="text"
placeholder="John"
value={firstName}
onChange={(e) => setFirstName(e.target.value)}
required
disabled={isLoading}
/>
</div>
<div className="space-y-2">
<Label htmlFor="lastName">Last Name</Label>
<Input
id="lastName"
type="text"
placeholder="Doe"
value={lastName}
onChange={(e) => setLastName(e.target.value)}
required
disabled={isLoading}
/>
</div>
</div>

<div className="space-y-2">
<Label htmlFor="email">Email</Label>
<Input
id="email"
type="email"
placeholder="john@example.com"
value={email}
onChange={(e) => setEmail(e.target.value)}
required
disabled={isLoading}
/>
</div>

<div className="space-y-2">
<Label htmlFor="password">Password</Label>
<Input
id="password"
type="password"
placeholder="••••••••"
value={password}
onChange={(e) => setPassword(e.target.value)}
required
disabled={isLoading}
minLength={8}
/>
<p className="text-xs text-muted-foreground">
Must be at least 8 characters long
</p>
</div>

<div className="space-y-2">
<Label htmlFor="confirmPassword">Confirm Password</Label>
<Input
id="confirmPassword"
type="password"
placeholder="••••••••"
value={confirmPassword}
onChange={(e) => setConfirmPassword(e.target.value)}
required
disabled={isLoading}
/>
</div>

{error && (
<div className="text-sm text-destructive bg-destructive/10 p-3 rounded-md">
{error}
</div>
)}

<div className="flex gap-3">
<Button
type="button"
variant="outline"
className="flex-1"
onClick={() => {
setShowEmailForm(false);
setError('');
}}
disabled={isLoading}
>
Back
</Button>
<Button
type="submit"
className="flex-1"
disabled={isLoading}
>
{isLoading ? 'Creating Account...' : 'Create Account'}
</Button>
</div>

{onSwitchToLogin && (
<div className="text-center text-sm text-muted-foreground pt-2">
Already have an account?{' '}
<button
type="button"
onClick={() => {
if (onSwitchToLogin) {
onSwitchToLogin();
}
}}
className="text-primary hover:underline font-medium"
disabled={isLoading}
>
Sign in
</button>
</div>
)}
</form>
)}

{canClose && !showEmailForm && (
<div className="text-center text-sm text-muted-foreground pt-2">
<p>By signing up, you agree to our Terms of Service and Privacy Policy</p>
</div>
)}
</DialogContent>
</Dialog>
);
};

export default SignupModal;
2 changes: 2 additions & 0 deletions app/src/config/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export const API_CONFIG = {
AUTH_STATUS: '/auth-status',
LOGIN_GOOGLE: '/login/google',
LOGIN_GITHUB: '/login/github',
LOGIN_EMAIL: '/login/email',
SIGNUP_EMAIL: '/signup/email',
LOGOUT: '/logout',

// Graph/Database management
Expand Down
Loading