> On Nov. 20, 2024, 4:01 p.m., Madhan Neethiraj wrote:
> > agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
> > Lines 112 (patched)
> > <https://reviews.apache.org/r/75280/diff/1/?file=2294734#file2294734line112>
> >
> > - POLICY_VALIDATION_ERR_NULL_POLICY_ITEM_ACCESS_TYPE =>
> > POLICY_VALIDATION_ERR_INVALID_ACCESS_TYPE
> > - "Invalid access type: [{0}]"
We already have an error message to handle invalid access types:
POLICY_VALIDATION_ERR_POLICY_ITEM_ACCESS_TYPE_INVALID(3022, "Invalid access
type: access type=[{0}], valid access types=[{1}]")
However, to handle cases where the access type is missing (e.g., with the
following payload):
"accesses": [
{
"isAllowed": true
}
]
I felt that the existing error message might not be appropriate, as it
specifically addresses invalid access types. To better reflect the nature of
the issue, I created a new error message to handle missing access types
explicitly.
If you think using the existing "Invalid access type" message would still be
more suitable or preferable for consistency, please let me know, and I’ll
update the implementation accordingly.
> On Nov. 20, 2024, 4:01 p.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/validation/RangerGdsValidator.java
> > Lines 882 (patched)
> > <https://reviews.apache.org/r/75280/diff/1/?file=2294736#file2294736line882>
> >
> > if (CollectionUtils.isNotEmpty(policyItem.getAccesses())) {
> > for (RangerPolicyItemAccess itemAccess : policyItem.getAccesses()) {
> > if (itemAccess == null) {
> > addValidationFailure(result,
> > ValidationErrorCode.POLICY_VALIDATION_ERR_NULL_POLICY_ITEM_ACCESS);
> > } else {
> > if (StringUtils.isEmpty(itemAccess.getType())) {
> > addValidationFailure(result,
> > ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_ACCESS_TYPE,
> > itemAccess.getType());
> > }
> > }
> > }
> > }
The condition if(CollectionUtils.isEmpty(policyItem.getAccesses()) ||
policyItem.getAccesses().contains(null)) checks for the following scenarios:
Empty Access List or List with Null values:
"accesses": []
or
"accesses": [null]
If this condition is satisfied, an additional check is performed to ensure that
each Access object within the list has a valid, non-empty access type.
If the logic needs to be updated to incorporate the provided code, an else
condition must also be included. This might make the implementation little
harder to follow.
if (CollectionUtils.isNotEmpty(policyItem.getAccesses())) {
for (RangerPolicyItemAccess itemAccess : policyItem.getAccesses()) {
if (itemAccess == null) {
addValidationFailure(result,
ValidationErrorCode.POLICY_VALIDATION_ERR_NULL_POLICY_ITEM_ACCESS);
} else {
if (StringUtils.isEmpty(itemAccess.getType())) {
addValidationFailure(result,
ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_ACCESS_TYPE,
itemAccess.getType());
}
}
}
} else {
addValidationFailure(result,
ValidationErrorCode.POLICY_VALIDATION_ERR_NULL_POLICY_ITEM_ACCESS);
}
- Radhika
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/75280/#review227076
-----------------------------------------------------------
On Nov. 20, 2024, 2:56 p.m., Radhika Kundam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/75280/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2024, 2:56 p.m.)
>
>
> Review request for ranger, Madhan Neethiraj and Ramesh Mani.
>
>
> Bugs: RANGER-5000
> https://issues.apache.org/jira/browse/RANGER-5000
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Dataset policy creation works even when the policy items are not properly
> formed in the policy.With this patch it'll validate policy items while
> creating policy. And below are the validations included.
>
> UseCase-1: Null policy items
> "policyItems": [
> null
> ]
> Error Msg: "msgDesc": "[ Validation failure: error code[3019], reason[policy
> items object was null], field[policy items], subfield[null], type[]]",
>
> UseCase-2: No principals or Principals object with empty or no values
> "policyItems": [
> {
> "delegateAdmin": false,
> "accesses": [
> {
> "type": "_ALL",
> "isAllowed": null
> }
> ]
> }
> ]
>
> "policyItems": [
> {
> "delegateAdmin": false,
> "accesses": [
> {
> "type": "_ALL",
> "isAllowed": null
> }
> ],
> "users": [
> " "
> ]
> }
>
> "policyItems": [
> {
> "delegateAdmin": false,
> "accesses": [
> {
> "type": "_ALL",
> "isAllowed": null
> }
> ],
> "users": [
>
> ]
> }
> ]
> Error Msg: "msgDesc": "[ Validation failure: error code[3020], reason[All of
> users, user-groups and roles collections on the policy item were
> null/empty], field[policy items], subfield[null], type[]]"
>
>
> UseCase-3: Access object with no or empty access type
>
> "policyItems": [
> {
> "delegateAdmin": false,
> "accesses": [
> {
> "isAllowed": true
> }
> ],
> "users": [
> "hive"
> ]
> }
> ]
>
> "policyItems": [
> {
> "delegateAdmin": false,
> "accesses": [
> {
> "type": " ",
> "isAllowed": true
> }
> ],
> "users": [
> "hive"
> ]
> }
> ]
> Error Msg: "msgDesc": "[ Validation failure: error code[4010], reason[policy
> items access object has empty or null values for type], field[policy items],
> subfield[null], type[]]"
>
> Note: No validation exists for isAllowed being null, as it will default to
> true if isAllowed is passed as NULL.
>
>
> Diffs
> -----
>
>
> agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
> 13a362437
> security-admin/src/main/java/org/apache/ranger/biz/GdsDBStore.java
> 768192e84
>
> security-admin/src/main/java/org/apache/ranger/validation/RangerGdsValidator.java
> c5d8200fc
>
>
> Diff: https://reviews.apache.org/r/75280/diff/1/
>
>
> Testing
> -------
>
> Tested locally.
>
>
> Thanks,
>
> Radhika Kundam
>
>
