Skip to content

Commit d589faf

Browse files
authored
Organizations: allow uploading avatar (#12254)
- The file size is set to 750kb as max - The image dimensions should be equal or less to 500px - Only jpg and png formats are allowed (this is a soft check anyway), we are not checking for the content, but django does check if the file is an image using pillow at least. - Images are publicly accessible and can be enumerated, I think this is fine, avatar images aren't sensible information. - pillow is a required dependency when using ImageField This requires creating a new bucket and the corresponding settings in ops. Closes readthedocs/readthedocs-corporate#1471
1 parent 63e16bd commit d589faf

File tree

14 files changed

+305
-1
lines changed

14 files changed

+305
-1
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ media/json
3838
media/man
3939
media/pdf
4040
media/static
41+
media/usercontent
4142
/static
4243
node_modules
4344
nodeenv

dockerfiles/nginx/web.conf.template

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ server {
1818
break;
1919
}
2020

21+
location /usercontent/ {
22+
proxy_pass http://storage:9000/usercontent/;
23+
break;
24+
}
25+
2126
location / {
2227
proxy_pass http://web:8000/;
2328
proxy_set_header X-Forwarded-Host $host;

readthedocs/organizations/forms.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from django.core.validators import EmailValidator
88
from django.db.models import Q
99
from django.utils.translation import gettext_lazy as _
10+
from PIL import Image
1011

1112
from readthedocs.core.history import SimpleHistoryModelForm
1213
from readthedocs.core.permissions import AdminPermission
@@ -36,7 +37,7 @@ class OrganizationForm(SimpleHistoryModelForm):
3637

3738
class Meta:
3839
model = Organization
39-
fields = ["name", "email", "description", "url"]
40+
fields = ["name", "email", "avatar", "description", "url"]
4041
labels = {
4142
"name": _("Organization Name"),
4243
"email": _("Billing Email"),
@@ -85,6 +86,22 @@ def clean_slug(self):
8586
raise forms.ValidationError(_("Slug is already used by another organization"))
8687
return slug
8788

89+
def clean_avatar(self):
90+
avatar = self.cleaned_data.get("avatar")
91+
if avatar:
92+
if avatar.size > 750 * 1024:
93+
raise forms.ValidationError(
94+
_("Avatar image size must not exceed 750KB."),
95+
)
96+
try:
97+
img = Image.open(avatar)
98+
except Exception:
99+
raise ValidationError("Could not process image. Please upload a valid image file.")
100+
width, height = img.size
101+
if width > 500 or height > 500:
102+
raise ValidationError("The image dimensions cannot exceed 500x500 pixels.")
103+
return avatar
104+
88105

89106
class OrganizationSignupFormBase(OrganizationForm):
90107
"""
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Generated by Django 5.2.3 on 2025-06-17 22:20
2+
import django.core.validators
3+
from django.db import migrations
4+
from django.db import models
5+
from django_safemigrate import Safe
6+
7+
import readthedocs.organizations.models
8+
9+
10+
class Migration(migrations.Migration):
11+
safe = Safe.before_deploy()
12+
13+
dependencies = [
14+
("organizations", "0015_remove_unused_indexes"),
15+
("projects", "0148_remove_unused_indexes"),
16+
]
17+
18+
operations = [
19+
migrations.AddField(
20+
model_name="historicalorganization",
21+
name="avatar",
22+
field=models.TextField(
23+
blank=True,
24+
help_text="Avatar for your organization (JPG or PNG format, max 500x500px, 750KB)",
25+
max_length=100,
26+
null=True,
27+
validators=[
28+
django.core.validators.FileExtensionValidator(
29+
allowed_extensions=["jpg", "jpeg", "png"]
30+
)
31+
],
32+
verbose_name="Avatar",
33+
),
34+
),
35+
migrations.AddField(
36+
model_name="organization",
37+
name="avatar",
38+
field=models.ImageField(
39+
blank=True,
40+
help_text="Avatar for your organization (JPG or PNG format, max 500x500px, 750KB)",
41+
null=True,
42+
storage=readthedocs.organizations.models._get_user_content_storage,
43+
upload_to=readthedocs.organizations.models._upload_organization_avatar_to,
44+
validators=[
45+
django.core.validators.FileExtensionValidator(
46+
allowed_extensions=["jpg", "jpeg", "png"]
47+
)
48+
],
49+
verbose_name="Avatar",
50+
),
51+
),
52+
migrations.AlterField(
53+
model_name="organization",
54+
name="projects",
55+
field=models.ManyToManyField(
56+
blank=True,
57+
related_name="organizations",
58+
to="projects.project",
59+
verbose_name="Projects",
60+
),
61+
),
62+
]

readthedocs/organizations/models.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
11
"""Organizations models."""
22

3+
from pathlib import Path
4+
from uuid import uuid4
5+
36
import structlog
47
from autoslug import AutoSlugField
58
from django.contrib.auth.models import User
69
from django.contrib.contenttypes.fields import GenericRelation
710
from django.contrib.contenttypes.models import ContentType
11+
from django.core.files.storage import storages
12+
from django.core.validators import FileExtensionValidator
813
from django.db import models
914
from django.urls import reverse
1015
from django.utils.crypto import salted_hmac
1116
from django.utils.translation import gettext_lazy as _
17+
from django_gravatar.helpers import get_gravatar_url
1218
from djstripe.enums import SubscriptionStatus
1319

1420
from readthedocs.core.history import ExtraHistoricalRecords
@@ -26,6 +32,36 @@
2632
log = structlog.get_logger(__name__)
2733

2834

35+
def _upload_organization_avatar_to(instance, filename):
36+
"""
37+
Generate the upload path for the organization avatar.
38+
39+
The name of the file is an UUID, and the extension is preserved.
40+
If the instance already has an avatar, we use its name to keep the same UUID.
41+
"""
42+
extension = filename.split(".")[-1].lower()
43+
try:
44+
previous_avatar = Organization.objects.get(pk=instance.pk).avatar
45+
except Organization.DoesNotExist:
46+
previous_avatar = None
47+
48+
if not previous_avatar:
49+
uuid = uuid4().hex
50+
else:
51+
uuid = Path(previous_avatar.name).stem
52+
return f"avatars/organizations/{uuid}.{extension}"
53+
54+
55+
def _get_user_content_storage():
56+
"""
57+
Get the storage for user content.
58+
59+
Use a function for storage instead of directly assigning the instance
60+
to avoid hardcoding the backend in the migration file.
61+
"""
62+
return storages["usercontent"]
63+
64+
2965
class Organization(models.Model):
3066
"""Organization model."""
3167

@@ -38,6 +74,7 @@ class Organization(models.Model):
3874
"projects.Project",
3975
verbose_name=_("Projects"),
4076
related_name="organizations",
77+
blank=True,
4178
)
4279
owners = models.ManyToManyField(
4380
User,
@@ -129,6 +166,16 @@ class Organization(models.Model):
129166
object_id_field="attached_to_id",
130167
)
131168

169+
avatar = models.ImageField(
170+
_("Avatar"),
171+
upload_to=_upload_organization_avatar_to,
172+
storage=_get_user_content_storage,
173+
validators=[FileExtensionValidator(allowed_extensions=["jpg", "jpeg", "png"])],
174+
blank=True,
175+
null=True,
176+
help_text="Avatar for your organization (JPG or PNG format, max 500x500px, 750KB)",
177+
)
178+
132179
# Managers
133180
objects = OrganizationQuerySet.as_manager()
134181
history = ExtraHistoricalRecords()
@@ -190,6 +237,14 @@ def save(self, *args, **kwargs):
190237
if self.stripe_customer:
191238
self.stripe_id = self.stripe_customer.id
192239

240+
# If the avatar is being changed, delete the previous one.
241+
try:
242+
previous_avatar = Organization.objects.get(pk=self.pk).avatar
243+
except Organization.DoesNotExist:
244+
previous_avatar = None
245+
if previous_avatar and previous_avatar != self.avatar:
246+
previous_avatar.delete(save=False)
247+
193248
super().save(*args, **kwargs)
194249

195250
def get_stripe_metadata(self):
@@ -214,6 +269,24 @@ def add_member(self, user, team):
214269
member = TeamMember.objects.create(team=team, member=user)
215270
return member
216271

272+
def get_avatar_url(self):
273+
"""
274+
Get the URL of the organization's avatar.
275+
276+
Use the `avatar` field if it exists, otherwise use
277+
the gravatar from the organization's email.
278+
"""
279+
if self.avatar:
280+
return self.avatar.url
281+
return get_gravatar_url(self.email, size=100)
282+
283+
def delete(self, *args, **kwargs):
284+
"""Override delete method to clean up related resources."""
285+
# Delete the avatar file.
286+
if self.avatar:
287+
self.avatar.delete(save=False)
288+
super().delete(*args, **kwargs)
289+
217290

218291
class OrganizationOwner(models.Model):
219292
"""Intermediate table for Organization <-> User relationships."""

readthedocs/organizations/tests/test_views.py

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import csv
2+
from django.core.files.uploadedfile import SimpleUploadedFile
3+
import io
24
import itertools
35
from unittest import mock
46

@@ -9,6 +11,7 @@
911
from django.urls import reverse
1012
from django.utils import timezone
1113
from django_dynamic_fixture import get
14+
from PIL import Image
1215

1316
from readthedocs.audit.models import AuditLog
1417
from readthedocs.core.utils import slugify
@@ -59,6 +62,106 @@ def test_update(self):
5962
# The slug hasn't changed.
6063
self.assertEqual(self.organization.slug, org_slug)
6164

65+
def _create_image(self, size=(100, 100), format='PNG'):
66+
"""Helper to create an in-memory image file."""
67+
image = Image.new(mode='RGB', size=size, color=(0, 0, 0))
68+
image_bytes = io.BytesIO()
69+
image.save(image_bytes, format=format)
70+
image_bytes.seek(0)
71+
return image_bytes
72+
73+
def test_update_avatar(self):
74+
avatar_file = SimpleUploadedFile(
75+
name='test.png',
76+
content=self._create_image(size=(100, 100)).read(),
77+
content_type='image/png'
78+
)
79+
80+
response = self.client.post(
81+
reverse("organization_edit", args=[self.organization.slug]),
82+
{
83+
"name": "New name",
84+
"email": "[email protected]",
85+
"description": "Description",
86+
"url": "https://readthedocs.org",
87+
"avatar": avatar_file,
88+
},
89+
)
90+
assert response.status_code == 302
91+
self.organization.refresh_from_db()
92+
assert self.organization.avatar
93+
assert self.organization.avatar.name.startswith("avatars/organizations/")
94+
assert self.organization.avatar.name.endswith(".png")
95+
96+
def test_update_avatar_invalid_dimensions(self):
97+
avatar_file = SimpleUploadedFile(
98+
name='test.png',
99+
content=self._create_image(size=(1000, 1000)).read(),
100+
content_type='image/png'
101+
)
102+
103+
response = self.client.post(
104+
reverse("organization_edit", args=[self.organization.slug]),
105+
{
106+
"name": "New name",
107+
"email": "[email protected]",
108+
"description": "Description",
109+
"url": "https://readthedocs.org",
110+
"avatar": avatar_file,
111+
},
112+
)
113+
assert response.status_code == 200
114+
form = response.context_data['form']
115+
assert not form.is_valid()
116+
assert 'avatar' in form.errors
117+
assert "The image dimensions cannot exceed" in form.errors['avatar'][0]
118+
119+
def test_update_avatar_invalid_image(self):
120+
avatar_file = SimpleUploadedFile(
121+
name='test.txt',
122+
content=b'This is not an image file.',
123+
content_type='text/plain'
124+
)
125+
126+
response = self.client.post(
127+
reverse("organization_edit", args=[self.organization.slug]),
128+
{
129+
"name": "New name",
130+
"email": "[email protected]",
131+
"description": "Description",
132+
"url": "https://readthedocs.org",
133+
"avatar": avatar_file,
134+
},
135+
)
136+
assert response.status_code == 200
137+
form = response.context_data['form']
138+
assert not form.is_valid()
139+
assert 'avatar' in form.errors
140+
assert "Upload a valid image." in form.errors['avatar'][0]
141+
142+
def test_update_avatar_invalid_extension(self):
143+
avatar_file = SimpleUploadedFile(
144+
name='test.gif',
145+
content=self._create_image(size=(100, 100), format='GIF').read(),
146+
content_type='image/gif'
147+
)
148+
149+
response = self.client.post(
150+
reverse("organization_edit", args=[self.organization.slug]),
151+
{
152+
"name": "New name",
153+
"email": "[email protected]",
154+
"description": "Description",
155+
"url": "https://readthedocs.org",
156+
"avatar": avatar_file,
157+
},
158+
)
159+
assert response.status_code == 200
160+
form = response.context_data['form']
161+
assert not form.is_valid()
162+
assert 'avatar' in form.errors
163+
assert "File extension “gif” is not allowed" in form.errors['avatar'][0]
164+
62165
def test_change_name(self):
63166
"""
64167
Changing the name of the organization won't change the slug.

readthedocs/settings/base.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import subprocess
77

88
import structlog
9+
from pathlib import Path
910
from celery.schedules import crontab
1011
from corsheaders.defaults import default_headers
1112
from django.conf.global_settings import PASSWORD_HASHERS
@@ -1170,6 +1171,13 @@ def STORAGES(self):
11701171
"staticfiles": {
11711172
"BACKEND": "readthedocs.storage.s3_storage.S3StaticStorage"
11721173
},
1174+
"usercontent": {
1175+
"BACKEND": "django.core.files.storage.FileSystemStorage",
1176+
"OPTIONS": {
1177+
"location": Path(self.MEDIA_ROOT) / "usercontent",
1178+
"allow_overwrite": True,
1179+
}
1180+
},
11731181
}
11741182

11751183
@property

0 commit comments

Comments
 (0)