[Statistics / Behavioural Statistics] Fix permission issue where users were allowed to see statistics from sites they don't have access to. #5252
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
September 27, 2019 15:25
…s were allowed to see statistics from sites they don't have access to.
PapillonMcGill
previously approved these changes
Sep 30, 2019
|
@racostas Thanks for submitting this patch. |
johnsaigle
reviewed
Sep 30, 2019
johnsaigle
reviewed
Sep 30, 2019
johnsaigle
reviewed
Sep 30, 2019
Co-Authored-By: PapillonMcGill <34311645+PapillonMcGill@users.noreply.github.com>
Co-Authored-By: John Saigle <4022790+johnsaigle@users.noreply.github.com>
Co-Authored-By: John Saigle <4022790+johnsaigle@users.noreply.github.com>
Co-Authored-By: John Saigle <4022790+johnsaigle@users.noreply.github.com>
|
Thanks @PapillonMcGill, @johnsaigle and @zaliqarosli !! |
|
@racostas This isn't passing Travis due to some coding style issues. If you run |
johnsaigle
added
Category: Bug
zaliqarosli
requested changes
Sep 30, 2019
Co-Authored-By: Zaliqa <zaliqa.rosli@mcin.ca>
Co-Authored-By: Zaliqa <zaliqa.rosli@mcin.ca>
| $DB =& \Database::singleton(); | ||
| $factory = \NDB_Factory::singleton(); | ||
| $DB = $factory->database(); | ||
| $user = $factory->user(); |
There was a problem hiding this comment.
Suggested change
| $user = $factory->user(); | |
| $user = $factory->user(); |
To pass coding style
|
|
||
| if (!$user->hasPermission('access_all_profiles')) { | ||
| $sitesString = implode(",", $user->getCenterIDs()); | ||
| $query .= " AND CenterID IN (" . $sitesString . ")"; |
There was a problem hiding this comment.
Suggested change
| $query .= " AND CenterID IN (" . $sitesString . ")"; | |
| $query .= " AND CenterID IN (" . $sitesString . ")"; |
here too
PapillonMcGill
previously approved these changes
Oct 1, 2019
zaliqarosli
previously approved these changes
Oct 1, 2019
|
I have no strong feelings on what branch it should go to. |
|
I think it should be properly tested and go to bugfix |
…e statistics calculation
racostas
dismissed stale reviews from zaliqarosli and PapillonMcGill
via
7d7d640
PapillonMcGill
approved these changes
Oct 28, 2019
ridz1208
added
the
State: Needs rebase
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the issue
A user with role Data Entry was able to see statistics from sites it does not belong/have access
For example if there are two users and two sites:
user Site 1 is associate only to Site_1.
user Site 2 is associate only to Site_2.
(Both of them have the role Data Entry and nothing more in the permissions)
When going to MainMenu->Reports->Statistics->Behavioural Statistics
The user of Site 1 is able to see the statistics for Site 2 and vice versa
Testing instructions
Now each user should be able to see only the statistics for the sites it belong/have access
Note:
This is a temp fix based on 21.0-released. A major refractory still pending.