Skip to content

Commit

Permalink
feature #39606 [Notifier] [Slack] Validate token syntax (OskarStark)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 5.3-dev branch.

Discussion
----------

[Notifier] [Slack] Validate token syntax

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | ---
| License       | MIT
| Doc PR        | -

This PR follows symfony/symfony#39560

@odolbeau @malteschlueter @norkunas @fabpot can you confirm all your tokens start with `xox`?

_From the Slack documentation:_
* Bot user token strings begin with `xoxb-`
* User token strings begin with `xoxp-`
* Workspace access token strings begin with `xoxa-2`

Commits
-------

59f29c592b [Notifier] [Slack] Validate token syntax
  • Loading branch information
fabpot committed Dec 28, 2020
2 parents 4a508c3 + c332a0a commit 373ee7c
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ CHANGELOG
* The bridge is not marked as `@experimental` anymore
* Check for maximum number of buttons in Slack action block
* Add HeaderBlock
* Slack access tokens needs to start with "xox" (see https://api.slack.com/authentication/token-types)

5.2.0
-----
Expand Down
5 changes: 5 additions & 0 deletions SlackTransport.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Notifier\Bridge\Slack;

use Symfony\Component\Notifier\Exception\InvalidArgumentException;
use Symfony\Component\Notifier\Exception\LogicException;
use Symfony\Component\Notifier\Exception\TransportException;
use Symfony\Component\Notifier\Exception\UnsupportedMessageTypeException;
Expand All @@ -33,6 +34,10 @@ final class SlackTransport extends AbstractTransport

public function __construct(string $accessToken, string $channel = null, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null)
{
if (!preg_match('/^xox(b-|p-|a-2)/', $accessToken)) {
throw new InvalidArgumentException('A valid Slack token needs to start with "xoxb-", "xoxp-" or "xoxa-2". See https://api.slack.com/authentication/token-types for further information.');
}

$this->accessToken = $accessToken;
$this->chatChannel = $channel;
$this->client = $client;
Expand Down
4 changes: 2 additions & 2 deletions Tests/SlackTransportFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function testCreateWithDsn()
{
$factory = $this->createFactory();

$transport = $factory->create(Dsn::fromString('slack://testUser@host.test/?channel=testChannel'));
$transport = $factory->create(Dsn::fromString('slack://xoxb-TestUser@host.test/?channel=testChannel'));

$this->assertSame('slack://host.test?channel=testChannel', (string) $transport);
}
Expand All @@ -33,7 +33,7 @@ public function testCreateWithDsnWithoutPath()
{
$factory = $this->createFactory();

$transport = $factory->create(Dsn::fromString('slack://testUser@host.test?channel=testChannel'));
$transport = $factory->create(Dsn::fromString('slack://xoxb-TestUser@host.test?channel=testChannel'));

$this->assertSame('slack://host.test?channel=testChannel', (string) $transport);
}
Expand Down
27 changes: 18 additions & 9 deletions Tests/SlackTransportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\Notifier\Bridge\Slack\SlackOptions;
use Symfony\Component\Notifier\Bridge\Slack\SlackTransport;
use Symfony\Component\Notifier\Exception\InvalidArgumentException;
use Symfony\Component\Notifier\Exception\LogicException;
use Symfony\Component\Notifier\Exception\TransportException;
use Symfony\Component\Notifier\Exception\UnsupportedMessageTypeException;
Expand All @@ -31,23 +32,31 @@ public function testToStringContainsProperties()
{
$channel = 'test Channel'; // invalid channel name to test url encoding of the channel

$transport = new SlackTransport('testToken', $channel, $this->createMock(HttpClientInterface::class));
$transport = new SlackTransport('xoxb-TestToken', $channel, $this->createMock(HttpClientInterface::class));
$transport->setHost('host.test');

$this->assertSame('slack://host.test?channel=test+Channel', (string) $transport);
}

public function testInstatiatingWithAnInvalidSlackTokenThrowsInvalidArgumentException()
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('A valid Slack token needs to start with "xoxb-", "xoxp-" or "xoxa-2". See https://api.slack.com/authentication/token-types for further information.');

new SlackTransport('token', 'testChannel', $this->createMock(HttpClientInterface::class));
}

public function testSupportsChatMessage()
{
$transport = new SlackTransport('testToken', 'testChannel', $this->createMock(HttpClientInterface::class));
$transport = new SlackTransport('xoxb-TestToken', 'testChannel', $this->createMock(HttpClientInterface::class));

$this->assertTrue($transport->supports(new ChatMessage('testChatMessage')));
$this->assertFalse($transport->supports($this->createMock(MessageInterface::class)));
}

public function testSendNonChatMessageThrowsLogicException()
{
$transport = new SlackTransport('testToken', 'testChannel', $this->createMock(HttpClientInterface::class));
$transport = new SlackTransport('xoxb-TestToken', 'testChannel', $this->createMock(HttpClientInterface::class));

$this->expectException(UnsupportedMessageTypeException::class);

Expand All @@ -70,7 +79,7 @@ public function testSendWithEmptyArrayResponseThrows()
return $response;
});

$transport = new SlackTransport('testToken', 'testChannel', $client);
$transport = new SlackTransport('xoxb-TestToken', 'testChannel', $client);

$transport->send(new ChatMessage('testMessage'));
}
Expand All @@ -93,14 +102,14 @@ public function testSendWithErrorResponseThrows()
return $response;
});

$transport = new SlackTransport('testToken', 'testChannel', $client);
$transport = new SlackTransport('xoxb-TestToken', 'testChannel', $client);

$transport->send(new ChatMessage('testMessage'));
}

public function testSendWithOptions()
{
$token = 'testToken';
$token = 'xoxb-TestToken';
$channel = 'testChannel';
$message = 'testMessage';

Expand Down Expand Up @@ -129,7 +138,7 @@ public function testSendWithOptions()

public function testSendWithNotification()
{
$token = 'testToken';
$token = 'xoxb-TestToken';
$channel = 'testChannel';
$message = 'testMessage';

Expand Down Expand Up @@ -172,14 +181,14 @@ public function testSendWithInvalidOptions()
return $this->createMock(ResponseInterface::class);
});

$transport = new SlackTransport('testToken', 'testChannel', $client);
$transport = new SlackTransport('xoxb-TestToken', 'testChannel', $client);

$transport->send(new ChatMessage('testMessage', $this->createMock(MessageOptionsInterface::class)));
}

public function testSendWith200ResponseButNotOk()
{
$token = 'testToken';
$token = 'xoxb-TestToken';
$channel = 'testChannel';
$message = 'testMessage';

Expand Down

0 comments on commit 373ee7c

Please sign in to comment.