secret is not put in the `TransferRequestMessage` (for legacy signaling flows)

[ndr-brt]

Bug Report

Describe the Bug

Currently the VaultDataAddressStore is reading a "secret" from the vault (based on the keyName) and puts it in the DataAddress when a TransferRequestMessage needs to be dispatched:

Connector/core/control-plane/control-plane-transfer/src/main/java/org/eclipse/edc/connector/controlplane/transfer/dataaddress/VaultDataAddressStore.java

Lines 118 to 122 in 64cadb7

	var dataDestination = Optional.ofNullable(originalDestination)
	.map(DataAddress::getKeyName)
	.map(key -> vault.resolveSecret(transferProcess.getParticipantContextId(), key))
	.map(secret -> originalDestination.toBuilder().property(EDC_DATA_ADDRESS_SECRET, secret).build())
	.orElse(originalDestination);

problem is that this happens only when the address is not stored in the vault, so, for new transfers, "never", so potentially all the PUSH transfers could be broken since version 0.16.0 (verified in downstream project mds-edc: PR).

note that this fix is necessary only for legacy signaling data flow, with the new DPS the DataAddress is completely managed by the DataPlane and it will already bring the related secrets with it.

Expected Behavior

if "keyName" is set, the secret is fetched from the vault in any case.

Context Information

• EDC 0.16.0
• current main as well

Possible implementation

when a DataAddress is stored, if it contains the keyName property, look up for the secret in the vault, if it exists, put it in the DataAddress and remove the keyName, then store the DataAddress in the vault.

This way, the secrets will always be stored together with the DataAddress securely in the vault

[ndr-brt]

Workaround fix: override the VaultDataAddressStore service with this implementation:

class VaultDataAddressStorePatch extends VaultDataAddressStore {

    private final Vault vault;

    public VaultDataAddressStorePatch(Vault vault, TypeTransformerRegistry typeTransformerRegistry, JsonLd jsonLd,
                                      DataPlaneProtocolInUse dataPlaneProtocolInUse) {
        super(vault, typeTransformerRegistry, jsonLd, dataPlaneProtocolInUse);
        this.vault = vault;
    }

    @Override
    public StoreResult<Void> store(DataAddress dataAddress, TransferProcess transferProcess) {
        var toBeStored = loadSecretFromTheVault(dataAddress, transferProcess);

        return super.store(toBeStored, transferProcess);
    }

    @Override
    public StoreResult<DataAddress> resolve(TransferProcess transferProcess) {
        return super.resolve(transferProcess)
                .map(dataAddress -> loadSecretFromTheVault(dataAddress, transferProcess));
    }

    private @NotNull DataAddress loadSecretFromTheVault(DataAddress dataAddress, TransferProcess transferProcess) {
        return Optional.of(dataAddress).map(DataAddress::getKeyName)
                .map(keyName -> vault.resolveSecret(transferProcess.getParticipantContextId(), keyName))
                .map(secret -> {
                    var properties = new HashMap<>(dataAddress.getProperties());
                    properties.remove(DataAddress.EDC_DATA_ADDRESS_KEY_NAME);
                    properties.put(DataAddress.EDC_DATA_ADDRESS_SECRET, secret);
                    return DataAddress.Builder.newInstance().properties(properties).build();
                })
                .orElse(dataAddress);
    }

}

that will take care to store the secret in the DataAddress given the contained keyName.
The same logic gets applied on resolve to support addresses that are already stored without secret.

NOTE: this workaround is only needed if you are used the deprecated legacy signaling data plane protocol.