diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000000..e1f53f12f4 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,20 @@ +# Security Policy + +## Reporting a Vulnerability + +**Short version: please report security issues by emailing security@maykinmedia.nl.** + +If you discover security issues in Open Inwoner or related projects under the same +organization, we request you to disclose these in a *responsible* way by e-mailing to +security@maykinmedia.nl. + +It is extremely useful if you have a reproducible test case and/or clear steps on how to +reproduce the vulnerability. + +Please do not report security issues on the public Github issue tracker, as this makes +it visible which exploits exist before a fix is available, potentially comprising a lot +of unprotected instances. + +Once you’ve submitted an issue via email, you should receive an acknowledgment from a +member of the security team as soon as possible, and depending on the action to be taken, +you may receive further followup emails. diff --git a/src/log_outgoing_requests/admin.py b/src/log_outgoing_requests/admin.py index 8289caa55a..fdc9531548 100644 --- a/src/log_outgoing_requests/admin.py +++ b/src/log_outgoing_requests/admin.py @@ -17,6 +17,8 @@ class OutgoingRequestsLogAdmin(admin.ModelAdmin): "timestamp", "req_content_type", "res_content_type", + "req_headers", + "res_headers", "trace", ) readonly_fields = fields @@ -30,7 +32,7 @@ class OutgoingRequestsLogAdmin(admin.ModelAdmin): "timestamp", ) list_filter = ("method", "status_code", "hostname") - search_fields = ("params", "query_params", "hostname") + search_fields = ("url", "params", "hostname") date_hierarchy = "timestamp" show_full_result_count = False diff --git a/src/log_outgoing_requests/formatters.py b/src/log_outgoing_requests/formatters.py index de29e75486..b12eae1154 100644 --- a/src/log_outgoing_requests/formatters.py +++ b/src/log_outgoing_requests/formatters.py @@ -3,6 +3,9 @@ class HttpFormatter(logging.Formatter): + def _formatHeaders(self, d): + return "\n".join(f"{k}: {v}" for k, v in d.items()) + def formatMessage(self, record): result = super().formatMessage(record) if record.name == "requests": @@ -10,14 +13,18 @@ def formatMessage(self, record): """ ---------------- request ---------------- {req.method} {req.url} + {reqhdrs} ---------------- response ---------------- {res.status_code} {res.reason} {res.url} + {reshdrs} """ ).format( req=record.req, res=record.res, + reqhdrs=self._formatHeaders(record.req.headers), + reshdrs=self._formatHeaders(record.res.headers), ) return result diff --git a/src/log_outgoing_requests/handlers.py b/src/log_outgoing_requests/handlers.py index 9158307df7..631f14761a 100644 --- a/src/log_outgoing_requests/handlers.py +++ b/src/log_outgoing_requests/handlers.py @@ -14,11 +14,15 @@ def emit(self, record): # save only the requests coming from the library requests if record and record.getMessage() == "Outgoing request": + safe_req_headers = record.req.headers.copy() + + if "Authorization" in safe_req_headers: + safe_req_headers["Authorization"] = "***hidden***" + if record.exc_info: trace = traceback.format_exc() parsed_url = urlparse(record.req.url) - kwargs = { "url": record.req.url, "hostname": parsed_url.hostname, @@ -29,7 +33,12 @@ def emit(self, record): "res_content_type": record.res.headers.get("Content-Type", ""), "timestamp": record.requested_at, "response_ms": int(record.res.elapsed.total_seconds() * 1000), + "req_headers": self.format_headers(safe_req_headers), + "res_headers": self.format_headers(record.res.headers), "trace": trace, } OutgoingRequestsLog.objects.create(**kwargs) + + def format_headers(self, headers): + return "\n".join(f"{k}: {v}" for k, v in headers.items()) diff --git a/src/log_outgoing_requests/log_requests.py b/src/log_outgoing_requests/log_requests.py index ea994ab9ae..bac831275e 100644 --- a/src/log_outgoing_requests/log_requests.py +++ b/src/log_outgoing_requests/log_requests.py @@ -30,7 +30,7 @@ def install_outgoing_requests_logging(): Session._original_request = Session.request def new_request(self, *args, **kwargs): - kwargs.setdefault("hooks", {"response": hook_requests_logging}) + self.hooks["response"].append(hook_requests_logging) return self._original_request(*args, **kwargs) Session.request = new_request diff --git a/src/log_outgoing_requests/migrations/0003_auto_20230321_0724.py b/src/log_outgoing_requests/migrations/0003_auto_20230321_0724.py new file mode 100644 index 0000000000..673313a1e8 --- /dev/null +++ b/src/log_outgoing_requests/migrations/0003_auto_20230321_0724.py @@ -0,0 +1,33 @@ +# Generated by Django 3.2.15 on 2023-03-21 06:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("log_outgoing_requests", "0002_alter_outgoingrequestslog_url"), + ] + + operations = [ + migrations.AddField( + model_name="outgoingrequestslog", + name="req_headers", + field=models.TextField( + blank=True, + help_text="The request headers.", + null=True, + verbose_name="Request headers", + ), + ), + migrations.AddField( + model_name="outgoingrequestslog", + name="res_headers", + field=models.TextField( + blank=True, + help_text="The response headers.", + null=True, + verbose_name="Response headers", + ), + ), + ] diff --git a/src/log_outgoing_requests/models.py b/src/log_outgoing_requests/models.py index a9f1e658b5..215e481a5c 100644 --- a/src/log_outgoing_requests/models.py +++ b/src/log_outgoing_requests/models.py @@ -13,6 +13,8 @@ class OutgoingRequestsLog(models.Model): default="", help_text=_("The url of the outgoing request."), ) + + # hostname is added so we can filter on it in the admin page hostname = models.CharField( verbose_name=_("Hostname"), max_length=255, @@ -52,6 +54,18 @@ class OutgoingRequestsLog(models.Model): blank=True, help_text=_("The content type of the response."), ) + req_headers = models.TextField( + verbose_name=_("Request headers"), + blank=True, + null=True, + help_text=_("The request headers."), + ) + res_headers = models.TextField( + verbose_name=_("Response headers"), + blank=True, + null=True, + help_text=_("The response headers."), + ) response_ms = models.PositiveIntegerField( verbose_name=_("Response in ms"), default=0, @@ -79,6 +93,9 @@ def __str__(self): ) @cached_property + def url_parsed(self): + return urlparse(self.url) + + @property def query_params(self): - parsed_url = urlparse(self.url) - return parsed_url.query + return self.url_parsed.query diff --git a/src/log_outgoing_requests/tests/test_logging.py b/src/log_outgoing_requests/tests/test_logging.py index 368d6f0da2..2bd094b059 100644 --- a/src/log_outgoing_requests/tests/test_logging.py +++ b/src/log_outgoing_requests/tests/test_logging.py @@ -17,6 +17,7 @@ def _setUpMocks(self, m): content=b"some content", ) + @override_settings(LOG_OUTGOING_REQUESTS_DB_SAVE=True) def test_outgoing_requests_are_logged(self, m): self._setUpMocks(m) @@ -30,9 +31,8 @@ def test_outgoing_requests_are_logged(self, m): @override_settings(LOG_OUTGOING_REQUESTS_DB_SAVE=True) def test_expected_data_is_saved_when_saving_enabled(self, m): - self._setUpMocks(m) - methods = [ + ("GET", requests.get, m.get), ("POST", requests.post, m.post), ("PUT", requests.put, m.put), ("PATCH", requests.patch, m.patch), @@ -45,10 +45,31 @@ def test_expected_data_is_saved_when_saving_enabled(self, m): mocked( "http://example.com/some-path?version=2.0", status_code=200, - content=b"some content", + json={"test": "data"}, + request_headers={ + "Authorization": "test", + "Content-Type": "text/html", + }, + headers={ + "Date": "Tue, 21 Mar 2023 15:24:08 GMT", + "Content-Type": "application/json", + }, + ) + expected_req_headers = ( + "User-Agent: python-requests/2.26.0\n" + "Accept-Encoding: gzip, deflate, br\n" + "Accept: */*\n" + "Connection: keep-alive\n" + "Authorization: ***hidden***\n" + "Content-Type: text/html" ) + if method not in ["HEAD", "GET"]: + expected_req_headers += "\nContent-Length: 0" - response = func("http://example.com/some-path?version=2.0") + response = func( + "http://example.com/some-path?version=2.0", + headers={"Authorization": "test", "Content-Type": "text/html"}, + ) request_log = OutgoingRequestsLog.objects.last() @@ -60,15 +81,33 @@ def test_expected_data_is_saved_when_saving_enabled(self, m): self.assertEqual(request_log.query_params, "version=2.0") self.assertEqual(response.status_code, 200) self.assertEqual(request_log.method, method) - self.assertEqual(request_log.req_content_type, "") - self.assertEqual(request_log.res_content_type, "") + self.assertEqual(request_log.req_content_type, "text/html") + self.assertEqual(request_log.res_content_type, "application/json") self.assertEqual(request_log.response_ms, 0) + self.assertEqual(request_log.req_headers, expected_req_headers) + self.assertEqual( + request_log.res_headers, + "Date: Tue, 21 Mar 2023 15:24:08 GMT\nContent-Type: application/json", + ) self.assertEqual( request_log.timestamp.strftime("%Y-%m-%d %H:%M:%S"), "2021-10-18 13:00:00", ) self.assertIsNone(request_log.trace) + @override_settings(LOG_OUTGOING_REQUESTS_DB_SAVE=True) + def test_authorization_header_is_hidden(self, m): + self._setUpMocks(m) + + requests.get( + "http://example.com/some-path?version=2.0", + headers={"Authorization": "test"}, + ) + log = OutgoingRequestsLog.objects.get() + + self.assertIn("Authorization: ***hidden***", log.req_headers) + + @override_settings(LOG_OUTGOING_REQUESTS_DB_SAVE=False) def test_data_is_not_saved_when_saving_disabled(self, m): self._setUpMocks(m) diff --git a/src/open_inwoner/accounts/models.py b/src/open_inwoner/accounts/models.py index 38a0eabd06..61a00c1a22 100644 --- a/src/open_inwoner/accounts/models.py +++ b/src/open_inwoner/accounts/models.py @@ -22,6 +22,7 @@ validate_phone_number, ) +from ..plans.models import PlanContact from .choices import ContactTypeChoices, LoginTypeChoices, StatusChoices, TypeChoices from .managers import ActionQueryset, DigidManager, UserManager, eHerkenningManager from .query import InviteQuerySet, MessageQuerySet @@ -284,6 +285,16 @@ def is_email_of_contact(self, email): or self.contacts_for_approval.filter(email=email).exists() ) + def get_plan_contact_new_count(self): + return ( + PlanContact.objects.filter(user=self, notify_new=True) + .exclude(plan__created_by=self) + .count() + ) + + def clear_plan_contact_new_count(self): + PlanContact.objects.filter(user=self).update(notify_new=False) + def is_digid_and_brp(self) -> bool: """ Returns whether user is logged in with digid and data has diff --git a/src/open_inwoner/accounts/templates/accounts/registration_necessary.html b/src/open_inwoner/accounts/templates/accounts/registration_necessary.html index 704fe25203..7094d3ccc3 100644 --- a/src/open_inwoner/accounts/templates/accounts/registration_necessary.html +++ b/src/open_inwoner/accounts/templates/accounts/registration_necessary.html @@ -3,6 +3,7 @@ {% block content %} +
{% render_grid %} {% render_column span=9 %} {% render_card tinted=True %} @@ -11,5 +12,6 @@

{% trans "Registratie voltooien" %}


{% endrender_card %} {% endrender_column %} {% endrender_grid %} +
{% endblock content %} diff --git a/src/open_inwoner/accounts/templates/django_registration/registration_form.html b/src/open_inwoner/accounts/templates/django_registration/registration_form.html index deb2a21166..3cf607e0c5 100644 --- a/src/open_inwoner/accounts/templates/django_registration/registration_form.html +++ b/src/open_inwoner/accounts/templates/django_registration/registration_form.html @@ -3,6 +3,7 @@ {% block content %} +
{% render_grid %} {% render_column start=5 span=5 %} {% render_card direction='horizontal' tinted=True %} @@ -23,5 +24,6 @@

{% trans "Registreren met E-mail" %}


{% endif %} {% endrender_grid %} +
{% endblock content %} diff --git a/src/open_inwoner/accounts/templates/registration/login.html b/src/open_inwoner/accounts/templates/registration/login.html index d319bb6f6c..d225c35a03 100644 --- a/src/open_inwoner/accounts/templates/registration/login.html +++ b/src/open_inwoner/accounts/templates/registration/login.html @@ -10,6 +10,7 @@ {% block content %} +
{% render_grid %} {% render_column start=5 span=5 %} {% render_card %} @@ -21,10 +22,13 @@

{% trans 'Welkom' %}

- {% link href='digid:login' text=_('Inloggen met DigiD') secondary=True icon='arrow_forward' extra_classes="link--digid" %} + {% url 'digid:login' as href %} + {% with href|addnexturl:next as href_with_next %} + {% link href=href_with_next text=_('Inloggen met DigiD') secondary=True icon='arrow_forward' extra_classes="link--digid" %} + {% endwith %} {% endrender_card %} {% endif %} - + {% get_solo 'mozilla_django_oidc_db.OpenIDConnectConfig' as oidc_config %} {% get_solo 'configurations.SiteConfiguration' as site_config %} {% if oidc_config.enabled %} @@ -36,7 +40,10 @@

{% trans 'Welkom' %}

{% else %}
{% endif %} - {% link text=site_config.openid_connect_login_text href='oidc_authentication_init' secondary=True icon='arrow_forward' icon_position="after" %} + {% url 'oidc_authentication_init' as href %} + {% with href|addnexturl:next as href_with_next %} + {% link text=site_config.openid_connect_login_text href=href_with_next secondary=True icon='arrow_forward' icon_position="after" %} + {% endwith %} {% endrender_card %} {% endif %} @@ -44,6 +51,7 @@

{% trans 'Welkom' %}

{% render_card tinted=True %} {% render_form id="login-form" method="POST" form=form %} {% csrf_token %} + {% input form.username %} {% input form.password %} {% button text=_('Wachtwoord vergeten?') href='password_reset' secondary=True transparent=True align='right' %} @@ -54,4 +62,5 @@

{% trans 'Welkom' %}

{% endrender_card %} {% endrender_column %} {% endrender_grid %} +
{% endblock content %} diff --git a/src/open_inwoner/accounts/tests/test_action_views.py b/src/open_inwoner/accounts/tests/test_action_views.py index 4b8b7c4f41..ad090df9b9 100644 --- a/src/open_inwoner/accounts/tests/test_action_views.py +++ b/src/open_inwoner/accounts/tests/test_action_views.py @@ -227,6 +227,12 @@ def test_action_history(self): self.assertEqual(response.status_code, 200) self.assertContains(response, self.action.name) + def test_action_history_breadcrumbs(self): + response = self.app.get(self.history_url, user=self.user) + crumbs = response.pyquery(".breadcrumbs__list-item") + self.assertIn(_("Mijn profiel"), crumbs[1].text_content()) + self.assertIn(_("Mijn acties"), crumbs[2].text_content()) + def test_action_history_not_your_action(self): other_user = UserFactory() self.app.get(self.history_url, user=other_user, status=404) diff --git a/src/open_inwoner/accounts/tests/test_auth.py b/src/open_inwoner/accounts/tests/test_auth.py index 71218d86f1..24b26fb049 100644 --- a/src/open_inwoner/accounts/tests/test_auth.py +++ b/src/open_inwoner/accounts/tests/test_auth.py @@ -819,6 +819,17 @@ def setUp(self): self.config.login_allow_registration = True self.config.save() + def test_login_page_has_next_url(self): + response = self.app.get(reverse("accounts:contact_list")) + self.assertRedirects( + response, + furl(reverse("login")).add({"next": reverse("accounts:contact_list")}).url, + ) + self.assertIn( + furl("").add({"next": reverse("accounts:contact_list")}).url, + response.follow(), + ) + def test_login(self): """Test that a user is successfully logged in.""" form = self.app.get(reverse("login")).forms["login-form"] diff --git a/src/open_inwoner/accounts/tests/test_contact_views.py b/src/open_inwoner/accounts/tests/test_contact_views.py index 84ff2db128..9edd9241d9 100644 --- a/src/open_inwoner/accounts/tests/test_contact_views.py +++ b/src/open_inwoner/accounts/tests/test_contact_views.py @@ -1,9 +1,12 @@ +import io + from django.core import mail +from django.core.files.images import ImageFile from django.urls import reverse from django.utils.translation import ugettext_lazy as _ from django_webtest import WebTest -from furl import furl +from PIL import Image from open_inwoner.accounts.models import User @@ -429,3 +432,43 @@ def test_notification_email_for_approval_is_not_sent_when_new_user(self): # Email should be the one for registration, not for approval self.assertEqual(email.subject, "Uitnodiging voor Open Inwoner Platform") + + def test_contacts_image_is_shown_in_contact_approval_section(self): + # prepare image + image = Image.new("RGB", (10, 10)) + byteIO = io.BytesIO() + image.save(byteIO, format="png") + img_bytes = byteIO.getvalue() + image = ImageFile(io.BytesIO(img_bytes), name="foo.jpg") + + # update user's type and image + existing_user = UserFactory( + email="ex@example.com", + contact_type=ContactTypeChoices.begeleider, + image=image, + ) + self.user.contact_type = ContactTypeChoices.begeleider + self.user.image = image + self.user.save() + self.user.contacts_for_approval.add(existing_user) + + # Receiver contact list page + response = self.app.get(self.list_url, user=existing_user) + avatar_class = response.pyquery(".avatar") + + self.assertIn(self.user.image.name, avatar_class[0].getchildren()[0].get("src")) + + def test_no_image_is_shown_in_contact_approval_section_when_no_image_set(self): + # update user's type + existing_user = UserFactory( + email="ex@example.com", contact_type=ContactTypeChoices.begeleider + ) + self.user.contact_type = ContactTypeChoices.begeleider + self.user.save() + self.user.contacts_for_approval.add(existing_user) + + # Receiver contact list page + response = self.app.get(self.list_url, user=existing_user) + avatar_class = response.pyquery(".avatar") + + self.assertFalse(avatar_class) diff --git a/src/open_inwoner/accounts/tests/test_user.py b/src/open_inwoner/accounts/tests/test_user.py index dede3fc7c4..cf14320d86 100644 --- a/src/open_inwoner/accounts/tests/test_user.py +++ b/src/open_inwoner/accounts/tests/test_user.py @@ -7,6 +7,7 @@ from open_inwoner.accounts.choices import LoginTypeChoices from open_inwoner.utils.hash import generate_email_from_string +from ...plans.tests.factories import PlanFactory from ..models import User from .factories import UserFactory @@ -73,3 +74,20 @@ def test_require_necessary_fields_oidc_openinwoner_email(self): login_type=LoginTypeChoices.oidc, email="test@example.org", oidc_id="test" ) self.assertTrue(user.require_necessary_fields()) + + def test_plan_contact_new_count_methods(self): + owner = UserFactory() + plan_1 = PlanFactory(created_by=owner) + plan_2 = PlanFactory(created_by=owner) + + user = UserFactory() + self.assertEqual(0, user.get_plan_contact_new_count()) + + plan_1.plan_contacts.add(user) + self.assertEqual(1, user.get_plan_contact_new_count()) + + plan_2.plan_contacts.add(user) + self.assertEqual(2, user.get_plan_contact_new_count()) + + user.clear_plan_contact_new_count() + self.assertEqual(0, user.get_plan_contact_new_count()) diff --git a/src/open_inwoner/accounts/tests/test_user_manager.py b/src/open_inwoner/accounts/tests/test_user_manager.py index 68e2527850..1ff629400b 100644 --- a/src/open_inwoner/accounts/tests/test_user_manager.py +++ b/src/open_inwoner/accounts/tests/test_user_manager.py @@ -34,4 +34,4 @@ def test_having_usable_email(self): UserFactory(first_name="empty", email="") actual = User.objects.having_usable_email() - self.assertEqual(list(actual), expected) + self.assertEqual(set(list(actual)), set(expected)) diff --git a/src/open_inwoner/accounts/views/cases.py b/src/open_inwoner/accounts/views/cases.py index e55e6ff134..ad38d8586b 100644 --- a/src/open_inwoner/accounts/views/cases.py +++ b/src/open_inwoner/accounts/views/cases.py @@ -397,7 +397,7 @@ def get_result_display(self, case: Zaak) -> str: result = fetch_single_result(case.resultaat) if result: return result.toelichting - return _("Onbekend") + return None def get_initiator_display(self, case: Zaak) -> str: roles = fetch_case_roles(case.url, RolOmschrijving.initiator) diff --git a/src/open_inwoner/components/templates/components/Action/Actions.html b/src/open_inwoner/components/templates/components/Action/Actions.html index d3489fa4b7..435d94e00b 100644 --- a/src/open_inwoner/components/templates/components/Action/Actions.html +++ b/src/open_inwoner/components/templates/components/Action/Actions.html @@ -26,7 +26,12 @@ {% button icon="edit" text=_("Bewerken") href=action_url icon_outlined=True transparent=True %}