[skip-stages=Flakey]
Change-Id: I6abefdfa9fed6dd4525c8786e93efa548b3710f2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/319603
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
Migration-Review: Jacob Burroughs <jburroughs@instructure.com>
so that we're not re-implementing it at multiple callsites
also remove unused error classes
Change-Id: I938d705729f2208532b4522eddbc8edfa4f2031f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/269561
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
refs FOO-1125
flag=non
TEST PLAN:
1) stats for things like ImperiumTimeouts should still
end up in datadog
2) sentry errors for the target error types should disappear
Change-Id: I6e97c04e3f6fcc3545b10418511934c89f20a419
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251536
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
closes RECNVS-388
refs RECNVS-387
test plan:
* smoke test that address book still works with canvas
Change-Id: I01fb1802f748494ac056eab26b196a7a4f7de10e
Reviewed-on: https://gerrit.instructure.com/150614
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Tested-by: Jenkins
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Xander Moffatt <xmoffatt@instructure.com>
Fixes issue with address-book "performance tap" plugin setting.
Test Plan:
* Navigate to "/plugins/address_book"
* Select "AddressBook performance tap" from "Answer address book
queries through" option and click "apply"
* Navigate to Inbox and compose new message
* Message should send without issue (previously gave "unexpected error
ID: ###" message)
closes CNVS-38284
Change-Id: I12d5dd50f716128d44599993152a4bf91e78555d
Reviewed-on: https://gerrit.instructure.com/119752
Tested-by: Jenkins
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
closes CNVS-35834
* allow specifying tree, service, and cluster for consul stuff
* check multiple consul keys for each setting (cluster, env, region, global)
test plan:
* an existing consul environment still works
Change-Id: I48e8fadeac2e140973bfc4b41c1cfb386532d15c
Reviewed-on: https://gerrit.instructure.com/125271
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
refs CNVS-37900
with the old MessageableUser implementation, the behavior is identical.
but when using the service, it allows using one HTTP request against the
service's new /recipients/counts endpoint instead of multiple HTTP
requests against the deprecated /recipients/count endpoint
test-plan:
- enable the address-book service
- a query that involves multiple contexts in the result still gets
counts for each of those contexts
- said query is faster, making one request for counts to the service
instead of one each
Change-Id: I2793984f00247b3af3ebb8ba9ab559b38889c045
Reviewed-on: https://gerrit.instructure.com/117677
Tested-by: Jenkins
Reviewed-by: Andrew Huff <ahuff@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
Since some environments share a consul datacenter we need to be able to
differentiate configurations.
Fixes: CNVS-34341
Test Plan:
- Nothing uses this yet but we need to make sure we haven't broken JWT
secrets, the RCE, and Address Book.
Change-Id: I496a8f7d2cafd02c3177a28b348679e552965c0d
Reviewed-on: https://gerrit.instructure.com/99650
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
fixes CNVS-35217, CNVS-35799, CNVS-35800
- allow Services::AddressBook.known_in_context to take a whitelist of
user_ids. enables...
- allow address_book.known_users to take a context parameter (replaces
include_context; see note below). enables...
- use known_users(users, context: ...) when normalizing recipients in
the conversations controller, in place of post-filtering results from
known_in_context(context, maybe_admin).
- remove is_admin flag from address_book.known_in_context; admin view
never utilized for this method
- also remove is_admin flag from address_book.search_users; admin view
is determined implicitly when context parameter present
- move admin_context checks from conversations controller and search
controller inside the address book
- don't apply read_as_admin check when evaluating params[:context] for
visible contexts in search results (it wasn't intended for that, only
for @admin_context, which is gone)
- finally, make passing of contexts consistent; all of known_users,
known_in_context, and search_users allow asset_strings _or_ context
objects
note on include_context: include_context was intended to force inclusion
of participants in an otherwise unconnected context for an externally
vetted admin, without restricting other contexts. the context parameter
is instead intended to restrict to participants known either through
that context, or as an admin over that context while other contexts are
ignored. this is more consistent with how the call was being used.
meanwhile, whether an admin view is appropriate for that context is
determined within the address book instead of by the caller.
when determining whether a context is eligible for admin view within
known_users or search_users, the sender must have read_as_admin
permission on the context _and_ not be a participant in the context's
course (if any). this allows restrictions due to the course
participation to override read_as_admin, as they should.
test-plan:
[CNVS-35217]
- have an account with an account admin
- have a course in that account with two teachers (one section-limited,
one not), and a student in a different section than the
section-limited teacher (all active, published, etc.)
- ingest the data into the address book service and configure canvas to
use the service
- as the account admin, search for the student in the inbox in the
context of the course; should find them.
- as the non-section-limited teacher, search for the student in the
inbox in the context of the course; should find them.
- as the section-limited teacher, search for the student in the inbox in
the context of the course; should not find them.
[CNVS-35799, CNVS-35800]
- add two more students to the same course, then remove one from the
course and deactivate the other
- re-ingest the data into the address book service (or have had the
kinesis stream hooked up during the change)
- as the non-section-limited teacher, search for the student that was
added but then removed; should not find them
- as the non-section-limited teacher, search for the student that was
deactivated; should not find them
- as the account admin, search for the student that was deactivated;
should not find them
Change-Id: I3233d8d45394c9233adc9888bfaf477e73bfde11
Reviewed-on: https://gerrit.instructure.com/107053
Tested-by: Jenkins
Reviewed-by: Andrew Huff <ahuff@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
Also, make Consul container accessible from the host.
Fixes: CNVS-35831
Refs: CNVS-34341, CNVS-32864
Test Plan:
- Smoke test RCS and Canvas running together to make sure they still
play nice.
Change-Id: I418d54a176677b1df8ec42a009752807908a847c
Reviewed-on: https://gerrit.instructure.com/99443
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
closes CNVS-34528
another option for the address book strategy chosen in the plugin.
combines answers from current MessageableUser implementation with
traffic directed to the service as if it were in use. but the service is
instructed via `ignore_result` parameter to reply to canvas as quickly
as possible, so that if the service does have performance issues, actual
canvas performance is not affected.
test-plan:
- set up canvas to be able to talk to the address book service
- have a published course with an active teacher and an active student
- do _not_ replicate that data into the address book service; i.e. the
service answer should be wrong if used
- choose the "AddressBook performance tap" as the address book
implementation in the plugin setting
- logged in as the teacher, query for the student in the inbox. should
find the student (backed by the messageableuser implementation)
- check the service request logs; even though the service response
wasn't used, there should still be a request made against it
- take down the service entirely
- repeat the query in the inbox. should still get the result from the
messageableuser implementation despite the service being unavailable
Change-Id: I031b7c397d2e20d74d7699a81ca8040064a8df75
Reviewed-on: https://gerrit.instructure.com/104112
Reviewed-by: Andrew Huff <ahuff@instructure.com>
Tested-by: Jenkins
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
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>
closes CNVS-34484
address-book service recently updated with pagination info available. in
the process, the shape of its response also changed. update for that
change in shape, and capture the pagination info it returns into the
bookmarker so it can be used for pagination here on canvas' end.
test-plan:
- have a published course with a teacher and >10 students
- have the address-book service running, your canvas configured to use
it, and your canvas' data ingested into the service
- as the teacher, got to conversations and start a new message
- in the recipients field, open the address book and drill into that
course's students
- should see all the course students (page traversal is happening
transparently)
- should see multiple requests in the address book service's logs
Change-Id: I665ac77d01d93e74ac41f6291b4dc698c0446429
Reviewed-on: https://gerrit.instructure.com/100273
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Tested-by: Jenkins
Product-Review: Jacob Fugal <jacob@instructure.com>
refs CNVS-31937
the secret stored in consul/dynamicsettings should be base64 encoded.
test-plan:
- NOTE: depends on having version of service that also base64 decodes
the shared secret
- put correct base64 encoded secret in config (consul,
dynamicsettings.yml, etc.), make requests against the service --
should work
- put incorrect but valid base64 encoded secret in config, make
requests against the service -- should fail gracefully and log an
error report about invalid jwt
- put invalid base64 secret in config (so decoding will fail), make
requests against the service -- should fail gracefully and log error
reports both about base64 decoding failure and invalid jwt
- remove any secret in config, make requests against the service --
should fail gracefully and log error reports both about base64
decoding failure and invalid jwt
Change-Id: I450fc5838301db3b0bb948e3754b0c1566495123
Reviewed-on: https://gerrit.instructure.com/90805
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Tested-by: Jenkins
Product-Review: Jacob Fugal <jacob@instructure.com>
refs CNVS-31277
closes CNVS-31939
test-plan:
- configure your canvas to use an address book service instance that
has authentication enabled and has your data loaded
- configure your canvas to use the same secret as that service
- search for recipients on a new conversation in the inbox
- should find the recipients
- configure your canvas to use a different secret than that service
- search for recipients on a new conversation in the inbox
- search should come back empty, but gracefully so (no page error)
- error reports should show the request failed due to authentication
Change-Id: I3a5f1c4ddcbbf830c9e7b98f6c8a39d4fdc82461
Reviewed-on: https://gerrit.instructure.com/90399
Tested-by: Jenkins
Reviewed-by: Andrew Huff <ahuff@instructure.com>
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
closes CNVS-29869
refs CNVS-31303
adds an implementation of the AddressBook facade backed by an external
address book service. cleans up some implementation of the facade to
faciliate that.
intentionally punts on pagination through the service for right now.
that will be covered in a later commit addressing CNVS-31303.
test-plan:
- depends on having an address-book service that fulfills the expected
contract
- if using consul, add the host for your address-book service to
config/consul.yml (see config/consul.yml.example)
- if not using consul, add it to config/dynamic_settings.yml (see
config/dynamic_settings.yml.example)
- go to your canvas' /plugins/address-book and configure your account
to use the service implementation
- smoke test the inbox address book UI in canvas
Change-Id: I4e89bd7c2ac64b5a4902905cbdd37c8283f7adf5
Reviewed-on: https://gerrit.instructure.com/88431
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
Tested-by: Jenkins
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
Jacob Burroughs