-
Notifications
You must be signed in to change notification settings - Fork 43
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
AAE-23566 Fix Audit Event Message Don't Follow Schema #1496
base: develop
Are you sure you want to change the base?
Changes from all commits
f2b5d3e
f773c8f
8c5b51e
2b3a7de
28af2de
6b8cbd5
6fa1ceb
bcc05d1
0e4406b
aa90753
0dd38da
d622bab
b11c01c
9d4a867
05123b7
4cbdb40
34da48e
add047b
6cdabcf
fb898df
6b9b586
7f4a0ac
f03ef60
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 |
---|---|---|
|
@@ -131,5 +131,9 @@ | |
<artifactId>spring-boot-configuration-processor</artifactId> | ||
<optional>true</optional> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.activiti.cloud</groupId> | ||
<artifactId>activiti-cloud-services-common-security</artifactId> | ||
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 should not use the security dependency inside the events module. This is will put Spring Security and Activiti Security classes on the application classpath, which may have the consequences by including transitive dependencies downstream. |
||
</dependency> | ||
</dependencies> | ||
</project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
import org.activiti.api.process.runtime.events.listener.ProcessEventListener; | ||
import org.activiti.api.runtime.shared.security.PrincipalIdentityProvider; | ||
import org.activiti.api.runtime.shared.security.SecurityContextPrincipalProvider; | ||
import org.activiti.cloud.identity.IdentityService; | ||
import org.activiti.cloud.services.events.ActorConstants; | ||
import org.activiti.engine.RuntimeService; | ||
|
||
|
@@ -29,15 +30,18 @@ public class ProcessStartedActorProviderEventListener implements ProcessEventLis | |
private final RuntimeService runtimeService; | ||
private final SecurityContextPrincipalProvider securityContextPrincipalProvider; | ||
private final PrincipalIdentityProvider principalIdentityProvider; | ||
private final IdentityService identityService; | ||
|
||
public ProcessStartedActorProviderEventListener( | ||
RuntimeService runtimeService, | ||
SecurityContextPrincipalProvider securityContextPrincipalProvider, | ||
PrincipalIdentityProvider principalIdentityProvider | ||
PrincipalIdentityProvider principalIdentityProvider, | ||
IdentityService identityService | ||
) { | ||
this.runtimeService = runtimeService; | ||
this.securityContextPrincipalProvider = securityContextPrincipalProvider; | ||
this.principalIdentityProvider = principalIdentityProvider; | ||
this.identityService = identityService; | ||
} | ||
|
||
@Override | ||
|
@@ -51,7 +55,7 @@ public void onEvent(ProcessCreatedEvent event) { | |
.ifPresent(details -> | ||
runtimeService.addUserIdentityLink( | ||
event.getEntity().getId(), | ||
principalIdentityProvider.getUserId(principal), | ||
identityService.findUserByName(event.getEntity().getInitiator()).getId(), | ||
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. It is not a good idea to use the identity service to enrich the identity link with initiator user guid, which will make an external rest api call to identity provider inside engine transaction. It will cause a delay for every new process instance which will impact the performance of the system in a negative way. Besides, not every process instance will have an initiator, because it may be created by system on timer or message events. In this case, the initiator will be null and it will result in NPE. |
||
ActorConstants.ACTOR_TYPE, | ||
details | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
|
||
import org.activiti.api.runtime.shared.identity.UserGroupManager; | ||
import org.activiti.api.runtime.shared.security.SecurityManager; | ||
import org.activiti.cloud.identity.IdentityService; | ||
import org.activiti.core.common.spring.security.policies.ProcessSecurityPoliciesManager; | ||
import org.activiti.engine.ManagementService; | ||
import org.activiti.engine.RepositoryService; | ||
|
@@ -54,6 +55,9 @@ static class MockRuntimeBundleApplication { | |
|
||
@MockBean | ||
private ProcessSecurityPoliciesManager processSecurityPoliciesManager; | ||
|
||
@MockBean | ||
private IdentityService identityService; | ||
Comment on lines
+58
to
+60
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. Let's not use mock beans in ITs. |
||
} | ||
|
||
@Test | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -172,5 +172,10 @@ | |
<groupId>org.activiti.cloud</groupId> | ||
<artifactId>activiti-cloud-services-connectors</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.springframework.security</groupId> | ||
<artifactId>spring-security-test</artifactId> | ||
<scope>test</scope> | ||
</dependency> | ||
Comment on lines
+175
to
+179
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. Why this dependency is needed? |
||
</dependencies> | ||
</project> |
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 should avoid using mock beans in integration tests.