eclipse-tractusx / sig-release

https://eclipse-tractusx.github.io/sig-release
Apache License 2.0
9 stars 10 forks source link

EDC - Add “isNoneOf” as additional implemented operand for BusinessPartnerNumber checks #775

Closed lgblaumeiser closed 3 months ago

lgblaumeiser commented 4 months ago

Requestor: @wern

Description

It is currently possible to add a BusinessPartnerNumberConstraint to AccessPolicies or even UsagePolicies (aka. ContractPolicies). However, so far only one operator, namely “eq”, is supported here (see org.eclipse.tractusx.edc.validation.businesspartner.functions.BusinessPartnerNumberPermissionFunction):

{
    "@context": {
        "@vocab": "https://w3id.org/edc/v0.0.1/ns/"
    },
    "@id": "7cb20eb1-08db-4532-bd74-ad480f551654",
    "policy": {
        "@context": [
            "https://www.w3.org/ns/odrl.jsonld",
            {
                "cx-policy": "https://w3id.org/catenax/policy/v1.0.0/"
            }
        ],
        "@type": "Policy",
        "profile": "cx-policy:profile2405",
        "permission": [
            {
                "action": "use",
                "constraint": {
                    "leftOperand": "BusinessPartnerNumber",
                    "operator": "eq",
                    "rightOperand": "BPNL00000000XXXX"
                }
            }
        ]
    }
}

What we need here is the ability to define access for ANY OTHER than the BPNs mentioned. In our case: all BPNs that have no biletaral contracts. Having the ability to formulate this in a “negative” way would ease our administration efforts extremely. The additional implementation of the operator “isNoneOf” would be needed to achieve this:

{
    "@context": {
        "@vocab": "https://w3id.org/edc/v0.0.1/ns/"
    },
    "@id": "7cb20eb1-08db-4532-bd74-ad480f551654",
    "policy": {
        "@context": [
            "https://www.w3.org/ns/odrl.jsonld",
            {
                "cx-policy": "https://w3id.org/catenax/policy/v1.0.0/"
            }
        ],
        "@type": "Policy",
        "profile": "cx-policy:profile2405",
        "permission": [
            {
                "action": "use",
                "constraint": {
                    "leftOperand": "BusinessPartnerNumber",
                    "operator": "isNoneOf",
                    "rightOperand": [ "BPNL00000000XXXX", "BPNL00000000YYYY", "BPNL00000000ZZZZ" ]
                }
            }
        ]
    }
}

Impact

A simple addon, impact is basically locally and potentially with components that support policy definitions

Additional information

wern commented 4 months ago

@lgblaumeiser thanks for raising this issue!

stephanbcbauer commented 4 months ago

Presented in the DRAFT Feature Freeze -> Committer is available

paullatzelsperger commented 4 months ago

supporting additional operators in the BPN permission functions, isNoneOf amongst others, is probably a good idea, if only for the sake of feature completeness. In fact, there is an issue about it (https://github.com/eclipse-tractusx/tractusx-edc/issues/1426).

However, the same behaviour can already be achieved by using the BusinessPartnerGroupFunction (documentation). It is the recommended way to define constraints on BPNs, because it does not require updating the policy when the group of BPNs is to be changed.

Specifically, to define an exclusion group, one would simply assign that group to each of the affected BPNs:

curl -X POST <BASEURL>/business-partner-groups \
-d '{
  "@context": {
    "tx": "https://w3id.org/tractusx/v0.0.1/ns/"
  },
  "@id": "tx:BPN000001234",
  "tx:groups": [
    "exclusion_group"
  ]
}' \
-H "x-api-key: supersecure"

and then declare a policy constraint like this:

{
  "@type": "http://www.w3.org/ns/odrl/2/Constraint",
  "leftOperand": "https://w3id.org/tractusx/v0.0.1/ns/BusinessPartnerGroup",
  "operator": "http://www.w3.org/ns/odrl/2/neq",
  "rightOperand": "exclusion_group"
}

then, when more BPNs are to be excluded in the future, it is sufficient to assign them the exclusion_group and that's it.

lgblaumeiser commented 4 months ago

@wern Can you comment on Pauls previous comment? In general, there is already everything in place to support your functional need. And actually, using groups is potentially the better way to handle this, as in your proposal you request to address a whole group of BPNs in the policy atom.

saschagr commented 4 months ago

Instead of Werner, I answer. We will use Pauls suggestion.

@paullatzelsperger Thanks for your proposal.

saschagr commented 3 months ago

@paullatzelsperger @lgblaumeiser @wern We had implemented our feature with BusinessPartnerGroup, but it doesn't work, because there are some implementation details(i think bugs) in org.eclipse.tractusx.edc.validation.businesspartner.functions.BusinessPartnerGroupFunction#evaluate which make problems

https://github.com/eclipse-tractusx/tractusx-edc/blob/07cdb518905cfe8568655ac043bf2c309e8debf0/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerGroupFunction.java#L124

    var bpn = participantAgent.getIdentity();
    var groups = store.resolveForBpn(bpn);

    // BPN not found in database
    if (groups.failed()) {
        policyContext.reportProblem(groups.getFailureDetail());
        return false;
    }
    var assignedGroups = groups.getContent();

    // BPN was found, but it does not have groups assigned.
    if (assignedGroups.isEmpty()) {
        policyContext.reportProblem("No groups were assigned to BPN " + bpn);
        return false;
    }

    // right-operand is anything other than String or Collection
    var rightOperand = parseRightOperand(rightValue, policyContext);
    if (rightOperand == null) {
        return false;
    }

If the bpn has no groups, the first if will result to false, which is not correct for org.eclipse.edc.policy.model.Operator#NEQ and other cases (use of InMemoryBusinessPartnerStore, i think the second implementation SqlBusinessPartnerStore behave the same, not tried, if not the second if came to the same wrong result)

paullatzelsperger commented 3 months ago

@saschagr could you file a bug report in the Tx-EDC repo?

lgblaumeiser commented 3 months ago

Issue found below is addressed in https://github.com/eclipse-tractusx/tractusx-edc/issues/1411, as a solution exists, this is closed