diff --git a/.gitignore b/.gitignore index 60949b9f50e..af59e4410fb 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,7 @@ media/json media/man media/pdf media/static +media/usercontent /static node_modules nodeenv diff --git a/dockerfiles/nginx/web.conf.template b/dockerfiles/nginx/web.conf.template index 14296611ea9..ed6d1faaeb8 100644 --- a/dockerfiles/nginx/web.conf.template +++ b/dockerfiles/nginx/web.conf.template @@ -18,6 +18,11 @@ server { break; } + location /usercontent/ { + proxy_pass http://storage:9000/usercontent/; + break; + } + location / { proxy_pass http://web:8000/; proxy_set_header X-Forwarded-Host $host; diff --git a/readthedocs/organizations/forms.py b/readthedocs/organizations/forms.py index 70adac8b40a..2f0ba5a79c8 100644 --- a/readthedocs/organizations/forms.py +++ b/readthedocs/organizations/forms.py @@ -7,6 +7,7 @@ from django.core.validators import EmailValidator from django.db.models import Q from django.utils.translation import gettext_lazy as _ +from PIL import Image from readthedocs.core.history import SimpleHistoryModelForm from readthedocs.core.permissions import AdminPermission @@ -36,7 +37,7 @@ class OrganizationForm(SimpleHistoryModelForm): class Meta: model = Organization - fields = ["name", "email", "description", "url"] + fields = ["name", "email", "avatar", "description", "url"] labels = { "name": _("Organization Name"), "email": _("Billing Email"), @@ -78,6 +79,22 @@ def clean_name(self): ) return name + def clean_avatar(self): + avatar = self.cleaned_data.get("avatar") + if avatar: + if avatar.size > 750 * 1024: + raise forms.ValidationError( + _("Avatar image size must not exceed 750KB."), + ) + try: + img = Image.open(avatar) + except Exception: + raise ValidationError("Could not process image. Please upload a valid image file.") + width, height = img.size + if width > 500 or height > 500: + raise ValidationError("The image dimensions cannot exceed 500x500 pixels.") + return avatar + class OrganizationSignupFormBase(OrganizationForm): """ diff --git a/readthedocs/organizations/migrations/0016_organization_avatar.py b/readthedocs/organizations/migrations/0016_organization_avatar.py new file mode 100644 index 00000000000..9b2637c4754 --- /dev/null +++ b/readthedocs/organizations/migrations/0016_organization_avatar.py @@ -0,0 +1,62 @@ +# Generated by Django 5.2.3 on 2025-06-17 22:20 +import django.core.validators +from django.db import migrations +from django.db import models +from django_safemigrate import Safe + +import readthedocs.organizations.models + + +class Migration(migrations.Migration): + safe = Safe.before_deploy() + + dependencies = [ + ("organizations", "0015_remove_unused_indexes"), + ("projects", "0148_remove_unused_indexes"), + ] + + operations = [ + migrations.AddField( + model_name="historicalorganization", + name="avatar", + field=models.TextField( + blank=True, + help_text="Avatar for your organization (JPG or PNG format, max 500x500px, 750KB)", + max_length=100, + null=True, + validators=[ + django.core.validators.FileExtensionValidator( + allowed_extensions=["jpg", "jpeg", "png"] + ) + ], + verbose_name="Avatar", + ), + ), + migrations.AddField( + model_name="organization", + name="avatar", + field=models.ImageField( + blank=True, + help_text="Avatar for your organization (JPG or PNG format, max 500x500px, 750KB)", + null=True, + storage=readthedocs.organizations.models._get_user_content_storage, + upload_to=readthedocs.organizations.models._upload_organization_avatar_to, + validators=[ + django.core.validators.FileExtensionValidator( + allowed_extensions=["jpg", "jpeg", "png"] + ) + ], + verbose_name="Avatar", + ), + ), + migrations.AlterField( + model_name="organization", + name="projects", + field=models.ManyToManyField( + blank=True, + related_name="organizations", + to="projects.project", + verbose_name="Projects", + ), + ), + ] diff --git a/readthedocs/organizations/models.py b/readthedocs/organizations/models.py index 731f05b13ba..6e4ce0916fb 100644 --- a/readthedocs/organizations/models.py +++ b/readthedocs/organizations/models.py @@ -1,14 +1,20 @@ """Organizations models.""" +from pathlib import Path +from uuid import uuid4 + import structlog from autoslug import AutoSlugField from django.contrib.auth.models import User from django.contrib.contenttypes.fields import GenericRelation from django.contrib.contenttypes.models import ContentType +from django.core.files.storage import storages +from django.core.validators import FileExtensionValidator from django.db import models from django.urls import reverse from django.utils.crypto import salted_hmac from django.utils.translation import gettext_lazy as _ +from django_gravatar.helpers import get_gravatar_url from djstripe.enums import SubscriptionStatus from readthedocs.core.history import ExtraHistoricalRecords @@ -26,6 +32,36 @@ log = structlog.get_logger(__name__) +def _upload_organization_avatar_to(instance, filename): + """ + Generate the upload path for the organization avatar. + + The name of the file is an UUID, and the extension is preserved. + If the instance already has an avatar, we use its name to keep the same UUID. + """ + extension = filename.split(".")[-1].lower() + try: + previous_avatar = Organization.objects.get(pk=instance.pk).avatar + except Organization.DoesNotExist: + previous_avatar = None + + if not previous_avatar: + uuid = uuid4().hex + else: + uuid = Path(previous_avatar.name).stem + return f"avatars/organizations/{uuid}.{extension}" + + +def _get_user_content_storage(): + """ + Get the storage for user content. + + Use a function for storage instead of directly assigning the instance + to avoid hardcoding the backend in the migration file. + """ + return storages["usercontent"] + + class Organization(models.Model): """Organization model.""" @@ -38,6 +74,7 @@ class Organization(models.Model): "projects.Project", verbose_name=_("Projects"), related_name="organizations", + blank=True, ) owners = models.ManyToManyField( User, @@ -129,6 +166,16 @@ class Organization(models.Model): object_id_field="attached_to_id", ) + avatar = models.ImageField( + _("Avatar"), + upload_to=_upload_organization_avatar_to, + storage=_get_user_content_storage, + validators=[FileExtensionValidator(allowed_extensions=["jpg", "jpeg", "png"])], + blank=True, + null=True, + help_text="Avatar for your organization (JPG or PNG format, max 500x500px, 750KB)", + ) + # Managers objects = OrganizationQuerySet.as_manager() history = ExtraHistoricalRecords() @@ -190,6 +237,14 @@ def save(self, *args, **kwargs): if self.stripe_customer: self.stripe_id = self.stripe_customer.id + # If the avatar is being changed, delete the previous one. + try: + previous_avatar = Organization.objects.get(pk=self.pk).avatar + except Organization.DoesNotExist: + previous_avatar = None + if previous_avatar and previous_avatar != self.avatar: + previous_avatar.delete(save=False) + super().save(*args, **kwargs) def get_stripe_metadata(self): @@ -214,6 +269,24 @@ def add_member(self, user, team): member = TeamMember.objects.create(team=team, member=user) return member + def get_avatar_url(self): + """ + Get the URL of the organization's avatar. + + Use the `avatar` field if it exists, otherwise use + the gravatar from the organization's email. + """ + if self.avatar: + return self.avatar.url + return get_gravatar_url(self.email, size=100) + + def delete(self, *args, **kwargs): + """Override delete method to clean up related resources.""" + # Delete the avatar file. + if self.avatar: + self.avatar.delete(save=False) + super().delete(*args, **kwargs) + class OrganizationOwner(models.Model): """Intermediate table for Organization <-> User relationships.""" diff --git a/readthedocs/organizations/tests/test_views.py b/readthedocs/organizations/tests/test_views.py index 33544ab98fd..e633fe7eb6d 100644 --- a/readthedocs/organizations/tests/test_views.py +++ b/readthedocs/organizations/tests/test_views.py @@ -1,4 +1,6 @@ import csv +from django.core.files.uploadedfile import SimpleUploadedFile +import io import itertools from unittest import mock @@ -9,6 +11,7 @@ from django.urls import reverse from django.utils import timezone from django_dynamic_fixture import get +from PIL import Image from readthedocs.audit.models import AuditLog from readthedocs.core.utils import slugify @@ -59,6 +62,106 @@ def test_update(self): # The slug hasn't changed. self.assertEqual(self.organization.slug, org_slug) + def _create_image(self, size=(100, 100), format='PNG'): + """Helper to create an in-memory image file.""" + image = Image.new(mode='RGB', size=size, color=(0, 0, 0)) + image_bytes = io.BytesIO() + image.save(image_bytes, format=format) + image_bytes.seek(0) + return image_bytes + + def test_update_avatar(self): + avatar_file = SimpleUploadedFile( + name='test.png', + content=self._create_image(size=(100, 100)).read(), + content_type='image/png' + ) + + response = self.client.post( + reverse("organization_edit", args=[self.organization.slug]), + { + "name": "New name", + "email": "dev@example.com", + "description": "Description", + "url": "https://readthedocs.org", + "avatar": avatar_file, + }, + ) + assert response.status_code == 302 + self.organization.refresh_from_db() + assert self.organization.avatar + assert self.organization.avatar.name.startswith("avatars/organizations/") + assert self.organization.avatar.name.endswith(".png") + + def test_update_avatar_invalid_dimensions(self): + avatar_file = SimpleUploadedFile( + name='test.png', + content=self._create_image(size=(1000, 1000)).read(), + content_type='image/png' + ) + + response = self.client.post( + reverse("organization_edit", args=[self.organization.slug]), + { + "name": "New name", + "email": "dev@example.com", + "description": "Description", + "url": "https://readthedocs.org", + "avatar": avatar_file, + }, + ) + assert response.status_code == 200 + form = response.context_data['form'] + assert not form.is_valid() + assert 'avatar' in form.errors + assert "The image dimensions cannot exceed" in form.errors['avatar'][0] + + def test_update_avatar_invalid_image(self): + avatar_file = SimpleUploadedFile( + name='test.txt', + content=b'This is not an image file.', + content_type='text/plain' + ) + + response = self.client.post( + reverse("organization_edit", args=[self.organization.slug]), + { + "name": "New name", + "email": "dev@example.com", + "description": "Description", + "url": "https://readthedocs.org", + "avatar": avatar_file, + }, + ) + assert response.status_code == 200 + form = response.context_data['form'] + assert not form.is_valid() + assert 'avatar' in form.errors + assert "Upload a valid image." in form.errors['avatar'][0] + + def test_update_avatar_invalid_extension(self): + avatar_file = SimpleUploadedFile( + name='test.gif', + content=self._create_image(size=(100, 100), format='GIF').read(), + content_type='image/gif' + ) + + response = self.client.post( + reverse("organization_edit", args=[self.organization.slug]), + { + "name": "New name", + "email": "dev@example.com", + "description": "Description", + "url": "https://readthedocs.org", + "avatar": avatar_file, + }, + ) + assert response.status_code == 200 + form = response.context_data['form'] + assert not form.is_valid() + assert 'avatar' in form.errors + assert "File extension “gif” is not allowed" in form.errors['avatar'][0] + def test_change_name(self): """ Changing the name of the organization won't change the slug. diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index d5e2f8e2180..d2446e17180 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -6,6 +6,7 @@ import subprocess import structlog +from pathlib import Path from celery.schedules import crontab from corsheaders.defaults import default_headers from django.conf.global_settings import PASSWORD_HASHERS @@ -1062,6 +1063,13 @@ def STORAGES(self): "staticfiles": { "BACKEND": "readthedocs.storage.s3_storage.S3StaticStorage" }, + "usercontent": { + "BACKEND": "django.core.files.storage.FileSystemStorage", + "OPTIONS": { + "location": Path(self.MEDIA_ROOT) / "usercontent", + "allow_overwrite": True, + } + }, } @property diff --git a/readthedocs/settings/docker_compose.py b/readthedocs/settings/docker_compose.py index 9658362a1a8..bd37a2f0784 100644 --- a/readthedocs/settings/docker_compose.py +++ b/readthedocs/settings/docker_compose.py @@ -239,5 +239,21 @@ def SOCIALACCOUNT_PROVIDERS(self): RTD_FILETREEDIFF_ALL = "RTD_FILETREEDIFF_ALL" in os.environ + @property + def STORAGES(self): + return { + "staticfiles": { + "BACKEND": "readthedocs.storage.s3_storage.S3StaticStorage" + }, + "usercontent": { + "BACKEND": "storages.backends.s3.S3Storage", + "OPTIONS": { + "bucket_name": os.environ.get("RTD_S3_USER_CONTENT_STORAGE_BUCKET", "usercontent"), + "url_protocol": "http:", + "custom_domain": self.PRODUCTION_DOMAIN + "/usercontent", + }, + }, + } + DockerBaseSettings.load_settings(__name__) diff --git a/readthedocs/settings/test.py b/readthedocs/settings/test.py index c32d2befab9..f2657651e02 100644 --- a/readthedocs/settings/test.py +++ b/readthedocs/settings/test.py @@ -1,4 +1,5 @@ import os +from pathlib import Path import textwrap from .base import CommunityBaseSettings @@ -140,6 +141,13 @@ def STORAGES(self): "staticfiles": { "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage", }, + "usercontent": { + "BACKEND": "django.core.files.storage.FileSystemStorage", + "OPTIONS": { + "location": Path(self.MEDIA_ROOT) / "usercontent", + "allow_overwrite": True, + }, + }, } diff --git a/requirements/deploy.txt b/requirements/deploy.txt index fb95c09b8ff..472dbedc225 100644 --- a/requirements/deploy.txt +++ b/requirements/deploy.txt @@ -286,6 +286,8 @@ parso==0.8.4 # via jedi pexpect==4.9.0 # via ipython +pillow==11.2.1 + # via -r requirements/pip.txt platformdirs==4.3.8 # via # -r requirements/pip.txt diff --git a/requirements/docker.txt b/requirements/docker.txt index cd00656cb06..6b623729429 100644 --- a/requirements/docker.txt +++ b/requirements/docker.txt @@ -305,6 +305,8 @@ pdbpp==0.11.6 # via -r requirements/docker.in pexpect==4.9.0 # via ipython +pillow==11.2.1 + # via -r requirements/pip.txt platformdirs==4.3.8 # via # -r requirements/pip.txt diff --git a/requirements/pip.in b/requirements/pip.in index 9818b389fc6..ab7b09dcc39 100644 --- a/requirements/pip.in +++ b/requirements/pip.in @@ -38,6 +38,9 @@ django-safemigrate # Impersonate users in the Django admin for support. django-impersonate +# For ImageField +Pillow + requests requests-toolbelt slumber diff --git a/requirements/pip.txt b/requirements/pip.txt index 8f23a73f59f..467d240334b 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -213,6 +213,8 @@ packaging==25.0 # drf-extensions # gunicorn # kombu +pillow==11.2.1 + # via -r requirements/pip.in platformdirs==4.3.8 # via virtualenv prompt-toolkit==3.0.51 diff --git a/requirements/testing.txt b/requirements/testing.txt index 6b400c5ca7b..01cfdfec5cd 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -290,6 +290,8 @@ packaging==25.0 # kombu # pytest # sphinx +pillow==11.2.1 + # via -r requirements/pip.txt platformdirs==4.3.8 # via # -r requirements/pip.txt