diff --git a/protected/humhub/components/access/ControllerAccess.php b/protected/humhub/components/access/ControllerAccess.php index ed17cc4423..b0efd6faf6 100644 --- a/protected/humhub/components/access/ControllerAccess.php +++ b/protected/humhub/components/access/ControllerAccess.php @@ -424,7 +424,7 @@ class ControllerAccess extends BaseObject */ public function validatePostRequest() { - return Yii::$app->request->method == 'POST'; + return Yii::$app->request->isPost; } /** diff --git a/protected/humhub/docs/CHANGELOG.md b/protected/humhub/docs/CHANGELOG.md index a13b829e3c..a522b703b1 100644 --- a/protected/humhub/docs/CHANGELOG.md +++ b/protected/humhub/docs/CHANGELOG.md @@ -9,6 +9,8 @@ HumHub Change Log - Fix: space archive activity wrong originator assignment - Fix: suppress "unable to determine dataType" error for aborted xhr requests - Enh: added `FunctionalTester::loginBySpaceUserGroup()` and `FunctionalTest::assertSpaceAccessStatus()` for ACL testing +- Fix #2721 delete space button not visible for system admin +- Enh: added `humhub\modules\space\behaviors\SpaceModelMembership::canDelete()` 1.3.0-beta.2 (July 18, 2018) ----------------------------- diff --git a/protected/humhub/modules/space/behaviors/SpaceModelMembership.php b/protected/humhub/modules/space/behaviors/SpaceModelMembership.php index 3f61f5644f..1d548c3206 100644 --- a/protected/humhub/modules/space/behaviors/SpaceModelMembership.php +++ b/protected/humhub/modules/space/behaviors/SpaceModelMembership.php @@ -157,6 +157,15 @@ class SpaceModelMembership extends Behavior return $this->_spaceOwner; } + /** + * @return bool checks if the current user is allowed to delete this space + * @since 1.3 + */ + public function canDelete() + { + return Yii::$app->user->isAdmin() || $this->isSpaceOwner(); + } + /** * Is given User owner of this Space * @param User|int|null $userId diff --git a/protected/humhub/modules/space/modules/manage/controllers/DefaultController.php b/protected/humhub/modules/space/modules/manage/controllers/DefaultController.php index 737dd4a637..1bb29b4908 100644 --- a/protected/humhub/modules/space/modules/manage/controllers/DefaultController.php +++ b/protected/humhub/modules/space/modules/manage/controllers/DefaultController.php @@ -8,6 +8,7 @@ namespace humhub\modules\space\modules\manage\controllers; +use humhub\modules\content\components\ContentContainerControllerAccess; use humhub\modules\space\components\UrlRule; use Yii; use humhub\modules\space\models\Space; @@ -34,10 +35,8 @@ class DefaultController extends Controller public function getAccessRules() { $result = parent::getAccessRules(); - $result[] = [ - 'userGroup' => [Space::USERGROUP_OWNER], 'actions' => ['archive', 'unarchive', 'delete'] - ]; - + $result[] = [ContentContainerControllerAccess::RULE_USER_GROUP_ONLY => [Space::USERGROUP_OWNER], 'actions' => ['archive', 'unarchive', 'delete']]; + $result[] = [ContentContainerControllerAccess::RULE_POST => ['archive', 'unarchive']]; return $result; } @@ -90,15 +89,10 @@ class DefaultController extends Controller // Create Activity when the space in archieved SpaceArchieved::instance()->from(Yii::$app->user->getIdentity())->about($space->owner)->save(); - if (Yii::$app->request->isAjax) { - Yii::$app->response->format = 'json'; - return [ - 'success' => true, - 'space' => Chooser::getSpaceResult($space, true, ['isMember' => true]) - ]; - } - - return $this->redirect($space->createUrl('/space/manage')); + return $this->asJson( [ + 'success' => true, + 'space' => Chooser::getSpaceResult($space, true, ['isMember' => true]) + ]); } /** @@ -136,5 +130,4 @@ class DefaultController extends Controller return $this->render('delete', ['model' => $model, 'space' => $this->getSpace()]); } - } diff --git a/protected/humhub/modules/space/modules/manage/controllers/MemberController.php b/protected/humhub/modules/space/modules/manage/controllers/MemberController.php index 2207eec525..ccefc99302 100644 --- a/protected/humhub/modules/space/modules/manage/controllers/MemberController.php +++ b/protected/humhub/modules/space/modules/manage/controllers/MemberController.php @@ -31,9 +31,7 @@ class MemberController extends Controller public function getAccessRules() { $result = parent::getAccessRules(); - $result[] = [ - 'userGroup' => [Space::USERGROUP_OWNER], 'actions' => ['change-owner'] - ]; + $result[] = ['userGroup' => [Space::USERGROUP_OWNER], 'actions' => ['change-owner']]; return $result; } @@ -186,7 +184,6 @@ class MemberController extends Controller if ($model->load(Yii::$app->request->post()) && $model->validate()) { $space->setSpaceOwner($model->ownerId); - return $this->redirect($space->getUrl()); } diff --git a/protected/humhub/modules/space/modules/manage/models/ChangeOwnerForm.php b/protected/humhub/modules/space/modules/manage/models/ChangeOwnerForm.php index 949d99a87a..32c457e4ed 100644 --- a/protected/humhub/modules/space/modules/manage/models/ChangeOwnerForm.php +++ b/protected/humhub/modules/space/modules/manage/models/ChangeOwnerForm.php @@ -8,6 +8,7 @@ namespace humhub\modules\space\modules\manage\models; +use humhub\modules\space\models\Space; use Yii; use yii\base\Model; use humhub\modules\space\models\Membership; @@ -60,7 +61,7 @@ class ChangeOwnerForm extends Model { $possibleOwners = []; - $query = Membership::find()->joinWith(['user', 'user.profile'])->andWhere(['space_membership.group_id' => 'admin', 'space_membership.space_id' => $this->space->id]); + $query = Membership::find()->joinWith(['user', 'user.profile'])->andWhere(['space_membership.group_id' => Space::USERGROUP_ADMIN, 'space_membership.space_id' => $this->space->id]); foreach ($query->all() as $membership) { $possibleOwners[$membership->user->id] = $membership->user->displayName; } diff --git a/protected/humhub/modules/space/modules/manage/views/default/delete.php b/protected/humhub/modules/space/modules/manage/views/default/delete.php index f40a948213..9bbd0e45b1 100644 --- a/protected/humhub/modules/space/modules/manage/views/default/delete.php +++ b/protected/humhub/modules/space/modules/manage/views/default/delete.php @@ -1,8 +1,9 @@
@@ -17,18 +18,14 @@ use yii\helpers\Html;


- + -
- labelEx($model, 'currentPassword'); ?> - passwordField($model, 'currentPassword', ['class' => 'form-control', 'rows' => '6']); ?> - error($model, 'currentPassword'); ?> -
+ field($model, 'currentPassword')->passwordInput(); ?> -
+
- 'btn btn-danger', 'data-ui-loader' => '']); ?> + submit() ?> - +
diff --git a/protected/humhub/modules/space/modules/manage/views/default/index.php b/protected/humhub/modules/space/modules/manage/views/default/index.php index 459e38ea73..4aaeea6635 100644 --- a/protected/humhub/modules/space/modules/manage/views/default/index.php +++ b/protected/humhub/modules/space/modules/manage/views/default/index.php @@ -5,6 +5,7 @@ use humhub\modules\space\widgets\SpaceNameColorInput; use humhub\widgets\DataSaved; use yii\bootstrap\ActiveForm; use yii\helpers\Html; +use humhub\widgets\Button; ?>
@@ -30,11 +31,7 @@ use yii\helpers\Html; -
- isSpaceOwner()) : ?> - createUrl('delete'), ['class' => 'btn btn-danger', 'data-post' => 'POST']); ?> - -
+ right()->link($model->createUrl('delete'))->visible($model->canDelete()) ?>
diff --git a/protected/humhub/modules/space/modules/manage/views/member/change-owner.php b/protected/humhub/modules/space/modules/manage/views/member/change-owner.php index 51e9c037b1..51c3e77e19 100644 --- a/protected/humhub/modules/space/modules/manage/views/member/change-owner.php +++ b/protected/humhub/modules/space/modules/manage/views/member/change-owner.php @@ -1,10 +1,15 @@ +
Manage members'); ?> @@ -15,11 +20,12 @@ use yii\widgets\ActiveForm;

- field($model, 'ownerId')->dropDownList($model->getNewOwnerArray()) ?> -
+ field($model, 'ownerId')->dropDownList($model->getNewOwnerArray()) ?> - 'btn btn-danger', 'data-confirm' => 'Are you really sure?']) ?> +
+ + action('client.submit')->confirm() ?> diff --git a/protected/humhub/modules/space/tests/codeception/functional/ArchiveCest.php b/protected/humhub/modules/space/tests/codeception/functional/ArchiveCest.php new file mode 100644 index 0000000000..0170f453aa --- /dev/null +++ b/protected/humhub/modules/space/tests/codeception/functional/ArchiveCest.php @@ -0,0 +1,34 @@ +assertSpaceAccessFalse(Space::USERGROUP_MEMBER, '/space/manage/default/archive'); + $I->assertSpaceAccessFalse(Space::USERGROUP_USER, '/space/manage/default/archive'); + $I->assertSpaceAccessFalse(Space::USERGROUP_MODERATOR, '/space/manage/default/archive'); + $I->assertSpaceAccessFalse(Space::USERGROUP_ADMIN, '/space/manage/default/archive'); + $I->assertSpaceAccessFalse(Space::USERGROUP_OWNER, '/space/manage/default/archive'); + $I->assertSpaceAccessTrue(Space::USERGROUP_OWNER, '/space/manage/default/archive', true); + } + + public function testSpaceArchiveSpace(FunctionalTester $I) + { + $space = $I->loginBySpaceUserGroup(Space::USERGROUP_OWNER); + $I->amOnSpace($space, '/space/manage/default/archive', true); + $I->amOnSpace($space); + $I->see('Archived'); + } +} \ No newline at end of file diff --git a/protected/humhub/modules/space/tests/codeception/functional/DeleteSpaceCest.php b/protected/humhub/modules/space/tests/codeception/functional/DeleteSpaceCest.php index 8c291c76c4..96cbdd8470 100644 --- a/protected/humhub/modules/space/tests/codeception/functional/DeleteSpaceCest.php +++ b/protected/humhub/modules/space/tests/codeception/functional/DeleteSpaceCest.php @@ -8,60 +8,25 @@ namespace enterprise\acceptance\modules\emailwhitelist; -use Yii; use humhub\modules\space\models\Space; use FunctionalTester; class DeleteSpaceCest { - - public function testOwnerDeletion(FunctionalTester $I) + public function testSpaceDeleteAccess(FunctionalTester $I) { - $I->wantTo('ensure the owner of the space is able to delete the space'); - $I->amUser(); - $space = $this->createSpace(); - $I->amOnRoute('/space/manage/default/delete', ['sguid' => $space->guid]); - $I->canSeeResponseCodeIs(200); - } - - public function testMemberDeletion(FunctionalTester $I) - { - $I->wantTo('ensure a member of the space is not able to delete the space'); - $I->amUser1(); - // User1 is member of Space3 - $I->amOnRoute('/space/manage/default/delete', ['sguid' =>'5396d499-20d6-4233-800b-c6c86e5fa34c']); - $I->canSeeResponseCodeIs(403); + $I->assertSpaceAccessFalse(Space::USERGROUP_MEMBER, '/space/manage/default/delete'); + $I->assertSpaceAccessFalse(Space::USERGROUP_USER, '/space/manage/default/delete'); + $I->assertSpaceAccessFalse(Space::USERGROUP_MODERATOR, '/space/manage/default/delete'); + $I->assertSpaceAccessFalse(Space::USERGROUP_ADMIN, '/space/manage/default/delete'); + $I->assertSpaceAccessTrue(Space::USERGROUP_OWNER, '/space/manage/default/delete'); + $space = $I->assertSpaceAccessStatus(Space::USERGROUP_OWNER, 302, '/space/manage/default/delete', [], ['DeleteForm[currentPassword]' => '123qwe']); + $I->amOnSpace($space); + $I->seeResponseCodeIs(404); } public function testSystemAdminDeletion(FunctionalTester $I) { - $I->wantTo('ensure a system admin is able to delete the space'); - $I->amAdmin(); - // User1 is member of Space3 - $I->amOnRoute('/space/manage/default/delete', ['sguid' =>'5396d499-20d6-4233-800b-c6c86e5fa34c']); - $I->canSeeResponseCodeIs(200); + $I->assertSpaceAccessTrue('root', '/space/manage/default/delete', [], ['DeleteForm[currentPassword]' => 'test']); } - - public function testAdminDeletion(FunctionalTester $I) - { - $I->wantTo('ensure a simple space admin is not able to delete the space'); - $I->amUser1(); - // User1 is admin of Space4 - $I->amOnRoute('/space/manage/default/delete', ['sguid' =>'5396d499-20d6-4233-800b-c6c86e5fa34d']); - $I->canSeeResponseCodeIs(403); - } - - private function createSpace() - { - $space = new Space([ - 'name' => 'DeleteSpaceTest' - ]); - - $space->created_by = Yii::$app->user->getId(); - $space->save(); - - return $space; - } - - } diff --git a/protected/humhub/modules/space/tests/codeception/functional/ManageAccessCest.php b/protected/humhub/modules/space/tests/codeception/functional/ManageAccessCest.php new file mode 100644 index 0000000000..0a47ea8762 --- /dev/null +++ b/protected/humhub/modules/space/tests/codeception/functional/ManageAccessCest.php @@ -0,0 +1,25 @@ +assertSpaceAccessFalse(Space::USERGROUP_MEMBER, '/space/manage'); + $I->assertSpaceAccessFalse(Space::USERGROUP_USER, '/space/manage'); + $I->assertSpaceAccessFalse(Space::USERGROUP_MODERATOR, '/space/manage'); + $I->assertSpaceAccessTrue(Space::USERGROUP_ADMIN, '/space/manage'); + $I->assertSpaceAccessTrue(Space::USERGROUP_OWNER, '/space/manage'); + } +} \ No newline at end of file diff --git a/protected/humhub/modules/space/tests/codeception/functional/ManageMembersCest.php b/protected/humhub/modules/space/tests/codeception/functional/ManageMembersCest.php new file mode 100644 index 0000000000..ef46def06c --- /dev/null +++ b/protected/humhub/modules/space/tests/codeception/functional/ManageMembersCest.php @@ -0,0 +1,46 @@ +assertSpaceAccessFalse(Space::USERGROUP_MEMBER, '/space/manage/member'); + $I->assertSpaceAccessFalse(Space::USERGROUP_USER, '/space/manage/member'); + $I->assertSpaceAccessFalse(Space::USERGROUP_MODERATOR, '/space/manage/member'); + $I->assertSpaceAccessTrue(Space::USERGROUP_ADMIN, '/space/manage/member'); + $I->assertSpaceAccessTrue(Space::USERGROUP_OWNER, '/space/manage/member'); + } + + public function testChangeOwnerAccess(FunctionalTester $I) + { + $I->assertSpaceAccessFalse(Space::USERGROUP_MEMBER, '/space/manage/member/change-owner'); + $I->assertSpaceAccessFalse(Space::USERGROUP_USER, '/space/manage/member/change-owner'); + $I->assertSpaceAccessFalse(Space::USERGROUP_MODERATOR, '/space/manage/member/change-owner'); + $I->assertSpaceAccessFalse(Space::USERGROUP_ADMIN, '/space/manage/member/change-owner'); + $I->assertSpaceAccessTrue(Space::USERGROUP_OWNER, '/space/manage/member/change-owner'); + + + $I->amAdmin(); + $I->amOnSpace4('/space/manage/member/change-owner', [], ['ChangeOwnerForm[ownerId]' => 2]); + $I->seeSuccessResponseCode(); + + $space = Space::findOne(4); + + if(!$space->ownerUser->id === 2) { + $I->see('Change owner did not work'); + } + + } +} \ No newline at end of file