From 93cb4eda5befb9146f743cec58f4cd29a3fe3f8b Mon Sep 17 00:00:00 2001 From: tanyar09 Date: Fri, 21 Nov 2025 13:33:13 -0500 Subject: [PATCH] feat: Implement auth user management API and UI for admin users This commit introduces a new Auth User management feature, allowing admins to create, update, delete, and list users in the auth database. A dedicated API has been implemented with endpoints for managing auth users, including validation for unique email addresses. The frontend has been updated to include a Manage Users page with tabs for backend and frontend users, enhancing the user experience. Additionally, modals for creating and editing auth users have been added, along with appropriate error handling and loading states. Documentation has been updated to reflect these changes. --- frontend/src/api/authUsers.ts | 65 ++ frontend/src/api/client.ts | 11 +- frontend/src/api/users.ts | 8 +- frontend/src/pages/Login.tsx | 8 +- frontend/src/pages/ManageUsers.tsx | 758 ++++++++++++++++----- scripts/grant_auth_db_delete_permission.sh | 64 ++ scripts/grant_auth_db_permissions.py | 99 +++ scripts/recreate_tables_web.py | 15 - scripts/setup_postgresql.sh | 1 + src/web/api/auth_users.py | 368 ++++++++++ src/web/api/users.py | 17 + src/web/app.py | 53 +- src/web/db/models.py | 5 +- src/web/schemas/auth_users.py | 56 ++ src/web/schemas/users.py | 8 +- 15 files changed, 1348 insertions(+), 188 deletions(-) create mode 100644 frontend/src/api/authUsers.ts create mode 100755 scripts/grant_auth_db_delete_permission.sh create mode 100755 scripts/grant_auth_db_permissions.py create mode 100644 src/web/api/auth_users.py create mode 100644 src/web/schemas/auth_users.py diff --git a/frontend/src/api/authUsers.ts b/frontend/src/api/authUsers.ts new file mode 100644 index 0000000..e6b1627 --- /dev/null +++ b/frontend/src/api/authUsers.ts @@ -0,0 +1,65 @@ +import apiClient from './client' + +export interface AuthUserResponse { + id: number + name: string | null + email: string + is_admin: boolean | null + has_write_access: boolean | null + created_at: string | null + updated_at: string | null +} + +export interface AuthUserCreateRequest { + email: string + name: string + password: string + is_admin: boolean + has_write_access: boolean +} + +export interface AuthUserUpdateRequest { + email: string + name: string + is_admin: boolean + has_write_access: boolean +} + +export interface AuthUsersListResponse { + items: AuthUserResponse[] + total: number +} + +export const authUsersApi = { + listUsers: async (): Promise => { + const { data } = await apiClient.get('/api/v1/auth-users') + return data + }, + + getUser: async (userId: number): Promise => { + const { data } = await apiClient.get(`/api/v1/auth-users/${userId}`) + return data + }, + + createUser: async (request: AuthUserCreateRequest): Promise => { + const { data } = await apiClient.post('/api/v1/auth-users', request) + return data + }, + + updateUser: async ( + userId: number, + request: AuthUserUpdateRequest + ): Promise => { + const { data } = await apiClient.put( + `/api/v1/auth-users/${userId}`, + request + ) + return data + }, + + deleteUser: async (userId: number): Promise => { + await apiClient.delete(`/api/v1/auth-users/${userId}`) + }, +} + + diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index bbf8d09..162bdac 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -23,11 +23,20 @@ apiClient.interceptors.response.use( (response) => response, (error) => { if (error.response?.status === 401) { + // Don't redirect if we're already on the login page (prevents clearing error messages) + const isLoginPage = window.location.pathname === '/login' + + // Always clear tokens on 401, but only redirect if not already on login page localStorage.removeItem('access_token') localStorage.removeItem('refresh_token') // Clear sessionStorage settings on authentication failure sessionStorage.removeItem('identify_settings') - window.location.href = '/login' + + // Only redirect if not already on login page + if (!isLoginPage) { + window.location.href = '/login' + } + // If on login page, just reject the error so the login component can handle it } return Promise.reject(error) } diff --git a/frontend/src/api/users.ts b/frontend/src/api/users.ts index 98cbc1a..8a1a45b 100644 --- a/frontend/src/api/users.ts +++ b/frontend/src/api/users.ts @@ -14,16 +14,16 @@ export interface UserResponse { export interface UserCreateRequest { username: string password: string - email?: string | null - full_name?: string | null + email: string + full_name: string is_active?: boolean is_admin?: boolean } export interface UserUpdateRequest { password?: string | null - email?: string | null - full_name?: string | null + email: string + full_name: string is_active?: boolean is_admin?: boolean } diff --git a/frontend/src/pages/Login.tsx b/frontend/src/pages/Login.tsx index ad01e9b..a41a9e0 100644 --- a/frontend/src/pages/Login.tsx +++ b/frontend/src/pages/Login.tsx @@ -11,13 +11,16 @@ export default function Login() { const navigate = useNavigate() useEffect(() => { - if (!isLoading && isAuthenticated) { + // Only redirect if user is already authenticated (e.g., visiting /login while logged in) + // Don't redirect on isLoading changes during login attempts + if (isAuthenticated && !isLoading) { navigate('/', { replace: true }) } }, [isAuthenticated, isLoading, navigate]) const handleSubmit = async (e: React.FormEvent) => { e.preventDefault() + e.stopPropagation() setError('') setLoading(true) @@ -35,7 +38,8 @@ export default function Login() { } } - if (isLoading) { + // Only show loading screen on initial auth check, not during login attempts + if (isLoading && !loading) { return
Loading...
} diff --git a/frontend/src/pages/ManageUsers.tsx b/frontend/src/pages/ManageUsers.tsx index 2b490ff..829b3f9 100644 --- a/frontend/src/pages/ManageUsers.tsx +++ b/frontend/src/pages/ManageUsers.tsx @@ -1,7 +1,13 @@ import { useState, useEffect } from 'react' import { usersApi, UserResponse, UserCreateRequest, UserUpdateRequest } from '../api/users' +import { authUsersApi, AuthUserResponse, AuthUserCreateRequest, AuthUserUpdateRequest } from '../api/authUsers' + +type TabType = 'backend' | 'frontend' export default function ManageUsers() { + const [activeTab, setActiveTab] = useState('backend') + + // Backend users state const [users, setUsers] = useState([]) const [loading, setLoading] = useState(true) const [error, setError] = useState(null) @@ -27,9 +33,35 @@ export default function ManageUsers() { is_admin: false, }) + // Frontend users state + const [authUsers, setAuthUsers] = useState([]) + const [authLoading, setAuthLoading] = useState(true) + const [authError, setAuthError] = useState(null) + const [showAuthCreateModal, setShowAuthCreateModal] = useState(false) + const [editingAuthUser, setEditingAuthUser] = useState(null) + + const [authCreateForm, setAuthCreateForm] = useState({ + email: '', + name: '', + password: '', + is_admin: false, + has_write_access: false, + }) + + const [authEditForm, setAuthEditForm] = useState({ + email: '', + name: '', + is_admin: false, + has_write_access: false, + }) + useEffect(() => { - loadUsers() - }, [filterActive, filterAdmin]) + if (activeTab === 'backend') { + loadUsers() + } else { + loadAuthUsers() + } + }, [activeTab, filterActive, filterAdmin]) const loadUsers = async () => { try { @@ -47,6 +79,19 @@ export default function ManageUsers() { } } + const loadAuthUsers = async () => { + try { + setAuthLoading(true) + setAuthError(null) + const response = await authUsersApi.listUsers() + setAuthUsers(response.items) + } catch (err: any) { + setAuthError(err.response?.data?.detail || 'Failed to load auth users') + } finally { + setAuthLoading(false) + } + } + const handleCreate = async () => { try { setError(null) @@ -66,11 +111,28 @@ export default function ManageUsers() { } } + const handleAuthCreate = async () => { + try { + setAuthError(null) + await authUsersApi.createUser(authCreateForm) + setShowAuthCreateModal(false) + setAuthCreateForm({ + email: '', + name: '', + password: '', + is_admin: false, + has_write_access: false, + }) + loadAuthUsers() + } catch (err: any) { + setAuthError(err.response?.data?.detail || 'Failed to create auth user') + } + } + const handleUpdate = async () => { if (!editingUser) return try { setError(null) - // Only send password if it's not empty const updateData: UserUpdateRequest = { ...editForm, password: editForm.password && editForm.password.trim() !== '' @@ -92,6 +154,30 @@ export default function ManageUsers() { } } + const handleAuthUpdate = async () => { + if (!editingAuthUser) return + try { + setAuthError(null) + const updateData: AuthUserUpdateRequest = { + email: authEditForm.email, + name: authEditForm.name, + is_admin: authEditForm.is_admin, + has_write_access: authEditForm.has_write_access, + } + await authUsersApi.updateUser(editingAuthUser.id, updateData) + setEditingAuthUser(null) + setAuthEditForm({ + email: '', + name: '', + is_admin: false, + has_write_access: false, + }) + loadAuthUsers() + } catch (err: any) { + setAuthError(err.response?.data?.detail || 'Failed to update auth user') + } + } + const handleDelete = async (userId: number, username: string) => { if (!confirm(`Are you sure you want to delete user "${username}"?`)) { return @@ -105,6 +191,19 @@ export default function ManageUsers() { } } + const handleAuthDelete = async (userId: number, name: string) => { + if (!confirm(`Are you sure you want to delete user "${name}"?`)) { + return + } + try { + setAuthError(null) + await authUsersApi.deleteUser(userId) + loadAuthUsers() + } catch (err: any) { + setAuthError(err.response?.data?.detail || 'Failed to delete auth user') + } + } + const startEdit = (user: UserResponse) => { setEditingUser(user) setEditForm({ @@ -116,160 +215,306 @@ export default function ManageUsers() { }) } + const startAuthEdit = (user: AuthUserResponse) => { + setEditingAuthUser(user) + setAuthEditForm({ + email: user.email || '', + name: user.name || '', + is_admin: user.is_admin === true, + has_write_access: user.has_write_access === true, + }) + } + const formatDate = (dateString: string | null) => { if (!dateString) return 'Never' return new Date(dateString).toLocaleString() } + const formatDateShort = (dateString: string | null) => { + if (!dateString) return '-' + const date = new Date(dateString) + return date.toISOString().split('T')[0] // YYYY-MM-DD format + } + return (

Manage Users

- {error && ( + {/* Tabs */} +
+ +
+ + {/* Error Messages */} + {error && activeTab === 'backend' && (
{error}
)} - - {/* Filters */} -
-
- - - + {authError && activeTab === 'frontend' && ( +
+ {authError}
-
+ )} - {/* Users Table */} -
- {loading ? ( -
Loading users...
- ) : users.length === 0 ? ( -
No users found
- ) : ( - - - - - - - - - - - - - - - {users.map((user) => ( - - - - - - - - - + {/* Backend Users Tab */} + {activeTab === 'backend' && ( + <> + {/* Filters */} +
+
+ + + +
+
+ + {/* Users Table */} +
+ {loading ? ( +
Loading users...
+ ) : users.length === 0 ? ( +
No users found
+ ) : ( +
- Username - - Email - - Full Name - - Status - - Role - - Created - - Last Login - - Actions -
- {user.username} - - {user.email || '-'} - - {user.full_name || '-'} - - - {user.is_active ? 'Active' : 'Inactive'} - - - - {user.is_admin ? 'Admin' : 'User'} - - - {formatDate(user.created_date)} - - {formatDate(user.last_login)} - - - -
+ + + + + + + + + + + + + + {users.map((user) => ( + + + + + + + + + + + ))} + +
+ Username + + Email + + Full Name + + Status + + Role + + Created + + Last Login + + Actions +
+ {user.username} + + {user.email || '-'} + + {user.full_name || '-'} + + + {user.is_active ? 'Active' : 'Inactive'} + + + + {user.is_admin ? 'Admin' : 'User'} + + + {formatDate(user.created_date)} + + {formatDate(user.last_login)} + + + +
+ )} +
+ + )} + + {/* Frontend Users Tab */} + {activeTab === 'frontend' && ( +
+ {authLoading ? ( +
Loading users...
+ ) : authUsers.length === 0 ? ( +
No users found
+ ) : ( + + + + + + + + + - ))} - -
+ Email + + Name + + Role + + Write Access + + Created + + Actions +
- )} -
+ + + {authUsers.map((user) => ( + + + {user.email || '-'} + + + {user.name || '-'} + + + + {user.is_admin === true ? 'Admin' : 'User'} + + + + + {user.has_write_access === true ? 'Yes' : 'No'} + + + + {formatDate(user.created_at)} + + + + + + + ))} + + + )} +
+ )} - {/* Create Modal */} + {/* Backend Create Modal */} {showCreateModal && (
@@ -306,7 +551,7 @@ export default function ManageUsers() {
@@ -342,17 +590,22 @@ export default function ManageUsers() { /> Active -
+
+ +
@@ -373,7 +626,7 @@ export default function ManageUsers() {
)} - {/* Edit Modal */} + {/* Backend Edit Modal */} {editingUser && (
@@ -401,7 +654,7 @@ export default function ManageUsers() {
@@ -437,17 +693,22 @@ export default function ManageUsers() { /> Active -
+
+ +
@@ -467,7 +728,186 @@ export default function ManageUsers() {
)} + + {/* Frontend Create Modal */} + {showAuthCreateModal && ( +
+
+

Create New Front End User

+
+
+ + + setAuthCreateForm({ ...authCreateForm, email: e.target.value }) + } + className="w-full px-3 py-2 border border-gray-300 rounded-md" + required + /> +
+
+ + + setAuthCreateForm({ ...authCreateForm, name: e.target.value }) + } + className="w-full px-3 py-2 border border-gray-300 rounded-md" + required + minLength={1} + /> +
+
+ + + setAuthCreateForm({ ...authCreateForm, password: e.target.value }) + } + className="w-full px-3 py-2 border border-gray-300 rounded-md" + required + minLength={6} + /> +
+
+ + +
+
+ + +
+
+
+ + +
+
+
+ )} + + {/* Frontend Edit Modal */} + {editingAuthUser && ( +
+
+

+ Edit Front End User: {editingAuthUser.name || editingAuthUser.email} +

+
+
+ + + setAuthEditForm({ ...authEditForm, email: e.target.value }) + } + className="w-full px-3 py-2 border border-gray-300 rounded-md" + required + /> +
+
+ + + setAuthEditForm({ ...authEditForm, name: e.target.value }) + } + className="w-full px-3 py-2 border border-gray-300 rounded-md" + required + /> +
+
+ + +
+
+
+ + +
+
+
+ )} ) } - diff --git a/scripts/grant_auth_db_delete_permission.sh b/scripts/grant_auth_db_delete_permission.sh new file mode 100755 index 0000000..26a0249 --- /dev/null +++ b/scripts/grant_auth_db_delete_permission.sh @@ -0,0 +1,64 @@ +#!/bin/bash +# Grant DELETE permission on auth database users table + +set -e + +echo "šŸ”§ Granting DELETE permission on auth database users table..." + +# Check if .env file exists +ENV_FILE=".env" +if [ ! -f "$ENV_FILE" ]; then + echo "āŒ Error: .env file not found" + exit 1 +fi + +# Extract DATABASE_URL_AUTH from .env file +AUTH_DB_URL=$(grep "^DATABASE_URL_AUTH=" "$ENV_FILE" | cut -d '=' -f2- | tr -d '"' | tr -d "'") + +if [ -z "$AUTH_DB_URL" ]; then + echo "āŒ Error: DATABASE_URL_AUTH not found in .env file" + exit 1 +fi + +# Parse the database URL +# Format: postgresql+psycopg2://user:password@host:port/database +# or: postgresql://user:password@host:port/database + +# Remove postgresql+psycopg2:// prefix if present +AUTH_DB_URL=${AUTH_DB_URL#postgresql+psycopg2://} +AUTH_DB_URL=${AUTH_DB_URL#postgresql://} + +# Extract components +# Split by @ to get user:pass and host:port/db +IFS='@' read -r CREDENTIALS REST <<< "$AUTH_DB_URL" +IFS=':' read -r DB_USER DB_PASS <<< "$CREDENTIALS" + +# Extract host, port, and database +IFS='/' read -r HOST_PORT DB_NAME <<< "$REST" +IFS=':' read -r DB_HOST DB_PORT <<< "$HOST_PORT" + +# Set defaults +DB_PORT=${DB_PORT:-5432} +DB_HOST=${DB_HOST:-localhost} + +echo "šŸ“‹ Database information:" +echo " Host: $DB_HOST" +echo " Port: $DB_PORT" +echo " Database: $DB_NAME" +echo " User: $DB_USER" +echo "" + +# Grant DELETE permission using psql as postgres superuser +echo "šŸ” Granting DELETE permission (requires sudo)..." +sudo -u postgres psql -d "$DB_NAME" << EOF +GRANT DELETE ON TABLE users TO "$DB_USER"; +\q +EOF + +if [ $? -eq 0 ]; then + echo "āœ… Successfully granted DELETE permission to user '$DB_USER'" +else + echo "āŒ Failed to grant permission" + exit 1 +fi + diff --git a/scripts/grant_auth_db_permissions.py b/scripts/grant_auth_db_permissions.py new file mode 100755 index 0000000..6a54ca3 --- /dev/null +++ b/scripts/grant_auth_db_permissions.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +"""Grant DELETE permission on auth database users table. + +This script grants DELETE permission to the database user specified in DATABASE_URL_AUTH. +It requires superuser access (postgres user) to grant permissions. +""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path +from urllib.parse import urlparse + +from dotenv import load_dotenv +from sqlalchemy import create_engine, text + +# Load environment variables +env_path = Path(__file__).parent.parent.parent / ".env" +load_dotenv(dotenv_path=env_path) + + +def parse_database_url(db_url: str) -> dict: + """Parse database URL into components.""" + # Handle postgresql+psycopg2:// format + if db_url.startswith("postgresql+psycopg2://"): + db_url = db_url.replace("postgresql+psycopg2://", "postgresql://") + + parsed = urlparse(db_url) + return { + "user": parsed.username, + "password": parsed.password, + "host": parsed.hostname or "localhost", + "port": parsed.port or 5432, + "database": parsed.path.lstrip("/"), + } + + +def grant_delete_permission() -> None: + """Grant DELETE permission on users table in auth database.""" + auth_db_url = os.getenv("DATABASE_URL_AUTH") + if not auth_db_url: + print("āŒ Error: DATABASE_URL_AUTH environment variable not set") + sys.exit(1) + + if not auth_db_url.startswith("postgresql"): + print("ā„¹ļø Auth database is not PostgreSQL. No permissions to grant.") + return + + db_info = parse_database_url(auth_db_url) + db_user = db_info["user"] + db_name = db_info["database"] + + print(f"šŸ“‹ Granting DELETE permission on users table...") + print(f" Database: {db_name}") + print(f" User: {db_user}") + + # Connect as postgres superuser to grant permissions + # Try to connect as postgres user (superuser) + try: + # Try to get postgres password from environment or use peer authentication + postgres_url = f"postgresql://postgres@{db_info['host']}:{db_info['port']}/{db_name}" + engine = create_engine(postgres_url) + + with engine.connect() as conn: + # Grant DELETE permission + conn.execute(text(f""" + GRANT DELETE ON TABLE users TO {db_user} + """)) + conn.commit() + + print(f"āœ… Successfully granted DELETE permission to user '{db_user}'") + return + except Exception as e: + # If connecting as postgres fails, try with the same user (might have grant privileges) + print(f"āš ļø Could not connect as postgres user: {e}") + print(f" Trying with current database user...") + + try: + engine = create_engine(auth_db_url) + with engine.connect() as conn: + # Try to grant permission + conn.execute(text(f""" + GRANT DELETE ON TABLE users TO {db_user} + """)) + conn.commit() + + print(f"āœ… Successfully granted DELETE permission to user '{db_user}'") + return + except Exception as e2: + print(f"āŒ Failed to grant permission: {e2}") + print(f"\nšŸ’” To grant permission manually, run as postgres superuser:") + print(f" sudo -u postgres psql -d {db_name} -c \"GRANT DELETE ON TABLE users TO {db_user};\"") + sys.exit(1) + + +if __name__ == "__main__": + grant_delete_permission() + diff --git a/scripts/recreate_tables_web.py b/scripts/recreate_tables_web.py index ee12172..73ff325 100644 --- a/scripts/recreate_tables_web.py +++ b/scripts/recreate_tables_web.py @@ -21,21 +21,6 @@ def recreate_tables(): Base.metadata.create_all(bind=engine) print("āœ… All tables created successfully!") - - # Stamp Alembic to latest migration - print("\nMarking database as up-to-date with migrations...") - from alembic.config import Config - from alembic import command - from alembic.script import ScriptDirectory - - alembic_cfg = Config("alembic.ini") - script = ScriptDirectory.from_config(alembic_cfg) - - # Get the latest revision - head = script.get_current_head() - print(f"Stamping database to revision: {head}") - command.stamp(alembic_cfg, head) - print("āœ… Database is now fresh and ready to use!") diff --git a/scripts/setup_postgresql.sh b/scripts/setup_postgresql.sh index 2cdbab6..53a61ee 100755 --- a/scripts/setup_postgresql.sh +++ b/scripts/setup_postgresql.sh @@ -59,3 +59,4 @@ echo "3. Run your application - it will connect to PostgreSQL automatically" + diff --git a/src/web/api/auth_users.py b/src/web/api/auth_users.py new file mode 100644 index 0000000..4f656b1 --- /dev/null +++ b/src/web/api/auth_users.py @@ -0,0 +1,368 @@ +"""Auth database user management endpoints - admin only.""" + +from __future__ import annotations + +from typing import Annotated + +from fastapi import APIRouter, Depends, HTTPException, Query, Response, status +from sqlalchemy import text +from sqlalchemy.orm import Session + +from src.web.api.auth import get_current_user +from src.web.api.users import get_current_admin_user +from src.web.db.session import get_auth_db, get_db +from src.web.schemas.auth_users import ( + AuthUserCreateRequest, + AuthUserResponse, + AuthUserUpdateRequest, + AuthUsersListResponse, +) +from src.web.utils.password import hash_password + +router = APIRouter(prefix="/auth-users", tags=["auth-users"]) + + +@router.get("", response_model=AuthUsersListResponse) +def list_auth_users( + current_admin: Annotated[dict, Depends(get_current_admin_user)], + auth_db: Session = Depends(get_auth_db), +) -> AuthUsersListResponse: + """List all users from auth database - admin only.""" + try: + # Query users from auth database with all columns from schema + result = auth_db.execute(text(""" + SELECT + id, + email, + name, + is_admin, + has_write_access, + created_at, + updated_at + FROM users + ORDER BY COALESCE(name, email) ASC + """)) + + rows = result.fetchall() + users = [] + for row in rows: + # Access row attributes directly - SQLAlchemy Row objects support attribute access + user_id = int(row.id) + email = str(row.email) + name = row.name if row.name is not None else None + + # Get boolean fields - convert to proper boolean + # These columns have defaults so they should always have values + is_admin = bool(row.is_admin) + has_write_access = bool(row.has_write_access) + + created_at = row.created_at + updated_at = row.updated_at + + users.append(AuthUserResponse( + id=user_id, + name=name, + email=email, + is_admin=is_admin, + has_write_access=has_write_access, + created_at=created_at, + updated_at=updated_at, + )) + + return AuthUsersListResponse(items=users, total=len(users)) + except Exception as e: + import traceback + error_detail = f"Failed to list auth users: {str(e)}\n{traceback.format_exc()}" + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=error_detail, + ) + + +@router.post("", response_model=AuthUserResponse, status_code=status.HTTP_201_CREATED) +def create_auth_user( + current_admin: Annotated[dict, Depends(get_current_admin_user)], + request: AuthUserCreateRequest, + auth_db: Session = Depends(get_auth_db), +) -> AuthUserResponse: + """Create a new user in auth database - admin only.""" + try: + # Check if user with same email already exists (email is unique) + check_result = auth_db.execute(text(""" + SELECT id FROM users + WHERE email = :email + """), {"email": request.email}) + + existing = check_result.first() + if existing: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"User with email '{request.email}' already exists", + ) + + # Insert new user + # Check database dialect for RETURNING support + dialect = auth_db.bind.dialect.name if auth_db.bind else 'postgresql' + supports_returning = dialect == 'postgresql' + + # Hash the password + password_hash = hash_password(request.password) + + if supports_returning: + result = auth_db.execute(text(""" + INSERT INTO users (email, name, password_hash, is_admin, has_write_access) + VALUES (:email, :name, :password_hash, :is_admin, :has_write_access) + RETURNING id, email, name, is_admin, has_write_access, created_at, updated_at + """), { + "email": request.email, + "name": request.name, + "password_hash": password_hash, + "is_admin": request.is_admin, + "has_write_access": request.has_write_access, + }) + auth_db.commit() + row = result.first() + else: + # SQLite - insert then select + auth_db.execute(text(""" + INSERT INTO users (email, name, password_hash, is_admin, has_write_access) + VALUES (:email, :name, :password_hash, :is_admin, :has_write_access) + """), { + "email": request.email, + "name": request.name, + "password_hash": password_hash, + "is_admin": request.is_admin, + "has_write_access": request.has_write_access, + }) + auth_db.commit() + # Get the last inserted row + result = auth_db.execute(text(""" + SELECT id, email, name, is_admin, has_write_access, created_at, updated_at + FROM users + WHERE id = last_insert_rowid() + """)) + row = result.first() + + if not row: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to create user", + ) + + is_admin = bool(row.is_admin) + has_write_access = bool(row.has_write_access) + + return AuthUserResponse( + id=row.id, + name=getattr(row, 'name', None), + email=row.email, + is_admin=is_admin, + has_write_access=has_write_access, + created_at=getattr(row, 'created_at', None), + updated_at=getattr(row, 'updated_at', None), + ) + except HTTPException: + raise + except Exception as e: + auth_db.rollback() + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to create auth user: {str(e)}", + ) + + +@router.get("/{user_id}", response_model=AuthUserResponse) +def get_auth_user( + current_admin: Annotated[dict, Depends(get_current_admin_user)], + user_id: int, + auth_db: Session = Depends(get_auth_db), +) -> AuthUserResponse: + """Get a specific auth user by ID - admin only.""" + try: + result = auth_db.execute(text(""" + SELECT id, email, name, is_admin, has_write_access, created_at, updated_at + FROM users + WHERE id = :user_id + """), {"user_id": user_id}) + + row = result.first() + if not row: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Auth user with ID {user_id} not found", + ) + + is_admin = bool(row.is_admin) + has_write_access = bool(row.has_write_access) + + return AuthUserResponse( + id=row.id, + name=getattr(row, 'name', None), + email=row.email, + is_admin=is_admin, + has_write_access=has_write_access, + created_at=getattr(row, 'created_at', None), + updated_at=getattr(row, 'updated_at', None), + ) + except HTTPException: + raise + except Exception as e: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to get auth user: {str(e)}", + ) + + +@router.put("/{user_id}", response_model=AuthUserResponse) +def update_auth_user( + current_admin: Annotated[dict, Depends(get_current_admin_user)], + user_id: int, + request: AuthUserUpdateRequest, + auth_db: Session = Depends(get_auth_db), +) -> AuthUserResponse: + """Update an auth user - admin only.""" + try: + # Check if user exists + check_result = auth_db.execute(text(""" + SELECT id FROM users WHERE id = :user_id + """), {"user_id": user_id}) + + if not check_result.first(): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Auth user with ID {user_id} not found", + ) + + # Check if email conflicts with another user (email is unique) + check_conflict = auth_db.execute(text(""" + SELECT id FROM users + WHERE id != :user_id AND email = :email + """), { + "user_id": user_id, + "email": request.email, + }) + + if check_conflict.first(): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"User with email '{request.email}' already exists", + ) + + # Update all fields (all are required) + dialect = auth_db.bind.dialect.name if auth_db.bind else 'postgresql' + supports_returning = dialect == 'postgresql' + + if supports_returning: + result = auth_db.execute(text(""" + UPDATE users + SET email = :email, + name = :name, + is_admin = :is_admin, + has_write_access = :has_write_access + WHERE id = :user_id + RETURNING id, email, name, is_admin, has_write_access, created_at, updated_at + """), { + "user_id": user_id, + "email": request.email, + "name": request.name, + "is_admin": request.is_admin, + "has_write_access": request.has_write_access, + }) + auth_db.commit() + row = result.first() + else: + # SQLite - update then select + auth_db.execute(text(""" + UPDATE users + SET email = :email, + name = :name, + is_admin = :is_admin, + has_write_access = :has_write_access + WHERE id = :user_id + """), { + "user_id": user_id, + "email": request.email, + "name": request.name, + "is_admin": request.is_admin, + "has_write_access": request.has_write_access, + }) + auth_db.commit() + # Get the updated row + result = auth_db.execute(text(""" + SELECT id, email, name, is_admin, has_write_access, created_at, updated_at + FROM users + WHERE id = :user_id + """), {"user_id": user_id}) + row = result.first() + + if not row: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to update user", + ) + + is_admin = bool(row.is_admin) + has_write_access = bool(row.has_write_access) + + return AuthUserResponse( + id=row.id, + name=getattr(row, 'name', None), + email=row.email, + is_admin=is_admin, + has_write_access=has_write_access, + created_at=getattr(row, 'created_at', None), + updated_at=getattr(row, 'updated_at', None), + ) + except HTTPException: + raise + except Exception as e: + auth_db.rollback() + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to update auth user: {str(e)}", + ) + + +@router.delete("/{user_id}") +def delete_auth_user( + current_admin: Annotated[dict, Depends(get_current_admin_user)], + user_id: int, + auth_db: Session = Depends(get_auth_db), +) -> Response: + """Delete an auth user - admin only.""" + try: + # Check if user exists + check_result = auth_db.execute(text(""" + SELECT id FROM users WHERE id = :user_id + """), {"user_id": user_id}) + + if not check_result.first(): + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Auth user with ID {user_id} not found", + ) + + # Delete user + auth_db.execute(text(""" + DELETE FROM users WHERE id = :user_id + """), {"user_id": user_id}) + + auth_db.commit() + + return Response(status_code=status.HTTP_204_NO_CONTENT) + except HTTPException: + raise + except Exception as e: + auth_db.rollback() + error_str = str(e) + # Check for permission errors + if "permission denied" in error_str.lower() or "insufficient privilege" in error_str.lower(): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Permission denied: The database user does not have DELETE permission on the users table. Please contact your database administrator.", + ) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to delete auth user: {error_str}", + ) + diff --git a/src/web/api/users.py b/src/web/api/users.py index 6d72e65..dc0d117 100644 --- a/src/web/api/users.py +++ b/src/web/api/users.py @@ -113,6 +113,14 @@ def create_user( detail=f"Username '{request.username}' already exists", ) + # Check if email already exists + existing_email = db.query(User).filter(User.email == request.email).first() + if existing_email: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Email address '{request.email}' is already in use", + ) + # Hash the password before storing password_hash = hash_password(request.password) @@ -174,6 +182,15 @@ def update_user( detail="Cannot remove your own admin status", ) + # Check if email is being changed and if the new email already exists + if request.email is not None and request.email != user.email: + existing_email = db.query(User).filter(User.email == request.email).first() + if existing_email: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Email address '{request.email}' is already in use", + ) + # Update fields if provided if request.password is not None: user.password_hash = hash_password(request.password) diff --git a/src/web/app.py b/src/web/app.py index bb92f33..a01896d 100644 --- a/src/web/app.py +++ b/src/web/app.py @@ -22,6 +22,7 @@ from src.web.api.reported_photos import router as reported_photos_router from src.web.api.pending_photos import router as pending_photos_router from src.web.api.tags import router as tags_router from src.web.api.users import router as users_router +from src.web.api.auth_users import router as auth_users_router from src.web.api.version import router as version_router from src.web.settings import APP_TITLE, APP_VERSION from src.web.db.base import Base, engine @@ -172,6 +173,54 @@ def ensure_user_password_change_required_column(inspector) -> None: print("āœ… Added password_change_required column to users table") +def ensure_user_email_unique_constraint(inspector) -> None: + """Ensure users table email column has a unique constraint.""" + if "users" not in inspector.get_table_names(): + return + + # Check if email column exists + columns = {col["name"] for col in inspector.get_columns("users")} + if "email" not in columns: + print("ā„¹ļø email column does not exist in users table yet") + return + + # Check if unique constraint already exists on email + dialect = engine.dialect.name + with engine.connect() as connection: + if dialect == "postgresql": + # Check if unique constraint exists + result = connection.execute(text(""" + SELECT constraint_name + FROM information_schema.table_constraints + WHERE table_name = 'users' + AND constraint_type = 'UNIQUE' + AND constraint_name LIKE '%email%' + """)) + if result.first(): + print("ā„¹ļø Unique constraint on email column already exists") + return + + # Try to add unique constraint (will fail if duplicates exist) + try: + print("šŸ”„ Adding unique constraint to email column...") + connection.execute(text("ALTER TABLE users ADD CONSTRAINT uq_users_email UNIQUE (email)")) + connection.commit() + print("āœ… Added unique constraint to email column") + except Exception as e: + # If constraint already exists or duplicates exist, that's okay + # API validation will prevent new duplicates + if "already exists" in str(e).lower() or "duplicate" in str(e).lower(): + print(f"ā„¹ļø Could not add unique constraint (may have duplicates): {e}") + else: + print(f"āš ļø Could not add unique constraint: {e}") + else: + # SQLite - unique constraint is handled at column level + # Check if column already has unique constraint + # SQLite doesn't easily support adding unique constraints to existing columns + # The model definition will handle it for new tables + print("ā„¹ļø SQLite: Unique constraint on email will be enforced by model definition for new tables") + + @asynccontextmanager async def lifespan(app: FastAPI): """Lifespan context manager for startup and shutdown events.""" @@ -205,6 +254,7 @@ async def lifespan(app: FastAPI): # Ensure new columns exist (backward compatibility without migrations) ensure_user_password_hash_column(inspector) ensure_user_password_change_required_column(inspector) + ensure_user_email_unique_constraint(inspector) except Exception as exc: print(f"āŒ Database initialization failed: {exc}") raise @@ -243,7 +293,8 @@ def create_app() -> FastAPI: app.include_router(reported_photos_router, prefix="/api/v1") app.include_router(pending_photos_router, prefix="/api/v1") app.include_router(tags_router, prefix="/api/v1") - app.include_router(users_router, prefix="/api/v1") + app.include_router(users_router, prefix="/api/v1") + app.include_router(auth_users_router, prefix="/api/v1") return app diff --git a/src/web/db/models.py b/src/web/db/models.py index d1e5018..ead827d 100644 --- a/src/web/db/models.py +++ b/src/web/db/models.py @@ -204,8 +204,8 @@ class User(Base): id = Column(Integer, primary_key=True, autoincrement=True, index=True) username = Column(Text, unique=True, nullable=False, index=True) password_hash = Column(Text, nullable=False) # Hashed password - email = Column(Text, nullable=True) - full_name = Column(Text, nullable=True) + email = Column(Text, unique=True, nullable=False, index=True) + full_name = Column(Text, nullable=False) is_active = Column(Boolean, default=True, nullable=False) is_admin = Column(Boolean, default=False, nullable=False, index=True) password_change_required = Column(Boolean, default=True, nullable=False, index=True) @@ -214,6 +214,7 @@ class User(Base): __table_args__ = ( Index("idx_users_username", "username"), + Index("idx_users_email", "email"), Index("idx_users_is_admin", "is_admin"), Index("idx_users_password_change_required", "password_change_required"), ) diff --git a/src/web/schemas/auth_users.py b/src/web/schemas/auth_users.py new file mode 100644 index 0000000..c7b0f36 --- /dev/null +++ b/src/web/schemas/auth_users.py @@ -0,0 +1,56 @@ +"""Auth database user management schemas for web API.""" + +from __future__ import annotations + +from datetime import datetime +from typing import Optional + +from pydantic import BaseModel, ConfigDict, EmailStr, Field + + +class AuthUserResponse(BaseModel): + """Auth user DTO returned from API.""" + + model_config = ConfigDict(from_attributes=True, protected_namespaces=()) + + id: int + name: Optional[str] = None + email: str + is_admin: Optional[bool] = None + has_write_access: Optional[bool] = None + created_at: Optional[datetime] = None + updated_at: Optional[datetime] = None + + +class AuthUserCreateRequest(BaseModel): + """Request payload to create a new auth user.""" + + model_config = ConfigDict(protected_namespaces=()) + + email: EmailStr = Field(..., description="Email address (unique, required)") + name: str = Field(..., min_length=1, max_length=200, description="Name (required)") + password: str = Field(..., min_length=6, description="Password (minimum 6 characters, required)") + is_admin: bool = Field(..., description="Admin role (required)") + has_write_access: bool = Field(..., description="Write access (required)") + + +class AuthUserUpdateRequest(BaseModel): + """Request payload to update an auth user.""" + + model_config = ConfigDict(protected_namespaces=()) + + email: EmailStr = Field(..., description="Email address (required)") + name: str = Field(..., min_length=1, max_length=200, description="Name (required)") + is_admin: bool = Field(..., description="Admin role (required)") + has_write_access: bool = Field(..., description="Write access (required)") + + +class AuthUsersListResponse(BaseModel): + """List of auth users.""" + + model_config = ConfigDict(protected_namespaces=()) + + items: list[AuthUserResponse] + total: int + + diff --git a/src/web/schemas/users.py b/src/web/schemas/users.py index 3df7d60..21ab89a 100644 --- a/src/web/schemas/users.py +++ b/src/web/schemas/users.py @@ -31,8 +31,8 @@ class UserCreateRequest(BaseModel): username: str = Field(..., min_length=1, max_length=100) password: str = Field(..., min_length=6, description="Password (minimum 6 characters)") - email: Optional[EmailStr] = None - full_name: Optional[str] = Field(None, max_length=200) + email: EmailStr = Field(..., description="Email address (required)") + full_name: str = Field(..., min_length=1, max_length=200, description="Full name (required)") is_active: bool = True is_admin: bool = False @@ -43,8 +43,8 @@ class UserUpdateRequest(BaseModel): model_config = ConfigDict(protected_namespaces=()) password: Optional[str] = Field(None, min_length=6, description="New password (minimum 6 characters, leave empty to keep current)") - email: Optional[EmailStr] = None - full_name: Optional[str] = Field(None, max_length=200) + email: EmailStr = Field(..., description="Email address (required)") + full_name: str = Field(..., min_length=1, max_length=200, description="Full name (required)") is_active: Optional[bool] = None is_admin: Optional[bool] = None