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.

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

Use signed URLs for videos inserted with the block #3

Closed
davidmpurdy opened this issue Aug 21, 2022 · 4 comments · Fixed by #20
Closed

Use signed URLs for videos inserted with the block #3

davidmpurdy opened this issue Aug 21, 2022 · 4 comments · Fixed by #20
Labels
enhancement Not a bug, an improvement

Comments

@davidmpurdy
Copy link
Collaborator

I'm not 100% sure how to approach this. Just making it a dynamic block looks to me like it would bring it into parity with the shortcode implementation. However, I have concerns about caching (e.g., where the page output is cached longer than the token expiration time).

Maybe the block should include a script that uses AJAX to get the token and set up the player? That would let you freely cache pages with videos while still retaining the benefits of signed URLs (e.g., videos could be restricted to logged-in users only but could still use page caching for those logged in users since the restriction would take place in the AJAX callback).

Any thoughts on changing the architecture in this way? I'm pretty sure that no matter what we do with the block (even just updating to the new iframe player) it will likely break editing for existing blocks. Thoughts on that and whether we would need to try and implement an update to existing blocks or if we could just make it a breaking change? Since blocks currently have the full (but old) script embedded, I'm thinking we could just leave them as-is (meaning they should keep chugging along but they couldn't be edited).

@B-Interactive
Copy link
Owner

B-Interactive commented Aug 23, 2022

That's a good point regarding potential for caching issues. I'll have a play around with the AJAX approach as well, as that sounds promising.

I'm still rather new to blocks, so I'm open to input on handling those and how best to handle legacy blocks (if they're to be handled at all). Leaving the legacy blocks as is would likely break them though, should one enforce signed URLs on the Cloudflare side. Not enforcing signed URLs undermines using signed URLs, as the unsigned video ID remains accessible to the end-user in either case (if using Cloudflare's player at least).

@davidmpurdy
Copy link
Collaborator Author

I've reached out to some folx, and the casual consensus is that an AJAX approach could work but that high volumes of un-cached calls will require more server resources (of course). Maybe signed URLs just need to have a "caveat emptor" attached to warn users that things will depend more than usual on their exact hosting configuration. I can't think of a silver bullet to make sure any kind of common cache is in sync with token expiration...

I'll move discussion of a block upgrade routine to a separate issue if that's alright with you - should help me keep my thoughts in order a bit.

@B-Interactive
Copy link
Owner

Perhaps the cache issue is something that might be addressed if/when it occurs. Although a relatively small use case, the site I manage has a token expiration of 60 minutes (default) and to date, no cache related issue has been experienced by me, or reported. Granted, there's a lot of factors at play with these sorts of potential issues (server infrastructure, caching mechanism, CDN, token expiration, video length, etc) so the possibility is far from ruled out by my limited test case.

Thinking about it a bit more, the AJAX approach would add additional latency. If I understand the communication path:

Current:

  • Client > Server > Cloudflare > Server > Client < Cloudflare[Content]

With AJAX:

  • Client > Server > Client > Server > Cloudflare > Server > Client < Cloudflare[Content]

More as a personal note, if using AJAX, managing authentication via nonce is necessary to prevent abuse of the REST API.

@davidmpurdy
Copy link
Collaborator Author

Every solution I can think of will rely on the specific server configuration in one way or another. If you're using it in production and it's working, maybe there's no need to try and pre-emptively solve it. I know I've run into "smart" caching configurations in the real world that sometimes keep a page cache around for days if it doesn't detect some reason to invalidate it. That's definitely the exception rather than the rule.

It does indeed add another client/server round trip, but the idea was maybe the tradeoff would be the ability to cache the full page request and let just the token request be un-cached.

  • Client > Server [page cache] > Client > Server [lighter un-cached AJAX token request] > Cloudflare [or Server transient] > Server > Client < Cloudflare[Content]

Either way, in my use case (a relatively large number of clients potentially viewing the same video at a similar time) I would keep the token in a transient to speed up its retrieval wherever it happens in the process.

I have to admit I haven't fully solved nonces - my thinking right now is in the direction of setting the nonce in a cookie on the login page and using that for the AJAX request. I haven't fully thought it through, though, so that may not work.

My actual use case is wanting to build a somewhat scalable site (not hundreds of thousands of users, but not dozens either) that only allows registered users to view videos (mostly to keep some kind of handle on charges for minutes viewed). I'm trying to figure out how to keep the hosting setup as economical and portable as possible. And maybe the best way to do that is just to make sure that the host never caches video pages for longer than the token expiration time. I don't want to overengineer this too much (especially at this stage of the game), just play out if there's a better approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Not a bug, an improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants