Fix lastModifiedBy on modification perfomed by the JpaRolloutExecutor (#1748)

1. The auditor is got on transaction commit - so haven't used the tenant & user context until now - write system
2. The start/stop/delete are called by the user (saved in lastModifiedBy) but then executed in JpaRolloutExecutor

So the change is:
1. Fix auditor for actions taken by JpaRolloutExecutor to be the createdBy
2. for start/stop/delete the auditor is set to the lastModifiedBy for the transaction (hence all action taken)

Signed-off-by: Marinov Avgustin <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2024-06-21 08:27:24 +03:00
committed by GitHub
parent b42765b4eb
commit 6e6f96a0f4
4 changed files with 55 additions and 7 deletions

3
.gitignore vendored
View File

@@ -55,6 +55,9 @@ classpath-data.json
# Intellij
*.iml
# local Spring configs
application-local.properties
# Maven
maven.properties
.flattened-pom.xml

View File

@@ -23,7 +23,6 @@ import jakarta.persistence.EntityManager;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.hawkbit.repository.DeploymentManagement;
import org.eclipse.hawkbit.repository.QuotaManagement;
import org.eclipse.hawkbit.repository.RepositoryProperties;
import org.eclipse.hawkbit.repository.RolloutApprovalStrategy;
import org.eclipse.hawkbit.repository.RolloutExecutor;
import org.eclipse.hawkbit.repository.RolloutGroupManagement;
@@ -60,6 +59,7 @@ import org.eclipse.hawkbit.repository.model.RolloutGroup.RolloutGroupStatus;
import org.eclipse.hawkbit.repository.model.RolloutGroup.RolloutGroupSuccessCondition;
import org.eclipse.hawkbit.repository.model.Target;
import org.eclipse.hawkbit.repository.model.helper.EventPublisherHolder;
import org.eclipse.hawkbit.security.SpringSecurityAuditorAware;
import org.eclipse.hawkbit.tenancy.TenantAware;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Slice;
@@ -152,20 +152,41 @@ public class JpaRolloutExecutor implements RolloutExecutor {
case CREATING:
handleCreateRollout((JpaRollout) rollout);
break;
case DELETING:
handleDeleteRollout((JpaRollout) rollout);
break;
case READY:
handleReadyRollout(rollout);
break;
case STARTING:
handleStartingRollout(rollout);
// the lastModifiedBy user is probably the user that has actually called the rollout start (unless overridden) - not the creator
SpringSecurityAuditorAware.setAuditorOverride(rollout.getLastModifiedBy());
try {
handleStartingRollout(rollout);
} finally {
// clear, ALWAYS, the set auditor override
SpringSecurityAuditorAware.clearAuditorOverride();
}
break;
case RUNNING:
handleRunningRollout((JpaRollout) rollout);
break;
case STOPPING:
handleStopRollout((JpaRollout) rollout);
// the lastModifiedBy user is probably the user that has actually called the rollout stop (unless overridden) - not the creator
SpringSecurityAuditorAware.setAuditorOverride(rollout.getLastModifiedBy());
try {
handleStopRollout((JpaRollout) rollout);
} finally {
// clear, ALWAYS, the set auditor override
SpringSecurityAuditorAware.clearAuditorOverride();
}
break;
case DELETING:
// the lastModifiedBy user is probably the user that has actually called the rollout delete (unless overridden) - not the creator
SpringSecurityAuditorAware.setAuditorOverride(rollout.getLastModifiedBy());
try {
handleDeleteRollout((JpaRollout) rollout);
} finally {
// clear, ALWAYS, the set auditor override
SpringSecurityAuditorAware.clearAuditorOverride();
}
break;
default:
log.error("Rollout in status {} not supposed to be handled!", rollout.getStatus());

View File

@@ -104,6 +104,8 @@ public class JpaRolloutHandler implements RolloutHandler {
DeploymentHelper.runInNewTransaction(txManager, handlerId + "-" + rolloutId, status -> {
rolloutManagement.get(rolloutId).ifPresentOrElse(
rollout -> {
// auditor is retrieved and set on transaction commit
// if not overridden, the system user will be the auditor
rollout.getAccessControlContext().ifPresentOrElse(
context -> // has stored context - executes it with it
contextAware.runInContext(

View File

@@ -24,8 +24,17 @@ import org.springframework.security.oauth2.core.oidc.user.OidcUser;
*/
public class SpringSecurityAuditorAware implements AuditorAware<String> {
// Sometimes 'system' need to override the auditor when do create/modify actions in context of a tenant and user.
// Though this could be made using runAsTenantAsUser sometimes (as in transaction) this override is needed
// after runAsTenantAsUser (because it seems that auditor is got in commit time).
// So this thread local variable provides option to override explicitly the auditor.
private static final ThreadLocal<String> AUDITOR_OVERRIDE = new ThreadLocal<>();
@Override
public Optional<String> getCurrentAuditor() {
if (AUDITOR_OVERRIDE.get() != null) {
return Optional.of(AUDITOR_OVERRIDE.get());
}
final Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
@@ -36,7 +45,20 @@ public class SpringSecurityAuditorAware implements AuditorAware<String> {
return Optional.ofNullable(getCurrentAuditor(authentication));
}
private static String getCurrentAuditor(final Authentication authentication) {
// Always shall be followed by {@link #clearAuditorOverride}
public static void setAuditorOverride(final String auditor) {
if (auditor == null) {
AUDITOR_OVERRIDE.remove();
} else {
AUDITOR_OVERRIDE.set(auditor);
}
}
public static void clearAuditorOverride() {
AUDITOR_OVERRIDE.remove();
}
protected String getCurrentAuditor(final Authentication authentication) {
if (authentication.getPrincipal() instanceof UserDetails) {
return ((UserDetails) authentication.getPrincipal()).getUsername();
}