Merge pull request #2108 from maxberger/master

UI: Add support for usernames with @ symbol

integ_tests/common.py

@@ -47,6 +47,8 @@ class Config:
     auth_type: AuthType
     sharing_type: SharingType
     extra_config: str = ""
+    admin_username: str = "admin"
+    user_username: str = "max"
 
 
 SHARING_HTPASSWD = Config(
@@ -55,6 +57,14 @@ SHARING_HTPASSWD = Config(
     sharing_type=SharingType.SHARING,
 )
 
+SHARING_HTPASSWD_USERSWITHDOMAIN = Config(
+    name="sharing_htpasswd_userswithdomain",
+    auth_type=AuthType.HTPASSWD,
+    sharing_type=SharingType.SHARING,
+    admin_username="admin@domain.tld",
+    user_username="max@domain.tld",
+)
+
 SHARING_XREMOTE = Config(
     name="sharing_xremote",
     auth_type=AuthType.XREMOTE,
@@ -122,12 +132,8 @@ database_path = {sharing_path}
 
     if config.auth_type == AuthType.HTPASSWD:
         with open(user_path, "w") as f:
-            f.write(
-                """admin:admi$pass#word
-max:maxpassword
-
-"""
-            )
+            f.write(f"{config.admin_username}:admi$pass#word\n")
+            f.write(f"{config.user_username}:userpassword\n")
 
     env = os.environ.copy()
     # Ensure the radicale package is in PYTHONPATH
@@ -182,7 +188,7 @@ def login(
     page.goto(radicale_server)
 
     if config.auth_type == AuthType.HTPASSWD:
-        page.fill('#loginscene input[data-name="user"]', "admin")
+        page.fill('#loginscene input[data-name="user"]', config.admin_username)
         page.fill('#loginscene input[data-name="password"]', "admi$pass#word")
         page.click('button:has-text("Next")')
 

integ_tests/test_sharing_login.py

@@ -25,25 +25,38 @@ from typing import Any, Generator
 import pytest
 from playwright.sync_api import Page, expect
 
-from integ_tests.common import (SHARING_HTPASSWD, create_collection, login,
+from integ_tests.common import (SHARING_HTPASSWD,
+                                SHARING_HTPASSWD_USERSWITHDOMAIN, Config,
+                                create_collection, login,
                                 start_radicale_server)
 
 
+@pytest.fixture(params=[SHARING_HTPASSWD, SHARING_HTPASSWD_USERSWITHDOMAIN])
+def radicale_server_config(request: pytest.FixtureRequest) -> Config:
+    return request.param
+
+
 @pytest.fixture
-def radicale_server(tmp_path: pathlib.Path) -> Generator[str, Any, None]:
-    yield from start_radicale_server(tmp_path, SHARING_HTPASSWD)
+def radicale_server(
+    tmp_path: pathlib.Path, radicale_server_config: Config
+) -> Generator[str, Any, None]:
+    yield from start_radicale_server(tmp_path, radicale_server_config)
 
 
 @pytest.mark.parametrize("permissions", ["ro", "rw"])
-def test_incoming_shares(page: Page, radicale_server: str, permissions: str) -> None:
+def test_incoming_shares(
+    page: Page, radicale_server: str, radicale_server_config: Config, permissions: str
+) -> None:
     # 1. Admin logs in and creates a map share for 'max'
-    login(page, radicale_server, SHARING_HTPASSWD)
+    login(page, radicale_server, radicale_server_config)
     create_collection(page, radicale_server)
 
     page.hover("article:not(.hidden)")
     page.click('article:not(.hidden) a[data-name="share"]', force=True, strict=True)
     page.click('button[data-name="sharebymap"]')
-    page.locator('input[data-name="shareuser"]').fill("max")
+    page.locator('input[data-name="shareuser"]').fill(
+        radicale_server_config.user_username
+    )
     page.locator('input[data-name="sharehref"]').fill("mapped")
     if permissions == "rw":
         page.check("#newshare_attr_permissions_rw")
@@ -57,8 +70,10 @@ def test_incoming_shares(page: Page, radicale_server: str, permissions: str) ->
     page.click('a[data-name="logout"]')
 
     # 3. Max logs in
-    page.fill('#loginscene input[data-name="user"]', "max")
-    page.fill('#loginscene input[data-name="password"]', "maxpassword")
+    page.fill(
+        '#loginscene input[data-name="user"]', radicale_server_config.user_username
+    )
+    page.fill('#loginscene input[data-name="password"]', "userpassword")
     page.click('button:has-text("Next")')
 
     # 4. Max sees the incoming share
@@ -113,12 +128,14 @@ def test_incoming_shares(page: Page, radicale_server: str, permissions: str) ->
     ).to_be_checked()
 
     # 6. Verify "shared by admin" and button visibility in the collection article
-    page.click('#incomingsharingscene button[data-name="cancel"]')
+    page.click('#incomingsharingscene button[data-name="close"]')
     expect(page.locator("#incomingsharingscene")).to_be_hidden()
 
     article = page.locator("article:not(.hidden)").first
     expect(article.locator('[data-name="shared-by"]')).to_be_visible()
-    expect(article.locator('[data-name="shared-by-owner"]')).to_have_text("admin")
+    expect(article.locator('[data-name="shared-by-owner"]')).to_have_text(
+        radicale_server_config.admin_username
+    )
 
     # Action buttons are only visible on mouseover
     article.hover()
@@ -138,11 +155,15 @@ def test_incoming_shares(page: Page, radicale_server: str, permissions: str) ->
     expect(page.locator('#incomingsharingscene span[data-name="error"]')).to_be_hidden()
 
 
-def test_no_incoming_shares_message(page: Page, radicale_server: str) -> None:
+def test_no_incoming_shares_message(
+    page: Page, radicale_server: str, radicale_server_config: Config
+) -> None:
     # 1. Max logs in
     page.goto(radicale_server)
-    page.fill('#loginscene input[data-name="user"]', "max")
-    page.fill('#loginscene input[data-name="password"]', "maxpassword")
+    page.fill(
+        '#loginscene input[data-name="user"]', radicale_server_config.user_username
+    )
+    page.fill('#loginscene input[data-name="password"]', "userpassword")
     page.click('button:has-text("Next")')
 
     # 2. Max goes to incoming shares scene
@@ -158,5 +179,5 @@ def test_no_incoming_shares_message(page: Page, radicale_server: str) -> None:
         page.locator('#incomingsharingscene [data-name="nosharesmessage"]')
     ).to_have_text("No incoming shares")
 
-    page.click('#incomingsharingscene button[data-name="cancel"]')
+    page.click('#incomingsharingscene button[data-name="close"]')
     expect(page.locator("#incomingsharingscene")).to_be_hidden()

radicale/web/internal_data/index.html

@@ -260,7 +260,7 @@
       </table>
       <p class="hidden" data-name="nosharesmessage">No incoming shares</p>
       <form>
-        <button type="button" class="green" data-name="cancel">Close</button>
+        <button type="button" class="green" data-name="close">Close</button>
       </form>
       <span class="error hidden" data-name="error"></span>
     </section>

radicale/web/internal_data/js/api/sharing.js

@@ -170,7 +170,7 @@ export class Share {
  * @param {function(Array<Share>, ?string):void} callback
  */
 export function reload_sharing_list(user, password, collection, callback) {
-    let body = collection ? { PathMapped: collection.href } : {};
+    let body = collection ? { PathMapped: decodeURIComponent(collection.href) } : {};
     return call_sharing_api(
         user,
         password,
@@ -241,7 +241,7 @@ export function add_share_by_token(
         password,
         "token/create",
         {
-            PathMapped: share.PathMapped,
+            PathMapped: decodeURIComponent(share.PathMapped),
             Permissions: share.Permissions,
             Enabled: share.EnabledByOwner,
             Hidden: share.HiddenByOwner,
@@ -280,13 +280,13 @@ export function add_share_by_map(
         password,
         "map/create",
         {
-            PathMapped: share.PathMapped,
+            PathMapped: decodeURIComponent(share.PathMapped),
             Permissions: share.Permissions,
             Enabled: share.EnabledByOwner,
             Hidden: share.HiddenByOwner,
             Properties: share.Properties,
             User: share.User,
-            PathOrToken: share.PathOrToken,
+            PathOrToken: decodeURIComponent(share.PathOrToken),
             Conversion: share.Conversion,
         },
         function (response) {
@@ -320,7 +320,7 @@ export function delete_share_by_token(
         user,
         password,
         "token/delete",
-        { PathOrToken: share.PathOrToken },
+        { PathOrToken: decodeURIComponent(share.PathOrToken) },
         function (response) {
             let json_response = JSON.parse(response);
             if (json_response["Status"] !== "success") {
@@ -352,7 +352,7 @@ export function delete_share_by_map(
         user,
         password,
         "map/delete",
-        { PathOrToken: share.PathOrToken },
+        { PathOrToken: decodeURIComponent(share.PathOrToken) },
         function (response) {
             let json_response = JSON.parse(response);
             if (json_response["Status"] !== "success") {
@@ -384,7 +384,7 @@ export function update_share_by_token(
         password,
         "token/update",
         {
-            PathOrToken: share.PathOrToken,
+            PathOrToken: decodeURIComponent(share.PathOrToken),
             Permissions: share.Permissions,
             Enabled: share.EnabledByOwner,
             Hidden: share.HiddenByOwner,
@@ -423,8 +423,8 @@ export function update_share_by_map(
         password,
         "map/update",
         {
-            PathOrToken: share.PathOrToken,
-            PathMapped: share.PathMapped,
+            PathOrToken: decodeURIComponent(share.PathOrToken),
+            PathMapped: decodeURIComponent(share.PathMapped),
             User: share.User,
             Permissions: share.Permissions,
             Enabled: share.EnabledByOwner,
@@ -466,7 +466,7 @@ export function update_incoming_share(
         password,
         share.ShareType + "/update",
         {
-            PathOrToken: share.PathOrToken,
+            PathOrToken: decodeURIComponent(share.PathOrToken),
             Enabled: share.EnabledByUser,
             Hidden: share.HiddenByUser,
         },

radicale/web/internal_data/js/scenes/CollectionsScene.js

@@ -202,7 +202,7 @@ export class CollectionsScene {
             let transformed_from = get_element(node, "[data-name=transformed-from]");
             let share = (shares || []).find(
                 s => (s.ShareType === "map") &&
-                    (s.PathOrToken || "").replace(/\/+$/, "") === (collection.href || "").replace(/\/+$/, ""));
+                    decodeURIComponent(s.PathOrToken || "").replace(/\/+$/, "") === decodeURIComponent(collection.href || "").replace(/\/+$/, ""));
             if (share) {
                 if (share.Owner !== this._user) {
                     share_info.classList.remove("hidden");

radicale/web/internal_data/js/scenes/CreateEditShareScene.js

@@ -46,7 +46,7 @@ export class CreateEditShareScene {
         this._shareType = shareType;
         this._share = share;
         this._edit = !!share;
-        this._pathMapped = collection.href;
+        this._pathMapped = decodeURIComponent(collection.href);
 
         this._html_scene = get_element_by_id("createeditsharescene");
         this._title = get_element(this._html_scene, "[data-name=title]");

radicale/web/internal_data/js/scenes/IncomingSharingScene.js

@@ -40,7 +40,7 @@ export class IncomingSharingScene {
         this._password = password;
 
         this._html_scene = get_element_by_id("incomingsharingscene");
-        this._cancel_btn = get_element(this._html_scene, "[data-name=cancel]");
+        this._close_btn = get_element(this._html_scene, "[data-name=close]");
         this._error_element = get_element(this._html_scene, "[data-name=error]");
         this._tbody = get_element(this._html_scene, "tbody[data-name=incomingsharesbody]");
         this._template = get_element(this._tbody, "[data-name=incomingsharerowtemplate]");
@@ -97,10 +97,10 @@ export class IncomingSharingScene {
         });
         this._nodes = [];
 
-        let prefix = "/" + this._user + "/";
+        let prefix = "/" + decodeURIComponent(this._user) + "/";
         let filtered_shares = shares.filter(
             share => (share.ShareType === "map")
-                && share.PathOrToken.startsWith(prefix));
+                && decodeURIComponent(share.PathOrToken).startsWith(prefix));
 
         if (filtered_shares.length === 0) {
             this._table.classList.add("hidden");
@@ -144,14 +144,14 @@ export class IncomingSharingScene {
 
     show() {
         this._html_scene.classList.remove("hidden");
-        this._cancel_btn.onclick = () => pop_scene();
+        this._close_btn.onclick = () => pop_scene();
         this._error_handler.clearError();
         collectionsCache.getIncomingShares(this._user, this._password, this._error_handler.setError, (shares) => this._render_shares(shares));
     }
 
     hide() {
         this._html_scene.classList.add("hidden");
-        this._cancel_btn.onclick = null;
+        this._close_btn.onclick = null;
         this._error_handler.clearError();
     }
 

radicale/web/internal_data/js/scenes/ShareCollectionScene.js

@@ -226,9 +226,13 @@ function add_share_rows(user, password, collection, shares, errorHandler) {
   shares.forEach(function (share) {
     let pathortoken = share["PathOrToken"] || "";
     let pathmapped = share["PathMapped"] || "";
+    let decodedHref = decodeURIComponent(collection.href).replace(/\/+$/, "") + "/";
+    let decodedPathMapped = decodeURIComponent(pathmapped).replace(/\/+$/, "") + "/";
+    let decodedPathOrToken = decodeURIComponent(pathortoken).replace(/\/+$/, "") + "/";
+
     if (
-      collection.href.includes(pathmapped) ||
-      collection.href.includes(pathortoken)
+      decodedHref.includes(decodedPathMapped) ||
+      decodedHref.includes(decodedPathOrToken)
     ) {
       if (share["ShareType"] === "token") {
         add_share_row_node(user, password, collection, share, token_template, "share", delete_share_by_token, errorHandler);

radicale/web/internal_data/js/utils/misc.js

@@ -56,9 +56,9 @@ export function random_hex(length) {
  * @param {HTMLInputElement} href_form A valid Input element or an onchange Event of an Input element.
  */
 export function cleanHREFinput(href_form) {
-    let currentTxtVal = href_form.value.trim().toLowerCase();
+    let currentTxtVal = href_form.value.trim()
     //Clean the HREF to remove not permitted chars
-    currentTxtVal = currentTxtVal.replace(/(?![0-9a-z\-\_\.])./g, '');
+    currentTxtVal = currentTxtVal.replace(/(?![0-9a-zA-Z\-\_\.\@])./g, '');
     //Clean the HREF to remove leading . (would result in hidden directory)
     currentTxtVal = currentTxtVal.replace(/^\./, '');
     href_form.value = currentTxtVal;