filter page views by accessible accounts

fixes CNVS-39916

test plan:
 * enable page views (repeat once each for db, cassandra, pv4)
 * have a user associated with multiple root accounts
 * as that user, browse around both accounts
 * as a site admin, view the user; you should see all page views
 * as an admin in only one of the accounts, you should see only
   the page views from the same account

Change-Id: I79c5788c30ef87c7ff6a2ad31a909f9947e7b35c
Reviewed-on: https://gerrit.instructure.com/129850
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

app/controllers/page_views_controller.rb

@@ -190,6 +190,7 @@ class PageViewsController < ApplicationController
       date_options[:newest] = end_time
       url_options[:end_time] = params[:end_time]
     end
+    date_options[:viewer] = @current_user
     page_views = @user.page_views(date_options)
     url = api_v1_user_page_views_url(url_options)
 
@@ -201,7 +202,7 @@ class PageViewsController < ApplicationController
       format.csv do
         cancel_cache_buster
 
-        csv = PageView::CsvReport.new(@user).generate
+        csv = PageView::CsvReport.new(@user, @current_user).generate
 
         options = {
           type: 'text/csv',

app/models/page_view.rb

@@ -298,15 +298,27 @@ class PageView < ActiveRecord::Base
   # basically, it responds to #paginate and returns a
   # WillPaginate::Collection-like object
   def self.for_user(user, options={})
+    viewer = options.delete(:viewer)
+    viewer = nil if viewer == user
+    viewer = nil if viewer && Account.site_admin.grants_any_right?(viewer, :view_statistics, :manage_students)
     user.shard.activate do
       if PageView.pv4?
-        pv4_client.for_user(user.global_id, **options)
+        result = pv4_client.for_user(user.global_id, **options)
+        result = AccountFilter.filter(result, viewer) if viewer
+        result
       elsif PageView.cassandra?
-        PageView::EventStream.for_user(user, options)
+        result = PageView::EventStream.for_user(user, options)
+        result = AccountFilter.filter(result, viewer) if viewer
+        result
       else
         scope = self.where(:user_id => user).order('created_at desc')
         scope = scope.where("created_at >= ?", options[:oldest]) if options[:oldest]
         scope = scope.where("created_at <= ?", options[:newest]) if options[:newest]
+        if viewer
+          accounts = user.associated_accounts.shard(user).select { |a| a.grants_any_right?(viewer, :view_statistics, :manage_students) }
+          accounts << nil
+          scope = scope.where(account_id: accounts)
+        end
         scope
       end
     end

app/models/page_view/account_filter.rb

@@ -0,0 +1,41 @@
+#
+# Copyright (C) 2017 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+class PageView
+  class AccountFilter
+    def self.filter(collection, viewer)
+      BookmarkedCollection.filter(collection, &new(viewer).method(:filter))
+    end
+
+    def initialize(viewer)
+      @viewer = viewer
+      @accounts = {}
+    end
+
+    def filter(pv)
+      return true if pv.account_id.nil?
+      return @accounts[pv.account_id] if @accounts.key?(pv.account_id)
+      # this weird chain is to efficiently check if the user has access to
+      # view statistics in any sub account of the given root account
+      @accounts[pv.account_id] = pv.account.
+        all_account_users_for(@viewer).
+        map(&:account).uniq.
+        any? { |au| au.grants_any_right?(@viewer, :view_statistics, :manage_students) }
+    end
+  end
+end

app/models/page_view/csv_report.rb

@@ -21,8 +21,9 @@ class PageView
 
     attr_reader :user, :limit
 
-    def initialize(user)
+    def initialize(user, viewer = nil)
       @user = user
+      @viewer = viewer
       @limit = Setting.get('page_views_csv_export_rows', '300').to_i
     end
 
@@ -49,7 +50,7 @@ class PageView
     end
 
     def page_views(page)
-      user.page_views.paginate(page: page, per_page: limit)
+      user.page_views(viewer: @viewer).paginate(page: page, per_page: limit)
     end
 
     def header

gems/bookmarked_collection/lib/bookmarked_collection.rb

@@ -71,6 +71,7 @@ module BookmarkedCollection
   require 'bookmarked_collection/composite_proxy'
   require 'bookmarked_collection/concat_collection'
   require 'bookmarked_collection/concat_proxy'
+  require 'bookmarked_collection/filter_proxy'
   require 'bookmarked_collection/merge_proxy'
   require 'bookmarked_collection/simple_bookmarker'
   require 'bookmarked_collection/wrap_proxy'
@@ -259,4 +260,10 @@ module BookmarkedCollection
   def self.concat(*collections)
     BookmarkedCollection::ConcatProxy.new(collections)
   end
+
+  # Filters the results of a collection to only include rows that the
+  # filter_proc returns true for.
+  def self.filter(collection, &filter_proc)
+    BookmarkedCollection::FilterProxy.new(collection, &filter_proc)
+  end
 end

gems/bookmarked_collection/lib/bookmarked_collection/filter_proxy.rb

@@ -0,0 +1,56 @@
+#
+# Copyright (C) 2017 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+class BookmarkedCollection::FilterProxy < BookmarkedCollection::Proxy
+  def initialize(collection, &filter_proc)
+    @collection = collection
+    @filter_proc = filter_proc
+    super(@collection.new_pager, nil)
+  end
+
+  def execute_pager(pager)
+    bookmark = pager.current_bookmark
+    subpager = @collection.new_pager
+
+    # keep paginating until we fill the pager
+    loop do
+      # always grab a full page, to avoid situations where we keep
+      # repeating the underlying query over and over searching for
+      # a single non-filtered item
+      subpager.per_page = pager.per_page + 1
+      subpager.current_bookmark = bookmark
+      subpager = @collection.execute_pager(subpager)
+
+      break if subpager.empty?
+      bookmark = subpager.next_bookmark
+
+      while pager.size < pager.per_page && !subpager.empty?
+        item = subpager.shift
+        next unless @filter_proc.call(item)
+        pager << item
+      end
+
+      break if bookmark.nil?
+      break if pager.per_page == pager.size
+    end
+
+    pager.next_bookmark = subpager.bookmark_for(pager.last) if !subpager.empty? || subpager.next_bookmark
+
+    pager
+  end
+end

gems/bookmarked_collection/spec/bookmarked_collection/proxy_spec.rb

@@ -71,5 +71,37 @@ describe BookmarkedCollection::Proxy do
     it 'should not set next_bookmark if the page was the last' do
       expect(@proxy.paginate(:per_page => @scope.count).next_bookmark).to be_nil
     end
+
+    context 'filtering' do
+      before do
+        middle_item = @scope.limit(2).last
+        bookmarker = BookmarkedCollection::SimpleBookmarker.new(@scope.klass, :id)
+        @proxy = BookmarkedCollection.wrap(bookmarker, @scope)
+        @proxy = BookmarkedCollection.filter(@proxy) do |item|
+          item != middle_item
+        end
+      end
+
+      it "excludes the middle item" do
+        pager = @proxy.paginate(per_page: 6)
+        expect(pager).to eq [@scope.first, @scope.last]
+        expect(pager.next_bookmark).to be_nil
+      end
+
+      it "repeats the subpager when there are excluded items" do
+        pager = @proxy.paginate(per_page: 1)
+        expect(pager).to eq [@scope.first]
+        expect(pager.next_bookmark).not_to be_nil
+        pager = @proxy.paginate(page: pager.next_page, per_page: 1)
+        expect(pager).to eq [@scope.last]
+        expect(pager.next_bookmark).to be_nil
+      end
+
+      it "gets the next_bookmark right on a boundary" do
+        pager = @proxy.paginate(per_page: 2)
+        expect(pager).to eq [@scope.first, @scope.last]
+        expect(pager.next_bookmark).to be_nil
+      end
+    end
   end
 end

spec/models/page_view_spec.rb

@@ -22,7 +22,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../cassandra_spec_helper.rb'
 describe PageView do
   before do
     # sets both @user and @course (@user is a teacher in @course)
-    course_model
+    course_model(account: Account.default.manually_created_courses_account)
     @page_view = PageView.new { |p| p.assign_attributes({ :created_at => Time.now, :url => "http://test.one/", :session_id => "phony", :context => @course, :controller => 'courses', :action => 'show', :user_request => true, :render_time => 0.01, :user_agent => 'None', :account_id => Account.default.id, :request_id => "abcde", :interaction_seconds => 5, :user => @user }) }
   end
 
@@ -140,6 +140,29 @@ describe PageView do
       expect(@user.page_views.paginate(:per_page => 2, :page => '3')).to eq [@page_view]
     end
 
+    context "filtering" do
+      it "restricts results to accounts that the viewer can see" do
+        @page_view.save!
+        user = @user
+        viewer1 = User.create!
+        viewer2 = account_admin_user
+        viewer3 = account_admin_user(account: @course.account)
+        expect(@course.account).not_to eq Account.default
+
+        other_root = Account.create!
+        user.pseudonyms.create!(account: other_root, unique_id: 'bob')
+        expect(user.associated_accounts).to be_include(other_root)
+        viewer4 = account_admin_user(account: other_root)
+        expect(user.grants_right?(viewer4, :view_statistics)).to eq true
+
+        expect(user.page_views(viewer: viewer1).paginate(per_page: 2)).to eq []
+        expect(user.page_views(viewer: viewer2).paginate(per_page: 2)).to eq [@page_view]
+        expect(user.page_views(viewer: viewer3).paginate(per_page: 2)).to eq [@page_view]
+        expect(user.page_views(viewer: viewer4).paginate(per_page: 2)).to eq []
+      end
+
+    end
+
     describe "db migrator" do
       it "should migrate the relevant page views" do
         a1 = account_model