Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fflib_QueryFactory database operation mode support #434

Conversation

chazwatkins
Copy link

@chazwatkins chazwatkins commented Dec 2, 2022

Goal

Add support for USER_MODE and SYSTEM_MODE database operation modes to fflib_QueryFactory and fflib_SObjectSelector that provides backward compatibility with the existing enforceFLS and enforceCRUD checks.

fflib_QueryFactory

When the operation mode is set, the enforceFLS is set to false to skip the fflib_SecurityUtils checks. Alternatively, when you call setEnforceFLS(true), the operation mode is set to null, and fflib_SecurityUtils checks are respected.

Operation Mode not set

fflib_QueryFactory qf = new fflib_QueryFactory(Account.SObjectType);
// enforceFLS -> false
qf.getOperationMode(); // null

// SELECT Id FROM Account

Specify WITH USER_MODE

fflib_QueryFactory qf = new fflib_QueryFactory(Account.SObjectType);

qf.withUserMode();
qf.getOperationMode(); // USER_MODE
// enforceFLS -> false
qf.getOperationMode(); // null

// SELECT Id FROM Account WITH USER_MODE

qf.setOperationMode(fflib_SecurityUtils.OperationMode.USER_MODE);
qf.getOperationMode(); // USER_MODE
// enforceFLS -> false

// SELECT Id FROM Account WITH USER_MODE

Specify WITH SYSTEM_MODE

Salesforce uses SYSTEM_MODE as the default operation mode when not specified in the query. withSystemMode() allows users to make their queries explicit if desired.

fflib_QueryFactory qf = new fflib_QueryFactory(Account.SObjectType);

qf.withSystemMode();
// enforceFLS -> false

qf.getOperationMode(); // SYSTEM_MODE
// SELECT Id FROM Account WITH SYSTEM_MODE

qf.setOperationMode(fflib_SecurityUtils.OperationMode.SYSTEM_MODE);
// SELECT Id FROM Account WITH SYSTEM_MODE

Works with other clauses

The `WITH {OPERATION_MODE}' clause is inserted after the WHERE clause and before any other clauses

SELECT Id FROM Account {WHERE clause} WITH {OPERATION_MODE} {...Other clauses}
fflib_QueryFactory qf = new fflib_QueryFactory(Account.SObjectType);

qf.setCondition(Account.Name + ' IN :names');
qf.setOrdering(Account.Name, fflib_QueryFactory.SortOrder.ASCENDING);
qf.setLimit(2);
// SELECT Id FROM Account WHERE Name IN :names ORDER BY Name ASC NULLS FIRST  LIMIT 2

qf.withUserMode();
// SELECT Id FROM Account WHERE Name IN :names WITH USER_MODE ORDER BY Name ASC NULLS FIRST  LIMIT 2

Only applies to top-level queries

fflib_QueryFactory qf = new fflib_QueryFactory(Account.SObjectType);

qf.subselectQuery('Contacts');
// SELECT Id, (SELECT Id FROM Contact) FROM Account

qf.withUserMode();
// SELECT Id, (SELECT Id FROM Contacts) FROM Account WITH USER_MODE

qf.withSystemMode();
// SELECT Id, (SELECT Id FROM Contacts) FROM Account WITH SYSTEM_MODE

fflib_SObjectSelector

isEnforcingFLS() and isEnforcingCRUD() now consider the operation mode to determine their result instead of directly returning the values of m_enforceFLS and m_enforceCRUD. This allows the user to get the correct state from the methods, regardless if they are using legacy enforcement or one of the new operation modes.

When the operation mode is set, the m_enforceFLS and m_enforceCRUD are false.

Operation Mode not set

isEnforcingFLS() and isEnforcingCRUD() returns the value of the private properties. fflib_SecurityUtils checks are respected.

AccountSelector selector = new AccountSelector();

// m_enforceFLS -> false
// m_enforceCRUD -> true
// selector.isEnforcingCRUD() -> true
// selector.isEnforcingFLS() -> false

selector.enforceFLS();
// m_enforceFLS -> true
// selector.isEnforcingFSL() -> true

selector.newQueryFactory();
// query factory enforceFLS -> true;
// SELECT Id FROM Account

Specify USER_MODE

isEnforcingFLS() and isEnforcingCRUD() return true, but the properties are false. fflib_SecurityUtils checks are skipped.

AccountSelector selector = new AccountSelector();

selector.withUserMode();
// m_enforceFLS -> false
// m_enforceCRUD -> false

// selector.isEnforcingCRUD() -> true
// selector.isEnforcingFLS() -> true

selector.newQueryFactory();
// query factory enforceFLS -> false;
// SELECT Id FROM Account WITH USER_MODE

Specify SYSTEM_MODE

isEnforcingFLS() and isEnforcingCRUD() return false, and the properties are set to false. fflib_SecurityUtils checks are skipped.

AccountSelector selector = new AccountSelector();

selector.withSystemMode();
// m_enforceFLS -> false
// m_enforceCRUD -> false

// selector.isEnforcingCRUD() -> false
// selector.isEnforcingFLS() -> false

selector.newQueryFactory();
// query factory enforceFLS -> false;
// SELECT Id FROM Account WITH SYSTEM_MODE

Add

Enum fflib_SecurityUtils.OperationMode

  • SYSTEM_MODE
  • USER_MODE

fflib_QueryFactory

  • void setOperationType(fflib_SecurityUtils.OperationMode mode)
  • void withUserMode()
  • void withSystemMode()
  • String getOperationMode()
  • Boolean isEnforcingFLS();

fflib_SObjectSelector

  • void setOperationType(fflib_SecurityUtils.OperationMode mode)
  • void withUserMode()
  • void withSystemMode()
  • String getOperationMode()

Change

fflib_QueryFactoryTest

  • Replace System asserts with new Assert class

fflib_SObjectSelector

  • Update m_enforceFLS to use getter/setter so when its value is changed to true, then the operationMode property will be set to null

fflib_SObjectSelectorTest

  • Replace System asserts with new Assert class

This change is Reviewable

@daveespo
Copy link
Contributor

daveespo commented Dec 2, 2022

Hi @chazwatkins -- thank you for the contribution. However, there is already a branch that has undergone some peer review and is nearly ready for merge to support User Mode. It also addresses a longstanding performance issue with QueryFactory so while a lot of your work overlaps the work on #419 (PR #420), your efforts don't go as far as the other proposal.

I'm going to close this out and would request that you participate in the review of PR #420

@daveespo daveespo closed this Dec 2, 2022
@chazwatkins
Copy link
Author

@daveespo The other PR has been open for nearly five months. When will it be merged?

I'm fine with contributing to the other PR instead. I'll port my API to your version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants