Skip to content

Commit 551af58

Browse files
AltamashShaikhAltamashShaikh
committed
Improves Slack ChannelID validation to allow only alphanumeric values
1 parent 6a6602c commit 551af58

File tree

4 files changed

+119
-3
lines changed

4 files changed

+119
-3
lines changed

‎Slack.php‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ public function validateReportParameters(&$parameters, $reportType)
116116
} elseif (empty($parameters[self::SLACK_CHANNEL_ID_PARAMETER])) {
117117
throw new \Exception(Piwik::translate('Slack_SlackChannelIdRequiredErrorMessage'));
118118
}
119+
$slackChannels = explode(',', (string) $parameters[self::SLACK_CHANNEL_ID_PARAMETER]);
120+
foreach ($slackChannels as $slackChannel) {
121+
if (!ctype_alnum($slackChannel)) {
122+
throw new \Exception(Piwik::translate('Slack_SlackChannelIdInvalidErrorMessage'));
123+
}
124+
}
119125
}
120126

121127
/**
@@ -384,8 +390,15 @@ public function uninstall()
384390
*/
385391
public function validateCustomAlertReportParameters($parameters, $alertMedium)
386392
{
387-
if ($alertMedium === self::SLACK_TYPE && empty($parameters[self::SLACK_CHANNEL_ID_PARAMETER])) {
388-
throw new \Exception(Piwik::translate('Slack_SlackChannelIdRequiredErrorMessage'));
393+
if ($alertMedium === self::SLACK_TYPE) {
394+
$settings = StaticContainer::get(SystemSettings::class);
395+
if (empty($settings->slackOauthToken->getValue())) {
396+
throw new \Exception(Piwik::translate('Slack_OauthTokenRequiredErrorMessage'));
397+
} elseif (empty($parameters[self::SLACK_CHANNEL_ID_PARAMETER])) {
398+
throw new \Exception(Piwik::translate('Slack_SlackChannelIdRequiredErrorMessage'));
399+
} elseif (!ctype_alnum($parameters[self::SLACK_CHANNEL_ID_PARAMETER])) {
400+
throw new \Exception(Piwik::translate('Slack_SlackChannelIdInvalidErrorMessage'));
401+
}
389402
}
390403
}
391404

‎lang/en.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"OauthTokenSettingDescription": "Enter your Slack OAuth Token generated from your Slack. %1$sLearn more%2$s.",
88
"PleaseFindYourReport": "Here is your %1$s report for %2$s",
99
"SlackChannelIdRequiredErrorMessage": "Slack Channel ID cannot be empty.",
10+
"SlackChannelIdInvalidErrorMessage": "Invalid Slack Channel ID, only alphanumeric values are allowed.",
1011
"SlackChannel": "Slack Channel",
1112
"SlackEnterYourSlackChannelIdHelpText": "Enter the Slack Channel ID of the channel that will receive these reports. To find the ID, go to Slack and open the channel details > About tab. %1$sLearn more%2$s",
1213
"SlackAlertContent": "%1$s has been triggered for website %2$s as the metric %3$s in report %4$s %5$s."

‎tests/Integration/CustomAlertsApiTest.php‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
namespace Piwik\Plugins\Slack\tests;
1111

12+
use Piwik\Container\StaticContainer;
13+
use Piwik\Plugins\Slack\SystemSettings;
1214
use Piwik\Tests\Framework\Fixture;
1315
use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
1416
use Piwik\Tests\Framework\TestingEnvironmentManipulator;
@@ -43,13 +45,41 @@ public function setUp(): void
4345
$this->idSite = Fixture::createWebsite('2012-08-09 11:22:33');
4446
}
4547

48+
public function testAddAlertShouldThrowExceptionIfSlackOauthTokenNotSet()
49+
{
50+
$this->expectException(\Exception::class);
51+
$this->expectExceptionMessage('Slack_OauthTokenRequiredErrorMessage');
52+
$this->addAlert('', false);
53+
}
54+
55+
public function testAddAlertShouldThrowExceptionIfSlackChannelIdNotEmptyButOauthTokenNotSet()
56+
{
57+
$this->expectException(\Exception::class);
58+
$this->expectExceptionMessage('Slack_OauthTokenRequiredErrorMessage');
59+
$this->addAlert('channelID', false);
60+
}
61+
4662
public function testAddAlertShouldThrowExceptionIfEmptySlackChannelId()
4763
{
4864
$this->expectException(\Exception::class);
4965
$this->expectExceptionMessage('Slack_SlackChannelIdRequiredErrorMessage');
5066
$this->addAlert();
5167
}
5268

69+
public function testAddAlertShouldThrowExceptionIfInvalidSlackChannelId()
70+
{
71+
$this->expectException(\Exception::class);
72+
$this->expectExceptionMessage('Slack_SlackChannelIdInvalidErrorMessage');
73+
$this->addAlert('ChanneldId1@123');
74+
}
75+
76+
public function testAddAlertShouldThrowExceptionIfInvalidSlackChannelId2()
77+
{
78+
$this->expectException(\Exception::class);
79+
$this->expectExceptionMessage('Slack_SlackChannelIdInvalidErrorMessage');
80+
$this->addAlert('ChannelID1,ChannelID2');
81+
}
82+
5383
public function testAddAlertSuccess()
5484
{
5585
$id = $this->addAlert($slackChannelId = 'channelID');
@@ -76,8 +106,13 @@ public function testUpdateAlertSuccess()
76106
$this->assertEquals('channelIDNew', $alert['slack_channel_id']);
77107
}
78108

79-
private function addAlert($slackChannelId = '')
109+
private function addAlert($slackChannelId = '', $createToken = true)
80110
{
111+
if ($createToken) {
112+
$settings = StaticContainer::get(SystemSettings::class);
113+
$settings->slackOauthToken->setValue('test123');
114+
}
115+
81116
return $this->api->addAlert(
82117
'Test Slack and Email',
83118
$this->idSite,

‎tests/Integration/SlackTest.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
namespace Piwik\Plugins\Slack\tests;
1111

1212
use Piwik\Container\StaticContainer;
13+
use Piwik\Piwik;
1314
use Piwik\Plugins\ScheduledReports\API;
1415
use Piwik\Plugins\ScheduledReports\ScheduledReports;
1516
use Piwik\Plugins\Slack\Slack;
@@ -113,6 +114,72 @@ public function getAlertData()
113114
];
114115
}
115116

117+
public function testValidateReportParametersSlackTokenNotSetShouldThrowOauthException()
118+
{
119+
$this->expectException(\Exception::class);
120+
$this->expectExceptionMessage('Slack_OauthTokenRequiredErrorMessage');
121+
$parameters = [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_GRAPHS_ONLY];
122+
Piwik::postEvent('ScheduledReports.validateReportParameters', [&$parameters, 'slack']);
123+
}
124+
125+
public function testValidateReportParametersSlackTokenNotSetShouldThrowSlackChannelIdEmptyException()
126+
{
127+
$settings = StaticContainer::get(SystemSettings::class);
128+
$settings->slackOauthToken->setValue('test123');
129+
$this->expectException(\Exception::class);
130+
$this->expectExceptionMessage('Slack_SlackChannelIdRequiredErrorMessage');
131+
$parameters = [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_GRAPHS_ONLY];
132+
Piwik::postEvent('ScheduledReports.validateReportParameters', [&$parameters, 'slack']);
133+
}
134+
135+
public function testValidateReportParametersSlackTokenNotSetShouldThrowSlackChannelIdEmptyException2()
136+
{
137+
$settings = StaticContainer::get(SystemSettings::class);
138+
$settings->slackOauthToken->setValue('test123');
139+
$this->expectException(\Exception::class);
140+
$this->expectExceptionMessage('Slack_SlackChannelIdRequiredErrorMessage');
141+
$parameters = [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_GRAPHS_ONLY, Slack::SLACK_CHANNEL_ID_PARAMETER => ''];
142+
Piwik::postEvent('ScheduledReports.validateReportParameters', [&$parameters, 'slack']);
143+
}
144+
145+
public function testValidateReportParametersSlackTokenNotSetShouldThrowSlackChannelIdInvalidException()
146+
{
147+
$settings = StaticContainer::get(SystemSettings::class);
148+
$settings->slackOauthToken->setValue('test123');
149+
$this->expectException(\Exception::class);
150+
$this->expectExceptionMessage('Slack_SlackChannelIdInvalidErrorMessage');
151+
$parameters = [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_GRAPHS_ONLY, Slack::SLACK_CHANNEL_ID_PARAMETER => 'test123@11'];
152+
Piwik::postEvent('ScheduledReports.validateReportParameters', [&$parameters, 'slack']);
153+
}
154+
155+
public function testValidateReportParametersSlackTokenNotSetShouldThrowSlackChannelIdInvalidException2()
156+
{
157+
$settings = StaticContainer::get(SystemSettings::class);
158+
$settings->slackOauthToken->setValue('test123');
159+
$this->expectException(\Exception::class);
160+
$this->expectExceptionMessage('Slack_SlackChannelIdInvalidErrorMessage');
161+
$parameters = [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_GRAPHS_ONLY, Slack::SLACK_CHANNEL_ID_PARAMETER => 'test123,Demo@11'];
162+
Piwik::postEvent('ScheduledReports.validateReportParameters', [&$parameters, 'slack']);
163+
}
164+
165+
public function testValidateReportParametersSlackTokenShouldNotThrowAnyException()
166+
{
167+
$settings = StaticContainer::get(SystemSettings::class);
168+
$settings->slackOauthToken->setValue('test123');
169+
$parameters = [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_GRAPHS_ONLY, Slack::SLACK_CHANNEL_ID_PARAMETER => 'test123'];
170+
Piwik::postEvent('ScheduledReports.validateReportParameters', [&$parameters, 'slack']);
171+
$this->assertNotEmpty($parameters);
172+
}
173+
174+
public function testValidateReportParametersSlackTokenShouldNotThrowAnyException2()
175+
{
176+
$settings = StaticContainer::get(SystemSettings::class);
177+
$settings->slackOauthToken->setValue('test123');
178+
$parameters = [ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_GRAPHS_ONLY, Slack::SLACK_CHANNEL_ID_PARAMETER => 'test123,dEmoA,abcd123'];
179+
Piwik::postEvent('ScheduledReports.validateReportParameters', [&$parameters, 'slack']);
180+
$this->assertNotEmpty($parameters);
181+
}
182+
116183
private function assertHasReport($login, $idSite)
117184
{
118185
$report = $this->getReport($login, $idSite);

0 commit comments

Comments
 (0)