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']], ]; }