Skip to content

Commit

Permalink
feat(core): restrict deletion of the attribute definition
Browse files Browse the repository at this point in the history
* We want to restrict the deletion of the attribute definition if this attribute is required for any service.
* We want to restrict it also for the case when this attribute is a source or a destination attribute of any application form item in some application form.
  • Loading branch information
HejdaJakub committed Aug 22, 2023
1 parent 00b936b commit b562024
Show file tree
Hide file tree
Showing 10 changed files with 245 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1730,10 +1730,11 @@ Map<String, Attribute> getEntitylessAttributesWithKeys(PerunSession sess, String
* <p>
* PRIVILEGE: Only PerunAdmin can delete existing attribute.
*
* @throws InternalErrorException if an exception raise in concrete implementation, the exception is wrapped in InternalErrorException
* @throws PrivilegeException if privileges are not given
* @throws InternalErrorException if an exception raise in concrete implementation, the exception is wrapped in InternalErrorException
* @throws PrivilegeException if privileges are not given
* @throws RelationExistsException if attribute definition has any relation to some application form item or to some service as a required attribute
*/
void deleteAttribute(PerunSession sess, AttributeDefinition attributeDefinition) throws PrivilegeException, AttributeNotExistsException;
void deleteAttribute(PerunSession sess, AttributeDefinition attributeDefinition) throws PrivilegeException, AttributeNotExistsException, RelationExistsException;

/**
* Deletes the attribute.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import cz.metacentrum.perun.core.api.exceptions.WrongModuleTypeException;
import cz.metacentrum.perun.core.api.exceptions.WrongReferenceAttributeValueException;
import cz.metacentrum.perun.core.implApi.modules.attributes.UserVirtualAttributesModuleImplApi;
import cz.metacentrum.perun.registrar.model.ApplicationForm;
import cz.metacentrum.perun.utils.graphs.Graph;
import cz.metacentrum.perun.utils.graphs.GraphTextFormat;

Expand Down Expand Up @@ -1974,9 +1975,10 @@ void setRequiredAttributes(
*
* @param sess
* @param attributeDefinition
* @throws InternalErrorException if an exception raise in concrete implementation, the exception is wrapped in InternalErrorException
* @throws InternalErrorException if an exception raise in concrete implementation, the exception is wrapped in InternalErrorException
* @throws RelationExistsException if attribute definition has any relation to some application form item or to some service as a required attribute
*/
void deleteAttribute(PerunSession sess, AttributeDefinition attributeDefinition);
void deleteAttribute(PerunSession sess, AttributeDefinition attributeDefinition) throws RelationExistsException;

/**
* Delete all authz for the attribute.
Expand Down Expand Up @@ -4867,5 +4869,23 @@ void mergeAttributesValues(PerunSession sess, Member member, List<Attribute> att
* @return list of namespaces
*/
List<String> getAllNamespaces(PerunSession sess);

/**
* Returns all application forms where the given attribute definition is a source or a destination attribute for any application from item
* @param sess session
* @param attr attribute definition
* @return list of application forms where the given attribute definition has relation to any application form item
*/
List<ApplicationForm> getAppFormsWhereAttributeRelated(PerunSession sess, AttributeDefinition attr);

/**
* Returns list of app form items' shortnames for which the given attribute is a source or a destination attribute in the given application form
*
* @param sess session
* @param appFormId id of application form
* @param attr attribute definition
* @return list of shortnames
*/
List<String> getAppFormItemsForAppFormAndAttribute(PerunSession sess, int appFormId, AttributeDefinition attr);
}

Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
import cz.metacentrum.perun.core.implApi.modules.attributes.SkipValueCheckDuringDependencyCheck;
import cz.metacentrum.perun.core.implApi.modules.attributes.UserVirtualAttributesModuleImplApi;
import cz.metacentrum.perun.core.implApi.modules.attributes.VirtualAttributesModuleImplApi;
import cz.metacentrum.perun.registrar.model.ApplicationForm;
import cz.metacentrum.perun.utils.graphs.Graph;
import cz.metacentrum.perun.utils.graphs.GraphEdge;
import cz.metacentrum.perun.utils.graphs.GraphTextFormat;
Expand Down Expand Up @@ -2586,9 +2587,27 @@ private boolean isCorrectNameSpace(String value) {
}

@Override
public void deleteAttribute(PerunSession sess, AttributeDefinition attribute) {
//Remove services' required attributes
//TODO
public void deleteAttribute(PerunSession sess, AttributeDefinition attribute) throws RelationExistsException {
//Check relation to services' required attributes
List<Service> relatedServices = getPerunBl().getServicesManagerBl().getServicesByAttributeDefinition(sess, attribute);
if (!relatedServices.isEmpty()) {
throw new RelationExistsException("Attribute definition with id: " + attribute.getId() + " is the required attribute for these services: "
+ relatedServices.stream().map(service -> service.getName() + " (id: " + service.getId() + ")").toList());
}

//Check relation to any application form as a source or destination attribute
List<ApplicationForm> applicationForms = getAppFormsWhereAttributeRelated(sess, attribute);
if (!applicationForms.isEmpty()) {
throw new RelationExistsException("Attribute definition with id: " + attribute.getId() + " has a relation (as a source or destination attribute) to these application form items: "
+ applicationForms.stream().map(appForm -> {
String message = "Application form items: " + getAppFormItemsForAppFormAndAttribute(sess, appForm.getId(), attribute);
message += appForm.getGroup() == null ?
" in the application form in VO with id: " + appForm.getVo().getId() :
" in the application form in Group with id: " + appForm.getGroup().getId() + " (under Vo with id: " + appForm.getVo().getId() + ")";
return message;
})
.toList());
}

//Remove attribute and all it's values
this.deleteAllAttributeAuthz(sess, attribute);
Expand Down Expand Up @@ -8893,6 +8912,18 @@ public List<String> getAllNamespaces(PerunSession sess) {
return getAttributesManagerImpl().getAllNamespaces(sess).stream().map(friendlyName -> friendlyName.split(":", 2)[1]).sorted().toList();
}

@Override
public
List<ApplicationForm>getAppFormsWhereAttributeRelated(PerunSession sess, AttributeDefinition attr) {
return getAttributesManagerImpl().getAppFormsWhereAttributeRelated(sess, attr);
}

@Override
public
List<String>getAppFormItemsForAppFormAndAttribute(PerunSession sess, int appFormId, AttributeDefinition attr) {
return getAttributesManagerImpl().getAppFormItemsForAppFormAndAttribute(sess, appFormId, attr);
}

// ------------ PRIVATE METHODS FOR ATTRIBUTE DEPENDENCIES LOGIC --------------
// These methods get one or two Perun Beans and return list of richAttributes
// of specific type defined by name of method which actually exists in Perun
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1617,7 +1617,7 @@ public AttributeDefinition createAttribute(PerunSession sess, AttributeDefinitio
}

@Override
public void deleteAttribute(PerunSession sess, AttributeDefinition attribute) throws PrivilegeException, AttributeNotExistsException {
public void deleteAttribute(PerunSession sess, AttributeDefinition attribute) throws PrivilegeException, AttributeNotExistsException, RelationExistsException {
Utils.checkPerunSession(sess);
getAttributesManagerBl().checkAttributeExists(sess, attribute);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@
import cz.metacentrum.perun.core.implApi.modules.attributes.UserFacilityVirtualAttributesModuleImplApi;
import cz.metacentrum.perun.core.implApi.modules.attributes.UserVirtualAttributesModuleImplApi;
import cz.metacentrum.perun.core.implApi.modules.attributes.VoAttributesModuleImplApi;
import cz.metacentrum.perun.registrar.model.ApplicationForm;
import cz.metacentrum.perun.registrar.model.ApplicationFormItem;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -320,6 +322,22 @@ private static ResultSetExtractor<List<AttributePolicyCollection>> getAttributeP

private static final RowMapper<String> ATTRIBUTE_FRIENDLY_NAMES_MAPPER = (rs, i) -> rs.getString("friendly_name");

private static final RowMapper<String> APP_FORM_ITEM_SHORTNAME_MAPPER = (rs, i) -> rs.getString("shortname");

private static final RowMapper<ApplicationForm> APPLICATION_FORM_ROW_MAPPER = (resultSet, i) -> {
ApplicationForm form = new ApplicationForm();
form.setId(resultSet.getInt("id"));
Vo vo = new Vo();
vo.setId(resultSet.getInt("vo_id"));
form.setVo(vo);
if (resultSet.getInt("group_id") > 0) {
Group grp = new Group();
grp.setId(resultSet.getInt("group_id"));
form.setGroup(grp);
}
return form;
};

static class SingleBeanAttributeRowMapper<T extends PerunBean> extends AttributeRowMapper<T, T> {
SingleBeanAttributeRowMapper(PerunSession sess, AttributesManagerImpl attributesManagerImpl, T attributeHolder) {
super(sess, attributesManagerImpl, attributeHolder, null);
Expand Down Expand Up @@ -4215,6 +4233,31 @@ public List<String> getAllNamespaces(PerunSession sess) {
return jdbc.query("SELECT friendly_name FROM attr_names WHERE friendly_name LIKE 'login-namespace:%%' AND attr_name NOT LIKE '%%def:virt%%'", ATTRIBUTE_FRIENDLY_NAMES_MAPPER);
}

@Override
public List<ApplicationForm> getAppFormsWhereAttributeRelated(PerunSession sess, AttributeDefinition attr) {
try {
String urn = attr.getName();
return jdbc.query("select distinct af.id, af.vo_id, af.group_id from application_form af " +
"join application_form_items afi on af.id = afi.form_id " +
"where afi.src_attr=? or afi.dst_attr=?", APPLICATION_FORM_ROW_MAPPER , urn, urn);
} catch (EmptyResultDataAccessException ex) {
return new ArrayList<>();
} catch (RuntimeException ex) {
throw new InternalErrorException(ex);
}
}

@Override
public List<String> getAppFormItemsForAppFormAndAttribute(PerunSession sess, int appFormId, AttributeDefinition attr) {
try {
String urn = attr.getName();
return jdbc.query("select id, shortname from application_form_items afi where afi.form_id=? and (afi.src_attr=? or afi.dst_attr=?)",
APP_FORM_ITEM_SHORTNAME_MAPPER, appFormId, urn, urn);
} catch (RuntimeException ex) {
throw new InternalErrorException(ex);
}
}

/**
* Escapes LIST_DELIMITER from the value.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import cz.metacentrum.perun.core.blImpl.AttributesManagerBlImpl;
import cz.metacentrum.perun.core.implApi.modules.attributes.AttributesModuleImplApi;
import cz.metacentrum.perun.core.implApi.modules.attributes.UserVirtualAttributesModuleImplApi;
import cz.metacentrum.perun.registrar.model.ApplicationForm;

import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -2816,4 +2817,22 @@ public interface AttributesManagerImplApi {
* @return list of namespaces
*/
List<String> getAllNamespaces(PerunSession sess);

/**
* Returns all application forms where the given attribute definition is a source or a destination attribute for any application from item
* @param sess session
* @param attr attribute definition
* @return list of application forms where the given attribute definition has relation to any application form item
*/
List<ApplicationForm> getAppFormsWhereAttributeRelated(PerunSession sess, AttributeDefinition attr);

/**
* Returns list of app form items' shortnames for which the given attribute is a source or a destination attribute in the given application form
*
* @param sess session
* @param appFormId id of application form
* @param attr attribute definition
* @return list of shortnames
*/
List<String> getAppFormItemsForAppFormAndAttribute(PerunSession sess, int appFormId, AttributeDefinition attr);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@
import cz.metacentrum.perun.core.api.Facility;
import cz.metacentrum.perun.core.api.Group;
import cz.metacentrum.perun.core.api.Member;
import cz.metacentrum.perun.core.api.PerunBean;
import cz.metacentrum.perun.core.api.PerunSession;
import cz.metacentrum.perun.core.api.Resource;
import cz.metacentrum.perun.core.api.RichAttribute;
import cz.metacentrum.perun.core.api.Service;
import cz.metacentrum.perun.core.api.User;
import cz.metacentrum.perun.core.api.Vo;
import cz.metacentrum.perun.core.blImpl.AttributesManagerBlImpl;
import cz.metacentrum.perun.core.blImpl.ServicesManagerBlImpl;
import cz.metacentrum.perun.core.impl.AttributesManagerImpl;
import cz.metacentrum.perun.core.impl.modules.attributes.urn_perun_user_attribute_def_virt_loa;
import cz.metacentrum.perun.core.impl.modules.attributes.urn_perun_user_attribute_def_virt_userCertDNs;
Expand Down Expand Up @@ -73,6 +72,7 @@ public class AttributesManagerBlImplUnitTests {
* Use when you need to mock only some methods.
*/
private final AttributesManagerBlImpl attrManagerBlImplMock = mock(AttributesManagerBlImpl.class);
private final ServicesManagerBlImpl serviceManagerBlImplMock = mock(ServicesManagerBlImpl.class);

private final AttributesManagerImpl attrManagerImplMock = mock(AttributesManagerImpl.class);
private final PerunSession sessionMock = mock(PerunSession.class);
Expand Down Expand Up @@ -348,6 +348,9 @@ public void deleteAttributeRemovesDependenciesForDeletedAttributeWhenSucceeded()
Set<AttributeDefinition> allDefinitions = Collections.singleton(A);
initializeModuleDependenciesMethod.invoke(attrManagerBlImpl, sessionMock, allDefinitions);

// enable deletion of attribute with relation to service
mockEnableAttributeDeletionWithRelationToService(A);

// delete attribute
attrManagerBlImpl.deleteAttribute(sessionMock, A);

Expand Down Expand Up @@ -381,6 +384,9 @@ public void deleteAttributeRemovesDeletedAttributeFromDependenciesWhenSucceeded(
Set<AttributeDefinition> allDefinitions = Sets.newHashSet(A, B);
initializeModuleDependenciesMethod.invoke(attrManagerBlImpl, sessionMock, allDefinitions);

// enable deletion of attribute with relation to service
mockEnableAttributeDeletionWithRelationToService(B);

attrManagerBlImpl.deleteAttribute(sessionMock, B);

Map<AttributeDefinition, Set<AttributeDefinition>> dependencies = getDependencies();
Expand All @@ -405,6 +411,9 @@ public void deleteAttributeRemovesDeletedAttributeFromStrongDependenciesWhenSucc
Set<AttributeDefinition> allDefinitions = Sets.newHashSet(A, B);
initializeModuleDependenciesMethod.invoke(attrManagerBlImpl, sessionMock, allDefinitions);

// enable deletion of attribute with relation to service
mockEnableAttributeDeletionWithRelationToService(B);

attrManagerBlImpl.deleteAttribute(sessionMock, B);

Map<AttributeDefinition, Set<AttributeDefinition>> strongDependencies = getStrongDependencies();
Expand All @@ -430,6 +439,9 @@ public void deleteAttributeDoesNotRemoveDependenciesForDeletedAttributeWhenFails
Set<AttributeDefinition> allDefinitions = Collections.singleton(A);
initializeModuleDependenciesMethod.invoke(attrManagerBlImpl, sessionMock, allDefinitions);

// enable deletion of attribute with relation to service
mockEnableAttributeDeletionWithRelationToService(A);

assertThatExceptionOfType(RuntimeException.class)
.isThrownBy(() -> attrManagerBlImpl.deleteAttribute(sessionMock, A));

Expand Down Expand Up @@ -468,6 +480,9 @@ public void deleteAttributeDoesNotRemoveDeletedAttributeFromDependenciesWhenFail
Set<AttributeDefinition> allDefinitions = Sets.newHashSet(A, B);
initializeModuleDependenciesMethod.invoke(attrManagerBlImpl, sessionMock, allDefinitions);

// enable deletion of attribute with relation to service
mockEnableAttributeDeletionWithRelationToService(B);

assertThatExceptionOfType(RuntimeException.class)
.isThrownBy(() -> attrManagerBlImpl.deleteAttribute(sessionMock, B));

Expand Down Expand Up @@ -498,6 +513,9 @@ public void deleteAttributeDoesNotRemoveDeletedAttributeFromStrongDependenciesWh
Set<AttributeDefinition> allDefinitions = Sets.newHashSet(A, B);
initializeModuleDependenciesMethod.invoke(attrManagerBlImpl, sessionMock, allDefinitions);

// enable deletion of attribute with relation to service
mockEnableAttributeDeletionWithRelationToService(B);

assertThatExceptionOfType(RuntimeException.class)
.isThrownBy(() -> attrManagerBlImpl.deleteAttribute(sessionMock, B));

Expand Down Expand Up @@ -694,4 +712,17 @@ private Method getPrivateMethodFromAtrManager(String methodName, Class<?>... arg
method.setAccessible(true);
return method;
}

/**
* Mock the getServicesByAttributeDefinition method for the given attribute to return an empty list
* This way we want to enable mocked deletion also for attribute which is required for any service
*
* @param attr attribute definition which should be deleted
*/
private void mockEnableAttributeDeletionWithRelationToService(AttributeDefinition attr) {
PerunBl perunBl = attrManagerBlImpl.getPerunBl();
when(perunBl.getServicesManagerBl()).thenReturn(serviceManagerBlImplMock);
when(serviceManagerBlImplMock.getServicesByAttributeDefinition(eq(sessionMock), eq(attr)))
.thenReturn(Collections.emptyList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5849,6 +5849,26 @@ public void deleteAttributeWhenAttributeNotExists() throws Exception {

}

@Test (expected=RelationExistsException.class)
public void deleteAttributeRequiredForService() throws Exception {
System.out.println(CLASS_NAME + "deleteAttributeRequiredForService");

AttributeDefinition attrDef = new AttributeDefinition();
attrDef.setDescription("attributesManagerTestAttrDef");
attrDef.setFriendlyName("attrDef");
attrDef.setNamespace("urn:perun:member:attribute-def:opt");
attrDef.setType(String.class.getName());
attributesManager.createAttribute(sess, attrDef);

Service service = setUpService();
Service service2 = setUpService2();

perun.getServicesManager().addRequiredAttribute(sess, service, attrDef);
perun.getServicesManager().addRequiredAttribute(sess, service2, attrDef);

attributesManager.deleteAttribute(sess, attrDef);
}

@Ignore
@Test (expected=RelationExistsException.class)
public void deleteAttributeWhenRelationExists() throws Exception {
Expand Down
Loading

0 comments on commit b562024

Please sign in to comment.