You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by "fmorg-git (via GitHub)" <gi...@apache.org> on 2026/06/03 22:54:57 UTC

[PR] Ranger 5628 - Support for actions in Ozone Service Definition [ranger]

fmorg-git opened a new pull request, #995:
URL: https://github.com/apache/ranger/pull/995

   ## What changes were proposed in this pull request?
   For upcoming Ozone STS feature, we need the ability to identify what actions are allowed for an STS token (ex GetObject, GetObjectTagging, PutObject, etc), in addition to the more granular existing permission system (read, write, create, etc). This ticket updates the Ranger policy evaluation to keep the existing permission evaluation for legacy reasons, and add optional action evaluation via policy condition in the UI. If the resource policy has an action and an action is supplied in the RequestContext, they must match (in addition to the permissions). If no action is identified in the resource policy, then all actions are allowed and only the permissions are required.
   
   Also there are several UI updates:
   
   Support actions in UI in only in Policy Conditions section, not on the overall Resource Policy.
   populate Ozone action choices based on permissions
   ensure actions wrap around instead of scrolling horizontally indefinitely.
   It also fixes latent React hook violations of having useState within a conditional (CommonComponents.jsx) and a loop (Editable.jsx).
   The UI updates are implemented in a way such that the hdfs service definition can later make use of the filtering actions by permissions behavior.
   
   https://issues.apache.org/jira/browse/RANGER-5628
   
   
   ## How was this patch tested?
   manual testing locally in Ranger docker, also end-to-end smoke tests locally with Ozone and Ranger
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3464043050


##########
security-admin/src/main/webapp/react-webapp/src/views/PolicyListing/PolicyPermissionItem.jsx:
##########
@@ -105,15 +107,53 @@ export default function PolicyPermissionItem(props) {
   };
 
   const grpResourcesKeys = useMemo(() => {
-    const { resources = [] } = serviceCompDetails;
+    const { resources = [] } = serviceCompDetails || {};
     const grpResources = groupBy(resources, "level");
     let grpResourcesKeys = [];
     for (const resourceKey in grpResources) {
       grpResourcesKeys.push(+resourceKey);
     }
     grpResourcesKeys = grpResourcesKeys.sort();
     return grpResourcesKeys;
-  }, []);
+  }, [serviceCompDetails?.resources]);
+
+  // Narrow dependency: only recompute when resource selection fields change,
+  // not on every keystroke in user/group/permission fields.
+  const resourceBlocks = Array.isArray(formValues?.additionalResources)
+    ? formValues.additionalResources
+    : [formValues];
+
+  const serializedResources = JSON.stringify(
+    resourceBlocks.map((b) =>
+      grpResourcesKeys.map((level) => b?.[`resourceName-${level}`]?.name || null)
+    )
+  );

Review Comment:
   updated - 47ac69e22cba9d41f2d6072f01a9989592140210.  It removes the `JSON.stringify()` altogether.



##########
security-admin/src/main/webapp/react-webapp/src/views/PolicyListing/PolicyPermissionItem.jsx:
##########
@@ -105,15 +107,53 @@ export default function PolicyPermissionItem(props) {
   };
 
   const grpResourcesKeys = useMemo(() => {
-    const { resources = [] } = serviceCompDetails;
+    const { resources = [] } = serviceCompDetails || {};
     const grpResources = groupBy(resources, "level");
     let grpResourcesKeys = [];
     for (const resourceKey in grpResources) {
       grpResourcesKeys.push(+resourceKey);
     }
     grpResourcesKeys = grpResourcesKeys.sort();

Review Comment:
   updated - 47ac69e22cba9d41f2d6072f01a9989592140210



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3399302533


##########
security-admin/src/main/webapp/react-webapp/src/hooks/usePruneStaleConditions.js:
##########
@@ -0,0 +1,145 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Keeps policyItems[].conditions["action-matches"] in sync when permissions or
+ * resources change. Mounted from PolicyPermissionItem; does not touch ip-range
+ * or other per-row conditions.
+ */
+
+import { useEffect } from "react";
+import { isArray, find, isEqual } from "lodash";
+import {
+  getSelectedAccessTypesForRow,
+  getAllowedActionMatchesForCondition,
+  isPerRowCondition,
+  parseConditionUiHint
+} from "Utils/policyConditionUtils";
+
+/**
+ * Stable dependency string: only permission accesses and per-row condition values.
+ * Avoids re-running when unrelated form fields (user/group names, etc.) change.
+ */
+const serializePruneDeps = (formValues, attrName) => {
+  const items = formValues?.[attrName];
+  if (!Array.isArray(items)) {
+    return "[]";
+  }
+  return JSON.stringify(
+    items.map((item, index) => ({
+      accesses: getSelectedAccessTypesForRow(formValues, attrName, index),
+      actionMatches: item?.conditions?.["action-matches"] ?? null
+    }))
+  );
+};
+
+export const usePruneStaleConditions = ({
+  formValues,
+  attrName,
+  form,
+  leafResourceTypes,
+  serviceCompDetails,
+  conditionDefVal,
+  actionReqsMap
+}) => {
+  const serializedPruneDeps = serializePruneDeps(formValues, attrName);
+
+  useEffect(() => {
+    if (!formValues[attrName] || !isArray(formValues[attrName])) {
+      return;
+    }

Review Comment:
   updated with a more robust fix: 2a5453566d3f6c7d87a0a4803fda567b7f4a0a54



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3398371434


##########
security-admin/src/main/webapp/react-webapp/src/views/PolicyListing/PolicyPermissionItem.jsx:
##########
@@ -466,10 +500,24 @@ export default function PolicyPermissionItem(props) {
                                         {...input}
                                         placement="auto"
                                         type="custom"
-                                        conditionDefVal={policyConditionUpdatedJSON(
-                                          serviceCompDetails.policyConditions
-                                        )}
-                                        selectProps={{ isMulti: true }}
+                                        conditionDefVal={conditionDefVal}
+                                        servicedefName={serviceCompDetails?.name}
+                                        actionFilterContext={{
+                                          selectedAccessTypes:
+                                            getSelectedAccessTypesForRow(formValues, attrName, index),
+                                          leafResourceTypes,
+                                          accessTypeDefs:
+                                            serviceCompDetails?.accessTypes
+                                        }}
+                                        selectProps={{
+                                          isMulti: true,
+                                          // Portal menus to body (see Editable CONDITION_POPOVER_SELECT_PROPS)
+                                          // so a long Action list with ALL permission does not jump the page.
+                                          menuPortalTarget: document.body,

Review Comment:
   `menuPortalTarget: document.body` will throw a ReferenceError in non-browser environments (SSR/unit tests) and also overrides Editable’s guarded default. Guard the `document` reference (or omit `menuPortalTarget` here and let Editable’s CONDITION_POPOVER_SELECT_PROPS handle it).



##########
pom.xml:
##########
@@ -892,6 +892,11 @@
             <name>jetbrains-intellij-dependencies</name>
             <url>https://packages.jetbrains.team/maven/p/ij/intellij-dependencies</url>
         </repository>
+        <repository>
+            <id>apache-ozone-staging</id>
+            <name>Apache Ozone 2.1.1 RC0 Staging</name>
+            <url>https://repository.apache.org/content/repositories/orgapacheozone-1062/</url>
+        </repository>

Review Comment:
   The root POM adds an Apache repository URL that appears to be a staging/RC repository (`repository.apache.org/content/repositories/...`). Depending on an RC/staging repo in the default build is brittle (repo can be removed or artifacts can change), and it affects all downstream consumers/build environments.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3398439702


##########
pom.xml:
##########
@@ -892,6 +892,11 @@
             <name>jetbrains-intellij-dependencies</name>
             <url>https://packages.jetbrains.team/maven/p/ij/intellij-dependencies</url>
         </repository>
+        <repository>
+            <id>apache-ozone-staging</id>
+            <name>Apache Ozone 2.1.1 RC0 Staging</name>
+            <url>https://repository.apache.org/content/repositories/orgapacheozone-1062/</url>
+        </repository>

Review Comment:
   yes, this is temporary - we are waiting for the Ozone 2.1.1 upstream release which is currently in voting process.  The release candidate was temporarily added so the code can compile



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3464043050


##########
security-admin/src/main/webapp/react-webapp/src/views/PolicyListing/PolicyPermissionItem.jsx:
##########
@@ -105,15 +107,53 @@ export default function PolicyPermissionItem(props) {
   };
 
   const grpResourcesKeys = useMemo(() => {
-    const { resources = [] } = serviceCompDetails;
+    const { resources = [] } = serviceCompDetails || {};
     const grpResources = groupBy(resources, "level");
     let grpResourcesKeys = [];
     for (const resourceKey in grpResources) {
       grpResourcesKeys.push(+resourceKey);
     }
     grpResourcesKeys = grpResourcesKeys.sort();
     return grpResourcesKeys;
-  }, []);
+  }, [serviceCompDetails?.resources]);
+
+  // Narrow dependency: only recompute when resource selection fields change,
+  // not on every keystroke in user/group/permission fields.
+  const resourceBlocks = Array.isArray(formValues?.additionalResources)
+    ? formValues.additionalResources
+    : [formValues];
+
+  const serializedResources = JSON.stringify(
+    resourceBlocks.map((b) =>
+      grpResourcesKeys.map((level) => b?.[`resourceName-${level}`]?.name || null)
+    )
+  );

Review Comment:
   updated - 47ac69e22cba9d41f2d6072f01a9989592140210



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3464046090


##########
security-admin/src/main/webapp/react-webapp/src/hooks/usePruneStaleConditions.js:
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Keeps policyItems[].conditions["action-matches"] in sync when permissions or
+ * resources change. Mounted from PolicyPermissionItem; does not touch ip-range
+ * or other per-row conditions.
+ */
+
+import { useEffect } from "react";
+import { isArray, find, isEqual } from "lodash";
+import {
+  getSelectedAccessTypesForRow,
+  getAllowedActionMatchesForCondition,
+  isPerRowCondition,
+  parseConditionUiHint
+} from "Utils/policyConditionUtils";
+
+/**
+ * Stable dependency string: only permission accesses and per-row condition values.
+ * Avoids re-running when unrelated form fields (user/group names, etc.) change.
+ */
+const serializePruneDeps = (formValues, attrName) => {
+  const items = formValues?.[attrName];
+  if (!Array.isArray(items)) {
+    return "[]";
+  }
+  return JSON.stringify(
+    items.map((item, index) => ({
+      accesses: getSelectedAccessTypesForRow(formValues, attrName, index),
+      actionMatches: item?.conditions?.["action-matches"] ?? null
+    }))
+  );
+};
+
+export const usePruneStaleConditions = ({
+  formValues,
+  attrName,
+  form,
+  leafResourceTypes,
+  serviceCompDetails,
+  conditionDefVal,
+  actionReqsMap
+}) => {
+  const serializedPruneDeps = serializePruneDeps(formValues, attrName);
+
+  useEffect(() => {
+    const items = formValues?.[attrName];
+    if (!items || !isArray(items)) {
+      return;
+    }
+
+    let hasChanges = false;
+    const newItems = [...items];
+
+    newItems.forEach((item, index) => {
+      if (!item.conditions) {
+        return;
+      }
+
+      const accesses = getSelectedAccessTypesForRow(formValues, attrName, index);
+      if (accesses.length === 0) {
+        return;
+      }

Review Comment:
   updated - 47ac69e22cba9d41f2d6072f01a9989592140210.  This is a good catch - I didn't realize that if all the permissions were removed, the action stayed around.  This fixes it.



##########
security-admin/src/main/webapp/react-webapp/src/components/Editable.jsx:
##########
@@ -677,14 +776,41 @@ const Editable = (props) => {
   const handleApply = () => {
     let errors, uiHintVal;
     if (selectValRef?.current) {
+      // Re-validate action-matches against current row permissions before persisting.
       sortBy(Object.keys(selectValRef.current)).map((property) => {
         let conditionObj = find(conditionDefVal, function (m) {
           if (m.name == property) {
             return m;
           }

Review Comment:
   updated - 47ac69e22cba9d41f2d6072f01a9989592140210



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "rameeshm (via GitHub)" <gi...@apache.org>.
rameeshm commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3487215028


##########
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerActionListMatcher.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.ranger.plugin.util;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
+
+public class RangerActionListMatcher {
+    private final boolean     allowAnyAction;
+    private final Set<String> exactActions = new HashSet<>();
+    private final String[]    prefixActions;
+
+    public RangerActionListMatcher(Collection<String> actions) {
+        boolean            allowAny       = CollectionUtils.isEmpty(actions);
+        final List<String> tempPrefixList = new ArrayList<>();
+
+        if (!allowAny) {
+            for (String a : actions) {
+                final String action = StringUtils.trimToEmpty(a);
+
+                if (action.isEmpty()) {
+                    continue;
+                }
+
+                if (action.equals("*")) {
+                    allowAny = true;
+                    exactActions.clear();
+                    tempPrefixList.clear();
+                    break;
+                }
+
+                if (action.endsWith("*")) {
+                    final String prefix = StringUtils.trimToEmpty(action.substring(0, action.length() - 1)).toLowerCase(Locale.ROOT);
+
+                    if (prefix.isEmpty()) {
+                        allowAny = true;
+                        exactActions.clear();
+                        tempPrefixList.clear();
+                        break;
+                    }
+
+                    tempPrefixList.add(prefix);
+                } else {
+                    exactActions.add(action.toLowerCase(Locale.ROOT));
+                }
+            }
+        }
+
+        this.allowAnyAction = allowAny;
+
+        if (!tempPrefixList.isEmpty()) {
+            tempPrefixList.sort((s1, s2) -> Integer.compare(s1.length(), s2.length()));
+
+            List<String> optimizedPrefixes = new ArrayList<>();
+            for (String p : tempPrefixList) {
+                boolean isCovered = false;
+                for (String optPrefix : optimizedPrefixes) {
+                    if (p.startsWith(optPrefix)) {
+                        isCovered = true;
+                        break;
+                    }
+                }
+                if (!isCovered) {
+                    optimizedPrefixes.add(p);
+                }
+            }
+            this.prefixActions = optimizedPrefixes.toArray(new String[0]);
+        } else {
+            this.prefixActions = new String[0];
+        }
+    }
+
+    public boolean isMatch(String requestAction) {
+        boolean ret = allowAnyAction;
+
+        if (!ret) {
+            // if action is not available, don't enforce action restrictions
+            if (StringUtils.isBlank(requestAction)) {
+                ret = true;

Review Comment:
   @mneethiraj  I thought the permission should takecare of the authorization if the specific action is not mentioned.  Is it that action should be there always and if not present is should deny? In normal authorization use case if no conditions are present we mark is as allowed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
mneethiraj commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3485951128


##########
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerActionListMatcher.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.ranger.plugin.util;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
+
+public class RangerActionListMatcher {
+    private final boolean     allowAnyAction;
+    private final Set<String> exactActions = new HashSet<>();
+    private final String[]    prefixActions;
+
+    public RangerActionListMatcher(Collection<String> actions) {
+        boolean            allowAny       = CollectionUtils.isEmpty(actions);
+        final List<String> tempPrefixList = new ArrayList<>();
+
+        if (!allowAny) {
+            for (String a : actions) {
+                final String action = StringUtils.trimToEmpty(a);
+
+                if (action.isEmpty()) {
+                    continue;
+                }
+
+                if (action.equals("*")) {
+                    allowAny = true;
+                    exactActions.clear();
+                    tempPrefixList.clear();
+                    break;
+                }
+
+                if (action.endsWith("*")) {
+                    final String prefix = StringUtils.trimToEmpty(action.substring(0, action.length() - 1)).toLowerCase(Locale.ROOT);
+
+                    if (prefix.isEmpty()) {
+                        allowAny = true;
+                        exactActions.clear();
+                        tempPrefixList.clear();
+                        break;
+                    }
+
+                    tempPrefixList.add(prefix);
+                } else {
+                    exactActions.add(action.toLowerCase(Locale.ROOT));
+                }
+            }
+        }
+
+        this.allowAnyAction = allowAny;
+
+        if (!tempPrefixList.isEmpty()) {
+            tempPrefixList.sort((s1, s2) -> Integer.compare(s1.length(), s2.length()));
+
+            List<String> optimizedPrefixes = new ArrayList<>();
+            for (String p : tempPrefixList) {
+                boolean isCovered = false;
+                for (String optPrefix : optimizedPrefixes) {
+                    if (p.startsWith(optPrefix)) {
+                        isCovered = true;
+                        break;
+                    }
+                }
+                if (!isCovered) {
+                    optimizedPrefixes.add(p);
+                }
+            }
+            this.prefixActions = optimizedPrefixes.toArray(new String[0]);
+        } else {
+            this.prefixActions = new String[0];
+        }
+    }
+
+    public boolean isMatch(String requestAction) {
+        boolean ret = allowAnyAction;
+
+        if (!ret) {
+            // if action is not available, don't enforce action restrictions
+            if (StringUtils.isBlank(requestAction)) {
+                ret = true;

Review Comment:
   Shouldn't missing action field in the request be treated as "not a match", since the expectation is to match specified actions only?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3399305135


##########
security-admin/src/main/webapp/react-webapp/src/views/PolicyListing/PolicyPermissionItem.jsx:
##########
@@ -466,10 +500,24 @@ export default function PolicyPermissionItem(props) {
                                         {...input}
                                         placement="auto"
                                         type="custom"
-                                        conditionDefVal={policyConditionUpdatedJSON(
-                                          serviceCompDetails.policyConditions
-                                        )}
-                                        selectProps={{ isMulti: true }}
+                                        conditionDefVal={conditionDefVal}
+                                        servicedefName={serviceCompDetails?.name}
+                                        actionFilterContext={{
+                                          selectedAccessTypes:
+                                            getSelectedAccessTypesForRow(formValues, attrName, index),
+                                          leafResourceTypes,
+                                          accessTypeDefs:
+                                            serviceCompDetails?.accessTypes
+                                        }}
+                                        selectProps={{
+                                          isMulti: true,
+                                          // Portal menus to body (see Editable CONDITION_POPOVER_SELECT_PROPS)
+                                          // so a long Action list with ALL permission does not jump the page.
+                                          menuPortalTarget: document.body,

Review Comment:
   updated with a simpler fix: e6327b6bd039ef52d8c82e203be4d8c034214f09



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "rameeshm (via GitHub)" <gi...@apache.org>.
rameeshm commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3487201333


##########
agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerActionMatcher.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ranger.plugin.conditionevaluator;
+
+import org.apache.ranger.plugin.policyengine.RangerAccessRequest;
+import org.apache.ranger.plugin.util.RangerActionListMatcher;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RangerActionMatcher extends RangerAbstractConditionEvaluator {
+    private static final Logger LOG = LoggerFactory.getLogger(RangerActionMatcher.class);
+
+    private RangerActionListMatcher actionMatcher = new RangerActionListMatcher(null);

Review Comment:
   @mneethiraj I shall review and create a new PR for this change



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "rameeshm (via GitHub)" <gi...@apache.org>.
rameeshm merged PR #995:
URL: https://github.com/apache/ranger/pull/995


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3458036673


##########
security-admin/src/main/webapp/react-webapp/src/views/PolicyListing/PolicyPermissionItem.jsx:
##########
@@ -105,15 +107,53 @@ export default function PolicyPermissionItem(props) {
   };
 
   const grpResourcesKeys = useMemo(() => {
-    const { resources = [] } = serviceCompDetails;
+    const { resources = [] } = serviceCompDetails || {};
     const grpResources = groupBy(resources, "level");
     let grpResourcesKeys = [];
     for (const resourceKey in grpResources) {
       grpResourcesKeys.push(+resourceKey);
     }
     grpResourcesKeys = grpResourcesKeys.sort();

Review Comment:
   Sorting `grpResourcesKeys` without a comparator sorts lexicographically, which can misorder numeric levels (e.g., `10` before `2`) and lead to incorrect resource-depth handling. Use a numeric comparator (e.g., `(a, b) => a - b`) after ensuring the keys are numbers.



##########
security-admin/src/main/webapp/react-webapp/src/views/PolicyListing/PolicyPermissionItem.jsx:
##########
@@ -105,15 +107,53 @@ export default function PolicyPermissionItem(props) {
   };
 
   const grpResourcesKeys = useMemo(() => {
-    const { resources = [] } = serviceCompDetails;
+    const { resources = [] } = serviceCompDetails || {};
     const grpResources = groupBy(resources, "level");
     let grpResourcesKeys = [];
     for (const resourceKey in grpResources) {
       grpResourcesKeys.push(+resourceKey);
     }
     grpResourcesKeys = grpResourcesKeys.sort();
     return grpResourcesKeys;
-  }, []);
+  }, [serviceCompDetails?.resources]);
+
+  // Narrow dependency: only recompute when resource selection fields change,
+  // not on every keystroke in user/group/permission fields.
+  const resourceBlocks = Array.isArray(formValues?.additionalResources)
+    ? formValues.additionalResources
+    : [formValues];
+
+  const serializedResources = JSON.stringify(
+    resourceBlocks.map((b) =>
+      grpResourcesKeys.map((level) => b?.[`resourceName-${level}`]?.name || null)
+    )
+  );

Review Comment:
   `serializedResources` is computed via `JSON.stringify(...)` on every render (even when unrelated fields change), which can be expensive for larger policies. Consider memoizing `serializedResources` itself (or subscribing only to the relevant resource fields) so the stringify work is avoided when resource selections haven't changed.



##########
security-admin/src/main/webapp/react-webapp/src/hooks/usePruneStaleConditions.js:
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Keeps policyItems[].conditions["action-matches"] in sync when permissions or
+ * resources change. Mounted from PolicyPermissionItem; does not touch ip-range
+ * or other per-row conditions.
+ */
+
+import { useEffect } from "react";
+import { isArray, find, isEqual } from "lodash";
+import {
+  getSelectedAccessTypesForRow,
+  getAllowedActionMatchesForCondition,
+  isPerRowCondition,
+  parseConditionUiHint
+} from "Utils/policyConditionUtils";
+
+/**
+ * Stable dependency string: only permission accesses and per-row condition values.
+ * Avoids re-running when unrelated form fields (user/group names, etc.) change.
+ */
+const serializePruneDeps = (formValues, attrName) => {
+  const items = formValues?.[attrName];
+  if (!Array.isArray(items)) {
+    return "[]";
+  }
+  return JSON.stringify(
+    items.map((item, index) => ({
+      accesses: getSelectedAccessTypesForRow(formValues, attrName, index),
+      actionMatches: item?.conditions?.["action-matches"] ?? null
+    }))
+  );
+};
+
+export const usePruneStaleConditions = ({
+  formValues,
+  attrName,
+  form,
+  leafResourceTypes,
+  serviceCompDetails,
+  conditionDefVal,
+  actionReqsMap
+}) => {
+  const serializedPruneDeps = serializePruneDeps(formValues, attrName);
+
+  useEffect(() => {
+    const items = formValues?.[attrName];
+    if (!items || !isArray(items)) {
+      return;
+    }
+
+    let hasChanges = false;
+    const newItems = [...items];
+
+    newItems.forEach((item, index) => {
+      if (!item.conditions) {
+        return;
+      }
+
+      const accesses = getSelectedAccessTypesForRow(formValues, attrName, index);
+      if (accesses.length === 0) {
+        return;
+      }

Review Comment:
   When a row's permissions are cleared (`accesses.length === 0`), the hook currently skips pruning, which can leave a stale `conditions['action-matches']` saved on that row. If the UI intends actions to be permission-scoped, consider explicitly deleting `action-matches` when no accesses are selected so invalid state can't silently persist.



##########
security-admin/src/main/webapp/react-webapp/src/components/Editable.jsx:
##########
@@ -677,14 +776,41 @@ const Editable = (props) => {
   const handleApply = () => {
     let errors, uiHintVal;
     if (selectValRef?.current) {
+      // Re-validate action-matches against current row permissions before persisting.
       sortBy(Object.keys(selectValRef.current)).map((property) => {
         let conditionObj = find(conditionDefVal, function (m) {
           if (m.name == property) {
             return m;
           }

Review Comment:
   This uses `.map()` only for side effects (the return value is ignored). Replacing with `.forEach()` (or a `for...of`) avoids allocating an unused array and makes the intent clearer.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3539922290


##########
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerActionListMatcher.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.ranger.plugin.util;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
+
+public class RangerActionListMatcher {
+    private final boolean     allowAnyAction;
+    private final Set<String> exactActions = new HashSet<>();
+    private final String[]    prefixActions;
+
+    public RangerActionListMatcher(Collection<String> actions) {
+        boolean            allowAny       = CollectionUtils.isEmpty(actions);
+        final List<String> tempPrefixList = new ArrayList<>();
+
+        if (!allowAny) {
+            for (String a : actions) {
+                final String action = StringUtils.trimToEmpty(a);
+
+                if (action.isEmpty()) {
+                    continue;
+                }
+
+                if (action.equals("*")) {
+                    allowAny = true;
+                    exactActions.clear();
+                    tempPrefixList.clear();
+                    break;
+                }
+
+                if (action.endsWith("*")) {
+                    final String prefix = StringUtils.trimToEmpty(action.substring(0, action.length() - 1)).toLowerCase(Locale.ROOT);
+
+                    if (prefix.isEmpty()) {
+                        allowAny = true;
+                        exactActions.clear();
+                        tempPrefixList.clear();
+                        break;
+                    }
+
+                    tempPrefixList.add(prefix);
+                } else {
+                    exactActions.add(action.toLowerCase(Locale.ROOT));
+                }
+            }
+        }
+
+        this.allowAnyAction = allowAny;
+
+        if (!tempPrefixList.isEmpty()) {
+            tempPrefixList.sort((s1, s2) -> Integer.compare(s1.length(), s2.length()));
+
+            List<String> optimizedPrefixes = new ArrayList<>();
+            for (String p : tempPrefixList) {
+                boolean isCovered = false;
+                for (String optPrefix : optimizedPrefixes) {
+                    if (p.startsWith(optPrefix)) {
+                        isCovered = true;
+                        break;
+                    }
+                }
+                if (!isCovered) {
+                    optimizedPrefixes.add(p);
+                }
+            }
+            this.prefixActions = optimizedPrefixes.toArray(new String[0]);
+        } else {
+            this.prefixActions = new String[0];
+        }
+    }
+
+    public boolean isMatch(String requestAction) {
+        boolean ret = allowAnyAction;
+
+        if (!ret) {
+            // if action is not available, don't enforce action restrictions
+            if (StringUtils.isBlank(requestAction)) {
+                ret = true;

Review Comment:
   hi all, this is a good catch - I added a fix in this PR https://github.com/apache/ranger/pull/1053 (I heard @rameeshm is travelling)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
mneethiraj commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3485951128


##########
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerActionListMatcher.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.ranger.plugin.util;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
+
+public class RangerActionListMatcher {
+    private final boolean     allowAnyAction;
+    private final Set<String> exactActions = new HashSet<>();
+    private final String[]    prefixActions;
+
+    public RangerActionListMatcher(Collection<String> actions) {
+        boolean            allowAny       = CollectionUtils.isEmpty(actions);
+        final List<String> tempPrefixList = new ArrayList<>();
+
+        if (!allowAny) {
+            for (String a : actions) {
+                final String action = StringUtils.trimToEmpty(a);
+
+                if (action.isEmpty()) {
+                    continue;
+                }
+
+                if (action.equals("*")) {
+                    allowAny = true;
+                    exactActions.clear();
+                    tempPrefixList.clear();
+                    break;
+                }
+
+                if (action.endsWith("*")) {
+                    final String prefix = StringUtils.trimToEmpty(action.substring(0, action.length() - 1)).toLowerCase(Locale.ROOT);
+
+                    if (prefix.isEmpty()) {
+                        allowAny = true;
+                        exactActions.clear();
+                        tempPrefixList.clear();
+                        break;
+                    }
+
+                    tempPrefixList.add(prefix);
+                } else {
+                    exactActions.add(action.toLowerCase(Locale.ROOT));
+                }
+            }
+        }
+
+        this.allowAnyAction = allowAny;
+
+        if (!tempPrefixList.isEmpty()) {
+            tempPrefixList.sort((s1, s2) -> Integer.compare(s1.length(), s2.length()));
+
+            List<String> optimizedPrefixes = new ArrayList<>();
+            for (String p : tempPrefixList) {
+                boolean isCovered = false;
+                for (String optPrefix : optimizedPrefixes) {
+                    if (p.startsWith(optPrefix)) {
+                        isCovered = true;
+                        break;
+                    }
+                }
+                if (!isCovered) {
+                    optimizedPrefixes.add(p);
+                }
+            }
+            this.prefixActions = optimizedPrefixes.toArray(new String[0]);
+        } else {
+            this.prefixActions = new String[0];
+        }
+    }
+
+    public boolean isMatch(String requestAction) {
+        boolean ret = allowAnyAction;
+
+        if (!ret) {
+            // if action is not available, don't enforce action restrictions
+            if (StringUtils.isBlank(requestAction)) {
+                ret = true;

Review Comment:
   It seems missing action field in the request should be treated as "not a match", since the expectation is to match specified actions only.



##########
agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerActionMatcher.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ranger.plugin.conditionevaluator;
+
+import org.apache.ranger.plugin.policyengine.RangerAccessRequest;
+import org.apache.ranger.plugin.util.RangerActionListMatcher;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RangerActionMatcher extends RangerAbstractConditionEvaluator {
+    private static final Logger LOG = LoggerFactory.getLogger(RangerActionMatcher.class);
+
+    private RangerActionListMatcher actionMatcher = new RangerActionListMatcher(null);

Review Comment:
   `actionMatcher` instantiated here is over written in `init()` - line 37 below. I suggest initializing to `null` and update `isMatched()` to handle `null` value.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3398371315


##########
security-admin/src/main/webapp/react-webapp/src/hooks/usePruneStaleConditions.js:
##########
@@ -0,0 +1,145 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Keeps policyItems[].conditions["action-matches"] in sync when permissions or
+ * resources change. Mounted from PolicyPermissionItem; does not touch ip-range
+ * or other per-row conditions.
+ */
+
+import { useEffect } from "react";
+import { isArray, find, isEqual } from "lodash";
+import {
+  getSelectedAccessTypesForRow,
+  getAllowedActionMatchesForCondition,
+  isPerRowCondition,
+  parseConditionUiHint
+} from "Utils/policyConditionUtils";
+
+/**
+ * Stable dependency string: only permission accesses and per-row condition values.
+ * Avoids re-running when unrelated form fields (user/group names, etc.) change.
+ */
+const serializePruneDeps = (formValues, attrName) => {
+  const items = formValues?.[attrName];
+  if (!Array.isArray(items)) {
+    return "[]";
+  }
+  return JSON.stringify(
+    items.map((item, index) => ({
+      accesses: getSelectedAccessTypesForRow(formValues, attrName, index),
+      actionMatches: item?.conditions?.["action-matches"] ?? null
+    }))
+  );
+};
+
+export const usePruneStaleConditions = ({
+  formValues,
+  attrName,
+  form,
+  leafResourceTypes,
+  serviceCompDetails,
+  conditionDefVal,
+  actionReqsMap
+}) => {
+  const serializedPruneDeps = serializePruneDeps(formValues, attrName);
+
+  useEffect(() => {
+    if (!formValues[attrName] || !isArray(formValues[attrName])) {
+      return;
+    }

Review Comment:
   `usePruneStaleConditions` dereferences `formValues[attrName]` without guarding `formValues`. If `formValues` is undefined during initial render, this will throw and break the policy form.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3456171506


##########
pom.xml:
##########
@@ -892,6 +892,11 @@
             <name>jetbrains-intellij-dependencies</name>
             <url>https://packages.jetbrains.team/maven/p/ij/intellij-dependencies</url>
         </repository>
+        <repository>
+            <id>apache-ozone-staging</id>
+            <name>Apache Ozone 2.1.1 RC0 Staging</name>
+            <url>https://repository.apache.org/content/repositories/orgapacheozone-1062/</url>
+        </repository>

Review Comment:
   Ozone 2.1.1 is now released, so I updated the PR to remove the staging repository



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
mneethiraj commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3521389035


##########
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerActionListMatcher.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.ranger.plugin.util;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
+
+public class RangerActionListMatcher {
+    private final boolean     allowAnyAction;
+    private final Set<String> exactActions = new HashSet<>();
+    private final String[]    prefixActions;
+
+    public RangerActionListMatcher(Collection<String> actions) {
+        boolean            allowAny       = CollectionUtils.isEmpty(actions);
+        final List<String> tempPrefixList = new ArrayList<>();
+
+        if (!allowAny) {
+            for (String a : actions) {
+                final String action = StringUtils.trimToEmpty(a);
+
+                if (action.isEmpty()) {
+                    continue;
+                }
+
+                if (action.equals("*")) {
+                    allowAny = true;
+                    exactActions.clear();
+                    tempPrefixList.clear();
+                    break;
+                }
+
+                if (action.endsWith("*")) {
+                    final String prefix = StringUtils.trimToEmpty(action.substring(0, action.length() - 1)).toLowerCase(Locale.ROOT);
+
+                    if (prefix.isEmpty()) {
+                        allowAny = true;
+                        exactActions.clear();
+                        tempPrefixList.clear();
+                        break;
+                    }
+
+                    tempPrefixList.add(prefix);
+                } else {
+                    exactActions.add(action.toLowerCase(Locale.ROOT));
+                }
+            }
+        }
+
+        this.allowAnyAction = allowAny;
+
+        if (!tempPrefixList.isEmpty()) {
+            tempPrefixList.sort((s1, s2) -> Integer.compare(s1.length(), s2.length()));
+
+            List<String> optimizedPrefixes = new ArrayList<>();
+            for (String p : tempPrefixList) {
+                boolean isCovered = false;
+                for (String optPrefix : optimizedPrefixes) {
+                    if (p.startsWith(optPrefix)) {
+                        isCovered = true;
+                        break;
+                    }
+                }
+                if (!isCovered) {
+                    optimizedPrefixes.add(p);
+                }
+            }
+            this.prefixActions = optimizedPrefixes.toArray(new String[0]);
+        } else {
+            this.prefixActions = new String[0];
+        }
+    }
+
+    public boolean isMatch(String requestAction) {
+        boolean ret = allowAnyAction;
+
+        if (!ret) {
+            // if action is not available, don't enforce action restrictions
+            if (StringUtils.isBlank(requestAction)) {
+                ret = true;

Review Comment:
   > if no conditions are present we mark is as allowed.
   
   @rameeshm  - yes, that is the valid behavior when condition doesn't have any value. However, when condition has a value, it must be satisfied for it to trigger the policy/policy-item - irrespective of whether the request has any value for `action` field. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] RANGER-5628: Support for actions in Ozone Service Definition [ranger]

Posted by "fmorg-git (via GitHub)" <gi...@apache.org>.
fmorg-git commented on code in PR #995:
URL: https://github.com/apache/ranger/pull/995#discussion_r3539923102


##########
agents-common/src/main/java/org/apache/ranger/plugin/conditionevaluator/RangerActionMatcher.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ranger.plugin.conditionevaluator;
+
+import org.apache.ranger.plugin.policyengine.RangerAccessRequest;
+import org.apache.ranger.plugin.util.RangerActionListMatcher;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RangerActionMatcher extends RangerAbstractConditionEvaluator {
+    private static final Logger LOG = LoggerFactory.getLogger(RangerActionMatcher.class);
+
+    private RangerActionListMatcher actionMatcher = new RangerActionListMatcher(null);

Review Comment:
   hi all, I added a fix in this PR https://github.com/apache/ranger/pull/1053 (I heard @rameeshm is travelling)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ranger.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org