-
Notifications
You must be signed in to change notification settings - Fork 1.9k
IGNITE-28944 Refactor SecurityBasicPermissionSet #13424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -29,8 +29,8 @@ | |||||
| import java.security.PrivilegedAction; | ||||||
| import java.security.PrivilegedActionException; | ||||||
| import java.security.PrivilegedExceptionAction; | ||||||
| import java.util.Arrays; | ||||||
| import java.util.Collection; | ||||||
| import java.util.EnumSet; | ||||||
| import java.util.HashMap; | ||||||
| import java.util.Map; | ||||||
| import java.util.Objects; | ||||||
|
|
@@ -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") | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| public static Map<String, Collection<SecurityPermission>> upcast(Map<String, EnumSet<SecurityPermission>> map) { | ||||||
| return (Map<String, Collection<SecurityPermission>>)(Map)map; | ||||||
| } | ||||||
|
|
||||||
| /** @param map Map. */ | ||||||
| @SuppressWarnings("rawtypes") | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| public static Map<String, EnumSet<SecurityPermission>> downcast(Map<String, Collection<SecurityPermission>> map) { | ||||||
| return (Map<String, EnumSet<SecurityPermission>>)(Map)map; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
|
|
@@ -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, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No line break is needed here. |
||||||
| Map<String, EnumSet<SecurityPermission>> permissions) { | ||||||
| if (F.isEmpty(permissions)) | ||||||
| return; | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,25 +22,32 @@ | |
| import java.io.ObjectOutputStream; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.EnumSet; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.stream.Collectors; | ||
| import org.apache.ignite.IgniteCheckedException; | ||
| import org.apache.ignite.internal.MarshallableMessage; | ||
| import org.apache.ignite.internal.Order; | ||
| import org.apache.ignite.internal.util.tostring.GridToStringInclude; | ||
| import org.apache.ignite.internal.util.typedef.internal.A; | ||
| import org.apache.ignite.internal.util.typedef.internal.S; | ||
| import org.apache.ignite.internal.util.typedef.internal.U; | ||
| import org.apache.ignite.marshaller.Marshaller; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| import static org.apache.ignite.internal.processors.security.SecurityUtils.compatibleServicePermissions; | ||
| import static org.apache.ignite.internal.processors.security.SecurityUtils.downcast; | ||
| import static org.apache.ignite.internal.processors.security.SecurityUtils.isSecurityCompatibilityMode; | ||
| import static org.apache.ignite.internal.processors.security.SecurityUtils.serializeVersion; | ||
| import static org.apache.ignite.internal.processors.security.SecurityUtils.upcast; | ||
|
|
||
| /** | ||
| * Simple implementation of {@link SecurityPermissionSet} interface. | ||
| * Provides convenient way to specify permission set in the XML configuration. | ||
| */ | ||
| public class SecurityBasicPermissionSet implements SecurityPermissionSet { | ||
| public class SecurityBasicPermissionSet implements SecurityPermissionSet, MarshallableMessage { | ||
| /** Serial version uid. */ | ||
| private static final long serialVersionUID = 0L; | ||
|
|
||
|
|
@@ -75,40 +82,40 @@ public class SecurityBasicPermissionSet implements SecurityPermissionSet { | |
| * | ||
| * @param cachePermissions Cache permissions. | ||
| */ | ||
| public void setCachePermissions(Map<String, Collection<SecurityPermission>> cachePermissions) { | ||
| public void setCachePermissions(Map<String, EnumSet<SecurityPermission>> cachePermissions) { | ||
| A.notNull(cachePermissions, "cachePermissions"); | ||
|
|
||
| this.cachePermissions = cachePermissions; | ||
| this.cachePermissions = upcast(cachePermissions); | ||
| } | ||
|
|
||
| /** | ||
| * Setter for set task permission map. | ||
| * | ||
| * @param taskPermissions Task permissions. | ||
| */ | ||
| public void setTaskPermissions(Map<String, Collection<SecurityPermission>> taskPermissions) { | ||
| public void setTaskPermissions(Map<String, EnumSet<SecurityPermission>> taskPermissions) { | ||
| A.notNull(taskPermissions, "taskPermissions"); | ||
|
|
||
| this.taskPermissions = taskPermissions; | ||
| this.taskPermissions = upcast(taskPermissions); | ||
| } | ||
|
|
||
| /** | ||
| * Setter for set service permission map. | ||
| * | ||
| * @param srvcPermissions Service permissions. | ||
| */ | ||
| public void setServicePermissions(Map<String, Collection<SecurityPermission>> srvcPermissions) { | ||
| public void setServicePermissions(Map<String, EnumSet<SecurityPermission>> srvcPermissions) { | ||
| A.notNull(taskPermissions, "servicePermissions"); | ||
|
|
||
| this.srvcPermissions = srvcPermissions; | ||
| this.srvcPermissions = upcast(srvcPermissions); | ||
| } | ||
|
|
||
| /** | ||
| * Setter for set collection system permission. | ||
| * | ||
| * @param sysPermissions System permissions. | ||
| */ | ||
| public void setSystemPermissions(Collection<SecurityPermission> sysPermissions) { | ||
| public void setSystemPermissions(EnumSet<SecurityPermission> sysPermissions) { | ||
| this.sysPermissions = sysPermissions; | ||
| } | ||
|
|
||
|
|
@@ -122,23 +129,23 @@ public void setDefaultAllowAll(boolean dfltAllowAll) { | |
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public Map<String, Collection<SecurityPermission>> cachePermissions() { | ||
| return cachePermissions; | ||
| @Override public Map<String, EnumSet<SecurityPermission>> cachePermissions() { | ||
| return downcast(cachePermissions); | ||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public Map<String, Collection<SecurityPermission>> taskPermissions() { | ||
| return taskPermissions; | ||
| @Override public Map<String, EnumSet<SecurityPermission>> taskPermissions() { | ||
| return downcast(taskPermissions); | ||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public Map<String, Collection<SecurityPermission>> servicePermissions() { | ||
| return srvcPermissions; | ||
| @Override public Map<String, EnumSet<SecurityPermission>> servicePermissions() { | ||
| return downcast(srvcPermissions); | ||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Nullable @Override public Collection<SecurityPermission> systemPermissions() { | ||
| return sysPermissions; | ||
| @Nullable @Override public EnumSet<SecurityPermission> systemPermissions() { | ||
| return (EnumSet<SecurityPermission>)sysPermissions; | ||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
|
|
@@ -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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo. |
||
| convert(); | ||
| } | ||
|
|
||
| /** */ | ||
| private void convert() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. convert -> normalize? |
||
| 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( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. toEnumSetMap -> normalizeValueType? |
||
| 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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can check if |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need something more meaningful here, or we should simply leave it empty.
The same below.