diff --git a/.gitignore b/.gitignore index d4bf7b2..ed6efb6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,5 @@ .env venv/ client-cpp/build/ -.claude/settings.json -.claude/settings.local.json -.vscode/settings.json +.claude/ +__pycache__/ diff --git a/docs/transcript_sarah.md b/docs/transcript_sarah.md index e21927a..e8fcb0a 100644 --- a/docs/transcript_sarah.md +++ b/docs/transcript_sarah.md @@ -149,3 +149,163 @@ For your app specifically: your auth routes will handle credentials and tokens, --- **Sarah:** update transcript + +**Claude:** Created `docs/transcript_sarah.md` with the full session to that point. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** now update the /register route to hash the password using ph before storing. no database insert yet, just replace the placeholder with the hash and return it in the response so we can test it + +**Claude:** Read `auth/routes.py` and found the hashing was already implemented — `ph.hash(data['password'])` on line 25 with the hash returned in the response. No changes needed. + +--- + +**Sarah:** good now remove the hash from the response. it should never be returned to the client. just return the 201 message + +**Claude:** Removed `'hash': password_hash` from the `jsonify` response. Route now returns only `{'message': 'User registered successfully'}` with 201. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** update /register to accept these additional fields in the JSON body: x25519_public_key, hpke_wrapped_private_key, argon2id_kek_salt. add them to the missing fields validation. then insert all fields into the users table using get_db(). generate the id with uuid.uuid4(), extract the salt from the argon2id hash string, set tofu_key_pinned_at to now. handle duplicate username or email with a 409 + +**Claude:** Proposed full implementation with imports (`uuid`, `datetime`, `mysql.connector`, `get_db`), `REQUIRED_FIELDS` list, salt extraction via `password_hash.split('$')[4]`, INSERT with column names `password_hash` and `argon2id_salt`, and `IntegrityError` errno 1062 → 409. + +**Sarah:** [REJECTED] — column names `password_hash` and `argon2id_salt` don't match schema. actual columns are `argon2id_server_hash` and `argon2id_server_salt`. + +**Claude:** Reapplied with corrected column names `argon2id_server_hash` and `argon2id_server_salt`. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** what happens if the INSERT succeeds but db.commit() throws an exception. is the row left in a partial state? + +**Claude:** Explained that the row is not persisted — InnoDB holds it in an uncommitted transaction until `commit()` is called. However, the existing code had no explicit rollback on commit failure; it relied on `close_db` teardown to implicitly roll back. Proposed adding `db.rollback()` explicitly in both the `IntegrityError` handler and a bare `except Exception` block. + +--- + +**Sarah:** yes apply that + +**Claude:** Added `db.rollback()` to the `IntegrityError` handler and added a new `except Exception` block with `db.rollback()` and `raise`. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** memory? update the transcript in the docs folder with my ai prompts and responses + +**Claude:** Clarified that "memory" referred to the auto-memory system, then updated `docs/transcript_sarah.md` with all exchanges from the session. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** before writing any code, what do i need to verify on /login and what should i return if it succeeds + +**Claude:** Outlined the full login verification sequence: validate JSON and required fields; look up user by username; return 401 (not 404) if not found to avoid username enumeration; call `ph.verify()` on the stored hash; handle `VerifyMismatchError` → 401; call `ph.check_needs_rehash()` and update if needed. Flagged timing attack risk — "user not found" returns faster than "wrong password" because argon2 is skipped. Fix: run `ph.verify(_DUMMY_HASH, password)` even when user is not found. On success, return JWT (with `user_id`, `username`, `exp`) plus `hpke_wrapped_private_key`, `argon2id_kek_salt`, `x25519_public_key` so the client can unwrap the private key locally. + +--- + +**Sarah:** you mentioned a timing attack fix using a dummy hash, what would that dummy hash be and where would it be defined + +**Claude:** Explained it is a real argon2id hash of an arbitrary string computed once at module load using the same `ph` instance, defined right after `ph`. The string doesn't need to be secret. Must be module-level (not per-request) to avoid doubling work on every failed login. + +--- + +**Sarah:** good. now write the /login route using everything you just described. use flask-jwt-extended for the token. JWT payload should include user_id, username and exp + +**Claude:** Noted `__init__.py` was missing `JWT_SECRET_KEY` in app config and `JWTManager` initialisation. Proposed updating both files, with `JWT_ACCESS_TOKEN_EXPIRES` set to 1 hour. + +**Sarah:** [REJECTED] — queried whether 1 hour is appropriate for a security project. + +**Claude:** Explained tradeoff: shorter expiry limits stolen-token exposure but forces re-login; proper fix is short-lived access tokens + refresh tokens. Asked whether to keep it simple with one token or implement refresh tokens. + +--- + +**Sarah:** change this to 15 minutes. this is a security project and we should default to the most secure option. add a comment explaining the tradeoff and why we chose 15 mins + +**Claude:** Updated `__init__.py` to set `JWT_SECRET_KEY`, `JWT_ACCESS_TOKEN_EXPIRES = timedelta(minutes=15)`, initialise `JWTManager(app)`, and added a comment explaining the 15-minute choice and the refresh token tradeoff. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** yes [proceed with /login route] + +**Claude:** Proposed full `/login` route: `VerifyMismatchError` and `create_access_token` imports, `_DUMMY_HASH` at module level, SELECT with `dictionary=True`, dummy verify on unknown username, `VerifyMismatchError` → 401, rehash block with its own cursor and rollback guard, JWT with `identity=user_id` and `additional_claims={'username': ...}`, response includes token and three client key fields. + +**Sarah:** [REJECTED] — queried whether reusing the `cursor` variable name after closing it in the SELECT block was safe for the rehash UPDATE. + +**Claude:** Confirmed safe — `cursor = db.cursor()` in the rehash block creates a new cursor object and reassigns the variable. The closed cursor from the SELECT block is unaffected. The connection `db` is intentionally reused. + +--- + +**Sarah:** yes [proceed] + +**Claude:** Applied full `/login` route to `auth/routes.py`. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** update transcript + +**Claude:** Updated `docs/transcript_sarah.md` with all exchanges from the session to that point. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** pin all dependencies in requirements.txt with minimum versions based on what's currently installed in the venv. use >= not == so we're not too restrictive + +**Claude:** Read `requirements.txt` (7 unpinned packages) and ran `pip freeze` against the venv. Updated all seven direct dependencies with `>=` minimum versions based on installed versions: `flask>=3.1.3`, `flask-jwt-extended>=4.7.4`, `argon2-cffi>=25.1.0`, `mysql-connector-python>=9.7.0`, `web3>=7.16.0`, `python-dotenv>=1.2.2`, `gunicorn>=26.0.0`. Transitive dependencies left unspecified. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** update the field validation in /register and /login to check that each required field is a non-empty string, not just present. return 400 if any field is not a string or is empty after stripping whitespace + +**Claude:** Added a `_invalid_fields(data, fields)` helper to `auth/routes.py` that rejects a field if it is missing, not a `str`, or blank after `.strip()`. Replaced the `not data.get(f)` list comprehension in both `/register` and `/login` with calls to this helper. Error message updated to "Missing or invalid fields". + +**Sarah:** [APPROVED] + +--- + +**Sarah:** update transcript + +**Claude:** Updated `docs/transcript_sarah.md` with all exchanges from the session to that point. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** add a check in both /register and /login that returns 400 if data is not a dict before calling _invalid_fields. something like: if not isinstance(data, dict) + +**Claude:** Replaced the `if not data:` guard in both `/register` and `/login` with `if not isinstance(data, dict):`, returning 400 with `'Request body must be a JSON object'`. This correctly rejects `None`, JSON arrays, strings, and numbers — anything that isn't an object — before `_invalid_fields` is ever called. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** update transcript + +**Claude:** Updated `docs/transcript_sarah.md` with all exchanges from the session to that point. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** use request.get_json(silent=True) in both /register and /login so malformed JSON returns None and hits our isinstance check instead of raising BadRequest + +**Claude:** Updated `auth/routes.py` lines 36 and 87 — changed `request.get_json()` to `request.get_json(silent=True)` in both `/register` and `/login`. Malformed JSON now returns `None` instead of raising a `BadRequest` exception, which falls cleanly into the existing `isinstance(data, dict)` guard and returns a 400. + +**Sarah:** [APPROVED] + +--- + +**Sarah:** update transcript diff --git a/requirements.txt b/requirements.txt index b783a6c..13f8714 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,7 @@ -Flask==3.1.3 -mysql-connector-python==9.3.0 -python-dotenv==1.2.2 +flask>=3.1.3 +flask-jwt-extended>=4.7.4 +argon2-cffi>=25.1.0 +mysql-connector-python>=9.7.0 +web3>=7.16.0 +python-dotenv>=1.2.2 +gunicorn>=26.0.0 diff --git a/server/app/__init__.py b/server/app/__init__.py index 45dbab6..1011519 100644 --- a/server/app/__init__.py +++ b/server/app/__init__.py @@ -1,6 +1,8 @@ import os import mysql.connector +from datetime import timedelta from flask import Flask, g, current_app +from flask_jwt_extended import JWTManager def get_db(): if 'db' not in g: @@ -32,6 +34,13 @@ def create_app(): app.config['DB_PASSWORD'] = os.getenv('DB_PASSWORD') app.config['DB_NAME'] = os.getenv('DB_NAME') app.config['SECRET_KEY'] = os.getenv('JWT_SECRET_KEY') + app.config['JWT_SECRET_KEY'] = os.getenv('JWT_SECRET_KEY') + # 15 minutes balances security and usability: short enough to limit exposure if a token + # is stolen, at the cost of requiring re-login for long sessions. A refresh token endpoint + # would remove that UX penalty, but adds scope — chosen not to implement for now. + app.config['JWT_ACCESS_TOKEN_EXPIRES'] = timedelta(minutes=15) + + JWTManager(app) @app.teardown_appcontext def close_db(e=None): diff --git a/server/app/auth/routes.py b/server/app/auth/routes.py index 00c09a3..7f9b6e1 100644 --- a/server/app/auth/routes.py +++ b/server/app/auth/routes.py @@ -1,16 +1,156 @@ -from flask import Blueprint, request, jsonify +from flask import Blueprint, request, jsonify, current_app +from argon2 import PasswordHasher +from argon2.exceptions import VerifyMismatchError +from flask_jwt_extended import create_access_token +import uuid +from datetime import datetime, timezone +import mysql.connector +from .. import get_db + +# Argon2id params (explicit to ensure stability across library versions): +# time_cost=3 — number of iterations; increases CPU cost for attackers +# memory_cost=65536 — 64MB RAM required per hash; resists GPU/ASIC brute force +# parallelism=4 — threads per hash; matched to typical server core count +# hash_len=32 — 256-bit output; exceeds minimum security margin +# salt_len=16 — 128-bit random salt; prevents precomputation attacks +ph = PasswordHasher(time_cost=3, memory_cost=65536, parallelism=4, hash_len=32, salt_len=16) +# Pre-computed at startup for timing attack mitigation in /login — see route for usage. +_DUMMY_HASH = ph.hash("dummy") auth_bp = Blueprint('auth', __name__) +REQUIRED_FIELDS = [ + 'username', 'email', 'password', + 'x25519_public_key', 'hpke_wrapped_private_key', 'argon2id_kek_salt', +] + +def _invalid_fields(data, fields): + """Return fields that are missing, not a string, or blank after stripping.""" + return [ + f for f in fields + if not isinstance(data.get(f), str) or not data[f].strip() + ] + @auth_bp.route('/register', methods=['POST']) def register(): - data = request.get_json() + data = request.get_json(silent=True) + + if not isinstance(data, dict): + return jsonify({'error': 'Request body must be a JSON object'}), 400 - if not data: - return jsonify({'error': 'Request body must be JSON'}), 400 + invalid = _invalid_fields(data, REQUIRED_FIELDS) + if invalid: + return jsonify({'error': f"Missing or invalid fields: {', '.join(invalid)}"}), 400 - missing = [f for f in ['username', 'email', 'password'] if not data.get(f)] - if missing: - return jsonify({'error': f"Missing required fields: {', '.join(missing)}"}), 400 + password_hash = ph.hash(data['password']) + # Hash format: $argon2id$v=19$m=...,t=...,p=...$$ + argon2id_salt = password_hash.split('$')[4] + + user_id = str(uuid.uuid4()) + now = datetime.now(timezone.utc) + + db = get_db() + cursor = db.cursor() + try: + cursor.execute( + """ + INSERT INTO users + (id, username, email, argon2id_server_hash, argon2id_server_salt, + x25519_public_key, hpke_wrapped_private_key, argon2id_kek_salt, + tofu_key_pinned_at) + VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s) + """, + ( + user_id, data['username'], data['email'], + password_hash, argon2id_salt, + data['x25519_public_key'], data['hpke_wrapped_private_key'], + data['argon2id_kek_salt'], now, + ), + ) + db.commit() + except mysql.connector.IntegrityError as e: + db.rollback() + if e.errno == 1062: + return jsonify({'error': 'Username or email already exists'}), 409 + raise + except Exception: + db.rollback() + raise + finally: + cursor.close() return jsonify({'message': 'User registered successfully'}), 201 + + +@auth_bp.route('/login', methods=['POST']) +def login(): + data = request.get_json(silent=True) + + if not isinstance(data, dict): + return jsonify({'error': 'Request body must be a JSON object'}), 400 + + invalid = _invalid_fields(data, ['username', 'password']) + if invalid: + return jsonify({'error': f"Missing or invalid fields: {', '.join(invalid)}"}), 400 + + db = get_db() + cursor = db.cursor(dictionary=True) + try: + cursor.execute( + """ + SELECT id, username, argon2id_server_hash, + hpke_wrapped_private_key, argon2id_kek_salt, x25519_public_key + FROM users + WHERE username = %s + """, + (data['username'],), + ) + user = cursor.fetchone() + finally: + cursor.close() + + if user is None: + # Run a verify against a dummy hash so response time is indistinguishable + # from a wrong-password attempt — prevents username enumeration via timing. + try: + ph.verify(_DUMMY_HASH, data['password']) + except Exception: + pass + return jsonify({'error': 'Invalid credentials'}), 401 + + try: + ph.verify(user['argon2id_server_hash'], data['password']) + except VerifyMismatchError: + return jsonify({'error': 'Invalid credentials'}), 401 + + if ph.check_needs_rehash(user['argon2id_server_hash']): + new_hash = ph.hash(data['password']) + new_salt = new_hash.split('$')[4] + cursor = db.cursor() + try: + cursor.execute( + """ + UPDATE users + SET argon2id_server_hash = %s, argon2id_server_salt = %s + WHERE id = %s + """, + (new_hash, new_salt, user['id']), + ) + db.commit() + except Exception as e: + db.rollback() + current_app.logger.error("Rehash failed for user %s: %s", user['id'], e) + finally: + cursor.close() + + token = create_access_token( + identity=str(user['id']), + additional_claims={'username': user['username']}, + ) + + return jsonify({ + 'token': token, + 'hpke_wrapped_private_key': user['hpke_wrapped_private_key'], + 'argon2id_kek_salt': user['argon2id_kek_salt'], + 'x25519_public_key': user['x25519_public_key'], + }), 200