petrov-mg commented on code in PR #13424:
URL: https://github.com/apache/ignite/pull/13424#discussion_r3706051472
##########
modules/core/src/main/java/org/apache/ignite/plugin/security/SecurityBasicPermissionSet.java:
##########
@@ -201,10 +208,47 @@ private void readObject(ObjectInputStream in) throws
IOException, ClassNotFoundE
else
srvcPermissions = Collections.emptyMap();
}
+
+ convert();
}
/** {@inheritDoc} */
@Override public String toString() {
return S.toString(SecurityBasicPermissionSet.class, this);
}
+
+ /** {@inheritDoc} */
+ @Override public void marshal(Marshaller marsh) throws
IgniteCheckedException {
+ // No-op.
+ }
+
+ /** {@inheritDoc} */
+ @Override public void unmarshal(Marshaller marsh, ClassLoader clsLdr)
throws IgniteCheckedException {
+ // Message framework uses ArrayList for ordinary collectons, so we
need convert it explicitly.
+ convert();
+ }
+
+ /** */
+ private void convert() {
Review Comment:
convert -> normalize?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityUtils.java:
##########
@@ -132,14 +132,26 @@ public static void restoreDefaultSerializeVersion() {
* @return Allow all service permissions.
*/
public static Map<String, Collection<SecurityPermission>>
compatibleServicePermissions() {
- Map<String, Collection<SecurityPermission>> srvcPerms = new
HashMap<>();
+ Map<String, EnumSet<SecurityPermission>> srvcPerms = new HashMap<>();
- srvcPerms.put("*", Arrays.asList(
+ srvcPerms.put("*", EnumSet.of(
SecurityPermission.SERVICE_CANCEL,
SecurityPermission.SERVICE_DEPLOY,
SecurityPermission.SERVICE_INVOKE));
- return srvcPerms;
+ return upcast(srvcPerms);
+ }
+
+ /** @param map Map. */
+ @SuppressWarnings("rawtypes")
Review Comment:
```suggestion
@SuppressWarnings({"rawtypes", "unchecked"})
```
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityUtils.java:
##########
@@ -132,14 +132,26 @@ public static void restoreDefaultSerializeVersion() {
* @return Allow all service permissions.
*/
public static Map<String, Collection<SecurityPermission>>
compatibleServicePermissions() {
- Map<String, Collection<SecurityPermission>> srvcPerms = new
HashMap<>();
+ Map<String, EnumSet<SecurityPermission>> srvcPerms = new HashMap<>();
- srvcPerms.put("*", Arrays.asList(
+ srvcPerms.put("*", EnumSet.of(
SecurityPermission.SERVICE_CANCEL,
SecurityPermission.SERVICE_DEPLOY,
SecurityPermission.SERVICE_INVOKE));
- return srvcPerms;
+ return upcast(srvcPerms);
+ }
+
+ /** @param map Map. */
+ @SuppressWarnings("rawtypes")
+ public static Map<String, Collection<SecurityPermission>>
upcast(Map<String, EnumSet<SecurityPermission>> map) {
+ return (Map<String, Collection<SecurityPermission>>)(Map)map;
+ }
+
+ /** @param map Map. */
+ @SuppressWarnings("rawtypes")
Review Comment:
```suggestion
@SuppressWarnings({"rawtypes", "unchecked"})
```
##########
modules/core/src/main/java/org/apache/ignite/plugin/security/SecurityBasicPermissionSet.java:
##########
@@ -201,10 +208,47 @@ private void readObject(ObjectInputStream in) throws
IOException, ClassNotFoundE
else
srvcPermissions = Collections.emptyMap();
}
+
+ convert();
}
/** {@inheritDoc} */
@Override public String toString() {
return S.toString(SecurityBasicPermissionSet.class, this);
}
+
+ /** {@inheritDoc} */
+ @Override public void marshal(Marshaller marsh) throws
IgniteCheckedException {
+ // No-op.
+ }
+
+ /** {@inheritDoc} */
+ @Override public void unmarshal(Marshaller marsh, ClassLoader clsLdr)
throws IgniteCheckedException {
+ // Message framework uses ArrayList for ordinary collectons, so we
need convert it explicitly.
Review Comment:
Typo.
##########
modules/core/src/test/java/org/apache/ignite/plugin/security/SecurityPermissionSetBuilderTest.java:
##########
@@ -143,8 +142,8 @@ public void testPermissionBuilder() {
* @param perms Permissions.
* @return Collection.
*/
- static Collection<SecurityPermission> permissions(SecurityPermission...
perms) {
- Collection<SecurityPermission> col = U.newHashSet(perms.length);
+ static EnumSet<SecurityPermission> permissions(SecurityPermission...
perms) {
Review Comment:
It seems that we can drop this method and use `EnumSet.of(<permisisons>)`
instead.
##########
modules/core/src/main/java/org/apache/ignite/plugin/security/SecurityBasicPermissionSet.java:
##########
@@ -201,10 +208,47 @@ private void readObject(ObjectInputStream in) throws
IOException, ClassNotFoundE
else
srvcPermissions = Collections.emptyMap();
}
+
+ convert();
}
/** {@inheritDoc} */
@Override public String toString() {
return S.toString(SecurityBasicPermissionSet.class, this);
}
+
+ /** {@inheritDoc} */
+ @Override public void marshal(Marshaller marsh) throws
IgniteCheckedException {
+ // No-op.
+ }
+
+ /** {@inheritDoc} */
+ @Override public void unmarshal(Marshaller marsh, ClassLoader clsLdr)
throws IgniteCheckedException {
+ // Message framework uses ArrayList for ordinary collectons, so we
need convert it explicitly.
+ convert();
+ }
+
+ /** */
+ private void convert() {
+ cachePermissions = toEnumSetMap(cachePermissions);
+ taskPermissions = toEnumSetMap(taskPermissions);
+ srvcPermissions = toEnumSetMap(srvcPermissions);
+ sysPermissions = copySafe(sysPermissions);
+ }
+
+ /**
+ * @param cachePermissions Cache permissions.
+ * @return Map with enum sets of security permissions.
+ */
+ public static Map<String, Collection<SecurityPermission>> toEnumSetMap(
Review Comment:
toEnumSetMap -> normalizeValueType?
##########
modules/core/src/main/java/org/apache/ignite/plugin/security/SecurityBasicPermissionSet.java:
##########
@@ -201,10 +208,47 @@ private void readObject(ObjectInputStream in) throws
IOException, ClassNotFoundE
else
srvcPermissions = Collections.emptyMap();
}
+
+ convert();
}
/** {@inheritDoc} */
@Override public String toString() {
return S.toString(SecurityBasicPermissionSet.class, this);
}
+
+ /** {@inheritDoc} */
+ @Override public void marshal(Marshaller marsh) throws
IgniteCheckedException {
+ // No-op.
+ }
+
+ /** {@inheritDoc} */
+ @Override public void unmarshal(Marshaller marsh, ClassLoader clsLdr)
throws IgniteCheckedException {
+ // Message framework uses ArrayList for ordinary collectons, so we
need convert it explicitly.
+ convert();
+ }
+
+ /** */
+ private void convert() {
+ cachePermissions = toEnumSetMap(cachePermissions);
+ taskPermissions = toEnumSetMap(taskPermissions);
+ srvcPermissions = toEnumSetMap(srvcPermissions);
+ sysPermissions = copySafe(sysPermissions);
+ }
+
+ /**
+ * @param cachePermissions Cache permissions.
+ * @return Map with enum sets of security permissions.
+ */
+ public static Map<String, Collection<SecurityPermission>> toEnumSetMap(
+ Map<String, Collection<SecurityPermission>> cachePermissions
+ ) {
+ return cachePermissions.entrySet().stream()
+ .collect(Collectors.toMap(Map.Entry::getKey, e ->
copySafe(e.getValue())));
+ }
+
+ /** */
+ private static EnumSet<SecurityPermission>
copySafe(Collection<SecurityPermission> col) {
+ return col != null ? EnumSet.copyOf(col) :
EnumSet.noneOf(SecurityPermission.class);
Review Comment:
We can check if `col` is `instanceof EnumSet<SecurityPermission>`
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityUtils.java:
##########
@@ -366,7 +378,8 @@ public static void authorizeAll(IgniteSecurity security,
SecurityPermissionSet p
}
/** */
- private static void authorizeAll(IgniteSecurity security, Map<String,
Collection<SecurityPermission>> permissions) {
+ private static void authorizeAll(IgniteSecurity security,
Review Comment:
No line break is needed here.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityUtils.java:
##########
@@ -132,14 +132,26 @@ public static void restoreDefaultSerializeVersion() {
* @return Allow all service permissions.
*/
public static Map<String, Collection<SecurityPermission>>
compatibleServicePermissions() {
- Map<String, Collection<SecurityPermission>> srvcPerms = new
HashMap<>();
+ Map<String, EnumSet<SecurityPermission>> srvcPerms = new HashMap<>();
- srvcPerms.put("*", Arrays.asList(
+ srvcPerms.put("*", EnumSet.of(
SecurityPermission.SERVICE_CANCEL,
SecurityPermission.SERVICE_DEPLOY,
SecurityPermission.SERVICE_INVOKE));
- return srvcPerms;
+ return upcast(srvcPerms);
+ }
+
+ /** @param map Map. */
Review Comment:
We need something more meaningful here, or we should simply leave it empty.
The same below.
--
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]
