Fix: media library image signedUrl cache busting issue

[thanos982]

What does it do?

Adds a new property called isUrlSigned to the admin upload file object, within the signFileUrls method. This property is later utilised in the ImageAssetCard component to determine whether cache-busting is required for the thumbnail URL.

Why is it needed?

Currently, when using the AWS S3 provider with AWS Signature Version 4 authentication with query params, images in the Asset Media Library appear broken. This is due to a "SignatureDoesNotMatch" error caused by the presence of the "updatedAt" parameter, which is used for cache busting. This error occurs because the "updatedAt" parameter is not included in the canonical URL for generating the AWS S3 signature.

This pull request adds the updatedAt cache busting parameter only when the thumbnail URLs are not signed, which fixes the issue.

How to test it?

• Configure media library S3 provider to use v4 authentication, then upload an image in the media library. The image shouldn't appear broken.
• Add images with and without the v4 S3 provider. Check the thumbnail urls - only the ones uploaded without S3 should have the ?updatedAt query param.

Related issue(s)/PR(s)

Related to #18443

[strapi-cla]

All committers have signed the CLA.

[Fryuni]

Tested locally and it fixes the problem for us :)

[joshuaellis]

@Marc-Roig can you review this change please 👍🏼

[Marc-Roig]

looks good to me,
my question about this is, is there another way of not appending an updated by attribute in the image url?

This is a fix that might work for s3 signed urls, but many other providers might fail if you append invalid attributes like that.

[thanos982]

@Marc-Roig I may be wrong but isn't the signFileUrls method of this file the default method which retrieves the signed urls from any (default or custom) provider? If it is, then the isUrlSigned property will be set correctly, regardless of which the current provider is.

[Marc-Roig]

what I mean is that other providers, without using any url signing, could fail loading an image with an "updatedAt" parameter .

this is completely unrelated to your work, rather to the fact that we inject the udpatedAt parameter into the image url !

[Marc-Roig]

Indeed, so far we have only seen this issue when using signed urls.

[kevinvugts]

@Marc-Roig I may be wrong but isn't the signFileUrls method of this file the default method which retrieves the signed urls from any (default or custom) provider? If it is, then the isUrlSigned property will be set correctly, regardless of which the current provider is.

Indeed, this is happening right now for the azure blob storage strapi upload plugin. It's causing double "?" parameters in the url and causes 403 forbidden signed errors.

[Marc-Roig]

@kevinvugts can you open another issue (or share it if you have already done this)
cc @joshuaellis

[joshuaellis]

I'm happy with the FE change, I need Marc to approve too though.

[kevinvugts]

This pull request is currently causing issues when using uploads with @strapi

When uploading images with a sass token to Azure Blob Storage using the strapi-provider-upload-azure-storage plugin this will cause a double "?" parameter in the url which is causing a 403 forbidden (signature invalid) issue on the Azure side.

This is caused by the new cacheBusting feature that you guys introduced to prevents the browser from caching the URL for all sizes in combination with react-query.

To be specific I have added the code below. This should be fixed or at least there should be a way to opt out of this.
File: https://github.com/strapi/strapi/blob/develop/packages/core/upload/admin/src/components/AssetCard/ImageAssetCard.jsx

import { CardAsset } from '@strapi/design-system';
import PropTypes from 'prop-types';

import { AssetCardBase } from './AssetCardBase';

export const ImageAssetCard = ({ height, width, thumbnail, size, alt, ...props }) => {
  // Prevents the browser from caching the URL for all sizes and allow react-query to make a smooth update
  // instead of a full refresh
  const urlWithCacheBusting = props.updatedAt ? `${thumbnail}?${props.updatedAt}` : thumbnail;

  return (
    <AssetCardBase {...props} subtitle={height && width && ` - ${width}✕${height}`} variant="Image">
      <CardAsset src={urlWithCacheBusting} size={size} alt={alt} />
    </AssetCardBase>
  );
};

ImageAssetCard.defaultProps = {
  height: undefined,
  width: undefined,
  selected: false,
  onEdit: undefined,
  onSelect: undefined,
  onRemove: undefined,
  size: 'M',
  updatedAt: undefined,
};

ImageAssetCard.propTypes = {
  alt: PropTypes.string.isRequired,
  extension: PropTypes.string.isRequired,
  height: PropTypes.number,
  name: PropTypes.string.isRequired,
  onEdit: PropTypes.func,
  onSelect: PropTypes.func,
  onRemove: PropTypes.func,
  width: PropTypes.number,
  thumbnail: PropTypes.string.isRequired,
  selected: PropTypes.bool,
  size: PropTypes.oneOf(['S', 'M']),
  updatedAt: PropTypes.string,
};