Skip to content

Commit

Permalink
Add ability to restrict certain key types to certain permissions. #35
Browse files Browse the repository at this point in the history
  • Loading branch information
ShaneMcC committed Mar 11, 2019
1 parent 9b7b9d0 commit 22e84f0
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 9 deletions.
24 changes: 23 additions & 1 deletion classes/twofactorkey.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,16 @@ public function setActive($value) {
return $this->setData('active', parseBool($value) ? 'true' : 'false');
}

public function getRequiredPermissionsForType() {
switch ($this->getType()) {
case "authy":
return ['2fa_push'];

default:
return [];
}
}

public function setType($value) {
// Clear key when changing type.
$this->setData('key', NULL);
Expand Down Expand Up @@ -255,10 +265,11 @@ public static function getKeyTypes() {
* - They are not one time, or they have not been used
* - expiry date is "0" or in the future
* - We have the required ability to validate
* - If a user is passed, also check that they have the appropriate permissions.
*
* @return True if key is usable.
*/
public function isUsableKey() {
public function isUsableKey($user = FALSE) {
// Key is usable.
$usable = $this->isActive();

Expand All @@ -277,6 +288,17 @@ public function isUsableKey() {
$usable = false;
}

if ($usable && $user !== FALSE) {
// Can we use this type of key.
$usable &= (($this->isPush() && $user->getPermission('2fa_push')) || $this->isCode());

// Check that the user still has the right permissions for this
// key.
foreach ($this->getRequiredPermissionsForType() as $perm) {
$usable &= $user->getPermission($perm);
}
}

return $usable;
}

Expand Down
1 change: 1 addition & 0 deletions classes/user.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class User extends DBObject {
'impersonate_users',
'system_service_mgmt',
'manage_articles',
'2fa_push',
];

// Permissions levels for unknown objects.
Expand Down
15 changes: 9 additions & 6 deletions web/1.0/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,13 @@
}

$keys = [];
foreach ($possibleKeys as $key) { if ($key->isUsableKey()) { $keys[] = $key; } }
foreach ($possibleKeys as $key) { if ($key->isUsableKey($user)) { $keys[] = $key; } }

$valid = true;
if (count($keys) > 0) {
$valid = false;
$testCode = isset($_SERVER['HTTP_X_2FA_KEY']) ? $_SERVER['HTTP_X_2FA_KEY'] : NULL;
$tryPush = isset($_SERVER['HTTP_X_2FA_PUSH']) && parseBool($_SERVER['HTTP_X_2FA_PUSH']);
$tryPush = isset($_SERVER['HTTP_X_2FA_PUSH']) && parseBool($_SERVER['HTTP_X_2FA_PUSH']) && $user->getPermission('2fa_push');

if ($testCode !== NULL) {
foreach ($keys as $key) {
Expand Down Expand Up @@ -242,10 +242,13 @@
} else {
$errorExtraData = '2FA key required.';
$resp->setHeader('login_error', '2fa_required');
foreach ($keys as $key) {
if ($key->isPush()) {
$resp->setHeader('2fa_push', '2fa push supported');
break;

if ($user->getPermission('2fa_push')) {
foreach ($keys as $key) {
if ($key->isPush()) {
$resp->setHeader('2fa_push', '2fa push supported');
break;
}
}
}
}
Expand Down
16 changes: 14 additions & 2 deletions web/1.0/methods/useradmin.php
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ protected function get2FAKeys($user) {
if ($v->isActive()) {
unset($result[$k]['key']);
}
$result[$k]['usable'] = parseBool($v->isUsableKey());
$result[$k]['usable'] = parseBool($v->isUsableKey($user));
$result[$k]['active'] = parseBool($result[$k]['active']);
$result[$k]['onetime'] = parseBool($result[$k]['onetime']);
}
Expand Down Expand Up @@ -492,7 +492,7 @@ protected function get2FAKey($user, $key) {
if ($key->isActive()) {
unset($k['key']);
}
$k['usable'] = $key->isUsableKey();
$k['usable'] = $key->isUsableKey($user);
$k['active'] = parseBool($k['active']);
$k['onetime'] = parseBool($k['onetime']);

Expand All @@ -513,6 +513,12 @@ protected function create2FAKey($user) {
$key->setOneTime($data['data']['onetime']);
}

foreach ($key->getRequiredPermissionsForType() as $perm) {
if (!$user->getPermission($perm)) {
$this->getContextKey('response')->sendError('Error creating key: Missing permission "' . $perm . '"');
}
}

try {
$key->setKey(TRUE);
} catch (TwoFactorKeyAutoValueException $e) {
Expand Down Expand Up @@ -613,6 +619,12 @@ protected function verify2FAKey($user, $key) {
$this->getContextKey('response')->sendError('No code provided for verification.');
}

foreach ($key->getRequiredPermissionsForType() as $perm) {
if (!$user->getPermission($perm)) {
$this->getContextKey('response')->sendError('Error verifying key: Missing permission "' . $perm . '"');
}
}

if ($key->isPush()) {
$this->getContextKey('response')->setHeader('info', 'Key will be verified in the background.');

Expand Down

0 comments on commit 22e84f0

Please sign in to comment.