Action set-finished.json would eventually have an invalid token value.

The issue might be related to caching of some sort as the result of getGenericH5PIntegrationSettings does not seem to be updated for each request, but would only be re-run when caches are cleared. The result is that security token becomes outdated after a certain number of hours.

In case of embed the solution seems to be to force no caching (please note the options part of the route):

h5p.content.embed:
  path: '/h5p/{id}/embed'
  defaults:
    _controller: '\Drupal\h5p\Controller\H5PEmbed::embed'
    _title: 'H5P Embed'
  requirements:
    _permission: 'access content'
    id: \d+
  options:
    no_cache: TRUE

The solutions for other pages/widgets/formatters using H5PIntegration might probably differ. The best approach would be to just disable the caching for those parameters being passed to the drupalSettings.

The code seems to be using cache busting logic for embed page and field formatter, though that does not seems to be enough:

$response = [
      '#cache' => [
        'tags' => [
          'h5p_content:' . $id,
        ],
      ],
    ];

The issue is that the drupalSettings will only be invalidated along with the entity itself, yet the token has nothing to do with the entity. Setting the value to be invalidated after certain time would only work if the time value for cached token creation time is available. The best simplest solution might be to set the max-age to be equal to 0 and prevent caching altogether.

CommentFileSizeAuthor
#6 3036486-disable-cached-user-6.patch597 bytesamourow
#5 3036486-disable-cached-user.patch597 bytesamourow

Issue fork h5p-3036486

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pjotr.savitski created an issue. See original summary.

pjotr.savitski’s picture

pjotr.savitski commented
Issue summary: View changes
Raman Starshykh’s picture

Raman Starshykh commented
Issue tags: -token +epam-contrib-2019.03
linichalexey’s picture

linichalexey commented
Issue tags: -epam-contrib-2019.03
amourow’s picture

amourow
he/him/his
commented
StatusFileSize
new 3036486-disable-cached-user.patch597 bytes

Change the cache attribute to 'max-age' => 0 fixed the problem
The user data in drupalSettings.h5p.H5PIntegration.user was cached and caused the token validation error.

amourow’s picture

amourow
he/him/his
commented
Version: 8.x-1.0-rc12 » 8.x-1.0-rc17
StatusFileSize
new 3036486-disable-cached-user-6.patch597 bytes

Reroll patch for latest dev

pjotr.savitski’s picture

pjotr.savitski commented

Submitted patch should fix the issue, but it will also fully disable caching as such and that might cause some performance issues.

Maybe max age could be set to the duration of that token. It seemed to be around one day, if I remember that correctly.

I am also not sure of removing cache tags is a good idea. The cache should be reset along with any changes to that content entity, with maximum cache age less than the token lifetime. I'd even say it might be a good idea to leave at least one hour (maybe even more) for the page to function properly without doing a refresh. The last aspect will surely be useful in case of large columns and presentations.

illeace’s picture

illeace commented

If I'm reading the H5P core library's validToken() function (vendor/h5p/h5p-core/h5p.classes.php line 3351) correctly, whenever a token is generated, it is going to be valid for at least 12 hours and up to 24 hours. Rather than disabling caching completely, what if the max-age was set to 12 hours?

illeace’s picture

illeace commented
Version: 8.x-1.0-rc17 » 2.0.x-dev
Assigned: Unassigned » illeace

illeace opened merge request !46

illeace’s picture

illeace commented
Status: Active » Needs review

OK, as @pjotr.savitski suggests, I've created an issue fork that sets the max age to align with the token expiration time. This ensures that the Drupal cache never serves an H5P with an already-expired token. This is ready to be tested, but a bit time-consuming since it involves waiting for 12 hours for the cache to expire.

illeace’s picture

illeace commented

The help simplify testing a bit, here's the test I've been using:

  1. Make sure Drupal's caching is enabled
  2. Create a Question Set H5P, or any other interactive that has a "Finish" button to submit results.
  3. Clear the Drupal cache.
  4. Make sure you are a logged in user and load a page that displays that H5P (it could be the normal node page or the embed URL). Start a timer as soon as the page loads.
  5. Open your browser's network inspector, set it to filter for "set-finished.json" to exclude any network activity aside from that callback URL, which is where your H5P "finished" data gets posted to.
  6. Complete the H5P and click the Finish button. You should see an item appear in the network inspector.
  7. Click that network item and ensure the response is JSON data with contents of {"success":true}
  8. Now the boring part ... wait 12 hours and reload the page.
  9. Repeat the steps above to ensure the "set-finished.json" response is still {"success":true}
maya maier’s picture

maya maier
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Tested, new token expiry works as expected. Marking RTBC.

sim_1 made their first commit to this issue’s fork.

  • sim_1 committed 7f2f64fb on 2.0.x authored by illeace 
    Issue #3036486 by amourow, pjotr.savitski, illeace, maya maier: Results...
sim_1’s picture

sim_1
she/her
commented
Status: Reviewed & tested by the community » Fixed

Merged and marking as fixed.

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.