Skip to content

Commit c1c55c3

Browse files
lachiebollachiebol
authored
Security changes, #PG-5170 (#42)
* limited invalid ssl to development mode, gating responses behind view access * Super user access now required, now getting responses using superuser token * cleaned up code * fix test
1 parent 51a5c5a commit c1c55c3

6 files changed

Lines changed: 360 additions & 35 deletions

File tree

‎API.php‎

Lines changed: 79 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use Piwik\Piwik;
1313
use Piwik\Plugins\ApiReference\Generation\PluginListProvider;
1414
use Piwik\Plugin\Manager;
15-
use Piwik\Plugins\ApiReference\Specs\SpecGenerator;
1615
use Piwik\Plugins\ApiReference\Specs\PathResolver;
1716

1817
/**
@@ -25,6 +24,8 @@
2524
*/
2625
class API extends \Piwik\Plugin\API
2726
{
27+
private const TRY_IT_OUT_NOTE_TRANSLATION_KEY = 'ApiReference_UseTryItOutForLiveResponse';
28+
2829
/**
2930
* Returns the plugin names used for ApiReference spec generation.
3031
*
@@ -87,9 +88,81 @@ public function getOpenApiSpec(string $pluginName, string $format = 'json'): arr
8788
throw new \Exception('OpenAPI spec file contains invalid JSON.');
8889
}
8990

91+
if (!Piwik::hasUserSuperUserAccess()) {
92+
$decodedSpec = $this->removeSuccessfulResponseExamples($decodedSpec);
93+
}
94+
9095
return $decodedSpec;
9196
}
9297

98+
/**
99+
* Remove embedded example payloads from successful 200 responses.
100+
*
101+
* @param array<string, mixed> $spec
102+
* @return array<string, mixed>
103+
*/
104+
protected function removeSuccessfulResponseExamples(array $spec): array
105+
{
106+
if (empty($spec['paths']) || !is_array($spec['paths'])) {
107+
return $spec;
108+
}
109+
110+
foreach ($spec['paths'] as &$pathItem) {
111+
if (!is_array($pathItem)) {
112+
continue;
113+
}
114+
115+
foreach ($pathItem as &$operation) {
116+
if (!is_array($operation)) {
117+
continue;
118+
}
119+
120+
$this->sanitizeSuccessfulResponse($operation);
121+
}
122+
unset($operation);
123+
}
124+
unset($pathItem);
125+
126+
return $spec;
127+
}
128+
129+
/**
130+
* Remove examples from a successful 200 response and append the try-it-out note once.
131+
*
132+
* @param array<string, mixed> $operation
133+
*/
134+
protected function sanitizeSuccessfulResponse(array &$operation): void
135+
{
136+
if (empty($operation['responses']) || !is_array($operation['responses'])) {
137+
return;
138+
}
139+
140+
if (!isset($operation['responses']['200']) || !is_array($operation['responses']['200'])) {
141+
return;
142+
}
143+
144+
$successfulResponse = &$operation['responses']['200'];
145+
146+
if (
147+
!empty($successfulResponse['description'])
148+
&& is_string($successfulResponse['description'])
149+
&& strpos($successfulResponse['description'], $this->getTryItOutNote()) === false
150+
) {
151+
$successfulResponse['description'] .= $this->getTryItOutNote();
152+
}
153+
154+
if (empty($successfulResponse['content']) || !is_array($successfulResponse['content'])) {
155+
return;
156+
}
157+
158+
foreach ($successfulResponse['content'] as &$content) {
159+
if (is_array($content)) {
160+
unset($content['example'], $content['examples'], $content['schema']);
161+
}
162+
}
163+
unset($content);
164+
}
165+
93166
protected function getSpecFilePath(string $pluginName): string
94167
{
95168
return $this->getSpecPathResolver()->getSpecFilePath($pluginName);
@@ -121,6 +194,11 @@ protected function validateJsonFormat(string $format): void
121194
}
122195
}
123196

197+
protected function getTryItOutNote(): string
198+
{
199+
return "\n\n" . Piwik::translate(self::TRY_IT_OUT_NOTE_TRANSLATION_KEY);
200+
}
201+
124202
protected function getSpecPathResolver(): PathResolver
125203
{
126204
return new PathResolver();
@@ -130,31 +208,4 @@ protected function getPluginListProvider(): PluginListProvider
130208
{
131209
return new PluginListProvider();
132210
}
133-
134-
/**
135-
* Generates an OpenAPI specification for one or more plugins and returns it immediately.
136-
*
137-
* @param string $plugin The plugin name to generate, or a comma-separated list of plugin names.
138-
* @param string $format The response format to generate. Supported values are `json` and `yaml`.
139-
* @return array<string, mixed>|string The generated OpenAPI specification as decoded JSON data for
140-
* `json`, or as a YAML string for `yaml`.
141-
*/
142-
public function getGeneratedOpenApiSpec(string $plugin, string $format)
143-
{
144-
Piwik::checkUserHasSomeViewAccess();
145-
146-
// Return an error if format is something other than JSON or YAML
147-
$allowedFormats = ['json', 'yaml'];
148-
if (!in_array(strtolower($format), $allowedFormats)) {
149-
throw new \Exception(
150-
Piwik::translate(
151-
'General_ExceptionInvalidReportRendererFormat',
152-
[$format, implode(', ', $allowedFormats)]
153-
)
154-
);
155-
}
156-
157-
$docString = (new SpecGenerator())->generatePluginDoc($plugin, $format);
158-
return strtolower($format) === 'json' ? json_decode($docString, true) : $docString;
159-
}
160211
}

‎Annotations/AnnotationGenerator.php‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Piwik\API\NoDefaultValue;
2121
use Piwik\API\Proxy;
2222
use Piwik\API\Request;
23+
use Piwik\Development;
2324
use Piwik\Http;
2425
use Piwik\Piwik;
2526
use Piwik\Plugin\Manager;
@@ -108,7 +109,7 @@ public function __construct(
108109
DocumentationGenerator $generator,
109110
?PathResolver $pathResolver = null,
110111
?ArtifactWriter $artifactWriter = null,
111-
bool $allowLocalRequests = false
112+
bool $allowLocalRequests = true
112113
) {
113114
$this->generator = $generator;
114115
$this->pathResolver = $pathResolver ?? new PathResolver();
@@ -1026,7 +1027,7 @@ protected function getDemoReportMetadata(): array
10261027
$file = null,
10271028
$followDepth = 0,
10281029
$acceptLanguage = false,
1029-
$acceptInvalidSslCertificate = true,
1030+
$acceptInvalidSslCertificate = $this->shouldAcceptInvalidSslCertificate(),
10301031
$byteRange = false,
10311032
$getExtendedInfo = true,
10321033
$httpMethod = 'GET'
@@ -1102,7 +1103,7 @@ protected function getExampleIfAvailable(string $url, bool $useLocalToken = fals
11021103
$file = null,
11031104
$followDepth = 0,
11041105
$acceptLanguage = false,
1105-
$acceptInvalidSslCertificate = true,
1106+
$acceptInvalidSslCertificate = $this->shouldAcceptInvalidSslCertificate(),
11061107
$byteRange = false,
11071108
$getExtendedInfo = true,
11081109
$httpMethod = 'GET'
@@ -1192,6 +1193,11 @@ protected function writeFile(string $filePath, string $contents)
11921193
return $this->artifactWriter->writeFile($filePath, $contents);
11931194
}
11941195

1196+
protected function shouldAcceptInvalidSslCertificate(): bool
1197+
{
1198+
return Development::isEnabled();
1199+
}
1200+
11951201
protected function getInstanceUrl(): string
11961202
{
11971203
return rtrim(SettingsPiwik::getPiwikUrl(), '/') . '/';
@@ -1398,7 +1404,7 @@ protected function determineResponses(array $rules, string $plugin, string $meth
13981404
$exampleValue = $this->getExampleIfAvailable($url);
13991405
// If the example lookup failed, try making the same request locally using a local token.
14001406
if (empty($exampleValue)) {
1401-
if ($this->allowLocalRequests) {
1407+
if ($this->shouldAllowLocalRequests()) {
14021408
$exampleValue = $this->getExampleIfAvailable($url, true);
14031409
}
14041410
}
@@ -2182,4 +2188,12 @@ protected function shouldUseParameterLevelExample(array $typesMap, string $examp
21822188

21832189
return is_array(json_decode($example, true));
21842190
}
2191+
2192+
protected function shouldAllowLocalRequests(): bool
2193+
{
2194+
$allowLocalRequests = $this->allowLocalRequests;
2195+
Piwik::postEvent('ApiReference.shouldAllowLocalRequests', [&$allowLocalRequests]);
2196+
2197+
return $allowLocalRequests;
2198+
}
21852199
}

‎lang/en.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"SwaggerPageRequestFailed": "The plugin whitelist could not be loaded.",
99
"SwaggerPageSearchNoResults": "No plugins match your search.",
1010
"SwaggerPageSearchPlaceholder": "Search by plugin name",
11+
"UseTryItOutForLiveResponse": "Example responses require Super User access. Use Try it out to see a live response.",
1112
"SwaggerPageSpecLoadFailed": "The OpenAPI spec could not be loaded for this plugin.",
1213
"SwaggerPageSpecNotAvailable": "The OpenAPI spec is not available for this plugin yet. %1$sLearn more%2$s",
1314
"UserAuthentication": "User authentication",

‎tests/Resources/MockAnnotationGenerator.php‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616

1717
class MockAnnotationGenerator extends AnnotationGenerator
1818
{
19-
public function __construct(DocumentationGenerator $generator)
19+
public function __construct(DocumentationGenerator $generator, bool $allowLocalRequests = true)
2020
{
21-
parent::__construct($generator);
21+
parent::__construct($generator, null, null, $allowLocalRequests);
2222

2323
// TODO - Extend the constructor behaviour
2424
}
@@ -110,4 +110,14 @@ public function shouldUseParameterLevelExample(array $typesMap, string $example)
110110
{
111111
return parent::shouldUseParameterLevelExample($typesMap, $example);
112112
}
113+
114+
public function shouldAcceptInvalidSslCertificate(): bool
115+
{
116+
return parent::shouldAcceptInvalidSslCertificate();
117+
}
118+
119+
public function shouldAllowLocalRequests(): bool
120+
{
121+
return parent::shouldAllowLocalRequests();
122+
}
113123
}

0 commit comments

Comments
 (0)