From cc13990800e81233ca47626f83b7547a9b1bf7ae Mon Sep 17 00:00:00 2001 From: Miko Nieminen Date: Wed, 2 Dec 2020 16:20:45 +0100 Subject: [PATCH 1/4] Upgrade colombe to 0.4.0 --- dune-project | 5 ++--- letters.opam | 5 ++--- letters.opam.locked | 43 ++++++++++++++++++++-------------------- lib/dune | 16 +++++++++++++-- lib/letters.ml | 5 +++-- lib/sendmail_handler.ml | 4 ++-- lib/sendmail_handler.mli | 2 +- 7 files changed, 45 insertions(+), 35 deletions(-) diff --git a/dune-project b/dune-project index 3aaaf37..7175bec 100644 --- a/dune-project +++ b/dune-project @@ -19,10 +19,9 @@ (depends (ocaml (>= 4.08.1)) (dune (>= 2.3)) - (emile (>= 1.0)) (mrmime (>= 0.3.1)) - (colombe (>= 0.3.0)) - (sendmail-lwt (>= 0.3.0)) + (colombe (>= 0.4.0)) + (sendmail (>= 0.4.0)) (fmt (>= 0.8.8)) (x509 (>= 0.9.0)) (ptime (>= 0.8.5)) diff --git a/letters.opam b/letters.opam index 8f6d2b6..eed9b13 100644 --- a/letters.opam +++ b/letters.opam @@ -12,10 +12,9 @@ bug-reports: "https://github.com/oxidizing/letters/issues" depends: [ "ocaml" {>= "4.08.1"} "dune" {>= "2.3"} - "emile" {>= "1.0"} "mrmime" {>= "0.3.1"} - "colombe" {>= "0.3.0"} - "sendmail-lwt" {>= "0.3.0"} + "colombe" {>= "0.4.0"} + "sendmail" {>= "0.4.0"} "fmt" {>= "0.8.8"} "x509" {>= "0.9.0"} "ptime" {>= "0.8.5"} diff --git a/letters.opam.locked b/letters.opam.locked index 9644284..298ca88 100644 --- a/letters.opam.locked +++ b/letters.opam.locked @@ -10,7 +10,7 @@ doc: "https://oxidizing.github.io/letters/" bug-reports: "https://github.com/oxidizing/letters/issues" depends: [ "angstrom" {= "0.15.0"} - "asn1-combinators" {= "0.2.3"} + "asn1-combinators" {= "0.2.4"} "astring" {= "0.8.5"} "base" {= "v0.14.0"} "base-bytes" {= "base"} @@ -19,10 +19,10 @@ depends: [ "base64" {= "3.4.0"} "bigarray-compat" {= "1.0.0"} "bigarray-overlap" {= "0.2.0"} - "bigstringaf" {= "0.6.1"} + "bigstringaf" {= "0.7.0"} "cmdliner" {= "1.0.4"} "coin" {= "0.1.3"} - "colombe" {= "0.3.0"} + "colombe" {= "0.4.0"} "conf-gmp" {= "2"} "conf-gmp-powm-sec" {= "2"} "conf-m4" {= "1"} @@ -36,13 +36,13 @@ depends: [ "dune" {= "2.7.1"} "dune-configurator" {= "2.7.1"} "duration" {= "0.1.3"} - "emile" {= "1.0"} + "emile" {= "1.1"} "eqaf" {= "0.7"} - "fiat-p256" {= "0.2.1"} + "fiat-p256" {= "0.2.3"} "fmt" {= "0.8.9"} "fpath" {= "0.7.3"} "gmap" {= "0.3.0"} - "hacl_x25519" {= "0.2.0"} + "hacl_x25519" {= "0.2.2"} "hex" {= "1.4.0"} "hkdf" {= "1.0.4"} "ipaddr" {= "5.0.1"} @@ -50,44 +50,43 @@ depends: [ "logs" {= "0.7.0"} "lwt" {= "5.3.0"} "macaddr" {= "5.0.1"} - "menhir" {= "20200624"} - "menhirLib" {= "20200624"} - "menhirSdk" {= "20200624"} - "mirage-crypto" {= "0.8.5"} - "mirage-crypto-pk" {= "0.8.5"} - "mirage-crypto-rng" {= "0.8.5"} + "menhir" {= "20201201"} + "menhirLib" {= "20201201"} + "menhirSdk" {= "20201201"} + "mirage-crypto" {= "0.8.7"} + "mirage-crypto-pk" {= "0.8.7"} + "mirage-crypto-rng" {= "0.8.7"} "mirage-no-solo5" {= "1"} "mirage-no-xen" {= "1"} "mmap" {= "1.1.0"} - "mrmime" {= "0.3.1"} + "mrmime" {= "0.3.2"} "mtime" {= "1.2.0"} - "num" {= "1.3"} + "num" {= "1.4"} "ocaml" {= "4.08.1"} "ocaml-compiler-libs" {= "v0.12.3"} "ocaml-config" {= "1"} - "ocaml-migrate-parsetree" {= "2.0.0"} + "ocaml-migrate-parsetree" {= "2.1.0"} "ocaml-syntax-shims" {= "1.0.0"} "ocamlbuild" {= "0.14.0"} "ocamlfind" {= "1.8.1"} "ocplib-endian" {= "1.1"} "parsexp" {= "v0.14.0"} - "pecu" {= "0.4"} + "pecu" {= "0.5"} "ppx_cstruct" {= "6.0.0"} "ppx_derivers" {= "1.2.1"} - "ppx_sexp_conv" {= "v0.14.1"} - "ppxlib" {= "0.17.0"} + "ppx_sexp_conv" {= "v0.14.2"} + "ppxlib" {= "0.20.0"} "ptime" {= "0.8.5"} "re" {= "1.9.0"} "result" {= "1.5"} "rosetta" {= "0.3.0"} "rresult" {= "0.6.0"} - "sendmail" {= "0.3.0"} - "sendmail-lwt" {= "0.3.0"} + "sendmail" {= "0.4.0"} "seq" {= "base"} "sexplib" {= "v0.14.0"} "sexplib0" {= "v0.14.0"} "stdlib-shims" {= "0.1.0"} - "tls" {= "0.12.5"} + "tls" {= "0.12.6"} "topkg" {= "1.0.3"} "uchar" {= "0.0.2"} "unstrctrd" {= "0.2"} @@ -95,7 +94,7 @@ depends: [ "uuuu" {= "0.2.0"} "x509" {= "0.11.2"} "yuscii" {= "0.3.0"} - "zarith" {= "1.10"} + "zarith" {= "1.11"} ] build: [ ["dune" "subst"] {pinned} diff --git a/lib/dune b/lib/dune index 7254dde..31b6b93 100644 --- a/lib/dune +++ b/lib/dune @@ -1,5 +1,17 @@ (library (name letters) (public_name letters) - (libraries mrmime ptime.clock.os lwt fpath colombe colombe.emile - sendmail-lwt sendmail.tls)) + (libraries + ptime.clock.os + lwt + lwt.unix + tls + tls.lwt + fpath + colombe + colombe.emile + mrmime + sendmail + sendmail.starttls + ) +) diff --git a/lib/letters.ml b/lib/letters.ml index 93d83f4..c3bdb85 100644 --- a/lib/letters.ml +++ b/lib/letters.ml @@ -192,7 +192,7 @@ let send ~config:c ~sender ~recipients ~message = let from_mailbox = match Emile.of_string sender with | Ok v -> v - | Error (`Invalid (_,_)) -> failwith "Invalid sender address" + | Error (`Invalid (_, _)) -> failwith "Invalid sender address" in let from_addr = match Colombe_emile.to_reverse_path from_mailbox with @@ -248,7 +248,8 @@ let send ~config:c ~sender ~recipients ~message = match res with | Ok () -> Lwt.return () | Error err -> - Lwt.fail_with (Fmt.str "Sending email failed, %a" Sendmail_with_tls.pp_error err) + Lwt.fail_with + (Fmt.str "Sending email failed, %a" Sendmail_with_starttls.pp_error err) else let* res = Sendmail_handler.run diff --git a/lib/sendmail_handler.ml b/lib/sendmail_handler.ml index 8ab3079..9322886 100644 --- a/lib/sendmail_handler.ml +++ b/lib/sendmail_handler.ml @@ -45,7 +45,7 @@ let run_with_starttls | None -> 465 in let tls = Tls.Config.client ~authenticator:tls_authenticator () in - let ctx = Sendmail_with_tls.Context_with_tls.make () in + let ctx = Sendmail_with_starttls.Context_with_tls.make () in let open Lwt.Infix in Lwt_unix.gethostbyname (Domain_name.to_string hostname) >>= fun res -> @@ -71,7 +71,7 @@ let run_with_starttls | None -> Lwt_scheduler.inj (Lwt.return None) in let fiber = - Sendmail_with_tls.sendmail + Sendmail_with_starttls.sendmail lwt rdwr { ic; oc } diff --git a/lib/sendmail_handler.mli b/lib/sendmail_handler.mli index aa3e0e0..99acbd0 100644 --- a/lib/sendmail_handler.mli +++ b/lib/sendmail_handler.mli @@ -7,7 +7,7 @@ val run_with_starttls -> from:Colombe.Reverse_path.t -> recipients:Colombe.Forward_path.t list -> mail:Mrmime.Mt.buffer Mrmime.Mt.stream - -> (unit, Sendmail_with_tls.error) Lwt_result.t + -> (unit, Sendmail_with_starttls.error) Lwt_result.t val run : hostname:'a Domain_name.t From adf39857aa260b178dffa67fd21543df9394f3b2 Mon Sep 17 00:00:00 2001 From: Miko Nieminen Date: Wed, 2 Dec 2020 16:21:38 +0100 Subject: [PATCH 2/4] Improve service tests - remove pointless repetation in tests - add test case for SMTP transparency (lines starting with ".") - add tests using mailtrap.io - update documentation --- .gitignore | 2 +- README.md | 47 +++++++----- service-test/test.ml | 173 +++++++++++++++++++++++++++---------------- 3 files changed, 140 insertions(+), 82 deletions(-) diff --git a/.gitignore b/.gitignore index 1f37726..9b6fdf0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ /_build/ /_opam/ .merlin -ethereal_account.json +*_account.json diff --git a/README.md b/README.md index 9c35ba5..6d4cb85 100644 --- a/README.md +++ b/README.md @@ -225,39 +225,50 @@ These tests are still somewhat far from good and you need to validate all result #### Service tests -These tests are somewhat slow and fragile and because of that these are expected to be run manually. +Because these tests are somewhat slow and fragile, you need to run them manually. Execution of these tests depends on test accounts on *ethereal.email* and *mailtrap.io*. Before execution, you need to create configuration files with authentication credentials for each service. You can generate these configuration files by using the shell command snippets given below, but for those you need to have `jq` application installed. If you don't have it or you don't want to install it, you can also create `ethereal_account.json` and `mailtrap_account.json` files manually. Both files have the following format: + +``` json-with-comments +{ + "host": "smtp.ethereal.email or smtp.mailtrap.io", + "port": 587, + "username": "username for SMTP authentication", + "password": "password for SMTP authentication", + "secure": false // we will always use encryption, but `false` causes use of STARTTLS +} +``` -First create *ethereal.email* account and store account details +To create temporary *ethereal.email* account and store the account details, you can execute the following one-liner: ``` shell -curl -d '{ "requestor": "letters", "version": "dev" }' "https://api.nodemailer.com/user" -X POST -H "Content-Type: application/json" > ethereal_account.json +curl -s -d '{ "requestor": "letters", "version": "dev" }' "https://api.nodemailer.com/user" -X POST -H "Content-Type: application/json" | jq '{ hostname: .smtp.host, port: .smtp.port, secure: false, username: .user, password: .pass, }'> ethereal_account.json ``` -Currently using `ethereal.email` service requires non-released version of `colombe` and -you need to check out the project, commit `edf757c58fce58c170c63e8a92d3bc81fe4d32ff` contains the needed fix. Then the version with the fix needs to be pinned in the build env: +For *mailtrap.io*, you need to create a personal account first and get the API key: +- [signup](https://mailtrap.io/register/signup?ref=header) +- [copy API token from Settings](https://mailtrap.io/settings) + +The configuration file you can create with following steps: +- create environment variable containing your API token: `export MAILTRAP_API_TOKEN=` +- run the following one-liner in terminal to create the configuration file: ``` shell -# Move to folder where colombe is checked out -pushd /path/to/colombe -# Switch to correct git commit in colombe repo -git switch --detach edf757c58fce58c170c63e8a92d3bc81fe4d32ff -# Switch to use same opam env that is used by letters -eval "$(opam env --switch $(dirs | cut -d ' ' -f 2) --set-switch)" -# Pin this specific version of colombe (and all related packages) -opam pin . -# Finally, return back to letters project -popd +curl -s -H "Authorization: Bearer ${MAILTRAP_API_TOKEN}" "https://mailtrap.io/api/v1/inboxes" | jq '.[0] | { hostname: .domain, port: .smtp_ports[2], secure: false, username: .username, password: .password }' > mailtrap_account.json ``` -Then execute these tests (actually this runs all tests): +Now you are ready to execute these tests. You can run them with the following command: ``` shell dune build @runtest-all ``` -And finally review that the email is correctly generated in the service: +Finally review that all emails are correctly received in *ethreal.email*: - login to https://ethereal.email/login using credentials from the `ethereal_account.json` -- check the content of messages: https://ethereal.email/messages +- check the content of new messages: https://ethereal.email/messages + +Also check that you can find all emails in the inbox in *mailtrap.io*: +- login to [mailtrap.io](https://mailtrap.io/signin) using your personal credentials +- select the first inbox (unless you use another one), from [inboxes](https://mailtrap.io/inboxes) +- check the content of new messages ## Credits diff --git a/service-test/test.ml b/service-test/test.ml index bdb2343..a7403a0 100644 --- a/service-test/test.ml +++ b/service-test/test.ml @@ -8,23 +8,34 @@ let get_ethereal_account_details () = * below is relative to the location of the executable under _build *) let json = Yojson.Basic.from_file "../../../ethereal_account.json" in - let username = json |> member "user" |> to_string in - let password = json |> member "pass" |> to_string in - let smtp_node = json |> member "smtp" in - let hostname = smtp_node |> member "host" |> to_string in - let port = smtp_node |> member "port" |> to_int in - let with_starttls = smtp_node |> member "secure" |> to_bool |> not in + let username = json |> member "username" |> to_string in + let password = json |> member "password" |> to_string in + let hostname = json |> member "hostname" |> to_string in + let port = json |> member "port" |> to_int in + let with_starttls = json |> member "secure" |> to_bool |> not in + Config.make ~username ~password ~hostname ~with_starttls + |> Config.set_port (Some port) + |> Lwt.return +;; + +let get_mailtrap_account_details () = + let open Yojson.Basic.Util in + (* see the README.md how to generate the account file and the path + * below is relative to the location of the executable under _build + *) + let json = Yojson.Basic.from_file "../../../mailtrap_account.json" in + let username = json |> member "username" |> to_string in + let password = json |> member "password" |> to_string in + let hostname = json |> member "hostname" |> to_string in + let port = json |> member "port" |> to_int in + let with_starttls = json |> member "secure" |> to_bool |> not in Config.make ~username ~password ~hostname ~with_starttls |> Config.set_port (Some port) |> Lwt.return ;; let test_send_plain_text_email config _ () = - let sender = - Yojson.Basic.from_file "../../../ethereal_account.json" - |> Yojson.Basic.Util.member "user" - |> Yojson.Basic.Util.to_string - in + let sender = "john@example.com" in let recipients = [ To "harry@example.com" ; To "larry@example.com" @@ -52,11 +63,7 @@ The team ;; let test_send_html_email config _ () = - let sender = - Yojson.Basic.from_file "../../../ethereal_account.json" - |> Yojson.Basic.Util.member "user" - |> Yojson.Basic.Util.to_string - in + let sender = "john@example.com" in let recipients = [ To "harry@example.com" ; To "larry@example.com" @@ -68,13 +75,14 @@ let test_send_html_email config _ () = let body = Html {| -

Hi there,

+Hi there,

have you already seen the very cool new web framework written in ocaml: Sihl +

-Regards,
-The team + Regards,
+ The team

|} in @@ -85,11 +93,7 @@ The team ;; let test_send_mixed_body_email config _ () = - let sender = - Yojson.Basic.from_file "../../../ethereal_account.json" - |> Yojson.Basic.Util.member "user" - |> Yojson.Basic.Util.to_string - in + let sender = "john@example.com" in let recipients = [ To "harry@example.com" ; To "larry@example.com" @@ -111,13 +115,14 @@ The team in let html = {| -

Hi there,

+Hi there,

have you already seen the very cool new web framework written in ocaml: Sihl +

-Regards,
-The team + Regards,
+ The team

|} in @@ -129,76 +134,118 @@ The team | Error reason -> Lwt.fail_with reason ;; +let test_send_mixed_body_email_with_dot_starting_a_line config _ () = + let sender = "john@example.com" in + let recipients = + [ To "harry@example.com" + ; To "larry@example.com" + ; Cc "bill@example.com" + ; Bcc "dave@example.com" + ] + in + let subject = "Mixed body email with dot starting a line in raw source" in + let text = + {| +This email is carefully crafted so that the dot in the URL is expected to hit +the beginning of a new line in the encoded body, this tests that the case is +correctly handled and the dot does not vanish:: https://github.com/oxidizing/sihl +|} + in + let html = + {| +
+This email is carefully crafted so that the dot in the URL is expected to hit +the beginning of a new line in the encoded body, this tests that the case is +correctly handled and the dot does not vanish that would render this URL +broken::: Sihl +
+|} + in + let mail = + build_email ~from:sender ~recipients ~subject ~body:(Mixed (text, html, None)) + in + match mail with + | Ok message -> send ~config ~sender ~recipients ~message + | Error reason -> Lwt.fail_with reason +;; + (* Run it *) let () = Lwt_main.run - (let* conf_with_ca_detect = get_ethereal_account_details () in - let conf_with_ca_cert_bundle = - Config.set_ca_cert "/etc/ssl/certs/ca-certificates.crt" conf_with_ca_detect + (let* ethereal_conf_with_ca_detect = get_ethereal_account_details () in + let* mailtrap_conf_with_ca_detect = get_mailtrap_account_details () in + let ethereal_conf_with_ca_cert_bundle = + Config.set_ca_cert + "/etc/ssl/certs/ca-certificates.crt" + ethereal_conf_with_ca_detect in - let conf_with_single_ca_cert = + let ethereal_conf_with_single_ca_cert = (* ethereal.mail's certificate is signed with Let's Encrypt Authority X3 * that signed by DST Root CA X3 *) - Config.set_ca_cert "/etc/ssl/certs/DST_Root_CA_X3.pem" conf_with_ca_detect + Config.set_ca_cert "/etc/ssl/certs/DST_Root_CA_X3.pem" ethereal_conf_with_ca_detect + in + let ethereal_conf_with_ca_path = + Config.set_ca_path "/etc/ssl/certs/" ethereal_conf_with_ca_detect in - let conf_with_ca_path = Config.set_ca_path "/etc/ssl/certs/" conf_with_ca_detect in Alcotest_lwt.run "STMP client" [ ( "use ethereal.email, auto-detect CA certs" , [ Alcotest_lwt.test_case - "Send plain text email" - `Slow - (test_send_plain_text_email conf_with_ca_detect) - ; Alcotest_lwt.test_case - "Send html email" + "Send plain text email, auto-detect CA certs" `Slow - (test_send_html_email conf_with_ca_detect) + (test_send_plain_text_email ethereal_conf_with_ca_detect) ; Alcotest_lwt.test_case - "Send send mixed body email" - `Slow - (test_send_mixed_body_email conf_with_ca_detect) - ] ) - ; ( "use ethereal.email, define certificate bundle location" - , [ Alcotest_lwt.test_case - "Send plain text email" + "Send plain text email, use CA cert bundle" `Slow - (test_send_plain_text_email conf_with_ca_cert_bundle) + (test_send_plain_text_email ethereal_conf_with_ca_cert_bundle) ; Alcotest_lwt.test_case - "Send html email" + "Send plain text email, use specific CA cert file" `Slow - (test_send_html_email conf_with_ca_cert_bundle) + (test_send_plain_text_email ethereal_conf_with_single_ca_cert) ; Alcotest_lwt.test_case - "Send send mixed body email" + "Send plain text email, load CA certificates from a folder" `Slow - (test_send_mixed_body_email conf_with_ca_cert_bundle) + (test_send_plain_text_email ethereal_conf_with_ca_path) ] ) - ; ( "use ethereal.email, define location single CA cert" + ; ( "use ethereal.email, test different email message types" , [ Alcotest_lwt.test_case "Send plain text email" `Slow - (test_send_plain_text_email conf_with_single_ca_cert) + (test_send_plain_text_email ethereal_conf_with_ca_detect) + ; Alcotest_lwt.test_case + "Send HTML only email" + `Slow + (test_send_html_email ethereal_conf_with_ca_detect) ; Alcotest_lwt.test_case - "Send html email" + "Send mixed content email with plain text and HTML" `Slow - (test_send_html_email conf_with_single_ca_cert) + (test_send_mixed_body_email ethereal_conf_with_ca_detect) ; Alcotest_lwt.test_case - "Send send mixed body email" + "Send email with content including line starting with dot (SMTP \ + Transparency)" `Slow - (test_send_mixed_body_email conf_with_single_ca_cert) + (test_send_mixed_body_email_with_dot_starting_a_line + ethereal_conf_with_ca_detect) ] ) - ; ( "use ethereal.email, define PEM folder path" + ; ( "use mailtrap, test different email message types" , [ Alcotest_lwt.test_case "Send plain text email" `Slow - (test_send_plain_text_email conf_with_ca_path) + (test_send_plain_text_email mailtrap_conf_with_ca_detect) + ; Alcotest_lwt.test_case + "Send HTML only email" + `Slow + (test_send_html_email mailtrap_conf_with_ca_detect) ; Alcotest_lwt.test_case - "Send html email" + "Send mixed content email with plain text and HTML" `Slow - (test_send_html_email conf_with_ca_path) + (test_send_mixed_body_email mailtrap_conf_with_ca_detect) ; Alcotest_lwt.test_case - "Send send mixed body email" + "Send email with content including line starting with dot (SMTP \ + Transparency)" `Slow - (test_send_mixed_body_email conf_with_ca_path) + (test_send_mixed_body_email_with_dot_starting_a_line + mailtrap_conf_with_ca_detect) ] ) ]) ;; From 8422abd61139b8d96776d04d1b0f6af32518fa62 Mon Sep 17 00:00:00 2001 From: Miko Nieminen Date: Thu, 3 Dec 2020 16:57:22 +0100 Subject: [PATCH 3/4] Install dune in GitHub workflow --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed0846a..d31559e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,6 +36,7 @@ jobs: - name: Install dependencies if: steps.cache-opam.outputs.cache-hit != 'true' run: | + opam install -y dune opam install -y . --deps-only --with-doc --with-test --locked --unlock-base - name: Recover from an Opam broken state if: steps.cache-opam.outputs.cache-hit == 'true' From 414090140bcb2acf1de39d70a0c65f02db13f313 Mon Sep 17 00:00:00 2001 From: Josef Erben Date: Fri, 4 Dec 2020 00:20:16 +0100 Subject: [PATCH 4/4] Disable macos test in GH actions, failing pipeline seems unrelated --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d31559e..8929b52 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,8 @@ jobs: fail-fast: false matrix: os: - - macos-latest + # Disable macos build for now since it keeps failing with dune not found + # - macos-latest - ubuntu-latest ocaml-version: - 4.11.1