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

Better defaults for smtp settings, fixes #24 #27

Merged
merged 3 commits into from Oct 17, 2018

Conversation

aspettl
Copy link
Contributor

@aspettl aspettl commented Aug 6, 2018

This PR contains the most simple implementation for #24: hardcode the defaults, no environment variable.

I tried it locally, it works for me. I modified the readme because the SMTP server has to be specified separately (otherwise it is localhost). And I added tls:// to the example because (hopefully) SMTP servers will require TLS before they accept SMTP AUTH. Anyway, activating encryption should always be a good idea.

@thomascube
Copy link
Member

Thanks for getting this done! However, I don't think we should add tls:// in the docker run command example. People tend to copy things without thinking and then complain if it doesn't work. You're certainly right that encryption should be used when connecting to both IMAP and SMTP but in a Docker setup those nodes might be in the same local Docker network and using TLS here might be an unwanted overhead and is maybe not even enabled. It's good to mention it in the variable descriptions but please remove it from the example command.

@aspettl
Copy link
Contributor Author

aspettl commented Aug 8, 2018

I removed the tls:// - but I hope a sysadmin would not be confused by this ;-)

I fear that the risk for someone just trying Roundcube is high that he/she will send a plaintext password over an unsecure connection - without noticing it. (I don't know whether Roundcube refuses to use plaintext passwords without SSL/TLS.)

@aspettl
Copy link
Contributor Author

aspettl commented Sep 29, 2018

Any more issues preventing the merge?

@compuguy
Copy link

compuguy commented Oct 1, 2018

I removed the tls:// - but I hope a sysadmin would not be confused by this ;-)

I fear that the risk for someone just trying Roundcube is high that he/she will send a plaintext password over an unsecure connection - without noticing it. (I don't know whether Roundcube refuses to use plaintext passwords without SSL/TLS.)

As a sysadmin, I agree with this. That and smtp_user and smtp_pass should be configured to pass-through the login credentials.

@compuguy
Copy link

@thomascube Any movement/progress on this pull request?

@thomascube
Copy link
Member

Sorry for the delay! Looks good now.

@thomascube thomascube merged commit 80114ef into roundcube:master Oct 17, 2018
@aspettl aspettl deleted the smtp_defaults branch October 17, 2018 20:44
J0WI added a commit to J0WI/roundcubemail-docker that referenced this pull request Oct 18, 2018
J0WI added a commit to J0WI/roundcubemail-docker that referenced this pull request Oct 28, 2018
J0WI added a commit to J0WI/roundcubemail-docker that referenced this pull request Jan 10, 2019
J0WI added a commit to J0WI/roundcubemail-docker that referenced this pull request Jan 10, 2019
J0WI added a commit to J0WI/roundcubemail-docker that referenced this pull request Jan 10, 2019
@Fridus
Copy link

Fridus commented May 21, 2019

Hi @aspettl ,
Could you add the parameters ROUNDCUBEMAIL_SMTP_USER and ROUNDCUBEMAIL_SMTP_PASS ?

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.

None yet

4 participants