From 67ac8ce1422a0ab3ff8790f1facad44aa50b6e1f Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Thu, 22 Oct 2026 12:16:01 +0000 Subject: [PATCH] fix: preview Jira issues whose project key contains digits Jira Cloud project keys are a letter followed by letters or digits, but the link preview only recognised keys made of letters, so a link such as https://example.atlassian.net/browse/AB12C-345 got a generic link card. The preview now also looks an issue up on the site the link points to, and only when the user is connected to that site: for an API token or a login and password, the host the user connected to or the host of the base URL kept from Jira; for OAuth, one of the authorised sites. It used to look the key up on the user's own Jira whatever the link's host, so a link to another company's site showed the user's own issue with the same key, and a user who authorised several sites with OAuth got the generic link card unless every site had an issue with that key, and then saw the last site's one. Connecting with an API token or a login and password now asks Jira for its base URL, which names the site when the connection goes through Atlassian's API gateway, and keeps it in place of any kept before. A gateway connection made before this change shows its link previews again once it is connected again. An issue that cannot be shown, such as one the user cannot see, now gets the generic link card without two PHP warnings in the log. Fixes #236 Signed-off-by: Oleksander Piskun --- CHANGELOG.md | 3 + lib/Controller/ConfigController.php | 8 + lib/Reference/JiraReferenceProvider.php | 33 +- lib/Service/JiraAPIService.php | 88 +++-- .../unit/Controller/ConfigControllerTest.php | 82 +++- .../Reference/JiraReferenceProviderTest.php | 156 +++++++- tests/unit/Service/JiraAPIServiceTest.php | 366 +++++++++++++++++- 7 files changed, 681 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5255469..2776307 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Show the issues of a Jira Cloud site on atlassian.net connected with an API token in the dashboard widget and the search, and its open tickets notification once the site is connected again - Show no avatar rather than a server error when an avatar image cannot be fetched - Report an answer that is not a JSON object or array, such as the HTML page of a proxy in front of Jira, as an error when connected with a login and password +- Show the link preview of an issue whose project key contains digits, such as AB12C-345 +- Show the link preview of an issue only from the Jira site the link points to, and only for a site the user is connected to, instead of the issue with the same key on the user's own site; a Jira Cloud site connected with an API token through Atlassian's API gateway before this change shows its link previews again once it is connected again +- Show the generic link preview without PHP warnings in the log when an issue cannot be shown, such as one the user cannot see ## 1.5.0 - 2026-09-29 diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index a251374..c836b44 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -149,6 +149,14 @@ public function connectToSoftware(string $url, string $login, string $password): $this->config->setUserValue($this->userId, Application::APP_ID, 'user_account_id', $info['accountId']); } $this->config->setUserValue($this->userId, Application::APP_ID, 'url', $targetInstanceUrl); + // the base URL Jira builds its links with, which differs when Nextcloud reaches Jira under another + // name, such as Atlassian's API gateway + $serverInfo = $this->networkService->basicRequest($targetInstanceUrl, $basicAuthHeader, 'rest/api/2/serverInfo'); + if (isset($serverInfo['baseUrl']) && is_string($serverInfo['baseUrl']) && $serverInfo['baseUrl'] !== '') { + $this->config->setUserValue($this->userId, Application::APP_ID, 'base_url', $serverInfo['baseUrl']); + } else { + $this->config->deleteUserValue($this->userId, Application::APP_ID, 'base_url'); + } $basicAuthHeader = $basicAuthHeader === '' ? '' : $this->crypto->encrypt($basicAuthHeader); $this->config->setUserValue($this->userId, Application::APP_ID, 'basic_auth_header', $basicAuthHeader); return new DataResponse(['user_name' => $info['displayName']]); diff --git a/lib/Reference/JiraReferenceProvider.php b/lib/Reference/JiraReferenceProvider.php index 3087bee..3182373 100644 --- a/lib/Reference/JiraReferenceProvider.php +++ b/lib/Reference/JiraReferenceProvider.php @@ -78,12 +78,14 @@ public function getSupportedSearchProviderIds(): array { */ public function matchReference(string $referenceText): bool { // the server asks the providers in turn about a link in any app until one matches, so only read the settings for a Jira link - if ($this->getJiraIssueId($referenceText) === null) { + $issueLink = $this->getIssueLink($referenceText); + if ($issueLink === null || $this->userId === null) { return false; } $adminLinkPreviewEnabled = $this->appConfig->getAppValueString('link_preview_enabled', '1', lazy: true) === '1'; $userLinkPreviewEnabled = $this->config->getUserValue($this->userId, Application::APP_ID, 'link_preview_enabled', '1') === '1'; - return $adminLinkPreviewEnabled && $userLinkPreviewEnabled; + return $adminLinkPreviewEnabled && $userLinkPreviewEnabled + && $this->jiraAPIService->isConnectedToSite($this->userId, $issueLink['host']); } /** @@ -92,12 +94,16 @@ public function matchReference(string $referenceText): bool { public function resolveReference(string $referenceText): ?IReference { if ($this->matchReference($referenceText)) { try { - $issueId = $this->getJiraIssueId($referenceText); - if ($issueId === null) { + $issueLink = $this->getIssueLink($referenceText); + if ($issueLink === null) { return $this->linkReferenceProvider->resolveReference($referenceText); } - $issueInfo = $this->jiraAPIService->getIssueInfo($this->userId, $issueId); + $issueInfo = $this->jiraAPIService->getIssueInfo($this->userId, $issueLink['key'], $issueLink['host']); + if (!isset($issueInfo['fields'])) { + // for example an issue the user cannot see + return $this->linkReferenceProvider->resolveReference($referenceText); + } $reference = new Reference($referenceText); $reference->setTitle($this->getMainText($issueInfo)); @@ -116,12 +122,19 @@ public function resolveReference(string $referenceText): ?IReference { return null; } - private function getJiraIssueId(string $referenceText): ?string { - $regex = '/https:\/\/[a-zA-Z0-9.-]+\.atlassian\.net\/browse\/([A-Z]+-\d+)/'; - if (preg_match($regex, $referenceText, $matches)) { - return $matches[1]; + /** + * The site host and the issue key of a link to an issue on a Jira Cloud site + * + * @return array{host: string, key: string}|null + */ + private function getIssueLink(string $referenceText): ?array { + // a project key is a letter followed by letters, digits or underscores, and the issue key adds a number; + // the link can go on with a path, a query, a fragment or the punctuation of the sentence it ends + $regex = '/^https:\/\/([a-z0-9.-]+\.atlassian\.net)\/browse\/([a-z][a-z0-9_]+-\d+)(?![a-z0-9_])/i'; + if (preg_match($regex, $referenceText, $matches) !== 1) { + return null; } - return null; + return ['host' => $matches[1], 'key' => $matches[2]]; } /** diff --git a/lib/Service/JiraAPIService.php b/lib/Service/JiraAPIService.php index c12381c..eba9693 100644 --- a/lib/Service/JiraAPIService.php +++ b/lib/Service/JiraAPIService.php @@ -125,7 +125,7 @@ private function sendNCNotification(string $userId, string $subject, array $para public function getJiraResources(string $userId): array { $strRes = $this->config->getUserValue($userId, Application::APP_ID, 'resources'); $resources = json_decode($strRes, true); - return ($resources && count($resources) > 0) ? $resources : []; + return is_array($resources) ? $resources : []; } /** @@ -388,8 +388,8 @@ private function isJiraCloudUrl(string $url): bool { * Whether a host is the Jira a user connected to with login and password (or API token): * the host they connected to, or the host of the base URL Jira builds its links with, which * differs when Nextcloud reaches Jira under another name, such as an internal address. Jira - * is asked for its base URL only for a host other than the connected one, and an answer it - * gives is kept until the user disconnects. + * is asked for its base URL when the user connects and, while none is kept, for a host other + * than the connected one; an answer it gives is kept until the user disconnects or connects again. */ private function isJiraHost(string $userId, string $basicAuthHeader, string $host): bool { if ($host === '') { @@ -419,48 +419,72 @@ private function getHost(string $url): string { return is_string($host) ? rtrim(strtolower($host), '.') : ''; } - public function getIssueInfo(string $userId, string $issueId): ?array { - $issueInfo = null; - $endPoint = 'rest/api/3/issue/' . $issueId; - - $basicAuthHeader = $this->config->getUserValue($userId, Application::APP_ID, 'basic_auth_header'); - $basicAuthHeader = $basicAuthHeader === '' ? '' : $this->crypto->decrypt($basicAuthHeader); + /** + * Whether the user is connected to the Jira site on this host + */ + public function isConnectedToSite(string $userId, string $host): bool { + return $this->getSite($userId, $host) !== null; + } - // self-hosted Jira - if ($basicAuthHeader !== '') { + /** + * The Jira site on this host the user is connected to: for a login and password or an API token, + * the host the user connected to or the host of the base URL kept from Jira (see isJiraHost), + * which differs when Nextcloud reaches Jira under another name, such as Atlassian's API gateway; + * for OAuth, one of the sites the user authorised. Only stored values decide, as the link preview + * asks this for every Jira link. + * + * @return array{url: string}|array{cloudId: string}|null + */ + private function getSite(string $userId, string $host): ?array { + $host = strtolower($host); + if ($this->config->getUserValue($userId, Application::APP_ID, 'basic_auth_header') !== '') { $jiraUrl = $this->config->getUserValue($userId, Application::APP_ID, 'url'); + $baseUrl = $this->config->getUserValue($userId, Application::APP_ID, 'base_url'); + $jiraHosts = array_filter([$this->getHost($jiraUrl), $this->getHost($baseUrl)], fn (string $jiraHost): bool => $jiraHost !== ''); + return in_array($host, $jiraHosts, true) ? ['url' => $jiraUrl] : null; + } + if ($this->config->getUserValue($userId, Application::APP_ID, 'token') === '') { + return null; + } + foreach ($this->getJiraResources($userId) as $resource) { + if (is_string($resource['id'] ?? null) && is_string($resource['url'] ?? null) + && $this->getHost($resource['url']) === $host) { + return ['cloudId' => $resource['id']]; + } + } + return null; + } + /** + * @param string $siteHost the host of the Jira site the issue is on + */ + public function getIssueInfo(string $userId, string $issueId, string $siteHost): ?array { + $site = $this->getSite($userId, $siteHost); + if ($site === null) { + return null; + } + $endPoint = 'rest/api/3/issue/' . $issueId; + + // a login and password or an API token + if (isset($site['url'])) { // check if there is a forced instance $forcedInstanceUrl = $this->appConfig->getAppValueString('forced_instance_url', lazy: true); - if ($forcedInstanceUrl !== '' && $forcedInstanceUrl !== $jiraUrl) { + if ($forcedInstanceUrl !== '' && $forcedInstanceUrl !== $site['url']) { return [ 'error' => 'Unauthorized Jira instance URL', ]; } - $issuesResult = $this->networkService->basicRequest($jiraUrl, $basicAuthHeader, $endPoint); - if (isset($issuesResult['error'])) { - return $issuesResult; - } - if (isset($issuesResult['fields'])) { - $issueInfo = $issuesResult; - } + $basicAuthHeader = $this->crypto->decrypt($this->config->getUserValue($userId, Application::APP_ID, 'basic_auth_header')); + $issuesResult = $this->networkService->basicRequest($site['url'], $basicAuthHeader, $endPoint); } else { // Jira cloud - $resources = $this->getJiraResources($userId); - - foreach ($resources as $resource) { - $cloudId = $resource['id']; - $issuesResult = $this->networkService->oauthRequest($userId, 'ex/jira/' . $cloudId . '/' . $endPoint); - if (!isset($issuesResult['error']) && isset($issuesResult['fields'])) { - $issueInfo = $issuesResult; - } else { - return $issuesResult; - } - } + $issuesResult = $this->networkService->oauthRequest($userId, 'ex/jira/' . $site['cloudId'] . '/' . $endPoint); } - - return $issueInfo; + if (isset($issuesResult['error'])) { + return $issuesResult; + } + return isset($issuesResult['fields']) ? $issuesResult : null; } /** diff --git a/tests/unit/Controller/ConfigControllerTest.php b/tests/unit/Controller/ConfigControllerTest.php index 3f2a59a..020773a 100644 --- a/tests/unit/Controller/ConfigControllerTest.php +++ b/tests/unit/Controller/ConfigControllerTest.php @@ -73,25 +73,38 @@ private function storeLazyAppValues(array $values): void { ); } + /** @var list the user values connectToSoftware deleted */ + private array $deletedUserValues = []; + /** * @return array the user values connectToSoftware stored */ - private function connectToSoftwareWith(array $myself): array { + private function connectToSoftwareWith(array $myself, array $serverInfo = []): array { $stored = []; $this->config->method('setUserValue')->willReturnCallback( function (string $userId, string $app, string $key, $value) use (&$stored): void { $stored[$key] = $value; } ); + $this->config->method('deleteUserValue')->willReturnCallback( + function (string $userId, string $app, string $key): void { + $this->deletedUserValues[] = $key; + } + ); $this->appConfig->method('getAppValueString')->willReturnCallback(fn (string $key, string $default = '') => $default); - $this->networkService->expects($this->once()) - ->method('basicRequest') - ->with('https://jira.example.com', base64_encode('login:password'), 'rest/api/2/myself') - ->willReturn($myself); + $asked = []; + $this->networkService->method('basicRequest')->willReturnCallback( + function (string $url, string $authHeader, string $endPoint) use (&$asked, $myself, $serverInfo): array { + $this->assertSame(['https://jira.example.com', base64_encode('login:password')], [$url, $authHeader]); + $asked[] = $endPoint; + return $endPoint === 'rest/api/2/myself' ? $myself : $serverInfo; + } + ); $response = $this->controller->connectToSoftware('https://jira.example.com', 'login', 'password'); $this->assertSame(['user_name' => 'Jane Doe'], $response->getData()); + $this->assertSame(['rest/api/2/myself', 'rest/api/2/serverInfo'], $asked); return $stored; } @@ -113,6 +126,46 @@ public function testConnectingToJiraCloudWithAnApiTokenStoresTheAccountId(): voi $this->assertSame('712020:abc', $stored['user_account_id']); } + /** + * Jira's base URL names the site its issue links point to, also when the user connected + * through another address, such as Atlassian's API gateway. + * + * @dataProvider provideBaseUrls + */ + public function testConnectingKeepsTheBaseUrlJiraGives(string $baseUrl): void { + $stored = $this->connectToSoftwareWith(['displayName' => 'Jane Doe'], ['baseUrl' => $baseUrl]); + + $this->assertSame($baseUrl, $stored['base_url']); + $this->assertNotContains('base_url', $this->deletedUserValues); + } + + public static function provideBaseUrls(): array { + return [ + 'a Jira Cloud site' => ['https://site-a.atlassian.net'], + 'a self-hosted Jira over http' => ['http://jira.example.com:8080/jira'], + 'the connected URL' => ['https://jira.example.com'], + ]; + } + + /** + * @dataProvider provideUnusableServerInfoAnswers + */ + public function testConnectingToAJiraGivingNoUsableBaseUrlForgetsAKeptOne(array $serverInfo): void { + $stored = $this->connectToSoftwareWith(['displayName' => 'Jane Doe'], $serverInfo); + + $this->assertArrayNotHasKey('base_url', $stored); + $this->assertContains('base_url', $this->deletedUserValues); + } + + public static function provideUnusableServerInfoAnswers(): array { + return [ + 'an error' => [['error' => 'Bad credentials']], + 'no base URL' => [['version' => '9.12.0']], + 'an empty base URL' => [['baseUrl' => '']], + 'a base URL that is no string' => [['baseUrl' => ['https://site-a.atlassian.net']]], + ]; + } + /** * The base URL kept for the avatars belongs to the Jira the user disconnects from; kept, it * would let the next Jira's credentials go to that host. @@ -198,6 +251,25 @@ public function testConnectingToSelfHostedJiraUsesTheForcedInstance(): void { $this->assertSame(['user_name' => '', 'error' => 'Bad credentials'], $response->getData()); } + public function testConnectingToTheForcedInstanceAsksItForTheBaseUrl(): void { + $this->storeLazyAppValues(['forced_instance_url' => 'https://forced.example.com']); + $asked = []; + $this->networkService->method('basicRequest')->willReturnCallback( + function (string $url, string $authHeader, string $endPoint) use (&$asked): array { + $asked[] = [$url, $endPoint]; + return $endPoint === 'rest/api/2/myself' ? ['displayName' => 'Jane Doe'] : ['baseUrl' => 'https://forced.example.com']; + } + ); + $this->config->expects($this->atLeastOnce())->method('setUserValue'); + + $this->controller->connectToSoftware('https://jira.example.com', 'jdoe', 'password'); + + $this->assertSame([ + ['https://forced.example.com', 'rest/api/2/myself'], + ['https://forced.example.com', 'rest/api/2/serverInfo'], + ], $asked); + } + public function testConnectingToSelfHostedJiraUsesTheGivenUrlWithoutAForcedInstance(): void { $this->storeLazyAppValues([]); $this->networkService->expects($this->once()) diff --git a/tests/unit/Reference/JiraReferenceProviderTest.php b/tests/unit/Reference/JiraReferenceProviderTest.php index ffc23f8..f6705d4 100644 --- a/tests/unit/Reference/JiraReferenceProviderTest.php +++ b/tests/unit/Reference/JiraReferenceProviderTest.php @@ -12,6 +12,7 @@ use OCA\Jira\Reference\JiraReferenceProvider; use OCA\Jira\Service\JiraAPIService; use OCP\AppFramework\Services\IAppConfig; +use OCP\Collaboration\Reference\Reference; use OCP\IConfig; use OCP\IL10N; use OCP\IURLGenerator; @@ -34,7 +35,16 @@ private function appConfigWithLazyValues(array $values): IAppConfig { return $appConfig; } - private function createProvider(IAppConfig $appConfig, array $userValues = [], ?IConfig $config = null): JiraReferenceProvider { + /** + * A provider for a user connected to every site, unless another service is given + */ + private function createProvider(IAppConfig $appConfig, array $userValues = [], ?IConfig $config = null, + ?JiraAPIService $jiraAPIService = null, ?string $userId = 'user1', + ?LinkReferenceProvider $linkReferenceProvider = null): JiraReferenceProvider { + if ($jiraAPIService === null) { + $jiraAPIService = $this->createMock(JiraAPIService::class); + $jiraAPIService->method('isConnectedToSite')->willReturn(true); + } if ($config === null) { $config = $this->createMock(IConfig::class); // a legacy read of a lazy key gets the default on the server @@ -43,19 +53,28 @@ private function createProvider(IAppConfig $appConfig, array $userValues = [], ? } return new JiraReferenceProvider( - $this->createMock(JiraAPIService::class), + $jiraAPIService, $appConfig, $config, $this->createMock(IL10N::class), $this->createMock(IURLGenerator::class), $this->createMock(ReferenceManager::class), - $this->createMock(LinkReferenceProvider::class), - 'user1', + $linkReferenceProvider ?? $this->createMock(LinkReferenceProvider::class), + $userId, ); } + /** + * A service that must not be asked, as the settings already decide + */ + private function unaskedService(): JiraAPIService { + $jiraAPIService = $this->createMock(JiraAPIService::class); + $jiraAPIService->expects($this->never())->method($this->anything()); + return $jiraAPIService; + } + public function testIssueLinksAreNotMatchedWhenTheAdminDisabledLinkPreviews(): void { - $provider = $this->createProvider($this->appConfigWithLazyValues(['link_preview_enabled' => '0'])); + $provider = $this->createProvider($this->appConfigWithLazyValues(['link_preview_enabled' => '0']), jiraAPIService: $this->unaskedService()); $this->assertFalse($provider->matchReference(self::ISSUE_URL)); } @@ -71,7 +90,7 @@ public function testIssueLinksAreMatchedWhenNothingIsStored(): void { } public function testIssueLinksAreNotMatchedWhenTheUserDisabledLinkPreviews(): void { - $provider = $this->createProvider($this->appConfigWithLazyValues(['link_preview_enabled' => '1']), ['link_preview_enabled' => '0']); + $provider = $this->createProvider($this->appConfigWithLazyValues(['link_preview_enabled' => '1']), ['link_preview_enabled' => '0'], jiraAPIService: $this->unaskedService()); $this->assertFalse($provider->matchReference(self::ISSUE_URL)); } @@ -85,4 +104,129 @@ public function testOtherLinksAreNotMatchedWithoutReadingTheAppConfig(): void { $this->assertFalse($this->createProvider($appConfig, config: $config)->matchReference('https://github.com/nextcloud/server/pull/1')); } + + /** + * @dataProvider provideIssueLinks + */ + public function testIssueLinksAreLookedUpByTheirKeyOnTheirSite(string $url, string $host, string $key): void { + $jiraAPIService = $this->createMock(JiraAPIService::class); + $jiraAPIService->method('isConnectedToSite')->with('user1', $host)->willReturn(true); + $jiraAPIService->expects($this->once()) + ->method('getIssueInfo') + ->with('user1', $key, $host) + ->willReturn(['key' => $key, 'fields' => ['summary' => 'An issue', 'creator' => null, 'priority' => null, 'status' => null]]); + $provider = $this->createProvider($this->appConfigWithLazyValues([]), jiraAPIService: $jiraAPIService); + + $this->assertTrue($provider->matchReference($url)); + $reference = $provider->resolveReference($url); + $this->assertSame('An issue', $reference?->getTitle()); + $this->assertSame('integration_jira_search', $reference?->getRichObjectType()); + } + + public static function provideIssueLinks(): array { + return [ + 'letters only' => ['https://ncintegration.atlassian.net/browse/NC-42', 'ncintegration.atlassian.net', 'NC-42'], + 'digits in the project key' => ['https://ncintegration.atlassian.net/browse/AB12C-345', 'ncintegration.atlassian.net', 'AB12C-345'], + 'a letter and a digit' => ['https://ncintegration.atlassian.net/browse/N2-7', 'ncintegration.atlassian.net', 'N2-7'], + 'an underscore' => ['https://ncintegration.atlassian.net/browse/NC_T-1', 'ncintegration.atlassian.net', 'NC_T-1'], + 'a lower case key' => ['https://ncintegration.atlassian.net/browse/nc2t-1', 'ncintegration.atlassian.net', 'nc2t-1'], + 'an upper case host' => ['https://NCintegration.Atlassian.NET/browse/NC-42', 'NCintegration.Atlassian.NET', 'NC-42'], + 'a query' => ['https://ncintegration.atlassian.net/browse/NC-42?focusedCommentId=10000', 'ncintegration.atlassian.net', 'NC-42'], + 'a fragment' => ['https://ncintegration.atlassian.net/browse/NC-42#comment', 'ncintegration.atlassian.net', 'NC-42'], + 'a trailing slash' => ['https://ncintegration.atlassian.net/browse/NC-42/', 'ncintegration.atlassian.net', 'NC-42'], + // the server takes these characters into a link, so a link that ends a sentence keeps its punctuation + 'a full stop' => ['https://ncintegration.atlassian.net/browse/NC-42.', 'ncintegration.atlassian.net', 'NC-42'], + 'a comma' => ['https://ncintegration.atlassian.net/browse/NC-42,', 'ncintegration.atlassian.net', 'NC-42'], + 'a closing parenthesis' => ['https://ncintegration.atlassian.net/browse/NC-42)', 'ncintegration.atlassian.net', 'NC-42'], + 'an exclamation mark' => ['https://ncintegration.atlassian.net/browse/NC-42!', 'ncintegration.atlassian.net', 'NC-42'], + 'a colon' => ['https://ncintegration.atlassian.net/browse/NC-42:', 'ncintegration.atlassian.net', 'NC-42'], + 'a semicolon' => ['https://ncintegration.atlassian.net/browse/NC-42;', 'ncintegration.atlassian.net', 'NC-42'], + 'another site' => ['https://other.atlassian.net/browse/OT-1', 'other.atlassian.net', 'OT-1'], + 'a site name starting with a digit' => ['https://9site.atlassian.net/browse/NC-1', '9site.atlassian.net', 'NC-1'], + ]; + } + + /** + * @dataProvider provideLinksThatAreNoJiraIssue + */ + public function testLinksThatAreNoJiraIssueAreNotMatched(string $url): void { + $jiraAPIService = $this->createMock(JiraAPIService::class); + $jiraAPIService->expects($this->never())->method($this->anything()); + + $this->assertFalse($this->createProvider($this->appConfigWithLazyValues([]), jiraAPIService: $jiraAPIService)->matchReference($url)); + } + + public static function provideLinksThatAreNoJiraIssue(): array { + return [ + 'a project key starting with a digit' => ['https://ncintegration.atlassian.net/browse/2NC-42'], + 'a project key of one letter' => ['https://ncintegration.atlassian.net/browse/N-42'], + 'no issue number' => ['https://ncintegration.atlassian.net/browse/NC'], + 'letters after the issue number' => ['https://ncintegration.atlassian.net/browse/NC-42abc'], + 'an underscore after the issue number' => ['https://ncintegration.atlassian.net/browse/NC-42_'], + 'no number after the dash' => ['https://ncintegration.atlassian.net/browse/NC-'], + 'a Jira link inside another link' => ['https://example.com/?url=https://ncintegration.atlassian.net/browse/NC-42'], + 'http' => ['http://ncintegration.atlassian.net/browse/NC-42'], + 'a host that only starts like a Jira site' => ['https://ncintegration.atlassian.net.example.com/browse/NC-42'], + 'a board with an issue open' => ['https://ncintegration.atlassian.net/jira/software/projects/NC/boards/1?selectedIssue=NC-42'], + ]; + } + + public function testIssueLinksOfASiteTheUserIsNotConnectedToAreNotMatched(): void { + $jiraAPIService = $this->createMock(JiraAPIService::class); + $jiraAPIService->expects($this->once()) + ->method('isConnectedToSite') + ->with('user1', 'someother-company.atlassian.net') + ->willReturn(false); + $jiraAPIService->expects($this->never())->method('getIssueInfo'); + $provider = $this->createProvider($this->appConfigWithLazyValues([]), jiraAPIService: $jiraAPIService); + + $this->assertFalse($provider->matchReference('https://someother-company.atlassian.net/browse/NC-42')); + } + + public function testIssueLinksAreNotMatchedWithoutAUser(): void { + $jiraAPIService = $this->createMock(JiraAPIService::class); + $jiraAPIService->expects($this->never())->method($this->anything()); + + $this->assertFalse($this->createProvider($this->appConfigWithLazyValues([]), jiraAPIService: $jiraAPIService, userId: null)->matchReference(self::ISSUE_URL)); + } + + /** + * @dataProvider provideIssueAnswersThatAreNoIssue + */ + public function testAnIssueThatCannotBeShownGetsTheGenericLinkPreview(?array $answer, bool $throws): void { + $jiraAPIService = $this->createMock(JiraAPIService::class); + $jiraAPIService->method('isConnectedToSite')->willReturn(true); + $getIssueInfo = $jiraAPIService->expects($this->once())->method('getIssueInfo'); + if ($throws) { + $getIssueInfo->willThrowException(new \RuntimeException('Jira could not be reached')); + } else { + $getIssueInfo->willReturn($answer); + } + $linkPreview = new Reference(self::ISSUE_URL); + $linkReferenceProvider = $this->createMock(LinkReferenceProvider::class); + $linkReferenceProvider->expects($this->once())->method('resolveReference')->with(self::ISSUE_URL)->willReturn($linkPreview); + $provider = $this->createProvider($this->appConfigWithLazyValues([]), jiraAPIService: $jiraAPIService, linkReferenceProvider: $linkReferenceProvider); + + $this->assertSame($linkPreview, $provider->resolveReference(self::ISSUE_URL)); + } + + public static function provideIssueAnswersThatAreNoIssue(): array { + return [ + 'an error' => [['error' => 'Issue does not exist or you do not have permission to see it.'], false], + 'an answer without fields' => [['key' => 'NC-42'], false], + 'no site' => [null, false], + 'an exception' => [null, true], + ]; + } + + public function testIssueLinksOfASiteTheUserIsNotConnectedToAreNotResolved(): void { + $jiraAPIService = $this->createMock(JiraAPIService::class); + $jiraAPIService->method('isConnectedToSite')->willReturn(false); + $jiraAPIService->expects($this->never())->method('getIssueInfo'); + $linkReferenceProvider = $this->createMock(LinkReferenceProvider::class); + $linkReferenceProvider->expects($this->never())->method('resolveReference'); + $provider = $this->createProvider($this->appConfigWithLazyValues([]), jiraAPIService: $jiraAPIService, linkReferenceProvider: $linkReferenceProvider); + + $this->assertNull($provider->resolveReference('https://someother-company.atlassian.net/browse/NC-42')); + } } diff --git a/tests/unit/Service/JiraAPIServiceTest.php b/tests/unit/Service/JiraAPIServiceTest.php index 2011c54..e888bc3 100644 --- a/tests/unit/Service/JiraAPIServiceTest.php +++ b/tests/unit/Service/JiraAPIServiceTest.php @@ -478,7 +478,7 @@ private function stubAvatarOfABasicAuthConnection(string $jiraUrl, string $store fn ($userId, $appName, $key, $default = '') => match ($key) { 'basic_auth_header' => 'encrypted', 'url' => $jiraUrl, - 'base_url' => $storedBaseUrl, + 'base_url' => $this->storedUserValues['base_url'] ?? $storedBaseUrl, default => $default, } ); @@ -499,6 +499,22 @@ function (string $url, string $authHeader, string $endPoint, array $params = []) ); } + /** + * A connection through Atlassian's API gateway that has no base URL kept gets its site's link + * previews once an avatar on another host made the app ask Jira for the base URL. + */ + public function testTheBaseUrlAnAvatarAskedForNamesTheSiteOfTheLinkPreviews(): void { + $this->stubAvatarOfABasicAuthConnection('https://api.atlassian.com/ex/jira/cloud-a', '', ['baseUrl' => 'https://site-a.atlassian.net'], 'https://secure.gravatar.com/avatar/abc'); + $requests = []; + $service = $this->serviceRecordingRequests($requests); + + $this->assertFalse($service->isConnectedToSite('user1', 'site-a.atlassian.net')); + $service->getJiraAvatar('user1', '712020:abc', ''); + $this->assertTrue($service->isConnectedToSite('user1', 'site-a.atlassian.net')); + $this->assertSame(['rest/api/2/user', 'rest/api/2/serverInfo'], $this->askedEndpoints); + $this->assertArrayNotHasKey('Authorization', $requests[0][1]); + } + public function testAKeptBaseUrlIsNotAskedAgain(): void { $this->stubAvatarOfABasicAuthConnection('http://10.0.0.5:8080', 'https://jira.example.com', ['baseUrl' => 'https://other.example.com'], 'https://jira.example.com/secure/useravatar?avatarId=1'); $requests = []; @@ -671,12 +687,358 @@ public static function provideSelfHostedUrls(): array { ]; } + private const TWO_SITES = '[{"id":"cloud-a","url":"https://site-a.atlassian.net","name":"site-a"},' + . '{"id":"cloud-b","url":"https://site-b.atlassian.net","name":"site-b"}]'; + + /** + * A user connected with OAuth to the given sites (the resources Atlassian answered). + */ + private function stubOAuthConnection(string $token, string $resources): void { + $this->storeLazyAppValues([]); + $this->config->method('getUserValue')->willReturnCallback( + fn ($userId, $appName, $key, $default = '') => match ($key) { + 'token' => $token, + 'resources' => $resources, + default => $default, + } + ); + } + + /** + * @dataProvider provideHostsOfTwoOAuthSites + */ + public function testAUserIsConnectedOnlyToTheSitesItAuthorisedWithOAuth(string $host, bool $connected): void { + $this->stubOAuthConnection('encrypted-token', self::TWO_SITES); + + $this->assertSame($connected, $this->apiService->isConnectedToSite('user1', $host)); + } + + public static function provideHostsOfTwoOAuthSites(): array { + return [ + 'the first site' => ['site-a.atlassian.net', true], + 'the second site' => ['site-b.atlassian.net', true], + 'upper case' => ['Site-B.Atlassian.NET', true], + 'another site' => ['site-c.atlassian.net', false], + 'a site host inside another host' => ['site-a.atlassian.net.example.com', false], + 'a host ending in a site host' => ['xsite-a.atlassian.net', false], + 'the end of a site host' => ['a.atlassian.net', false], + 'a subdomain of a site host' => ['x.site-a.atlassian.net', false], + ]; + } + + public function testAUserWhoseOAuthTokenIsGoneIsConnectedToNoSite(): void { + $this->stubOAuthConnection('', self::TWO_SITES); + + $this->assertFalse($this->apiService->isConnectedToSite('user1', 'site-a.atlassian.net')); + } + + public function testSitesWithoutAnIdOrAUrlAreSkipped(): void { + $this->stubOAuthConnection('encrypted-token', '[{"id":"cloud-a"},{"url":"https://site-b.atlassian.net"},' + . '{"id":7,"url":"https://site-c.atlassian.net"},{"id":"cloud-d","url":["https://site-d.atlassian.net"]}]'); + $this->networkService->expects($this->never())->method('oauthRequest'); + + foreach (['site-a', 'site-b', 'site-c', 'site-d'] as $site) { + $this->assertFalse($this->apiService->isConnectedToSite('user1', $site . '.atlassian.net'), $site); + $this->assertNull($this->apiService->getIssueInfo('user1', 'NC-1', $site . '.atlassian.net'), $site); + } + } + + /** + * @dataProvider provideHostsOfAnApiTokenSite + */ + public function testAUserConnectedWithAnApiTokenIsConnectedOnlyToItsSite(string $host, bool $connected): void { + $this->stubBasicAuthConnection('https://site-a.atlassian.net/'); + + $this->assertSame($connected, $this->apiService->isConnectedToSite('user1', $host)); + } + + public static function provideHostsOfAnApiTokenSite(): array { + return [ + 'its site' => ['site-a.atlassian.net', true], + 'upper case' => ['SITE-A.atlassian.net', true], + 'another site' => ['site-b.atlassian.net', false], + 'a host ending in its site host' => ['xsite-a.atlassian.net', false], + 'its site host inside another host' => ['site-a.atlassian.net.atlassian.net', false], + 'a subdomain of its site host' => ['x.site-a.atlassian.net', false], + 'the end of its site host' => ['a.atlassian.net', false], + ]; + } + + /** + * @dataProvider provideConnectedUrlsOfASite + */ + public function testTheConnectedUrlIsComparedByItsHost(string $jiraUrl): void { + $this->stubBasicAuthConnection($jiraUrl); + + $this->assertTrue($this->apiService->isConnectedToSite('user1', 'site-a.atlassian.net')); + } + + public static function provideConnectedUrlsOfASite(): array { + return [ + 'upper case' => ['https://SITE-A.Atlassian.NET'], + 'a trailing dot' => ['https://site-a.atlassian.net./'], + 'a path' => ['https://site-a.atlassian.net/jira'], + ]; + } + + /** + * A renamed site, for example: connected under its old name, Jira's base URL has the new one. + */ + public function testAnApiTokenIsConnectedToTheConnectedHostAndToTheBaseUrlHost(): void { + $this->stubApiTokenConnection('https://old-name.atlassian.net', 'https://new-name.atlassian.net'); + + $this->assertTrue($this->apiService->isConnectedToSite('user1', 'old-name.atlassian.net')); + $this->assertTrue($this->apiService->isConnectedToSite('user1', 'new-name.atlassian.net')); + } + + public function testAConnectedHostThatOnlyStartsWithASiteHostIsNotThatSite(): void { + $this->stubBasicAuthConnection('https://site-a.atlassian.net.example.com'); + + $this->assertFalse($this->apiService->isConnectedToSite('user1', 'site-a.atlassian.net')); + } + + public function testAnAuthorisedSiteIsComparedByTheHostOfItsUrl(): void { + $this->stubOAuthConnection('encrypted-token', '[{"id":"cloud-a","url":"https://Site-A.Atlassian.NET./","name":"Site A"}]'); + + $this->assertTrue($this->apiService->isConnectedToSite('user1', 'site-a.atlassian.net')); + } + + /** + * Gateway URLs of different sites share their host, so the forced instance is compared as a URL. + */ + private function stubGatewayConnectionWithAForcedOne(string $cloudId, string $baseUrl): void { + $this->storeLazyAppValues(['forced_instance_url' => 'https://api.atlassian.com/ex/jira/cloud-a']); + $this->crypto->method('decrypt')->with('encrypted')->willReturn('dXNlcjpwYXNz'); + $this->config->method('getUserValue')->willReturnCallback( + fn ($userId, $appName, $key, $default = '') => match ($key) { + 'basic_auth_header' => 'encrypted', + 'url' => 'https://api.atlassian.com/ex/jira/' . $cloudId, + 'base_url' => $baseUrl, + default => $default, + } + ); + } + + public function testTheForcedInstanceGetsItsIssues(): void { + $this->stubGatewayConnectionWithAForcedOne('cloud-a', 'https://site-a.atlassian.net'); + $issue = ['key' => 'NC-1', 'fields' => ['summary' => 'An issue']]; + $this->networkService->expects($this->once()) + ->method('basicRequest') + ->with('https://api.atlassian.com/ex/jira/cloud-a', 'dXNlcjpwYXNz', 'rest/api/3/issue/NC-1') + ->willReturn($issue); + + $this->assertSame($issue, $this->apiService->getIssueInfo('user1', 'NC-1', 'site-a.atlassian.net')); + } + + public function testAnotherGatewaySiteThanTheForcedOneIsRefused(): void { + $this->stubGatewayConnectionWithAForcedOne('cloud-b', 'https://site-b.atlassian.net'); + $this->networkService->expects($this->never())->method('basicRequest'); + + $this->assertSame(['error' => 'Unauthorized Jira instance URL'], $this->apiService->getIssueInfo('user1', 'NC-1', 'site-b.atlassian.net')); + } + + public function testAnApiTokenWithoutAUrlIsConnectedToNoSite(): void { + $this->stubBasicAuthConnection(''); + + $this->assertFalse($this->apiService->isConnectedToSite('user1', 'site-a.atlassian.net')); + } + + /** + * @dataProvider provideHostsOfAGatewayConnection + */ + public function testAnApiTokenIsConnectedToTheHostOfTheKeptBaseUrl(string $baseUrl, string $host, bool $connected): void { + $this->stubApiTokenConnection('https://api.atlassian.com/ex/jira/cloud-a', $baseUrl); + + $this->assertSame($connected, $this->apiService->isConnectedToSite('user1', $host)); + } + + public static function provideHostsOfAGatewayConnection(): array { + return [ + 'the site of the base URL' => ['https://site-a.atlassian.net', 'site-a.atlassian.net', true], + 'a base URL in upper case' => ['https://Site-A.Atlassian.NET/', 'site-a.atlassian.net', true], + 'a base URL with a trailing dot' => ['https://site-a.atlassian.net./', 'site-a.atlassian.net', true], + 'another site' => ['https://site-a.atlassian.net', 'site-b.atlassian.net', false], + 'a host ending in the site host' => ['https://site-a.atlassian.net', 'xsite-a.atlassian.net', false], + 'the site host inside another host' => ['https://site-a.atlassian.net', 'site-a.atlassian.net.atlassian.net', false], + 'a subdomain of the site host' => ['https://site-a.atlassian.net', 'x.site-a.atlassian.net', false], + 'the end of the site host' => ['https://site-a.atlassian.net', 'a.atlassian.net', false], + 'no base URL kept' => ['', 'site-a.atlassian.net', false], + 'no host and no base URL kept' => ['', '', false], + ]; + } + + /** + * Resources are whatever the user's settings hold, and the link preview asks for every Jira link. + * + * @dataProvider provideResourcesThatAreNoArray + */ + public function testResourcesThatAreNoArrayAreNoSite(string $resources): void { + $this->stubOAuthConnection('encrypted-token', $resources); + + $this->assertFalse($this->apiService->isConnectedToSite('user1', 'site-a.atlassian.net')); + } + + public static function provideResourcesThatAreNoArray(): array { + return [ + 'a number' => ['5'], + 'true' => ['true'], + 'a string' => ['"https://site-a.atlassian.net"'], + 'no JSON' => ['site-a'], + ]; + } + + public function testAUserWithoutAJiraAccountIsConnectedToNoSite(): void { + $this->stubUserConfig('', ''); + + $this->assertFalse($this->apiService->isConnectedToSite('user1', 'site-a.atlassian.net')); + } + + /** + * @dataProvider provideHostsOfTwoOAuthSitesWithTheirCloudIds + */ + public function testAnIssueIsAskedOnlyFromTheOAuthSiteOfItsLink(string $host, string $cloudId): void { + $this->stubOAuthConnection('encrypted-token', self::TWO_SITES); + $issue = ['key' => 'NC2T-1', 'fields' => ['summary' => 'An issue']]; + $this->networkService->expects($this->once()) + ->method('oauthRequest') + ->with('user1', 'ex/jira/' . $cloudId . '/rest/api/3/issue/NC2T-1') + ->willReturn($issue); + + $this->assertSame($issue, $this->apiService->getIssueInfo('user1', 'NC2T-1', $host)); + } + + public static function provideHostsOfTwoOAuthSitesWithTheirCloudIds(): array { + return [ + 'the first site' => ['site-a.atlassian.net', 'cloud-a'], + 'the second site' => ['site-b.atlassian.net', 'cloud-b'], + ]; + } + + public function testAnIssueOfAnotherSiteIsNotAskedWithOAuth(): void { + $this->stubOAuthConnection('encrypted-token', self::TWO_SITES); + $this->networkService->expects($this->never())->method('oauthRequest'); + + $this->assertNull($this->apiService->getIssueInfo('user1', 'NC-1', 'site-c.atlassian.net')); + } + + public function testAnIssueIsAskedFromTheSiteOfAnApiToken(): void { + $this->stubBasicAuthConnection('https://site-a.atlassian.net'); + $issue = ['key' => 'NC2T-1', 'fields' => ['summary' => 'An issue']]; + $this->networkService->expects($this->once()) + ->method('basicRequest') + ->with('https://site-a.atlassian.net', 'dXNlcjpwYXNz', 'rest/api/3/issue/NC2T-1') + ->willReturn($issue); + + $this->assertSame($issue, $this->apiService->getIssueInfo('user1', 'NC2T-1', 'site-a.atlassian.net')); + } + + public function testAnIssueOfAnotherSiteIsNotAskedWithAnApiToken(): void { + $this->stubApiTokenConnection('https://site-a.atlassian.net', 'https://site-a.atlassian.net'); + $this->networkService->expects($this->never())->method('basicRequest'); + + $this->assertNull($this->apiService->getIssueInfo('user1', 'NC-1', 'site-b.atlassian.net')); + } + + /** + * The link preview asks for every Jira link in every app whether the user is connected to its + * site, so the answer comes from the stored values alone: no request, no decryption. + * + * @dataProvider provideForcedInstances + */ + public function testTheSiteCheckOfAnApiTokenAsksNothing(string $forcedInstanceUrl): void { + $this->storeLazyAppValues(['forced_instance_url' => $forcedInstanceUrl]); + $this->config->method('getUserValue')->willReturnCallback( + fn ($userId, $appName, $key, $default = '') => match ($key) { + 'basic_auth_header' => 'encrypted', + 'url' => 'https://api.atlassian.com/ex/jira/cloud-a', + default => $default, + } + ); + $this->crypto->expects($this->never())->method('decrypt'); + $this->networkService->expects($this->never())->method('basicRequest'); + + $this->assertFalse($this->apiService->isConnectedToSite('user1', 'site-a.atlassian.net')); + $this->assertTrue($this->apiService->isConnectedToSite('user1', 'api.atlassian.com')); + } + + public static function provideForcedInstances(): array { + return [ + 'no forced instance' => [''], + 'another forced instance' => ['https://forced.example.com'], + ]; + } + + /** + * A user connected with an API token to the given URL, with the base URL Jira gave and an OAuth + * token as stored. + */ + private function stubApiTokenConnection(string $jiraUrl, string $baseUrl = '', string $token = ''): void { + $this->storeLazyAppValues([]); + $this->crypto->method('decrypt')->with('encrypted')->willReturn('dXNlcjpwYXNz'); + $this->config->method('getUserValue')->willReturnCallback( + fn ($userId, $appName, $key, $default = '') => match ($key) { + 'basic_auth_header' => 'encrypted', + 'url' => $jiraUrl, + 'base_url' => $baseUrl, + 'token' => $token, + 'resources' => self::TWO_SITES, + default => $default, + } + ); + } + + /** + * Atlassian's API gateway, which scoped API tokens need, serves a Jira Cloud site under + * api.atlassian.com, while its issue links name the site. + */ + public function testAnApiTokenThroughTheGatewayIsConnectedToTheSiteOfTheKeptBaseUrl(): void { + $this->stubApiTokenConnection('https://api.atlassian.com/ex/jira/cloud-a', 'https://site-a.atlassian.net'); + $issue = ['key' => 'NC-1', 'fields' => ['summary' => 'An issue']]; + $this->networkService->expects($this->once()) + ->method('basicRequest') + ->with('https://api.atlassian.com/ex/jira/cloud-a', 'dXNlcjpwYXNz', 'rest/api/3/issue/NC-1') + ->willReturn($issue); + + $this->assertTrue($this->apiService->isConnectedToSite('user1', 'site-a.atlassian.net')); + $this->assertFalse($this->apiService->isConnectedToSite('user1', 'site-b.atlassian.net')); + $this->assertSame($issue, $this->apiService->getIssueInfo('user1', 'NC-1', 'site-a.atlassian.net')); + } + + public function testAnApiTokenWinsOverAStoredOAuthToken(): void { + $this->stubApiTokenConnection('https://site-a.atlassian.net', token: 'encrypted-token'); + $issue = ['key' => 'NC-1', 'fields' => ['summary' => 'An issue']]; + $this->networkService->expects($this->never())->method('oauthRequest'); + $this->networkService->expects($this->once()) + ->method('basicRequest') + ->with('https://site-a.atlassian.net', 'dXNlcjpwYXNz', 'rest/api/3/issue/NC-1') + ->willReturn($issue); + + $this->assertSame($issue, $this->apiService->getIssueInfo('user1', 'NC-1', 'site-a.atlassian.net')); + } + + /** + * @dataProvider provideAnswersThatAreNoIssue + */ + public function testAnAnswerThatIsNoIssueIsPassedOnOnlyAsAnError(array $answer, ?array $expected): void { + $this->stubOAuthConnection('encrypted-token', self::TWO_SITES); + $this->networkService->method('oauthRequest')->willReturn($answer); + + $this->assertSame($expected, $this->apiService->getIssueInfo('user1', 'NC-1', 'site-a.atlassian.net')); + } + + public static function provideAnswersThatAreNoIssue(): array { + return [ + 'an error' => [['error' => 'Bad credentials'], ['error' => 'Bad credentials']], + 'no fields' => [['key' => 'NC-1'], null], + ]; + } + public static function provideSelfHostedRequests(): array { return [ 'notifications' => ['getNotifications', ['user1']], 'projects' => ['getProjects', ['user1']], 'search' => ['search', ['user1', 'crash']], - 'issue info' => ['getIssueInfo', ['user1', 'NC-1']], + 'issue info' => ['getIssueInfo', ['user1', 'NC-1', 'jira.example.com']], 'account info' => ['getAccountInfo', ['user1', '', 'jdoe']], ]; }