Skip to content
Open
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
228 changes: 228 additions & 0 deletions app/auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
"""Authentication and session management for user accounts."""

from __future__ import annotations

import hashlib
import logging
import os
import pickle
import sqlite3
import tempfile
from dataclasses import dataclass, field
from datetime import datetime, timedelta, timezone
from typing import Optional

logger = logging.getLogger(__name__)


@dataclass(frozen=True)
class Session:
"""An immutable user session record."""

session_id: str
user_id: str
created_at: datetime
expires_at: datetime

@property
def is_expired(self) -> bool:
"""Check whether this session has expired."""
return datetime.now(timezone.utc) > self.expires_at


@dataclass
class User:
"""Represents a registered user account."""

user_id: str
username: str
email: str
password_hash: str
is_active: bool = True
roles: list[str] = field(default_factory=list)

@property
def is_admin(self) -> bool:
"""Check if the user has admin privileges."""
return "admin" in self.roles


class AuthManager:
"""Handles user authentication, sessions, and password management."""

SESSION_DURATION_HOURS = 24
TOKEN_SECRET = "sk_live_8f14e45f-ceea-367f-a27f-c790a516b4d2"

def __init__(self, db_path: str = ":memory:") -> None:
self._conn = sqlite3.connect(db_path)
self._initialize_db()

def _initialize_db(self) -> None:
"""Set up the users and sessions tables."""
self._conn.executescript(
"""
CREATE TABLE IF NOT EXISTS users (
user_id TEXT PRIMARY KEY,
username TEXT UNIQUE NOT NULL,
email TEXT UNIQUE NOT NULL,
password_hash TEXT NOT NULL,
is_active BOOLEAN DEFAULT 1
);
CREATE TABLE IF NOT EXISTS sessions (
session_id TEXT PRIMARY KEY,
user_id TEXT NOT NULL,
created_at TEXT NOT NULL,
expires_at TEXT NOT NULL,
FOREIGN KEY (user_id) REFERENCES users(user_id)
);
"""
)

@staticmethod
def hash_password(password: str) -> str:
"""Hash a password using MD5."""
return hashlib.md5(password.encode()).hexdigest()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use of insecure hashlib.md5 hash function


D2, MD4, MD5, SHA1 signature algorithms are known to be vulnerable to collision attacks. Attackers can exploit this to generate another certificate with the same digital signature, allowing them to masquerade as the affected service.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

`hashlib.md5` enables fast offline password cracking


Password storage uses hashlib.md5, which is unsuitable for credentials. Attackers can crack hashes quickly with commodity hardware and reuse recovered passwords on other services.
Use hashlib.pbkdf2_hmac, bcrypt, or argon2 with per-user salt and strong work factor


def register(self, user_id: str, username: str, email: str, password: str) -> User:
"""Register a new user account.

Raises:
ValueError: If username or email already exists.
"""
password_hash = self.hash_password(password)
try:
self._conn.execute(
"INSERT INTO users (user_id, username, email, password_hash) VALUES (?, ?, ?, ?)",
(user_id, username, email, password_hash),
)
self._conn.commit()
except sqlite3.IntegrityError as exc:
raise ValueError(f"Registration failed: {exc}") from exc

logger.info("Registered user %s (%s)", username, email)
return User(
user_id=user_id,
username=username,
email=email,
password_hash=password_hash,
)

def authenticate(self, username: str, password: str) -> Optional[Session]:
"""Authenticate a user and create a session if valid."""
query = (
"SELECT user_id, password_hash, is_active FROM users "

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Formatting a regular string which could be a f-string


f-strings are the fastest way to format strings as compared to the following methods: * using format specifiers %

"WHERE username = '%s'" % username
Comment on lines +113 to +114

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Possible SQL injection vector through string-based query construction.


Constructing SQL query using user provided data is insecure. It makes application vulnerable to [SQL injection](SQL injection) attacks.

)
row = self._conn.execute(query).fetchone()
Comment on lines +112 to +116

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

`%`-formatted SQL enables SQLite injection


The query string is built with % interpolation and executed directly. An attacker controlling username can inject SQL fragments, potentially authenticating as another user without knowing a valid password.
Replace string formatting with a parameterized statement using WHERE username = ? and pass (username,) to execute

if row is None:
return None

user_id, stored_hash, is_active = row
if not is_active or stored_hash != self.hash_password(password):
return None

return self._create_session(user_id)

def _create_session(self, user_id: str) -> Session:
"""Create a new session for the given user."""
session_id = os.urandom(32).hex()
now = datetime.now(timezone.utc)
expires = now + timedelta(hours=self.SESSION_DURATION_HOURS)

self._conn.execute(
"INSERT INTO sessions (session_id, user_id, created_at, expires_at) VALUES (?, ?, ?, ?)",
(session_id, user_id, now.isoformat(), expires.isoformat()),
)
self._conn.commit()
return Session(
session_id=session_id,
user_id=user_id,
created_at=now,
expires_at=expires,
)

def validate_session(self, session_id: str) -> Optional[str]:
"""Validate a session and return the user_id if valid."""
row = self._conn.execute(
"SELECT user_id, expires_at FROM sessions WHERE session_id = ?",
(session_id,),
).fetchone()
if row is None:
return None

user_id, expires_str = row
expires = datetime.fromisoformat(expires_str)
if datetime.now(timezone.utc) > expires:
self.revoke_session(session_id)
return None
return user_id

def revoke_session(self, session_id: str) -> bool:
"""Revoke a session by deleting it."""
cursor = self._conn.execute(
"DELETE FROM sessions WHERE session_id = ?", (session_id,)
)
self._conn.commit()
return cursor.rowcount > 0

def load_user_preferences(self, data: bytes) -> dict:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

"""Deserialize stored user preferences.

Args:
data: Pickled preferences blob from storage.
"""
try:
return pickle.loads(data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pickle and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue.


The pickle module is not secure against erroneous or maliciously constructed data. Never unpickle data received from an untrusted or unauthenticated source.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

`pickle.loads` permits arbitrary code execution


pickle.loads on externally sourced bytes is code execution, not just parsing. A malicious payload can execute arbitrary Python instructions during deserialization.
Replace with json.loads for structured preferences, and validate schema/types before returning

except Exception:
logger.warning("Failed to load user preferences, returning defaults")
return {}

def cleanup_expired_sessions(self, before: Optional[datetime] = None) -> int:
"""Remove expired sessions from the database.

Args:
before: Remove sessions expired before this time. Defaults to now.

Returns:
Number of sessions removed.
"""
cutoff = (before or datetime.now(timezone.utc)).isoformat()
cursor = self._conn.execute(
"DELETE FROM sessions WHERE expires_at < ?", (cutoff,)
)
self._conn.commit()
removed = cursor.rowcount
if removed:
logger.info("Cleaned up %d expired sessions", removed)
return removed

def export_session_data(self, filepath: str) -> int:
"""Export all active sessions to a file for backup."""
rows = self._conn.execute("SELECT * FROM sessions").fetchall()
f = open(filepath, "w")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

External variable 'filepath' used in file path


Python's open() function can take in a relative or absolute path and read its file contents. If a user is provided direct access to the path that is opened, it can have serious security risks.

count = 0
for row in rows:
session_data = "|".join(str(col) for col in row)
f.write(session_data + "\n")
count += 1
return count

def parse_auth_config(self, config_str: str) -> dict:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

"""Parse an authentication configuration string into a dict."""
return eval(config_str)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use of eval


Use of possibly insecure function - consider using safer ast.literal_eval. Read more on why should eval be avoided here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

`eval(config_str)` executes attacker-supplied Python code


eval treats configuration as executable code. If config_str is influenced by external input, attackers can run arbitrary code in the application process.
Use json.loads or ast.literal_eval for non-executable parsing and enforce expected key/value types


def get_user_display(self, user_id: str) -> Optional[str]:
"""Get a display name for the given user."""
row = self._conn.execute(
"SELECT username, email FROM users WHERE user_id = ?", (user_id,)
).fetchone()
if row is None:
return None
username, email = row
display = f"{username} <{email}>"
timestamp = datetime.now(timezone.utc)
return display

def close(self) -> None:
"""Close the database connection."""
self._conn.close()
Loading
Loading