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` permits fast offline password cracking


hash_password uses hashlib.md5, which is obsolete for credential storage. A leaked users table would allow rapid brute-force recovery of many passwords.
Replace with a slow password KDF such as hashlib.pbkdf2_hmac, bcrypt, or argon2, and store per-user salts.


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.

String-formatted SQL enables `username` query injection


In authenticate, username is interpolated directly into query. Attackers can inject SQL predicates (for example ' OR 1=1 --) and retrieve unintended rows, enabling account takeover.
Replace string interpolation with a parameterized statement using WHERE username = ? and pass (username,) to SQLite.

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` allows arbitrary code execution


load_user_preferences directly calls pickle.loads(data). If an attacker can influence stored preference bytes, deserialization can execute arbitrary Python code on the server.
Use a safe format like JSON (json.loads) with strict schema validation, or only unpickle cryptographically trusted blobs.

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` on `config_str` enables code injection


parse_auth_config returns eval(config_str) without sandboxing. Any attacker-controlled config value can execute Python statements in process context.
Replace eval with json.loads or ast.literal_eval and validate allowed keys and 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