Skip to content

Commit

Permalink
oauth2: Strict Matching
Browse files Browse the repository at this point in the history 
This is a rewrite of osTicket/osTicket-plugins#263 where it was
attempting to implement Strict Matching. This fully implements Strict
Matching and adds functionality to check for this when retrieving a
token and when saving SMTP with Same As Remote Mailbox. This also adds a
new disabled field to the General Settings to show what email will be
used as well as a help tip that further explains Strict Matching.
  • Loading branch information
@JediKev
JediKev committed Feb 14, 2023
1 parent 6126363 commit 7275d67
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 12 deletions.
47 changes: 45 additions & 2 deletions include/class.email.php
View file
Expand Up @@ -487,6 +487,17 @@ public function isAuthBackendEnabled() {
: true;
}

public function isStrict() {
return $this->getConfig()->getStrictMatching();
}

public function checkStrictMatching($token=null) {
$token ??= $this->getAccessToken();
return ($token && $token->isMatch(
$this->getEmail()->getEmail(),
$this->isStrict()));
}

public function shouldAuthorize() {
// check status and make sure it's oauth
if (!$this->isAuthBackendEnabled() || !$this->isOAuthAuth())
Expand All @@ -497,7 +508,10 @@ public function shouldAuthorize() {
// changed somehow
|| !($token=$cred->getAccessToken($this->getConfigSignature()))
// Check if expired
|| $token->isExpired());
|| $token->isExpired()
// If Strict Matching is enabled ensure the email matches
// the Resource Owner
|| !$this->checkStrictMatching($token));

}

Expand Down Expand Up @@ -537,6 +551,11 @@ public function getEmail() {
return $this->email;
}

public function getAccessToken() {
$cred = $this->getFreshCredentials();
return $cred ? $cred->getAccessToken($this->getConfigSignature()) : null;
}

private function getOAuth2Backend($auth=null) {
$auth = $auth ?: $this->getAuthBk();
return Oauth2AuthorizationBackend::getBackend($auth);
Expand All @@ -550,6 +569,7 @@ public function getOAuth2ConfigDefaults() {
'name' => sprintf('%s (%s)',
$email->getEmail(), $this->getType()),
'isactive' => 1,
'strict_matching' => $this->isStrict(),
'notes' => sprintf(
__('OAuth2 Authorization for %s'), $email->getEmail()),
];
Expand Down Expand Up @@ -624,7 +644,7 @@ private function getNamespace() {
$this->getId());
}

private function getConfig() {
protected function getConfig() {
if (!isset($this->config))
$this->config = new EmailAccountConfig($this->getNamespace());
return $this->config;
Expand Down Expand Up @@ -688,6 +708,8 @@ public function saveAuth($auth, $form, &$errors) {
// Auth backend can be changed on update
$this->auth_bk = $auth;
$this->save();
// Update Strict Matching
$this->getConfig()->setStrictMatching($_POST['strict_matching'] ? 1 : 0);
} elseif (!isset($errors['err'])) {
$errors['err'] = sprintf('%s %s',
__('Error Saving'),
Expand Down Expand Up @@ -1242,6 +1264,16 @@ static public function objects() {
->filter(['type' => 'smtp']);
}

public function isMailboxAuth() {
return (strcasecmp($this->getAuthBk(), 'mailbox') === 0);
}

protected function getConfig() {
return $this->isMailboxAuth()
? $this->getEmail()->getMailboxAccount()->getConfig()
: parent::getConfig();
}

public function allowSpoofing() {
return ($this->allow_spoofing);
}
Expand Down Expand Up @@ -1295,6 +1327,9 @@ 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())
$_errors['smtp_auth_bk'] = sprintf('%s and %s', __('Resource Owner'), __('Email Mismatch'));

if (!$_errors) {
$this->active = $vars['smtp_active'] ? 1 : 0;
$this->host = $vars['smtp_host'];
Expand Down Expand Up @@ -1340,6 +1375,14 @@ static function create($ht=false) {
*
*/
class EmailAccountConfig extends Config {
public function getStrictMatching() {
return $this->get('strict_matching', false);
}

public function setStrictMatching($mode) {
return $this->set('strict_matching', !!$mode);
}

public function updateInfo($vars) {
return parent::updateAll($vars);
}
Expand Down
4 changes: 4 additions & 0 deletions include/class.oauth2.php
View file
Expand Up @@ -95,6 +95,10 @@ public function isExpired() {
return $this->hasExpired();
}

public function isMatch($email, $strict=false) {
return !$strict ? true : (strcasecmp($this->getResourceOwnerEmail(), $email) === 0);
}

public function getAuthRequest() {
if ($this->hasExpired())
throw new Exception('Access Token is Expired');
Expand Down
9 changes: 9 additions & 0 deletions include/i18n/en_US/help/tips/emails.email.yaml
View file
Expand Up @@ -170,3 +170,12 @@ header_spoofing:
This advanced setting is generally used when sending mail from
aliases of this mail box.
strict_matching:
title: Strict Email Matching
content: >
<b>Enable</b> this option to require the Email Address and the Token's Resource Owner to match.
This is useful for preventing accidental authorization of a completely different account.
<br><br>
<b>Disable</b> this option to allow a different Resource Owner for the Token than the Email
Address. This is useful for Shared Mailboxes/Aliases/etc. where you need to use a Service
Account or Global Admin to authorize on behalf of the email.
35 changes: 25 additions & 10 deletions include/staff/templates/email-oauth2auth.tmpl.php
View file
Expand Up @@ -14,6 +14,7 @@
? array_merge($info, $_POST) : $info, true);
$action = sprintf('#email/%d/auth/config/%s/%s',
$email->getId(), $type, $auth);
$email = $account->getEmail()->email;
?>
<h3><?php echo __('OAuth2 Authorization'); ?></h3>
<b><a class="close" href="#"><i class="icon-remove-circle"></i></a></b>
Expand Down Expand Up @@ -51,20 +52,11 @@
<thead>
<tr>
<th colspan="2">
<em><?php echo __('Name and Status'); ?></em>
<em><?php echo __('General Settings'); ?></em>
</th>
</tr>
</thead>
<tbody>
<tr>
<td width="180" class="required"><?php echo __('Name'); ?>:</td>
<td>
<input size="50" type="text" autofocus
name="name"
value="<?php echo $info['name']; ?>"/>
<span class="error">*<br/> <?php echo $errors['name']; ?></span>
</td>
</tr>
<tr>
<td width="180" class="required"><?php echo __('Status'); ?>:</td>
<td><select name="isactive">
Expand All @@ -77,6 +69,29 @@
</select>
</td>
</tr>
<tr>
<td width="180" class="required"><?php echo __('Name'); ?>:</td>
<td>
<input size="50" type="text" autofocus
name="name"
value="<?php echo $info['name']; ?>"/>
<span class="error">*<br/> <?php echo $errors['name']; ?></span>
</td>
</tr>
<tr>
<td width="180"><b><?php echo __('Email Address'); ?>:</b></td>
<td>
<input size="35" type="text" autofocus
name="name"
disabled="disabled"
value="<?php echo $email; ?>"/>&nbsp;
<input type="checkbox" name="strict_matching"
<?php if ($info['strict_matching']) echo 'checked="checked"'; ?>>
&nbsp;<?php echo __('Strict Matching'); ?>
<i class="help-tip icon-question-sign" href="#strict_matching"></i>
<span class="error"><br/> <?php echo $errors['name']; ?></span>
</td>
</tr>
</tbody>
<tbody>
<tr>
Expand Down

0 comments on commit 7275d67

Please sign in to comment.