Skip to content

UploaderStrategy still requires api_secret if parameter was already added externally #125

Description

@amirulzin

This issue affects all current http artifacts:

if (requiresSigning(action, options)) {
uploader.signRequestParams(params, options);
} else {
Util.clearEmpty(params);
}

Compared to the implementation on cloudinary-android:

if (requiresSigning(action, options)) {
    String apiKey = ObjectUtils.asString(options.get("api_key"), this.cloudinary().config.apiKey);
    if (apiKey == null)
        throw new IllegalArgumentException("Must supply api_key");
    if (options.containsKey("signature") && options.containsKey("timestamp")) {
        params.put("timestamp", options.get("timestamp"));
        params.put("signature", options.get("signature"));
        params.put("api_key", apiKey);
    } else {
        String apiSecret = ObjectUtils.asString(options.get("api_secret"), this.cloudinary().config.apiSecret);
        if (apiSecret == null)
            throw new IllegalArgumentException("Must supply api_secret");
        params.put("timestamp", Long.valueOf(System.currentTimeMillis() / 1000L).toString());
        params.put("signature", this.cloudinary().apiSignRequest(params, apiSecret));
        params.put("api_key", apiKey);
    }
}

Cloudinary Android Source: https://github.com/cloudinary/cloudinary_android/blob/f7a0b32cfd9f2b6507bb6461043e6c89d9478c03/lib/src/main/java/com/cloudinary/android/UploaderStrategy.java#L43-L59

My suggestion is to implement that for each http artifact or simply refactor the android implementation upwards to the core AbstractUploaderStrategy.

This allows cloudinary-java users to not have to enter api_secret if say the signature is generated via other microservices/authorization servers. This also allows easier server-side integration tests.

While I understand the audience for cloudinary-java is more towards server users where the secret key is most likely exposed within a monolithic service, the two reasons above are currently forcing us to mangle a separate UploaderStrategy which simply use the above Android part instead.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions