oauth2: Bug Fixes

This addresses some bugs found during testing. This adds a check to `SmtpAccount::getConfig()` that ensures we have an email and MailboxAccount before attempting to call `getConfig()` and adds descriptive comments. This also removes the `isActive()` check from the SmtpAccount `save()` method as we only care about if it's currently set to active. Lastly, this changes the variable name for the email address from `$email` to `$addr` so it doesn't conflict with the original `$email` variable.
osTicket · Feb 16, 2023 · cb3625e · cb3625e
1 parent ca913ba
commit cb3625e
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 6 deletions.
diff --git a/include/class.email.php b/include/class.email.php
@@ -1276,10 +1276,17 @@ public function isMailboxAuth() {
         return (strcasecmp($this->getAuthBk(), 'mailbox') === 0);
     }
 
+    /*
+     * Check if using mailbox auth and MailboxAccount exists if so
+     * return the MailboxAccount config, otherwise return it's own
+     * config
+     */
     protected function getConfig() {
-        return $this->isMailboxAuth()
-                ? $this->getEmail()->getMailboxAccount()->getConfig()
-                : parent::getConfig();
+        if ($this->isMailboxAuth()
+                && ($email=$this->getEmail())
+                && ($account=$email->getMailboxAccount()))
+            return $account->getConfig();
+        return parent::getConfig();
     }
 
     public function allowSpoofing() {
@@ -1335,7 +1342,11 @@ protected function setInfo($vars, &$errors) {
                 && !($creds=$this->getFreshCredentials($vars['smtp_auth_bk'])))
             $_errors['smtp_auth_bk'] = __('Configure Authentication');
 
-        if (($vars['smtp_auth_bk'] === 'mailbox') && !$this->checkStrictMatching())
+        // Check if set to active and using mailbox auth, if so check strict
+        // matching.
+        if ($vars['smtp_active'] == 1
+                && ($vars['smtp_auth_bk'] === 'mailbox')
+                && !$this->checkStrictMatching())
             $_errors['smtp_auth_bk'] = sprintf('%s and %s', __('Resource Owner'), __('Email Mismatch'));
 
         if (!$_errors) {

diff --git a/include/staff/templates/email-oauth2auth.tmpl.php b/include/staff/templates/email-oauth2auth.tmpl.php
@@ -14,7 +14,7 @@
         ? array_merge($info, $_POST) : $info, true);
 $action = sprintf('#email/%d/auth/config/%s/%s',
         $email->getId(), $type, $auth);
-$email = $account->getEmail()->email;
+$addr = $account->getEmail()->email;
 ?>
 <h3><?php echo __('OAuth2 Authorization'); ?></h3>
 <b><a class="close" href="#"><i class="icon-remove-circle"></i></a></b>
@@ -84,7 +84,7 @@
                     <input size="35" type="text" autofocus
                         name="name"
                         disabled="disabled"
-                        value="<?php echo $email; ?>"/>&nbsp;
+                        value="<?php echo $addr; ?>"/>&nbsp;
                     <input type="checkbox" name="strict_matching"
                         <?php if ($info['strict_matching']) echo 'checked="checked"'; ?>>
                     &nbsp;<?php echo __('Strict Matching'); ?>