gianm commented on code in PR #17564:
URL: https://github.com/apache/druid/pull/17564#discussion_r1915746639
##########
server/src/main/java/org/apache/druid/server/security/Access.java:
##########
@@ -21,52 +21,111 @@
import com.google.common.base.Strings;
import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.query.policy.Policy;
+import javax.annotation.Nullable;
+import java.util.Optional;
+
+/**
+ * Represents the outcome of verifying permissions to perform an {@link
Action} on a {@link Resource}, along with any
+ * policy restrictions.
+ */
public class Access
{
public static final String DEFAULT_ERROR_MESSAGE = "Unauthorized";
+ public static final String DEFAULT_AUTHORIZED_MESSAGE = "Authorized";
- public static final Access OK = new Access(true);
- public static final Access DENIED = new Access(false);
+ public static final Access OK = allow();
+ public static final Access DENIED = deny("");
private final boolean allowed;
private final String message;
+ // A policy restriction on top of table-level read access. It should be
empty if there are no policy restrictions
+ // or if access is requested for an action other than reading the table.
+ private final Optional<Policy> policy; // should this be a list?
Review Comment:
please remove this comment: `should this be a list?`
##########
server/src/main/java/org/apache/druid/server/security/Access.java:
##########
@@ -21,52 +21,111 @@
import com.google.common.base.Strings;
import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.query.policy.Policy;
+import javax.annotation.Nullable;
+import java.util.Optional;
+
+/**
+ * Represents the outcome of verifying permissions to perform an {@link
Action} on a {@link Resource}, along with any
+ * policy restrictions.
+ */
public class Access
{
public static final String DEFAULT_ERROR_MESSAGE = "Unauthorized";
+ public static final String DEFAULT_AUTHORIZED_MESSAGE = "Authorized";
- public static final Access OK = new Access(true);
- public static final Access DENIED = new Access(false);
+ public static final Access OK = allow();
+ public static final Access DENIED = deny("");
private final boolean allowed;
private final String message;
+ // A policy restriction on top of table-level read access. It should be
empty if there are no policy restrictions
Review Comment:
please elevate this to Javadoc on `Authorizer#authorizer`. They should
clarify that only policies relevant to the `Resource` and `Action` should be
included in the `Access` object.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
