[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [websites/sso-kde-org] app: Reorganize ACL save logic. Add some negative tests for ACL
From: Sayak Banerjee <sayakb () kde ! org>
Date: 2014-10-26 17:16:15
Message-ID: E1XiRQN-0008CJ-Dk () scm ! kde ! org
[Download RAW message or body]
Git commit bbad317a18eb1450d544c5c076de6bf6c21064d0 by Sayak Banerjee.
Committed on 26/10/2014 at 17:16.
Pushed by sayakb into branch 'master'.
Reorganize ACL save logic. Add some negative tests for ACL
M +19 -21 app/lib/components/Access.php
M +23 -0 app/tests/helpers/TestHelper.php
M +102 -7 app/tests/steps/PermissionTest.php
http://commits.kde.org/websites/sso-kde-org/bbad317a18eb1450d544c5c076de6bf6c21064d0
diff --git a/app/lib/components/Access.php b/app/lib/components/Access.php
index 99d633f..7f6c76f 100755
--- a/app/lib/components/Access.php
+++ b/app/lib/components/Access.php
@@ -447,26 +447,6 @@ class Access {
'flag' => 'required|exists:acl_flags,name',
);
- // Set field to 0 for non-field permissions
- if ( ! str_contains($entry->flag, 'field'))
- {
- $entry->field = 0;
- }
-
- // Set field to 0 and object to 'all' for manage permissions
- if (str_contains($entry->flag, 'manage'))
- {
- $entry->field = 0;
- $entry->object_id = 0;
- $entry->object_type = ACLTypes::ALL;
- }
-
- // Determine the field rules
- if ($entry->field != 0)
- {
- $rules['field'] = 'required|exists:fields,id';
- }
-
// Determine the subject lookup rules
if (isset($entry->subject_type))
{
@@ -493,8 +473,15 @@ class Access {
// Based on the flag, determine whether we need the object
if (isset($entry->flag))
{
+ // Set object to 'all' for manage permissions
+ if (str_contains($entry->flag, 'manage'))
+ {
+ $entry->object_id = 0;
+ $entry->object_type = ACLTypes::ALL;
+ }
+
// We need the object_type for all view/edit permissions
- if (str_contains($entry->flag, 'edit') || str_contains($entry->flag, 'view'))
+ else if (str_contains($entry->flag, 'edit') || str_contains($entry->flag, 'view'))
{
$rules['object_type'] = 'required|exists:acl_types,id';
@@ -517,6 +504,17 @@ class Access {
}
}
}
+
+ // Determine whether field is required. If not, we set the
+ // value to 0
+ if (str_contains($entry->flag, 'field') && ! str_contains($entry->flag, 'manage'))
+ {
+ $rules['field'] = 'required|exists:fields,id';
+ }
+ else
+ {
+ $entry->field = 0;
+ }
}
// Create the validator
diff --git a/app/tests/helpers/TestHelper.php b/app/tests/helpers/TestHelper.php
index 624cba7..fbf5169 100755
--- a/app/tests/helpers/TestHelper.php
+++ b/app/tests/helpers/TestHelper.php
@@ -197,6 +197,29 @@ class TestHelper {
return $token;
}
+ /**
+ * Creates an ACL entry in the test database
+ *
+ * @static
+ * @access public
+ * @param int $subjectType
+ * @param int $subjectId
+ * @return ACL
+ */
+ public static function createPermission($subjectType, $subjectId)
+ {
+ $acl = ACL::create(array(
+ 'flag' => 'user_manage',
+ 'subject_type' => $subjectType,
+ 'subject_id' => $subjectId,
+ 'object_type' => ACLTypes::ALL,
+ 'object_id' => 0,
+ 'field_id' => 0,
+ ));
+
+ return $acl;
+ }
+
}
?>
diff --git a/app/tests/steps/PermissionTest.php b/app/tests/steps/PermissionTest.php
index 9c98b77..ceb7917 100755
--- a/app/tests/steps/PermissionTest.php
+++ b/app/tests/steps/PermissionTest.php
@@ -91,22 +91,22 @@ class PermissionTest extends KeychainTestCase {
{
$admin = TestHelper::createUser(UserStatus::ACTIVE, true)->user;
$user = TestHelper::createUser(UserStatus::ACTIVE)->user;
- $subject = TestHelper::createGroup(GroupTypes::OPEN, $user)->group;
- $object = TestHelper::createGroup()->group;
+ $groupSubject = TestHelper::createGroup(GroupTypes::OPEN, $user)->group;
+ $groupObject = TestHelper::createGroup()->group;
$this->be($admin);
$this->call('POST', 'permission/index', array(
'subject_type' => ACLTypes::GROUP,
- 'subject_id' => $subject->id,
+ 'subject_id' => $groupSubject->id,
'object_type' => ACLTypes::GROUP,
- 'object_id' => $object->id,
+ 'object_id' => $groupObject->id,
'flag' => ACLFlags::GROUP_EDIT,
));
$this->be($user);
$this->assertSessionHas('messages.success');
- $this->assertTrue(Access::check(ACLFlags::GROUP_EDIT, $object));
+ $this->assertTrue(Access::check(ACLFlags::GROUP_EDIT, $groupObject));
}
/**
@@ -120,7 +120,7 @@ class PermissionTest extends KeychainTestCase {
{
$admin = TestHelper::createUser(UserStatus::ACTIVE, true)->user;
$user = TestHelper::createUser(UserStatus::ACTIVE)->user;
- $object = TestHelper::createGroup(GroupTypes::OPEN, $admin)->group;
+ $group = TestHelper::createGroup(GroupTypes::OPEN, $admin)->group;
$field = TestHelper::createField();
$this->be($admin);
@@ -129,7 +129,7 @@ class PermissionTest extends KeychainTestCase {
'subject_type' => ACLTypes::USER,
'subject_id' => $user->id,
'object_type' => ACLTypes::GROUP,
- 'object_id' => $object->id,
+ 'object_id' => $group->id,
'flag' => ACLFlags::FIELD_EDIT,
'field' => $field->id,
));
@@ -140,6 +140,101 @@ class PermissionTest extends KeychainTestCase {
}
/**
+ * Tests the postIndex method of the controller with an invalid subject_type
+ *
+ * @access public
+ * @return void
+ */
+ public function testPostIndexInvalidSubject()
+ {
+ $admin = TestHelper::createUser(UserStatus::ACTIVE, true)->user;
+ $user = TestHelper::createUser(UserStatus::ACTIVE)->user;
+
+ $this->be($admin);
+
+ $this->call('POST', 'permission/index', array(
+ 'subject_type' => 0,
+ 'flag' => ACLFlags::USER_MANAGE,
+ ));
+
+ $this->assertSessionHas('messages.error', Lang::get('permission.invalid_subject'));
+ }
+
+ /**
+ * Tests the postIndex method of the controller with an invalid object_type
+ *
+ * @access public
+ * @return void
+ */
+ public function testPostIndexMissingObject()
+ {
+ $admin = TestHelper::createUser(UserStatus::ACTIVE, true)->user;
+ $user = TestHelper::createUser(UserStatus::ACTIVE)->user;
+
+ $this->be($admin);
+
+ $this->call('POST', 'permission/index', array(
+ 'subject_type' => ACLTypes::USER,
+ 'subject_id' => $user->id,
+ 'flag' => ACLFlags::USER_EDIT,
+ ));
+
+ $this->assertSessionHas('messages.error');
+ }
+
+ /**
+ * Tests the postIndex method of the controller with an invalid field
+ *
+ * @access public
+ * @return void
+ */
+ public function testPostIndexMissingField()
+ {
+ $admin = TestHelper::createUser(UserStatus::ACTIVE, true)->user;
+ $user = TestHelper::createUser(UserStatus::ACTIVE)->user;
+ $group = TestHelper::createGroup(GroupTypes::OPEN, $admin)->group;
+
+ $this->be($admin);
+
+ $this->call('POST', 'permission/index', array(
+ 'subject_type' => ACLTypes::USER,
+ 'subject_id' => $user->id,
+ 'object_type' => ACLTypes::GROUP,
+ 'object_id' => $group->id,
+ 'flag' => ACLFlags::FIELD_EDIT,
+ ));
+
+ $this->assertSessionHas('messages.error');
+ }
+
+ /**
+ * Tests the postIndex method of the controller with a duplicate
+ * entry
+ *
+ * @access public
+ * @return void
+ */
+ public function testPostIndexDuplicate()
+ {
+ $admin = TestHelper::createUser(UserStatus::ACTIVE, true)->user;
+ $user = TestHelper::createUser(UserStatus::ACTIVE)->user;
+ $acl = TestHelper::createPermission(ACLTypes::USER, $user->id);
+
+ $this->be($admin);
+
+ $this->call('POST', 'permission/index', array(
+ 'subject_type' => ACLTypes::USER,
+ 'subject_id' => $user->id,
+ 'flag' => ACLFlags::USER_MANAGE,
+ ));
+
+ $this->be($user);
+ $this->assertSessionHas('messages.success');
+ $this->assertTrue(Access::check(ACLFlags::USER_MANAGE));
+ $this->assertEquals(1, ACL::where('subject_id', $user->id)->count());
+ }
+
+ /**
* Tests the getRemove method of the controller
*
* @access public
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic