Skip to content

Commit

Permalink
Resolve task owner lazily
Browse files Browse the repository at this point in the history
The task owner was resolved when creating the task instance, leading
to a lot of unnecessary repository get operations. Now it is resolved
only when needed.

Induced changes:
- operation result is now among parameters for audit method

Unrelated changes:
- fixed two bugs related to creating tasks from templates
(schedulingState vs. executionState)
  • Loading branch information
mederly committed Feb 23, 2021
1 parent 675b09a commit c7dca6c
Show file tree
Hide file tree
Showing 47 changed files with 254 additions and 179 deletions.
Expand Up @@ -635,6 +635,7 @@ public static TaskType createSingleRecurrenceTask(String taskName, QName applica
task.setBinding(TaskBindingType.LOOSE);
task.setCategory(category);
task.setExecutionStatus(TaskExecutionStateType.RUNNABLE);
task.setSchedulingState(TaskSchedulingStateType.READY);
task.setRecurrence(TaskRecurrenceType.SINGLE);
task.setThreadStopAction(ThreadStopActionType.RESTART);
task.setHandlerUri(pageBase.getTaskService().getHandlerUriForCategory(category));
Expand Down
Expand Up @@ -286,8 +286,9 @@ public static <T extends ObjectType> PrismObject<T> loadObject(Class<T> type, St
// point to an object that the current user cannot read. This is no big deal.
// Just do not display that object.
subResult.recordHandledError(e);
PrismObject<? extends FocusType> taskOwner = task.getOwner(result);
LOGGER.debug("User {} is not authorized to read {} {}",
task.getOwner() != null ? task.getOwner().getName() : null, type.getSimpleName(), oid);
taskOwner != null ? taskOwner.getName() : null, type.getSimpleName(), oid);
return null;
} catch (ObjectNotFoundException e) {
if (allowNotFound) {
Expand Down
Expand Up @@ -787,7 +787,8 @@ private void loadParentOrgs(PrismObject<O> object) {
// the permission to read target org
// It is OK to just ignore it.
subResult.muteLastSubresultError();
LOGGER.debug("User {} does not have permission to read parent org unit {} (ignoring error)", task.getOwner().getName(), parentOrgRef.getOid());
PrismObject<? extends FocusType> taskOwner = task.getOwner(subResult);
LOGGER.debug("User {} does not have permission to read parent org unit {} (ignoring error)", taskOwner.getName(), parentOrgRef.getOid());
} catch (Exception ex) {
subResult.recordWarning(createStringResource("PageAdminObjectDetails.message.loadParentOrgs.warning", parentOrgRef.getOid()).getString(), ex);
LOGGER.warn("Cannot load parent org {}: {}", parentOrgRef.getOid(), ex.getMessage(), ex);
Expand Down
Expand Up @@ -12,6 +12,8 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import com.evolveum.midpoint.schema.result.OperationResult;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.Authentication;
Expand All @@ -35,6 +37,8 @@

public class AuditedAccessDeniedHandler extends MidpointAccessDeniedHandler {

private static final String OP_AUDIT_EVENT = AuditedAccessDeniedHandler.class.getName() + ".auditEvent";

@Autowired private TaskManager taskManager;
@Autowired private AuditService auditService;
@Autowired private PrismContext prismContext;
Expand All @@ -55,6 +59,8 @@ protected boolean handleInternal(HttpServletRequest request, HttpServletResponse
}

private void auditEvent(HttpServletRequest request, Authentication authentication, AccessDeniedException accessDeniedException) {
OperationResult result = new OperationResult(OP_AUDIT_EVENT); // Eventually we should get this from the caller

MidPointPrincipal principal = SecurityUtils.getPrincipalUser(authentication);
PrismObject<? extends FocusType> user = principal != null ? principal.getFocus().asPrismObject() : null;

Expand Down Expand Up @@ -83,6 +89,6 @@ private void auditEvent(HttpServletRequest request, Authentication authenticatio
record.setSessionIdentifier(request.getRequestedSessionId());
record.setMessage(accessDeniedException.getMessage());

auditService.audit(record, task);
auditService.audit(record, task, result);
}
}
Expand Up @@ -43,6 +43,8 @@ public class AuditedLogoutHandler extends SimpleUrlLogoutSuccessHandler {

private static final Trace LOGGER = TraceManager.getTrace(AuditedLogoutHandler.class);

private static final String OP_AUDIT_EVENT = AuditedLogoutHandler.class.getName() + ".auditEvent";

@Autowired private TaskManager taskManager;
@Autowired private AuditService auditService;
@Autowired private SystemObjectCache systemObjectCache;
Expand Down Expand Up @@ -87,6 +89,8 @@ public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse resp
}

private void auditEvent(HttpServletRequest request, Authentication authentication) {
OperationResult result = new OperationResult(OP_AUDIT_EVENT); // Eventually we should get this from the caller

MidPointPrincipal principal = SecurityUtils.getPrincipalUser(authentication);
PrismObject<? extends FocusType> user = principal != null ? principal.getFocus().asPrismObject() : null;

Expand All @@ -101,7 +105,7 @@ private void auditEvent(HttpServletRequest request, Authentication authenticatio
}
SystemConfigurationType system = null;
try {
system = systemObjectCache.getSystemConfiguration(new OperationResult("LOAD SYSTEM CONFIGURATION")).asObjectable();
system = systemObjectCache.getSystemConfiguration(result).asObjectable();
} catch (SchemaException e) {
LOGGER.error("Couldn't get system configuration from cache", e);
}
Expand All @@ -127,6 +131,6 @@ private void auditEvent(HttpServletRequest request, Authentication authenticatio
record.setNodeIdentifier(taskManager.getNodeId());
record.setSessionIdentifier(sessionId);

auditService.audit(record, task);
auditService.audit(record, task, result);
}
}
Expand Up @@ -7,10 +7,10 @@

package com.evolveum.midpoint.model.api;

import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.security.api.ConnectionEnvironment;
import com.evolveum.midpoint.task.api.Task;
import com.evolveum.midpoint.xml.ns._public.common.common_3.FocusType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.UserType;

/**
* Created by Viliam Repan (lazyman).
Expand All @@ -21,5 +21,5 @@ public interface ModelAuditRecorder {

void auditLoginFailure(String username, FocusType focus, ConnectionEnvironment connEnv, String message);

void auditLogout(ConnectionEnvironment connEnv, Task task);
void auditLogout(ConnectionEnvironment connEnv, Task task, OperationResult result);
}
Expand Up @@ -6,6 +6,8 @@
*/
package com.evolveum.midpoint.model.impl.controller;

import static com.evolveum.midpoint.xml.ns._public.common.common_3.TaskSchedulingStateType.READY;

import static java.util.Collections.emptyList;
import static java.util.Collections.singleton;

Expand Down Expand Up @@ -1666,6 +1668,7 @@ public TaskType submitTaskFromTemplate(String templateTaskOid, List<Item<?, ?>>
newTask.setTaskIdentifier(null);
newTask.setOwnerRef(createObjectRef(principal.getFocus(), prismContext));
newTask.setExecutionStatus(RUNNABLE);
newTask.setSchedulingState(READY);
for (Item<?, ?> extensionItem : extensionItems) {
newTask.asPrismObject().getExtension().add(extensionItem.clone());
}
Expand Down
Expand Up @@ -243,10 +243,10 @@ private <T extends Objectable> EventResult importParsedObject(PrismObject<T> pri
String channel = SchemaConstants.CHANNEL_OBJECT_IMPORT_URI;
metaData.setCreateChannel(channel);
metaData.setCreateTimestamp(clock.currentTimeXMLGregorianCalendar());
if (task.getOwner() != null) {
metaData.setCreatorRef(ObjectTypeUtil.createObjectRef(task.getOwner(), prismContext));
}
object.asObjectable().setMetadata(metaData);
if (task.getOwnerRef() != null) {
metaData.setCreatorRef(task.getOwnerRef().clone());
}
object.asObjectable().setMetadata(metaData);
}

objectResult.computeStatus();
Expand Down
Expand Up @@ -10,6 +10,7 @@
import java.util.*;
import javax.xml.datatype.XMLGregorianCalendar;

import com.evolveum.midpoint.prism.util.CloneUtil;
import com.evolveum.midpoint.prism.xml.XmlTypeConverter;
import com.evolveum.midpoint.repo.common.util.OperationExecutionWriter;
import com.evolveum.midpoint.schema.constants.ObjectTypes;
Expand Down Expand Up @@ -190,7 +191,7 @@ private OperationExecutionType createExecutionRecord(Collection<LensObjectDeltaO
private void setOperationContext(OperationExecutionType operation, OperationResultStatusType overallStatus, Context<?> ctx) {
operation.setStatus(overallStatus);
// TODO what if the real initiator is different? (e.g. when executing approved changes)
operation.setInitiatorRef(ObjectTypeUtil.createObjectRef(ctx.task.getOwner(), prismContext));
operation.setInitiatorRef(CloneUtil.clone(ctx.task.getOwnerRef()));
operation.setChannel(ctx.lensContext.getChannel());
operation.setTimestamp(ctx.now);
}
Expand Down
Expand Up @@ -63,8 +63,8 @@ public <F extends ObjectType> void setRequestMetadataInContext(LensContext<F> co
private MetadataType collectRequestMetadata(XMLGregorianCalendar now, Task task) {
MetadataType metaData = new MetadataType(prismContext);
metaData.setRequestTimestamp(now);
if (task.getOwner() != null) {
metaData.setRequestorRef(createObjectRef(task.getOwner(), prismContext));
if (task.getOwnerRef() != null) {
metaData.setRequestorRef(task.getOwnerRef().clone());
}
// It is not necessary to store requestor comment here as it is preserved in context.options field.
return metaData;
Expand Down Expand Up @@ -268,8 +268,8 @@ private <F extends ObjectType> void applyCreateMetadata(LensContext<F> context,
String channel = LensUtil.getChannel(context, task);
metaData.setCreateChannel(channel);
metaData.setCreateTimestamp(now);
if (task.getOwner() != null) {
metaData.setCreatorRef(createObjectRef(task.getOwner(), prismContext));
if (task.getOwnerRef() != null) {
metaData.setCreatorRef(task.getOwnerRef().clone());
}
metaData.setCreateTaskRef(task.getOid() != null ? task.getSelfReference() : null);
}
Expand Down Expand Up @@ -304,7 +304,7 @@ public <F extends ObjectType, T extends ObjectType> Collection<ItemDelta<?,?>> c
List<ItemDelta<?, ?>> deltas = new ArrayList<>(prismContext.deltaFor(objectType)
.item(metadataPath.append(MetadataType.F_MODIFY_CHANNEL)).replace(LensUtil.getChannel(context, task))
.item(metadataPath.append(MetadataType.F_MODIFY_TIMESTAMP)).replace(now)
.item(metadataPath.append(MetadataType.F_MODIFIER_REF)).replace(createObjectRef(task.getOwner(), prismContext))
.item(metadataPath.append(MetadataType.F_MODIFIER_REF)).replace(CloneUtil.clone(task.getOwnerRef()))
.item(metadataPath.append(MetadataType.F_MODIFY_TASK_REF)).replaceRealValues(
task.getOid() != null ? singleton(task.getSelfReference()) : emptySet())
.asItemDeltas());
Expand Down
Expand Up @@ -88,7 +88,6 @@ private void submitTask(OperationResult result, TaskType newTask)
ModelExecuteOptions options = new ModelExecuteOptions(actx.beans.prismContext).preAuthorized();
Collection<ObjectDeltaOperation<? extends ObjectType>> operations = actx.beans.modelService.executeChanges(deltas, options, actx.task, result);
String oid = ObjectDeltaOperation.findAddDeltaOid(operations, newTask.asPrismObject());
System.out.println("New task OID = " + oid);
result.setAsynchronousOperationReference(oid);
}
}
Expand Up @@ -10,6 +10,7 @@
import static com.evolveum.midpoint.model.api.util.ReferenceResolver.Source.MODEL;
import static com.evolveum.midpoint.schema.util.ObjectTypeUtil.createObjectRef;
import static com.evolveum.midpoint.xml.ns._public.common.common_3.TaskExecutionStateType.RUNNABLE;
import static com.evolveum.midpoint.xml.ns._public.common.common_3.TaskSchedulingStateType.READY;

import com.evolveum.midpoint.model.api.util.ReferenceResolver;

Expand Down Expand Up @@ -91,6 +92,7 @@ TaskType createEmptyTask(OperationResult result)
newTask.setTaskIdentifier(null);
newTask.setOwnerRef(createObjectRef(principal.getFocus(), beans.prismContext));
newTask.setExecutionStatus(RUNNABLE);
newTask.setSchedulingState(READY);
return newTask;
}

Expand Down
Expand Up @@ -93,16 +93,17 @@ private void auditLogin(@Nullable String username, @Nullable FocusType focus, @N
}

@Override
public void auditLogout(ConnectionEnvironment connEnv, Task task) {
public void auditLogout(ConnectionEnvironment connEnv, Task task, OperationResult result) {
if (!SecurityUtil.isAuditedLoginAndLogout(getSystemConfig(), connEnv.getChannel())) {
return;
}
AuditEventRecord record = new AuditEventRecord(AuditEventType.TERMINATE_SESSION, AuditEventStage.REQUEST);
record.setInitiatorAndLoginParameter(task.getOwner(), prismContext);
PrismObject<? extends FocusType> taskOwner = task.getOwner(result);
record.setInitiatorAndLoginParameter(taskOwner, prismContext);
record.setTimestamp(System.currentTimeMillis());
record.setOutcome(OperationResultStatus.SUCCESS);
storeConnectionEnvironment(record, connEnv);
auditHelper.audit(record, null, task, new OperationResult(SecurityHelper.class.getName() + ".auditLogout"));
auditHelper.audit(record, null, task, result);
}

private SystemConfigurationType getSystemConfig() {
Expand Down
Expand Up @@ -72,7 +72,7 @@ public void audit(AuditEventRecord record, ObjectDeltaSchemaLevelUtil.NameResolv
try {
LOGGER.trace("Auditing the record:\n{}", record.debugDumpLazily());
resolveNamesInDeltas(record, externalNameResolver, result);
auditService.audit(record, task);
auditService.audit(record, task, result);
} catch (Throwable t) {
result.recordFatalError(t);
throw t;
Expand Down
Expand Up @@ -39,13 +39,14 @@
* See also https://wiki.evolveum.com/display/midPoint/Linked+objects.
*
* There are two kinds of organizations here:
*
* - departments:
* - orgs are managed directly in midPoint
* - members (users-dcs-xxxx, users-cc-xxxx) are recomputed by default, and using direct iterative bulk action task
* * orgs are managed directly in midPoint
* * members (users-dcs-xxxx, users-cc-xxxx) are recomputed by default, and using direct iterative bulk action task
* - clubs:
* - orgs are managed by reconciliation from special "clubs" resource, although members are managed by midPoint
* - members (alice, bob, chuck) are NOT recomputed by default
* - recompute is done using delayed triggers, and is requested e.g. when synchronizing clubs from "clubs" resource
* * orgs are managed by reconciliation from special "clubs" resource, although members are managed by midPoint
* * members (alice, bob, chuck) are NOT recomputed by default
* * recompute is done using delayed triggers, and is requested e.g. when synchronizing clubs from "clubs" resource
*/
@ContextConfiguration(locations = { "classpath:ctx-model-intest-test-main.xml" })
@DirtiesContext(classMode = ClassMode.AFTER_CLASS)
Expand Down
Expand Up @@ -1204,7 +1204,7 @@ public void test560StartTaskFromTemplate() throws Exception {

// cleaning up the tasks

Thread.sleep(5000L); // cleanup is set to 1 second after completion
Thread.sleep(5000L); // cleanup is set to 1 second after completion

importObjectFromFile(TASK_TRIGGER_SCANNER_FILE);

Expand Down
Expand Up @@ -9,6 +9,8 @@

import javax.annotation.PostConstruct;

import com.evolveum.midpoint.xml.ns._public.common.common_3.FocusType;

import org.jetbrains.annotations.NotNull;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
Expand Down Expand Up @@ -159,8 +161,9 @@ private ResourceObjectEventImpl createRequest(OperationStatus status,
event.setRequestee(new SimpleObjectRefImpl(notificationsUtil, user.asObjectable()));
} // otherwise, appropriate messages were already logged

if (task != null && task.getOwner() != null) {
event.setRequester(new SimpleObjectRefImpl(notificationsUtil, task.getOwner()));
PrismObject<? extends FocusType> taskOwner = task != null ? task.getOwner(result) : null;
if (taskOwner != null) {
event.setRequester(new SimpleObjectRefImpl(notificationsUtil, taskOwner));
} else {
LOGGER.warn("No owner for task {}, therefore no requester will be set for event {}", task, event.getId());
}
Expand Down
Expand Up @@ -53,45 +53,45 @@ public void init() {

@Override
public void onCampaignStart(AccessCertificationCampaignType campaign, Task task, OperationResult result) {
CertCampaignEvent event = certEventFactory.createOnCampaignStartEvent(campaign, task);
CertCampaignEvent event = certEventFactory.createOnCampaignStartEvent(campaign, task, result);
eventHelper.processEvent(event, task, result);
}

@Override
public void onCampaignEnd(AccessCertificationCampaignType campaign, Task task, OperationResult result) {
CertCampaignEvent event = certEventFactory.createOnCampaignEndEvent(campaign, task);
CertCampaignEvent event = certEventFactory.createOnCampaignEndEvent(campaign, task, result);
eventHelper.processEvent(event, task, result);
}

@Override
public void onCampaignStageStart(AccessCertificationCampaignType campaign, Task task, OperationResult result) {
CertCampaignStageEvent event = certEventFactory.createOnCampaignStageStartEvent(campaign, task);
CertCampaignStageEvent event = certEventFactory.createOnCampaignStageStartEvent(campaign, task, result);
eventHelper.processEvent(event, task, result);
}

@Override
public void onCampaignStageDeadlineApproaching(AccessCertificationCampaignType campaign, Task task, OperationResult result) {
CertCampaignStageEvent event = certEventFactory.createOnCampaignStageDeadlineApproachingEvent(campaign, task);
CertCampaignStageEvent event = certEventFactory.createOnCampaignStageDeadlineApproachingEvent(campaign, task, result);
eventHelper.processEvent(event, task, result);
}

@Override
public void onCampaignStageEnd(AccessCertificationCampaignType campaign, Task task, OperationResult result) {
CertCampaignStageEvent event = certEventFactory.createOnCampaignStageEndEvent(campaign, task);
CertCampaignStageEvent event = certEventFactory.createOnCampaignStageEndEvent(campaign, task, result);
eventHelper.processEvent(event, task, result);
}

@Override
public void onReviewRequested(ObjectReferenceType reviewerOrDeputyRef, ObjectReferenceType actualReviewerRef,
List<AccessCertificationCaseType> cases, AccessCertificationCampaignType campaign, Task task, OperationResult result) {
CertReviewEvent event = certEventFactory.createReviewRequestedEvent(reviewerOrDeputyRef, actualReviewerRef, cases, campaign, task);
CertReviewEvent event = certEventFactory.createReviewRequestedEvent(reviewerOrDeputyRef, actualReviewerRef, cases, campaign, task, result);
eventHelper.processEvent(event, task, result);
}

@Override
public void onReviewDeadlineApproaching(ObjectReferenceType reviewerOrDeputyRef, ObjectReferenceType actualReviewerRef,
List<AccessCertificationCaseType> cases, AccessCertificationCampaignType campaign, Task task, OperationResult result) {
CertReviewEvent event = certEventFactory.createReviewDeadlineApproachingEvent(reviewerOrDeputyRef, actualReviewerRef, cases, campaign, task);
CertReviewEvent event = certEventFactory.createReviewDeadlineApproachingEvent(reviewerOrDeputyRef, actualReviewerRef, cases, campaign, task, result);
eventHelper.processEvent(event, task, result);
}
}

0 comments on commit c7dca6c

Please sign in to comment.