Provide dependency injection for SecurityUtils for testing and alternative security implementations other than fflib_SecurityUtils

[jondavis9898]

trafficstars

This enhancement resulted from discussion in #56.

Currently the SecurityUtils class is used for various FLS & CRUD checks. The static methods in this class are used in various places in the library (e.g. fflib_QueryFactory) where these checks need to occur. In the case where a custom security solution is required, performing these checks is still necessary but there is no way to override how the check is performed. Custom security can be implemented in other ways but this potentially leads to redundant checks and also a confusing design because checks are done in different places.

Suggestion would be something along the lines of the following:

1. Create a new interface fflib_ISecurity. To begin with, the methods on this interface can mirror the static methods on fflib_SecurityUtils
2. Inject an instance of fflib_ISecurity in to the places where it is needed (e.g. fflib_QueryFactory) and have the QF use the injected instance rather than the concrete static methods.
3. SObjectDomain could also receive an instance of fflib_ISecurity and its permission checking could be updated to defer to the class instead of directly checking describe.

Making a change as described above would yield the following benefits:

1. All "security" logic would be in one place and anywhere in the library where security checks were required could defer to the security class injected.
2. Allows for custom security implementations while still providing base level security functionality
3. Improve testability

[afawcett]

I think architecture wise it just makes sense to have this interface in place as you say. Do you have any examples of what customisations to the current way security is implemented you want to do?

[jondavis9898]

Our security model is rather complicated but I'll do my best to summarize the use case which should draw out examples.

Use Case: We support multiple "companies" within a single SFDC org meaning that under the parent company, there are multiple "companies" that the data must be partitioned against. Security is applied at the "Object" and "Company" level. An individual can play a role in multiple companies and their access will be different in each company. For example, "Joe" might be a "Sales Person" in Company "A" and a "Sales Person" and "Logistics" in Company "B". When Joe is viewing data in "A" he can see/edit "Accounts" but not "Shipments". When in Company "B", he can see both. The number of combinations of this is endless so we can't rely on Roles & Profiles since SFDC only permits 1 role and 1 profile per user. Also, record types don't work because of several reasons. Given that, our solution is:

• Every object in our system has a field called "Company" which is a lookup to a custom object called "Company"
• Record level access is determined by the individual and their permissions to a given "Company" and that "Object"
• Sharing Rules are applied against the custom field "Company" in each object
• Custom "FLS" and "CRUD" is enforced through a security module we've built
• Sharing rules don't provide a solution for FLS
• Since we have 100+ fields in some objects, splitting them out to separate objects to rely on object level security would result in object explosion

Given the above, all FLS & CRUD is performed via code that is instrumented in SObjectDomain classes. The check itself looks to our "metadata" to determine if the user should or should not be permitted to CRUD on that object and/or change a particular field.

In short, I think the method signatures that exist today would meet our needs, we would just need to make a custom version that implements the new ISecurity interface.

Hope that helps. It's rather ugly and extremely complex but after months of trying to find a better way, this appears the only approach that will work on SFDC. The only alternative is to have a seperate organziation for each company but that would result in 60 SFDC licenses becoming 60*<number of companies> licenses which would be cost prohibitive.

[afawcett]

I'm still a little lost if i am honest, your use case above is a mix of Sharing, CRUD and FLS security requirements together. On the platform Sharing is record accessibility and does not influence CRUD or FLS security and vice versa, the two are defined under entirely separate parts of the Setup menu, and are driven by different scopes, one is record level other is user level.

Security best practices on the platform state that we need to check against the platforms metadata (aka Profiles / Permissions Sets) for CRUD and FLS support (this is really important for managed packages as you'll fail security review if not) and leverage 'with sharing' for Sharing rules. While I think i get what your wanting to do here, which is conditionally enforcing CRUD and FLS at Record level, i have to advise it feels at odds with the platform, that is "if" i've understood correctly....

That said, i'm not wanting to stand in the way of creating an dependency injection between SecurityUtil's and its consumers in this framework (indeed it would help with unit tests as well), i just really want to be able to support the first use case for doing it and i'm struggling....

[afawcett]

Since thinking about it further in order to implement won't you need to grant full access to all objects and fields at a platform level (via profile and permission sets), in order to then successfully apply your own security model to effectively re-constrain what the user is allowed to do?

[afawcett]

Thinking about this some more, I'm happy if the use case becomes "we want provide dependency injection for SecurityUtils for testing and alternative security implementations other than fflib_SecurityUtils", and leave it to the developer just how thats done! :+1: I'm probably getting to far into what your needs are here i confess.

So lets have a go at DI for this area then... can we plan out the changes here perhaps, where would be the injection point (e.g. where do you provide the alternative impl) and what would be on the interface, also what changes to the existing SecurityUtils class would be required? etc...

[jondavis9898]

Thanks Andrew, I think your understanding of our use case is accurate. It's definitely at odds with standard platform practice but we ultimately have no choice without having separate orgs for each "company." We start with a private security model with the user gaining object and record level access via standard security constructs. However, once the user has that access, they need to be further constrained by data within the record and there is no built in way to do this currently since the only record level conditional data security option is Sharing rules but sharing rules don't support FLS. I'd be more than happy and would appreciate the opportunity if you were interested to have a separate discussion around this. While I've spent a lot of time trying to design for this solution with folks that have worked with the platform for 10+ years, I'm very new to SFDC so possibly we are overlooking a way to do what we need using stock concepts.

With that said, I like where you landed shifting this issue to focus on using DI and improving testability rather than on custom CRUD/FLS. In the end, it will yield the same net result (allowing for custom implementations) but stays true to general improvements to the library rather than odd and unique use cases such as the one I have.

I've updated the issue name to reflect this. I'm going to spend some time putting together an outline of what I think this might look like and will post here as soon as I can. Thanks again!

[afawcett]

Awesome, your very welcome, look forward to see your further thoughts. :+1:

[jondavis9898]

Sorry for the delay on getting back to this, it's been a very busy last few months. Here are my thoughts on how to provide for this feature.

Regarding the interface (e.g. fflib_ISecurityUtils), the methods would be the same as those that are currently static methods on fflib_SecurityUtils (e.g. checkInsert, checkRead, checkUpdate, etc.).

Providing the alternate implementation is where things get a little complicated because the library currently does not have "Container" (e.g. Factory) to use during runtime within the library itself. I've come up with 3 different ways we can provide for DI:

1. Add a "Container" to fflib_apex_common core - Within fflib_Application, create a singleton servicefactory property from which an instance of fflib_ISecurityUtils could be retrieved. The factory would have a method to "update/change" a service registration so that consumers of the library would change the registration of the fflib_ISecurityUtils class. By default, it would be fflib_SecurityUtilsImpl. The existing fflib_SecurityUtils would be modified to use the service pattern and obtain an instance of fflib_ISecurityUtils from the factory. When consumers did not require an alternate security implementation, this approach would be completely transparent to them. When an alternate security approach was required, consumers would need to call a method on the servicefactory (e.g. register(Type, Type)), to register their own implementation of fflib_ISecurityUtils (this would replace the default registration for fflib_ISecurityUtils). The downside to this approach is that since there is no "entry point" in SFDC (e.g. something that gets called by the platform itself at the start of every execution context), consumers would have to call this method anytime they would be using a core framework class that internally uses fflib_ISecurityUtils (e.g. fflib_SObjectSelector which uses fflib_QueryFactory which uses fflib_ISecurityUtils).

2. Property Injection - Within fflib_SecurityUtils, add a static property that defaults to a new instance of fflib_SecurityUtilsImpl. Consumers could change to an alternate implementation by calling a static method (e.g. setSecurityUtils) providing it an instance of the alternate implementation. fflib_SecurityUtils would then use this static when calling interface methods. The downside to this approach is that since there is no "entry point" in SFDC (e.g. something that gets called by the platform itself at the start of every execution context), consumers would have to call this method anytime they would be using a core framework class that internally uses fflib_ISecurityUtils (e.g. fflib_SObjectSelector which uses fflib_QueryFactory which uses fflib_ISecurityUtils).

3. Constructor Injection - For any class that requires fflib_ISecurityUtils, provide a constructor that accepts an instance of fflib_ISecurityUtils. Currently, only fflib_QueryFactory uses fflib_SecurityUtils. New constructor overloads would be added and then newQueryFactory() and configureQueryFactory() methods of fflib_SObjectSelector would be modified similarly. Using Constructor injection, fflib_SecurityUtils itself could be deprecated. This approach has the upside of not requiring any "extra" method calls like the two approaches above would. Consumers would simply call the overload providing an instance of the alternate implementation. There is a parameter enforceFLS in this code path currently so this new parameter would align with that parameter allowing consumers to enforceFLS using the default implementation or provide an alternate implementation. The upside here is that no extra methods are required to be called like they would in the two approaches above. The downside is that, while transparent, this is the most invasive change (although it would not provide much risk).

My overall thoughts are that it would be a benefit in this case and potentially further cases to have some type of "Container" for the library itself. Unfortunately, SFDC doesn't make doing this very easy. For that reason, my recommendation is Constructor Injection (approach number 3) as it's transparent to existing code while providing method overloads to allow for alternate implementations. fflib_SecurityUtils could still exist for backwards compat (if any consumer is using this class directly) but could be deprecated at some point. One other thing I suggest be modified is changing fflib_SObjectDomain to use fflib_ISecurityUtils where the "EnforcingTriggerCRUDSecurity" is currently made instead of directly checking SObjectDescribe. This would move all security checking to the same place.

Would love to hear any other approaches that others can think of.

Look forward to your thoughts. Thanks!