diff --git a/API.php b/API.php index bd519e6..28721df 100644 --- a/API.php +++ b/API.php @@ -12,7 +12,6 @@ use Piwik\Piwik; use Piwik\Plugins\ApiReference\Generation\PluginListProvider; use Piwik\Plugin\Manager; -use Piwik\Plugins\ApiReference\Specs\SpecGenerator; use Piwik\Plugins\ApiReference\Specs\PathResolver; /** @@ -25,6 +24,8 @@ */ class API extends \Piwik\Plugin\API { + private const TRY_IT_OUT_NOTE_TRANSLATION_KEY = 'ApiReference_UseTryItOutForLiveResponse'; + /** * Returns the plugin names used for ApiReference spec generation. * @@ -87,9 +88,81 @@ public function getOpenApiSpec(string $pluginName, string $format = 'json'): arr throw new \Exception('OpenAPI spec file contains invalid JSON.'); } + if (!Piwik::hasUserSuperUserAccess()) { + $decodedSpec = $this->removeSuccessfulResponseExamples($decodedSpec); + } + return $decodedSpec; } + /** + * Remove embedded example payloads from successful 200 responses. + * + * @param array $spec + * @return array + */ + protected function removeSuccessfulResponseExamples(array $spec): array + { + if (empty($spec['paths']) || !is_array($spec['paths'])) { + return $spec; + } + + foreach ($spec['paths'] as &$pathItem) { + if (!is_array($pathItem)) { + continue; + } + + foreach ($pathItem as &$operation) { + if (!is_array($operation)) { + continue; + } + + $this->sanitizeSuccessfulResponse($operation); + } + unset($operation); + } + unset($pathItem); + + return $spec; + } + + /** + * Remove examples from a successful 200 response and append the try-it-out note once. + * + * @param array $operation + */ + protected function sanitizeSuccessfulResponse(array &$operation): void + { + if (empty($operation['responses']) || !is_array($operation['responses'])) { + return; + } + + if (!isset($operation['responses']['200']) || !is_array($operation['responses']['200'])) { + return; + } + + $successfulResponse = &$operation['responses']['200']; + + if ( + !empty($successfulResponse['description']) + && is_string($successfulResponse['description']) + && strpos($successfulResponse['description'], $this->getTryItOutNote()) === false + ) { + $successfulResponse['description'] .= $this->getTryItOutNote(); + } + + if (empty($successfulResponse['content']) || !is_array($successfulResponse['content'])) { + return; + } + + foreach ($successfulResponse['content'] as &$content) { + if (is_array($content)) { + unset($content['example'], $content['examples'], $content['schema']); + } + } + unset($content); + } + protected function getSpecFilePath(string $pluginName): string { return $this->getSpecPathResolver()->getSpecFilePath($pluginName); @@ -121,6 +194,11 @@ protected function validateJsonFormat(string $format): void } } + protected function getTryItOutNote(): string + { + return "\n\n" . Piwik::translate(self::TRY_IT_OUT_NOTE_TRANSLATION_KEY); + } + protected function getSpecPathResolver(): PathResolver { return new PathResolver(); @@ -130,31 +208,4 @@ protected function getPluginListProvider(): PluginListProvider { return new PluginListProvider(); } - - /** - * Generates an OpenAPI specification for one or more plugins and returns it immediately. - * - * @param string $plugin The plugin name to generate, or a comma-separated list of plugin names. - * @param string $format The response format to generate. Supported values are `json` and `yaml`. - * @return array|string The generated OpenAPI specification as decoded JSON data for - * `json`, or as a YAML string for `yaml`. - */ - public function getGeneratedOpenApiSpec(string $plugin, string $format) - { - Piwik::checkUserHasSomeViewAccess(); - - // Return an error if format is something other than JSON or YAML - $allowedFormats = ['json', 'yaml']; - if (!in_array(strtolower($format), $allowedFormats)) { - throw new \Exception( - Piwik::translate( - 'General_ExceptionInvalidReportRendererFormat', - [$format, implode(', ', $allowedFormats)] - ) - ); - } - - $docString = (new SpecGenerator())->generatePluginDoc($plugin, $format); - return strtolower($format) === 'json' ? json_decode($docString, true) : $docString; - } } diff --git a/Annotations/AnnotationGenerator.php b/Annotations/AnnotationGenerator.php index a6ef232..8216e24 100644 --- a/Annotations/AnnotationGenerator.php +++ b/Annotations/AnnotationGenerator.php @@ -20,6 +20,7 @@ use Piwik\API\NoDefaultValue; use Piwik\API\Proxy; use Piwik\API\Request; +use Piwik\Development; use Piwik\Http; use Piwik\Piwik; use Piwik\Plugin\Manager; @@ -108,7 +109,7 @@ public function __construct( DocumentationGenerator $generator, ?PathResolver $pathResolver = null, ?ArtifactWriter $artifactWriter = null, - bool $allowLocalRequests = false + bool $allowLocalRequests = true ) { $this->generator = $generator; $this->pathResolver = $pathResolver ?? new PathResolver(); @@ -1026,7 +1027,7 @@ protected function getDemoReportMetadata(): array $file = null, $followDepth = 0, $acceptLanguage = false, - $acceptInvalidSslCertificate = true, + $acceptInvalidSslCertificate = $this->shouldAcceptInvalidSslCertificate(), $byteRange = false, $getExtendedInfo = true, $httpMethod = 'GET' @@ -1102,7 +1103,7 @@ protected function getExampleIfAvailable(string $url, bool $useLocalToken = fals $file = null, $followDepth = 0, $acceptLanguage = false, - $acceptInvalidSslCertificate = true, + $acceptInvalidSslCertificate = $this->shouldAcceptInvalidSslCertificate(), $byteRange = false, $getExtendedInfo = true, $httpMethod = 'GET' @@ -1192,6 +1193,11 @@ protected function writeFile(string $filePath, string $contents) return $this->artifactWriter->writeFile($filePath, $contents); } + protected function shouldAcceptInvalidSslCertificate(): bool + { + return Development::isEnabled(); + } + protected function getInstanceUrl(): string { return rtrim(SettingsPiwik::getPiwikUrl(), '/') . '/'; @@ -1398,7 +1404,7 @@ protected function determineResponses(array $rules, string $plugin, string $meth $exampleValue = $this->getExampleIfAvailable($url); // If the example lookup failed, try making the same request locally using a local token. if (empty($exampleValue)) { - if ($this->allowLocalRequests) { + if ($this->shouldAllowLocalRequests()) { $exampleValue = $this->getExampleIfAvailable($url, true); } } @@ -2182,4 +2188,12 @@ protected function shouldUseParameterLevelExample(array $typesMap, string $examp return is_array(json_decode($example, true)); } + + protected function shouldAllowLocalRequests(): bool + { + $allowLocalRequests = $this->allowLocalRequests; + Piwik::postEvent('ApiReference.shouldAllowLocalRequests', [&$allowLocalRequests]); + + return $allowLocalRequests; + } } diff --git a/lang/en.json b/lang/en.json index 7bc68f8..115b685 100644 --- a/lang/en.json +++ b/lang/en.json @@ -8,6 +8,7 @@ "SwaggerPageRequestFailed": "The plugin whitelist could not be loaded.", "SwaggerPageSearchNoResults": "No plugins match your search.", "SwaggerPageSearchPlaceholder": "Search by plugin name", + "UseTryItOutForLiveResponse": "Example responses require Super User access. Use Try it out to see a live response.", "SwaggerPageSpecLoadFailed": "The OpenAPI spec could not be loaded for this plugin.", "SwaggerPageSpecNotAvailable": "The OpenAPI spec is not available for this plugin yet. %1$sLearn more%2$s", "UserAuthentication": "User authentication", diff --git a/tests/Resources/MockAnnotationGenerator.php b/tests/Resources/MockAnnotationGenerator.php index e0d6248..6dd20e1 100644 --- a/tests/Resources/MockAnnotationGenerator.php +++ b/tests/Resources/MockAnnotationGenerator.php @@ -16,9 +16,9 @@ class MockAnnotationGenerator extends AnnotationGenerator { - public function __construct(DocumentationGenerator $generator) + public function __construct(DocumentationGenerator $generator, bool $allowLocalRequests = true) { - parent::__construct($generator); + parent::__construct($generator, null, null, $allowLocalRequests); // TODO - Extend the constructor behaviour } @@ -110,4 +110,14 @@ public function shouldUseParameterLevelExample(array $typesMap, string $example) { return parent::shouldUseParameterLevelExample($typesMap, $example); } + + public function shouldAcceptInvalidSslCertificate(): bool + { + return parent::shouldAcceptInvalidSslCertificate(); + } + + public function shouldAllowLocalRequests(): bool + { + return parent::shouldAllowLocalRequests(); + } } diff --git a/tests/Unit/APITest.php b/tests/Unit/APITest.php index 70a6387..52f215c 100644 --- a/tests/Unit/APITest.php +++ b/tests/Unit/APITest.php @@ -16,6 +16,7 @@ use PHPUnit\Framework\TestCase; use Piwik\Access; use Piwik\Container\StaticContainer; +use Piwik\Piwik; use Piwik\Plugins\ApiReference\API; use Piwik\Plugins\ApiReference\Generation\PluginListProvider; use Piwik\Plugins\ApiReference\Specs\PathResolver; @@ -62,6 +63,111 @@ public function testGetOpenApiSpecReturnsDecodedJsonForPlugin() $this->assertSame($expectedSpec, $result); } + public function testGetOpenApiSpecKeepsSuccessfulExamplesForSuperUsers(): void + { + StaticContainer::getContainer()->set(Access::class, new FakeAccess(true, [], [1], 'superUser')); + + $expectedSpec = $this->getSpecFixtureWithResponseExamples(); + $api = $this->buildApiMock('/tmp/CustomAlerts_openapi_spec_v1.0.0.json', true, json_encode($expectedSpec)); + + $result = $api->getOpenApiSpec('CustomAlerts'); + + $this->assertSame($expectedSpec, $result); + } + + public function testGetOpenApiSpecRemovesOnlySuccessfulExamplesForUsersWithoutSiteOneAccess(): void + { + StaticContainer::getContainer()->set(Access::class, new FakeAccess(false, [], [2], 'otherViewer')); + + $api = $this->buildApiMock( + '/tmp/CustomAlerts_openapi_spec_v1.0.0.json', + true, + json_encode($this->getSpecFixtureWithResponseExamples()) + ); + $expectedTryItOutNote = $this->callProtectedMethod($api, 'getTryItOutNote'); + + $result = $api->getOpenApiSpec('CustomAlerts'); + + $this->assertArrayNotHasKey('example', $result['paths']['/endpoint']['get']['responses']['200']['content']['application/json']); + $this->assertArrayNotHasKey('examples', $result['paths']['/endpoint']['get']['responses']['200']['content']['application/json']); + $this->assertArrayNotHasKey('schema', $result['paths']['/endpoint']['get']['responses']['200']['content']['application/json']); + $this->assertSame( + 'kept error example', + $result['paths']['/endpoint']['get']['responses']['400']['content']['application/json']['example'] + ); + $this->assertSame( + ['type' => 'string', 'example' => 'stay put'], + $result['paths']['/endpoint']['get']['parameters'][0]['schema'] + ); + $this->assertSame( + ['value' => ['id' => 99]], + $result['paths']['/endpoint']['get']['requestBody']['content']['application/json']['examples']['request'] + ); + $this->assertSame( + 'Success' . $expectedTryItOutNote, + $result['paths']['/endpoint']['get']['responses']['200']['description'] + ); + } + + public function testGetOpenApiSpecDoesNotDuplicateTryItOutNote(): void + { + StaticContainer::getContainer()->set(Access::class, new FakeAccess(false, [], [2], 'otherViewer')); + + $spec = $this->getSpecFixtureWithResponseExamples(); + $api = $this->buildApiMock('/tmp/CustomAlerts_openapi_spec_v1.0.0.json', true, json_encode($spec)); + $expectedTryItOutNote = $this->callProtectedMethod($api, 'getTryItOutNote'); + $spec['paths']['/endpoint']['get']['responses']['200']['description'] .= $expectedTryItOutNote; + $api->method('readSpecFile')->willReturn(json_encode($spec)); + + $result = $api->getOpenApiSpec('CustomAlerts'); + + $this->assertSame( + 'Success' . $expectedTryItOutNote, + $result['paths']['/endpoint']['get']['responses']['200']['description'] + ); + } + + public function testRemoveSuccessfulResponseExamplesLeavesOperationWithoutResponsesUnchanged(): void + { + $api = new API(); + $spec = [ + 'paths' => [ + '/endpoint' => [ + 'get' => [ + 'summary' => 'No responses here', + ], + ], + ], + ]; + + $this->assertSame($spec, $this->callProtectedMethod($api, 'removeSuccessfulResponseExamples', [$spec])); + } + + public function testRemoveSuccessfulResponseExamplesLeavesOperationWithoutSuccessfulResponseUnchanged(): void + { + $api = new API(); + $spec = [ + 'paths' => [ + '/endpoint' => [ + 'get' => [ + 'responses' => [ + '400' => [ + 'description' => 'Error', + 'content' => [ + 'application/json' => [ + 'example' => 'keep me', + ], + ], + ], + ], + ], + ], + ], + ]; + + $this->assertSame($spec, $this->callProtectedMethod($api, 'removeSuccessfulResponseExamples', [$spec])); + } + public function testGetAllowedPluginsReturnsProviderValues(): void { $provider = $this->createMock(PluginListProvider::class); @@ -123,7 +229,9 @@ public function testGetOpenApiSpecThrowsExceptionWhenFormatIsInvalid() $api = $this->buildApiMock('/tmp/CustomAlerts_openapi_spec_v1.0.0.json', true, '{}'); $this->expectException(\Exception::class); - $this->expectExceptionMessage("Report format 'yaml' not valid"); + $this->expectExceptionMessage( + Piwik::translate('General_ExceptionInvalidReportRendererFormat', ['yaml', 'json']) + ); $api->getOpenApiSpec('CustomAlerts', 'yaml'); } @@ -171,6 +279,77 @@ private function buildApiMock(string $filePath, bool $isReadable, $fileContents return $api; } + /** + * @return array + */ + private function getSpecFixtureWithResponseExamples(): array + { + return [ + 'openapi' => '3.1.0', + 'paths' => [ + '/endpoint' => [ + 'get' => [ + 'parameters' => [ + [ + 'name' => 'label', + 'schema' => [ + 'type' => 'string', + 'example' => 'stay put', + ], + ], + ], + 'requestBody' => [ + 'content' => [ + 'application/json' => [ + 'examples' => [ + 'request' => [ + 'value' => ['id' => 99], + ], + ], + ], + ], + ], + 'responses' => [ + '200' => [ + 'description' => 'Success', + 'content' => [ + 'application/json' => [ + 'schema' => [ + 'type' => 'object', + 'properties' => [ + 'row' => [ + 'type' => 'array', + 'items' => [ + 'type' => 'object', + 'additionalProperties' => true, + ], + ], + ], + ], + 'example' => ['value' => 'remove me'], + 'examples' => [ + 'success' => [ + 'value' => ['another' => 'remove me'], + ], + ], + ], + ], + ], + '400' => [ + 'description' => 'Error', + 'content' => [ + 'application/json' => [ + 'example' => 'kept error example', + ], + ], + ], + ], + ], + ], + ], + ]; + } + /** * @param object $object * @param string $methodName diff --git a/tests/Unit/AnnotationGeneratorTest.php b/tests/Unit/AnnotationGeneratorTest.php index 8029726..15bfecb 100644 --- a/tests/Unit/AnnotationGeneratorTest.php +++ b/tests/Unit/AnnotationGeneratorTest.php @@ -16,6 +16,9 @@ use PHPUnit\Framework\TestCase; use Piwik\API\DocumentationGenerator; use Piwik\API\NoDefaultValue; +use Piwik\Config; +use Piwik\Development; +use Piwik\Piwik; use Piwik\Plugins\ApiReference\Annotations\AnnotationGenerator; use Piwik\Plugins\ApiReference\ApiReference; use Piwik\Plugins\ApiReference\tests\Resources\MockAnnotationGenerator; @@ -174,11 +177,25 @@ class AnnotationGeneratorTest extends TestCase */ private static $exampleSchemas; + /** + * @var bool + */ + private static $disableLocalRequestsByEvent = false; + /** * @var AnnotationGenerator */ private $annotationGenerator; + public static function setUpBeforeClass(): void + { + Piwik::addAction('ApiReference.shouldAllowLocalRequests', function (&$allowLocalRequests): void { + if (self::$disableLocalRequestsByEvent) { + $allowLocalRequests = false; + } + }); + } + public function setUp(): void { $this->annotationGenerator = new AnnotationGenerator(new DocumentationGenerator()); @@ -370,6 +387,33 @@ public function getExampleIfAvailable(string $url, bool $useLocalToken = false, ); } + public function testShouldAllowLocalRequestsDefaultsToTrue(): void + { + $annotationGenerator = new MockAnnotationGenerator(new DocumentationGenerator()); + + $this->assertTrue($annotationGenerator->shouldAllowLocalRequests()); + } + + public function testShouldAllowLocalRequestsCanBeDisabledByConstructor(): void + { + $annotationGenerator = new MockAnnotationGenerator(new DocumentationGenerator(), false); + + $this->assertFalse($annotationGenerator->shouldAllowLocalRequests()); + } + + public function testShouldAllowLocalRequestsCanBeDisabledByEvent(): void + { + self::$disableLocalRequestsByEvent = true; + + try { + $annotationGenerator = new MockAnnotationGenerator(new DocumentationGenerator()); + + $this->assertFalse($annotationGenerator->shouldAllowLocalRequests()); + } finally { + self::$disableLocalRequestsByEvent = false; + } + } + public function testGetParamInfoFromDocBlock(): void { // TODO - Update to use resource file and/or dataprovider to test more than one comment block @@ -1319,6 +1363,25 @@ public function testShouldUseParameterLevelExampleForScalarArrayUnions(): void $this->assertFalse($annotationGenerator->shouldUseParameterLevelExample(['string' => null, 'array' => 'string'], 'one')); } + public function testShouldAcceptInvalidSslCertificateMatchesDevelopmentMode(): void + { + $annotationGenerator = new MockAnnotationGenerator(new DocumentationGenerator()); + $defaultValue = Config::getInstance()->Development['enabled'] ?? 0; + + try { + Config::getInstance()->Development['enabled'] = 0; + $this->resetDevelopmentModeCache(); + $this->assertFalse($annotationGenerator->shouldAcceptInvalidSslCertificate()); + + Config::getInstance()->Development['enabled'] = 1; + $this->resetDevelopmentModeCache(); + $this->assertTrue($annotationGenerator->shouldAcceptInvalidSslCertificate()); + } finally { + Config::getInstance()->Development['enabled'] = $defaultValue; + $this->resetDevelopmentModeCache(); + } + } + /** * @dataProvider getTestDataForWrapStringWithQuotes * @@ -1415,4 +1478,11 @@ public function testCompileOperationLines(): void // TODO - compileOperationLines method $this->expectNotToPerformAssertions(); } + + private function resetDevelopmentModeCache(): void + { + $reflection = new \ReflectionProperty(Development::class, 'isEnabled'); + $reflection->setAccessible(true); + $reflection->setValue(null, null); + } }