Skip to content

Commit

Permalink
Merge pull request #7215 from SalesforceFoundation/feature/248__secur…
Browse files Browse the repository at this point in the history
…ity-bug-fixes

Security Bug Fixes
  • Loading branch information
npsp-reedestockton authored Nov 16, 2023
2 parents 0432b3a + d8ff2e9 commit bd0b2e9
Show file tree
Hide file tree
Showing 19 changed files with 563 additions and 68 deletions.
44 changes: 43 additions & 1 deletion force-app/main/default/classes/CON_ContactMerge_CTRL.cls
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ public with sharing class CON_ContactMerge_CTRL {

public Boolean canContinueWithMerge { get;set; }

public List<FieldSetMember> fieldSetMembers {
get {
if (fieldSetMembers == null) {
fieldSetMembers = SObjectType.Contact.fieldSets.ContactMergeFoundFS.getFields();
}
return fieldSetMembers;
}
set;
}

public Boolean hasContactObjectDeletePermission() {
return UTIL_Describe.getObjectDescribe('Contact').isDeletable();
}
Expand Down Expand Up @@ -923,7 +933,7 @@ public with sharing class CON_ContactMerge_CTRL {
public PageReference search() {
try {
step = 2;
this.searchResults = wrapQueryResults(searchRecords());
this.searchResults = wrapQueryResults(stripInaccessibleResultFields(searchRecords()));


} catch (exception ex) {
Expand All @@ -949,6 +959,33 @@ public with sharing class CON_ContactMerge_CTRL {
}
}

private List<SObject> stripInaccessibleResultFields(List<SObject> searchResults) {
SObjectAccessDecision accessDecision =
Security.stripInaccessible(AccessType.READABLE, searchResults);

Map<String, Set<String>> removedFields = accessDecision.getRemovedFields();
if (!removedFields.isEmpty()) {
List<FieldSetMember> strippedFieldSetMembers = new List<FieldSetMember>();
for (FieldSetMember fsMember:fieldSetMembers) {
Boolean fieldRemoved = false;
for (String field:removedFields.get('Contact')) {
if (fieldRemoved) {
continue;
}
if (fsMember.getFieldPath().contains(field)) {
fieldRemoved = true;
}
}
if (!fieldRemoved) {
strippedFieldSetMembers.add(fsMember);
}
}
fieldSetMembers = strippedFieldSetMembers;
}

return accessDecision.getRecords();
}

/***********************************************************************************************
* @description Wraps the Query(SOSL and SOQL) results.
* @param searchResults The list of SObjects to wrap.
Expand All @@ -971,6 +1008,11 @@ public with sharing class CON_ContactMerge_CTRL {
* @return PageReference The page that it redirects to. Same page user is in.
*/
public PageReference mergeContacts() {
if (!(Contact.SObjectType.getDescribe().isMergeable()) && Account.SObjectType.getDescribe().isMergeable()){
ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.Error, System.Label.commonAccessErrorMessage));
return null;
}

Contact winningContact = getWinningContact();

if (winningContact == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,15 @@ public with sharing class CON_DeleteContactOverrideSelector {
}

public Account getAccountRecord(Id accountId) {
return [SELECT Id, Name FROM Account WHERE Id = :accountId];
return [SELECT Id, Name FROM Account WHERE Id = :accountId WITH SECURITY_ENFORCED];
}

public List<Case> getCasesRelatedToContact(Id contactId) {
return [
SELECT CaseNumber, ContactId
FROM Case
WHERE ContactId = :contactId
WITH SECURITY_ENFORCED
];
}

Expand All @@ -91,6 +92,7 @@ public with sharing class CON_DeleteContactOverrideSelector {
SELECT CaseNumber, AccountId
FROM Case
WHERE AccountId = :accountId
WITH SECURITY_ENFORCED
];
}

Expand All @@ -99,6 +101,7 @@ public with sharing class CON_DeleteContactOverrideSelector {
SELECT Name, AccountId, Primary_Contact__c, Primary_Contact__r.AccountId, IsWon, IsClosed
FROM Opportunity
WHERE Primary_Contact__c = :contactId
WITH SECURITY_ENFORCED
];
}

Expand All @@ -107,6 +110,7 @@ public with sharing class CON_DeleteContactOverrideSelector {
SELECT Name, AccountId, IsWon, IsClosed
FROM Opportunity
WHERE AccountId = :accountId
WITH SECURITY_ENFORCED
];
}

Expand All @@ -115,6 +119,7 @@ public with sharing class CON_DeleteContactOverrideSelector {
SELECT Name, npe03__Contact__c
FROM npe03__Recurring_Donation__c
WHERE npe03__Contact__c = :contactId
WITH SECURITY_ENFORCED
];
}
}
10 changes: 10 additions & 0 deletions force-app/main/default/classes/CON_DeleteContactOverride_CTRL.cls
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,13 @@ public with sharing class CON_DeleteContactOverride_CTRL {
public PageReference deleteContact() {
if (isDeleteContactOnly()) {

if (!(UTIL_Permissions.canDelete('Contact', false) &&
UTIL_Permissions.canDelete('Opportunity', false) &&
UTIL_Permissions.canDelete('npe03__Recurring_Donation__c', false)))
{
throw new UTIL_Permissions.InsufficientPermissionException(System.Label.commonAccessErrorMessage);
}

ContactCascadeDelete cascadeDelete = new ContactCascadeDelete(contactRecord, contactOverrideSelector);
cascadeDelete.validate();
cascadeDelete.deleteOpportunities();
Expand Down Expand Up @@ -307,6 +314,9 @@ public with sharing class CON_DeleteContactOverride_CTRL {
Account account = contactOverrideSelector.getAccountRecord(accountId);

try {
if (!UTIL_Permissions.canDelete('Account', false)) {
throw new UTIL_Permissions.InsufficientPermissionException(System.Label.commonAccessErrorMessage);
}
AccountCascadeDelete cascadeDelete = new AccountCascadeDelete(account, contactOverrideSelector);
cascadeDelete.validate();

Expand Down
63 changes: 62 additions & 1 deletion force-app/main/default/classes/GE_GiftEntryController.cls
Original file line number Diff line number Diff line change
Expand Up @@ -424,17 +424,44 @@ public with sharing class GE_GiftEntryController {
@AuraEnabled
public static DataImport__c upsertDataImport(String dataImport) {
DataImport__c dataImportObject = (DataImport__c)JSON.deserialize(dataImport, DataImport__c.class);

try {
// As currently implemented, Gift Entry already checks everything checked in canUpsertDataImport() before
// allowing access. As a result, canUpsertDataImport() should never return false. It is implemented solely
// as a defense against future modifications since it is an AuraEnabled method that could be used outside
// of the currently implemented flow.
if (!canUpsertDataImport(dataImportObject)) {
throw new UTIL_Permissions.InsufficientPermissionException(System.Label.commonAccessErrorMessage);
}
upsert dataImportObject Id;

return dataImportObject;
} catch (UTIL_Permissions.InsufficientPermissionException e) {
throw new AuraHandledException(e.getMessage());
} catch (Exception e) {
String JSONExceptionData = ERR_ExceptionData.createExceptionWrapperJSONString(e);

throw buildDmlException(JSONExceptionData);
}
}

private static Boolean canUpsertDataImport(DataImport__c dataImportObject) {
if (!UTIL_Permissions.canCreate(UTIL_Namespace.StrAllNSPrefix('DataImport__c'))) {
return false;
}

for (String fieldName : dataImportObject.getPopulatedFieldsAsMap().keySet()) {
if (!UTIL_Permissions.canUpdate(UTIL_Namespace.StrAllNSPrefix('DataImport__c'),
UTIL_Namespace.StrAllNSPrefix(fieldName), false)) {
if (!fieldName.equalsIgnoreCase('Id')) {
return false;
}
}
}

return true;
}

/*******************************************************************************************************
* @description Run the DataImport process on a single gift
* @param dataImport DataImport record to be processed
Expand Down Expand Up @@ -1062,7 +1089,6 @@ public with sharing class GE_GiftEntryController {
* @return FormTemplateWrapper: Wrapper object of the list of deleted template names and the result
* of the DML action
*/
@AuraEnabled
public static String [] deleteFormTemplates(String[] ids) {
String[] formTemplateNames = new String[] {};
Form_Template__c[] templates = [
Expand Down Expand Up @@ -1162,6 +1188,17 @@ public with sharing class GE_GiftEntryController {
String description,
String formatVersion,
String templateJSON) {

Set<String> fieldsToCheck = new Set<String>{
'Name',
'Description__c',
'Template_JSON__c',
'Format_Version__c'
};
if (!canUpsertFormTemplate(fieldsToCheck)) {
throw new UTIL_Permissions.InsufficientPermissionException(System.Label.commonAccessErrorMessage);
}

if (templateJSON != null) {
Form_Template__c templateObj = new Form_Template__c(Id = id,
Name = name,
Expand All @@ -1175,6 +1212,22 @@ public with sharing class GE_GiftEntryController {
return null;
}


private static Boolean canUpsertFormTemplate(Set<String> fieldsToCheck) {
if (!UTIL_Permissions.canCreate(UTIL_Namespace.StrAllNSPrefix('Form_Template__c'))) {
return false;
}

for (String fieldName : fieldsToCheck) {
if (!UTIL_Permissions.canUpdate(UTIL_Namespace.StrAllNSPrefix('Form_Template__c'),
UTIL_Namespace.StrAllNSPrefix(fieldName), false)) {
return false;
}
}

return true;
}

/*******************************************************************************************************
* @description Method checks if the provided name is in use by another existing Form Template.
*
Expand Down Expand Up @@ -1345,6 +1398,14 @@ public with sharing class GE_GiftEntryController {
}

private static String retrieveBatchCurrencyIsoCode (Id batchId) {
// As currently implemented, Gift Entry already verifies edit access to DataImportBatch__c before allowing
// access. As a result, a permission error should never be encountered here. It is implemented solely as a
// defense against future modifications since it is called by an AuraEnabled method that could be used
// outside of the currently implemented flow.
if (!UTIL_Permissions.canRead(UTIL_Namespace.StrAllNSPrefix('DataImportBatch__c'), false)) {
throw new UTIL_Permissions.InsufficientPermissionException(System.Label.commonAccessErrorMessage);
}

String query = new UTIL_Query()
.withSelectFields(new Set<String>{UTIL_Currency.CURRENCY_ISO_CODE_FIELD})
.withFrom(DataImportBatch__c.SObjectType)
Expand Down
21 changes: 21 additions & 0 deletions force-app/main/default/classes/HH_CampaignDedupeBTN_CTRL.cls
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ public without sharing class HH_CampaignDedupeBTN_CTRL {
********************************************************************************************************/
public static integer MarkDuplicatesFromList(ID campaignId, list<CampaignMember> listCM) {

validateAccessPermissions();

final String DUPE_STATUS_SUFFIX = System.Label.hhCmpDedupeStatus;

//check for the Household Duplicate status values we need
Expand Down Expand Up @@ -291,6 +293,25 @@ public without sharing class HH_CampaignDedupeBTN_CTRL {
return returnsize;
}

private static void validateAccessPermissions() {
if (!UTIL_Permissions.canUpdate('CampaignMember', 'Status', false)) {
throw new UTIL_Permissions.InsufficientPermissionException(System.Label.commonAccessErrorMessage);
}

Set<String> cmsFields = new Set<String>{
'CampaignId',
'Label',
'HasResponded',
'SortOrder'
};
for (String cmsField : cmsFields) {
if (!(UTIL_Permissions.canRead('CampaignMemberStatus', cmsField, false) &&
UTIL_Permissions.canCreate('CampaignMemberStatus', cmsField, false))) {
throw new UTIL_Permissions.InsufficientPermissionException(System.Label.commonAccessErrorMessage);
}
}
}

/*******************************************************************************************************
* @description Adds a Message to the visualforce page
* @param arg the message string
Expand Down
Loading

0 comments on commit bd0b2e9

Please sign in to comment.