Skip to content

Commit ae579ea

Browse files
committed
fix: Shared PDF authentication bypass (CVE-2026-34376)
1 parent 39a5077 commit ae579ea

7 files changed

Lines changed: 167 additions & 52 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Generated by Django 5.2.12 on 2026-03-28 11:18
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
('pdf', '0024_remove_unneeded_null_true'),
10+
('sessions', '0001_initial'),
11+
]
12+
13+
operations = [
14+
migrations.AddField(
15+
model_name='sharedpdf',
16+
name='sessions',
17+
field=models.ManyToManyField(to='sessions.session'),
18+
),
19+
]

pdfding/pdf/models/shared_pdf_models.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from uuid import uuid4
33

44
from django.contrib.humanize.templatetags.humanize import naturaltime
5+
from django.contrib.sessions.models import Session
56
from django.db import models
67
from django.utils.translation import gettext_lazy as _
78
from pdf.models.helpers import get_collection_dir
@@ -30,6 +31,7 @@ class SharedPdf(models.Model):
3031
password = models.CharField(max_length=128, null=True, blank=True, help_text=_('Optional'))
3132
expiration_date = models.DateTimeField(null=True, blank=True)
3233
deletion_date = models.DateTimeField(null=True, blank=True)
34+
sessions = models.ManyToManyField(Session)
3335

3436
def __str__(self) -> str:
3537
return self.name # pragma: no cover

pdfding/pdf/services/shared_pdf_services.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,34 @@
11
from datetime import datetime, timedelta, timezone
22

3+
from django.contrib.sessions.models import Session
4+
from pdf.models.shared_pdf_models import SharedPdf
5+
6+
7+
def check_shared_access_allowed_by_identifier(identifier: str, session: Session):
8+
"""Check if access to shared pdf is allowed based on session."""
9+
10+
shared_pdf = SharedPdf.objects.get(pk=identifier)
11+
12+
return check_shared_access_allowed(shared_pdf, session)
13+
14+
15+
def check_shared_access_allowed(shared_pdf: SharedPdf, session: Session):
16+
"""Check if access to shared pdf is allowed based on session."""
17+
18+
if (
19+
session
20+
and (session.get_expiry_date() - datetime.now(timezone.utc)).total_seconds() > 0
21+
and shared_pdf.sessions.filter(session_key=session.session_key).count()
22+
):
23+
return True
24+
else:
25+
return False
26+
327

428
def get_future_datetime(time_input: str) -> datetime | None:
529
"""
630
Gets a datetime in the future from now based on the input. Input is in the format _d_h_m, e.g. 1d0h22m.
7-
If input is an empty string returns None
31+
If input is an empty string returns None.
832
"""
933

1034
if not time_input:

pdfding/pdf/templates/viewer.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
<link rel="stylesheet" href="../../static/pdfjs/web/viewer.css">
3939
<link rel="stylesheet" href="{% static 'css/pdf_viewer.css' %}">
4040
<link rel="icon" type="image/x-icon" href="{% static 'images/logo_with_circle.svg' %}">
41-
{# commented until more languages get added to PdfDing }
41+
{# commented until more languages get added to PdfDing #}
4242
{# <link rel="resource" type="application/l10n" href="../../static/pdfjs/web/locale/locale.json"/> #}
4343
<style>
4444
:root{

pdfding/pdf/tests/test_services/test_shared_pdf_services.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,55 @@
11
from datetime import datetime, timedelta, timezone
2+
from unittest import mock
23

34
from django.contrib.auth.models import User
5+
from django.contrib.sessions.models import Session
46
from django.test import TestCase
5-
from pdf.services.shared_pdf_services import get_future_datetime
7+
from django.urls import reverse
8+
from pdf.models.pdf_models import Pdf
9+
from pdf.models.shared_pdf_models import SharedPdf
10+
from pdf.services.shared_pdf_services import (
11+
check_shared_access_allowed,
12+
check_shared_access_allowed_by_identifier,
13+
get_future_datetime,
14+
)
615

716

817
class TestSharedPdfServices(TestCase):
918
def setUp(self):
1019
self.user = User.objects.create_user(username='username', password='password', email='a@a.com')
1120

21+
def test_check_shared_access_allowed(self):
22+
# get dummy request
23+
response = self.client.get(reverse('pdf_overview'))
24+
request = response.wsgi_request
25+
26+
# create dummy session
27+
request.session.create()
28+
pdf = Pdf.objects.create(name='bla', collection_id=self.user.id)
29+
shared_pdf = SharedPdf.objects.create(pdf=pdf, name='share')
30+
31+
assert not check_shared_access_allowed(shared_pdf, request.session)
32+
33+
shared_pdf.sessions.add(Session.objects.get(session_key=request.session.session_key))
34+
assert check_shared_access_allowed(shared_pdf, request.session)
35+
36+
request.session.set_expiry(-1)
37+
assert not check_shared_access_allowed(shared_pdf, request.session)
38+
39+
@mock.patch('pdf.services.shared_pdf_services.check_shared_access_allowed')
40+
def test_check_shared_access_allowed_by_identifier(self, mock_check):
41+
# get dummy request
42+
response = self.client.get(reverse('pdf_overview'))
43+
request = response.wsgi_request
44+
45+
# create dummy session
46+
request.session.create()
47+
pdf = Pdf.objects.create(name='bla', collection_id=self.user.id)
48+
shared_pdf = SharedPdf.objects.create(pdf=pdf, name='share')
49+
50+
check_shared_access_allowed_by_identifier(shared_pdf.id, request.session)
51+
mock_check.assert_called_once_with(shared_pdf, request.session)
52+
1253
def test_get_future_datetime(self):
1354
expected_result = datetime.now(timezone.utc) + timedelta(days=1, hours=0, minutes=22)
1455
generated_result = get_future_datetime('1d0h22m')

pdfding/pdf/tests/test_views/test_share_views.py

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
from io import BytesIO
33
from unittest.mock import MagicMock, patch
44

5+
import pytest
56
from django.contrib.auth.hashers import make_password
67
from django.contrib.auth.models import User
78
from django.contrib.messages import get_messages
9+
from django.http.response import Http404
810
from django.test import Client, TestCase
911
from django.urls import reverse
1012
from pdf.forms import (
@@ -258,10 +260,22 @@ def setUp(self):
258260
self.pdf = None
259261
set_up(self)
260262

261-
def test_get_object(self):
263+
@patch('pdf.services.shared_pdf_services.check_shared_access_allowed', return_value=True)
264+
def test_get_object(self, mock_check):
265+
# get dummy request
266+
response = self.client.get(reverse('pdf_overview'))
267+
shared_pdf = SharedPdf.objects.create(pdf=self.pdf, name='share')
268+
269+
self.assertEqual(shared_pdf.pdf, PdfPublicMixin.get_object(response.wsgi_request, shared_pdf.id))
270+
271+
@patch('pdf.services.shared_pdf_services.check_shared_access_allowed', return_value=False)
272+
def test_get_object_404(self, mock_check):
273+
# get dummy request
274+
response = self.client.get(reverse('pdf_overview'))
262275
shared_pdf = SharedPdf.objects.create(pdf=self.pdf, name='share')
263276

264-
self.assertEqual(shared_pdf.pdf, PdfPublicMixin.get_object(None, shared_pdf.id))
277+
with pytest.raises(Http404, match='Access to shared pdf not allowed!'):
278+
PdfPublicMixin.get_object(response.wsgi_request, shared_pdf.id)
265279

266280

267281
class TestBaseSharedPdfPublicView(TestCase):
@@ -279,7 +293,7 @@ def test_get_object(self):
279293
self.assertEqual(shared_pdf, BaseSharedPdfPublicView.get_shared_pdf_public(None, shared_pdf.id))
280294

281295

282-
class TestLoginNotRequiredViews(TestCase):
296+
class TestViewSharedPdf(TestCase):
283297
username = 'user'
284298
password = '12345'
285299

@@ -289,7 +303,8 @@ def setUp(self):
289303
set_up(self)
290304
self.shared_pdf = SharedPdf.objects.create(pdf=self.pdf, name='shared_pdf')
291305

292-
def test_view_get_active(self):
306+
@patch('pdf.views.share_views.check_shared_access_allowed', return_value=False)
307+
def test_view_get_active_no_active_session(self, mock_check):
293308
# test without http referer
294309
response = self.client.get(reverse('view_shared_pdf', kwargs={'identifier': self.shared_pdf.id}))
295310

@@ -298,22 +313,17 @@ def test_view_get_active(self):
298313
self.assertEqual(response.context['host'], 'testserver')
299314
self.assertEqual(response.context['form'], ViewSharedPasswordForm)
300315

301-
def test_view_get_inactive(self):
302-
inactive_shared_pdf = SharedPdf.objects.create(pdf=self.pdf, name='inactive_shared_pdf', views=2, max_views=1)
316+
@patch('pdf.views.share_views.get_viewer_theme_and_color')
317+
@patch('pdf.views.share_views.check_shared_access_allowed', return_value=True)
318+
def test_view_get_active_active_session(self, mock_check, mock_get_viewer_theme_and_color):
303319
# test without http referer
304-
response = self.client.get(reverse('view_shared_pdf', kwargs={'identifier': inactive_shared_pdf.id}))
305-
306-
self.assertTemplateUsed(response, 'view_shared_inactive.html')
307320

308-
@patch('pdf.views.share_views.get_viewer_theme_and_color')
309-
def test_view_post_active_no_password(self, mock_get_viewer_theme_and_color):
310321
mock_get_viewer_theme_and_color.return_value = 'dark', '4 4 4'
311-
312322
self.shared_pdf.pdf.revision = 2
313323
self.shared_pdf.pdf.save()
314324
self.assertEqual(self.shared_pdf.views, 0)
315325

316-
response = self.client.post(reverse('view_shared_pdf', kwargs={'identifier': self.shared_pdf.id}))
326+
response = self.client.get(reverse('view_shared_pdf', kwargs={'identifier': self.shared_pdf.id}))
317327
self.assertEqual(response.context['shared_pdf_id'], self.shared_pdf.id)
318328
self.assertEqual(response.context['current_page'], 1)
319329
self.assertEqual(response.context['revision'], 2)
@@ -326,41 +336,47 @@ def test_view_post_active_no_password(self, mock_get_viewer_theme_and_color):
326336
shared_pdf = SharedPdf.objects.get(pk=self.shared_pdf.id)
327337
self.assertEqual(shared_pdf.views, 1)
328338

329-
@patch('pdf.views.share_views.get_viewer_theme_and_color')
330-
def test_view_post_active_correct_password(self, mock_get_viewer_theme_and_color):
331-
mock_get_viewer_theme_and_color.return_value = 'dark', '4 4 4'
339+
def test_view_get_inactive(self):
340+
inactive_shared_pdf = SharedPdf.objects.create(pdf=self.pdf, name='inactive_shared_pdf', views=2, max_views=1)
341+
# test without http referer
342+
response = self.client.get(reverse('view_shared_pdf', kwargs={'identifier': inactive_shared_pdf.id}))
332343

333-
self.shared_pdf.pdf.revision = 2
334-
self.shared_pdf.pdf.save()
344+
self.assertTemplateUsed(response, 'view_shared_inactive.html')
335345

346+
def test_view_post_active_no_password(self):
347+
unprotected_shared_pdf = SharedPdf.objects.create(pdf=self.pdf, name='unprotected_shared_pdf')
348+
assert unprotected_shared_pdf.sessions.count() == 0
349+
350+
response = self.client.post(reverse('view_shared_pdf', kwargs={'identifier': unprotected_shared_pdf.id}))
351+
352+
assert unprotected_shared_pdf.sessions.count() == 1
353+
self.assertRedirects(response, reverse('view_shared_pdf', kwargs={'identifier': unprotected_shared_pdf.id}))
354+
355+
def test_view_post_active_wrong_password(self):
336356
protected_shared_pdf = SharedPdf.objects.create(
337357
pdf=self.pdf, name='protected_shared_pdf', password=make_password('some_pw')
338358
)
339359

340360
response = self.client.post(
341-
reverse('view_shared_pdf', kwargs={'identifier': protected_shared_pdf.id}),
342-
data={'password_input': 'some_pw'},
361+
reverse('view_shared_pdf', kwargs={'identifier': protected_shared_pdf.id}), data={'password_input': 'wrong'}
343362
)
344363

345-
self.assertEqual(response.context['shared_pdf_id'], protected_shared_pdf.id)
346-
self.assertEqual(response.context['theme_color'], '4 4 4')
347-
self.assertEqual(response.context['theme'], 'dark')
348-
self.assertEqual(response.context['revision'], 2)
349-
self.assertEqual(response.context['user_view_bool'], False)
350-
self.assertTemplateUsed(response, 'viewer.html')
351-
352-
protected_shared_pdf = SharedPdf.objects.get(pk=protected_shared_pdf.id)
353-
self.assertEqual(protected_shared_pdf.views, 1)
364+
self.assertIsInstance(response.context['form'], ViewSharedPasswordForm)
365+
self.assertTemplateUsed(response, 'view_shared_info.html')
354366

355-
def test_view_post_active_wrong_password(self):
356-
protected_shared_pdf = SharedPdf.objects.create(pdf=self.pdf, name='protected_shared_pdf', password='some_pw')
367+
def test_view_post_active_correct_password(self):
368+
protected_shared_pdf = SharedPdf.objects.create(
369+
pdf=self.pdf, name='protected_shared_pdf', password=make_password('some_pw')
370+
)
371+
assert protected_shared_pdf.sessions.count() == 0
357372

358373
response = self.client.post(
359-
reverse('view_shared_pdf', kwargs={'identifier': protected_shared_pdf.id}), data={'password_input': 'wrong'}
374+
reverse('view_shared_pdf', kwargs={'identifier': protected_shared_pdf.id}),
375+
data={'password_input': 'some_pw'},
360376
)
361377

362-
self.assertIsInstance(response.context['form'], ViewSharedPasswordForm)
363-
self.assertTemplateUsed(response, 'view_shared_info.html')
378+
assert protected_shared_pdf.sessions.count() == 1
379+
self.assertRedirects(response, reverse('view_shared_pdf', kwargs={'identifier': protected_shared_pdf.id}))
364380

365381
def test_view_post_inactive(self):
366382
inactive_shared_pdf = SharedPdf.objects.create(pdf=self.pdf, name='inactive_shared_pdf', views=2, max_views=1)

pdfding/pdf/views/share_views.py

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55
from base import base_views
66
from django.contrib import messages
77
from django.contrib.auth.decorators import login_not_required
8+
from django.contrib.sessions.models import Session
89
from django.core.files import File
910
from django.db.models import Q, QuerySet
1011
from django.db.models.functions import Lower
11-
from django.http import HttpRequest
12-
from django.shortcuts import render
12+
from django.http import Http404, HttpRequest
13+
from django.shortcuts import redirect, render
1314
from django.urls import reverse
1415
from django.utils.decorators import method_decorator
1516
from django.utils.translation import gettext_lazy as _
@@ -26,7 +27,11 @@
2627
)
2728
from pdf.models.shared_pdf_models import SharedPdf
2829
from pdf.services.pdf_services import check_object_access_allowed
29-
from pdf.services.shared_pdf_services import get_future_datetime
30+
from pdf.services.shared_pdf_services import (
31+
check_shared_access_allowed,
32+
check_shared_access_allowed_by_identifier,
33+
get_future_datetime,
34+
)
3035
from pdf.services.workspace_services import get_shared_pdfs_of_workspace
3136
from pdf.views.pdf_views import PdfMixin
3237
from qrcode.image import svg
@@ -150,7 +155,7 @@ class SharedPdfMixin(BaseShareMixin):
150155
@staticmethod
151156
@check_object_access_allowed
152157
def get_object(request: HttpRequest, identifier: str):
153-
"""Get the shered pdf specified by the ID"""
158+
"""Get the shared pdf specified by the ID"""
154159

155160
user_profile = request.user.profile
156161
shared_pdf = user_profile.all_shared_pdfs.get(id=identifier)
@@ -217,13 +222,15 @@ def process_field(cls, field_name: str, shared_pdf: SharedPdf, request: HttpRequ
217222

218223
class PdfPublicMixin:
219224
@staticmethod
220-
@check_object_access_allowed
221-
def get_object(_, shared_id: str):
225+
def get_object(request: HttpRequest, shared_id: str):
222226
"""Get the shared pdf specified by the ID"""
223227

224-
shared_pdf = SharedPdf.objects.get(pk=shared_id)
228+
if check_shared_access_allowed_by_identifier(shared_id, request.session):
229+
shared_pdf = SharedPdf.objects.get(pk=shared_id)
225230

226-
return shared_pdf.pdf
231+
return shared_pdf.pdf
232+
else:
233+
raise Http404('Access to shared pdf not allowed!')
227234

228235

229236
class BaseSharedPdfPublicView(View):
@@ -302,6 +309,8 @@ def get(self, request: HttpRequest, identifier: str):
302309

303310
if shared_pdf.inactive or shared_pdf.deleted:
304311
return render(request, 'view_shared_inactive.html')
312+
elif check_shared_access_allowed(shared_pdf, request.session):
313+
return self.render_shared_pdf_view(request, shared_pdf)
305314
else:
306315
return render(
307316
request,
@@ -315,15 +324,19 @@ def post(self, request: HttpRequest, identifier: str):
315324
if shared_pdf.inactive:
316325
return render(request, 'view_shared_inactive.html')
317326
else:
318-
if shared_pdf.password:
319-
form = ViewSharedPasswordForm(request.POST, shared_pdf=shared_pdf)
327+
form = ViewSharedPasswordForm(request.POST, shared_pdf=shared_pdf)
328+
329+
if not shared_pdf.password or form.is_valid():
330+
if not request.session or not request.session.session_key:
331+
request.session.create()
332+
# set session expiry to 1 week
333+
request.session.set_expiry(604800)
334+
request.session.save()
320335

321-
if form.is_valid():
322-
return self.render_shared_pdf_view(request, shared_pdf)
323-
else:
324-
return render(request, 'view_shared_info.html', {'shared_pdf': shared_pdf, 'form': form})
336+
shared_pdf.sessions.add(Session.objects.get(session_key=request.session.session_key))
337+
return redirect('view_shared_pdf', identifier=shared_pdf.id)
325338
else:
326-
return self.render_shared_pdf_view(request, shared_pdf)
339+
return render(request, 'view_shared_info.html', {'shared_pdf': shared_pdf, 'form': form})
327340

328341
@staticmethod
329342
def render_shared_pdf_view(request: HttpRequest, shared_pdf: SharedPdf):

0 commit comments

Comments
 (0)