OpenConext / Stepup-Middleware

Stepup Middleware
Apache License 2.0
3 stars 2 forks source link

Wrap address and name into an Address #377

Closed MKodde closed 2 years ago

MKodde commented 2 years ago

The to and from methods of the TemplatedEmail ask for addresses as its parameter. They are resolved quite magically. Setting a string with a plain mail address is possible, adding multiple parameters is cool too.

The issue we ran into was that the addres param was followed by a common name string. Both where resoved as being mail addresses. Causing the application to blow up on the common name. The solution is to provide Address objects. An address consists of an address and a name.

See: https://www.pivotaltracker.com/n/projects/1163646/stories/183362518 See: https://symfony.com/doc/4.4/mailer.html#email-addresses See: #352 work was missing in that PR that made the mailer malfunction.

:warning: Alert! This PR was merged prematurely. It will be reopened in a new PR. As dependency injection issues are turning their head

MKodde commented 2 years ago

@phavekes I have not tested the bugfix in my environment. But I'm quite certain this resolves the issue(s). Can you verify this on test2?

tvdijen commented 2 years ago

I think maybe a case was missed or something.. It's not a file that was changed in this PR, but definitely related...

Sep 29 17:35:35 sv2010531 STEPUP-MIDDLEWARE[7676]: {"channel":"app","level":"CRITICAL","message":"Call to a member function getEmail() on string","context":{"exception":{"class":"Error","message":"Call to a member function getEmail() on string","code":0,"file":"/apps/installation/Stepup-Middleware/Stepup-Middleware-5.0.1/src/Surfnet/StepupMiddleware/CommandHandlingBundle/Identity/Service/EmailVerificationMailService.php:140"}},"extra":{"server":"middleware.stepup-tst.ssc-ict.overheid-i.nl","application":"middleware","request_id":"18c6ff9d398eae1c965d3fd07a52f7df"}}

I'm kinda surprised this passed CI.. Any static analysis tool would have caught this? However, after fixing those, I end up with something harder to fix;

Sep 29 17:55:55 sv2010532 STEPUP-MIDDLEWARE[11599]: {"channel":"app","level":"CRITICAL","message":"A \"Symfony\Bridge\Twig\Mime\TemplatedEmail\" context cannot have an \"email\" entry as this is a reserved variable.","context":{"exception":{"class":"Symfony\Component\Mime\Exception\InvalidArgumentException","message":"A \"Symfony\Bridge\Twig\Mime\TemplatedEmail\" context cannot have an \"email\" entry as this is a reserved variable.","code":0,"file":"/apps/installation/Stepup-Middleware/Stepup-Middleware-5.0.1/vendor/symfony/twig-bridge/Mime/BodyRenderer.php:58"}},"extra":{"art":"34934","server":"middleware.stepup-tst.ssc-ict.overheid-i.nl","application":"middleware","request_id":"85ece1a355fd1209c66830e3b478bc87"}}