ffraenz / private-composer-installer

Composer install helper outsourcing sensitive keys from the package URL into environment variables
MIT License
227 stars 16 forks source link

Add support for intermediary URLs such as EDD #49

Open mcaskill opened 3 months ago

mcaskill commented 3 months ago

Fix ffraenz/private-composer-installer#22 by allowing a package URL indirection to take place.

Continuation of ffraenz/private-composer-installer#45 to implement suggested changes.

Summary of suggested changes by @ffraenz to implement:

Additional features:

Example:

"indirection": {
    "http": {
        "method": "POST"
    },
    "ssl": {
        "passphrase": "{%PACKAGE_SSL_PW}"
    },
    "parse": {
        "format": "json",
        "download_key": "data.0.download_url",
        "version_key": "data.0.version"
    }
}

[!NOTE]
[2024-08-09] Feature branch has been unit tested but has not been tested yet with an actual indirect package.

[!IMPORTANT] [2024-08-06] This pull request's branch is based on mcaskill:feature/maintenance #50 in order to work.

mcaskill commented 2 months ago

This pull request is ready for review and testing.

Below are my responses to @ffraenz's comments on the previous pull request.


[…]

What I have in mind is adding a hash suffix #config-1 to the dist URL that we may use to target a specific configuration in extra.private-composer-installer.configs (for lack of a better name). This way multiple packages (from the same vendor) could reuse the same config. I would keep a config generic to allow for future additions that may be unrelated to an indirection feature.

[…]

A possible issue with a hash suffix for the preset is that a hash suffix might already be defined or will be appended by fulfillVersionPlaceholder()'s fallback.

Example: https://example.com/api/download?v=foo#preset-1#v1.2.3

I have not tested the integrity of cache keys and the lock file with preset fragments.


I don't like the way you provide the key location in the JSON payload using an object structure. I would go with a simple dot-notation string e.g. foo.bar.0.download_url to specify it.

Resolved.


Keep in mind that dist URLs without any placeholders should be ignored by this plugin to not alter unrelated packages. An indirection call must only be made if an indirection is actually configured (I think that's not the case right now in your code).

The proposed changes expects the inline package to either:

ffraenz commented 1 month ago

@mcaskill Thank you so much for your work! I will come back to you and review it ASAP.

mcaskill commented 1 month ago

I just remembered a missing requirement to make this feature reliable: validate that the intermediary's version matches the author's required version in the package.

For example, EasyDigitalDownloads and Gravity Forms only ever offer the most recent version of their WordPress plugin.

The proposed parse.key should be renamed to parse.download_key and a parse.version_key be added.

We did this junaidbhura/composer-wp-pro-plugins to avoid downloading a newer version and caching it under the older version's key. For that Composer plugin, we added a requirement on composer/semver. We would have to see if we might be able to get away with just version_compare(). We would need the same requirement to ensure we can match the version formats and variations between author and vendor.

mcaskill commented 1 month ago

I've done some additional work today to add support for a version_key, support for unserialize() (Gravity Forms uses serialize() for its response), and a clean-up of tests. I'll try to push these changes tomorrow.

mcaskill commented 1 month ago

I've deployed the latest changes and this should be ready for review and advanced testing. I have not tested them in a real world scenario yet.

The decrease in coverage reported by Coveralls is because only the report for Composer v1 is submitted. This new feature is limited to Composer v2.