codeigniter4 / shield

Authentication and Authorization for CodeIgniter 4
https://shield.codeigniter.com
MIT License
366 stars 133 forks source link

Emails do not deliver #330

Closed sammyskills closed 2 years ago

sammyskills commented 2 years ago

Hi all,

Has anyone tried to use any of the auth actions (Email2FA or EmailActivator) and got an email delivered?

I've tried severally using gmail's smpt, cpanel email, etc, none delivered. I only got the message:

Cannot send email for user: kuqib@mailinator.com

First, the code does not help in knowing the exact error as one can not see the actual problem with printDebugger(): https://github.com/codeigniter4/shield/blob/835bec6a6bfde617f5f5c3f14935e8b61282c20b/src/Authentication/Actions/EmailActivator.php#L50-L52 and when I tried to edit the code to show the exact error message, I got this EXIM error:

We do not authorize the use of this system to transport unsolicited,  and/or bulk e-mail

Secondly, I recently tried using phpmailer (by extending the default activator class) with the same email settings and mails delivered successfully. But then, I couldn't get the code to be sent as it is called from a private method: https://github.com/codeigniter4/shield/blob/835bec6a6bfde617f5f5c3f14935e8b61282c20b/src/Authentication/Actions/EmailActivator.php#L104-L120

So, here's what I'm proposing:

What do you think guys? I can make a PR if that's okay.

kenjis commented 2 years ago

Thank you for reporting.

The email error should be logged in CI's log file: https://codeigniter4.github.io/CodeIgniter4/installation/troubleshooting.html#codeigniter-error-logs If it is not logged, it is a bug of the Email class.

I recently tried using phpmailer (by extending the default activator class) with the same email settings and mails delivered successfully.

I don't think it is the same email settings. If it is the same, both should be able to send email. Can you compare SMTP logs and find what's different?

kenjis commented 2 years ago

Can we have the default auth actions display or log the exact email error if it fails, for proper debugging?

What's the exact email error? How?

Can we also make the createIdentity() method a protected method instead of a private method so it can be used by child classes?

I'm fine.

sammyskills commented 2 years ago

Thank you for reporting.

The email error should be logged in CI's log file: https://codeigniter4.github.io/CodeIgniter4/installation/troubleshooting.html#codeigniter-error-logs If it is not logged, it is a bug of the Email class.

I have tried the default email class, and emails didn't deliver. I also discovered that my live website emails stopped working after I upgraded from v4.1.9 to v4.2.1. Prior to this time, emails delivered successfully. So, maybe, it is a bug of the Email class.

What's the exact email error? How?

Currently, when an email fails, it throws a RuntimeException: Cannot send email for user:. See: https://github.com/codeigniter4/shield/blob/e3cafec4a05da47ea3f177f4c0af8eab4601ab42/src/Authentication/Actions/EmailActivator.php#L50-L52 https://github.com/codeigniter4/shield/blob/e3cafec4a05da47ea3f177f4c0af8eab4601ab42/src/Authentication/Actions/Email2FA.php#L80-L82

IMO, this email error is vague as it doesn't help or state the reason why it failed, even in the error logs file.

kenjis commented 2 years ago

It seems there is no big change in the Email class.

$ git diff v4.1.9...v4.2.1 system/Email/
diff --git a/system/Email/Email.php b/system/Email/Email.php
index 966c08e6f..753dfe7d2 100644
--- a/system/Email/Email.php
+++ b/system/Email/Email.php
@@ -1046,10 +1046,8 @@ class Email
             $output .= $line . $this->newline;
         }

-        if ($unwrap) {
-            foreach ($unwrap as $key => $val) {
-                $output = str_replace('{{unwrapped' . $key . '}}', $val, $output);
-            }
+        foreach ($unwrap as $key => $val) {
+            $output = str_replace('{{unwrapped' . $key . '}}', $val, $output);
         }

         return $output;
kenjis commented 2 years ago

IMO, this email error is vague as it doesn't help or state the reason why it failed, even in the error logs file.

What do you mean? I think you can see the reason in the log file.

If you can improve the Exception message, please send a PR. I'll merge it.

sammyskills commented 2 years ago

What do you mean? I think you can see the reason in the log file.

Unfortunately, this is what is logged in the log file:

CRITICAL - 2022-08-04 11:12:59 --> Cannot send email for user: cateh@mailinator.com
in VENDORPATH\codeigniter4\shield\src\Authentication\Actions\EmailActivator.php on line 51.
 1 VENDORPATH\codeigniter4\shield\src\Controllers\ActionController.php(52): CodeIgniter\Shield\Authentication\Actions\EmailActivator->show()
 2 VENDORPATH\codeigniter4\shield\src\Controllers\ActionController.php(40): CodeIgniter\Shield\Controllers\ActionController->show()
 3 SYSTEMPATH\CodeIgniter.php(895): CodeIgniter\Shield\Controllers\ActionController->_remap('show')
 4 SYSTEMPATH\CodeIgniter.php(466): CodeIgniter\CodeIgniter->runController(Object(CodeIgniter\Shield\Controllers\ActionController))
 5 SYSTEMPATH\CodeIgniter.php(349): CodeIgniter\CodeIgniter->handleRequest(null, Object(Config\Cache), false)
 6 FCPATH\index.php(55): CodeIgniter\CodeIgniter->run()
 7 SYSTEMPATH\Commands\Server\rewrite.php(43): require_once('FCPATH\\index.php')

If you can improve the Exception message, please send a PR. I'll merge it.

Of course, I was looking at showing the headers error from the printDebugger() like so: $email->printDebugger(['headers']). I'll create a PR to that effect now.

kenjis commented 2 years ago

Is there any ERROR log? The Email class outputs ERROR log when error occurs.

If there is no ERROR log, it is a bug of CodeIgniter's Email class. It would be appreciated if you create issues in the CodeIgniter4 repository.

sammyskills commented 2 years ago

If there is no ERROR log, it is a bug of CodeIgniter's Email class. It would be appreciated if you create issues in the CodeIgniter4 repository.

Sure, I will do just that after testing with a fresh CI4 installation, just to be "very" sure.
Meanwhile, I created a PR #345 to address this issue.