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

Jump to bottom

build(maildev): implement maildev for development #1339

Merged
merged 7 commits into from
Mar 23, 2021
Merged

build(maildev): implement maildev for development #1339

merged 7 commits into from
Mar 23, 2021

Conversation

orbitalsqwib
Copy link
Contributor

Problem

Currently local development requires hardcoding OTPs to "111111" and using the dummy MailerNoOp service.

Integrating maildev with for use in local development instead will close the security risks from accidentally generating dummy OTPs in production, and align development to be closer to the production setup.

Closes #951

Solution

  • Implemented maildev for local development
  • Removed MailerNoOp and Nodemailer Direct Transport
  • Refactored code paths to always use maildev for development
  • Updated end-to-end tests to use maildev for retrieving OTPs

Deploy Notes

  • Removed the nodemailer-direct-transport library
  • Installed the maildev library, and added the maildev/maildev container to docker-compose

@orbitalsqwib @LoneRifle
build(email): implement maildev for local development
d34d950 
Currently local development requires hardcoding OTPs to "111111" and using the dummy MailerNoOp
service.

-implemented maildev for local development
-added an environment variable to toggle
maildev, defaults to true

fix #951
@orbitalsqwib @LoneRifle
fix(docker-compose): change defaults for maildev enabled to false
54afa1d 
Currently, tests are not able to take into account maildev when testing functionality.

-disable maildev by default until tests can be updated
@orbitalsqwib
test(login-procedure): integrate maildev into login procedures
06982d0 
fix #951
@orbitalsqwib
refactor(maildev): refactor maildev for permanent use in dev
c561652 
- Removed MailerNoOp and Nodemailer Direct Transport
- Removed `USE_MAILDEV` env var
- Refactored code paths to always use maildev for development
- Improved LoginProcedure to use regex to find OTP

fix #951
@orbitalsqwib orbitalsqwib requested a review from LoneRifle March 22, 2021 09:17
LoneRifle
LoneRifle approved these changes Mar 23, 2021
View reviewed changes
Copy link
Contributor

@LoneRifle LoneRifle left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm otherwise. I think we should go ahead and undertake the stretch goal that would provide a custom maildev image that modifies the binary to log mail contents to maintain the quick input of OTPs that devs currently enjoy

src/server/inversify.config.ts Outdated Show resolved Hide resolved
src/types/server/util/nodemailer.d.ts Outdated Show resolved Hide resolved
src/server/config.ts Outdated Show resolved Hide resolved
src/server/services/email.ts Outdated Show resolved Hide resolved
@orbitalsqwib
refactor(transport-options): refactor nodemailer transport options
292e63c 
- Refactored Nodemailer transport option configurations for clarity
- Cleaned up email, inversify and config files

fix #951
LoneRifle
LoneRifle reviewed Mar 23, 2021
View reviewed changes
package.json Outdated Show resolved Hide resolved
@orbitalsqwib
build(maildev): remove maildev from dependencies
e1bee6c 
- Removed maildev from dependencies as maildev is currently run through a docker container

fix #951
@orbitalsqwib orbitalsqwib requested a review from LoneRifle March 23, 2021 03:23
LoneRifle
LoneRifle reviewed Mar 23, 2021
View reviewed changes
src/server/config.ts Outdated Show resolved Hide resolved
@orbitalsqwib orbitalsqwib merged commit 0487bbb into develop Mar 23, 2021
@orbitalsqwib orbitalsqwib deleted the build/maildev branch March 23, 2021 03:41
@snyk-bot snyk-bot mentioned this pull request Jul 28, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LoneRifle LoneRifle LoneRifle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use maildev for local development
2 participants
@orbitalsqwib @LoneRifle