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

Move public webdav auth to share manager #23736

Merged
merged 3 commits into from
Apr 8, 2016
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 8 additions & 5 deletions apps/dav/appinfo/v1/publicwebdav.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@
OC_Util::obEnd();

// Backends
$authBackend = new OCA\DAV\Connector\PublicAuth(\OC::$server->getConfig(), \OC::$server->getRequest());
$authBackend = new OCA\DAV\Connector\PublicAuth(
\OC::$server->getRequest(),
\OC::$server->getShareManager(),
\OC::$server->getSession()
);

$serverFactory = new OCA\DAV\Connector\Sabre\ServerFactory(
\OC::$server->getConfig(),
Expand All @@ -56,10 +60,9 @@
}

$share = $authBackend->getShare();
$rootShare = \OCP\Share::resolveReShare($share);
$owner = $rootShare['uid_owner'];
$isWritable = $share['permissions'] & (\OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_CREATE);
$fileId = $share['file_source'];
$owner = $share->getShareOwner();
$isWritable = $share->getPermissions() & (\OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_CREATE);
$fileId = $share->getNodeId();

if (!$isWritable) {
\OC\Files\Filesystem::addStorageWrapper('readonly', function ($mountPoint, $storage) {
Expand Down
81 changes: 36 additions & 45 deletions apps/dav/lib/connector/publicauth.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,31 +26,41 @@

namespace OCA\DAV\Connector;

use OCP\IConfig;
use OCP\IRequest;
use OCP\ISession;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager;

/**
* Class PublicAuth
*
* @package OCA\DAV\Connector
*/
class PublicAuth extends \Sabre\DAV\Auth\Backend\AbstractBasic {

/**
* @var \OCP\IConfig
*/
private $config;

/** @var \OCP\Share\IShare */
private $share;

/**
* @var IRequest
*/
/** @var IManager */
private $shareManager;

/** @var ISession */
private $session;

/** @var IRequest */
private $request;

/**
* @param \OCP\IConfig $config
* @param IRequest $request
* @param IManager $shareManager
* @param ISession $session
*/
public function __construct(IConfig $config,
IRequest $request) {
$this->config = $config;
public function __construct(IRequest $request,
IManager $shareManager,
ISession $session) {
$this->request = $request;
$this->shareManager = $shareManager;
$this->session = $session;
}

/**
Expand All @@ -66,42 +76,23 @@ public function __construct(IConfig $config,
* @throws \Sabre\DAV\Exception\NotAuthenticated
*/
protected function validateUserPass($username, $password) {
$linkItem = \OCP\Share::getShareByToken($username, false);
\OC_User::setIncognitoMode(true);
$this->share = $linkItem;
if (!$linkItem) {
try {
$share = $this->shareManager->getShareByToken($username);
} catch (ShareNotFound $e) {
return false;
}

if ((int)$linkItem['share_type'] === \OCP\Share::SHARE_TYPE_LINK &&
$this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes') !== 'yes') {
$this->share['permissions'] &= ~(\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);
}
$this->share = $share;

// check if the share is password protected
if (isset($linkItem['share_with'])) {
if ($linkItem['share_type'] == \OCP\Share::SHARE_TYPE_LINK) {
// Check Password
$newHash = '';
if(\OC::$server->getHasher()->verify($password, $linkItem['share_with'], $newHash)) {
/**
* FIXME: Migrate old hashes to new hash format
* Due to the fact that there is no reasonable functionality to update the password
* of an existing share no migration is yet performed there.
* The only possibility is to update the existing share which will result in a new
* share ID and is a major hack.
*
* In the future the migration should be performed once there is a proper method
* to update the share's password. (for example `$share->updatePassword($password)`
*
* @link https://github.com/owncloud/core/issues/10671
*/
if(!empty($newHash)) {
\OC_User::setIncognitoMode(true);

}
// check if the share is password protected
if ($share->getPassword() !== null) {
if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
if ($this->shareManager->checkPassword($share, $password)) {
return true;
} else if (\OC::$server->getSession()->exists('public_link_authenticated')
&& \OC::$server->getSession()->get('public_link_authenticated') === $linkItem['id']) {
} else if ($this->session->exists('public_link_authenticated')
&& $this->session->get('public_link_authenticated') === $share->getId()) {
return true;
} else {
if (in_array('XMLHttpRequest', explode(',', $this->request->getHeader('X-Requested-With')))) {
Expand All @@ -112,7 +103,7 @@ protected function validateUserPass($username, $password) {
}
return false;
}
} else if ($linkItem['share_type'] == \OCP\Share::SHARE_TYPE_REMOTE) {
} else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_REMOTE) {
return true;
} else {
return false;
Expand All @@ -123,7 +114,7 @@ protected function validateUserPass($username, $password) {
}

/**
* @return array
* @return \OCP\Share\IShare
*/
public function getShare() {
return $this->share;
Expand Down
170 changes: 170 additions & 0 deletions apps/dav/tests/unit/connector/publicauth.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
<?php

namespace OCA\DAV\Tests\Unit\Connector;

use OCP\IRequest;
use OCP\ISession;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager;

class PublicAuth extends \Test\TestCase {

/** @var ISession|\PHPUnit_Framework_MockObject_MockObject */
private $session;
/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
private $request;
/** @var IManager|\PHPUnit_Framework_MockObject_MockObject */
private $shareManager;
/** @var \OCA\DAV\Connector\PublicAuth */
private $auth;

/** @var string */
private $oldUser;

protected function setUp() {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing parent::setUp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

parent::setUp();

$this->session = $this->getMock('\OCP\ISession');
$this->request = $this->getMock('\OCP\IRequest');
$this->shareManager = $this->getMock('\OCP\Share\IManager');

$this->auth = new \OCA\DAV\Connector\PublicAuth(
$this->request,
$this->shareManager,
$this->session
);

// Store current user
$this->oldUser = \OC_User::getUser();
}

protected function tearDown() {
\OC_User::setIncognitoMode(false);

// Set old user
\OC_User::setUserId($this->oldUser);
\OC_Util::setupFS($this->oldUser);

parent::tearDown();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing parent::tearDown

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


public function testNoShare() {
$this->shareManager->expects($this->once())
->method('getShareByToken')
->willThrowException(new ShareNotFound());

$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);

$this->assertFalse($result);
}

public function testShareNoPassword() {
$share = $this->getMock('OCP\Share\IShare');
$share->method('getPassword')->willReturn(null);

$this->shareManager->expects($this->once())
->method('getShareByToken')
->willReturn($share);

$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);

$this->assertTrue($result);
}

public function testSharePasswordFancyShareType() {
$share = $this->getMock('OCP\Share\IShare');
$share->method('getPassword')->willReturn('password');
$share->method('getShareType')->willReturn(42);

$this->shareManager->expects($this->once())
->method('getShareByToken')
->willReturn($share);

$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);

$this->assertFalse($result);
}


public function testSharePasswordRemote() {
$share = $this->getMock('OCP\Share\IShare');
$share->method('getPassword')->willReturn('password');
$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_REMOTE);

$this->shareManager->expects($this->once())
->method('getShareByToken')
->willReturn($share);

$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);

$this->assertTrue($result);
}

public function testSharePasswordLinkValidPassword() {
$share = $this->getMock('OCP\Share\IShare');
$share->method('getPassword')->willReturn('password');
$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);

$this->shareManager->expects($this->once())
->method('getShareByToken')
->willReturn($share);

$this->shareManager->expects($this->once())
->method('checkPassword')->with(
$this->equalTo($share),
$this->equalTo('password')
)->willReturn(true);

$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);

$this->assertTrue($result);
}

public function testSharePasswordLinkValidSession() {
$share = $this->getMock('OCP\Share\IShare');
$share->method('getPassword')->willReturn('password');
$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);
$share->method('getId')->willReturn('42');

$this->shareManager->expects($this->once())
->method('getShareByToken')
->willReturn($share);

$this->shareManager->method('checkPassword')
->with(
$this->equalTo($share),
$this->equalTo('password')
)->willReturn(false);

$this->session->method('exists')->with('public_link_authenticated')->willReturn(true);
$this->session->method('get')->with('public_link_authenticated')->willReturn('42');

$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);

$this->assertTrue($result);
}

public function testSharePasswordLinkInvalidSession() {
$share = $this->getMock('OCP\Share\IShare');
$share->method('getPassword')->willReturn('password');
$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);
$share->method('getId')->willReturn('42');

$this->shareManager->expects($this->once())
->method('getShareByToken')
->willReturn($share);

$this->shareManager->method('checkPassword')
->with(
$this->equalTo($share),
$this->equalTo('password')
)->willReturn(false);

$this->session->method('exists')->with('public_link_authenticated')->willReturn(true);
$this->session->method('get')->with('public_link_authenticated')->willReturn('43');

$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);

$this->assertFalse($result);
}
}
20 changes: 19 additions & 1 deletion lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -976,14 +976,32 @@ public function getSharesByPath(\OCP\Files\Node $path, $page=0, $perPage=50) {
public function getShareByToken($token) {
$provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_LINK);

$share = $provider->getShareByToken($token);
try {
$share = $provider->getShareByToken($token);
} catch (ShareNotFound $e) {
//Ignore
}

// If it is not a link share try to fetch a federated share by token
if ($share === null) {
$provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_REMOTE);
$share = $provider->getShareByToken($token);
}

if ($share->getExpirationDate() !== null &&
$share->getExpirationDate() <= new \DateTime()) {
$this->deleteShare($share);
throw new ShareNotFound();
}

/*
* Reduce the permissions for link shares if public upload is not enabled
*/
if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK &&
!$this->shareApiLinkAllowPublicUpload()) {
$share->setPermissions($share->getPermissions() & ~(\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE));
}

return $share;
}

Expand Down
19 changes: 19 additions & 0 deletions tests/lib/share20/managertest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2084,6 +2084,25 @@ public function testGetShareByTokenNotExpired() {
$this->assertSame($share, $res);
}

public function testGetShareByTokenPublicSharingDisabled() {
$share = $this->manager->newShare();
$share->setShareType(\OCP\Share::SHARE_TYPE_LINK)
->setPermissions(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);

$this->config->method('getAppValue')->will($this->returnValueMap([
['core', 'shareapi_allow_public_upload', 'yes', 'no'],
]));

$this->defaultProvider->expects($this->once())
->method('getShareByToken')
->willReturn('validToken')
->willReturn($share);

$res = $this->manager->getShareByToken('validToken');

$this->assertSame(\OCP\Constants::PERMISSION_READ, $res->getPermissions());
}

public function testCheckPasswordNoLinkShare() {
$share = $this->getMock('\OCP\Share\IShare');
$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER);
Expand Down