Commit 514a314e authored by Artem Hrybeniuk's avatar Artem Hrybeniuk Committed by Matija Obreza
Browse files

Using String for SecuredAction

parent e944a90b
......@@ -102,11 +102,11 @@ public class GGCESecurityConfig {
} else {
LOG.trace("Checking actionAllowed('{}', {}, Site#{})", action, permission, ((Site) site).getId());
}
SecuredAction securedAction = securedActionRepository.getByActionAndSite(SecurityAction.valueOf(action), (Site) site);
SecuredAction securedAction = securedActionRepository.getByActionAndSite(action, (Site) site);
if (site != null && securedAction == null) {
LOG.trace("Checking no-site actionAllowed('{}', {}, null)", action, permission);
securedAction = securedActionRepository.getByActionAndSite(SecurityAction.valueOf(action), null);
securedAction = securedActionRepository.getByActionAndSite(action, null);
}
if (LOG.isDebugEnabled()) {
......
......@@ -16,15 +16,17 @@
package org.gringlobal.model.community;
import javax.persistence.Cacheable;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.EnumType;
import javax.persistence.Enumerated;
import javax.persistence.ManyToOne;
import javax.persistence.PersistenceException;
import javax.persistence.PrePersist;
import javax.persistence.PreUpdate;
import javax.persistence.Table;
import javax.persistence.UniqueConstraint;
import io.swagger.v3.oas.annotations.media.Schema;
import org.apache.commons.lang3.StringUtils;
import org.genesys.blocks.model.AuditedVersionedModel;
import org.genesys.blocks.model.Copyable;
import org.genesys.blocks.security.model.AclAwareModel;
......@@ -35,6 +37,8 @@ import org.gringlobal.model.Site;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import java.util.Arrays;
/**
* GG-CE Security Action is an ACL OID to which ACL entries are assigned.
*
......@@ -54,8 +58,9 @@ public class SecuredAction extends AuditedVersionedModel implements AclAwareMode
@ManyToOne(cascade = {}, optional = true)
private Site site;
@Enumerated(EnumType.STRING)
private SecurityAction action;
@Column(name = "action")
@Schema(implementation = SecurityAction.class)
private String action;
@JsonProperty(value = "parentActionId")
@JsonSerialize(using = EntityIdSerializer.class)
......@@ -65,12 +70,12 @@ public class SecuredAction extends AuditedVersionedModel implements AclAwareMode
public SecuredAction() {
}
public SecuredAction(SecurityAction action, Site site) {
public SecuredAction(String action, Site site) {
this.action = action;
this.site = site;
}
public SecuredAction(SecurityAction action, Site site, SecuredAction parentAction) {
public SecuredAction(String action, Site site, SecuredAction parentAction) {
this.action = action;
this.site = site;
this.parentAction = parentAction;
......@@ -86,6 +91,11 @@ public class SecuredAction extends AuditedVersionedModel implements AclAwareMode
// Parent action is not available if site is null
this.parentAction = null;
}
try {
SecurityAction.valueOf(this.action);
} catch (IllegalArgumentException e) {
throw new PersistenceException("Invalid value for secured action: ".concat(action), e);
}
}
/**
......@@ -111,7 +121,7 @@ public class SecuredAction extends AuditedVersionedModel implements AclAwareMode
*
* @return the action
*/
public SecurityAction getAction() {
public String getAction() {
return action;
}
......@@ -120,7 +130,7 @@ public class SecuredAction extends AuditedVersionedModel implements AclAwareMode
*
* @param action the action to set
*/
public void setAction(SecurityAction action) {
public void setAction(String action) {
this.action = action;
}
......
......@@ -35,6 +35,6 @@ public interface SecuredActionRepository extends JpaRepository<SecuredAction, Lo
* @param site the site, or null
* @return registered {@link SecuredAction}
*/
SecuredAction getByActionAndSite(SecurityAction action, Site site);
SecuredAction getByActionAndSite(String action, Site site);
}
......@@ -22,6 +22,8 @@ import java.net.URL;
import java.nio.file.Paths;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.transaction.Transactional;
......@@ -42,6 +44,7 @@ import org.gringlobal.model.Site;
import org.gringlobal.model.SysUser;
import org.gringlobal.model.community.CommunityAppSettings;
import org.gringlobal.model.community.CommunityCodeValues;
import org.gringlobal.model.community.QSecuredAction;
import org.gringlobal.model.community.SecuredAction;
import org.gringlobal.model.community.SecurityAction;
import org.gringlobal.persistence.InventoryMaintenancePolicyRepository;
......@@ -223,11 +226,11 @@ public class ApplicationStartup implements InitializingBean {
// Each security action requires an entry
for (SecurityAction action : SecurityAction.values()) {
var securedAction = securedActionRepository.getByActionAndSite(action, null);
var securedAction = securedActionRepository.getByActionAndSite(action.name(), null);
if (securedAction == null) {
LOG.warn("Registering GG-CE Security action {}", action);
securedAction = new SecuredAction();
securedAction.setAction(action);
securedAction.setAction(action.name());
securedActionRepository.save(securedAction);
customAclService.createOrUpdatePermissions(securedAction, groupAdmins); // this doesn't do much for groupAdmins
}
......@@ -238,6 +241,21 @@ public class ApplicationStartup implements InitializingBean {
LOG.error("Could not set up ACL: {}", e.getMessage(), e);
}
// Remove invalid actions
try {
asAdmin(() -> {
var validActions = Stream.of(SecurityAction.values()).map(Enum::name).collect(Collectors.toList());
var invalidActions = securedActionRepository.findAll(QSecuredAction.securedAction.action.notIn(validActions));
invalidActions.forEach(invalidAction -> {
LOG.warn("Removing SecuredAction with invalid action code: {} site={}", invalidAction.getAction(), invalidAction.getSite());
});
securedActionRepository.deleteAll(invalidActions);
return true;
});
} catch (Throwable e) {
LOG.error("Could not delete invalid actions", e);
}
// GROUP_ADMINS needs all permissions
try {
asAdmin(() -> {
......@@ -245,7 +263,7 @@ public class ApplicationStartup implements InitializingBean {
// Each security action requires an entry
for (SecurityAction action : SecurityAction.values()) {
var securedAction = securedActionRepository.getByActionAndSite(action, null);
var securedAction = securedActionRepository.getByActionAndSite(action.name(), null);
if (securedAction != null) {
LOG.warn("Allowing {} all permissions on GG-CE Security action {}", groupAdmins.getSid(), action);
customAclService.setPermissions(securedAction, groupAdmins, new Permissions().grantAll());
......
......@@ -145,11 +145,11 @@ public abstract class AbstractSecureApiTest extends AbstractApiTest {
// Each security action requires an entry
for (SecurityAction action : SecurityAction.values()) {
var securedAction = securedActionRepository.getByActionAndSite(action, null);
var securedAction = securedActionRepository.getByActionAndSite(action.name(), null);
if (securedAction == null) {
LOG.debug("Registering GG-CE Security action {}", action);
securedAction = new SecuredAction();
securedAction.setAction(action);
securedAction.setAction(action.name());
securedActionRepository.save(securedAction);
customAclService.createOrUpdatePermissions(securedAction, groupAdmins);
}
......
......@@ -89,11 +89,11 @@ public abstract class AbstractSecurityTest extends AbstractServicesTest {
// Each security action requires an entry
for (SecurityAction action : SecurityAction.values()) {
var securedAction = securedActionRepository.getByActionAndSite(action, null);
var securedAction = securedActionRepository.getByActionAndSite(action.name(), null);
if (securedAction == null) {
LOG.debug("Registering GG-CE Security action {}", action);
securedAction = new SecuredAction();
securedAction.setAction(action);
securedAction.setAction(action.name());
securedActionRepository.save(securedAction);
customAclService.createOrUpdatePermissions(securedAction, groupAdmins);
}
......
......@@ -57,7 +57,7 @@ public class AccessionApiSecurityTest extends AbstractSecureApiTest {
public void beforeTest() throws Exception {
super.beforeTest();
assertThat(ggceSec, notNullValue());
var passportDataAction = securedActionRepository.getByActionAndSite(SecurityAction.PassportData, null);
var passportDataAction = securedActionRepository.getByActionAndSite(SecurityAction.PassportData.name(), null);
assertThat(passportDataAction, notNullValue());
asAdmin(() -> TransactionHelper.executeInTransaction(false, () -> {
......@@ -70,7 +70,7 @@ public class AccessionApiSecurityTest extends AbstractSecureApiTest {
customAclService.setPermissions(passportDataAction, groupCurators, new Permissions().grantAll());
var passportDataActionSite1 = new SecuredAction(SecurityAction.PassportData, DEFAULT_SITE, passportDataAction);
var passportDataActionSite1 = new SecuredAction(SecurityAction.PassportData.name(), DEFAULT_SITE, passportDataAction);
passportDataActionSite1 = securedActionRepository.save(passportDataActionSite1);
// No delete for GROUP_MAIZE (curator2)
......
......@@ -61,7 +61,7 @@ public class AccessionSecurityTest extends AbstractSecurityTest {
public void beforeTest() throws Exception {
super.beforeTest();
assertThat(ggceSec, notNullValue());
var passportDataAction = securedActionRepository.getByActionAndSite(SecurityAction.PassportData, null);
var passportDataAction = securedActionRepository.getByActionAndSite(SecurityAction.PassportData.name(), null);
assertThat(passportDataAction, notNullValue());
asAdmin(() -> TransactionHelper.executeInTransaction(false, () -> {
......@@ -74,7 +74,7 @@ public class AccessionSecurityTest extends AbstractSecurityTest {
customAclService.setPermissions(passportDataAction, groupCurators, new Permissions().grantAll());
var passportDataActionSite1 = new SecuredAction(SecurityAction.PassportData, DEFAULT_SITE, passportDataAction);
var passportDataActionSite1 = new SecuredAction(SecurityAction.PassportData.name(), DEFAULT_SITE, passportDataAction);
passportDataActionSite1 = securedActionRepository.save(passportDataActionSite1);
// No delete for GROUP_MAIZE (curator2)
......
......@@ -48,7 +48,7 @@ public class CooperatorSecurityTest extends AbstractSecurityTest {
public void beforeTest() throws Exception {
super.beforeTest();
var cooperatorDataAction = securedActionRepository.getByActionAndSite(SecurityAction.CooperatorData, null);
var cooperatorDataAction = securedActionRepository.getByActionAndSite(SecurityAction.CooperatorData.name(), null);
assertThat(cooperatorDataAction, notNullValue());
asAdmin(() -> TransactionHelper.executeInTransaction(false, () -> {
......
......@@ -59,8 +59,8 @@ public class InventorySecurityTest extends AbstractSecurityTest {
super.beforeTest();
assertThat(ggceSec, notNullValue());
var inventoryDataAction = securedActionRepository.getByActionAndSite(SecurityAction.InventoryData, null);
var passportDataAction = securedActionRepository.getByActionAndSite(SecurityAction.PassportData, null);
var inventoryDataAction = securedActionRepository.getByActionAndSite(SecurityAction.InventoryData.name(), null);
var passportDataAction = securedActionRepository.getByActionAndSite(SecurityAction.PassportData.name(), null);
assertThat(inventoryDataAction, notNullValue());
asAdmin(() -> TransactionHelper.executeInTransaction(false, () -> {
......@@ -74,7 +74,7 @@ public class InventorySecurityTest extends AbstractSecurityTest {
customAclService.setPermissions(inventoryDataAction, groupCurators, new Permissions().grantAll());
customAclService.setPermissions(passportDataAction, groupCurators, new Permissions().grantAll());
var inventoryDataActionSite1 = new SecuredAction(SecurityAction.InventoryData, DEFAULT_SITE, inventoryDataAction);
var inventoryDataActionSite1 = new SecuredAction(SecurityAction.InventoryData.name(), DEFAULT_SITE, inventoryDataAction);
inventoryDataActionSite1 = securedActionRepository.save(inventoryDataActionSite1);
// No delete for GROUP_MAIZE (curator2)
......
......@@ -64,7 +64,7 @@ public class InventoryViabilitySecurityTest extends AbstractSecurityTest {
super.beforeTest();
assertThat(ggceSec, notNullValue());
var viabilityTestAction = securedActionRepository.getByActionAndSite(SecurityAction.ViabilityTest, null);
var viabilityTestAction = securedActionRepository.getByActionAndSite(SecurityAction.ViabilityTest.name(), null);
assertThat(viabilityTestAction, notNullValue());
asAdmin(() -> TransactionHelper.executeInTransaction(false, () -> {
......@@ -72,7 +72,7 @@ public class InventoryViabilitySecurityTest extends AbstractSecurityTest {
customAclService.setPermissions(viabilityTestAction, groupCurators, new Permissions().grantAll());
// Limit DEFAULT_SITE for GROUP_MAIZE
var viabilityTestActionSite1 = new SecuredAction(SecurityAction.ViabilityTest, DEFAULT_SITE, viabilityTestAction);
var viabilityTestActionSite1 = new SecuredAction(SecurityAction.ViabilityTest.name(), DEFAULT_SITE, viabilityTestAction);
viabilityTestActionSite1 = securedActionRepository.save(viabilityTestActionSite1);
// No delete for GROUP_MAIZE (curator2)
......@@ -83,7 +83,7 @@ public class InventoryViabilitySecurityTest extends AbstractSecurityTest {
var groupMaize = AppContextHelper.ensureAuthoritySid("GROUP_MAIZE");
customAclService.setPermissions(viabilityTestActionSite1, groupMaize, customPermissions);
assertThat(securedActionRepository.getByActionAndSite(SecurityAction.ViabilityTest, DEFAULT_SITE), notNullValue());
assertThat(securedActionRepository.getByActionAndSite(SecurityAction.ViabilityTest.name(), DEFAULT_SITE), notNullValue());
// As admin
accession = addAccessionToDB(addTaxonomySpeciesToDB(addTaxonomyGenusToDB("Hordeum"), "vulgare"));
......
......@@ -46,12 +46,12 @@ public class PermissionControllerTest extends AbstractSecureApiTest {
@WithUserDetails(value = AbstractServicesTest.USER_ADMINISTRATOR)
public void testInvalidSecuredActions() {
// Cannot have duplicate action on null site
var securedAction = new SecuredAction(SecurityAction.PassportData, null);
var securedAction = new SecuredAction(SecurityAction.PassportData.name(), null);
assertThrows(InvalidApiUsageException.class, () -> {
actionService.create(securedAction);
});
var siteAction = new SecuredAction(SecurityAction.PassportData, IITA_SITE);
var siteAction = new SecuredAction(SecurityAction.PassportData.name(), IITA_SITE);
actionService.create(siteAction);
// Cannot have duplicate action in one site
assertThrows(InvalidApiUsageException.class, () -> {
......@@ -149,7 +149,7 @@ public class PermissionControllerTest extends AbstractSecureApiTest {
@Test
@WithUserDetails(value = AbstractServicesTest.USER_ADMINISTRATOR)
public void createSecuredActionWithParent() throws Exception {
var securedAction = securedActionRepository.getByActionAndSite(SecurityAction.Request, null);
var securedAction = securedActionRepository.getByActionAndSite(SecurityAction.Request.name(), null);
assertThat(securedAction, notNullValue());
var content = "{\"siteId\":{\"id\": 1},\"action\":\"Request\",\"parentActionId\":{\"id\":" + securedAction.getId() + "}}";
......@@ -178,7 +178,7 @@ public class PermissionControllerTest extends AbstractSecureApiTest {
@Test
@WithUserDetails(value = AbstractServicesTest.USER_ADMINISTRATOR)
public void cannotDeleteSecuredActionWithoutSite() throws Exception {
var securedAction = securedActionRepository.getByActionAndSite(SecurityAction.Request, null);
var securedAction = securedActionRepository.getByActionAndSite(SecurityAction.Request.name(), null);
assertThat(securedAction, notNullValue());
/*@formatter:off*/
......@@ -195,7 +195,7 @@ public class PermissionControllerTest extends AbstractSecureApiTest {
@Test
@WithUserDetails(value = AbstractServicesTest.USER_ADMINISTRATOR)
public void deleteSecuredAction() throws Exception {
var securedAction = actionService.create(new SecuredAction(SecurityAction.Request, DEFAULT_SITE));
var securedAction = actionService.create(new SecuredAction(SecurityAction.Request.name(), DEFAULT_SITE));
assertThat(securedAction, notNullValue());
assertThat(securedAction.getId(), notNullValue());
......@@ -215,7 +215,7 @@ public class PermissionControllerTest extends AbstractSecureApiTest {
@Test
@WithUserDetails(value = AbstractServicesTest.USER_ADMINISTRATOR)
public void deleteSecuredActionService() {
var securedAction = actionService.create(new SecuredAction(SecurityAction.Request, DEFAULT_SITE));
var securedAction = actionService.create(new SecuredAction(SecurityAction.Request.name(), DEFAULT_SITE));
assertThat(securedAction, notNullValue());
assertThat(securedAction.getId(), notNullValue());
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment