From 34a0e557ac6dfd2923050a9b6bf6f453872f38a4 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 1 Aug 2022 12:01:53 -0700 Subject: [PATCH] Create Unresolved CRUD/FLS violation when SFGE is unable to resolve an ApexValue --- .../com/salesforce/apex/jorje/JorjeUtil.java | 5 +- .../apex/operations/FlsValidationCentral.java | 24 ++++-- .../operations/FlsViolationCreatorUtil.java | 4 +- .../rules/fls/apex/UnresolvedCrudFlsTest.java | 81 +++++++++++++++++++ 4 files changed, 102 insertions(+), 12 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/apex/jorje/JorjeUtil.java b/sfge/src/main/java/com/salesforce/apex/jorje/JorjeUtil.java index 8e0bc46f1..d2704c808 100644 --- a/sfge/src/main/java/com/salesforce/apex/jorje/JorjeUtil.java +++ b/sfge/src/main/java/com/salesforce/apex/jorje/JorjeUtil.java @@ -135,10 +135,7 @@ public static class JorjeCompilationException extends SfgeRuntimeException { } } - /** - * Increments log level of BaseApexLexer class from jorje jar - * to avoid printing info logs. - */ + /** Increments log level of BaseApexLexer class from jorje jar to avoid printing info logs. */ private static void incrementLogLevel() { Logger jorjeLogger = Logger.getLogger(BaseApexLexer.class.getName()); jorjeLogger.setLevel(Level.WARNING); diff --git a/sfge/src/main/java/com/salesforce/rules/fls/apex/operations/FlsValidationCentral.java b/sfge/src/main/java/com/salesforce/rules/fls/apex/operations/FlsValidationCentral.java index af1b01ca7..f479de1cf 100644 --- a/sfge/src/main/java/com/salesforce/rules/fls/apex/operations/FlsValidationCentral.java +++ b/sfge/src/main/java/com/salesforce/rules/fls/apex/operations/FlsValidationCentral.java @@ -4,7 +4,6 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import com.salesforce.config.SfgeConfigProvider; -import com.salesforce.exception.TodoException; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.symbols.ScopeUtil; import com.salesforce.graph.symbols.SymbolProvider; @@ -156,18 +155,31 @@ public void createExpectedValidations(DmlStatementVertex vertex, SymbolProvider final BaseSFVertex childVertex = dmlStatementVertexChildren.get(0); final ValidationConverter validationConverter = new ValidationConverter(validationType); - final Set validationReps; + final Set validationReps = new HashSet<>(); if (childVertex instanceof ChainedVertex) { final Optional> apexValue = ScopeUtil.resolveToApexValue(symbols, (ChainedVertex) childVertex); if (apexValue.isPresent()) { - validationReps = validationConverter.convertToExpectedValidations(apexValue.get()); + validationReps.addAll( + validationConverter.convertToExpectedValidations(apexValue.get())); } else { - throw new TodoException( - "Apex value not detected for dml's child vertex: " + childVertex); + if (LOGGER.isWarnEnabled()) { + LOGGER.warn( + "TODO: Apex value not detected for dml's child vertex: " + childVertex); + // TODO: add telemetry + } + violations.add( + FlsViolationCreatorUtil.createUnresolvedCrudFlsViolation( + validationType, vertex)); } } else { - throw new TodoException("Child vertex of DML is not a chained vertex: " + childVertex); + if (LOGGER.isWarnEnabled()) { + LOGGER.warn("TODO: Child vertex of DML is not a chained vertex: " + childVertex); + // TODO: add telemetry + } + violations.add( + FlsViolationCreatorUtil.createUnresolvedCrudFlsViolation( + validationType, vertex)); } expectedValidations.addAll(validationReps); diff --git a/sfge/src/main/java/com/salesforce/rules/fls/apex/operations/FlsViolationCreatorUtil.java b/sfge/src/main/java/com/salesforce/rules/fls/apex/operations/FlsViolationCreatorUtil.java index 7276b5d87..a5b2d04e3 100644 --- a/sfge/src/main/java/com/salesforce/rules/fls/apex/operations/FlsViolationCreatorUtil.java +++ b/sfge/src/main/java/com/salesforce/rules/fls/apex/operations/FlsViolationCreatorUtil.java @@ -5,7 +5,7 @@ import com.salesforce.graph.ops.ApexValueUtil; import com.salesforce.graph.symbols.apex.ApexValue; import com.salesforce.graph.vertex.ChainedVertex; -import com.salesforce.graph.vertex.MethodCallExpressionVertex; +import com.salesforce.graph.vertex.SFVertex; import java.util.Collection; import java.util.HashSet; import java.util.Set; @@ -85,7 +85,7 @@ static FlsViolationInfo getFlsViolationInfo( } static FlsViolationInfo createUnresolvedCrudFlsViolation( - FlsConstants.FlsValidationType validationType, MethodCallExpressionVertex sinkVertex) { + FlsConstants.FlsValidationType validationType, SFVertex sinkVertex) { final FlsViolationInfo violationInfo = new UnresolvedCrudFlsViolationInfo(validationType); violationInfo.setSinkVertex(sinkVertex); diff --git a/sfge/src/test/java/com/salesforce/rules/fls/apex/UnresolvedCrudFlsTest.java b/sfge/src/test/java/com/salesforce/rules/fls/apex/UnresolvedCrudFlsTest.java index 8e794238d..29038d326 100644 --- a/sfge/src/test/java/com/salesforce/rules/fls/apex/UnresolvedCrudFlsTest.java +++ b/sfge/src/test/java/com/salesforce/rules/fls/apex/UnresolvedCrudFlsTest.java @@ -138,4 +138,85 @@ public void testReadWithUnresolvedBinaryExpression() { sourceCode, expectUnresolvedCrudFls(3, FlsConstants.FlsValidationType.READ)); } + + @Test + public void testDmlOnSecondLevelObject() { + String sourceCode = + "public class MyClass {\n" + + " public void foo(My_Obj__c myObj) {\n" + + " myObj.Custom_Field__c.Name = 'Acme Inc.';\n" + + " update myObj.Custom_Field__c;\n" + + " }\n" + + "}\n"; + + assertViolations( + ApexFlsViolationRule.getInstance(), + sourceCode, + expect(4, FlsConstants.FlsValidationType.UPDATE, "Custom_Field__c")); + } + + @Test + public void testDmlOnReferenceObject() { + String sourceCode = + "public class MyClass {\n" + + " public void foo(My_Obj__c myObj) {\n" + + " myObj.Custom_Field__c.Reference__r.Name = 'Acme Inc.';\n" + + " update myObj.Custom_Field__c.Reference__r;\n" + + " }\n" + + "}\n"; + + assertViolations( + ApexFlsViolationRule.getInstance(), + sourceCode, + expectUnresolvedCrudFls(4, FlsConstants.FlsValidationType.UPDATE)); + } + + @Test + public void testDmlOnMethodReturnValue() { + String sourceCode = + "public class MyClass {\n" + + " public void foo() {\n" + + " /* sfge-disable-next-line ApexFlsViolationRule */\n" + + " List accounts = [SELECT Id, Name FROM Account WHERE Type='something'];\n" + + " Map> listByType = new Map>();\n" + + " listByType.put('Account', accounts);\n" + + " delete listByType.get('Account');\n" + + " }\n" + + "}\n"; + + assertViolations( + ApexFlsViolationRule.getInstance(), + sourceCode, + expect(7, FlsConstants.FlsValidationType.DELETE, "Account")); + } + + @Test + public void testDmlOnMethodReturnValueIndeterminant() { + String sourceCode = + "public class MyClass {\n" + + " public void foo(Map> listByType) {\n" + + " delete listByType.get('Account');\n" + + " }\n" + + "}\n"; + + assertViolations( + ApexFlsViolationRule.getInstance(), + sourceCode, + expect(3, FlsConstants.FlsValidationType.DELETE, "SObject")); + } + + @Test + public void testDmlOnMethodInvokedbyMethod() { + String sourceCode = + "public class MyClass {\n" + + " public void foo(Map> listByType, Schema.SObjectType sObjectType) {\n" + + " delete listByType.get(sObjectType.getDescribe().getName());\n" + + " }\n" + + "}\n"; + + assertViolations( + ApexFlsViolationRule.getInstance(), + sourceCode, + expect(3, FlsConstants.FlsValidationType.DELETE, "SObject")); + } }