search

salesagility / SuiteCRM

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
4.44k stars 2.07k forks source link

Wrong SQL query generated in Module Report (7.11.x) with limited permission user #7380

Open kangmin88 opened 5 years ago

kangmin88 commented 5 years ago

Limited permission user can not work with module report in version 7.11.x. No data display.

Issue

All report have related field can not excute. When user run Report, log appear wrong query and can not excute. Error only appear if report setup related field from other module. Ex: With Report Tasks, I add fields from module Opportunites (Opp name, Opp Type...). In SQL query generated, table name convert to alias, but next line not use alias, still use extract table name => make query error.

LEFT JOIN opportunities tasks:opportunities ON tasks.parent_id=tasks:opportunities.id AND tasks:opportunities.deleted=0 AND tasks.parent_type = 'Opportunities' AND ( opportunities.assigned_user_id ='b1c16b21-c717-7d66-7795-5c948a1488b4' or EXISTS (SELECT 1 => MySQL error 1054: Unknown column 'opportunities.assigned_user_id' in 'on clause'

opportunities.assigned_user_id need change to tasks:opportunities.assigned_user_id

But if change permission List (Opportunites) from Ower/Group to All. Report can work well.

Expected Behavior

Actual Behavior

Possible Fix

Fix query generated

Steps to Reproduce

  1. Create Report Task.
  2. Choose main module is Tasks. Add field Task Name
  3. Choose related module Opportunites. Add field Opp Name
  4. Setup Role A: Opportunities have permission List is Ower/Group.
  5. User in role A run report => Error

Context

All user in system have role with limited permission (only can see record ower/group). And now no one can use report.

Your Environment

kangmin88 commented 5 years ago

With other reports, Same error appear. Ex: Report Opptunites, if add fields from Accounts => wrong query with alias Accounts. And if i change Accounts permission List from Ower/Group to All, report can work well.

kangmin88 commented 5 years ago

Any update for this isue?. i try deploy new system and still have error with module report. Problem is permission List ower/group. When create a role, if module X set permission List ower/group, module X will not be related in Report. Wrong query generated when end user run report

Mac-Rae commented 5 years ago

@eggsurplus Any thoughts on this issue being related to Security Suite and not reports directly?

eggsurplus commented 5 years ago

@Mac-Rae Looks possible. I'll take a deeper look.

kangmin88 commented 5 years ago

Sorry guys. Can you give me snippet code to fix issue (or some file code I can replace and rebuild). I upgrade 7.9 to 7.11 because sercurity update, i can not roll back. All team leader, manager in the system could not do anything with report for more 2 week... I can not config permission to All, data is private with group and not share.

eggsurplus commented 5 years ago

I've been looking at this this evening, but have nothing immediate for you. I'll report back when there is information or a solution to share.

eggsurplus commented 5 years ago

@Mac-Rae The issue comes down to AOR_Report::build_report_access_query() in the first if block. The following gets called:

$module->getOwnerWhere

The problem is that the alias doesn't get passed to getOwnerWhere and there isn't a way to currently do so. Shortly after that chunk you will see another getOwnerWhere called (same issue) and then getGroupWhere where the alias is passed for the table name.

Because the alias isn't passed the query that gets built does so with the wrong table alias causing the original query error in this issue.

My proposed solution is to alter SugarBean::getOwnerWhere to pass the alias:

public function getOwnerWhere($user_id, $table_alias='')

Then if it is passed use that instead of $this->table_name.

Could someone handle the PR for that?

kangmin88 commented 5 years ago

Thanks @eggsurplus I created new function getOwnerWhere_fixAlias for module report (to pass alias), and seem it work well. This is only a temporary measure. Hope it will be fixed in next version. I think this is a critical error

pgorod commented 5 years ago

@kangmin88 do you want to try creating a Pull Request with this fix?