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

Autodetect SMTP auth methods #7356

Closed
xoxys opened this issue Sep 26, 2023 · 4 comments · Fixed by #7361
Closed

Autodetect SMTP auth methods #7356

xoxys opened this issue Sep 26, 2023 · 4 comments · Fixed by #7361
Assignees
Labels
Category:Enhancement Add new functionality
Milestone

Comments

@xoxys
Copy link
Contributor

xoxys commented Sep 26, 2023

Is your feature request related to a problem? Please describe.

It's required to explicitly set NOTIFICATIONS_SMTP_AUTHENTICATION on most production systems, as the default is set to none. This is not optimal, especially as the used mail library supports auto-detection of the supported auth methods of the server.

https://github.com/xhit/go-simple-mail/blob/v2.16.0/email.go#L152

You might want to add some more sanitizing in the same step to avoid issues. For example, what happens if username + password are provided, but auth is not supported by the server. Example https://github.com/go-gitea/gitea/blob/main/services/mailer/mailer.go#L259

Auth/security related options shouldn't fail silently.

Describe the solution you'd like

Switch to NOTIFICATIONS_SMTP_AUTHENTICATION=auto as default.

@rhafer rhafer added the Category:Enhancement Add new functionality label Sep 26, 2023
@rhafer rhafer self-assigned this Sep 26, 2023
@rhafer
Copy link
Contributor

rhafer commented Sep 26, 2023

Hm, NOTIFICATIONS_SMTP_ENCRYPTION also defaults to none. Which IMO is a really bad default as well.

@xoxys
Copy link
Contributor Author

xoxys commented Sep 26, 2023

But it's hard to guess and depends on the port. Also, common in other projects that it must be set.

@xoxys
Copy link
Contributor Author

xoxys commented Sep 26, 2023

If you want to touch this part, you might want to take #7345 into account as well.

@rhafer
Copy link
Contributor

rhafer commented Sep 26, 2023

But it's hard to guess and depends on the port.

True, IMO the defaults should be either:

  • port=465 and encryption=ssltls
  • port=587 and encryption=starttls

or

  • no defaults at all

I am leaning towards the last options. Simply because we're already using an empty default for the host.

Or current defaults (port=1025 and encryption=none) don't make any sense IMO.

rhafer added a commit to rhafer/ocis that referenced this issue Sep 26, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: owncloud#7356
rhafer added a commit to rhafer/ocis that referenced this issue Sep 27, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: owncloud#7356
rhafer added a commit to rhafer/ocis that referenced this issue Sep 27, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: owncloud#7356
rhafer added a commit to rhafer/ocis that referenced this issue Sep 27, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: owncloud#7356
rhafer added a commit to rhafer/ocis that referenced this issue Sep 28, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: owncloud#7356
rhafer added a commit to rhafer/ocis that referenced this issue Sep 28, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: owncloud#7356
rhafer added a commit to rhafer/ocis that referenced this issue Oct 5, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: owncloud#7356
rhafer added a commit to rhafer/ocis that referenced this issue Oct 5, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: owncloud#7356
rhafer added a commit to rhafer/ocis that referenced this issue Oct 5, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: owncloud#7356
rhafer added a commit that referenced this issue Oct 5, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: #7356
ownclouders pushed a commit that referenced this issue Oct 5, 2023
This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION.
Which will make the notifications service automatically pick an authentication
mechanism that the server supports. This is also the new default behavior.

This also removes most of the other default settings for the SMTP
configuration. The default values were of no real use for this service.

Closes: #7356
@micbar micbar added this to the Release 5.0.0 milestone Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Enhancement Add new functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants