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

SmtpMailer: add support for AUTH PLAIN #31

Merged
merged 1 commit into from Jul 31, 2016
Merged

SmtpMailer: add support for AUTH PLAIN #31

merged 1 commit into from Jul 31, 2016

Conversation

@grongor
Copy link
Contributor

grongor commented Jun 27, 2016

It's strange that there are some SMTP servers not supporting the LOGIN mechanism but I stumbled upon one ... so here is the fix :)

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented Jun 27, 2016

I wonder how did you manage accidentally set the executable flag on the file 😄

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented Jun 27, 2016

$this->write(base64_encode($this->password), 235, 'password');
$authMechanisms = [];
foreach (preg_split("~\r?\n~", $ehloResponse) as $line) {
if (strpos($line, '250-AUTH ') === 0) {

This comment has been minimized.

Copy link
@JanTvrdik

JanTvrdik Jun 27, 2016

Contributor

shouldn't this be 250 AUTH? EDIT: https://tools.ietf.org/html/rfc4954#section-4.1 uses both so I don't really know

@grongor

This comment has been minimized.

Copy link
Contributor Author

grongor commented Jun 27, 2016

... some Windows magic I guess, I don't usually push code on this computer so it got a bit clumsy :D I will try to fix it ...

How should the STARTTLS be related? If it is required and for some reason it fails to activate then the execution stops - the credentials are never sent.

The space after return code is used to indicate the last line of the response. The link you posted is a good example of that.

@grongor

This comment has been minimized.

Copy link
Contributor Author

grongor commented Jun 27, 2016

Thanks for pointing out the space vs dash issue - I updated the implementation to count with both of them (because the AUTH can also be the last line of the response).

And btw the +x flag was set by PhpStorm on Windows ... I don't know why but it happened again. Weird.

$authMechanisms = explode(' ', trim($matches[1]));
}

if (array_search('PLAIN', $authMechanisms, true) !== false) {

This comment has been minimized.

Copy link
@JanTvrdik

JanTvrdik Jun 27, 2016

Contributor

can be simplified with in_array


Also now it uses PLAIN auth whenever possible, shouldn't we prefer LOGIN auth when both plain and login auth are allowed?

This comment has been minimized.

Copy link
@grongor

grongor Jun 27, 2016

Author Contributor

Alright, I will change it to in_array.

I also though about it and I don't think that there is a reason to prefer LOGIN mechanism over PLAIN. They both offer same security. The PLAIN mechanism will be just a bit faster ... correct me if I'm wrong.

@dg

This comment has been minimized.

Copy link
Member

dg commented Jul 31, 2016

Thanks

@dg dg merged commit f2bf695 into nette:master Jul 31, 2016
1 of 2 checks passed
1 of 2 checks passed
coverage/coveralls Coverage decreased (-1.4%) to 72.603%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
dg added a commit that referenced this pull request Jul 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.