fix(jira): link tickets by their public address

docs/providers/documentation/jira-on-prem-provider.mdx

@@ -7,4 +7,19 @@ import AutoGeneratedSnippet from '/snippets/providers/jiraonprem-snippet-autogen
 
 This is on-prem Jira provider documentation, for regular please check [Jira Provider](./jira-provider.md).
 
-<AutoGeneratedSnippet />
+<AutoGeneratedSnippet />
+## Public ticket links
+
+Keep returns a `ticket_url` for every issue it creates or updates, built from the host the provider
+connects to. That host is not always one a person can open - Jira may sit behind a proxy, or Keep
+may reach it over a cluster-internal address.
+
+Set `ticket_creation_url` to the public new-issue link and the returned links point at that host
+instead. Any of the shapes Jira hands out is understood:
+
+- `https://jira.company.com/secure/CreateIssue.jspa`
+- `https://jira.company.com/secure/CreateIssue!default.jspa?pid=10000&issuetype=1`
+- `https://jira.company.com/jira/secure/CreateIssue.jspa`, Jira served under a context path
+- `https://jira.company.com`, a plain base URL, with or without a scheme
+
+Leave the field empty and the links come from the host the provider connects to.

docs/providers/documentation/jira-provider.mdx

@@ -236,6 +236,22 @@ with:
     - Infrastructure
 ```
 
+## Public ticket links
+
+Keep returns a `ticket_url` for every issue it creates or updates, built from the host the provider
+connects to. That host is not always one a person can open - Jira may sit behind a proxy, or Keep
+may reach it over a cluster-internal address.
+
+Set `ticket_creation_url` to the public new-issue link and the returned links point at that host
+instead. Any of the shapes Jira hands out is understood:
+
+- `https://company.atlassian.net/secure/CreateIssue.jspa`
+- `https://company.atlassian.net/secure/CreateIssue!default.jspa?pid=10000&issuetype=1`
+- `https://company.atlassian.net/jira/secure/CreateIssue.jspa`, Jira served under a context path
+- `https://company.atlassian.net`, a plain base URL, with or without a scheme
+
+Leave the field empty and the links come from the host the provider connects to.
+
 ## Notes
 
 ## Useful Links

docs/snippets/providers/jira-snippet-autogenerated.mdx

@@ -6,7 +6,7 @@ This provider requires authentication.
 - **email**: Atlassian Jira Email (required: True, sensitive: False)
 - **api_token**: Atlassian Jira API Token (required: True, sensitive: True)
 - **host**: Atlassian Jira Host (required: True, sensitive: False)
-- **ticket_creation_url**: URL for creating new tickets (optional, will use default if not provided) (required: False, sensitive: False)
+- **ticket_creation_url**: URL for creating new tickets (optional, will use default if not provided), also used as the public base for the ticket links Keep returns (required: False, sensitive: False)
 
 Certain scopes may be required to perform specific actions or queries via the provider. Below is a summary of relevant scopes and their use cases:
 - **BROWSE_PROJECTS**: Browse Jira Projects (mandatory) 

docs/snippets/providers/jiraonprem-snippet-autogenerated.mdx

@@ -5,7 +5,7 @@ Do not edit it manually, as it will be overwritten */}
 This provider requires authentication.
 - **host**: Jira Host (required: True, sensitive: False)
 - **personal_access_token**: Jira PAT (required: True, sensitive: True)
-- **ticket_creation_url**: URL for creating new tickets (required: False, sensitive: False)
+- **ticket_creation_url**: URL for creating new tickets, also used as the public base for the ticket links Keep returns (required: False, sensitive: False)
 - **verify**: Verify the Jira server's TLS certificate (required: False, sensitive: False)
 
 Certain scopes may be required to perform specific actions or queries via the provider. Below is a summary of relevant scopes and their use cases:

keep/providers/jira_provider/jira_provider.py

@@ -5,7 +5,7 @@
 import dataclasses
 import json
 from typing import List, Optional
-from urllib.parse import urlencode, urljoin
+from urllib.parse import urlencode, urljoin, urlsplit, urlunsplit
 
 import pydantic
 import requests
@@ -53,7 +53,7 @@ class JiraProviderAuthConfig:
     ticket_creation_url: str = dataclasses.field(
         metadata={
             "required": False,
-            "description": "URL for creating new tickets (optional, will use default if not provided)",
+            "description": "URL for creating new tickets (optional, will use default if not provided), also used as the public base for the ticket links Keep returns",
             "sensitive": False,
             "hint": "https://keephq.atlassian.net/secure/CreateIssue.jspa",
         },
@@ -190,6 +190,42 @@ def jira_host(self) -> str:
         self._host = host
         return self._host
 
+    @property
+    def browse_host(self) -> str:
+        """Base url for the links a human clicks.
+
+        The client sometimes has to connect through an internal address - a cluster
+        shim, a reverse proxy - and a link built from that address is unreachable for
+        anyone outside. ticket_creation_url carries the public address, so the base
+        comes from there, with the connection host standing in when it is unset.
+        """
+        configured = (self.authentication_config.ticket_creation_url or "").strip()
+        if not configured:
+            return self.jira_host
+
+        parts = urlsplit(configured)
+        if not parts.scheme or not parts.netloc:
+            # urlsplit sees a host only behind a scheme, so a bare one gets https;
+            # a value naming no host keeps an empty netloc either way
+            parts = urlsplit(f"https://{configured}")
+        # without a host there is nothing to build a link from, and the connection
+        # host is at least a link that opens
+        if not parts.netloc or any(char.isspace() for char in parts.netloc):
+            return self.jira_host
+
+        # the field points at the new-issue form, and Jira spells that form in more
+        # than one way - CreateIssue.jspa, CreateIssue!default.jspa, either of them
+        # behind a context path and carrying a ?pid= - so drop a trailing .jspa page
+        # together with the /secure/ holding it, and keep whatever is left as the base
+        segments = [segment for segment in parts.path.split("/") if segment]
+        if segments and segments[-1].lower().endswith(".jspa"):
+            segments.pop()
+            if segments and segments[-1].lower() == "secure":
+                segments.pop()
+
+        path = "/" + "/".join(segments) if segments else ""
+        return urlunsplit((parts.scheme, parts.netloc, path, "", ""))
+
     def dispose(self):
         """
         No need to dispose of anything, so just do nothing.
@@ -596,7 +632,7 @@ def _notify(
 
                 issue_key = self._extract_issue_key_from_issue_id(issue_id)
 
-                result["ticket_url"] = f"{self.jira_host}/browse/{issue_key}"
+                result["ticket_url"] = f"{self.browse_host}/browse/{issue_key}"
 
                 # Apply transition if requested
                 if transition_to:
@@ -626,7 +662,7 @@ def _notify(
                 custom_fields=custom_fields,
                 **kwargs,
             )
-            result["ticket_url"] = f"{self.jira_host}/browse/{result['issue']['key']}"
+            result["ticket_url"] = f"{self.browse_host}/browse/{result['issue']['key']}"
 
             # Apply transition if requested (on newly created issue)
             if transition_to:

keep/providers/jiraonprem_provider/jiraonprem_provider.py

@@ -5,7 +5,7 @@
 import dataclasses
 import json
 from typing import List
-from urllib.parse import urlencode, urljoin
+from urllib.parse import urlencode, urljoin, urlsplit, urlunsplit
 
 import pydantic
 import requests
@@ -43,7 +43,7 @@ class JiraonpremProviderAuthConfig:
     ticket_creation_url: str = dataclasses.field(
         metadata={
             "required": False,
-            "description": "URL for creating new tickets",
+            "description": "URL for creating new tickets, also used as the public base for the ticket links Keep returns",
             "sensitive": False,
             "hint": "https://jira.onprem.com/secure/CreateIssue.jspa",
         },
@@ -204,6 +204,42 @@ def jira_host(self):
         except Exception:
             return self.authentication_config.host
 
+    @property
+    def browse_host(self) -> str:
+        """Base url for the links a human clicks.
+
+        The client sometimes has to connect through an internal address - a cluster
+        shim, a reverse proxy - and a link built from that address is unreachable for
+        anyone outside. ticket_creation_url carries the public address, so the base
+        comes from there, with the connection host standing in when it is unset.
+        """
+        configured = (self.authentication_config.ticket_creation_url or "").strip()
+        if not configured:
+            return self.jira_host
+
+        parts = urlsplit(configured)
+        if not parts.scheme or not parts.netloc:
+            # urlsplit sees a host only behind a scheme, so a bare one gets https;
+            # a value naming no host keeps an empty netloc either way
+            parts = urlsplit(f"https://{configured}")
+        # without a host there is nothing to build a link from, and the connection
+        # host is at least a link that opens
+        if not parts.netloc or any(char.isspace() for char in parts.netloc):
+            return self.jira_host
+
+        # the field points at the new-issue form, and Jira spells that form in more
+        # than one way - CreateIssue.jspa, CreateIssue!default.jspa, either of them
+        # behind a context path and carrying a ?pid= - so drop a trailing .jspa page
+        # together with the /secure/ holding it, and keep whatever is left as the base
+        segments = [segment for segment in parts.path.split("/") if segment]
+        if segments and segments[-1].lower().endswith(".jspa"):
+            segments.pop()
+            if segments and segments[-1].lower() == "secure":
+                segments.pop()
+
+        path = "/" + "/".join(segments) if segments else ""
+        return urlunsplit((parts.scheme, parts.netloc, path, "", ""))
+
     def dispose(self):
         """
         No need to dispose of anything, so just do nothing.
@@ -554,7 +590,7 @@ def _notify(
 
                 issue_key = self._extract_issue_key_from_issue_id(issue_id)
 
-                result["ticket_url"] = f"{self.jira_host}/browse/{issue_key}"
+                result["ticket_url"] = f"{self.browse_host}/browse/{issue_key}"
 
                 self.logger.info("Updated a jira issue: " + str(result))
                 return result
@@ -577,7 +613,7 @@ def _notify(
                 priority=priority,
                 **kwargs,
             )
-            result["ticket_url"] = f"{self.jira_host}/browse/{result['issue']['key']}"
+            result["ticket_url"] = f"{self.browse_host}/browse/{result['issue']['key']}"
             self.logger.info("Notified jira!")
 
             return result

tests/test_jira_provider.py

@@ -355,3 +355,177 @@ def test_notify_with_string_kwargs_handling(self, jira_provider):
 
                 # If we get here without the "string indices must be integers" error, the fix worked
                 assert result is not None
+
+    def test_browse_host_defaults_to_the_connection_host(
+        self, jira_provider, jiraonprem_provider
+    ):
+        """With ticket_creation_url unset the links come from the connection host"""
+        assert jira_provider.browse_host == jira_provider.jira_host
+        assert jiraonprem_provider.browse_host == jiraonprem_provider.jira_host
+
+    @staticmethod
+    def _jiraonprem_with_creation_url(context_manager, ticket_creation_url):
+        return JiraonpremProvider(
+            context_manager,
+            "test_jiraonprem",
+            ProviderConfig(
+                description="Test Jira On-Prem Provider",
+                authentication={
+                    "host": "https://jira.internal.svc",
+                    "personal_access_token": "test_token",
+                    "ticket_creation_url": ticket_creation_url,
+                },
+            ),
+        )
+
+    @staticmethod
+    def _jira_with_creation_url(context_manager, ticket_creation_url):
+        return JiraProvider(
+            context_manager,
+            "test_jira",
+            ProviderConfig(
+                description="Test Jira Provider",
+                authentication={
+                    "email": "test@example.com",
+                    "api_token": "test_token",
+                    "host": "https://jira.internal.svc",
+                    "ticket_creation_url": ticket_creation_url,
+                },
+            ),
+        )
+
+    def test_browse_host_reads_the_public_url_from_the_create_form_link(
+        self, context_manager
+    ):
+        """The field holds a link to the new-issue form, the base of it is the public host"""
+        provider = self._jiraonprem_with_creation_url(
+            context_manager, "https://jira.company.com/secure/CreateIssue.jspa"
+        )
+
+        assert provider.browse_host == "https://jira.company.com"
+        assert provider.jira_host == "https://jira.internal.svc"
+
+    @pytest.mark.parametrize(
+        "configured, expected",
+        [
+            # the link the UI hands out
+            (
+                "https://jira.company.com/secure/CreateIssue.jspa",
+                "https://jira.company.com",
+            ),
+            # Jira spells the same form in more than one way, and hangs the project
+            # and issue type off it as a query string
+            (
+                "https://jira.company.com/secure/CreateIssue!default.jspa?pid=10000&issuetype=1",
+                "https://jira.company.com",
+            ),
+            (
+                "https://jira.company.com/secure/CreateIssueDetails!init.jspa",
+                "https://jira.company.com",
+            ),
+            # served under a context path, which the browse link has to keep
+            (
+                "https://company.com/jira/secure/CreateIssue.jspa?pid=10000",
+                "https://company.com/jira",
+            ),
+            # a plain base url, with or without a trailing slash or a scheme
+            ("https://jira.company.com", "https://jira.company.com"),
+            ("https://jira.company.com/", "https://jira.company.com"),
+            ("jira.company.com", "https://jira.company.com"),
+            ("jira.company.com/secure/CreateIssue.jspa", "https://jira.company.com"),
+            # a port and a plain-http host survive as they are
+            (
+                "jira.company.com:8443/secure/CreateIssue.jspa",
+                "https://jira.company.com:8443",
+            ),
+            ("https://jira.company.com:8443", "https://jira.company.com:8443"),
+            (
+                "http://jira.company.com/secure/CreateIssue.jspa",
+                "http://jira.company.com",
+            ),
+            # copied out of a text field with whitespace around it
+            (
+                "  https://jira.company.com/secure/CreateIssue.jspa  ",
+                "https://jira.company.com",
+            ),
+        ],
+    )
+    def test_browse_host_reads_every_shape_of_the_create_form_link(
+        self, context_manager, configured, expected
+    ):
+        """Both providers derive the same public base from the link they are given"""
+        assert (
+            self._jiraonprem_with_creation_url(context_manager, configured).browse_host
+            == expected
+        )
+        assert (
+            self._jira_with_creation_url(context_manager, configured).browse_host
+            == expected
+        )
+
+    @pytest.mark.parametrize(
+        "configured",
+        ["", "   ", "/secure/CreateIssue.jspa", "/jira/", "not a url at all"],
+    )
+    def test_browse_host_falls_back_when_the_link_names_no_host(
+        self, context_manager, configured
+    ):
+        """A blank, host-less or unreadable value leaves the links on the connection host"""
+        provider = self._jiraonprem_with_creation_url(context_manager, configured)
+
+        assert provider.browse_host == provider.jira_host
+
+    @patch("requests.put")
+    @patch("requests.get")
+    def test_jiraonprem_ticket_url_points_at_the_public_host(
+        self, mock_get, mock_put, context_manager
+    ):
+        """An updated issue is linked by its public address, not the internal one"""
+        provider = JiraonpremProvider(
+            context_manager,
+            "test_jiraonprem",
+            ProviderConfig(
+                description="Test Jira On-Prem Provider",
+                authentication={
+                    "host": "https://jira.internal.svc",
+                    "personal_access_token": "test_token",
+                    "ticket_creation_url": "https://jira.company.com/secure/CreateIssue.jspa",
+                },
+            ),
+        )
+
+        mock_get.return_value.status_code = 200
+        mock_get.return_value.json.return_value = {"key": "TEST-123"}
+        mock_put.return_value.status_code = 204
+
+        result = provider._notify(issue_id="TEST-123", summary="Test Summary")
+
+        assert result["ticket_url"] == "https://jira.company.com/browse/TEST-123"
+
+    @patch("requests.put")
+    @patch("requests.get")
+    def test_jira_cloud_ticket_url_points_at_the_public_host(
+        self, mock_get, mock_put, context_manager
+    ):
+        """The cloud provider builds the link from the same field"""
+        provider = JiraProvider(
+            context_manager,
+            "test_jira",
+            ProviderConfig(
+                description="Test Jira Provider",
+                authentication={
+                    "email": "test@example.com",
+                    "api_token": "test_token",
+                    "host": "https://jira.internal.svc",
+                    "ticket_creation_url": "https://company.atlassian.net/secure/CreateIssue.jspa",
+                },
+            ),
+        )
+
+        mock_get.return_value.status_code = 200
+        mock_get.return_value.json.return_value = {"key": "TEST-123"}
+        mock_put.return_value.status_code = 204
+
+        result = provider._notify(issue_id="TEST-123", summary="Test Summary")
+
+        assert result["ticket_url"] == "https://company.atlassian.net/browse/TEST-123"