Skip to content

[Mailer] Add MSGraph API Transport #52546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[Mailer] Add MSGraph API Transport #52546

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5388c84
initial commit
nguyenk Nov 10, 2023
5dd6b58
fix CI
nguyenk Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix CI
  • Loading branch information
@nguyenk
nguyenk committed Nov 10, 2023
commit 5dd6b58e1ac83668c92a71632b326a0d4c7e1881
3 changes: 1 addition & 2 deletions ...onent/Mailer/Bridge/MicrosoftGraph/Tests/Transport/MicrosoftGraphTransportFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,11 @@ public static function incompleteDsnProvider(): iterable
yield [new Dsn('microsoft+graph', 'default', null, null)];
}

public function testInvalidDsnHost(): void
public function testInvalidDsnHost()
{
$factory = $this->getFactory();

$this->expectException(InvalidArgumentException::class);
$factory->create(new Dsn('microsoft+graph', 'some-wrong-national-cloud', self::USER, self::PASSWORD, null, ['tenant' => self::TENANT]));
}

}
16 changes: 6 additions & 10 deletions src/Symfony/Component/Mailer/Bridge/MicrosoftGraph/Transport/MicrosoftGraphTransport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

namespace Symfony\Component\Mailer\Bridge\MicrosoftGraph\Transport;

use GuzzleHttp\Exception\ClientException;
use GuzzleHttp\Psr7\Stream;
use GuzzleHttp\Psr7\Utils;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using guzzle/psr7 without depending on it is a no-go.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it looks like the microsoft graph SDK supports any PSR-7 stream, the transport should rather take a StreamFactory in its constructor.

use Microsoft\Graph\Generated\Models\BodyType;
Expand All @@ -26,7 +25,6 @@
use Microsoft\Graph\Generated\Users\Item\SendMail\SendMailPostRequestBody;
use Microsoft\Graph\GraphServiceClient;
use Microsoft\Kiota\Authentication\Oauth\ClientCredentialContext;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you instantiate this class directly which comes from a different package, you should depend on it explicitly instead of relying on the transitive dependency in the microsoft SDK to avoid issues if new major versions are done.

use Safe\Exceptions\JsonException;
use Symfony\Component\Mailer\Bridge\MicrosoftGraph\Exception\SenderNotFoundException;
use Symfony\Component\Mailer\Bridge\MicrosoftGraph\Exception\SendMailException;
use Symfony\Component\Mailer\Envelope;
Expand All @@ -39,8 +37,6 @@
use Symfony\Component\Mime\RawMessage;
use Symfony\Contracts\Cache\CacheInterface;

use function Safe\json_decode;

class MicrosoftGraphTransport implements TransportInterface
{
private GraphServiceClient $graphServiceClient;
Expand Down Expand Up @@ -68,7 +64,6 @@ public function send(RawMessage $message, Envelope $envelope = null): ?SentMessa
throw new SendEmailError(sprintf("This mailer can only handle mails of class '%s' or it's subclasses, instance of %s passed", Email::class, $message::class));
}


$this->sendMail($message);

return new SentMessage($message, $envelope);
Expand All @@ -85,7 +80,7 @@ private function sendMail(Email $message): void
try {
$this->graphServiceClient->users()->byUserId($senderAddress)->sendMail()->post($body)->wait();
} catch (ODataError $error) {
if ('ErrorInvalidUser' === $error->getError()->getCode()){
if ('ErrorInvalidUser' === $error->getError()->getCode()) {
throw new SenderNotFoundException("Sender email address '".$senderAddress."' could not be found when calling the Graph API. This is usually because the email address doesn't exist in the tenant.", 404, $error);
}
throw new SendMailException('Something went wrong while sending email', $error->getCode(), $error);
Expand All @@ -104,19 +99,19 @@ private function convertEmailToGraphMessage(Email $source): Message
$message->setFrom(self::convertAddressToGraphRecipient($source->getFrom()[0]));

// to
$message->setToRecipients(\array_map(
$message->setToRecipients(array_map(
static fn (Address $address) => self::convertAddressToGraphRecipient($address),
$source->getTo()
));

// CC
$message->setCcRecipients(\array_map(
$message->setCcRecipients(array_map(
static fn (Address $address) => self::convertAddressToGraphRecipient($address),
$source->getCc()
));

// BCC
$message->setBccRecipients(\array_map(
$message->setBccRecipients(array_map(
static fn (Address $address) => self::convertAddressToGraphRecipient($address),
$source->getBcc()
));
Expand All @@ -128,7 +123,7 @@ private function convertEmailToGraphMessage(Email $source): Message
$itemBody->setContentType(new BodyType(BodyType::HTML));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic looks broken in case the Email only has a plaintext body.

$message->setBody($itemBody);

$message->setAttachments(\array_map(
$message->setAttachments(array_map(
static fn (DataPart $attachment) => self::convertAttachmentGraphAttachment($attachment),
$source->getAttachments()
));
Expand All @@ -143,6 +138,7 @@ private static function convertAddressToGraphRecipient(Address $source): Recipie
$emailAddress->setAddress($source->getAddress());
$emailAddress->setName($source->getName());
$recipient->setEmailAddress($emailAddress);

return $recipient;
}

Expand Down
11 changes: 5 additions & 6 deletions ...mfony/Component/Mailer/Bridge/MicrosoftGraph/Transport/MicrosoftGraphTransportFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
namespace Symfony\Component\Mailer\Bridge\MicrosoftGraph\Transport;

use Microsoft\Graph\Core\NationalCloud;
use phpDocumentor\Reflection\Exception\PcreException;
use Symfony\Component\Mailer\Exception\IncompleteDsnException;
use Symfony\Component\Mailer\Exception\InvalidArgumentException;
use Symfony\Component\Mailer\Exception\UnsupportedSchemeException;
Expand All @@ -28,9 +27,9 @@ final class MicrosoftGraphTransportFactory extends AbstractTransportFactory
private const CLOUD_MAP = [
'default' => NationalCloud::GLOBAL,
'germany' => NationalCloud::GERMANY,
'china' => NationalCloud::CHINA,
'us-dod' => NationalCloud::US_DOD,
'us-gov' => NationalCloud::US_GOV,
'china' => NationalCloud::CHINA,
'us-dod' => NationalCloud::US_DOD,
'us-gov' => NationalCloud::US_GOV,
];

public function __construct(
Expand All @@ -56,8 +55,8 @@ public function create(Dsn $dsn): TransportInterface
if (null === $tenantId) {
throw new IncompleteDsnException("Transport 'microsoft+graph' requires the 'tenant' option");
}
if (!isset(self::CLOUD_MAP[$dsn->getHost()])){
throw new InvalidArgumentException(sprintf("Transport 'microsoft+graph' one of these hosts : '%s'", implode(", ", self::CLOUD_MAP)));
if (!isset(self::CLOUD_MAP[$dsn->getHost()])) {
throw new InvalidArgumentException(sprintf("Transport 'microsoft+graph' one of these hosts : '%s'", implode(', ', self::CLOUD_MAP)));
}

// This parses the MAILER_DSN containing Microsoft Graph API credentials
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment looks wrong to me.

Expand Down