Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ImpersonateEvents for SnappyMail (and others) #180

Merged
merged 5 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions lib/Controller/LogoutController.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
use OCP\ISession;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\EventDispatcher\IEventDispatcher;
use OCA\Impersonate\Events\EndImpersonateEvent;

class LogoutController extends Controller {
/** @var IUserManager */
Expand All @@ -20,6 +22,8 @@ class LogoutController extends Controller {
private $logger;
/** @var ISession */
private $session;
/** @var IEventDispatcher */
private $eventDispatcher;

/**
* @param string $appName
Expand All @@ -34,23 +38,25 @@ public function __construct($appName,
IUserManager $userManager,
IUserSession $userSession,
ISession $session,
LoggerInterface $logger) {
LoggerInterface $logger,
IEventDispatcher $eventDispatcher) {
parent::__construct($appName, $request);
$this->userManager = $userManager;
$this->userSession = $userSession;
$this->session = $session;
$this->logger = $logger;
$this->eventDispatcher = $eventDispatcher;
}

/**
* @UseSession
* @NoAdminRequired
*/
public function logout(string $userId): JSONResponse {
$user = $this->session->get('oldUserId');
$user = $this->userManager->get($user);
$impersonator = $this->session->get('oldUserId');
$impersonator = $this->userManager->get($impersonator);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would recommend different variable names for the uid and the user object

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had the same feeling and almost changed it back then. Well, almost. 👍


if ($user === null) {
if ($impersonator === null) {
return new JSONResponse(
sprintf(
'No user found for %s',
Expand All @@ -60,7 +66,10 @@ public function logout(string $userId): JSONResponse {
);
}

$this->userSession->setUser($user);
$impersonatee = $this->userManager->get($userId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the more clearer distinction on the name of the Getter function on the event.
...tee vs ...tor is visually so similar. $impersonatedUser would be more different.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same gut feeling, 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, actually as i did in the Event class… =D

$this->eventDispatcher->dispatchTyped(new EndImpersonateEvent($impersonator, $impersonatee));

$this->userSession->setUser($impersonator);

$this->logger->info(
sprintf(
Expand Down
48 changes: 23 additions & 25 deletions lib/Controller/SettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
use OCP\IUser;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\EventDispatcher\IEventDispatcher;
use OCA\Impersonate\Events\BeginImpersonateEvent;

class SettingsController extends Controller {
/** @var IUserManager */
Expand All @@ -40,27 +42,19 @@ class SettingsController extends Controller {
private $logger;
/** @var IL10N */
private $l;
/** @var IEventDispatcher */
private $eventDispatcher;

/**
* @param string $appName
* @param IRequest $request
* @param IUserManager $userManager
* @param IGroupManager $groupManager
* @param IUserSession $userSession
* @param ISession $session
* @param IConfig $config
* @param LoggerInterface $logger
* @param IL10N $l
*/
public function __construct($appName,
public function __construct(string $appName,
IRequest $request,
IUserManager $userManager,
IGroupManager $groupManager,
IUserSession $userSession,
ISession $session,
IConfig $config,
LoggerInterface $logger,
IL10N $l) {
IL10N $l,
IEventDispatcher $eventDispatcher) {
parent::__construct($appName, $request);
$this->userManager = $userManager;
$this->groupManager = $groupManager;
Expand All @@ -69,29 +63,30 @@ public function __construct($appName,
$this->config = $config;
$this->logger = $logger;
$this->l = $l;
$this->eventDispatcher = $eventDispatcher;
}

/**
* @UseSession
* @NoAdminRequired
*/
public function impersonate(string $userId): JSONResponse {
/** @var IUser $currentUser */
$currentUser = $this->userSession->getUser();
/** @var IUser $impersonator */
$impersonator = $this->userSession->getUser();

$this->logger->warning(
sprintf(
'User %s trying to impersonate user %s',
$currentUser->getUID(),
$impersonator->getUID(),
$userId
),
[
'app' => 'impersonate',
]
);

$user = $this->userManager->get($userId);
if ($user === null) {
$impersonatee = $this->userManager->get($userId);
if ($impersonatee === null) {
return new JSONResponse(
[
'message' => $this->l->t('User not found'),
Expand All @@ -100,8 +95,8 @@ public function impersonate(string $userId): JSONResponse {
);
}

if (!$this->groupManager->isAdmin($currentUser->getUID())
&& !$this->groupManager->getSubAdmin()->isUserAccessible($currentUser, $user)) {
if (!$this->groupManager->isAdmin($impersonator->getUID())
&& !$this->groupManager->getSubAdmin()->isUserAccessible($impersonator, $impersonatee)) {
return new JSONResponse(
[
'message' => $this->l->t('Insufficient permissions to impersonate user'),
Expand All @@ -112,7 +107,7 @@ public function impersonate(string $userId): JSONResponse {

$authorized = json_decode($this->config->getAppValue('impersonate', 'authorized', '["admin"]'));
if (!empty($authorized)) {
$userGroups = $this->groupManager->getUserGroupIds($currentUser);
$userGroups = $this->groupManager->getUserGroupIds($impersonator);

if (!array_intersect($userGroups, $authorized)) {
return new JSONResponse(
Expand All @@ -124,7 +119,7 @@ public function impersonate(string $userId): JSONResponse {
}
}

if ($user->getLastLogin() === 0) {
if ($impersonatee->getLastLogin() === 0) {
return new JSONResponse(
[
'message' => $this->l->t('Cannot impersonate the user because it was never logged in'),
Expand All @@ -133,7 +128,7 @@ public function impersonate(string $userId): JSONResponse {
);
}

if ($user->getUID() === $currentUser->getUID()) {
if ($impersonatee->getUID() === $impersonator->getUID()) {
return new JSONResponse(
[
'message' => $this->l->t('Cannot impersonate yourself'),
Expand All @@ -152,9 +147,12 @@ public function impersonate(string $userId): JSONResponse {
]
);
if ($this->session->get('oldUserId') === null) {
$this->session->set('oldUserId', $currentUser->getUID());
$this->session->set('oldUserId', $impersonator->getUID());
}
$this->userSession->setUser($user);

$this->eventDispatcher->dispatchTyped(new BeginImpersonateEvent($impersonator, $impersonatee));

$this->userSession->setUser($impersonatee);
return new JSONResponse();
}
}
51 changes: 51 additions & 0 deletions lib/Events/AImpersonateEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2022 SnappyMail
*
* @author Arthur Schiwon <blizzz@arthur-schiwon.de>
* @author the-djmaze
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
*/

namespace OCA\Impersonate\Events;

use OCP\EventDispatcher\Event;
use OCP\IUser;

abstract class AImpersonateEvent extends Event {
private IUser $impersonator;

private IUser $impersonatee;

public function __construct(IUser $impersonator, IUser $impersonatee) {
parent::__construct();
$this->impersonator = $impersonator;
$this->impersonatee = $impersonatee;
}

public function getImpersonator(): IUser {
return $this->impersonator;
}

public function getImpersonatedUser(): IUser {
return $this->impersonatee;
}
}
30 changes: 30 additions & 0 deletions lib/Events/BeginImpersonateEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2022 Arthur Schiwon <blizzz@arthur-schiwon.de>
*
* @author Arthur Schiwon <blizzz@arthur-schiwon.de>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
*/

namespace OCA\Impersonate\Events;

class BeginImpersonateEvent extends AImpersonateEvent {
}
30 changes: 30 additions & 0 deletions lib/Events/EndImpersonateEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2022 Arthur Schiwon <blizzz@arthur-schiwon.de>
*
* @author Arthur Schiwon <blizzz@arthur-schiwon.de>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
*/

namespace OCA\Impersonate\Events;

class EndImpersonateEvent extends AImpersonateEvent {
}
14 changes: 0 additions & 14 deletions tests/phpunit.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<?xml version="1.0" encoding="utf-8" ?>
<phpunit bootstrap="unit/bootstrap.php"
strict="true"
verbose="true"
convertDeprecationsToExceptions="true"
timeoutForSmallTests="900"
Expand All @@ -10,18 +9,5 @@
<testsuite name='Nextcloud - Impersonate App Tests'>
<directory suffix='Test.php'>.</directory>
</testsuite>
<!-- filters for code coverage -->
<filter>
<whitelist>
<directory suffix=".php">../../impersonate</directory>
<exclude>
<directory suffix=".php">../../impersonate/tests</directory>
</exclude>
</whitelist>
</filter>
<logging>
<!-- and this is where your report will be written -->
<log type="coverage-clover" target="./clover.xml"/>
</logging>
</phpunit>

Loading