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.

Sign up for GitHub

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

View restricted access content #51

Open
wants to merge 3 commits into
base: 2.x
Choose a base branch
from
Open

View restricted access content #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bf9fdca
Update IIIFManifestParser.php
kylehuynh205 Jul 6, 2023
e46b10f
Update openseadragon.module
kylehuynh205 Jul 6, 2023
9ceb5d4
Tune the code for phpcs test
kylehuynh205 Jul 6, 2023
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
21 changes: 19 additions & 2 deletions openseadragon.module
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,22 @@ function template_preprocess_openseadragon_formatter(&$variables) {
$variables['#attached']['library'] = [
'openseadragon/init',
];
$access_token = \Drupal::service('jwt.authentication.jwt')->generateToken();
$variables['#attached']['drupalSettings']['openseadragon'][$openseadragon_viewer_id] = [
'basePath' => Url::fromUri($iiif_address),
'fitToAspectRatio' => $viewer_settings['fit_to_aspect_ratio'],
'options' => [
'id' => $openseadragon_viewer_id,
'prefixUrl' => 'https://cdnjs.cloudflare.com/ajax/libs/openseadragon/2.4.2/images/',
'tileSources' => $tile_sources,

// For dsu-utsc.
Copy link
Contributor

Choose a reason for hiding this comment

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

No longer quite true if this is to be pushed into the base code.

'loadTilesWithAjax' => TRUE,
'ajaxWithCredentials' => TRUE,
'ajaxHeaders' => [
"Authorization" => "Bearer " . $access_token,
'token' => $access_token,
],
Comment on lines +102 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

Much the same as the previous PR, these tokens are generally expected to be valid for a limited length of time, so we would have to propagate cache headers preventing them from being used beyond their lifetime to prevent serving up responses with invalid tokens.

Comment on lines +100 to +107
Copy link
Member

Choose a reason for hiding this comment

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

This is feedback from a Slack conversation.

Testing this on our instance, if we leave this two blocks in (kylehuynh205@9ceb5d4#diff-6d693de3726ed28241d0e4b5045e66eff4e2e82da162a08975768d342ec2ed7eR161-R167 and kylehuynh205@9ceb5d4#diff-6d693de3726ed28241d0e4b5045e66eff4e2e82da162a08975768d342ec2ed7eR101-R107), images are not displayed in the viewer, and we get CORS errors.

If I remove the two blocks, everything works fine, including the original issue I had with the viewer not working on paged objects that used OSD.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a good compromise here would be to add these values loadTilesWithAjax, ajaxWithCredentials, ajaxHeaders, but do not set them by default, and make them configurable in the settings page for the OSD module? So something like:

'loadTilesWithAjax' => FALSE,
'ajaxWithCredentials' => FALSE,

...then if the above is set to TRUE, add the ajaxHeaders section.

] + $viewer_settings,
];

Expand All @@ -111,10 +120,10 @@ function template_preprocess_openseadragon_formatter(&$variables) {
*/
function template_preprocess_openseadragon_iiif_manifest_block(&$variables) {
$cache_meta = CacheableMetadata::createFromRenderArray($variables);

$access_token = \Drupal::service('jwt.authentication.jwt')->generateToken();
// Get the tile sources from the manifest.
$parser = \Drupal::service('openseadragon.manifest_parser');
$tile_sources = $parser->getTileSources($variables['iiif_manifest_url']);
$tile_sources = $parser->getTileSources($variables['iiif_manifest_url'], $access_token);

if (empty($tile_sources)) {
$cache_meta->applyTo($variables);
Expand Down Expand Up @@ -148,6 +157,14 @@ function template_preprocess_openseadragon_iiif_manifest_block(&$variables) {
'id' => $openseadragon_viewer_id,
'prefixUrl' => 'https://cdnjs.cloudflare.com/ajax/libs/openseadragon/2.4.2/images/',
'tileSources' => $tile_sources,

// For dsu-utsc.
Copy link
Contributor

Choose a reason for hiding this comment

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

No longer quite true if this is to be pushed into the base code.

'loadTilesWithAjax' => TRUE,
'ajaxWithCredentials' => TRUE,
'ajaxHeaders' => [
"Authorization" => "Bearer " . $access_token,
'token' => $access_token,
],
Comment on lines +162 to +167
Copy link
Contributor

Choose a reason for hiding this comment

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

Much the same as the previous PR, these tokens are generally expected to be valid for a limited length of time, so we would have to propagate cache headers preventing them from being used beyond their lifetime to prevent serving up responses with invalid tokens.

] + $viewer_settings,
];

Expand Down
11 changes: 9 additions & 2 deletions src/IIIFManifestParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,13 @@ public function __construct(
*
* @param string $manifest_url
* The location of the IIIF manifest, which can include tokens.
* @param string $access_token
* The JWT Access token.
*
* @return array
* The URLs of all the tile sources in a manifest.
*/
public function getTileSources($manifest_url) {
public function getTileSources($manifest_url, $access_token = NULL) {

// Try to construct the URL out of a tokenized string
// if the node is available.
Expand All @@ -85,7 +87,12 @@ public function getTileSources($manifest_url) {

try {
// Request the manifest.
$manifest_response = $this->httpClient->get($manifest_url);
// $manifest_response = $this->httpClient->get($manifest_url);
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented out code shouldn't be here.

$manifest_response = $this->httpClient->request('GET', $manifest_url, [
'headers' => [
'Authorization' => 'Bearer ' . $access_token,
],
]);
Comment on lines +91 to +95
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure that this makes sense in the case $access_token was not passed, as we would generate an Authorization header of just Bearer (or possibly Bearer null?).

Really, if we want to be sure to always have a token here, why not just generate it here?


// Decode the manifest json.
$manifest_string = (string) $manifest_response->getBody();
Expand Down