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.

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

Allow mail to be sent without starttls() and login creds if desired #641

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

MaverikMoody
Copy link
Contributor

@MaverikMoody MaverikMoody commented Jan 22, 2025

Proposed changes

Addresses some of the limitations outlined in the README for the SMTP output.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update

Checklist

  • Lint and unit tests pass locally with my changes (if applicable)
  • I have run pre-commit (pre-commit in the repo)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Linked to the relevant github issue or github discussion

@MaverikMoody MaverikMoody force-pushed the smtp-no-login branch 2 times, most recently from f182dc9 to a9e1ae9 Compare January 22, 2025 00:24
@thinkst-daniel
Copy link

Hi @MaverikMoody, thanks, this is a valid change.

Although I think we could possibly get away with a slightly simpler implementation, where we simply check if smtp_username and smtp_password are set instead. i.e. something like:

if smtp_username is not None or smtp_password is not None:
    server.starttls()
    server.login(smtp_username, smtp_password)

What do you think?

@MaverikMoody
Copy link
Contributor Author

Thanks for taking a look at this, I agree that something simpler here is totally fine and I've updated the PR to reflect your suggestion. I know some projects prefer to have explicit switches required for anything that could be deemed an insecure transmission of data, so I thought I would err on the side of caution here. Let me know if you'd like any additional changes.

Copy link

@thinkst-daniel thinkst-daniel left a comment

Choose a reason for hiding this comment

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

Thanks for making the tweak @MaverikMoody

@MaverikMoody
Copy link
Contributor Author

Thanks again for the review, are we waiting on anything else for getting this merged?

@thinkst-daniel
Copy link

thinkst-daniel commented Feb 10, 2025

Thanks @MaverikMoody, I just wanted to double-check that it works as expected on my set up. Does it work as expected on yours?

The change is working as expected:

SMTP creds set:
➜  canarytokens-docker git:(master) ✗ docker exec -it c97965588723 bash
root@c97965588723:/srv# c97965588723
bash: c97965588723: command not found
root@c97965588723:/srv# poetry run python
Python 3.9.20 (main, Oct 19 2024, 03:58:16)
[GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from canarytokens.settings import SwitchboardSettings
>>> switchboard_settings = SwitchboardSettings()
>>> smtp_username = switchboard_settings.SMTP_USERNAME
>>> smtp_password = switchboard_settings.SMTP_PASSWORD
>>> print(smtp_username)
test
>>> print(smtp_password)
logmein
>>> if smtp_username is not None and smtp_password is not None:
...      print("do smtp auth, etc")
... else:
...      print("dont do smtp auth, just send it!")
...
do smtp auth, etc
>>>

SMTP creds not set:
root@aefaf15aca7d:/srv# poetry run python
Python 3.9.20 (main, Oct 19 2024, 03:58:16)
[GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from canarytokens.settings import SwitchboardSettings
>>> switchboard_settings = SwitchboardSettings()
>>> smtp_username = switchboard_settings.SMTP_USERNAME
>>> smtp_password = switchboard_settings.SMTP_PASSWORD
>>> print(smtp_username)
None
>>> print(smtp_password)
None
>>> if smtp_username is not None and smtp_password is not None:
...      print("do smtp auth, etc")
... else:
...      print("dont do smtp auth, just send it!")
...
dont do smtp auth, just send it!
>>>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants