Skip to content
Permalink
Browse files Browse the repository at this point in the history
Backport code audit from 1.2
Fixes issue with permissions checking
  • Loading branch information
daftspunk committed Apr 1, 2021
1 parent 7814696 commit 016a297
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/Auth/Manager.php
Expand Up @@ -241,7 +241,7 @@ public function findUserByCredentials(array $credentials)
foreach ($hashedCredentials as $credential => $value) {
if (!$user->checkHashValue($credential, $value)) {
// Incorrect password
if ($credential == 'password') {
if ($credential === 'password') {
throw new AuthException(sprintf(
'A user was found to match all plain text credentials however hashed credential "%s" did not match.',
$credential
Expand Down
8 changes: 4 additions & 4 deletions src/Auth/Models/Role.php
Expand Up @@ -84,7 +84,7 @@ public function hasAccess($permissions, $all = true)

// We will make sure that the merged permission does not
// exactly match our permission, but starts with it.
if ($checkPermission != $rolePermission && starts_with($rolePermission, $checkPermission) && $value == 1) {
if ($checkPermission != $rolePermission && starts_with($rolePermission, $checkPermission) && $value === 1) {
$matched = true;
break;
}
Expand All @@ -102,7 +102,7 @@ public function hasAccess($permissions, $all = true)

// We will make sure that the merged permission does not
// exactly match our permission, but ends with it.
if ($checkPermission != $rolePermission && ends_with($rolePermission, $checkPermission) && $value == 1) {
if ($checkPermission != $rolePermission && ends_with($rolePermission, $checkPermission) && $value === 1) {
$matched = true;
break;
}
Expand All @@ -121,14 +121,14 @@ public function hasAccess($permissions, $all = true)

// We will make sure that the merged permission does not
// exactly match our permission, but starts with it.
if ($checkGroupPermission != $permission && starts_with($permission, $checkGroupPermission) && $value == 1) {
if ($checkGroupPermission != $permission && starts_with($permission, $checkGroupPermission) && $value === 1) {
$matched = true;
break;
}
}
// Otherwise, we'll fallback to standard permissions checking where
// we match that permissions explicitly exist.
elseif ($permission == $rolePermission && $rolePermissions[$permission] == 1) {
elseif ($permission === $rolePermission && $rolePermissions[$permission] === 1) {
$matched = true;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Auth/Models/Throttle.php
Expand Up @@ -99,7 +99,7 @@ public function clearLoginAttempts()
// anything either as clearing login attempts
// makes us unsuspended. We need to manually
// call unsuspend() in order to unsuspend.
if ($this->getLoginAttempts() == 0 or $this->is_suspended) {
if ($this->getLoginAttempts() === 0 or $this->is_suspended) {
return;
}

Expand Down
14 changes: 7 additions & 7 deletions src/Auth/Models/User.php
Expand Up @@ -190,7 +190,7 @@ public function checkPersistCode($persistCode)
return false;
}

return $persistCode == $this->persist_code;
return $persistCode === $this->persist_code;
}

//
Expand Down Expand Up @@ -278,7 +278,7 @@ public function checkResetPasswordCode($resetCode)
return false;
}

return ($this->reset_password_code == $resetCode);
return ($this->reset_password_code === $resetCode);
}

/**
Expand Down Expand Up @@ -386,7 +386,7 @@ public function removeGroup($group)
public function inGroup($group)
{
foreach ($this->getGroups() as $_group) {
if ($_group->getKey() == $group->getKey()) {
if ($_group->getKey() === $group->getKey()) {
return true;
}
}
Expand Down Expand Up @@ -481,7 +481,7 @@ public function hasPermission($permissions, $all = true)

// We will make sure that the merged permission does not
// exactly match our permission, but starts with it.
if ($checkPermission != $mergedPermission && starts_with($mergedPermission, $checkPermission) && $value == 1) {
if ($checkPermission != $mergedPermission && starts_with($mergedPermission, $checkPermission) && $value === 1) {

This comment has been minimized.

Copy link
@prhost

prhost Apr 7, 2021

Member

apparently this $value is returning a string and not an integer. Any idea what it might be?

Screen Shot 2021-04-07 at 00 35 40

This comment has been minimized.

Copy link
@daftspunk

daftspunk Apr 7, 2021

Author Member

Damn, must be coming from the database. Cast as int here 5bd1a28

Thanks for the heads up. What database engine are you using?

This comment has been minimized.

Copy link
@prhost

prhost Apr 7, 2021

Member

mysql

$matched = true;
break;
}
Expand All @@ -496,7 +496,7 @@ public function hasPermission($permissions, $all = true)

// We will make sure that the merged permission does not
// exactly match our permission, but ends with it.
if ($checkPermission != $mergedPermission && ends_with($mergedPermission, $checkPermission) && $value == 1) {
if ($checkPermission != $mergedPermission && ends_with($mergedPermission, $checkPermission) && $value === 1) {
$matched = true;
break;
}
Expand All @@ -515,15 +515,15 @@ public function hasPermission($permissions, $all = true)

// We will make sure that the merged permission does not
// exactly match our permission, but starts with it.
if ($checkMergedPermission != $permission && starts_with($permission, $checkMergedPermission) && $value == 1) {
if ($checkMergedPermission != $permission && starts_with($permission, $checkMergedPermission) && $value === 1) {
$matched = true;
break;
}
}

// Otherwise, we'll fallback to standard permissions checking where
// we match that permissions explicitly exist.
elseif ($permission == $mergedPermission && $mergedPermissions[$permission] == 1) {
elseif ($permission === $mergedPermission && $mergedPermissions[$permission] === 1) {
$matched = true;
break;
}
Expand Down

0 comments on commit 016a297

Please sign in to comment.