Skip to content

Commit f65d862

Browse files
tomerqodotomerqodo
committed
Apply changes for benchmark PR
1 parent 5cb16af commit f65d862

File tree

9 files changed

+94
-38
lines changed

9 files changed

+94
-38
lines changed

‎apps/dav/lib/Connector/Sabre/Auth.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,11 @@ public function __construct(
5555
* @see https://github.com/owncloud/core/issues/13245
5656
*/
5757
public function isDavAuthenticated(string $username): bool {
58-
return !is_null($this->session->get(self::DAV_AUTHENTICATED))
59-
&& $this->session->get(self::DAV_AUTHENTICATED) === $username;
58+
if (is_null($this->session->get(self::DAV_AUTHENTICATED))) {
59+
return false;
60+
}
61+
62+
return $this->session->get(self::DAV_AUTHENTICATED) === $username;
6063
}
6164

6265
/**

‎apps/dav/lib/Connector/Sabre/PublicAuth.php‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,7 @@ private function checkToken(): array {
137137
\OC_User::setIncognitoMode(true);
138138

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

@@ -180,17 +179,17 @@ protected function validateUserPass($username, $password) {
180179
if ($share->getShareType() === IShare::TYPE_LINK
181180
|| $share->getShareType() === IShare::TYPE_EMAIL
182181
|| $share->getShareType() === IShare::TYPE_CIRCLE) {
182+
// Validate password if provided
183183
if ($this->shareManager->checkPassword($share, $password)) {
184184
// If not set, set authenticated session cookie
185-
if (!$this->session->exists(self::DAV_AUTHENTICATED)
186-
|| $this->session->get(self::DAV_AUTHENTICATED) !== $share->getId()) {
187-
$this->session->set(self::DAV_AUTHENTICATED, $share->getId());
185+
if (!$this->isShareInSession($share)) {
186+
$this->addShareToSession($share);
188187
}
189188
return true;
190189
}
191190

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

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

225224
return $this->share;
226225
}
226+
227+
private function addShareToSession(IShare $share): void {
228+
$allowedShareIds = $this->session->get(self::DAV_AUTHENTICATED) ?? [];
229+
if (!is_array($allowedShareIds)) {
230+
$allowedShareIds = [];
231+
}
232+
233+
$allowedShareIds[] = $share->getId();
234+
$this->session->set(self::DAV_AUTHENTICATED, $allowedShareIds);
235+
}
236+
237+
private function isShareInSession(IShare $share): bool {
238+
if (!$this->session->exists(self::DAV_AUTHENTICATED)) {
239+
return false;
240+
}
241+
242+
$allowedShareIds = $this->session->get(self::DAV_AUTHENTICATED);
243+
if (!is_array($allowedShareIds)) {
244+
return false;
245+
}
246+
247+
return in_array($share->getId(), $allowedShareIds);
248+
}
227249
}

‎apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ public function testInvalidSharePasswordLinkValidSession(): void {
316316
)->willReturn(false);
317317

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

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

‎apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,15 @@ public function createFederatedShare($shareWith, $token, $password = '') {
9090
}
9191

9292
// make sure that user is authenticated in case of a password protected link
93-
$storedPassword = $share->getPassword();
94-
$authenticated = $this->session->get(PublicAuth::DAV_AUTHENTICATED) === $share->getId()
93+
$allowedShareIds = $this->session->get(PublicAuth::DAV_AUTHENTICATED);
94+
if (!is_array($allowedShareIds)) {
95+
$allowedShareIds = [];
96+
}
97+
98+
$authenticated = in_array($share->getId(), $allowedShareIds)
9599
|| $this->shareManager->checkPassword($share, $password);
100+
101+
$storedPassword = $share->getPassword();
96102
if (!empty($storedPassword) && !$authenticated) {
97103
$response = new JSONResponse(
98104
['message' => 'No permission to access the share'],

‎apps/files_sharing/lib/Controller/ShareController.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,12 @@ protected function authSucceeded() {
197197
}
198198

199199
// For share this was always set so it is still used in other apps
200-
$this->session->set(PublicAuth::DAV_AUTHENTICATED, $this->share->getId());
200+
$allowedShareIds = $this->session->get(PublicAuth::DAV_AUTHENTICATED);
201+
if (!is_array($allowedShareIds)) {
202+
$allowedShareIds = [];
203+
}
204+
205+
$this->session->set(PublicAuth::DAV_AUTHENTICATED, array_merge($allowedShareIds, [$this->share->getId()]));
201206
}
202207

203208
protected function authFailed() {

‎lib/public/AppFramework/AuthPublicShareController.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,7 @@ final public function authenticate(string $password = '', string $passwordReques
158158
$this->session->regenerateId(true, true);
159159
$response = $this->getRedirect();
160160

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

164163
$this->authSucceeded();
165164

‎lib/public/AppFramework/PublicShareController.php‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,21 @@
2525
* @since 14.0.0
2626
*/
2727
abstract class PublicShareController extends Controller {
28-
/** @var ISession */
29-
protected $session;
28+
29+
public const DAV_AUTHENTICATED_FRONTEND = 'public_link_authenticated_frontend';
3030

3131
/** @var string */
3232
private $token;
3333

3434
/**
3535
* @since 14.0.0
3636
*/
37-
public function __construct(string $appName,
37+
public function __construct(
38+
string $appName,
3839
IRequest $request,
39-
ISession $session) {
40+
protected ISession $session,
41+
) {
4042
parent::__construct($appName, $request);
41-
42-
$this->session = $session;
4343
}
4444

4545
/**
@@ -98,8 +98,7 @@ public function isAuthenticated(): bool {
9898
}
9999

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

@@ -116,4 +115,31 @@ public function isAuthenticated(): bool {
116115
*/
117116
public function shareNotFound() {
118117
}
118+
119+
/**
120+
* Validate the token and password hash stored in session
121+
*/
122+
protected function validateTokenSession(string $token, string $passwordHash): bool {
123+
$allowedTokensJSON = $this->session->get(self::DAV_AUTHENTICATED_FRONTEND) ?? '[]';
124+
$allowedTokens = json_decode($allowedTokensJSON, true);
125+
if (!is_array($allowedTokens)) {
126+
$allowedTokens = [];
127+
}
128+
129+
return ($allowedTokens[$token] ?? '') === $passwordHash;
130+
}
131+
132+
/**
133+
* Store the token and password hash in session
134+
*/
135+
protected function storeTokenSession(string $token, string $passwordHash = ''): void {
136+
$allowedTokensJSON = $this->session->get(self::DAV_AUTHENTICATED_FRONTEND) ?? '[]';
137+
$allowedTokens = json_decode($allowedTokensJSON, true);
138+
if (!is_array($allowedTokens)) {
139+
$allowedTokens = [];
140+
}
141+
142+
$allowedTokens[$token] = $passwordHash;
143+
$this->session->set(self::DAV_AUTHENTICATED_FRONTEND, json_encode($allowedTokens));
144+
}
119145
}

‎tests/lib/AppFramework/Controller/AuthPublicShareControllerTest.php‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,15 @@ public function testAuthenticateValidPassword(): void {
112112
['public_link_authenticate_redirect', json_encode(['foo' => 'bar'])],
113113
]);
114114

115-
$tokenSet = false;
116-
$hashSet = false;
115+
$tokenStored = false;
117116
$this->session
118117
->method('set')
119-
->willReturnCallback(function ($key, $value) use (&$tokenSet, &$hashSet) {
120-
if ($key === 'public_link_authenticated_token' && $value === 'token') {
121-
$tokenSet = true;
122-
return true;
123-
}
124-
if ($key === 'public_link_authenticated_password_hash' && $value === 'hash') {
125-
$hashSet = true;
118+
->willReturnCallback(function ($key, $value) use (&$tokenStored) {
119+
if ($key === AuthPublicShareController::DAV_AUTHENTICATED_FRONTEND) {
120+
$decoded = json_decode($value, true);
121+
if (isset($decoded['token']) && $decoded['token'] === 'hash') {
122+
$tokenStored = true;
123+
}
126124
return true;
127125
}
128126
return false;
@@ -134,7 +132,6 @@ public function testAuthenticateValidPassword(): void {
134132
$result = $this->controller->authenticate('password');
135133
$this->assertInstanceOf(RedirectResponse::class, $result);
136134
$this->assertSame('myLink!', $result->getRedirectURL());
137-
$this->assertTrue($tokenSet);
138-
$this->assertTrue($hashSet);
135+
$this->assertTrue($tokenStored);
139136
}
140137
}

‎tests/lib/AppFramework/Controller/PublicShareControllerTest.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,8 @@ public function testIsAuthenticatedNotPasswordProtected(bool $protected, string
7474
$controller = new TestController('app', $this->request, $this->session, $hash2, $protected);
7575

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

8280
$controller->setToken($token2);
8381

0 commit comments

Comments
 (0)