Skip to content

Commit

Permalink
Merge pull request #6466 from JediKev/oauth2/strict-matching
Browse files Browse the repository at this point in the history 
Reviewed-By: protich <peter@osticket.com>, JediKev <kevin@enhancesoft.com>
  • Loading branch information
@JediKev
JediKev committed Mar 7, 2023
2 parents e76e91a + c0cd8b1 commit 76149d6
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 15 deletions.
17 changes: 17 additions & 0 deletions include/ajax.email.php
View file
Expand Up @@ -39,4 +39,21 @@ function configureAuth($id, $type, $auth) {
$template = sprintf('email-%sauth.tmpl.php', $authtype);
include INCLUDE_DIR . "staff/templates/$template";
}

/*
* Delete the OAuth2 token
*/
function deleteToken($id, $type) {
// Check to make sure the email exists
if (!($email=Email::lookup($id)))
Http::response(404, 'Unknown Email');
// Get the authentication account
if (!($account=$email->getAuthAccount($type)))
Http::response(404, 'Unknown Authentication Type');
// Destory the account config which will delete the token
if ($account->destroyConfig())
Http::response(201, __('Token Deleted Successfully.'));
// Couldn't delete the Token
Http::response(404, 'Unable to delete token.');
}
}
76 changes: 72 additions & 4 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 All @@ -522,7 +536,8 @@ public function getBkId() {
$id = sprintf('%s:%d',
$this->getAuthBk(), $this->getId());
if ($this->isOAuthAuth())
$id .= sprintf(':%d', $this->getAuthId());
$id .= sprintf(':%d:%b',
$this->getAuthId(), $this->isStrict()); #TODO: Remove strict and delegate to email account

$this->bkId = $id;
}
Expand All @@ -537,6 +552,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 +570,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 +645,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 +709,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 @@ -960,6 +983,13 @@ public function updateCredentials($auth, $vars, &$errors) {
return $this->save();
}

/*
* Destory the account config
*/
function destroyConfig() {
return $this->getConfig()->destroy();
}

function update($vars, &$errors) {
return false;
}
Expand Down Expand Up @@ -987,7 +1017,7 @@ function save($refetch=false) {

function delete() {
// Destroy the Email config
$this->getConfig()->destroy();
$this->destroyConfig();
// Delete the Plugin instance
if ($this->isOAuthAuth() && ($i=$this->getOAuth2Instance()))
$i->delete();
Expand Down Expand Up @@ -1242,6 +1272,23 @@ static public function objects() {
->filter(['type' => 'smtp']);
}

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() {
if ($this->isMailboxAuth()
&& ($email=$this->getEmail())
&& ($account=$email->getMailboxAccount()))
return $account->getConfig();
return parent::getConfig();
}

public function allowSpoofing() {
return ($this->allow_spoofing);
}
Expand Down Expand Up @@ -1295,6 +1342,13 @@ protected function setInfo($vars, &$errors) {
&& !($creds=$this->getFreshCredentials($vars['smtp_auth_bk'])))
$_errors['smtp_auth_bk'] = __('Configure Authentication');

// 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) {
$this->active = $vars['smtp_active'] ? 1 : 0;
$this->host = $vars['smtp_host'];
Expand Down Expand Up @@ -1340,6 +1394,20 @@ static function create($ht=false) {
*
*/
class EmailAccountConfig extends Config {
/*
* Get strict matching (default: true)
*/
public function getStrictMatching() {
return $this->get('strict_matching', true);
}

/*
* Set strict matching
*/
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.
71 changes: 60 additions & 11 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);
$addr = $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 $addr; ?>"/>&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 Expand Up @@ -108,7 +123,14 @@
__('Expired Access Token gets auto-refreshed on use'));
}
?>
<table class="form_table" width="940" border="0" cellspacing="0" cellpadding="2">
<a class="red button action-button pull-right" style="margin-bottom:8px;"
id="token-delete" data-toggle="tooltip"
href="<?php echo sprintf('#email/%d/auth/config/%s/delete', $email->getId(), $type); ?>"
title="Delete" data-original-title="Delete">
<i class="icon-trash"></i>&nbsp;<?php echo __('Delete Token'); ?>
</a>
<div class="clear"></div>
<table class="form_table" width="100%" border="0" cellspacing="0" cellpadding="2">
<thead>
<tr>
<th colspan="2">
Expand Down Expand Up @@ -158,3 +180,30 @@
</span>
</p>
</form>
<script type="text/javascript">
$(function() {
$('#oauth-tabs_container').on('click', 'a#token-delete', function(e) {
e.preventDefault();
if (confirm(__('Are you sure?'))) {
$.ajax({
url: 'ajax.php/' + $(this).attr('href').substr(1),
type: 'POST',
success: function(json) {
// Remove the Token tab completely
$('#popup a[href="#token"]').parent().remove();
$('#popup div#token').remove();
// Add success banner
$('#popup form').before('<div id="msg_notice">'+json+'</div>');
// Load the IdP tab
$('#popup a[href="#idp"]').click();
},
error: function(json) {
// Add error banner
$('#popup form').before('<div id="msg_error">'+json.responseText+'</div>');
}
});
}
return false;
});
});
</script>
1 change: 1 addition & 0 deletions scp/ajax.php
View file
Expand Up @@ -316,6 +316,7 @@ function staffLoginPage($msg='Unauthorized') {
)),
url('^/email', patterns('ajax.email.php:EmailAjaxAPI',
url_post('^/(?P<id>\d+)/stash$', 'stashFormData'),
url_post('^/(?P<id>\d+)/auth/config/(?P<type>\w+)/delete$', 'deleteToken'),
url('^/(?P<id>\d+)/auth/config/(?P<type>\w+)/(?P<auth>.+)$', 'configureAuth'),
))
);
Expand Down

0 comments on commit 76149d6

Please sign in to comment.