Skip to content

Commit de61d31

Browse files
committed
Adding cookie-based authentication (as configurable option) so the files can be properly downloaded via links even when authentication is required.
1 parent 10b169c commit de61d31

File tree

5 files changed

+58
-32
lines changed

5 files changed

+58
-32
lines changed

app/V1Module/security/AccessManager.php

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ class AccessManager
3939
/** @var int Expiration time of newly issued invitation tokens (in seconds) */
4040
private $invitationExpiration;
4141

42+
/** @var string|null Name of the cookie where to look for the token */
43+
private $tokenCookieName;
44+
4245
public function __construct(array $parameters, Users $users)
4346
{
4447
$this->users = $users;
@@ -48,6 +51,7 @@ public function __construct(array $parameters, Users $users)
4851
$this->issuer = Arrays::get($parameters, "issuer", "https://recodex.mff.cuni.cz");
4952
$this->audience = Arrays::get($parameters, "audience", "https://recodex.mff.cuni.cz");
5053
$this->usedAlgorithm = Arrays::get($parameters, "usedAlgorithm", "HS256");
54+
$this->tokenCookieName = Arrays::get($parameters, "tokenCookieName", null);
5155
JWT::$leeway = Arrays::get($parameters, "leeway", 10); // 10 seconds
5256
}
5357

@@ -140,9 +144,9 @@ public function getUser(AccessToken $token): User
140144
*/
141145
public function issueToken(
142146
User $user,
143-
string $effectiveRole = null,
147+
?string $effectiveRole = null,
144148
array $scopes = [],
145-
int $exp = null,
149+
?int $exp = null,
146150
array $payload = []
147151
) {
148152
if (!$user->isAllowed()) {
@@ -208,7 +212,7 @@ public function issueInvitationToken(
208212
string $titlesBefore = "",
209213
string $titlesAfter = "",
210214
array $groupsIds = [],
211-
int $invitationExpiration = null,
215+
?int $invitationExpiration = null,
212216
): string {
213217
$token = InvitationToken::create(
214218
$invitationExpiration ?? $this->invitationExpiration,
@@ -227,25 +231,30 @@ public function issueInvitationToken(
227231
* Extract the access token from the request.
228232
* @return string|null The access token parsed from the HTTP request, or null if there is no access token.
229233
*/
230-
public static function getGivenAccessToken(IRequest $request)
234+
public function getGivenAccessToken(IRequest $request)
231235
{
232236
$accessToken = $request->getQuery("access_token");
233237
if ($accessToken !== null && Strings::length($accessToken) > 0) {
234-
return $accessToken; // the token specified in the URL is prefered
238+
return $accessToken; // the token specified in the URL is preferred
235239
}
236240

237241
// if the token is not in the URL, try to find the "Authorization" header with the bearer token
238242
$authorizationHeader = $request->getHeader("Authorization");
239-
240-
if ($authorizationHeader === null) {
241-
return null;
243+
if ($authorizationHeader !== null) {
244+
$parts = Strings::split($authorizationHeader, "/ /");
245+
if (count($parts) === 2) {
246+
list($bearer, $accessToken) = $parts;
247+
if ($bearer === "Bearer" && !str_contains($accessToken, " ") && Strings::length($accessToken) > 0) {
248+
return $accessToken;
249+
}
250+
}
242251
}
243252

244-
$parts = Strings::split($authorizationHeader, "/ /");
245-
if (count($parts) === 2) {
246-
list($bearer, $accessToken) = $parts;
247-
if ($bearer === "Bearer" && !str_contains($accessToken, " ") && Strings::length($accessToken) > 0) {
248-
return $accessToken;
253+
// finally, try fallback to cookie if configured
254+
if ($this->tokenCookieName !== null) {
255+
$cookieToken = $request->getCookie($this->tokenCookieName);
256+
if ($cookieToken !== null && Strings::length($cookieToken) > 0) {
257+
return $cookieToken; // token found in the cookie
249258
}
250259
}
251260

app/config/config.local.neon.example

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ parameters:
66
address: "https://your.recodex.domain"
77

88
async:
9-
pollingInterval: 10 # seconds (you may set this to larger values if inotify wakeups are allowed)
10-
# inotify can wake the async worker (immediately once an async opertion is issued)
9+
pollingInterval: 10 # seconds (you may set this to larger values if inotify wake-ups are allowed)
10+
# inotify can wake the async worker (immediately once an async operation is issued)
1111
inotify: false # set to true only if your system (and PHP) supports inotify (not available on Windows, extension required on Linux)
1212

1313
fileStorage: # where the local files are being stored
@@ -17,7 +17,7 @@ parameters:
1717
root: %appDir%/../storage/hash # this should be replaced with path to existing directory
1818

1919
submissions:
20-
locked: false # if set to true, the API will not be accepting submissions (and it will be incidated in can-submit/permission hints)
20+
locked: false # if set to true, the API will not be accepting submissions (and it will be indicated in can-submit/permission hints)
2121
lockedReason: # Localized message with reason displayed in UI, why the submissions are locked (ignored if locked == false)
2222
cs: "Odevzdávání řešení bylo zablokováno v konfiguraci aplikace."
2323
en: "Submitting new solutions is currently locked out in the application configuration."
@@ -28,12 +28,13 @@ parameters:
2828
expiration: 604800 # 7 days in seconds
2929
invitationExpiration: 604800 # of an invitation token (7 days in seconds)
3030
verificationKey: "recodex-123" # this should be a really secret string
31+
tokenCookieName: 'recodex_accessToken' # web-app config value 'PERSISTENT_TOKENS_KEY_PREFIX' + '_accessToken', null if only Authorization header is used
3132

3233
broker:
3334
address: "tcp://127.0.0.1:9658"
3435
auth:
3536
username: "user" # these credentials must match credentials
36-
password: "pass" # in broker configration file
37+
password: "pass" # in broker configuration file
3738

3839
monitor:
3940
address: "wss://your.recodex.domain:443/ws"
@@ -59,7 +60,7 @@ parameters:
5960
footerUrl: "%webapp.address%"
6061
from: "ReCodEx <[email protected]>"
6162
defaultAdminTo: "Administrator <[email protected]>"
62-
#debugMode: true # in debug mode, no messages are sent via SMPT (you should also active archiving)
63+
#debugMode: true # in debug mode, no messages are sent via SMTP (you should also active archiving)
6364
#archivingDir: "%appDir%/../log/email-debug" # a directory where copies of all emails sent are stored (in text files)
6465

6566
exercises:
@@ -72,7 +73,7 @@ parameters:
7273
solutionSizeLimitDefault: 262144 # 256 KiB, max. size for all submitted files (default, configurable per assignment)
7374

7475
removeInactiveUsers:
75-
# How long the user has to be inactive to warant the removal (null = never remove students, 1 month is minimum).
76+
# How long the user has to be inactive to warrant the removal (null = never remove students, 1 month is minimum).
7677
# Please note that the length of the auth. token expiration should be considered (readonly tokens may expire after 1 year).
7778
threshold: "2 years"
7879

app/config/config.neon

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ parameters:
2323
versionFormat: "{tag}"
2424

2525
async:
26-
pollingInterval: 60 # seconds (you may set this to larger values if inotify wakeups are allowed)
26+
pollingInterval: 60 # seconds (you may set this to larger values if inotify wake-ups are allowed)
2727
retries: 3 # how many times each async job is retried when failing
28-
# inotify can wake the async worker (immediately once an async opertion is issued)
28+
# inotify can wake the async worker (immediately once an async operation is issued)
2929
inotify: false # set to true only if your system (and PHP) supports inotify (not available on Windows, extension required on Linux)
3030
inotifyFile: %tempDir%/async-inotify-file # file used as inotify rod for triggering async worker on dispatch
3131
restartWorkerAfter: # memory leak precaution - worker is restarted once in a while
@@ -44,7 +44,7 @@ parameters:
4444
address: "https://your.recodex.domain"
4545

4646
submissions:
47-
locked: false # if set to true, the API will not be accepting submissions (and it will be incidated in can-submit/permission hints)
47+
locked: false # if set to true, the API will not be accepting submissions (and it will be indicated in can-submit/permission hints)
4848
lockedReason: # Localized message with reason displayed in UI, why the submissions are locked (ignored if locked == false)
4949
cs: "Odevzdávání řešení bylo zablokováno v konfiguraci aplikace."
5050
en: "Submitting new solutions is currently locked out in the application configuration."
@@ -57,6 +57,7 @@ parameters:
5757
invitationExpiration: 86400 # of an invitation token (seconds)
5858
usedAlgorithm: HS256
5959
verificationKey: "recodex-123"
60+
tokenCookieName: 'recodex_accessToken' # web-app config value 'PERSISTENT_TOKENS_KEY_PREFIX' + '_accessToken', null if only Authorization header is used
6061

6162
broker: # connection to broker
6263
address: "tcp://127.0.0.1:9658"
@@ -198,7 +199,7 @@ parameters:
198199
deletedEmailSuffix: "@deleted.recodex" # Suffix string appended to an email address of a user, when account is deleted
199200

200201
removeInactiveUsers:
201-
# How long the user has to be inactive to warant the removal (null = never remove students, 1 month is minimum).
202+
# How long the user has to be inactive to warrant the removal (null = never remove students, 1 month is minimum).
202203
# Please note that the length of the auth. token expiration should be considered (readonly tokens may expire after 1 year).
203204
disableAfter: "2 years"
204205
deleteAfter: null # null = never, if not null, better be > disableAfter
@@ -225,7 +226,7 @@ mail: # configuration of sending mails
225226
password: "" # password to the server
226227
secure: "tls" # security, values are empty for no security, "ssl" or "tls"
227228
context: # additional parameters, depending on used mail engine
228-
ssl: # examle self-signed certificates can be allowed as verify_peer and verify_peer_name to false and allow_self_signed to true under ssl key (see example)
229+
ssl: # example self-signed certificates can be allowed as verify_peer and verify_peer_name to false and allow_self_signed to true under ssl key (see example)
229230
verify_peer: false
230231
verify_peer_name: false
231232
allow_self_signed: true

app/model/view/InstanceViewFactory.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ public function getInstance(Instance $instance, ?User $loggedUser = null): array
4444

4545
return [
4646
"id" => $instance->getId(),
47-
"name" => $localizedRootGroup ? $localizedRootGroup->getName() : "", // BC
48-
"description" => $localizedRootGroup ? $localizedRootGroup->getDescription() : "", // BC
4947
"hasValidLicence" => $instance->hasValidLicense(),
5048
"isOpen" => $instance->isOpen(),
5149
"isAllowed" => $instance->isAllowed(),
@@ -56,6 +54,10 @@ public function getInstance(Instance $instance, ?User $loggedUser = null): array
5654
"rootGroup" => $this->groupViewFactory->getGroup($instance->getRootGroup()),
5755
"rootGroupId" => $instance->getRootGroup()->getId(),
5856
"extensions" => $extensions,
57+
58+
// deprecated
59+
"name" => $localizedRootGroup ? $localizedRootGroup->getName() : "", // BC
60+
"description" => $localizedRootGroup ? $localizedRootGroup->getDescription() : "", // BC
5961
];
6062
}
6163

tests/AccessToken/AccessManager.phpt

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -227,47 +227,60 @@ class TestAccessManager extends Tester\TestCase
227227
$token = "abcdefg";
228228
$url = new UrlScript("https://www.whatever.com/bla/bla/bla?x=y&access_token=$token");
229229
$request = new Request($url);
230-
Assert::equal($token, AccessManager::getGivenAccessToken($request));
230+
231+
$users = Mockery::mock(App\Model\Repository\Users::class);
232+
$manager = new AccessManager(["verificationKey" => "abc"], $users);
233+
Assert::equal($token, $manager->getGivenAccessToken($request));
231234
}
232235

233236
public function testExtractFromEmptyQuery()
234237
{
235238
$token = "";
236239
$url = new UrlScript("https://www.whatever.com/bla/bla/bla?x=y&access_token=$token");
237240
$request = new Request($url);
238-
Assert::null(AccessManager::getGivenAccessToken($request));
241+
$users = Mockery::mock(App\Model\Repository\Users::class);
242+
$manager = new AccessManager(["verificationKey" => "abc"], $users);
243+
Assert::null($manager->getGivenAccessToken($request));
239244
}
240245

241246
public function testExtractFromHeader()
242247
{
243248
$token = "abcdefg";
244249
$url = new UrlScript("https://www.whatever.com/bla/bla/bla?x=y");
245250
$request = new Request($url, [], [], [], ["Authorization" => "Bearer $token"]);
246-
Assert::equal($token, AccessManager::getGivenAccessToken($request));
251+
$users = Mockery::mock(App\Model\Repository\Users::class);
252+
$manager = new AccessManager(["verificationKey" => "abc"], $users);
253+
Assert::equal($token, $manager->getGivenAccessToken($request));
247254
}
248255

249256
public function testExtractFromHeaderWrongType()
250257
{
251258
$token = "abcdefg";
252259
$url = new UrlScript("https://www.whatever.com/bla/bla/bla?x=y");
253260
$request = new Request($url, [], [], [], ["Authorization" => "Basic $token"]);
254-
Assert::null(AccessManager::getGivenAccessToken($request));
261+
$users = Mockery::mock(App\Model\Repository\Users::class);
262+
$manager = new AccessManager(["verificationKey" => "abc"], $users);
263+
Assert::null($manager->getGivenAccessToken($request));
255264
}
256265

257266
public function testExtractFromHeaderEmpty()
258267
{
259268
$token = "";
260269
$url = new UrlScript("https://www.whatever.com/bla/bla/bla?x=y");
261270
$request = new Request($url, [], [], [], ["Authorization" => "Basic $token"]);
262-
Assert::null(AccessManager::getGivenAccessToken($request));
271+
$users = Mockery::mock(App\Model\Repository\Users::class);
272+
$manager = new AccessManager(["verificationKey" => "abc"], $users);
273+
Assert::null($manager->getGivenAccessToken($request));
263274
}
264275

265276
public function testExtractFromHeaderWithSpace()
266277
{
267278
$token = "";
268279
$url = new UrlScript("https://www.whatever.com/bla/bla/bla?x=y");
269280
$request = new Request($url, [], [], [], ["Authorization" => "Bearer $token and more!"]);
270-
Assert::null(AccessManager::getGivenAccessToken($request));
281+
$users = Mockery::mock(App\Model\Repository\Users::class);
282+
$manager = new AccessManager(["verificationKey" => "abc"], $users);
283+
Assert::null($manager->getGivenAccessToken($request));
271284
}
272285
}
273286

0 commit comments

Comments
 (0)