Skip to content

Commit

Permalink
🐛 [#4390] Fix URL conversion messing with templates
Browse files Browse the repository at this point in the history
The previous patch (183a08e) fixed
the reported behaviour by making all URLs fully qualified,
instead of building relative URLs to the admin if the domain
happens to be the same.

However, we have some 'URLs' that are template variables and
are substituted out by the backend, e.g. '{{ continue_url }}'
which got mangled by this patch by taking the current location
and using that as the root - the template fragment was being
interpreted as a relative URL. This breaks those templates.

This patch addresses both cases by just leaving URLs untouched
and doing no conversion at all - relative URLs stay literally
that (and if they're template fragments, our backend properly
resolves them), while absolute URLs (to another form, for
example) that are pasted in stay absolute.

This leaves real relative URLs ambiguous, but such URLs
don't make much sense in Open Forms anyway and we recommend
users to always use fully qualified absolute URLs.

Backport-of: #4391
  • Loading branch information
sergei-maertens committed Jun 14, 2024
1 parent 1180aba commit 42e5d54
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 2 deletions.
3 changes: 1 addition & 2 deletions src/openforms/conf/tinymce_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@
"default_link_target": "_blank",
"link_default_protocol": "https",
"link_assume_external_targets": true,
"relative_urls": false,
"remove_script_host": false
"convert_urls": false
}
66 changes: 66 additions & 0 deletions src/openforms/tests/e2e/test_tinymce_configuration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
from django.test import tag
from django.urls import reverse

from asgiref.sync import sync_to_async
from furl import furl
from playwright.async_api import expect

from openforms.config.models import GlobalConfiguration

from .base import E2ETestCase, browser_page, create_superuser


class TinyMCEConfigurationTests(E2ETestCase):

def setUp(self):
super().setUp()

self.addCleanup(GlobalConfiguration.clear_cache)

@tag("gh-4390")
async def test_link_handling(self):
"""
Test that hyperlinks in WYSIWYG content are handled appropriately.
1. Links with the same domain/prefix as where the admin is running must stay
absolute, and not be converted to relative paths. See #4368 for more
information.
2. Link targets may be template variables, like ``{{ continue_ url }}``, these
may not be post-processed and get prefixed with the (current) location to
make them absolute.
"""

@sync_to_async
def setUpTestData():
config = GlobalConfiguration.get_solo()
config.save_form_email_content_en = f"""
<p><a href="{self.live_server_url}/some-form-slug/">Go to form<a><p>
<p><a href="{{{{ some_variable }}}}">Variable link</a></p>
"""
config.save()

await setUpTestData()

admin_url = furl(self.live_server_url) / reverse(
"admin:config_globalconfiguration_change", args=(1,)
)

await create_superuser()
async with browser_page() as page:
await self._admin_login(page)
await page.goto(str(admin_url))

content_frame = page.frame_locator("#id_save_form_email_content_en_ifr")
await expect(content_frame.get_by_label("Rich Text Area.")).to_be_visible()

absolute_link = content_frame.get_by_role("link", name="Go to form")
await expect(absolute_link).to_be_visible()
await expect(absolute_link).to_have_attribute(
"data-mce-href", f"{self.live_server_url}/some-form-slug/"
)

variable_link = content_frame.get_by_role("link", name="Variable link")
await expect(variable_link).to_be_visible()
await expect(variable_link).to_have_attribute(
"data-mce-href", "{{ some_variable }}"
)

0 comments on commit 42e5d54

Please sign in to comment.