Skip to content
Open
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
7 changes: 5 additions & 2 deletions apps/dav/lib/Connector/Sabre/Auth.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,11 @@ public function __construct(
* @see https://github.com/owncloud/core/issues/13245
*/
public function isDavAuthenticated(string $username): bool {
return !is_null($this->session->get(self::DAV_AUTHENTICATED))
&& $this->session->get(self::DAV_AUTHENTICATED) === $username;
if (is_null($this->session->get(self::DAV_AUTHENTICATED))) {
return false;
}

return $this->session->get(self::DAV_AUTHENTICATED) === $username;
}

/**
Expand Down
36 changes: 29 additions & 7 deletions apps/dav/lib/Connector/Sabre/PublicAuth.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ private function checkToken(): array {
\OC_User::setIncognitoMode(true);

// If already authenticated
if ($this->session->exists(self::DAV_AUTHENTICATED)
&& $this->session->get(self::DAV_AUTHENTICATED) === $share->getId()) {
if ($this->isShareInSession($share)) {
return [true, $this->principalPrefix . $token];
}

Expand Down Expand Up @@ -180,17 +179,17 @@ protected function validateUserPass($username, $password) {
if ($share->getShareType() === IShare::TYPE_LINK
|| $share->getShareType() === IShare::TYPE_EMAIL
|| $share->getShareType() === IShare::TYPE_CIRCLE) {
// Validate password if provided
if ($this->shareManager->checkPassword($share, $password)) {
// If not set, set authenticated session cookie
if (!$this->session->exists(self::DAV_AUTHENTICATED)
|| $this->session->get(self::DAV_AUTHENTICATED) !== $share->getId()) {
$this->session->set(self::DAV_AUTHENTICATED, $share->getId());
if (!$this->isShareInSession($share)) {
$this->addShareToSession($share);
}
return true;
}

if ($this->session->exists(PublicAuth::DAV_AUTHENTICATED)
&& $this->session->get(PublicAuth::DAV_AUTHENTICATED) === $share->getId()) {
// We are already authenticated for this share in the session
if ($this->isShareInSession($share)) {
return true;
}

Expand Down Expand Up @@ -224,4 +223,27 @@ public function getShare(): IShare {

return $this->share;
}

private function addShareToSession(IShare $share): void {
$allowedShareIds = $this->session->get(self::DAV_AUTHENTICATED) ?? [];
if (!is_array($allowedShareIds)) {
$allowedShareIds = [];
}

$allowedShareIds[] = $share->getId();
$this->session->set(self::DAV_AUTHENTICATED, $allowedShareIds);
}

private function isShareInSession(IShare $share): bool {
if (!$this->session->exists(self::DAV_AUTHENTICATED)) {
return false;
}

$allowedShareIds = $this->session->get(self::DAV_AUTHENTICATED);
if (!is_array($allowedShareIds)) {
return false;
}

return in_array($share->getId(), $allowedShareIds);
}
}
2 changes: 1 addition & 1 deletion apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ public function testInvalidSharePasswordLinkValidSession(): void {
)->willReturn(false);

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,15 @@ public function createFederatedShare($shareWith, $token, $password = '') {
}

// make sure that user is authenticated in case of a password protected link
$storedPassword = $share->getPassword();
$authenticated = $this->session->get(PublicAuth::DAV_AUTHENTICATED) === $share->getId()
$allowedShareIds = $this->session->get(PublicAuth::DAV_AUTHENTICATED);
if (!is_array($allowedShareIds)) {
$allowedShareIds = [];
}

$authenticated = in_array($share->getId(), $allowedShareIds)
|| $this->shareManager->checkPassword($share, $password);

$storedPassword = $share->getPassword();
if (!empty($storedPassword) && !$authenticated) {
$response = new JSONResponse(
['message' => 'No permission to access the share'],
Expand Down
7 changes: 6 additions & 1 deletion apps/files_sharing/lib/Controller/ShareController.php
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,12 @@ protected function authSucceeded() {
}

// For share this was always set so it is still used in other apps
$this->session->set(PublicAuth::DAV_AUTHENTICATED, $this->share->getId());
$allowedShareIds = $this->session->get(PublicAuth::DAV_AUTHENTICATED);
if (!is_array($allowedShareIds)) {
$allowedShareIds = [];
}

$this->session->set(PublicAuth::DAV_AUTHENTICATED, array_merge($allowedShareIds, [$this->share->getId()]));
}

protected function authFailed() {
Expand Down
3 changes: 1 addition & 2 deletions lib/public/AppFramework/AuthPublicShareController.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ final public function authenticate(string $password = '', string $passwordReques
$this->session->regenerateId(true, true);
$response = $this->getRedirect();

$this->session->set('public_link_authenticated_token', $this->getToken());
$this->session->set('public_link_authenticated_password_hash', $this->getPasswordHash());
$this->storeTokenSession($this->getToken(), $this->getPasswordHash());

$this->authSucceeded();

Expand Down
42 changes: 34 additions & 8 deletions lib/public/AppFramework/PublicShareController.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,21 @@
* @since 14.0.0
*/
abstract class PublicShareController extends Controller {
/** @var ISession */
protected $session;

public const DAV_AUTHENTICATED_FRONTEND = 'public_link_authenticated_frontend';

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

/**
* @since 14.0.0
*/
public function __construct(string $appName,
public function __construct(
string $appName,
IRequest $request,
ISession $session) {
protected ISession $session,
) {
parent::__construct($appName, $request);

$this->session = $session;
}

/**
Expand Down Expand Up @@ -98,8 +98,7 @@ public function isAuthenticated(): bool {
}

// If we are authenticated properly
if ($this->session->get('public_link_authenticated_token') === $this->getToken()
&& $this->session->get('public_link_authenticated_password_hash') === $this->getPasswordHash()) {
if ($this->validateTokenSession($this->getToken(), $this->getPasswordHash())) {
return true;
}

Expand All @@ -116,4 +115,31 @@ public function isAuthenticated(): bool {
*/
public function shareNotFound() {
}

/**
* Validate the token and password hash stored in session
*/
protected function validateTokenSession(string $token, string $passwordHash): bool {
$allowedTokensJSON = $this->session->get(self::DAV_AUTHENTICATED_FRONTEND) ?? '[]';
$allowedTokens = json_decode($allowedTokensJSON, true);
if (!is_array($allowedTokens)) {
$allowedTokens = [];
}

return ($allowedTokens[$token] ?? '') === $passwordHash;
}

/**
* Store the token and password hash in session
*/
protected function storeTokenSession(string $token, string $passwordHash = ''): void {
$allowedTokensJSON = $this->session->get(self::DAV_AUTHENTICATED_FRONTEND) ?? '[]';
$allowedTokens = json_decode($allowedTokensJSON, true);
if (!is_array($allowedTokens)) {
$allowedTokens = [];
}

$allowedTokens[$token] = $passwordHash;
$this->session->set(self::DAV_AUTHENTICATED_FRONTEND, json_encode($allowedTokens));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,15 @@ public function testAuthenticateValidPassword(): void {
['public_link_authenticate_redirect', json_encode(['foo' => 'bar'])],
]);

$tokenSet = false;
$hashSet = false;
$tokenStored = false;
$this->session
->method('set')
->willReturnCallback(function ($key, $value) use (&$tokenSet, &$hashSet) {
if ($key === 'public_link_authenticated_token' && $value === 'token') {
$tokenSet = true;
return true;
}
if ($key === 'public_link_authenticated_password_hash' && $value === 'hash') {
$hashSet = true;
->willReturnCallback(function ($key, $value) use (&$tokenStored) {
if ($key === AuthPublicShareController::DAV_AUTHENTICATED_FRONTEND) {
$decoded = json_decode($value, true);
if (isset($decoded['token']) && $decoded['token'] === 'hash') {
$tokenStored = true;
}
return true;
}
return false;
Expand All @@ -134,7 +132,6 @@ public function testAuthenticateValidPassword(): void {
$result = $this->controller->authenticate('password');
$this->assertInstanceOf(RedirectResponse::class, $result);
$this->assertSame('myLink!', $result->getRedirectURL());
$this->assertTrue($tokenSet);
$this->assertTrue($hashSet);
$this->assertTrue($tokenStored);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,8 @@ public function testIsAuthenticatedNotPasswordProtected(bool $protected, string
$controller = new TestController('app', $this->request, $this->session, $hash2, $protected);

$this->session->method('get')
->willReturnMap([
['public_link_authenticated_token', $token1],
['public_link_authenticated_password_hash', $hash1],
]);
->with(PublicShareController::DAV_AUTHENTICATED_FRONTEND)
->willReturn("{\"$token1\":\"$hash1\"}");

$controller->setToken($token2);

Expand Down
Loading