Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix BPNL Group Validation Operator's Logic #1665

Open
bmg13 opened this issue Nov 8, 2024 · 0 comments · May be fixed by #1673
Open

Fix BPNL Group Validation Operator's Logic #1665

bmg13 opened this issue Nov 8, 2024 · 0 comments · May be fixed by #1673
Labels
bug Something isn't working triage all new issues awaiting classification

Comments

@bmg13
Copy link
Contributor

bmg13 commented Nov 8, 2024

Describe the bug

Regarding Policies, the operators used in the BPN Group validation do not seem to match to their desired behaviour.

  • For the opertator isAllOf, if a BPN is assigned to groups A, B and C and a Policy is created allowing access to A and B the validation will fail and it shouldn't since the assigned groups are all present in the Policy constraint.
  • For the operator isPartOf, the validation is done considering that all assigned groups for a BPNL are contained in the Policy constraint. It should validate if any of the allowed groups in the Policy are present in the ones assigned to the BPNL, similar to isAnyOf.

To Reproduce

Add a testing BPNL to 3 dummy groups and create a Policy with isAllOf for 2 of the groups to which BPNL is assigned, create a Contract Definition and fetch the catalog.

Expected behavior

Dividing the expected behaviour per incorrect operator:

  • isAllOf
    The validation should be done of assigned groups in the BPNL include all allowed in the Policy.

  • isPartOf
    Instead of being the same validation of isAllOf, should be the same as isAnyOf.

Regarding the equals (eq) and not equals (neq) operators these seem to not make much sense here. They appear to not add needed (additional) validation since the isAllOf, isAnyOf and isNoneOf should cover all the possibilities eq and neq provide. So, here is added the suggestion to remove these two from the BPN group validation (not removed for comparison equality of BPNL's when groups are not considered).

Possible Implementation

Proposed solution would be updating the BusinessPartnerGroupFunction by:

  • remove eq and neq as available operators;
  • make the IN (isPartOf) operator to call the evaluateIsAnyOf() instead;
  • create a method for isAllOf (named evaluateIsAllOf()) responsible for the ensuring all the Policy allowed groups are included in the BPNL assigned ones (and not the other way around);
  • update the BusinessPartnerGroupFunctionTest tests, accordingly.
@bmg13 bmg13 added bug Something isn't working triage all new issues awaiting classification labels Nov 8, 2024
@bmg13 bmg13 linked a pull request Nov 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage all new issues awaiting classification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant