Fix JWT QSH generation for urls with repeated parameters (pycontribs#1157)

* Fix QshGenerator to combine repeated query string parameters into csv
* add test for QshGenerator.generate_qsh

Author: robinhughes

jira/client.py

@@ -37,7 +37,7 @@
     cast,
     no_type_check,
 )
-from urllib.parse import urlparse
+from urllib.parse import parse_qs, quote, urlparse
 
 import requests
 from pkg_resources import parse_version
@@ -177,19 +177,32 @@ def __init__(self, context_path):
         self.context_path = context_path
 
     def __call__(self, req):
+        qsh = self._generate_qsh(req)
+        return hashlib.sha256(qsh.encode("utf-8")).hexdigest()
+
+    def _generate_qsh(self, req):
         parse_result = urlparse(req.url)
 
         path = (
             parse_result.path[len(self.context_path) :]
             if len(self.context_path) > 1
             else parse_result.path
         )
-        # Per Atlassian docs, use %20 for whitespace when generating qsh for URL
-        # https://developer.atlassian.com/cloud/jira/platform/understanding-jwt/#qsh
-        query = "&".join(sorted(parse_result.query.split("&"))).replace("+", "%20")
+
+        # create canonical query string according to docs at:
+        # https://developer.atlassian.com/cloud/jira/platform/understanding-jwt-for-connect-apps/#qsh
+        params = parse_qs(parse_result.query, keep_blank_values=True)
+        joined = {
+            key: ",".join(self._sort_and_quote_values(params[key])) for key in params
+        }
+        query = "&".join(f"{key}={joined[key]}" for key in sorted(joined.keys()))
+
         qsh = f"{req.method.upper()}&{path}&{query}"
+        return qsh
 
-        return hashlib.sha256(qsh.encode("utf-8")).hexdigest()
+    def _sort_and_quote_values(self, values):
+        ordered_values = sorted(values)
+        return [quote(value, safe="~") for value in ordered_values]
 
 
 class JiraCookieAuth(AuthBase):

tests/test_qsh.py

@@ -0,0 +1,47 @@
+import pytest
+
+from jira.client import QshGenerator
+
+
+class MockRequest:
+    def __init__(self, method, url):
+        self.method = method
+        self.url = url
+
+
+@pytest.mark.parametrize(
+    "method,url,expected",
+    [
+        ("GET", "http://example.com", "GET&&"),
+        # empty parameter
+        ("GET", "http://example.com?key=&key2=A", "GET&&key=&key2=A"),
+        # whitespace
+        ("GET", "http://example.com?key=A+B", "GET&&key=A%20B"),
+        # tilde
+        ("GET", "http://example.com?key=A~B", "GET&&key=A~B"),
+        # repeated parameters
+        (
+            "GET",
+            "http://example.com?key2=Z&key1=X&key3=Y&key1=A",
+            "GET&&key1=A,X&key2=Z&key3=Y",
+        ),
+        # repeated parameters with whitespace
+        (
+            "GET",
+            "http://example.com?key2=Z+A&key1=X+B&key3=Y&key1=A+B",
+            "GET&&key1=A%20B,X%20B&key2=Z%20A&key3=Y",
+        ),
+    ],
+    ids=[
+        "no parameters",
+        "empty parameter",
+        "whitespace",
+        "tilde",
+        "repeated parameters",
+        "repeated parameters with whitespace",
+    ],
+)
+def test_qsh(method, url, expected):
+    gen = QshGenerator("http://example.com")
+    req = MockRequest(method, url)
+    assert gen._generate_qsh(req) == expected