Merge pull request #21655 from nuwang/improve_oidc_username_tests

Add tests for oidc usernames

Author: jmchilton

test/integration/oidc/galaxy-realm-export.json

@@ -483,6 +483,40 @@
       "notBefore": 0,
       "groups": []
     },
+    {
+      "id": "f5e8b2c4-9a1d-4e3f-8c6a-7b2d5e4f3c1a",
+      "createdTimestamp": 1694376671733,
+      "username": "rincewind_test",
+      "enabled": true,
+      "totp": false,
+      "emailVerified": true,
+      "firstName": "Rincewind",
+      "lastName": "Ankh-Morpork",
+      "email": "[email protected]",
+      "attributes": {
+        "preferred_username": [
+          "Rincewind (Ankh-Morpork)"
+        ]
+      },
+      "credentials": [
+        {
+          "id": "e5d4c3b2-a1f0-9e8d-7c6b-5a4f3e2d1c0b",
+          "type": "password",
+          "userLabel": "My password",
+          "createdDate": 1694376754826,
+          "secretData": "{\"value\":\"uNBI+UnpCLpXWHhm/tPSnnhuINiNw2MNt1XeDmImJaQ=\",\"salt\":\"fHS/FpnORylnSIco16UHwA==\",\"additionalParameters\":{}}",
+          "credentialData": "{\"hashIterations\":27500,\"algorithm\":\"pbkdf2-sha256\",\"additionalParameters\":{}}"
+        }
+      ],
+      "disableableCredentialTypes": [],
+      "requiredActions": [],
+      "realmRoles": [
+        "galaxy-access-role",
+        "default-roles-gxyrealm"
+      ],
+      "notBefore": 0,
+      "groups": []
+    },
     {
       "id": "24ffa3ff-d351-4d5e-b10b-8d615082ec9d",
       "createdTimestamp": 1694376671733,
@@ -2565,4 +2599,4 @@
   "clientPolicies": {
     "policies": []
   }
-}
+}

test/integration/oidc/test_auth_oidc.py

@@ -261,6 +261,24 @@ def test_oidc_login_new_user(self):
         self._assert_status_code_is(response, 200)
         assert response.json()["email"] == "[email protected]"
 
+    def test_oidc_login_username_sanitization(self):
+        """Test that OIDC usernames with special characters are properly sanitized."""
+        _, response = self._login_via_keycloak("rincewind_test", KEYCLOAK_TEST_PASSWORD, save_cookies=True)
+
+        response = self._get("users/current")
+        self._assert_status_code_is(response, 200)
+        assert response.json()["email"] == "[email protected]"
+
+        username = response.json()["username"]
+        from galaxy.security.validate_user_input import validate_publicname_str
+
+        error = validate_publicname_str(username)
+        assert error == "", f"OIDC-created username '{username}' is invalid: {error}"
+        assert "(" not in username, f"Username '{username}' should not contain parentheses"
+        assert ")" not in username, f"Username '{username}' should not contain parentheses"
+        assert " " not in username, f"Username '{username}' should not contain spaces"
+        assert username == username.lower(), f"Username '{username}' should be lowercase"
+
     def test_oidc_login_repeat_no_notification(self):
         """
         Test that repeat logins do NOT show the 'identity has been linked' notification.

test/unit/util/test_utils.py

@@ -177,3 +177,28 @@ def test_validate_doi_fail_too_long():
     doi = f"doi:10.1000/{long_suffix}"
     assert util.validate_doi(doi)
     assert not util.validate_doi(doi + "a")  # Increase length by 1 past max limit
+
+
+@pytest.mark.parametrize(
+    "input_name,expected_output",
+    [
+        # Existing documented behavior
+        ("My Cool Object", "My-Cool-Object"),
+        ("!My Cool Object!", "My-Cool-Object"),
+        ("Hello\u20a9\u25ce\u0491\u029f\u217e", "Hello"),
+        # Additional edge cases
+        ("simple", "simple"),
+        ("UPPERCASE", "UPPERCASE"),  # Note: lowercase applied separately
+        ("with-dash", "with-dash"),
+        ("with spaces", "with-spaces"),
+        ("  multiple   spaces  ", "-multiple-spaces"),
+        ("trailing!", "trailing"),
+        ("!leading", "leading"),
+        ("special@#$chars", "specialchars"),
+        ("parentheses(test)", "parenthesestest"),
+        ("Rincewind (Ankh-Morpork)", "Rincewind-Ankh-Morpork"),
+    ],
+)
+def test_ready_name_for_url(input_name, expected_output):
+    """Test that ready_name_for_url correctly sanitizes names for URL use."""
+    assert util.ready_name_for_url(input_name) == expected_output