Skip to content

Commit

Permalink
Merge pull request #253 from owncloud/basic_auth_guest_only
Browse files Browse the repository at this point in the history
Add config option to allow basic auth only for guests
  • Loading branch information
jnweiger authored Sep 28, 2022
2 parents f9c6c0a + ad1fcc5 commit 8795d14
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 4 deletions.
2 changes: 1 addition & 1 deletion appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ More information on setup, configuration and migration can be found in the ownCl
<repository type="git">https://github.com/owncloud/openidconnect.git</repository>
<screenshot>https://raw.githubusercontent.com/owncloud/screenshots/master/openidconnect/openidconnect.png</screenshot>
<dependencies>
<owncloud min-version="10" max-version="10"/>
<owncloud min-version="10.4" max-version="10"/>
<php min-version="7.3" />
</dependencies>
<types>
Expand Down
10 changes: 9 additions & 1 deletion lib/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use Jumbojett\OpenIDConnectClientException;
use OC;
use OC\HintException;
use OCA\OpenIdConnect\LoginChecker;
use OCP\AppFramework\App;

class Application extends App {
Expand Down Expand Up @@ -68,13 +69,20 @@ public function boot(): void {
$userSession = $server->getUserSession();
$urlGenerator = $server->getURLGenerator();
$request = $server->getRequest();
$config = $server->getConfig();
$loginChecker = $this->getContainer()->query(LoginChecker::class);

$loginPage = new LoginPageBehaviour($this->logger, $userSession, $urlGenerator, $request);
$loginPage->handleLoginPageBehaviour($openIdConfig);

// Add event listener
$dispatcher = $server->getEventDispatcher();
$eventHandler = new EventHandler($dispatcher, $request, $userSession, $session);
$eventHandler = new EventHandler($dispatcher, $request, $userSession, $session, $loginChecker);
$eventHandler->registerEventHandler();
if ($config->getSystemValue('openid-connect.basic_auth_guest_only', false)) {
// the login hook is currently used only for this feature
$eventHandler->registerLoginHook();
}

// verify the session
$sessionVerifier = new SessionVerifier($this->logger, $session, $userSession, $memCacheFactory, $dispatcher, $client);
Expand Down
17 changes: 16 additions & 1 deletion lib/EventHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
namespace OCA\OpenIdConnect;

use OCA\OpenIdConnect\Sabre\OpenIdSabreAuthBackend;
use OCA\OpenIdConnect\LoginChecker;
use OCP\IRequest;
use OCP\ISession;
use OCP\IUserSession;
Expand All @@ -39,17 +40,21 @@ class EventHandler {
private $userSession;
/** @var ISession */
private $session;
/** @var LoginChecker */
private $loginChecker;

public function __construct(
EventDispatcherInterface $dispatcher,
IRequest $request,
IUserSession $userSession,
ISession $session
ISession $session,
LoginChecker $loginChecker
) {
$this->dispatcher = $dispatcher;
$this->request = $request;
$this->userSession = $userSession;
$this->session = $session;
$this->loginChecker = $loginChecker;
}

public function registerEventHandler(): void {
Expand All @@ -67,6 +72,16 @@ public function registerEventHandler(): void {
});
}

public function registerLoginHook(): void {
$this->dispatcher->addListener('user.afterlogin', function ($event) {
$eventArgs = $event->getArguments();
if (!isset($eventArgs['loginType'])) {
$eventArgs['loginType'] = null; // auth modules (oidc) don't have loginType associated
}
$this->loginChecker->ensurePasswordLoginJustForGuest($eventArgs['loginType'], $eventArgs['uid']); // will throw a LoginException if not guest
});
}

/**
* @return OpenIdSabreAuthBackend
* @throws \OCP\AppFramework\QueryException
Expand Down
48 changes: 48 additions & 0 deletions lib/LoginChecker.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php
/**
* @author Juan Pablo Villafañez Ramos <[email protected]>
*
* @copyright Copyright (c) 2022, ownCloud GmbH
* @license GPL-2.0
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* 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 General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\OpenIdConnect;

use OCP\IL10N;
use OC\Helper\UserTypeHelper;
use OC\User\LoginException;

class LoginChecker {
/** @var UserTypeHelper */
private $userTypeHelper;
/** @var IL10N*/
private $l10n;

public function __construct(UserTypeHelper $userTypeHelper, IL10N $l10n) {
$this->userTypeHelper = $userTypeHelper;
$this->l10n = $l10n;
}

/**
* @param string $uid the uid to check if it's a guest or not
* @throws LoginException if the uid isn't a guest
*/
public function ensurePasswordLoginJustForGuest($loginType, $uid) {
if (!$this->userTypeHelper->isGuestUser($uid) && $loginType === 'password') {
throw new LoginException($this->l10n->t('Only guests are allowed through this authentication mechanism'));
}
}
}
36 changes: 35 additions & 1 deletion tests/unit/EventHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,17 @@
namespace OCA\OpenIdConnect\Tests\Unit;

use OCA\OpenIdConnect\EventHandler;
use OCA\OpenIdConnect\LoginChecker;
use OCP\IRequest;
use OCP\ISession;
use OCP\IUser;
use OCP\IUserSession;
use OCP\SabrePluginEvent;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\DAV\Auth\Plugin;
use Sabre\DAV\Server;
use Symfony\Component\EventDispatcher\Event;
use Symfony\Component\EventDispatcher\GenericEvent;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Test\TestCase;

Expand All @@ -44,16 +47,19 @@ class EventHandlerTest extends TestCase {
* @var MockObject | EventDispatcherInterface
*/
private $dispatcher;
/** @var LoginChecker */
private $loginChecker;

protected function setUp(): void {
parent::setUp();
$this->dispatcher = $this->createMock(EventDispatcherInterface::class);
$request = $this->createMock(IRequest::class);
$session = $this->createMock(ISession::class);
$userSession = $this->createMock(IUserSession::class);
$this->loginChecker = $this->createMock(LoginChecker::class);

$this->eventHandler = $this->getMockBuilder(EventHandler::class)
->setConstructorArgs([$this->dispatcher, $request, $userSession, $session])
->setConstructorArgs([$this->dispatcher, $request, $userSession, $session, $this->loginChecker])
->onlyMethods(['createAuthBackend'])
->getMock();
}
Expand Down Expand Up @@ -94,4 +100,32 @@ public function testListener(): void {
$this->eventHandler->expects(self::once())->method('createAuthBackend');
$this->eventHandler->registerEventHandler();
}

public function testAfterLoginHook(): void {
$this->loginChecker->expects($this->once())
->method('ensurePasswordLoginJustForGuest')
->with('customLoginType', 'myUid');

$user = $this->createMock(IUser::class);
$event = new GenericEvent(null, ['loginType' => 'customLoginType', 'user' => $user, 'uid' => 'myUid', 'password' => 'mypassword']);

$this->dispatcher->method('addListener')->willReturnCallback(static function ($name, $callback) use ($event) {
$callback($event);
});
$this->eventHandler->registerLoginHook();
}

public function testAfterLoginHookNoLoginType(): void {
$this->loginChecker->expects($this->once())
->method('ensurePasswordLoginJustForGuest')
->with(null, 'myUid');

$user = $this->createMock(IUser::class);
$event = new GenericEvent(null, ['user' => $user, 'uid' => 'myUid', 'password' => 'mypassword']);

$this->dispatcher->method('addListener')->willReturnCallback(static function ($name, $callback) use ($event) {
$callback($event);
});
$this->eventHandler->registerLoginHook();
}
}
75 changes: 75 additions & 0 deletions tests/unit/LoginCheckerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php
/**
* @author Juan Pablo Villafañez Ramos <[email protected]>
*
* @copyright Copyright (c) 2022, ownCloud GmbH
* @license GPL-2.0
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* 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 General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OCA\OpenIdConnect\Tests\Unit;

use OCA\OpenIdConnect\LoginChecker;
use OCP\IL10N;
use OC\Helper\UserTypeHelper;
use OC\User\LoginException;
use Test\TestCase;

class LoginCheckerTest extends TestCase {
/** @var UserTypeHelper */
private $userTypeHelper;
/** @var IL10N */
private $l10n;
/** @var LoginChecker */
private $loginChecker;

protected function setUp(): void {
parent::setUp();
$this->userTypeHelper = $this->createMock(UserTypeHelper::class);
$this->l10n = $this->createMock(IL10N::class);

$this->loginChecker = new LoginChecker($this->userTypeHelper, $this->l10n);
}

public function ensurePasswordLoginJustForGuestDataProvider() {
return [
[true, 'password'],
[true, 'token'],
[true, 'apache'],
[true, null],
[false, 'token'],
[false, 'apache'],
[false, 'null'],
];
}

/**
* @dataProvider ensurePasswordLoginJustForGuestDataProvider
*/
public function testEnsurePasswordLoginJustForGuest($isGuest, $loginType) {
$this->userTypeHelper->expects($this->once())
->method('isGuestUser')
->willReturn($isGuest);
$this->assertNull($this->loginChecker->ensurePasswordLoginJustForGuest($loginType, 'myUser'));
}

public function testEnsurePasswordLoginJustForGuestNotGuest() {
$this->expectException(LoginException::class);

$this->userTypeHelper->method('isGuestUser')->willReturn(false);
$this->loginChecker->ensurePasswordLoginJustForGuest('password', 'muUser');
}
}

0 comments on commit 8795d14

Please sign in to comment.