Skip to content

Commit

Permalink
Merge pull request #189 from owncloud/fix-encryption
Browse files Browse the repository at this point in the history
Fix encryption issues after the impersonator logs out
  • Loading branch information
JammingBen authored Nov 26, 2021
2 parents ae3b5ef + 55af37b commit cd28bf0
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 38 deletions.
1 change: 1 addition & 0 deletions appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ As a security measure, the application lets ownCloud administrators restrict the
<licence>AGPL</licence>
<author>Jörn Friedrich Dreyer, Sujith Haridasan</author>
<version>0.5.0</version>
<namespace>Impersonate</namespace>
<documentation>
<admin>https://doc.owncloud.org/server/latest/admin_manual/issues/impersonate_users.html</admin>
</documentation>
Expand Down
47 changes: 38 additions & 9 deletions lib/Util.php
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<?php
/**
* @author Sujith Haridasan <[email protected]>
* @author Jannik Stehle <[email protected]>
*
* @copyright Copyright (c) 2017, ownCloud GmbH
* @copyright Copyright (c) 2021, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
Expand Down Expand Up @@ -58,23 +59,51 @@ public function __construct(
}

/**
* Switch from admin/subAdmin $impersonator to $user
* Switch from admin/subAdmin $impersonator to $user (and vice versa).
* $impersonator is empty in case the impersonator switches back to their profile.
* The impersonator must be given via the $user attribute then.
*
* @param IUser $user
* @param mixed $impersonator
*/
public function switchUser(IUser $user, $impersonator) {
$this->tokenProvider->invalidateToken($this->session->getId());
$this->userSession->setUser($user);
$this->userSession->createSessionToken($this->request, $user->getUID(), $user->getUID());
//Store the session var impersonator with the impersonator value
if (($this->session->get('impersonator') === null) &&
($impersonator !== null)) {
$this->session->set('impersonator', $impersonator);
}

//Remove the session variable impersonator set as it is a logout
if (($impersonator === '') || ($impersonator === null)) {
if ($impersonator !== null) {
// Store the session var impersonator with the impersonator value
if ($this->session->get('impersonator') === null) {
$this->session->set('impersonator', $impersonator);
}

// Save the impersonator's encryption key in another session variable to reset it again later.
// This is neccessary because the "privateKey" var gets removed on logout.
$encryptionInitialized = $this->session->get('encryptionInitialized');
if ($encryptionInitialized !== null) {
$this->session->set('impersonatorEncryptionInitialized', $encryptionInitialized);
}

$privateKey = $this->session->get('privateKey');
if ($privateKey !== null) {
$this->session->set('impersonatorPrivateKey', $privateKey);
}
} else {
// Remove the session variable impersonator set as it is a logout
$this->session->remove('impersonator');

// Reset encryption key for the impersonator.
$impersonatorEncryptionInitialized = $this->session->get('impersonatorEncryptionInitialized');
if ($impersonatorEncryptionInitialized !== null) {
$this->session->set('encryptionInitialized', $impersonatorEncryptionInitialized);
$this->session->remove('impersonatorEncryptionInitialized');
}

$impersonatorPrivateKey = $this->session->get('impersonatorPrivateKey');
if ($impersonatorPrivateKey !== null) {
$this->session->set('privateKey', $impersonatorPrivateKey);
$this->session->remove('impersonatorPrivateKey');
}
}
}
}
90 changes: 61 additions & 29 deletions tests/lib/UtilTest.php
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<?php
/**
* @author Sujith Haridasan <[email protected]>
* @author Jannik Stehle <[email protected]>
*
* @copyright Copyright (c) 2017, ownCloud GmbH
* @copyright Copyright (c) 2021, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
Expand Down Expand Up @@ -30,48 +31,79 @@
use Test\TestCase;

class UtilTest extends TestCase {
/** @var ISession | \PHPUnit\Framework\MockObject\MockObject */
/** @var ISession | \PHPUnit\Framework\MockObject\MockObject */
private $session;
/** @var DefaultTokenProvider | \PHPUnit\Framework\MockObject\MockObject */
/** @var DefaultTokenProvider | \PHPUnit\Framework\MockObject\MockObject */
private $tokenProvider;
/** @var Session | \PHPUnit\Framework\MockObject\MockObject */
/** @var Session | \PHPUnit\Framework\MockObject\MockObject */
private $userSession;
/** @var IRequest | \PHPUnit\Framework\MockObject\MockObject */
/** @var IRequest | \PHPUnit\Framework\MockObject\MockObject */
private $request;
/** @var Util */
/** @var Util */
private $util;
/** @var IUser | \PHPUnit\Framework\MockObject\MockObject */
private $user;

public function setUp(): void {
$this->session = $this->createMock(ISession::class);
$this->tokenProvider = $this->createMock(DefaultTokenProvider::class);
$this->userSession = $this->createMock(Session::class);
$this->request = $this->createMock(IRequest::class);
$this->util = new Util($this->session, $this->userSession, $this->request, $this->tokenProvider);
$this->user = $this->createMock(IUser::class);
parent::setUp();
}

public function impersonator() {
return [
[null],
['admin']
];
public function testSwitchUserWithImpersonator() {
$impersonator = 'admin';
$encryptionInitialized = 1;
$privateKey = 'privateKey';
$this->session
->expects($this->exactly(3))
->method('get')
->withConsecutive(
['impersonator'],
['encryptionInitialized'],
['privateKey'],
)
->willReturn(null, $encryptionInitialized, $privateKey);
$this->session
->expects($this->exactly(3))
->method('set')
->withConsecutive(
['impersonator', $impersonator],
['impersonatorEncryptionInitialized', $encryptionInitialized],
['impersonatorPrivateKey', $privateKey],
);
$this->util->switchUser($this->user, $impersonator);
}

/**
* @dataProvider impersonator
* @param $impersonator
*/
public function testSwitchUser($impersonator) {
$user = $this->createMock(IUser::class);
$this->util = new Util($this->session, $this->userSession, $this->request, $this->tokenProvider);
if ($impersonator !== null) {
$this->session
->method('get')
->willReturn($impersonator);
}
$this->util->switchUser($user, $impersonator);
if ($impersonator === null) {
$this->assertEquals($this->session->exists('impersonator'), false);
} else {
$this->assertEquals($this->session->get('impersonator'), $impersonator);
}
public function testSwitchUserWithoutImpersonator() {
$encryptionInitialized = 1;
$privateKey = 'privateKey';
$this->session
->expects($this->exactly(2))
->method('get')
->withConsecutive(
['impersonatorEncryptionInitialized'],
['impersonatorPrivateKey'],
)
->willReturn($encryptionInitialized, $privateKey);
$this->session
->expects($this->exactly(2))
->method('set')
->withConsecutive(
['encryptionInitialized', $encryptionInitialized],
['privateKey', $privateKey],
);
$this->session
->expects($this->exactly(3))
->method('remove')
->withConsecutive(
['impersonator'],
['impersonatorEncryptionInitialized'],
['impersonatorPrivateKey'],
);
$this->util->switchUser($this->user, null);
}
}

0 comments on commit cd28bf0

Please sign in to comment.