From 9aa9008f8225800740f3885ab72ceea49c22db77 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Sat, 13 Jun 2020 20:30:44 +0200 Subject: [PATCH 1/8] add mail background worker --- app/app.php | 6 +-- app/controllers/api/account.php | 48 ++++++++------------ app/controllers/api/teams.php | 26 ++++------- app/workers/mails.php | 47 +++++++++++++++++++ docker/supervisord.conf | 17 +++++++ tests/e2e/Services/Account/AccountBase.php | 4 ++ tests/e2e/Services/Teams/TeamsBaseClient.php | 2 + 7 files changed, 100 insertions(+), 50 deletions(-) create mode 100644 app/workers/mails.php diff --git a/app/app.php b/app/app.php index 35f208b335..e6bebd2e5b 100644 --- a/app/app.php +++ b/app/app.php @@ -27,7 +27,7 @@ $services = include __DIR__.'/config/services.php'; // List of services $webhook = new Event('v1-webhooks', 'WebhooksV1'); $audit = new Event('v1-audits', 'AuditsV1'); $usage = new Event('v1-usage', 'UsageV1'); -$deletes = new Event('v1-deletes', 'DeletesV1'); +$mail = new Event('v1-mails', 'MailsV1'); /** * Get All verified client URLs for both console and current projects @@ -53,7 +53,7 @@ $clients = array_unique(array_merge($clientsConsole, array_map(function ($node) return false; })))); -$utopia->init(function () use ($utopia, $request, $response, &$user, $project, $console, $roles, $webhook, $audit, $usage, $clients) { +$utopia->init(function () use ($utopia, $request, $response, &$user, $project, $console, $roles, $webhook, $mail, $audit, $usage, $clients) { $route = $utopia->match($request); @@ -226,7 +226,7 @@ $utopia->init(function () use ($utopia, $request, $response, &$user, $project, $ ; }); -$utopia->shutdown(function () use ($response, $request, $webhook, $audit, $usage, $deletes, $mode, $project, $utopia) { +$utopia->shutdown(function () use ($response, $request, $webhook, $mail, $audit, $usage, $mode, $project, $utopia) { /* * Trigger events for background workers diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 282a5d9a22..eb4bc38db6 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1,7 +1,7 @@ post('/v1/account') ->param('password', '', function () { return new Password(); }, 'User password. Must be between 6 to 32 chars.') ->param('name', '', function () { return new Text(100); }, 'User name.', true) ->action( - function ($email, $password, $name) use ($register, $request, $response, $audit, $projectDB, $project, $webhook, $oauth2Keys) { + function ($email, $password, $name) use ($request, $response, $audit, $projectDB, $project, $webhook, $oauth2Keys) { if ('console' === $project->getId()) { $whitlistEmails = $project->getAttribute('authWhitelistEmails'); $whitlistIPs = $project->getAttribute('authWhitelistIPs'); @@ -1053,7 +1053,7 @@ $utopia->post('/v1/account/recovery') ->param('email', '', function () { return new Email(); }, 'User email.') ->param('url', '', function () use ($clients) { return new Host($clients); }, 'URL to redirect the user back to your app from the recovery email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.') ->action( - function ($email, $url) use ($request, $response, $projectDB, $register, $audit, $project) { + function ($email, $url) use ($request, $response, $projectDB, $mail, $audit, $project) { $profile = $projectDB->getCollection([ // Get user by email address 'limit' => 1, 'first' => true, @@ -1106,19 +1106,13 @@ $utopia->post('/v1/account/recovery') ->setParam('{{redirect}}', $url) ; - $mail = $register->get('smtp'); /* @var $mail \PHPMailer\PHPMailer\PHPMailer */ - - $mail->addAddress($profile->getAttribute('email', ''), $profile->getAttribute('name', '')); - - $mail->Subject = Locale::getText('account.emails.recovery.title'); - $mail->Body = $body->render(); - $mail->AltBody = strip_tags($body->render()); - - try { - $mail->send(); - } catch (\Exception $error) { - throw new Exception('Error sending mail: ' . $error->getMessage(), 500); - } + $mail + ->setParam('recipient', $profile->getAttribute('email', '')) + ->setParam('name', $profile->getAttribute('name', '')) + ->setParam('subject', Locale::getText('account.emails.recovery.title')) + ->setParam('body', $body->render()) + ->trigger(); + ; $audit ->setParam('userId', $profile->getId()) @@ -1214,7 +1208,7 @@ $utopia->post('/v1/account/verification') ->label('abuse-key', 'url:{url},email:{param-email}') ->param('url', '', function () use ($clients) { return new Host($clients); }, 'URL to redirect the user back to your app from the verification email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.') // TODO add built-in confirm page ->action( - function ($url) use ($request, $response, $register, $user, $project, $projectDB, $audit) { + function ($url) use ($request, $response, $mail, $user, $project, $projectDB, $audit) { $verificationSecret = Auth::tokenGenerator(); $verification = new Document([ @@ -1255,19 +1249,13 @@ $utopia->post('/v1/account/verification') ->setParam('{{redirect}}', $url) ; - $mail = $register->get('smtp'); /* @var $mail \PHPMailer\PHPMailer\PHPMailer */ - - $mail->addAddress($user->getAttribute('email'), $user->getAttribute('name')); - - $mail->Subject = Locale::getText('account.emails.verification.title'); - $mail->Body = $body->render(); - $mail->AltBody = strip_tags($body->render()); - - try { - $mail->send(); - } catch (\Exception $error) { - throw new Exception('Problem sending mail: ' . $error->getMessage(), 500); - } + $mail + ->setParam('recipient', $user->getAttribute('email')) + ->setParam('name', $user->getAttribute('name')) + ->setParam('subject', Locale::getText('account.emails.verification.title')) + ->setParam('body', $body->render()) + ->trigger() + ; $audit ->setParam('userId', $user->getId()) diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index ded2445faa..cd545954b8 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -1,6 +1,6 @@ post('/v1/teams/:teamId/memberships') ->param('roles', [], function () { return new ArrayList(new Text(128)); }, 'Array of strings. Use this param to set the user roles in the team. A role can be any string. Learn more about [roles and permissions](/docs/permissions).') ->param('url', '', function () use ($clients) { return new Host($clients); }, 'URL to redirect the user back to your app from the invitation email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.') // TODO add our own built-in confirm page ->action( - function ($teamId, $email, $name, $roles, $url) use ($response, $register, $project, $user, $audit, $projectDB, $mode) { + function ($teamId, $email, $name, $roles, $url) use ($response, $mail, $project, $user, $audit, $projectDB) { $name = (empty($name)) ? $email : $name; $team = $projectDB->getDocument($teamId); @@ -332,21 +332,13 @@ $utopia->post('/v1/teams/:teamId/memberships') ->setParam('{{redirect}}', $url) ; - $mail = $register->get('smtp'); /* @var $mail \PHPMailer\PHPMailer\PHPMailer */ - - $mail->addAddress($email, $name); - - $mail->Subject = sprintf(Locale::getText('account.emails.invitation.title'), $team->getAttribute('name', '[TEAM-NAME]'), $project->getAttribute('name', ['[APP-NAME]'])); - $mail->Body = $body->render(); - $mail->AltBody = strip_tags($body->render()); - - try { - if(APP_MODE_ADMIN !== $mode) { // No need in comfirmation when in admin mode - $mail->send(); - } - } catch (\Exception $error) { - throw new Exception('Error sending mail: ' . $error->getMessage(), 500); - } + $mail + ->setParam('recipient', $email) + ->setParam('name', $name) + ->setParam('subject', sprintf(Locale::getText('account.emails.invitation.title'), $team->getAttribute('name', '[TEAM-NAME]'), $project->getAttribute('name', ['[APP-NAME]']))) + ->setParam('body', $body->render()) + ->trigger(); + ; $audit ->setParam('userId', $invitee->getId()) diff --git a/app/workers/mails.php b/app/workers/mails.php new file mode 100644 index 0000000000..cb2fbd80fd --- /dev/null +++ b/app/workers/mails.php @@ -0,0 +1,47 @@ +args['recipient']; + $name = $this->args['name']; + $subject = $this->args['subject']; + $body = $this->args['body']; + + $mail = $register->get('smtp'); /* @var $mail \PHPMailer\PHPMailer\PHPMailer */ + + $mail->addAddress($recipient, $name); + $mail->Subject = $subject; + $mail->Body = $body; + $mail->AltBody = strip_tags($body); + + try { + $mail->send(); + } catch (\Exception $error) { + throw new Exception('Error sending mail: ' . $error->getMessage(), 500); + } + } + + public function tearDown() + { + // ... Remove environment for this job + } +} diff --git a/docker/supervisord.conf b/docker/supervisord.conf index 747e4d590d..ed94f03311 100644 --- a/docker/supervisord.conf +++ b/docker/supervisord.conf @@ -65,6 +65,23 @@ stdout_logfile_maxbytes=5000000 stderr_logfile=/dev/stderr stderr_logfile_maxbytes = 0 +[program:v1-mails] +command=php /usr/share/nginx/html/vendor/bin/resque +autostart=true +autorestart=true +priority=10 +environment=QUEUE='v1-mails',APP_INCLUDE='/usr/share/nginx/html/app/workers/mails.php',REDIS_BACKEND='%(ENV__APP_REDIS_HOST)s:%(ENV__APP_REDIS_PORT)s' +stdout_events_enabled=true +stderr_events_enabled=true +stopsignal=QUIT +startretries=10 +;stdout_logfile=/dev/stdout +;stdout_logfile_maxbytes=0 +stdout_logfile=/var/log/appwrite.log +stdout_logfile_maxbytes=5000000 +stderr_logfile=/dev/stderr +stderr_logfile_maxbytes = 0 + [program:v1-audits] command=php /usr/share/nginx/html/vendor/bin/resque autostart=true diff --git a/tests/e2e/Services/Account/AccountBase.php b/tests/e2e/Services/Account/AccountBase.php index 4e6d01d71e..97ca751035 100644 --- a/tests/e2e/Services/Account/AccountBase.php +++ b/tests/e2e/Services/Account/AccountBase.php @@ -658,6 +658,8 @@ trait AccountBase $this->assertNotEmpty($response['body']['$id']); $this->assertEquals(2, $response['body']['type']); $this->assertIsNumeric($response['body']['expire']); + + sleep(1); $lastEmail = $this->getLastEmail(); @@ -950,6 +952,8 @@ trait AccountBase $this->assertNotEmpty($response['body']['$id']); $this->assertEquals(3, $response['body']['type']); $this->assertIsNumeric($response['body']['expire']); + + sleep(1); $lastEmail = $this->getLastEmail(); diff --git a/tests/e2e/Services/Teams/TeamsBaseClient.php b/tests/e2e/Services/Teams/TeamsBaseClient.php index ceb0d86e5c..de0a140ec4 100644 --- a/tests/e2e/Services/Teams/TeamsBaseClient.php +++ b/tests/e2e/Services/Teams/TeamsBaseClient.php @@ -65,6 +65,8 @@ trait TeamsBaseClient $this->assertIsInt($response['body']['joined']); $this->assertEquals(false, $response['body']['confirm']); + sleep(1); + $lastEmail = $this->getLastEmail(); $this->assertEquals($email, $lastEmail['to'][0]['address']); From 5c364f9e947b331bc2fa8da990da22453478b700 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Sun, 14 Jun 2020 01:04:05 +0200 Subject: [PATCH 2/8] fix(tests) extend waiting time for --- tests/e2e/Scopes/Scope.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/Scopes/Scope.php b/tests/e2e/Scopes/Scope.php index f535d43633..f2018bd211 100644 --- a/tests/e2e/Scopes/Scope.php +++ b/tests/e2e/Scopes/Scope.php @@ -33,7 +33,7 @@ abstract class Scope extends TestCase protected function getLastEmail():array { - sleep(3); + sleep(5); $emails = json_decode(file_get_contents('http://maildev/email'), true); if($emails && is_array($emails)) { From 39edab15caae905ebe4b24339eaff5e244eca22d Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Sun, 14 Jun 2020 01:04:40 +0200 Subject: [PATCH 3/8] fix(tests) remove sleep before fetching emails --- tests/e2e/Services/Account/AccountBase.php | 4 ---- tests/e2e/Services/Teams/TeamsBaseClient.php | 2 -- 2 files changed, 6 deletions(-) diff --git a/tests/e2e/Services/Account/AccountBase.php b/tests/e2e/Services/Account/AccountBase.php index 97ca751035..794f6f2577 100644 --- a/tests/e2e/Services/Account/AccountBase.php +++ b/tests/e2e/Services/Account/AccountBase.php @@ -659,8 +659,6 @@ trait AccountBase $this->assertEquals(2, $response['body']['type']); $this->assertIsNumeric($response['body']['expire']); - sleep(1); - $lastEmail = $this->getLastEmail(); $this->assertEquals($email, $lastEmail['to'][0]['address']); @@ -953,8 +951,6 @@ trait AccountBase $this->assertEquals(3, $response['body']['type']); $this->assertIsNumeric($response['body']['expire']); - sleep(1); - $lastEmail = $this->getLastEmail(); $this->assertEquals($email, $lastEmail['to'][0]['address']); diff --git a/tests/e2e/Services/Teams/TeamsBaseClient.php b/tests/e2e/Services/Teams/TeamsBaseClient.php index de0a140ec4..ceb0d86e5c 100644 --- a/tests/e2e/Services/Teams/TeamsBaseClient.php +++ b/tests/e2e/Services/Teams/TeamsBaseClient.php @@ -65,8 +65,6 @@ trait TeamsBaseClient $this->assertIsInt($response['body']['joined']); $this->assertEquals(false, $response['body']['confirm']); - sleep(1); - $lastEmail = $this->getLastEmail(); $this->assertEquals($email, $lastEmail['to'][0]['address']); From 9ec49b90b32e085b7cfe6a801e513f7fac29cdfd Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Sun, 14 Jun 2020 01:06:05 +0200 Subject: [PATCH 4/8] add event property to mail worker --- app/controllers/api/account.php | 2 ++ app/controllers/api/teams.php | 13 +++++++------ app/workers/mails.php | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index eb4bc38db6..17369b0c36 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1107,6 +1107,7 @@ $utopia->post('/v1/account/recovery') ; $mail + ->setParam('event', 'account.recovery.create') ->setParam('recipient', $profile->getAttribute('email', '')) ->setParam('name', $profile->getAttribute('name', '')) ->setParam('subject', Locale::getText('account.emails.recovery.title')) @@ -1250,6 +1251,7 @@ $utopia->post('/v1/account/verification') ; $mail + ->setParam('event', 'account.verification.create') ->setParam('recipient', $user->getAttribute('email')) ->setParam('name', $user->getAttribute('name')) ->setParam('subject', Locale::getText('account.emails.verification.title')) diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index cd545954b8..bff9629af7 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -333,12 +333,13 @@ $utopia->post('/v1/teams/:teamId/memberships') ; $mail - ->setParam('recipient', $email) - ->setParam('name', $name) - ->setParam('subject', sprintf(Locale::getText('account.emails.invitation.title'), $team->getAttribute('name', '[TEAM-NAME]'), $project->getAttribute('name', ['[APP-NAME]']))) - ->setParam('body', $body->render()) - ->trigger(); - ; + ->setParam('event', 'teams.membership.create') + ->setParam('recipient', $email) + ->setParam('name', $name) + ->setParam('subject', sprintf(Locale::getText('account.emails.invitation.title'), $team->getAttribute('name', '[TEAM-NAME]'), $project->getAttribute('name', ['[APP-NAME]']))) + ->setParam('body', $body->render()) + ->trigger(); + ; $audit ->setParam('userId', $invitee->getId()) diff --git a/app/workers/mails.php b/app/workers/mails.php index cb2fbd80fd..c46fda89dc 100644 --- a/app/workers/mails.php +++ b/app/workers/mails.php @@ -21,6 +21,7 @@ class MailsV1 { global $register; + $event = $this->args['event']; $recipient = $this->args['recipient']; $name = $this->args['name']; $subject = $this->args['subject']; From 89e7a5c2c45ec6cbcbfb36d1c878eda18de75870 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Sun, 14 Jun 2020 01:06:19 +0200 Subject: [PATCH 5/8] rebase into master --- app/app.php | 3 ++- app/controllers/api/teams.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/app.php b/app/app.php index e6bebd2e5b..e01a57c137 100644 --- a/app/app.php +++ b/app/app.php @@ -28,6 +28,7 @@ $webhook = new Event('v1-webhooks', 'WebhooksV1'); $audit = new Event('v1-audits', 'AuditsV1'); $usage = new Event('v1-usage', 'UsageV1'); $mail = new Event('v1-mails', 'MailsV1'); +$deletes = new Event('v1-deletes', 'DeletesV1'); /** * Get All verified client URLs for both console and current projects @@ -226,7 +227,7 @@ $utopia->init(function () use ($utopia, $request, $response, &$user, $project, $ ; }); -$utopia->shutdown(function () use ($response, $request, $webhook, $mail, $audit, $usage, $mode, $project, $utopia) { +$utopia->shutdown(function () use ($response, $request, $webhook, $deletes, $audit, $usage, $mode, $project, $utopia) { /* * Trigger events for background workers diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index bff9629af7..0f71b0f346 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -215,7 +215,7 @@ $utopia->post('/v1/teams/:teamId/memberships') ->param('roles', [], function () { return new ArrayList(new Text(128)); }, 'Array of strings. Use this param to set the user roles in the team. A role can be any string. Learn more about [roles and permissions](/docs/permissions).') ->param('url', '', function () use ($clients) { return new Host($clients); }, 'URL to redirect the user back to your app from the invitation email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.') // TODO add our own built-in confirm page ->action( - function ($teamId, $email, $name, $roles, $url) use ($response, $mail, $project, $user, $audit, $projectDB) { + function ($teamId, $email, $name, $roles, $url) use ($response, $mail, $project, $user, $audit, $projectDB, $mode) { $name = (empty($name)) ? $email : $name; $team = $projectDB->getDocument($teamId); From 6b365d5e730c5c9b8b703028af5057d5f583226c Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Sun, 14 Jun 2020 01:14:30 +0200 Subject: [PATCH 6/8] remove unnecessary change --- app/app.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/app.php b/app/app.php index e01a57c137..dcab392a73 100644 --- a/app/app.php +++ b/app/app.php @@ -227,7 +227,7 @@ $utopia->init(function () use ($utopia, $request, $response, &$user, $project, $ ; }); -$utopia->shutdown(function () use ($response, $request, $webhook, $deletes, $audit, $usage, $mode, $project, $utopia) { +$utopia->shutdown(function () use ($response, $request, $webhook, $audit, $usage, $deletes, $mode, $project, $utopia) { /* * Trigger events for background workers From 06c71f40b9f449296d3553e8f36699f2a4e66363 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Sun, 14 Jun 2020 10:12:26 +0200 Subject: [PATCH 7/8] extend timeout for email tests --- tests/e2e/Scopes/Scope.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/Scopes/Scope.php b/tests/e2e/Scopes/Scope.php index f2018bd211..842e9a34b6 100644 --- a/tests/e2e/Scopes/Scope.php +++ b/tests/e2e/Scopes/Scope.php @@ -33,7 +33,7 @@ abstract class Scope extends TestCase protected function getLastEmail():array { - sleep(5); + sleep(10); $emails = json_decode(file_get_contents('http://maildev/email'), true); if($emails && is_array($emails)) { From eb7e80a628b8dfb036c2593b0a300a2a61c37fb8 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Sun, 14 Jun 2020 13:44:57 +0200 Subject: [PATCH 8/8] fix admin mode check --- app/controllers/api/teams.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 0f71b0f346..54ff361a54 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -332,14 +332,16 @@ $utopia->post('/v1/teams/:teamId/memberships') ->setParam('{{redirect}}', $url) ; - $mail - ->setParam('event', 'teams.membership.create') - ->setParam('recipient', $email) - ->setParam('name', $name) - ->setParam('subject', sprintf(Locale::getText('account.emails.invitation.title'), $team->getAttribute('name', '[TEAM-NAME]'), $project->getAttribute('name', ['[APP-NAME]']))) - ->setParam('body', $body->render()) - ->trigger(); - ; + if(APP_MODE_ADMIN !== $mode) { // No need in comfirmation when in admin mode + $mail + ->setParam('event', 'teams.membership.create') + ->setParam('recipient', $email) + ->setParam('name', $name) + ->setParam('subject', sprintf(Locale::getText('account.emails.invitation.title'), $team->getAttribute('name', '[TEAM-NAME]'), $project->getAttribute('name', ['[APP-NAME]']))) + ->setParam('body', $body->render()) + ->trigger(); + ; + } $audit ->setParam('userId', $invitee->getId())