Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add self signed certificate for smtp transport option #40

Merged
merged 3 commits into from
Apr 7, 2023
Merged

Add self signed certificate for smtp transport option #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4c4b016
Add self signed certificate option
rupal-bq Apr 6, 2023
b7afa3e
Fix rejectUnauthorized value
rupal-bq Apr 6, 2023
464ad76
Update user doc
rupal-bq Apr 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions USER_GUIDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Option | Default Value | Valid Options | Environment Variable | Description
-n, --filename | opensearch-report-timestamp | - | OPENSEARCH_FILENAME | file name of the report
-a, --auth | none | basic, saml, cognito | - | authentication type for the report
-t, --tenant | private | - | - | tenants in opensearch dashboards
--multitenancy | true | true, false | - | enable or disable multi-tenancy
-c, --credentials | - | - | OPENSEARCH_USERNAME and OPENSEARCH_PASSWORD | login credentials in the format of username:password for connecting to url
-s, --from | - | - | OPENSEARCH_FROM | email address of the sender
-r, --to | - | - | OPENSEARCH_TO | email address of the recipient
Expand All @@ -39,6 +40,7 @@ Option | Default Value | Valid Options | Environment Variable | Description
--smtpsecure | - | - | OPENSEARCH_SMTP_SECURE | if true the connection will use TLS when connecting to server.
--subject | This is an email containing your dashboard report | - | OPENSEARCH_SUBJECT | subject for the email
--note | Hi,\nHere is the latest report! | string or path to text file | OPENSEARCH_EMAIL_NOTE | The email body
--selfsignedcerts | false | true, false | - | enable or disable self-signed certicates for smtp transport
| - | - | - | CHROMIUM_PATH | path to chromium directory

You can also find this information using help command.
Expand Down
17 changes: 13 additions & 4 deletions src/arguments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const { program, Option } = require('commander');
const { exit } = require('process');
const fs = require('fs');
const ora = require('ora');
const { AUTH, CLI_COMMAND_NAME, DEFAULT_AUTH, DEFAULT_FILENAME, DEFAULT_FORMAT, DEFAULT_MIN_HEIGHT, DEFAULT_TENANT, DEFAULT_WIDTH, ENV_VAR, FORMAT, TRANSPORT_TYPE, DEFAULT_EMAIL_SUBJECT, DEFAULT_EMAIL_NOTE, DEFAULT_MULTI_TENANCY } = require('./constants.js');
const { AUTH, CLI_COMMAND_NAME, DEFAULT_AUTH, DEFAULT_FILENAME, DEFAULT_FORMAT, DEFAULT_MIN_HEIGHT, DEFAULT_TENANT, DEFAULT_WIDTH, ENV_VAR, FORMAT, TRANSPORT_TYPE, DEFAULT_EMAIL_SUBJECT, DEFAULT_EMAIL_NOTE, DEFAULT_MULTI_TENANCY, DEFAULT_SELF_SIGNED_CERTIFICATES } = require('./constants.js');
const dotenv = require("dotenv");
dotenv.config();
const spinner = ora('');
Expand All @@ -30,8 +30,8 @@ async function getCommandArguments() {
.addOption(new Option('-t, --tenant <tenant>', 'tenants in opensearch dashboards')
.default(DEFAULT_TENANT))
.addOption(new Option('--multitenancy <flag>', 'enable or disable multi-tenancy')
.default(DEFAULT_MULTI_TENANCY)
.choices(['true', 'false']))
.default(DEFAULT_MULTI_TENANCY)
.choices(['true', 'false']))
.addOption(new Option('-f, --format <type>', 'file format for the report')
.default(DEFAULT_FORMAT)
.choices([FORMAT.PDF, FORMAT.PNG, FORMAT.CSV]))
Expand Down Expand Up @@ -65,6 +65,9 @@ async function getCommandArguments() {
.addOption(new Option('--note <note>', 'email body (string or path to text file)')
.default(DEFAULT_EMAIL_NOTE)
.env(ENV_VAR.EMAIL_NOTE))
.addOption(new Option('--selfsignedcerts <flag>', 'enable or disable self-signed certicates for smtp transport')
.default(DEFAULT_SELF_SIGNED_CERTIFICATES)
.choices(['true', 'false']));

program.addHelpText('after', `
Note: The tenant in the url has the higher priority than tenant value provided as command option.`);
Expand Down Expand Up @@ -94,6 +97,8 @@ async function getEventArguments(event) {
event['note'] = DEFAULT_EMAIL_NOTE;
if (event.multitenancy === undefined)
event['multitenancy'] = DEFAULT_MULTI_TENANCY;
if (event.selfsignedcerts === undefined)
event['selfsignedcerts'] = DEFAULT_SELF_SIGNED_CERTIFICATES;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try this merge strategy :

const defaults = {
  auth: DEFAULT_AUTH,
  tenant: DEFAULT_TENANT,
  ... (etc)
}

Object.assign(defaults, ...event)

I think would both clarify the defaults, and make the fallback assignments a one-liner.


return getOptions(event);
}
Expand Down Expand Up @@ -121,7 +126,8 @@ function getOptions(options) {
subject: null,
time: null,
note: null,
emailbody: null
emailbody: null,
selfsignedcerts: null
}

// Set url.
Expand Down Expand Up @@ -213,6 +219,9 @@ function getOptions(options) {
}
commandOptions.note = getHtml(commandOptions.note);

// Set self signed certificate flag.
commandOptions.selfsignedcerts = options.selfsignedcerts;

spinner.succeed('Fetched argument values');
return commandOptions;
}
Expand Down
3 changes: 2 additions & 1 deletion src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const DEFAULT_FILENAME = 'opensearch-report';
const DEFAULT_EMAIL_SUBJECT = 'This is an email containing your opensearch dashboard report';
const DEFAULT_EMAIL_NOTE = 'Hi,\nHere is the latest report!';
const DEFAULT_MULTI_TENANCY = true;
const DEFAULT_SELF_SIGNED_CERTIFICATES = false;

const REPORT_TYPE = {
DASHBOARD: 'Dashboard',
Expand Down Expand Up @@ -73,5 +74,5 @@ const TRANSPORT_TYPE = {

module.exports = {
CLI_COMMAND_NAME, DEFAULT_AUTH, DEFAULT_TENANT, DEFAULT_FORMAT, DEFAULT_WIDTH, DEFAULT_MIN_HEIGHT, DEFAULT_FILENAME, DEFAULT_EMAIL_SUBJECT,
DEFAULT_EMAIL_NOTE, REPORT_TYPE, SELECTOR, FORMAT, AUTH, URL_SOURCE, ENV_VAR, TRANSPORT_TYPE, DEFAULT_MULTI_TENANCY
DEFAULT_EMAIL_NOTE, REPORT_TYPE, SELECTOR, FORMAT, AUTH, URL_SOURCE, ENV_VAR, TRANSPORT_TYPE, DEFAULT_MULTI_TENANCY, DEFAULT_SELF_SIGNED_CERTIFICATES
};
34 changes: 19 additions & 15 deletions src/email-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ try {
// Do not set AWS_SDK_LOAD_CONFIG if aws config file is missing.
}

module.exports = async function sendEmail(filename, url, sender, recipient, transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword, subject, note, emailbody) {
module.exports = async function sendEmail(filename, url, sender, recipient, transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword, subject, note, emailbody, selfsignedcerts) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might start to make sense to turn this into an object-reference, with an interface. Naming might be hard, though I could make more sense and less brain-power out of arguments named {emailContent, smtpConfig} with associated interfaces.

This would also mean that further config changes (new options) do not cause change on this method (or at least this method signature

if (transport !== undefined && (transport === 'smtp' || ses !== undefined) && sender !== undefined && recipient !== undefined) {
spinner.start('Sending email...');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like making this module to two different things : 1. format/send email. 2. control UI/UX.

I think the only question in splitting this up would be if there is expected some "on-the-fly" UX while the email-sending process is ongoing... In our modern computing environment, there shouldn't be a timing issue... emails never take > 10sec so send.. or else they fail immediately.

} else {
Expand All @@ -41,7 +41,7 @@ module.exports = async function sendEmail(filename, url, sender, recipient, tran

let mailOptions = getmailOptions(url, sender, recipient, filename, subject, note, emailbody);

let transporter = getTransporter(transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword);
let transporter = getTransporter(transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword, selfsignedcerts);

transporter.use("compile", hbs({
viewEngine: {
Expand All @@ -54,21 +54,22 @@ module.exports = async function sendEmail(filename, url, sender, recipient, tran

// send email
return new Promise((success, fail) => {
transporter.sendMail(mailOptions, function (err, info) {
if (err) {
spinner.fail('Error sending email' + err);
fail(err);
exit(1);
} else {
spinner.succeed('Email sent successfully');
deleteTemporaryImage(emailbody);
success(info);
}
});
transporter.sendMail(mailOptions, function (err, info) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a lot more clear using a try..catch.. .and if necessary a throw on self-detected error (err value is not null?)

if (err) {
spinner.fail('Error sending email' + err);
fail(err);
exit(1);
} else {
spinner.succeed('Email sent successfully');
deleteTemporaryImage(emailbody);
success(info);
}
});
});
}

const getTransporter = (transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword, transporter) => {
const getTransporter = (transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword, selfsignedcerts, transporter) => {
var rejectSelfSignedCerts = selfsignedcerts === 'true' ? false : true;
if (transport === 'ses') {
transporter = nodemailer.createTransport({
SES: ses
Expand All @@ -81,7 +82,10 @@ const getTransporter = (transport, smtphost, smtpport, smtpsecure, smtpusername,
auth: {
user: smtpusername,
pass: smtppassword,
}
},
tls: {
rejectUnauthorized: rejectSelfSignedCerts,
},
});
}
return transporter;
Expand Down
3 changes: 2 additions & 1 deletion src/run.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ module.exports = async function run(args) {
options.smtppassword,
options.subject,
options.note,
options.emailbody
options.emailbody,
options.selfsignedcerts
);
}
1 change: 1 addition & 0 deletions test/help.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Options:
--smtppassword <password> smtp password (env: OPENSEARCH_SMTP_PASSWORD)
--subject <subject> email subject (default: "This is an email containing your opensearch dashboard report", env: OPENSEARCH_EMAIL_SUBJECT)
--note <note> email body (string or path to text file) (default: "Hi,\\nHere is the latest report!", env: OPENSEARCH_EMAIL_NOTE)
--selfsignedcerts <flag> enable or disable self-signed certicates for smtp transport (choices: "true", "false", default: false)
-h, --help display help for command

Note: The tenant in the url has the higher priority than tenant value provided as command option.
Expand Down