Merge pull request #4011 from ndm2/stmp-auth-reponse-evaluation-fix

Make SMTP auth reply code checks work properly.

lib/Cake/Network/Email/SmtpTransport.php

@@ -191,18 +191,22 @@ class SmtpTransport extends AbstractTransport {
  */
 	protected function _auth() {
 		if (isset($this->_config['username']) && isset($this->_config['password'])) {
-			$authRequired = $this->_smtpSend('AUTH LOGIN', '334|503');
-			if ($authRequired == '334') {
-				if (!$this->_smtpSend(base64_encode($this->_config['username']), '334')) {
+			$replyCode = $this->_smtpSend('AUTH LOGIN', '334|500|502|504');
+			if ($replyCode == '334') {
+				try {
+					$this->_smtpSend(base64_encode($this->_config['username']), '334');
+				} catch (SocketException $e) {
 					throw new SocketException(__d('cake_dev', 'SMTP server did not accept the username.'));
 				}
-				if (!$this->_smtpSend(base64_encode($this->_config['password']), '235')) {
+				try {
+					$this->_smtpSend(base64_encode($this->_config['password']), '235');
+				} catch (SocketException $e) {
 					throw new SocketException(__d('cake_dev', 'SMTP server did not accept the password.'));
 				}
-			} elseif ($authRequired == '504') {
-				throw new SocketException(__d('cake_dev', 'SMTP authentication method not allowed, check if SMTP server requires TLS'));
-			} elseif ($authRequired != '503') {
-				throw new SocketException(__d('cake_dev', 'SMTP does not require authentication.'));
+			} elseif ($replyCode == '504') {
+				throw new SocketException(__d('cake_dev', 'SMTP authentication method not allowed, check if SMTP server requires TLS.'));
+			} else {
+				throw new SocketException(__d('cake_dev', 'AUTH command not recognized or not implemented, SMTP server may not require authentication.'));
 			}
 		}
 	}

lib/Cake/Test/Case/Network/Email/SmtpTransportTest.php

@@ -130,6 +130,7 @@ class SmtpTransportTest extends CakeTestCase {
  * testConnectEhloTlsOnNonTlsServer method
  *
  * @expectedException SocketException
+ * @expectedExceptionMessage SMTP server did not accept the connection or trying to connect to non TLS SMTP server using TLS.
  * @return void
  */
 	public function testConnectEhloTlsOnNonTlsServer() {
@@ -150,6 +151,7 @@ class SmtpTransportTest extends CakeTestCase {
  * testConnectEhloNoTlsOnRequiredTlsServer method
  *
  * @expectedException SocketException
+ * @expectedExceptionMessage SMTP authentication method not allowed, check if SMTP server requires TLS.
  * @return void
  */
 	public function testConnectEhloNoTlsOnRequiredTlsServer() {
@@ -189,6 +191,7 @@ class SmtpTransportTest extends CakeTestCase {
  * testConnectFail method
  *
  * @expectedException SocketException
+ * @expectedExceptionMessage SMTP server did not accept the connection.
  * @return void
  */
 	public function testConnectFail() {
@@ -224,6 +227,90 @@ class SmtpTransportTest extends CakeTestCase {
 	}
 
 /**
+ * testAuthNotRecognized method
+ *
+ * @expectedException SocketException
+ * @expectedExceptionMessage AUTH command not recognized or not implemented, SMTP server may not require authentication.
+ * @return void
+ */
+	public function testAuthNotRecognized() {
+		$this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
+		$this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false));
+		$this->socket->expects($this->at(2))->method('read')->will($this->returnValue("500 5.3.3 Unrecognized command\r\n"));
+		$this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story'));
+		$this->SmtpTransport->auth();
+	}
+
+/**
+ * testAuthNotImplemented method
+ *
+ * @expectedException SocketException
+ * @expectedExceptionMessage AUTH command not recognized or not implemented, SMTP server may not require authentication.
+ * @return void
+ */
+	public function testAuthNotImplemented() {
+		$this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
+		$this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false));
+		$this->socket->expects($this->at(2))->method('read')->will($this->returnValue("502 5.3.3 Command not implemented\r\n"));
+		$this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story'));
+		$this->SmtpTransport->auth();
+	}
+
+/**
+ * testAuthBadSequence method
+ *
+ * @expectedException SocketException
+ * @expectedExceptionMessage SMTP Error: 503 5.5.1 Already authenticated
+ * @return void
+ */
+	public function testAuthBadSequence() {
+		$this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
+		$this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false));
+		$this->socket->expects($this->at(2))->method('read')->will($this->returnValue("503 5.5.1 Already authenticated\r\n"));
+		$this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story'));
+		$this->SmtpTransport->auth();
+	}
+
+/**
+ * testAuthBadUsername method
+ *
+ * @expectedException SocketException
+ * @expectedExceptionMessage SMTP server did not accept the username.
+ * @return void
+ */
+	public function testAuthBadUsername() {
+		$this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
+		$this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false));
+		$this->socket->expects($this->at(2))->method('read')->will($this->returnValue("334 Login\r\n"));
+		$this->socket->expects($this->at(3))->method('write')->with("bWFyaw==\r\n");
+		$this->socket->expects($this->at(4))->method('read')->will($this->returnValue(false));
+		$this->socket->expects($this->at(5))->method('read')->will($this->returnValue("535 5.7.8 Authentication failed\r\n"));
+		$this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story'));
+		$this->SmtpTransport->auth();
+	}
+
+/**
+ * testAuthBadPassword method
+ *
+ * @expectedException SocketException
+ * @expectedExceptionMessage SMTP server did not accept the password.
+ * @return void
+ */
+	public function testAuthBadPassword() {
+		$this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
+		$this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false));
+		$this->socket->expects($this->at(2))->method('read')->will($this->returnValue("334 Login\r\n"));
+		$this->socket->expects($this->at(3))->method('write')->with("bWFyaw==\r\n");
+		$this->socket->expects($this->at(4))->method('read')->will($this->returnValue(false));
+		$this->socket->expects($this->at(5))->method('read')->will($this->returnValue("334 Pass\r\n"));
+		$this->socket->expects($this->at(6))->method('write')->with("c3Rvcnk=\r\n");
+		$this->socket->expects($this->at(7))->method('read')->will($this->returnValue(false));
+		$this->socket->expects($this->at(8))->method('read')->will($this->returnValue("535 5.7.8 Authentication failed\r\n"));
+		$this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story'));
+		$this->SmtpTransport->auth();
+	}
+
+/**
  * testAuthNoAuth method
  *
  * @return void