feat/add-secure-url-token-to-imgix-global-config

rahulk94 commented 2 years ago

Description

This PR adds support for the secureURLToken parameter from imgix-core-js by passing this through if it configured in the Ember application's Imgix configuration.

This is a non-breaking feature and should be releasable in a minor.

New Feature

NA - If this is a big feature with breaking changes, consider [opening an issue][issues] to discuss first. This is completely up to you, but please keep in mind that your PR might not be accepted.
[ ] Run unit tests to ensure all existing tests are still passing
[x] Add new passing unit tests to cover the code introduced by your PR
[x] Update the readme
[x] Add some steps so we can test your cool new feature!
[x] The PR title is in the conventional commit format: e.g. feat(<area>): added new way to do this cool thing #issue-number
[x] Add your info to the contributors array in package.json!

Steps to Test

Setup ember-cli-imgix with an Imgix source which requires secure tokens to be provided with images. Without passing through secureURLToken these image requests should be failing and returning a 403 forbidden.
Add the global configuration for secureURLToken and restart your Ember app
The images should now come back successfully.

I've verified this against our internal application however do not have a token or demo app I could share for this unfortunately.

commit-lint[bot] commented 2 years ago

Features

expose secure url token from Imgix client through imgix app config (81c576a)

Documentation

add missing configuration options to global config schema (9b594be)

Contributors

rahulk94

Commit-Lint commands

You can trigger Commit-Lint actions by commenting on this PR: - `@Commit-Lint merge patch` will merge dependabot PR on "patch" versions (X.X.Y - Y change) - `@Commit-Lint merge minor` will merge dependabot PR on "minor" versions (X.Y.Y - Y change) - `@Commit-Lint merge major` will merge dependabot PR on "major" versions (Y.Y.Y - Y change) - `@Commit-Lint merge disable` will desactivate merge dependabot PR - `@Commit-Lint review` will approve dependabot PR - `@Commit-Lint stop review` will stop approve dependabot PR

frederickfogerty commented 2 years ago

Gday @rahulk94, thanks so much for the PR. At first glance the code looks good, but in principle we are very likely not to merge this. Up to this point, we have taken a hard-line stance to not support secureURLToken in our client-libraries. As Ember, React, Vue are client-libraries, it would require this token to be transmitted to the client. Then, a malicious user could take that token and user your imgix account to render images that they wanted, causing your bill to increase. If your source is a web proxy, they could proxy any image on the internet!

I hope you can see why we cannot support this feature in our client libraries. If there was a way to reliably ensure that this token was only used on the server (e.g. for static sites), then we could support this. But unfortunately this is not possible as far as we are aware. If you are using Ember for SSG, then obviously you are welcome to extend this library (like you have done) and enable this functionality.

rahulk94 commented 2 years ago

As Ember, React, Vue are client-libraries, it would require this token to be transmitted to the client

Huh, sweet yeah that makes sense.

Is there any recommendations on how the secureURLToken should be used for single page apps? We currently do use server side rendering so our first image loads will use the token server side, but then how would you recommend we go about doing this client side (or is the recommendation "don't" 😂 ) ?

frederickfogerty commented 2 years ago

Is there any recommendations on how the secureURLToken should be used for single page apps? We currently do use server side rendering so our first image loads will use the token server side, but then how would you recommend we go about doing this client side (or is the recommendation "don't" 😂 ) ?

There is no general recommendation for this since in each case it probably needs to be tailored to your use case. For you I would probably just recommend doing the server side rendering with a modified version of the SDK (that you have already done), and then just don't do anything client side. If you needed to do something client side, you can do something like this that we suggested for a react-imgix user

I'll close this now since I think we've reached agreement about this PR. Happy to re-open or discuss more if necessary

rahulk94 commented 2 years ago

Thanks for the info @frederickfogerty!

imgix / ember-cli-imgix