{"action":"created","comment":{"url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/comments/212840305","pull_request_review_id":149551870,"id":212840305,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIxMjg0MDMwNQ==","diff_hunk":"@@ -228,18 +223,55 @@ public function canAccess()\n             return true;\n         }\n \n-        // Check if we have access to current section on the current subsite.\n-        $accessibleSites = $this->owner->sectionSites(true, 'Main site', $member);\n-        return $accessibleSites->count() && $accessibleSites->find('ID', SubsiteState::singleton()->getSubsiteId());\n+        // Check we have a member\n+        if (!$member) {\n+            return null;\n+        }\n+\n+        // Check that some subsites exist first\n+        if (!Subsite::get()->exists()) {\n+            return null;\n+        }\n+\n+        // Get the current subsite ID\n+        $currentSubsiteId = SubsiteState::singleton()->getSubsiteId();\n+\n+        // Check against the current user's associated groups\n+        $allowedInSubsite = false;\n+        foreach ($member->Groups() as $group) {\n+            // If any of the current user's groups have been given explicit access to the subsite, delegate to core\n+            if ($group->AccessAllSubsites) {\n+                $allowedInSubsite = true;\n+                break;\n+            }\n+\n+            // Check if any of the current user's groups have been given explicit access to the current subsite\n+            $groupSubsiteIds = $group->Subsites()->column('ID');\n+            if (in_array($currentSubsiteId, $groupSubsiteIds)) {\n+                $allowedInSubsite = true;","path":"src/Extensions/LeftAndMainSubsites.php","position":160,"original_position":160,"commit_id":"6af985420f896063694fc4f5aacfcc3c08c8a3a9","original_commit_id":"6af985420f896063694fc4f5aacfcc3c08c8a3a9","user":{"login":"robbieaverill","id":5170590,"node_id":"MDQ6VXNlcjUxNzA1OTA=","avatar_url":"https://avatars1.githubusercontent.com/u/5170590?v=4","gravatar_id":"","url":"https://api.github.com/users/robbieaverill","html_url":"https://github.com/robbieaverill","followers_url":"https://api.github.com/users/robbieaverill/followers","following_url":"https://api.github.com/users/robbieaverill/following{/other_user}","gists_url":"https://api.github.com/users/robbieaverill/gists{/gist_id}","starred_url":"https://api.github.com/users/robbieaverill/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robbieaverill/subscriptions","organizations_url":"https://api.github.com/users/robbieaverill/orgs","repos_url":"https://api.github.com/users/robbieaverill/repos","events_url":"https://api.github.com/users/robbieaverill/events{/privacy}","received_events_url":"https://api.github.com/users/robbieaverill/received_events","type":"User","site_admin":false},"body":"Yeah I'd considered that. I think the reason I left it like this is because the two if statements contribute to the same criteria of \"is allowed in subsite\" so I left them separate for readability. That OK?","created_at":"2018-08-26T22:03:34Z","updated_at":"2018-08-26T22:03:35Z","html_url":"https://github.com/silverstripe/silverstripe-subsites/pull/388#discussion_r212840305","pull_request_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/388","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/comments/212840305"},"html":{"href":"https://github.com/silverstripe/silverstripe-subsites/pull/388#discussion_r212840305"},"pull_request":{"href":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/388"}},"in_reply_to_id":212523161},"pull_request":{"url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/388","id":210639515,"node_id":"MDExOlB1bGxSZXF1ZXN0MjEwNjM5NTE1","html_url":"https://github.com/silverstripe/silverstripe-subsites/pull/388","diff_url":"https://github.com/silverstripe/silverstripe-subsites/pull/388.diff","patch_url":"https://github.com/silverstripe/silverstripe-subsites/pull/388.patch","issue_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/issues/388","number":388,"state":"open","locked":false,"title":"FIX CMS permission checks for subsite are now handled in the state context","user":{"login":"robbieaverill","id":5170590,"node_id":"MDQ6VXNlcjUxNzA1OTA=","avatar_url":"https://avatars1.githubusercontent.com/u/5170590?v=4","gravatar_id":"","url":"https://api.github.com/users/robbieaverill","html_url":"https://github.com/robbieaverill","followers_url":"https://api.github.com/users/robbieaverill/followers","following_url":"https://api.github.com/users/robbieaverill/following{/other_user}","gists_url":"https://api.github.com/users/robbieaverill/gists{/gist_id}","starred_url":"https://api.github.com/users/robbieaverill/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/robbieaverill/subscriptions","organizations_url":"https://api.github.com/users/robbieaverill/orgs","repos_url":"https://api.github.com/users/robbieaverill/repos","events_url":"https://api.github.com/users/robbieaverill/events{/privacy}","received_events_url":"https://api.github.com/users/robbieaverill/received_events","type":"User","site_admin":false},"body":"We now check the subsite state for the context and validate it against the current member's group permissions using the SilverStripe ORM relationships instead of using SQL queries.\n\nMore granular permission checks e.g. canView etc are still up to data models to define and handle.\n\nI think this is semver patch safe. I've used a workaround in `LeftAndMainSubsites::canAccess` to allow a passed member argument without adding it to the method signature (not semver safe).\n\nAlso the removal of `SubsiteXHRControllerTest` may be a little semver unsafe, but it falls back to calling the method from `LeftAndMainSubsites` so I'm OK with it.\n\nFixes #358","created_at":"2018-08-24T05:02:14Z","updated_at":"2018-08-26T22:03:35Z","closed_at":null,"merged_at":null,"merge_commit_sha":"bb21f08e5acc5bba75f7fedfa059e026a1073498","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":587718479,"node_id":"MDU6TGFiZWw1ODc3MTg0Nzk=","url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/labels/affects/v4","name":"affects/v4","color":"5319e7","default":false},{"id":587718524,"node_id":"MDU6TGFiZWw1ODc3MTg1MjQ=","url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/labels/change/patch","name":"change/patch","color":"fef2c0","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/388/commits","review_comments_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/388/comments","review_comment_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/comments{/number}","comments_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/issues/388/comments","statuses_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/statuses/6af985420f896063694fc4f5aacfcc3c08c8a3a9","head":{"label":"creative-commoners:pulls/2.1/fix-role-permissions","ref":"pulls/2.1/fix-role-permissions","sha":"6af985420f896063694fc4f5aacfcc3c08c8a3a9","user":{"login":"creative-commoners","id":28884460,"node_id":"MDEyOk9yZ2FuaXphdGlvbjI4ODg0NDYw","avatar_url":"https://avatars2.githubusercontent.com/u/28884460?v=4","gravatar_id":"","url":"https://api.github.com/users/creative-commoners","html_url":"https://github.com/creative-commoners","followers_url":"https://api.github.com/users/creative-commoners/followers","following_url":"https://api.github.com/users/creative-commoners/following{/other_user}","gists_url":"https://api.github.com/users/creative-commoners/gists{/gist_id}","starred_url":"https://api.github.com/users/creative-commoners/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/creative-commoners/subscriptions","organizations_url":"https://api.github.com/users/creative-commoners/orgs","repos_url":"https://api.github.com/users/creative-commoners/repos","events_url":"https://api.github.com/users/creative-commoners/events{/privacy}","received_events_url":"https://api.github.com/users/creative-commoners/received_events","type":"Organization","site_admin":false},"repo":{"id":94381336,"node_id":"MDEwOlJlcG9zaXRvcnk5NDM4MTMzNg==","name":"silverstripe-subsites","full_name":"creative-commoners/silverstripe-subsites","owner":{"login":"creative-commoners","id":28884460,"node_id":"MDEyOk9yZ2FuaXphdGlvbjI4ODg0NDYw","avatar_url":"https://avatars2.githubusercontent.com/u/28884460?v=4","gravatar_id":"","url":"https://api.github.com/users/creative-commoners","html_url":"https://github.com/creative-commoners","followers_url":"https://api.github.com/users/creative-commoners/followers","following_url":"https://api.github.com/users/creative-commoners/following{/other_user}","gists_url":"https://api.github.com/users/creative-commoners/gists{/gist_id}","starred_url":"https://api.github.com/users/creative-commoners/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/creative-commoners/subscriptions","organizations_url":"https://api.github.com/users/creative-commoners/orgs","repos_url":"https://api.github.com/users/creative-commoners/repos","events_url":"https://api.github.com/users/creative-commoners/events{/privacy}","received_events_url":"https://api.github.com/users/creative-commoners/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/creative-commoners/silverstripe-subsites","description":"Subsites module for SilverStripe CMS","fork":true,"url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites","forks_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/forks","keys_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/keys{/key_id}","collaborators_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/teams","hooks_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/hooks","issue_events_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/issues/events{/number}","events_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/events","assignees_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/assignees{/user}","branches_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/branches{/branch}","tags_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/tags","blobs_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/git/refs{/sha}","trees_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/git/trees{/sha}","statuses_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/statuses/{sha}","languages_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/languages","stargazers_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/stargazers","contributors_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/contributors","subscribers_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/subscribers","subscription_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/subscription","commits_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/commits{/sha}","git_commits_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/git/commits{/sha}","comments_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/comments{/number}","issue_comment_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/issues/comments{/number}","contents_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/contents/{+path}","compare_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/compare/{base}...{head}","merges_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/merges","archive_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/downloads","issues_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/issues{/number}","pulls_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/pulls{/number}","milestones_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/milestones{/number}","notifications_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/labels{/name}","releases_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/releases{/id}","deployments_url":"https://api.github.com/repos/creative-commoners/silverstripe-subsites/deployments","created_at":"2017-06-14T23:40:00Z","updated_at":"2018-08-20T13:38:51Z","pushed_at":"2018-08-24T05:00:14Z","git_url":"git://github.com/creative-commoners/silverstripe-subsites.git","ssh_url":"git@github.com:creative-commoners/silverstripe-subsites.git","clone_url":"https://github.com/creative-commoners/silverstripe-subsites.git","svn_url":"https://github.com/creative-commoners/silverstripe-subsites","homepage":"http://addons.silverstripe.org/add-ons/silverstripe/subsites","size":2391,"stargazers_count":0,"watchers_count":0,"language":"PHP","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":0,"open_issues":0,"watchers":0,"default_branch":"1.3"}},"base":{"label":"silverstripe:2.1","ref":"2.1","sha":"039a7a8c8429eb3496857838d6f784c3342aacfb","user":{"login":"silverstripe","id":379348,"node_id":"MDEyOk9yZ2FuaXphdGlvbjM3OTM0OA==","avatar_url":"https://avatars0.githubusercontent.com/u/379348?v=4","gravatar_id":"","url":"https://api.github.com/users/silverstripe","html_url":"https://github.com/silverstripe","followers_url":"https://api.github.com/users/silverstripe/followers","following_url":"https://api.github.com/users/silverstripe/following{/other_user}","gists_url":"https://api.github.com/users/silverstripe/gists{/gist_id}","starred_url":"https://api.github.com/users/silverstripe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/silverstripe/subscriptions","organizations_url":"https://api.github.com/users/silverstripe/orgs","repos_url":"https://api.github.com/users/silverstripe/repos","events_url":"https://api.github.com/users/silverstripe/events{/privacy}","received_events_url":"https://api.github.com/users/silverstripe/received_events","type":"Organization","site_admin":false},"repo":{"id":1236940,"node_id":"MDEwOlJlcG9zaXRvcnkxMjM2OTQw","name":"silverstripe-subsites","full_name":"silverstripe/silverstripe-subsites","owner":{"login":"silverstripe","id":379348,"node_id":"MDEyOk9yZ2FuaXphdGlvbjM3OTM0OA==","avatar_url":"https://avatars0.githubusercontent.com/u/379348?v=4","gravatar_id":"","url":"https://api.github.com/users/silverstripe","html_url":"https://github.com/silverstripe","followers_url":"https://api.github.com/users/silverstripe/followers","following_url":"https://api.github.com/users/silverstripe/following{/other_user}","gists_url":"https://api.github.com/users/silverstripe/gists{/gist_id}","starred_url":"https://api.github.com/users/silverstripe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/silverstripe/subscriptions","organizations_url":"https://api.github.com/users/silverstripe/orgs","repos_url":"https://api.github.com/users/silverstripe/repos","events_url":"https://api.github.com/users/silverstripe/events{/privacy}","received_events_url":"https://api.github.com/users/silverstripe/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/silverstripe/silverstripe-subsites","description":"Subsites module for SilverStripe CMS","fork":false,"url":"https://api.github.com/repos/silverstripe/silverstripe-subsites","forks_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/forks","keys_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/keys{/key_id}","collaborators_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/teams","hooks_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/hooks","issue_events_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/issues/events{/number}","events_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/events","assignees_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/assignees{/user}","branches_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/branches{/branch}","tags_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/tags","blobs_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/git/refs{/sha}","trees_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/git/trees{/sha}","statuses_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/statuses/{sha}","languages_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/languages","stargazers_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/stargazers","contributors_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/contributors","subscribers_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/subscribers","subscription_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/subscription","commits_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/commits{/sha}","git_commits_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/git/commits{/sha}","comments_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/comments{/number}","issue_comment_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/issues/comments{/number}","contents_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/contents/{+path}","compare_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/compare/{base}...{head}","merges_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/merges","archive_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/downloads","issues_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/issues{/number}","pulls_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls{/number}","milestones_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/milestones{/number}","notifications_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/labels{/name}","releases_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/releases{/id}","deployments_url":"https://api.github.com/repos/silverstripe/silverstripe-subsites/deployments","created_at":"2011-01-10T04:33:56Z","updated_at":"2018-08-23T22:27:23Z","pushed_at":"2018-08-24T05:02:15Z","git_url":"git://github.com/silverstripe/silverstripe-subsites.git","ssh_url":"git@github.com:silverstripe/silverstripe-subsites.git","clone_url":"https://github.com/silverstripe/silverstripe-subsites.git","svn_url":"https://github.com/silverstripe/silverstripe-subsites","homepage":"http://addons.silverstripe.org/add-ons/silverstripe/subsites","size":2390,"stargazers_count":61,"watchers_count":61,"language":"PHP","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":91,"mirror_url":null,"archived":false,"open_issues_count":36,"license":{"key":"bsd-3-clause","name":"BSD 3-Clause \"New\" or \"Revised\" License","spdx_id":"BSD-3-Clause","url":"https://api.github.com/licenses/bsd-3-clause","node_id":"MDc6TGljZW5zZTU="},"forks":91,"open_issues":36,"watchers":61,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/388"},"html":{"href":"https://github.com/silverstripe/silverstripe-subsites/pull/388"},"issue":{"href":"https://api.github.com/repos/silverstripe/silverstripe-subsites/issues/388"},"comments":{"href":"https://api.github.com/repos/silverstripe/silverstripe-subsites/issues/388/comments"},"review_comments":{"href":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/388/comments"},"review_comment":{"href":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/silverstripe/silverstripe-subsites/pulls/388/commits"},"statuses":{"href":"https://api.github.com/repos/silverstripe/silverstripe-subsites/statuses/6af985420f896063694fc4f5aacfcc3c08c8a3a9"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"robbieaverill"}}