Skip to content

Commit

Permalink
SAK-32285 Create audit database records when adding or deleting users… (
Browse files Browse the repository at this point in the history
sakaiproject#4366)

* SAK-32285 Create audit database records when adding or deleting users via the entity broker.

* SAK-32285 Fixed tests and resolved bug for entity broker auditing.

* SAK-32285 Minor changes made following feedback.
  • Loading branch information
RebeccaMiller-Which authored and ern committed May 16, 2017
1 parent 68ce6b6 commit 5aeb5f9
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 8 deletions.
4 changes: 4 additions & 0 deletions entitybroker/core-providers/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@
<groupId>org.sakaiproject.kernel</groupId>
<artifactId>sakai-kernel-util</artifactId>
</dependency>
<dependency>
<groupId>org.sakaiproject.userauditservice</groupId>
<artifactId>userauditservice-api</artifactId>
</dependency>
</dependencies>
<build>
<resources>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
import java.util.Map;
import java.util.Set;

import org.sakaiproject.user.api.User;
import org.sakaiproject.user.api.UserDirectoryService;
import org.sakaiproject.user.api.UserNotDefinedException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sakaiproject.api.privacy.PrivacyManager;
Expand Down Expand Up @@ -64,6 +67,8 @@
import org.sakaiproject.site.api.Site;
import org.sakaiproject.site.api.SiteService;
import org.sakaiproject.site.api.SiteService.SelectionType;
import org.sakaiproject.userauditservice.api.UserAuditRegistration;
import org.sakaiproject.userauditservice.api.UserAuditService;

import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.core.type.TypeReference;
Expand Down Expand Up @@ -113,6 +118,16 @@ public void setSecurityService(SecurityService securityService){
this.securityService = securityService;
}

private static UserDirectoryService userDirectoryService;
public void setUserDirectoryService(UserDirectoryService userDirectoryService) {
this.userDirectoryService = userDirectoryService;
}

private static UserAuditRegistration userAuditRegistration;
public void setUserAuditRegistration(UserAuditRegistration userAuditRegistration) {
this.userAuditRegistration = userAuditRegistration;
}

public static String PREFIX = "membership";

public String getEntityPrefix() {
Expand Down Expand Up @@ -175,13 +190,19 @@ public boolean unjoinCurrentUserFromSite(EntityView view, Map<String, Object> pa
} else if ("site".equals(siteId)) {
siteId = view.getPathSegment(3);
}

if (siteId == null) {
throw new IllegalArgumentException(
"siteId must be set in order to unjoin sites, set in params or in the URL /unjoin/site/siteId");
}
checkSiteSecurity(siteId);
try {
siteService.unjoin(siteId);
//String user = sessionManager().getCurrentSessionUserId();
String currentUserEid = userEntityProvider.getCurrentUser(view).getEid(); //userDirectoryService.getCurrentUser().getEid();
String roleId = siteService.getSite(siteId).getJoinerRole();
List<String[]> userAuditList = Collections.singletonList(new String[]{siteId,currentUserEid,roleId, UserAuditService.USER_AUDIT_ACTION_REMOVE,userAuditRegistration.getDatabaseSourceKey(),currentUserEid});
userAuditRegistration.addToUserAuditing(userAuditList);
} catch (IdUnusedException e) {
throw new IllegalArgumentException("The siteId provided (" + siteId
+ ") could not be found: " + e, e);
Expand Down Expand Up @@ -840,11 +861,15 @@ public String createEntity(EntityReference ref, Object entity, Map<String, Objec

checkSiteSecurity(sg.site.getId());

String[] userAuditString;
List<String[]> userAuditList = new ArrayList<>();

// check for a batch add
String[] userIds = checkForBatch(params, userId);
// now add all the memberships
String memberId = "";
String currentUserId = developerHelperService.getCurrentUserId();

// now add all the memberships
for (int i = 0; i < userIds.length; i++) {
if (sg.group == null) {
// site only
Expand All @@ -862,6 +887,17 @@ public String createEntity(EntityReference ref, Object entity, Map<String, Objec
sg.site.addMember(userIds[i], roleId, active, false);
saveSiteMembership(sg.site);
}
User user = null;
// Add change to user_audits_log table.
try {
user = userDirectoryService.getUser(userIds[i]);
}
catch (UserNotDefinedException e) {
log.error(".createEntity: User with id {} doesn't exist", userIds[i]);
}
userAuditString = new String[]{sg.site.getId(),user.getDisplayId(), roleId, UserAuditService.USER_AUDIT_ACTION_ADD,
userAuditRegistration.getDatabaseSourceKey(), currentUserId};
userAuditList.add(userAuditString);
} else {
// group and site
try {
Expand All @@ -877,6 +913,11 @@ public String createEntity(EntityReference ref, Object entity, Map<String, Objec
memberId = em.getId();
}
}

if (userAuditList.size() > 0) {
userAuditRegistration.addToUserAuditing(userAuditList);
}

if (userIds.length > 1) {
log.info("Batch add memberships: siteId="
+ ((sg.site == null) ? "none" : sg.site.getId()) + ",groupId="
Expand Down Expand Up @@ -908,13 +949,33 @@ public void deleteEntity(EntityReference ref, Map<String, Object> params) {
}
String userId = parts[0];
SiteGroup sg = findLocationByReference(parts[1]);

String[] userAuditString;
List<String[]> userAuditList = new ArrayList<>();

// check for a batch
String[] userIds = checkForBatch(params, userId);
for (int i = 0; i < userIds.length; i++) {
if (sg.group == null) {
// site only
sg.site.removeMember(userIds[i]);
saveSiteMembership(sg.site);
Site site = sg.site;

// Add change to user_audits_log table.
String role = site.getUserRole(userIds[i]).getId();
String displayId = null;
try {
displayId = userDirectoryService.getUser(userIds[i]).getDisplayId();
} catch (UserNotDefinedException e) {
log.error(".deleteEntity: User with id {} not defined", userIds[i]);
}

userAuditString = new String[]{site.getId(), displayId, role, UserAuditService.USER_AUDIT_ACTION_REMOVE,
userAuditRegistration.getDatabaseSourceKey(), developerHelperService.getCurrentUserId()};
userAuditList.add(userAuditString);

site.removeMember(userIds[i]);
saveSiteMembership(site);

} else {
// group and site
try {
Expand All @@ -925,6 +986,11 @@ public void deleteEntity(EntityReference ref, Map<String, Object> params) {
}
}
}

if (userAuditList.size() > 0) {
userAuditRegistration.addToUserAuditing(userAuditList);
}

if (userIds.length > 1) {
log.info("Batch remove memberships: siteId="
+ ((sg.site == null) ? "none" : sg.site.getId()) + ",groupId="
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.sakaiproject.authz.api.Member;
import org.sakaiproject.authz.api.Role;
import org.sakaiproject.entity.api.ResourceProperties;
import org.sakaiproject.entitybroker.DeveloperHelperService;
import org.sakaiproject.entitybroker.EntityReference;
Expand All @@ -40,6 +42,10 @@
import org.sakaiproject.site.api.Group;
import org.sakaiproject.site.api.Site;
import org.sakaiproject.site.api.SiteService;
import org.sakaiproject.user.api.User;
import org.sakaiproject.user.api.UserDirectoryService;
import org.sakaiproject.user.api.UserNotDefinedException;
import org.sakaiproject.userauditservice.api.UserAuditRegistration;
import org.sakaiproject.util.BaseResourceProperties;
import org.sakaiproject.authz.api.SecurityService;
import org.sakaiproject.api.privacy.PrivacyManager;
Expand All @@ -64,16 +70,25 @@ public class MembershipEntityProviderTest {
private SecurityService securityService;
@Mock
private PrivacyManager privacyManager;
private UserDirectoryService userDirectoryService;
private UserAuditRegistration userAuditRegistrationService;

@Before
public void setUp() {
public void setUp() throws UserNotDefinedException {
MockitoAnnotations.initMocks(this);
provider = new MembershipEntityProvider();
provider.setSiteService(siteService);
provider.setDeveloperHelperService(developerHelperService);
provider.setUserEntityProvider(userEntityProvider);
provider.setSecurityService(securityService);
provider.setPrivacyManager(privacyManager);
userDirectoryService = Mockito.mock(UserDirectoryService.class);
userAuditRegistrationService = Mockito.mock(UserAuditRegistration.class);
provider.setUserDirectoryService(userDirectoryService);
provider.setUserAuditRegistration(userAuditRegistrationService);
provider.setPrivacyManager(privacyManager);

User user = mock(User.class);
when(userDirectoryService.getUser("user-foo")).thenReturn(user);
}

@Test
Expand Down Expand Up @@ -406,7 +421,7 @@ public void getEntitiesPreservesDotsInGroupIds() throws IdUnusedException {
// org.sakaiproject.mock.domain.Member to createEntity() doesn't actually work.

@Test
public void createEntityPreservesDotsInSiteIdsInEntityMembers() throws IdUnusedException, PermissionException {
public void createEntityPreservesDotsInSiteIdsInEntityMembers() throws IdUnusedException, PermissionException, UserNotDefinedException {
EntityMember member = new EntityMember();
member.setUserId("user-foo");
member.setMemberRole("role-foo");
Expand Down Expand Up @@ -467,6 +482,10 @@ public void deleteEntityPreservesDotsInSiteIds() throws IdUnusedException, Permi
Member member = mock(Member.class);
when(member.getUserId()).thenReturn("user-foo");
when(member.getUserEid()).thenReturn("user-foo");
Role role = mock(Role.class);
when(role.getId()).thenReturn("role-foo");
when(site.getUserRole("user-foo")).thenReturn(role);

Set<Member> members = new HashSet<Member>();
members.add(member);
when(site.getMembers()).thenReturn(members);
Expand Down Expand Up @@ -544,6 +563,14 @@ public void unjoinCurrentUserToSitePreservesDotsInSiteIdPathParams_Format1() thr
entityView.setMethod(EntityView.Method.POST);
entityView.setViewKey(EntityView.VIEW_NEW);

EntityUser entityUser = mock(EntityUser.class);
when(entityUser.getEid()).thenReturn("user-foo");
when(userEntityProvider.getCurrentUser(entityView)).thenReturn(entityUser);

Site site = mock(Site.class);
when(site.getJoinerRole()).thenReturn("role-foo");
when(siteService.getSite("site.with.dots")).thenReturn(site);

assertTrue(provider.unjoinCurrentUserFromSite(entityView, new HashMap<String, Object>()));
verify(siteService).unjoin("site.with.dots");
}
Expand All @@ -554,6 +581,14 @@ public void unjoinCurrentUserToSitePreservesDotsInSiteIdPathParams_Format2() thr
entityView.setMethod(EntityView.Method.POST);
entityView.setViewKey(EntityView.VIEW_NEW);

EntityUser entityUser = mock(EntityUser.class);
when(entityUser.getEid()).thenReturn("user-foo");
when(userEntityProvider.getCurrentUser(entityView)).thenReturn(entityUser);

Site site = mock(Site.class);
when(site.getJoinerRole()).thenReturn("role-foo");
when(siteService.getSite("site.with.dots")).thenReturn(site);

assertTrue(provider.unjoinCurrentUserFromSite(entityView, new HashMap<String, Object>()));
verify(siteService).unjoin("site.with.dots");
}
Expand All @@ -566,6 +601,14 @@ public void unjoinCurrentUserToSitePreservesDotsInSiteIdQueryParams() throws Per
Map<String,Object> params = new HashMap<String,Object>();
params.put("siteId", "site.with.dots");

EntityUser entityUser = mock(EntityUser.class);
when(entityUser.getEid()).thenReturn("user-foo");
when(userEntityProvider.getCurrentUser(entityView)).thenReturn(entityUser);

Site site = mock(Site.class);
when(site.getJoinerRole()).thenReturn("role-foo");
when(siteService.getSite("site.with.dots")).thenReturn(site);

assertTrue(provider.unjoinCurrentUserFromSite(entityView, params));
verify(siteService).unjoin("site.with.dots");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@
<property name="userEntityProvider" ref="org.sakaiproject.entitybroker.providers.UserEntityProvider" />
<property name="emailService" ref="org.sakaiproject.email.api.EmailService" />
<property name="privacyManager" ref="org.sakaiproject.api.privacy.PrivacyManager" />
<property name="securityService" ref="org.sakaiproject.authz.api.SecurityService" />
<property name="securityService" ref="org.sakaiproject.authz.api.SecurityService" />
<property name="userDirectoryService" ref="org.sakaiproject.user.api.UserDirectoryService" />
<property name="userAuditRegistration" ref="org.sakaiproject.userauditservice.api.UserAuditRegistration.direct" />
</bean>

<bean parent="org.sakaiproject.entitybroker.entityprovider.AbstractEntityProvider"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ M = {0} ({1})
S = Self
#M will display as Last Name, First Name (user's eid)
M = {0} ({1})
S = Self
S = Self
D = Entity broker ({1})
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,15 @@
<property name="databaseSourceKey"><value>S</value></property>
</bean>

<bean id="org.sakaiproject.userauditservice.api.UserAuditRegistration.direct"
parent="org.sakaiproject.userauditservice.api.UserAuditRegistration"
class="org.sakaiproject.sitemanage.impl.UserAuditSiteManageImpl"
init-method="init">
<property name="bundleLocation"><value>UserAuditService</value></property>
<property name="databaseSourceKey"><value>D</value></property>
<property name="hasParameters"><value>true</value></property>
</bean>

<bean id="org.sakaiproject.sitemanage.impl.job.SeedSitesAndUsersJob"
class="org.sakaiproject.sitemanage.impl.job.SeedSitesAndUsersJob"
init-method="init">
Expand Down

0 comments on commit 5aeb5f9

Please sign in to comment.