From 15e9231ae3d7202065b510964f701332e52caa33 Mon Sep 17 00:00:00 2001
From: Andrew Ruthven <andrew@etc.gen.nz>
Date: Sat, 24 Feb 2024 23:50:50 +1300
Subject: [PATCH] Unescape URLs passed in as external bindings.

Previously PHP was escaping things like ampersands, which then caused
us to have broken URLs.

These aren't displayed anywhere in the UI, so no need to encode them anywhere.

Closes #314.
---
 inc/ui/principal-edit.php                     |  2 +-
 .../binding/1310-BIND-external-webui.result   | 26 +++++
 .../binding/1310-BIND-external-webui.test     | 97 +++++++++++++++++++
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 testing/tests/binding/1310-BIND-external-webui.result
 create mode 100644 testing/tests/binding/1310-BIND-external-webui.test

diff --git a/inc/ui/principal-edit.php b/inc/ui/principal-edit.php
index ec5c10ab..860e119f 100644
--- a/inc/ui/principal-edit.php
+++ b/inc/ui/principal-edit.php
@@ -937,7 +937,7 @@ function binding_row_editor() {
     $_POST['parent_container'] = $parent;
 
     //  external binds shouldn't ever point back to ourselves but they should be a valid http[s] url
-    $href = $_POST['source'];
+    $href = htmlspecialchars_decode($_POST['source']);
     if ( preg_match ( '{^(?:https?://|file:///)([^/]+)(:[0-9]\+)?/.+$}', $href, $matches )
         && strcasecmp( $matches[0], 'localhost' ) !== 0 && strcasecmp( $matches[0], '127.0.0.1' ) !== 0
         && strcasecmp( $matches[0], $_SERVER['SERVER_NAME'] ) !== 0 && strcasecmp( $matches[0], $_SERVER['SERVER_ADDR'] ) !== 0
diff --git a/testing/tests/binding/1310-BIND-external-webui.result b/testing/tests/binding/1310-BIND-external-webui.result
new file mode 100644
index 00000000..a0f645ad
--- /dev/null
+++ b/testing/tests/binding/1310-BIND-external-webui.result
@@ -0,0 +1,26 @@
+1..9
+# Subtest: Login
+    1..4
+    ok 1 - Fetch first page
+    ok 2 - Not logged in
+    ok 3 - Login to site
+    ok 4 - Logged in
+ok 1 - Login
+ok 2 - Correct users edit page
+ok 3 - Create bind into another users namespace - should fail
+ok 4 - Error present
+ok 5 - Submit updated fields
+ok 6 - Correct status message is present
+ok 7 - Binding is present
+ok 8 - Submit updated fields
+ok 9 - Correct error message is present
+
+            count: >0<
+
+          bind_id: >1192<
+  bound_source_id: >1057<
+  dav_displayname: >Moon Phases - user3<
+         dav_name: >/user3/moon/<
+     external_url: >http://regression/testfiles/mooncal.ics?lang=de&phases%5Bfull%5D=true&phases%5Bnew%5D=true&phases%5Bquarter%5D=true&phases%5Bdaily%5D=false&events%5Blunareclipse%5D=true&events%5Bsolareclipse%5D=true&events%5Bmoonlanding%5D=false&before=P6M&after=P2Y&zone=CET<
+ parent_container: >/user3/<
+
diff --git a/testing/tests/binding/1310-BIND-external-webui.test b/testing/tests/binding/1310-BIND-external-webui.test
new file mode 100644
index 00000000..3f904dfe
--- /dev/null
+++ b/testing/tests/binding/1310-BIND-external-webui.test
@@ -0,0 +1,97 @@
+MODE=TAP,9
+
+BEGINPERL
+
+my $mech;
+
+subtest 'Login' => sub {
+    plan tests => 4;
+
+    $mech = webui_login(
+        username => 'user3',
+        password => 'user3',
+        url      => "http://$webhost",
+    );
+};
+
+$mech->follow_link( text_regex => qr/View My Details/ );
+$mech->text_contains('Principal: User 3', 'Correct users edit page');
+
+# Create bind into another users namespace
+$mech->submit_form_ok(
+    {
+        form_number => 4,
+        button      => 'bindingrow',
+        fields      => {
+            dav_name        => '/user1/bogus',
+            dav_displayname => 'Bogus bind',
+            source          => 'http://regression/testfiles/bogus.ics',
+        },
+    }, "Create bind into another users namespace - should fail"
+);
+
+$mech->content_contains(
+    'Can only bind collections into the current principal\'s namespace',
+    'Error present');
+
+# Create bind
+$mech->submit_form_ok(
+    {
+        form_number => 4,
+        button      => 'bindingrow',
+        fields      => {
+            dav_name        => '/user3/moon',
+            dav_displayname => 'Moon Phases - user3',
+            source          => 'http://regression/testfiles/mooncal.ics?lang=de&phases%5Bfull%5D=true&phases%5Bnew%5D=true&phases%5Bquarter%5D=true&phases%5Bdaily%5D=false&events%5Blunareclipse%5D=true&events%5Bsolareclipse%5D=true&events%5Bmoonlanding%5D=false&before=P6M&after=P2Y&zone=CET',
+        },
+    }, "Submit updated fields"
+);
+
+$mech->save_content('/tmp/form.html');
+
+$mech->content_contains(
+    'Creating new binding for this principal',
+    'Correct status message is present');
+
+$mech->content_contains(
+    'Moon Phases - user3',
+    'Binding is present');
+
+# Create duplicate bind - should fail
+$mech->submit_form_ok(
+    {
+        form_number => 4,
+        button      => 'bindingrow',
+        fields      => {
+            dav_name        => '/user3/moon',
+            dav_displayname => 'Moon Phases - user3',
+            source          => 'http://regression/testfiles/bogus.ics',
+        },
+    }, "Submit updated fields"
+);
+
+$mech->content_contains(
+    'A resource already exists at the destination.',
+    'Correct error message is present');
+
+ENDPERL
+
+# Bogus bind isn't present - should be 0.
+BEGINQUERY
+SELECT count(*)
+FROM dav_binding
+WHERE dav_displayname = 'Bogus Bind'
+ENDQUERY
+
+# Bind is present with correct URL
+BEGINQUERY
+SELECT bind_id,
+       bound_source_id,
+       parent_container,
+       dav_name,
+       dav_displayname,
+       external_url
+FROM dav_binding
+WHERE dav_displayname = 'Moon Phases - user3'
+ORDER BY bind_id
+ENDQUERY