Skip to content

Commit

Permalink
feat: [CO-526] Prevent delegated admins creating DDLs (#145)
Browse files Browse the repository at this point in the history
* fix: [CO-526] prevent delegated admins creating DDLs

* chore: cleanup optimize imports and formatting

* chore: add unit tests for CreateDistributionList

should throw ServiceException when a delegatedAdmin requests to create dynamic distribution list using CreateDistributionList SOAP command

* chore: update SOAP api doc
  • Loading branch information
keshavbhatt authored Jan 30, 2023
1 parent 2096db6 commit 9090306
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
* <br />
* Notes:
* <ul>
* <li> dynamic - create a dynamic distribution list
* <li> dynamic - create a dynamic distribution list (global admins only)
* <li> Extra attrs: <b>description</b>, <b>zimbraNotes</b>
* </ul>
* <b>Access</b>: domain admin sufficient
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,97 +5,96 @@

package com.zimbra.cs.service.admin;

import java.util.List;
import java.util.Map;

import org.apache.commons.lang.StringUtils;

import com.zimbra.common.service.ServiceException;
import com.zimbra.common.soap.AdminConstants;
import com.zimbra.common.soap.Element;
import com.zimbra.common.util.ZimbraLog;
import com.zimbra.cs.account.Group;
import com.zimbra.cs.account.Provisioning;
import com.zimbra.cs.account.accesscontrol.AdminRight;
import com.zimbra.cs.account.accesscontrol.Rights.Admin;
import com.zimbra.cs.account.accesscontrol.TargetType;
import com.zimbra.soap.JaxbUtil;
import com.zimbra.cs.account.accesscontrol.generated.AdminRights;
import com.zimbra.soap.ZimbraSoapContext;
import com.zimbra.soap.admin.message.CreateDistributionListRequest;
import java.util.List;
import java.util.Map;
import org.apache.commons.lang.StringUtils;

public class CreateDistributionList extends AdminDocumentHandler {

/**
* must be careful and only allow access to domain if domain admin
*/
@Override
public boolean domainAuthSufficient(Map context) {
return true;
}

/**
* @return true - which means accept responsibility for measures to prevent account harvesting by delegate admins
*/
@Override
public boolean defendsAgainstDelegateAdminAccountHarvesting() {
return true;
/**
* must be careful and only allow access to domain if domain admin
*/
@Override
public boolean domainAuthSufficient(Map<String, Object> context) {
return true;
}

/**
* @return true - which means accept responsibility for measures to prevent account harvesting by
* delegate admins
*/
@Override
public boolean defendsAgainstDelegateAdminAccountHarvesting() {
return true;
}

@Override
public Element handle(Element request, Map<String, Object> context)
throws ServiceException {

ZimbraSoapContext zsc = getZimbraSoapContext(context);
Provisioning prov = Provisioning.getInstance();
CreateDistributionListRequest req = zsc.elementToJaxb(request);

if (StringUtils.isEmpty(req.getName())) {
throw ServiceException.INVALID_REQUEST(String.format("missing %s", AdminConstants.E_NAME),
null);
}

@Override
public Element handle(Element request, Map<String, Object> context)
throws ServiceException {
String name = req.getName().toLowerCase();

ZimbraSoapContext zsc = getZimbraSoapContext(context);
Provisioning prov = Provisioning.getInstance();
CreateDistributionListRequest req = zsc.elementToJaxb(request);
Map<String, Object> attrs = req.getAttrsAsOldMultimap(true);

if (StringUtils.isEmpty(req.getName())) {
throw ServiceException.INVALID_REQUEST(String.format("missing %s", AdminConstants.E_NAME), null);
}
boolean dynamic = Boolean.TRUE.equals(req.getDynamic());

String name = req.getName().toLowerCase();

Map<String, Object> attrs = req.getAttrsAsOldMultimap(true);
if (dynamic) {
// see issue CO-526
if (zsc.getAuthToken().isDelegatedAdmin()) {
throw ServiceException.INVALID_REQUEST(
"Delegated Admins are not allowed to create Dynamic Distribution Lists", null);
}
checkDomainRightByEmail(zsc, name, AdminRights.R_createGroup);
checkSetAttrsOnCreate(zsc, TargetType.group, name, attrs);
} else {
checkDomainRightByEmail(zsc, name, AdminRights.R_createDistributionList);
checkSetAttrsOnCreate(zsc, TargetType.dl, name, attrs);
}

boolean dynamic = Boolean.TRUE.equals(req.getDynamic());
Group group = prov.createGroup(name, attrs, dynamic);

if (dynamic) {
checkDomainRightByEmail(zsc, name, Admin.R_createGroup);
checkSetAttrsOnCreate(zsc, TargetType.group, name, attrs);
} else {
checkDomainRightByEmail(zsc, name, Admin.R_createDistributionList);
checkSetAttrsOnCreate(zsc, TargetType.dl, name, attrs);
}
ZimbraLog.security.info(ZimbraLog.encodeAttrs(
new String[]{"cmd", "CreateDistributionList", "name", name}, attrs));

preGroupCreation(request, zsc, attrs);
Group group = prov.createGroup(name, attrs, dynamic);
Element response = getResponseElement(zsc);

ZimbraLog.security.info(ZimbraLog.encodeAttrs(
new String[] {"cmd", "CreateDistributionList","name", name}, attrs));
GetDistributionList.encodeDistributionList(response, group);

Element response = getResponseElement(zsc);
return response;
}

GetDistributionList.encodeDistributionList(response, group);
@Override
public void docRights(List<AdminRight> relatedRights, List<String> notes) {
relatedRights.add(AdminRights.R_createDistributionList);
relatedRights.add(AdminRights.R_createGroup);
notes.add(String.format(AdminRightCheckPoint.Notes.MODIFY_ENTRY,
AdminRights.R_modifyDistributionList.getName(), "distribution list"));
notes.add(String.format(AdminRightCheckPoint.Notes.MODIFY_ENTRY,
AdminRights.R_modifyGroup.getName(), "group"));
}

return response;
}

@Override
public void docRights(List<AdminRight> relatedRights, List<String> notes) {
relatedRights.add(Admin.R_createDistributionList);
relatedRights.add(Admin.R_createGroup);
notes.add(String.format(AdminRightCheckPoint.Notes.MODIFY_ENTRY,
Admin.R_modifyDistributionList.getName(), "distribution list"));
notes.add(String.format(AdminRightCheckPoint.Notes.MODIFY_ENTRY,
Admin.R_modifyGroup.getName(), "group"));
}

protected void preGroupCreation(Element request, ZimbraSoapContext zsc, Map<String, Object> attrs)
throws ServiceException {
// do nothing
}

protected Element getResponseElement(ZimbraSoapContext zsc) {
return zsc.createElement(AdminConstants.CREATE_DISTRIBUTION_LIST_RESPONSE);
}
@Override
protected Element getResponseElement(ZimbraSoapContext zsc) {
return zsc.createElement(AdminConstants.CREATE_DISTRIBUTION_LIST_RESPONSE);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package com.zimbra.cs.service.admin;

import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

import com.zimbra.common.account.ProvisioningConstants;
import com.zimbra.common.service.ServiceException;
import com.zimbra.common.soap.SoapProtocol;
import com.zimbra.cs.account.Account;
import com.zimbra.cs.account.AuthToken;
import com.zimbra.cs.account.DynamicGroup;
import com.zimbra.cs.account.Provisioning;
import com.zimbra.cs.mailbox.MailboxTestUtil;
import com.zimbra.soap.JaxbUtil;
import com.zimbra.soap.SoapEngine;
import com.zimbra.soap.ZimbraSoapContext;
import com.zimbra.soap.admin.message.CreateDistributionListRequest;
import com.zimbra.soap.admin.type.Attr;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

public class CreateDistributionListTest {

private final static String DOMAIN_NAME = "test.com";
private final static String ADMIN_USER_NAME = "admin";
private final static String DOMAIN_ADMIN_EMAIL = ADMIN_USER_NAME + "@" + DOMAIN_NAME;
private final static String DISTRIBUTION_LIST_NAME = "developers" + "@" + DOMAIN_NAME;
private final static String DOMAIN_ADMIN_PASSWORD = "assext";

private static Provisioning provisioningSpy;

@Rule
public final ExpectedException exceptionRule = ExpectedException.none();

@BeforeClass
public static void init() throws Exception {
MailboxTestUtil.initServer();
provisioningSpy = spy(Provisioning.getInstance());
}

@AfterClass
public static void tearDown() throws Exception {
MailboxTestUtil.clearData();
}

/**
* Delegated Admins are not allowed to create Dynamic Distribution Lists see issue CO-526
*
* @throws ServiceException if any
*/
@Test
public void shouldThrowServiceExceptionWhenDelegatedAdminRequestToCreateDynamicDistributionList()
throws ServiceException {

final HashMap<String, Object> attrs = new HashMap<>();

//create domain
provisioningSpy.createDomain(DOMAIN_NAME, attrs);

//create domain admin user
attrs.put(Provisioning.A_zimbraIsDelegatedAdminAccount,
ProvisioningConstants.TRUE);
final Account domainAdminAccount = provisioningSpy.createAccount(DOMAIN_ADMIN_EMAIL,
DOMAIN_ADMIN_PASSWORD,
attrs);

//AuthToken mock, set auth account as delegatedAdmin
final AuthToken authTokenMock = mock(AuthToken.class);
when(authTokenMock.isDelegatedAdmin()).thenReturn(true);

//create soap context with delegated admin auth
final Map<String, Object> context = new HashMap<>();
final ZimbraSoapContext zsc = new ZimbraSoapContext(authTokenMock, domainAdminAccount.getId(),
SoapProtocol.Soap12, SoapProtocol.Soap12);
context.put(SoapEngine.ZIMBRA_CONTEXT, zsc);

//create CreateDistributionListRequest
List<Attr> attributes = new ArrayList<>();
attributes.add(new Attr(Provisioning.A_memberURL,
"ldap:///??sub?(&amp;(objectClass=zimbraAccount)(ZimbraAccountStatus=active))"));
attributes.add(new Attr(Provisioning.A_zimbraIsACLGroup, ProvisioningConstants.TRUE));
final CreateDistributionListRequest createDistributionListRequest = new CreateDistributionListRequest(
DISTRIBUTION_LIST_NAME, attributes, true);

//create CreateDistributionList
final CreateDistributionList createDistributions = new CreateDistributionList();

//stub createGroup method of provisioning since it is not supported without real provisioning backend
final DynamicGroup mockDGroup = mock(DynamicGroup.class);
doReturn(mockDGroup).when(provisioningSpy).createGroup(anyString(), anyMap(), anyBoolean());

//setup tests cases
exceptionRule.expect(ServiceException.class);
exceptionRule.expectMessage(
"Delegated Admins are not allowed to create Dynamic Distribution Lists");

//execute request
createDistributions.handle(JaxbUtil.jaxbToElement(createDistributionListRequest), context);
}
}

0 comments on commit 9090306

Please sign in to comment.