Add SMTP outbound support to the bridge. #4

abbyck · 2021-07-16T16:48:42Z

Currently uses hardcoded 'To' address on the outbound.
DKIM signing support is also added.

Signed-off-by: Abhinav Krishna C K me@abhy.me

* Currently uses hardcoded 'To' address on the outbound. * DKIM signing support is also added. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Half-Shot

Looking on the right path, I've added a bunch of comments that I'd like you to look at first, and I'll review the rest afterwards. I think generally you can get away with using more promises here which is a bit more modern and easier to read and maintain :)

config/config-sample.yaml

src/bridge.js

src/email/outbound.js

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Includes other cosmetic changes. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Remove devHost/devPort and decide on next hop based on smtpHost. Fix resloved MX records sorting. Unify tryConnect for resolvedMX and relaying hop. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Resolve email from userId. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Remove hardcoded 'to' address by getting the joined memebers and filtering out the email users. Also, 'from' address is calculated using the roomAlias of the corresponding bridged room. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Currently the display name is not set. This commit enables to set Matrix display name based on the 'from' header. If that's empty, email ID will be set as the display name. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Now the bridge will check for STARTSSL availability with connecting SMTP server and will try to upgrade the connection to use TLS (Opportunistic TLS). Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Half-Shot

This is looking great. I've added a bunch of nits, but it's all suggestions around code styling rather than actual concerns of the logic. This is really exciting to see :)

config/config-sample.yaml

Half-Shot · 2021-08-13T17:30:24Z

config/config-sample.yaml

+  dkimSelector: ''
+  # Enable Opportunistic TLS support for outbound emails.
+  startTLS: false
+  # TLS Key file location


If any of these are optional, I'd encourage you to put a hash before them and prefixthe description with "Optional: ". Helps us admins out

Half-Shot · 2021-08-13T17:34:14Z

config/email-config-schema.yaml

@@ -3,7 +3,7 @@ required: ["bridge"]
 properties:
    bridge:
        type: "object"
-        required: ["domain", "homeserverUrl", "mxDomain", "mailPort"]
+        required: ["domain", "homeserverUrl", "mxDomain", "mailPort", "dkimEnabled", "startTLS"]


I believe there is a way to do dependencies, so you must specify TLSKey / TLSCert if you enable startTLS (json-schema/json-schema#158 (comment)). It took me ages to Google for mind.

Another more straight-forward option is to just have a tls object which contains tlskey and tlscert properties which are both requried. If the tls object is not specified, tls is disabled

Half-Shot · 2021-08-13T17:34:33Z

config/email-config-schema.yaml

+            type: "string"
+          dkimSelector:
+            type: "string"
+          dkimEnabled:


Same comment about dependencies. An object might be the way to go.

src/bridge.js

src/email/outbound.js

Half-Shot · 2021-08-13T17:49:04Z

src/email/outbound.js

+                    else {
+                        // check for ESMTP/ignore-case
+                        if (/\besmtp\b/i.test(msg)) {
+                            // TODO: determine AUTH type; auth login, auth crm-md5, auth plain


Suggestion: For each of the TODOs you want to do later, make an issue in this repo and link back to it as a comment. It means any community member can:

See what doesn't work yet

See what they can work on to make it work better

Find the code line the issue relates to

src/email/outbound.js

Co-authored-by: Will Hunt <willh@matrix.org>

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

'@_email_'.length improves readability by a lot. Co-authored-by: Will Hunt <willh@matrix.org>

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Co-authored-by: Will Hunt <willh@matrix.org>

meaningful function name Signed-off-by: Abhinav Krishna C K <me@abhy.me>

…ail into smtpoutbound

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Add SMTP outbound support to the bridge.

cca5374

* Currently uses hardcoded 'To' address on the outbound. * DKIM signing support is also added. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Half-Shot reviewed Jul 23, 2021

View reviewed changes

abbyck added 9 commits July 23, 2021 19:07

Merge branch 'main' into smtpoutbound

9fa8bd0

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Fix spacing and newlines.

ec2a6ea

Includes other cosmetic changes. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Reorganize connectMx logic.

ab0714c

Remove devHost/devPort and decide on next hop based on smtpHost. Fix resloved MX records sorting. Unify tryConnect for resolvedMX and relaying hop. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Send matrix messages to the corresponding bridge user by email.

69ea8ca

Resolve email from userId. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Send outbound mails to the correct email user.

177505e

Remove hardcoded 'to' address by getting the joined memebers and filtering out the email users. Also, 'from' address is calculated using the roomAlias of the corresponding bridged room. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Use then/catch to handle resolveMX

635a30e

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Fix AS termination on invalid inbound rcptTo address.

77bb14c

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Use async/await everywhere for outbound.

0905e64

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Set display name based on 'from' header or email ID.

04af65a

Currently the display name is not set. This commit enables to set Matrix display name based on the 'from' header. If that's empty, email ID will be set as the display name. Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Half-Shot self-requested a review August 6, 2021 16:25

abbyck added 3 commits August 8, 2021 00:05

Add STARTSSL support.

f0ef18c

Now the bridge will check for STARTSSL availability with connecting SMTP server and will try to upgrade the connection to use TLS (Opportunistic TLS). Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Add necessary config options for DKIM and STARTSSL (outbound).

9819cd5

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Fix variable case mismatch and a type coercion during comparision

bd7f9d8

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Half-Shot approved these changes Aug 13, 2021

View reviewed changes

abbyck and others added 12 commits August 14, 2021 19:04

Follow the JS way :)

1731228

Co-authored-by: Will Hunt <willh@matrix.org>

Remove noisy docstring

a0b5a6a

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Simpler undefined check

2eab70a

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Another magic number removal :)

89a4b37

'@_email_'.length improves readability by a lot. Co-authored-by: Will Hunt <willh@matrix.org>

Add docstrings for sendToSMTP, getAddresses and others.

2b10076

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Add type.

b9453a3

Co-authored-by: Will Hunt <willh@matrix.org>

w() -> writeToSocket()

d675b19

meaningful function name Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Merge branch 'smtpoutbound' of github.com:abbyck/matrix-appservice-em…

da6477b

…ail into smtpoutbound

Move function definitions above the call.

2ee3c1d

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Use const object instead of hardcoded SMTP codes.

bf9734d

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Get mxid localpart from MatrixUser

f6235e3

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

Organize config files better.

3d34cbe

Signed-off-by: Abhinav Krishna C K <me@abhy.me>

abbyck merged commit f3c65c8 into main Aug 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SMTP outbound support to the bridge. #4

Add SMTP outbound support to the bridge. #4

abbyck commented Jul 16, 2021

Half-Shot left a comment

Half-Shot left a comment

Half-Shot Aug 13, 2021

Half-Shot Aug 13, 2021

Half-Shot Aug 13, 2021

Half-Shot Aug 13, 2021

Add SMTP outbound support to the bridge. #4

Add SMTP outbound support to the bridge. #4

Conversation

abbyck commented Jul 16, 2021

Half-Shot left a comment

Choose a reason for hiding this comment

Half-Shot left a comment

Choose a reason for hiding this comment

Half-Shot Aug 13, 2021

Choose a reason for hiding this comment

Half-Shot Aug 13, 2021

Choose a reason for hiding this comment

Half-Shot Aug 13, 2021

Choose a reason for hiding this comment

Half-Shot Aug 13, 2021

Choose a reason for hiding this comment