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

Authentication is only applied when both the username and password have values. #2937

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

LeoQuote
Copy link
Contributor

@LeoQuote LeoQuote commented Mar 22, 2024

Description

no auth in smtp is implemented in #2765 , but there's a bug, SMTP.sendmail requires the first sender address as the first argument.

When username is set to none, the auth is skipped but sendmail would fail due to the type error

When username is set to empty string, the auth is skipped , mail was sent, but this mail envelope does not include sender info, which could lead to rejection by server.

So the correct configuration for no-auth smtp would be

[email protected]
SMTP_PASSWORD="" # or just dont set

In the current logic, if a username is provided, authentication will be performed, which is not as expected. Therefore, the logic of authentication at this point needs to be modified from authenticating when a username is present to authenticating only when both username and password are provided.

Type of Change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • import SMPTClient and try to send a mail
>>> from smtp import SMTPClient
>>> c = SMTPClient("mail.example.com", 25, "[email protected]", "", "[email protected]")
>>> c.send({"subject": "test", "to": "[email protected]", "html": "<p>hello</p>"})

And I can receive the mail.

Suggested Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
  • optional I have made corresponding changes to the documentation
  • optional I have added tests that prove my fix is effective or that my feature works
  • optional New and existing unit tests pass locally with my changes

`smtp.sendmail` need from_addr as the first argument, if username is not set or set as none, the mail would not be sent or received.
@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Mar 22, 2024
@crazywoola
Copy link
Member

Hello can you add my wechat crazyphage I will invite you to the contributors' group

@dosubot dosubot bot added the lgtm label Mar 22, 2024
@crazywoola crazywoola merged commit cc75412 into langgenius:main Mar 22, 2024
7 checks passed
@LeoQuote LeoQuote deleted the patch-1 branch March 22, 2024 10:08
HuberyHuV1 pushed a commit to HuberyHuV1/dify that referenced this pull request Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants