permission context in addressbook service call

closes CNVS-33774

when making an addressbook service call, include information about the
courses and accounts in which the sender (if any) has (or lacks)
relevant permissions.

test-plan:
- have canvas communicating with the address book service
- set up a user who's:
  - a teacher in one course (grants send_messages permission)
  - an observer in another course (lacks send_messages permission)
  - an admin in a separate subaccount (grants read_roster permission)
- visit the inbox and search for a recipient (search details irrelevant)
- look in the request logs of the address book service; the requests
  for your search should include:
  - restricted_course_ids parameter that:
    - includes the id of the course in which the user's an observer
    - does not include the id of the course in which the user's a
      teacher
  - visible_account_ids parameter that:
    - includes the id of the account in which the user's an admin
    - does not include the id of the other accounts the user's
      associated with

Change-Id: I2c1c3dcb9d69a4053e19c8803f27024d4b3d653c
Reviewed-on: https://gerrit.instructure.com/100599
Reviewed-by: Andrew Huff <ahuff@instructure.com>
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Tested-by: Jenkins
Product-Review: Jacob Fugal <jacob@instructure.com>

Author: lukfugl

lib/services/address_book.rb

@@ -135,28 +135,36 @@ def query_params(params={})
         query_params[:cursor] = params[:cursor] if params[:cursor]
         query_params[:per_page] = params[:per_page] if params[:per_page]
         query_params[:search] = params[:search] if params[:search]
-        query_params[:for_sender] = serialize_user(params[:sender]) if params[:sender]
+        if params[:sender]
+          sender = params[:sender]
+          sender = User.find(sender) unless sender.is_a?(User)
+          visible_accounts = sender.associated_accounts.select{ |account| account.grants_right?(sender, :read_roster) }
+          restricted_courses = sender.all_courses.reject{ |course| course.grants_right?(sender, :send_messages) }
+          query_params[:for_sender] = serialize_item(sender)
+          query_params[:visible_account_ids] = serialize_list(visible_accounts) unless visible_accounts.empty?
+          query_params[:restricted_course_ids] = serialize_list(restricted_courses) unless restricted_courses.empty?
+        end
         query_params[:in_context] = serialize_context(params[:context]) if params[:context]
-        query_params[:user_ids] = serialize_users(params[:user_ids]) if params[:user_ids]
-        query_params[:exclude_ids] = serialize_users(params[:exclude_ids]) if params[:exclude_ids]
+        query_params[:user_ids] = serialize_list(params[:user_ids]) if params[:user_ids]
+        query_params[:exclude_ids] = serialize_list(params[:exclude_ids]) if params[:exclude_ids]
         query_params[:weak_checks] = 1 if params[:weak_checks]
         query_params
       end
 
-      def serialize_user(user)
-        Shard.global_id_for(user)
+      def serialize_item(item)
+        Shard.global_id_for(item)
       end
 
-      def serialize_users(users)
-        users.map{ |user| serialize_user(user) }.join(',')
+      def serialize_list(list) # can be either IDs or objects (e.g. User)
+        list.map{ |item| serialize_item(item) }.join(',')
       end
 
       def serialize_context(context)
         if context.respond_to?(:global_asset_string)
           context.global_asset_string
         else
           context_type, context_id, scope = context.split('_', 3)
-          global_context_id = Shard.global_id_for(context_id)
+          global_context_id = serialize_item(context_id)
           asset_string = "#{context_type}_#{global_context_id}"
           asset_string += "_#{scope}" if scope
           asset_string

spec/lib/services/address_book_spec.rb

@@ -47,6 +47,24 @@ def not_match(*args)
       ::RSpec::Matchers::AliasedNegatedMatcher.new(match(*args), ->{})
     end
 
+    matcher :with_param do |param, value|
+      match do |url|
+        if value.is_a?(Array)
+          return false unless url =~ %r{[?&]#{param}=(\d+(%2C\d+)*)(?:&|$)}
+          actual = $1.split('%2C').map(&:to_i)
+          return actual.sort == value.sort
+        else
+          url =~ %r{[?&]#{param}=#{value}(?:&|$)}
+        end
+      end
+    end
+
+    matcher :with_param_present do |param|
+      match do |url|
+        url =~ %r{[?&]#{param}=}
+      end
+    end
+
     describe "jwt" do
       it "signs with the base64 decoded secret from the configuration" do
         jwt = Services::AddressBook.jwt
@@ -77,65 +95,81 @@ def not_match(*args)
       end
 
       it "normalizes sender from a User to its global ID as for_sender param" do
-        expect_request(%r{for_sender=#{@sender.global_id}})
+        expect_request(with_param(:for_sender, @sender.global_id))
         Services::AddressBook.recipients(sender: @sender)
       end
 
       it "normalizes sender from an ID to a global ID as for_sender param" do
-        expect_request(%r{for_sender=#{@sender.global_id}})
+        expect_request(with_param(:for_sender, @sender.global_id))
         Services::AddressBook.recipients(sender: @sender.id)
       end
 
+      it "includes the sender's visible accounts" do
+        account1 = account_model
+        account2 = account_model
+        account_admin_user(user: @sender, account: account1)
+        account_admin_user(user: @sender, account: account2)
+        expect_request(with_param(:visible_account_ids, [account1.global_id, account2.global_id]))
+        Services::AddressBook.recipients(sender: @sender)
+      end
+
+      it "includes the sender's restricted courses" do
+        course1 = course_with_observer(user: @sender, active_all: true).course
+        course2 = course_with_observer(user: @sender, active_all: true).course
+        expect_request(with_param(:restricted_course_ids, [course1.global_id, course2.global_id]))
+        Services::AddressBook.recipients(sender: @sender)
+      end
+
       it "normalizes context from e.g. a Course to its global asset string as in_context param" do
-        expect_request(%r{in_context=#{@course.global_asset_string}})
+        expect_request(with_param(:in_context, @course.global_asset_string))
         Services::AddressBook.recipients(context: @course)
       end
 
       it "normalizes context from an asset string to a global asset string as in_context param" do
-        expect_request(%r{in_context=#{@course.global_asset_string}})
+        expect_request(with_param(:in_context, @course.global_asset_string))
         Services::AddressBook.recipients(context: @course.asset_string)
       end
 
       it "normalizes context from a scoped asset string to a scoped global asset string as in_context param" do
-        expect_request(%r{in_context=#{@course.global_asset_string}_students})
+        expect_request(with_param(:in_context, "#{@course.global_asset_string}_students"))
         Services::AddressBook.recipients(context: "#{@course.asset_string}_students")
       end
 
       it "normalizes user_ids from Users to a comma-separated list of their global IDs as user_ids param" do
         recipient1 = user_model
         recipient2 = user_model
-        expect_request(%r{user_ids=#{recipient1.global_id}%2C#{recipient2.global_id}})
+        expect_request(with_param(:user_ids, [recipient1.global_id, recipient2.global_id]))
         Services::AddressBook.recipients(user_ids: [recipient1, recipient2])
       end
 
       it "normalizes user_ids from IDs to a comma-separated list of global IDs as user_ids param" do
         recipient1 = user_model
         recipient2 = user_model
-        expect_request(%r{user_ids=#{recipient1.global_id}%2C#{recipient2.global_id}})
+        expect_request(with_param(:user_ids, [recipient1.global_id, recipient2.global_id]))
         Services::AddressBook.recipients(user_ids: [recipient1.id, recipient2.id])
       end
 
       it "normalizes exclude_ids from Users to a comma-separated list of their global IDs as exclude_ids param" do
         recipient1 = user_model
         recipient2 = user_model
-        expect_request(%r{exclude_ids=#{recipient1.global_id}%2C#{recipient2.global_id}})
+        expect_request(with_param(:exclude_ids, [recipient1.global_id, recipient2.global_id]))
         Services::AddressBook.recipients(exclude_ids: [recipient1, recipient2])
       end
 
       it "normalizes exclude_ids from IDs to a comma-separated list of global IDs as exclude_ids param" do
         recipient1 = user_model
         recipient2 = user_model
-        expect_request(%r{exclude_ids=#{recipient1.global_id}%2C#{recipient2.global_id}})
+        expect_request(with_param(:exclude_ids, [recipient1.global_id, recipient2.global_id]))
         Services::AddressBook.recipients(exclude_ids: [recipient1.id, recipient2.id])
       end
 
       it "normalizes weak_checks to 1 if truthy" do
-        expect_request(%r{weak_checks=1})
+        expect_request(with_param(:weak_checks, 1))
         Services::AddressBook.recipients(weak_checks: true)
       end
 
       it "omits weak_checks if falsey" do
-        expect_request(not_match(%r{weak_checks=}))
+        expect_request(not_match(with_param_present(:weak_checks)))
         Services::AddressBook.recipients(weak_checks: false)
       end
 
@@ -191,12 +225,12 @@ def not_match(*args)
       end
 
       it "normalizes sender same as recipients" do
-        expect_request(%r{for_sender=#{@sender.global_id}}, body: @response)
+        expect_request(with_param(:for_sender, @sender.global_id), body: @response)
         Services::AddressBook.count_recipients(sender: @sender)
       end
 
       it "normalizes context same as recipients" do
-        expect_request(%r{in_context=#{@course.global_asset_string}}, body: @response)
+        expect_request(with_param(:in_context, @course.global_asset_string), body: @response)
         Services::AddressBook.count_recipients(context: @course)
       end
 
@@ -214,40 +248,40 @@ def not_match(*args)
       end
 
       it "passes the sender to the recipients call" do
-        expect_request(%r{sender=})
+        expect_request(with_param_present(:for_sender))
         Services::AddressBook.common_contexts(@sender, [1, 2, 3])
       end
 
       it "passes the user_ids to the recipients call" do
         recipient1 = user_model
         recipient2 = user_model
-        expect_request(%r{user_ids=#{recipient1.global_id}%2C#{recipient2.global_id}})
+        expect_request(with_param(:user_ids, [recipient1.global_id, recipient2.global_id]))
         Services::AddressBook.common_contexts(@sender, [recipient1.id, recipient2.id])
       end
     end
 
     describe "roles_in_context" do
       it "makes a recipient request with no sender" do
         expect_request(%r{/recipients\?})
-        expect_request(%r{sender=}).never
+        expect_request(with_param_present(:for_sender)).never
         Services::AddressBook.roles_in_context(@course, [1, 2, 3])
       end
 
       it "passes the user_ids to the recipients call" do
         recipient1 = user_model
         recipient2 = user_model
         expect(recipient1.global_id).to eql(Shard.global_id_for(recipient1.id))
-        expect_request(%r{user_ids=#{recipient1.global_id}%2C#{recipient2.global_id}})
+        expect_request(with_param(:user_ids, [recipient1.global_id, recipient2.global_id]))
         Services::AddressBook.roles_in_context(@course, [recipient1.id, recipient2.id])
       end
 
       it "passes the context to the recipients call" do
-        expect_request(%r{in_context=#{@course.global_asset_string}})
+        expect_request(with_param(:in_context, @course.global_asset_string))
         Services::AddressBook.roles_in_context(@course, [1, 2, 3])
       end
 
       it "uses the course as the context for a course section" do
-        expect_request(%r{in_context=#{@course.global_asset_string}})
+        expect_request(with_param(:in_context, @course.global_asset_string))
         Services::AddressBook.roles_in_context(@course.default_section, [1, 2, 3])
       end
     end
@@ -259,17 +293,17 @@ def not_match(*args)
       end
 
       it "passes the sender to the recipients call if not is_admin" do
-        expect_request(%r{sender=#{@sender.global_id}})
+        expect_request(with_param(:for_sender, @sender.global_id))
         Services::AddressBook.known_in_context(@sender, @course.asset_string)
       end
 
       it "omits the sender form the recipients call if is_admin" do
-        expect_request(not_match(%r{sender=}))
+        expect_request(not_match(with_param_present(:for_sender)))
         Services::AddressBook.known_in_context(@sender, @course.asset_string, true)
       end
 
       it "passes the context to the recipients call" do
-        expect_request(%r{in_context=#{@course.global_asset_string}})
+        expect_request(with_param(:in_context, @course.global_asset_string))
         Services::AddressBook.known_in_context(@sender, @course.asset_string)
       end
 
@@ -302,12 +336,12 @@ def not_match(*args)
       end
 
       it "passes the sender to the count_recipients call" do
-        expect_request(%r{sender=#{@sender.global_id}})
+        expect_request(with_param(:for_sender, @sender.global_id))
         Services::AddressBook.count_in_context(@sender, @course.asset_string)
       end
 
       it "passes the context to the count_recipients call" do
-        expect_request(%r{in_context=#{@course.global_asset_string}})
+        expect_request(with_param(:in_context, @course.global_asset_string))
         Services::AddressBook.count_in_context(@sender, @course.asset_string)
       end
 
@@ -325,35 +359,35 @@ def not_match(*args)
       end
 
       it "only has search parameter by default" do
-        expect_request(%r{search=bob})
-        expect_request(%r{in_context=}).never
-        expect_request(%r{exclude_ids=}).never
-        expect_request(%r{weak_checks=}).never
+        expect_request(with_param(:search, 'bob'))
+        expect_request(with_param_present(:in_context)).never
+        expect_request(with_param_present(:exclude_ids)).never
+        expect_request(with_param_present(:weak_checks)).never
         Services::AddressBook.search_users(@sender, search: 'bob')
       end
 
       it "passes context to recipients call" do
-        expect_request(%r{in_context=})
+        expect_request(with_param_present(:in_context))
         Services::AddressBook.search_users(@sender, search: 'bob', context: @course)
       end
 
       it "includes sender if is_admin but no context given" do
-        expect_request(%r{sender=})
+        expect_request(with_param_present(:for_sender))
         Services::AddressBook.search_users(@sender, search: 'bob', is_admin: true)
       end
 
       it "omits sender if is_admin specified with context" do
-        expect_request(not_match(%r{sender=}))
+        expect_request(not_match(with_param_present(:sender)))
         Services::AddressBook.search_users(@sender, search: 'bob', context: @course, is_admin: true)
       end
 
       it "passes exclude_ids to recipients call" do
-        expect_request(%r{exclude_ids=})
+        expect_request(with_param_present(:exclude_ids))
         Services::AddressBook.search_users(@sender, search: 'bob', exclude_ids: [1, 2, 3])
       end
 
       it "passes weak_checks flag along to recipients" do
-        expect_request(%r{weak_checks=})
+        expect_request(with_param_present(:weak_checks))
         Services::AddressBook.search_users(@sender, search: 'bob', weak_checks: true)
       end
 
@@ -375,12 +409,12 @@ def not_match(*args)
       end
 
       it "passes cursor parameter to the service" do
-        expect_request(%r{cursor=12})
+        expect_request(with_param(:cursor, 12))
         Services::AddressBook.search_users(@sender, {search: 'bob'}, {cursor: 12})
       end
 
       it "passes per_page parameter to the service" do
-        expect_request(%r{per_page=20})
+        expect_request(with_param(:per_page, 20))
         Services::AddressBook.search_users(@sender, {search: 'bob'}, {per_page: 20})
       end
     end