[
https://issues.apache.org/jira/browse/GEODE-4310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16330677#comment-16330677
]
ASF GitHub Bot commented on GEODE-4310:
---------------------------------------
jdeppe-pivotal commented on a change in pull request #1300: GEODE-4310: allow
ResourcePermission to take Strings as arguments for…
URL: https://github.com/apache/geode/pull/1300#discussion_r162382519
##########
File path:
geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java
##########
@@ -40,102 +43,129 @@
public static String ALL_KEYS = "*";
public enum Resource {
- NULL, CLUSTER, DATA
+ ALL, NULL, CLUSTER, DATA;
+
+ public String getName() {
+ if (this == ALL) {
+ return ResourcePermission.ALL;
+ }
+ return name();
+ }
}
public enum Operation {
- NULL, MANAGE, WRITE, READ
+ ALL, NULL, MANAGE, WRITE, READ;
+ public String getName() {
+ if (this == ALL) {
+ return ResourcePermission.ALL;
+ }
+ return name();
+ }
}
- // when ALL is specified, we need it to convert to string "*" instead of
"ALL".
public enum Target {
- ALL(ResourcePermission.ALL), DISK, GATEWAY, QUERY, DEPLOY;
-
- private String name;
-
- Target() {}
-
- Target(String name) {
- this.name = name;
- }
-
+ ALL, DISK, GATEWAY, QUERY, DEPLOY;
public String getName() {
- if (name != null) {
- return name;
+ if (this == ALL) {
+ return ResourcePermission.ALL;
}
return name();
}
}
- // these default values are used when creating a lock around an operation
- private Resource resource = Resource.NULL;
- private Operation operation = Operation.NULL;
+ // these default values are used when creating an allow-all lock around an
operation
+ private String resource = NULL;
+ private String operation = NULL;
private String target = ALL;
private String key = ALL;
- public ResourcePermission() {
- this(Resource.NULL, Operation.NULL, ALL, ALL);
- }
+ public ResourcePermission() {}
- public ResourcePermission(String resource, String operation) {
+ public ResourcePermission(Resource resource, Operation operation) {
this(resource, operation, ALL, ALL);
}
- public ResourcePermission(String resource, String operation, String target) {
+ public ResourcePermission(Resource resource, Operation operation, String
target) {
this(resource, operation, target, ALL);
}
- public ResourcePermission(String resource, String operation, String target,
String key) {
- this((resource == null) ? Resource.NULL :
Resource.valueOf(resource.toUpperCase()),
- (operation == null) ? Operation.NULL :
Operation.valueOf(operation.toUpperCase()), target,
+ public ResourcePermission(Resource resource, Operation operation, Target
target) {
+ this(resource, operation, target, ALL);
+ }
+
+ public ResourcePermission(Resource resource, Operation operation, Target
target, String key) {
+ this(resource == null ? null : resource.getName(),
+ operation == null ? null : operation.getName(), target == null ? null
: target.getName(),
key);
}
- public ResourcePermission(Resource resource, Operation operation) {
- this(resource, operation, ALL, ALL);
+ public ResourcePermission(Resource resource, Operation operation, String
target, String key) {
+ this(resource == null ? null : resource.getName(),
+ operation == null ? null : operation.getName(), target, key);
}
- public ResourcePermission(Resource resource, Operation operation, String
target) {
- this(resource, operation, target, ALL);
+ public ResourcePermission(String resource, String operation) {
+ this(resource, operation, ALL, ALL);
}
- public ResourcePermission(Resource resource, Operation operation, Target
target) {
+ public ResourcePermission(String resource, String operation, String target) {
this(resource, operation, target, ALL);
}
- public ResourcePermission(Resource resource, Operation operation, Target
target,
- String targetKey) {
- this(resource, operation, target.getName(), targetKey);
- }
+ public ResourcePermission(String resource, String operation, String target,
String key) {
+ // what's eventually stored are either "*", "NULL" or a valid enum except
ALL
+ // fields are never null.
+ this.resource = parsePart(resource, r -> Resource.valueOf(r).getName());
+ this.operation = parsePart(operation, o -> Operation.valueOf(o).getName());
- public ResourcePermission(Resource resource, Operation operation, String
target, String key) {
- if (resource != null) {
- this.resource = resource;
- }
- if (operation != null) {
- this.operation = operation;
- }
- if (target != null) {
+ if (target != null)
this.target = StringUtils.stripStart(target, Region.SEPARATOR);
- }
- if (key != null) {
+ if (key != null)
Review comment:
Please add a block around the condition even if it's just one line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> improve ResourcePermission to take strings as argument for resource and
> operation
> ---------------------------------------------------------------------------------
>
> Key: GEODE-4310
> URL: https://issues.apache.org/jira/browse/GEODE-4310
> Project: Geode
> Issue Type: Improvement
> Components: security
> Reporter: Jinmei Liao
> Priority: Major
> Labels: pull-request-available
>
> would be nice to be able to create ResourcePermission("*", "*") that would
> mean that this operation requires all permissions
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
