Skip to content

68 validation for uploaded user profile images #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
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
8 changes: 5 additions & 3 deletions docs/installation.qmd
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ If you use VSCode with Docker to develop in a container, the following VSCode De
{
"name": "Python 3",
"image": "mcr.microsoft.com/devcontainers/python:1-3.12-bookworm",
"postCreateCommand": "sudo apt update && sudo apt install -y python3-dev libpq-dev graphviz && uv venv && uv sync",
"postCreateCommand": "sudo apt update && sudo apt install -y python3-dev libpq-dev graphviz libwebp-dev && uv venv && uv sync",
"features": {
"ghcr.io/va-h/devcontainers-features/uv:1": {
"version": "latest"
Expand Down Expand Up @@ -61,7 +61,7 @@ Install Docker Desktop and Docker Compose for your operating system by following
For Ubuntu/Debian:

``` bash
sudo apt update && sudo apt install -y python3-dev libpq-dev
sudo apt update && sudo apt install -y python3-dev libpq-dev libwebp-dev
```

For macOS:
Expand Down Expand Up @@ -165,7 +165,9 @@ Before running the development server, make sure the development database is run
uvicorn main:app --host 0.0.0.0 --port 8000 --reload
```

Navigate to http://localhost:8000/
Navigate to http://localhost:8000/.

(Note: If startup fails with a sqlalchemy/psycopg2 connection error, make sure that Docker Desktop and the database service are running and that the environment variables in the `.env` file are correctly populated, and then try again.)

## Lint types with mypy

Expand Down
2 changes: 1 addition & 1 deletion index.qmd
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Install Docker Desktop and Coker Compose for your operating system by following
For Ubuntu/Debian:

``` bash
sudo apt update && sudo apt install -y python3-dev libpq-dev
sudo apt update && sudo apt install -y python3-dev libpq-dev libwebp-dev
```

For macOS:
Expand Down
8 changes: 8 additions & 0 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from utils.auth import get_user_with_relations, get_optional_user, NeedsNewTokens, get_user_from_reset_token, PasswordValidationError, AuthenticationError
from utils.models import User, Organization
from utils.db import get_session, set_up_db
from utils.images import MAX_FILE_SIZE, MIN_DIMENSION, MAX_DIMENSION, ALLOWED_CONTENT_TYPES

logger = logging.getLogger("uvicorn.error")
logger.setLevel(logging.DEBUG)
Expand Down Expand Up @@ -246,6 +247,13 @@ async def read_dashboard(
async def read_profile(
params: dict = Depends(common_authenticated_parameters)
):
# Add image constraints to the template context
params.update({
"max_file_size_mb": MAX_FILE_SIZE / (1024 * 1024), # Convert bytes to MB
"min_dimension": MIN_DIMENSION,
"max_dimension": MAX_DIMENSION,
"allowed_formats": list(ALLOWED_CONTENT_TYPES.keys())
})
return templates.TemplateResponse(params["request"], "users/profile.html", params)


Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ dependencies = [
"resend<3.0.0,>=2.4.0",
"bcrypt<5.0.0,>=4.2.0",
"fastapi<1.0.0,>=0.115.5",
"pillow>=11.0.0",
]

[dependency-groups]
Expand Down
13 changes: 11 additions & 2 deletions routers/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from typing import Optional
from utils.models import User, DataIntegrityError
from utils.auth import get_session, get_authenticated_user, verify_password, PasswordValidationError
from utils.images import validate_and_process_image

router = APIRouter(prefix="/user", tags=["user"])

Expand Down Expand Up @@ -61,11 +62,19 @@ async def update_profile(
user: User = Depends(get_authenticated_user),
session: Session = Depends(get_session)
):
# Handle avatar update
if user_profile.avatar_file:
processed_image, content_type = validate_and_process_image(
user_profile.avatar_file,
user_profile.avatar_content_type
)
user_profile.avatar_file = processed_image
user_profile.avatar_content_type = content_type

# Update user details
user.name = user_profile.name
user.email = user_profile.email

# Handle avatar update

if user_profile.avatar_file:
user.avatar_data = user_profile.avatar_file
user.avatar_content_type = user_profile.avatar_content_type
Expand Down
2 changes: 1 addition & 1 deletion templates/authentication/register.html
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<!-- Password Input -->
<div class="mb-3">
<label for="password" class="form-label">Password</label>
<!-- Make sure 9g,X*88w[6"W passes validation -->
<!-- Make sure 9g,X*88w[6"W and ^94cPSf2^)z2^,& pass validation -->
<input type="password" class="form-control" id="password" name="password"
pattern="(?=.*\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[@$!%*?&\{\}\<\>\.\,\\\\'#\-_=\+\(\)\[\]:;\|~\/])[A-Za-z\d@$!%*?&\{\}\<\>\.\,\\\\'#\-_=\+\(\)\[\]:;\|~\/]{8,}"
title="Must contain at least one number, one uppercase and lowercase letter, one special character, and at least 8 or more characters"
Expand Down
57 changes: 57 additions & 0 deletions templates/users/profile.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ <h1 class="mb-4">User Profile</h1>
<div class="mb-3">
<label for="avatar_file" class="form-label">Avatar</label>
<input type="file" class="form-control" id="avatar_file" name="avatar_file" accept="image/*">
<div class="form-text">
<ul class="mb-0">
<li>Maximum file size: {{ max_file_size_mb }} MB</li>
<li>Minimum dimension: {{ min_dimension }}x{{ min_dimension }} pixels</li>
<li>Maximum dimension: {{ max_dimension }}x{{ max_dimension }} pixels</li>
<li>Allowed formats: {{ allowed_formats|join(', ') }}</li>
<li>Image will be cropped to a square</li>
</ul>
</div>
</div>
<button type="submit" class="btn btn-primary">Save Changes</button>
</form>
Expand Down Expand Up @@ -103,5 +112,53 @@ <h1 class="mb-4">User Profile</h1>
editProfile.style.display = 'block';
}
}

document.getElementById('avatar_file').addEventListener('change', function(e) {
const file = e.target.files[0];
if (!file) return;

// Get constraints from your template variables
const maxSizeMB = "{{ max_file_size_mb }}";
const minDimension = "{{ min_dimension }}";
const maxDimension = "{{ max_dimension }}";
const allowedFormats = "{{ allowed_formats }}";


// Check file size
const fileSizeMB = file.size / (1024 * 1024);
if (fileSizeMB > maxSizeMB) {
alert(`File size must be less than ${maxSizeMB}MB`);
this.value = '';
return;
}

// Check file format
const fileFormat = file.type.split('/')[1];
if (!allowedFormats.includes(fileFormat)) {
alert(`File format must be one of: ${allowedFormats.join(', ')}`);
this.value = '';
return;
}

// Check dimensions
const img = new Image();
img.src = URL.createObjectURL(file);

img.onload = function() {
URL.revokeObjectURL(this.src);

if (this.width < minDimension || this.height < minDimension) {
alert(`Image dimensions must be at least ${minDimension}x${minDimension} pixels`);
e.target.value = '';
return;
}

if (this.width > maxDimension || this.height > maxDimension) {
alert(`Image dimensions must not exceed ${maxDimension}x${maxDimension} pixels`);
e.target.value = '';
return;
}
};
});
</script>
{% endblock %}
103 changes: 103 additions & 0 deletions tests/test_images.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import pytest
from PIL import Image
import io
from utils.images import (
validate_and_process_image,
InvalidImageError,
MAX_FILE_SIZE,
MIN_DIMENSION,
MAX_DIMENSION
)

def create_test_image(width: int, height: int, format: str = 'PNG') -> bytes:
"""Helper function to create test images"""
image = Image.new('RGB', (width, height), color='red')
output = io.BytesIO()
image.save(output, format=format)
return output.getvalue()

def test_webp_dependencies_are_installed():
"""Test that webp dependencies are installed"""
assert '.webp' in Image.registered_extensions(), "WebP dependencies are not installed (e.g., libwebp-dev on Linux)"

def test_valid_square_image():
"""Test processing a valid square image"""
image_data = create_test_image(500, 500)
processed_data, content_type = validate_and_process_image(image_data, 'image/png')

# Verify the processed image
processed_image = Image.open(io.BytesIO(processed_data))
assert processed_image.size == (500, 500)
assert content_type == 'image/png'

def test_valid_rectangular_image():
"""Test processing a valid rectangular image"""
image_data = create_test_image(800, 600)
processed_data, content_type = validate_and_process_image(image_data, 'image/png')

# Verify the processed image
processed_image = Image.open(io.BytesIO(processed_data))
assert processed_image.size == (600, 600)
assert content_type == 'image/png'

def test_minimum_size_image():
"""Test processing an image with minimum allowed dimensions"""
image_data = create_test_image(MIN_DIMENSION, MIN_DIMENSION)
processed_data, content_type = validate_and_process_image(image_data, 'image/png')

processed_image = Image.open(io.BytesIO(processed_data))
assert processed_image.size == (100, 100)

def test_too_small_image():
"""Test that too small images are rejected"""
image_data = create_test_image(MIN_DIMENSION - 1, MIN_DIMENSION - 1)
with pytest.raises(InvalidImageError) as exc_info:
validate_and_process_image(image_data, 'image/png')
assert "Image too small" in str(exc_info.value.detail)

def test_too_large_image():
"""Test that too large images are rejected"""
image_data = create_test_image(MAX_DIMENSION + 1, MAX_DIMENSION + 1)
with pytest.raises(InvalidImageError) as exc_info:
validate_and_process_image(image_data, 'image/png')
assert "Image too large" in str(exc_info.value.detail)

def test_invalid_file_type():
"""Test that invalid file types are rejected"""
image_data = create_test_image(500, 500)
with pytest.raises(InvalidImageError) as exc_info:
validate_and_process_image(image_data, 'image/gif')
assert "Invalid file type" in str(exc_info.value.detail)

def test_file_too_large():
"""Test that files exceeding MAX_FILE_SIZE are rejected"""
# Create a large file that exceeds MAX_FILE_SIZE
large_image_data = b'0' * (MAX_FILE_SIZE + 1)
with pytest.raises(InvalidImageError) as exc_info:
validate_and_process_image(large_image_data, 'image/png')
assert "File too large" in str(exc_info.value.detail)

def test_corrupt_image_data():
"""Test that corrupt image data is rejected"""
corrupt_data = b'not an image'
with pytest.raises(InvalidImageError) as exc_info:
validate_and_process_image(corrupt_data, 'image/png')
assert "Invalid image file" in str(exc_info.value.detail)

def test_different_image_formats():
"""Test processing different valid image formats"""
formats = [
('JPEG', 'image/jpeg'),
('PNG', 'image/png'),
('WEBP', 'image/webp')
]

for format_name, content_type in formats:
image_data = create_test_image(500, 500, format_name)
processed_data, result_type = validate_and_process_image(image_data, content_type)

# Verify the processed image
processed_image = Image.open(io.BytesIO(processed_data))
assert processed_image.size == (500, 500)
# Output should match input format
assert result_type == content_type
56 changes: 45 additions & 11 deletions tests/test_user.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
from fastapi.testclient import TestClient
from httpx import Response
from sqlmodel import Session
from unittest.mock import patch

from main import app
from utils.models import User
from utils.images import InvalidImageError

# Mock data for consistent testing
MOCK_IMAGE_DATA = b"processed fake image data"
MOCK_CONTENT_TYPE = "image/png"


def test_update_profile_unauthorized(unauth_client: TestClient):
Expand All @@ -23,11 +29,12 @@ def test_update_profile_unauthorized(unauth_client: TestClient):
assert response.headers["location"] == app.url_path_for("read_login")


def test_update_profile_authorized(auth_client: TestClient, test_user: User, session: Session):
@patch('routers.user.validate_and_process_image')
def test_update_profile_authorized(mock_validate, auth_client: TestClient, test_user: User, session: Session):
"""Test that authorized users can edit their profile"""

# Create test image data
test_image_data = b"fake image data"
# Configure mock to return processed image data
mock_validate.return_value = (MOCK_IMAGE_DATA, MOCK_CONTENT_TYPE)

# Update profile
response: Response = auth_client.post(
Expand All @@ -37,7 +44,7 @@ def test_update_profile_authorized(auth_client: TestClient, test_user: User, ses
"email": "[email protected]",
},
files={
"avatar_file": ("test_avatar.jpg", test_image_data, "image/jpeg")
"avatar_file": ("test_avatar.jpg", b"fake image data", "image/jpeg")
},
follow_redirects=False
)
Expand All @@ -48,8 +55,11 @@ def test_update_profile_authorized(auth_client: TestClient, test_user: User, ses
session.refresh(test_user)
assert test_user.name == "Updated Name"
assert test_user.email == "[email protected]"
assert test_user.avatar_data == test_image_data
assert test_user.avatar_content_type == "image/jpeg"
assert test_user.avatar_data == MOCK_IMAGE_DATA
assert test_user.avatar_content_type == MOCK_CONTENT_TYPE

# Verify mock was called correctly
mock_validate.assert_called_once()


def test_update_profile_without_avatar(auth_client: TestClient, test_user: User, session: Session):
Expand Down Expand Up @@ -110,18 +120,21 @@ def test_delete_account_success(auth_client: TestClient, test_user: User, sessio
assert user is None


def test_get_avatar_authorized(auth_client: TestClient, test_user: User):
@patch('routers.user.validate_and_process_image')
def test_get_avatar_authorized(mock_validate, auth_client: TestClient, test_user: User):
"""Test getting user avatar"""
# Configure mock to return processed image data
mock_validate.return_value = (MOCK_IMAGE_DATA, MOCK_CONTENT_TYPE)

# First upload an avatar
test_image_data = b"fake image data"
auth_client.post(
app.url_path_for("update_profile"),
data={
"name": test_user.name,
"email": test_user.email,
},
files={
"avatar_file": ("test_avatar.jpg", test_image_data, "image/jpeg")
"avatar_file": ("test_avatar.jpg", b"fake image data", "image/jpeg")
},
)

Expand All @@ -130,8 +143,8 @@ def test_get_avatar_authorized(auth_client: TestClient, test_user: User):
app.url_path_for("get_avatar")
)
assert response.status_code == 200
assert response.content == test_image_data
assert response.headers["content-type"] == "image/jpeg"
assert response.content == MOCK_IMAGE_DATA
assert response.headers["content-type"] == MOCK_CONTENT_TYPE


def test_get_avatar_unauthorized(unauth_client: TestClient):
Expand All @@ -142,3 +155,24 @@ def test_get_avatar_unauthorized(unauth_client: TestClient):
)
assert response.status_code == 303
assert response.headers["location"] == app.url_path_for("read_login")


# Add new test for invalid image
@patch('routers.user.validate_and_process_image')
def test_update_profile_invalid_image(mock_validate, auth_client: TestClient):
"""Test that invalid images are rejected"""
# Configure mock to raise InvalidImageError
mock_validate.side_effect = InvalidImageError("Invalid test image")

response: Response = auth_client.post(
app.url_path_for("update_profile"),
data={
"name": "Updated Name",
"email": "[email protected]",
},
files={
"avatar_file": ("test_avatar.jpg", b"invalid image data", "image/jpeg")
},
)
assert response.status_code == 400
assert "Invalid test image" in response.text
Loading
Loading