feat: implement Oauth2 transport creation in SMTPMailService - #1729
feat: implement Oauth2 transport creation in SMTPMailService#1729gnyiri wants to merge 18 commits into
Conversation
jekabs-karklins
left a comment
jekabs-karklins
left a comment
There was a problem hiding this comment.
Hello @gnyiri ,
I have verified that the changes in this PR still works fine in our development environment and emails are properly sent out👍🏻
I have added two comments that would be very nice to address before merging in.
| } | ||
|
|
||
| private async createTransport() { | ||
| if ( |
There was a problem hiding this comment.
Maybe we can have a env like USE_SMTP_OAUTH_2 so that its easier and more straight forward to switch between the two.
mutambaraf
left a comment
mutambaraf
left a comment
There was a problem hiding this comment.
Just also did a manual email check and our emails are not being send on my local machine .Having error "{"error":{"code":"ESOCKET","command":"CONN"}".
| if (!this.authToken?.expiresOn) { | ||
| throw new Error('Invalid token: Missing expiresOn property'); | ||
| } |
There was a problem hiding this comment.
I think we can do this check when we set authToken " this.authToken = await cca.acquireTokenByClientCredential(tokenRequest);
if (!this.authToken || !this.authToken.accessToken) {
throw new Error('Failed to get access token');
}"
jekabs-karklins
left a comment
jekabs-karklins
left a comment
There was a problem hiding this comment.
Looks good from my side but @mutambaraf has some more good comments
3 participants
At ELI we move from SMTP basic auth to Oauth2.
Add Oauth2 transport creation in SMTP Mail Service.
See above.
How Has This Been Tested
Manual tests + backend tests.
Fixes
N/A
Changes
SMTP Mail Service.
Depends on
N/A
Tests included/Docs Updated?