[Mailer] Add MSGraph API Transport #52546

nguyenk · 2023-11-10T18:33:09Z

Q	A
Branch?	7.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	N/A
License	MIT

This PR is initially comes from #48888 : Microsoft now forces OAuth for sending emails in SMTP which does not seem possible with the current mailer implementation.

I created a gist for a workaround

And I ended up proposing this PR after @xabbuh proposed

This is a Microsoft Graph API implementation, not an SMTP based.

carsonbot · 2023-11-10T18:33:15Z

Hey!

Thanks for your PR. You are targeting branch "5.4" but it seems your PR description refers to branch "6.4 for features".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

carsonbot · 2023-11-10T18:33:18Z

It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you.

Cheers!

Carsonbot

stof · 2023-11-10T18:42:13Z

This cannot go in the 6.4 branch (nor 7.0 btw). We are 1.5 month after the feature freeze.

nguyenk · 2023-11-10T18:45:14Z

This cannot go in the 6.4 branch (nor 7.0 btw). We are 1.5 month after the feature freeze.

@stof sorry, this is my first contrib. What should I do then ?

stof · 2023-11-10T19:23:47Z

@nguyenk the 7.1 branch is not yet created to open it to contributions, because we would prefer the community to focus on stabilizing the current beta versions. We create the next branch only once the RC versions are released. So you will need to rebase your work once that happens.

stof

This PR also misses the registration of the new bridge in FrameworkBundle and in \Symfony\Component\Mailer\Transport::FACTORY_CLASSES to make it usable through the DSN system.

stof · 2023-11-10T18:43:29Z

src/Symfony/Component/Mailer/Bridge/MicrosoftGraph/Transport/MicrosoftGraphTransport.php

+namespace Symfony\Component\Mailer\Bridge\MicrosoftGraph\Transport;
+
+use GuzzleHttp\Psr7\Stream;
+use GuzzleHttp\Psr7\Utils;


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

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

stof · 2023-11-10T18:46:39Z

src/Symfony/Component/Mailer/Bridge/MicrosoftGraph/Transport/MicrosoftGraphTransport.php

+        $tokenRequestContext = new ClientCredentialContext(
+            $this->tenantId,
+            $this->clientId,
+            $this->clientSecret


Are those properties used elsewhere than here ? If no, they should not be properties at all.

Btw, I'm wondering whether we should really take those scalars here vs injecting directly the actual dependencies (when using the factory, the instantiation would happen there). This would be more consistent with the way other bridges with dependencies are implemented (see the amazon bridge for instance)

stof · 2023-11-10T18:48:02Z

src/Symfony/Component/Mailer/Bridge/MicrosoftGraph/Transport/MicrosoftGraphTransport.php

+        $message->setSubject($source->getSubject() ?? 'No subject');
+        $itemBody = new ItemBody();
+        $itemBody->setContent((string) $source->getHtmlBody());
+        $itemBody->setContentType(new BodyType(BodyType::HTML));


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

stof · 2023-11-10T18:52:09Z

src/Symfony/Component/Mailer/Bridge/MicrosoftGraph/Transport/MicrosoftGraphTransport.php

+use Microsoft\Graph\Generated\Models\Recipient;
+use Microsoft\Graph\Generated\Users\Item\SendMail\SendMailPostRequestBody;
+use Microsoft\Graph\GraphServiceClient;
+use Microsoft\Kiota\Authentication\Oauth\ClientCredentialContext;


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.

stof · 2023-11-10T18:55:53Z

src/Symfony/Component/Mailer/Bridge/MicrosoftGraph/Transport/MicrosoftGraphTransportFactory.php

+            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


this comment looks wrong to me.

stof · 2023-11-10T18:56:45Z

src/Symfony/Component/Mailer/Bridge/MicrosoftGraph/composer.json

+    "require": {
+        "php": ">=8.1",
+        "symfony/mailer": "^5.4|^6.4|^7.0",
+        "thecodingmachine/safe": "^2.5.0",


Why this dependency ?

stof · 2023-11-10T18:57:39Z

src/Symfony/Component/Mailer/Bridge/MicrosoftGraph/phpunit.xml.dist

+<?xml version="1.0" encoding="UTF-8"?>
+
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.4/phpunit.xsd"


this looks wrong to me, given that our testsuite does not use PHPUnit 10 yet (as our tooling is not yet compatible with it)

stof · 2023-11-10T18:59:03Z

src/Symfony/Component/Mailer/Bridge/MicrosoftGraph/Transport/MicrosoftGraphTransport.php

+        private readonly string $tenantId,
+        private readonly string $clientId,
+        private readonly string $clientSecret,
+        private readonly CacheInterface $cache,


this cache looks unused.

nguyenk · 2023-11-13T06:18:39Z

Thank you a lot for you feedback @stof I will have a close look ASAP.

Cheers

pounard · 2024-02-20T17:23:52Z

Hello, this is kind of related, I opened a PR #54013 for a new component proposal which is about managing access tokens (OAuth2, and possibly some others). I think this might interest you @nguyenk since Microsoft access tokens are OAuth2 tokens.

I created this PR after I tried to implement connection to Office 365 / Outlook with the SMTP component, and really think it would worth it to share/aggregate our respective work and patches in order to move in the direction to a more generic, user configurable and re-usable solution.

@stof I know that you are a meticulous, attentive reviewer, I'd very much appreciate your sentiments about this PR as well.

Thanks in advance to all of you.

JoppeDC · 2024-04-09T11:25:52Z

@nguyenk This seems like a pretty promising bridge! This is also a friendly ping to see how things are going and if you need any help :)

nguyenk · 2024-04-23T06:45:55Z

Hello @pounard thank you for the hint. I would definitively look into this PR and use it once (I hope) it's merged.

BTW, big 🆙 for you're work on that 👏

pounard · 2024-04-23T07:25:12Z

Hello @pounard thank you for the hint. I would definitively look into this PR and use it once (I hope) it's merged.

BTW, big 🆙 for you're work on that 👏

Thanks ! It seems it's hard to get traction from core team when you don't have a name in the community, so any help for pushing it forward would be great.

OskarStark · 2024-04-23T07:45:45Z

Hi, as we are currently in feature freeze for 7.1 there is not so much attention on following topics, sorry for that.

AndreasA · 2024-09-26T11:05:01Z

any news regarding this? will it be in 7.2 or 7.3?

creiner · 2025-04-14T09:00:14Z

Are there any news regarding this PR?
Microsoft annouced the retirement of smtp basic auth in sept 2025.

https://techcommunity.microsoft.com/blog/exchange/exchange-online-to-retire-basic-auth-for-client-submission-smtp-auth/4114750

OskarStark · 2025-04-14T10:30:28Z

Feel free to take over and finish this PR 👍🏻

creiner · 2025-04-15T07:02:04Z

If this is ok for @nguyenk I could have a look at it?

nguyenk · 2025-04-15T07:17:56Z

@creiner thanks for asking. Please do, it's been too long I didn't make any move on this one.

Thank you.

xabbuh · 2025-05-08T09:26:45Z

closing here as the PR looks stalled, feel free to reopen when you have time to finish it

AndreasA · 2025-05-09T12:39:31Z

@xabbuh it would still be great to get this feature before Symfony 7.4 release as Microsoft will soon remove normal SMTP altogether.

What were the TODOs in this PR and @creiner did you take over the PR / create a new PR for this feature?

creiner · 2025-05-13T07:23:30Z

I implemented the comments and opened a new PR #60408 for continuing the work.

nguyenk requested review from lyrixx, yceruto, chalasr, dunglas, OskarStark and xabbuh as code owners November 10, 2023 18:33

carsonbot added Status: Needs Review Feature Mailer labels Nov 10, 2023

carsonbot added this to the 5.4 milestone Nov 10, 2023

carsonbot changed the title ~~[MAILER] - Add MSGraph API Transpoirt~~ [Mailer] - Add MSGraph API Transpoirt Nov 10, 2023

nguyenk changed the base branch from 5.4 to 6.4 November 10, 2023 18:33

initial commit

5388c84

nguyenk changed the title ~~[Mailer] - Add MSGraph API Transpoirt~~ [Mailer] - Add MSGraph API Transport Nov 10, 2023

fix CI

5dd6b58

nguyenk force-pushed the mailer-component/add-microsoft-graph-api-mailer-bridge branch from d71c385 to 5dd6b58 Compare November 10, 2023 18:41

stof requested changes Nov 10, 2023

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Nov 10, 2023

OskarStark changed the title ~~[Mailer] - Add MSGraph API Transport~~ [Mailer] Add MSGraph API Transport Nov 13, 2023

nicolas-grekas mentioned this pull request Dec 29, 2023

Mailer : add microsoft graph api mailer bridge #52512

Closed

nicolas-grekas modified the milestones: 5.4, 7.1 Dec 29, 2023

nguyenk mentioned this pull request May 12, 2024

[Mailer] Support OAuth2 #48888

Open

xabbuh modified the milestones: 7.1, 7.2 May 15, 2024

fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024

xabbuh closed this May 8, 2025

creiner mentioned this pull request May 13, 2025

[Mailer] Add MSGraph API Transport #60408

Open

Uh oh!

[Mailer] Add MSGraph API Transport #52546

[Mailer] Add MSGraph API Transport #52546

Uh oh!

Conversation

nguyenk commented Nov 10, 2023 • edited by OskarStark Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carsonbot commented Nov 10, 2023

Uh oh!

carsonbot commented Nov 10, 2023

Uh oh!

stof commented Nov 10, 2023

Uh oh!

nguyenk commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stof commented Nov 10, 2023

Uh oh!

stof left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nguyenk commented Nov 13, 2023

Uh oh!

pounard commented Feb 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoppeDC commented Apr 9, 2024

Uh oh!

nguyenk commented Apr 23, 2024

Uh oh!

pounard commented Apr 23, 2024

Uh oh!

OskarStark commented Apr 23, 2024

Uh oh!

AndreasA commented Sep 26, 2024

Uh oh!

creiner commented Apr 14, 2025

Uh oh!

OskarStark commented Apr 14, 2025

Uh oh!

creiner commented Apr 15, 2025

Uh oh!

nguyenk commented Apr 15, 2025

Uh oh!

xabbuh commented May 8, 2025

Uh oh!

AndreasA commented May 9, 2025

Uh oh!

creiner commented May 13, 2025

Uh oh!

Uh oh!

nguyenk commented Nov 10, 2023 •

edited by OskarStark

Loading

nguyenk commented Nov 10, 2023 •

edited

Loading

pounard commented Feb 20, 2024 •

edited

Loading