closes CORE-1981
test plan
- rake db:initial_setup should run
Change-Id: I4ddebf513a4b75c7aa4ca16d9126198ced2acfc6
Reviewed-on: https://gerrit.instructure.com/170486
Tested-by: Jenkins
Reviewed-by: James Williams <jamesw@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
Refs ADMIN-1543
Test Plan:
* Discussion Topics should return their `created_at`
attribute from their respective APIs
Change-Id: Idbf600fa9db434019fbd1a7426224085351d1a5f
Reviewed-on: https://gerrit.instructure.com/169347
Tested-by: Jenkins
QA-Review: Mysti Sadler <mysti@instructure.com>
Product-Review: Dan Minkevitch <dan@instructure.com>
Reviewed-by: Carl Kibler <ckibler@instructure.com>
- For the `$Membership.role` and `$com.Instructure.membership.roles`
custom params, calculate role URNs using
`LIS_V2_LTI_ADVANTAGE_ROLE_MAP` for 1.3 tools, else
`LIS_V2_ROLE_MAP`.
- This way the `https://purl.imsglobal.org/spec/lti/claim/roles`
launch claim matches up with the NRPS `roles` field _except_ that
NRPS `roles` doesn't include system nor institution roles
(N+1 issues).
- Really the only meaningful difference is the TA role URN, where
`LIS_V2_LTI_ADVANTAGE_ROLE_MAP` corrects a typo in the
`LIS_V2_ROLE_MAP`. See `SubstitutionsHelper` line ~61 for all the
differences.
- Since the contents of `LIS_V2_LTI_ADVANTAGE_ROLE_MAP` changed
to include additional roles to support launches but which should not
be used as NRPS role filters, `MembershipsProvider.lis_roles` also
needed to change to filter those roles out. So took the opportunity
to rename that method to `queryable_roles` to avoid confusion with
`GroupMembershipDecorator.lti_roles` and
`CourseEnrollmentsDecorator.lti_roles` methods that perform the
mapping in the _opposite_ direction.
Closes LTIA-42
Test Plan
- Place a 1.3 tool in a `Course` with several active members, at
least one of which is in a TA role.
- Ensure the tool's configuration includes mappings for the
`$Membership.role` and `$com.Instructure.membership.roles` custom
params.
- Launch the tool for several members and verify correct contents
of:
- `https://purl.imsglobal.org/spec/lti/claim/roles`
- `https://purl.imsglobal.org/spec/lti/claim/custom['com_instructure_membership_roles']`
- `https://purl.imsglobal.org/spec/lti/claim/custom['membership_role']`
In particular, the first two should match and for a TA all three
should include
`http://purl.imsglobal.org/vocab/lis/v2/membership/Instructor#TeachingAssistant`
(capital 'I').
- Invoke NRPS for this tool and `Course`. For each `member`, the
`roles` field should match the first two launch fields listed
above _except_ that system and institution roles are _not_ listed.
The `com_instructure_membership_roles` and `membership_role` custom
params should render unexpanded, i.e. literally as
`$com.Instructure.membership.roles` and `$Membership.role`.
Change-Id: I315220edd0b5500934ede9a82047cc0206fbd8f5
Reviewed-on: https://gerrit.instructure.com/169626
Tested-by: Jenkins
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
Product-Review: Karl Lloyd <karl@instructure.com>
- Adds support for outputting two top-level claims
in the NRPS v2 per-member `message` array element:
- `https://www.instructure.com/canvas_user_id`
- `https://www.instructure.com/canvas_user_login_id`
- While the `picture` claim is not included here, it
_is_ normally included in a LTI 1.3 launch request, but
with a different lookup mechanims than what NRPS v2 had
been using prevously (`User.avatar_url` rather than
`User.avatar_image_url`). So for launch<->NRPS parity,
the latter has been changed to match the former.
- Since we now need to whitelist the "public" claim
group, we needed a way to exclude the `role_scope_mentor`
claimn because it results in a N+1. So that claim was promoted
to its own group.
- Since claim groups were being re-organized, took the
opportunity to expanded claim group tests to verify
`privacy_level` enforcement.
References LTIA-41
Test Plan
- Place a LTI 1.3-enabled tool in a `Course` with several
active members.
- Invoke the NRPS v2 service for that tool and `Course`.
- Verify correct rendering of the `picture` field in `members`
elements.
- Verify correct rendering of
`https://www.instructure.com/canvas_user_id` and
`https://www.instructure.com/canvas_user_login_id` top-level
claims in each `member`'s `message` array element.
- Adjust the tool's `privacy_policy` to be non-`public`, verify
that neither `https://www.instructure.com/canvas_user_id` and
`https://www.instructure.com/canvas_user_login_id` claim
renders.
Change-Id: Ic3f306591214dd25815e8e4acc6f5c8d3d3aafac
Reviewed-on: https://gerrit.instructure.com/169525
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Expands the `vnd.Canvas.Person.email.sis` custom param into
the per-`member` `message` array in NRPS v2 responses.
- The custom param expansion itself was already present - just
needed an alternate code path to avoid N+1 queries when
`CommunicationChannel` and `Pseudonym` associations are already
pre-loaded into `User`, as they are for NRPS v2.
Refs LTIA-41
Test Plan:
- Confgure a LTI 1.3/Advantage tool with the
`$vnd.Canvas.Person.email.sis` custom param.
- Place that tool into a `Course` with active members having
at least one user with a `Pseudonym` linked to a SIS
`CommunicationChannel` (ideally separate from the `User`'s
primary `Pseudonym`)
- Request NRPS v2 for that `Course`, specifying the course's LTI ID
as the value of the `rlid` query parameter.
- Verify that, for all active `Course` members, the expanded
`$vnd.Canvas.Person.email.sis` custom param
in the per-`member` NRPS v2 `message` array reflects the email
associated with the `User`'s SIS `CommunicationChannel` if present
or `null` otherwise.
Change-Id: I0c88b205608d4bc7b0c2974fe2766d5c59a90f76
Reviewed-on: https://gerrit.instructure.com/169325
QA-Review: Bill Smith <bsmith@instructure.com>
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
- Output a smaller version of a LTI 1.3 launch payload for
each NRPS v2 member when the NRPS request includes a `rlid`
query parameter.
- Most claims and custom params from the launch payload are
excluded either because:
- They describe the context and would thus be redundant, or
- They pose performance problems (N+1 queries, usually), or
- They are absent from the spec example, e.g.
`https://purl.imsglobal.org/spec/lti/claim/version`, or
- They require additional development and thus need to be
handled as a separate task.
- See `memberships_provider.rb` ~line 68 for list of
custom params supported in this commit. (More coming later.)
- Vast majority of the patch has to do with tests against
`JwtMessage`, which was modified to allow claims to be turned
on/off via a new white/blacklist mechanism in `AppUtil`.
- Custom param white/blacklisting is handled directly in
`VariableExpander` to satisfy the LTI rule that unsupported
params should just be echoed as-is. This (instead of keeping
all the white/blacklist support in `JwtMessage` ensures
consistent behavior w/r/t `VariableExpander`'s more sophisticated
features, specifically its suport for expanding variables embedded
into larger strings.
Closes LTIA-40
Test Plan
- Configure a LTI 1.3/Advantage tool with the supported set of
custom params listed in `memberships_provider.rb` starting ~line 68.
If using the POST
`/api/lti/accounts/:account_id/developer_keys/tool_configuration`
API, this is done by setting
`tool_configuration.settings.custom_fields` to a JSON object where
keys are the param name to be rendered into LTI payloads and values
are the $-prefixed custom param names themselves. Include several
nonsense entries as well as unsupported entries e.g.:
```
// ... snip ...
"tool_configuration": {
"settings": {
// ... snip ...
"custom_fields": {
"person_name_full": "$Person.name.full",
"person_name_display": "$Person.name.display",
"person_name_family": "$Person.name.family",
"person_name_given": "$Person.name.given",
"canvas_user_isrootaccountadmin": "$Canvas.user.isRootAccountAdmin"
"unsupported_param_1": "$unsupported.param.1",
"unsupported_param_2": "$unsupported.param.2"
}
// ... snip ...
}
// ... snip ...
}
// ... snip ...
```
- Place this tool into a course, ensure the course has several active
members.
- Launch the tool in order to observe the course context's LTI
identifier. Use that identifier as the value of the NRPS `rlid`
parameter, e.g. a GET to:
`/api/lti/courses/1/names_and_roles?rlid=4dde05e8ca1973bcca9bffc13e1548820eee93a3`
- Each `members` array element in the response should have a
`message` array with a single element being the simplified
representation of a LTI 1.3 launch payload, were that user to launch
the context referenced by `rlid`.
- The `message` entry should have two top level claims:
- `"https://purl.imsglobal.org/spec/lti/claim/message_type": "LtiResourceLinkRequest"`
- `"https://purl.imsglobal.org/spec/lti/claim/custom": <object>`
- The `custom` claim should include an entry for each `custom_fields`
key/value pair configured above, with supported entries being
correctly expanded and nonsense and unsupported entries being echoed
as-is.
- Repeat for a group context in the same course (still using the
course's LTI ID as the `rlid` value). Results should be the same,
though scoped to group membership.
Change-Id: If2591c62c494756d65774e3115abeca19935c988
Reviewed-on: https://gerrit.instructure.com/169090
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
- Pull custom LTI Advantage errors out of `AdvantageServices` and
`MembershipsProvider` into a new `Lti::Ims::AdvantageErrors`
module.
- Arrange those errors into a hierarchy and define default
`status_code` and `api_message` values for each. These can be
used for generic mapping to LTI Advantage API response messages.
(`api_message` is intended to contain a more terse, conservative
description of the problem, as compared to `message` which may
have additional debugging details suitable only for logging.)
- Also change casing on the default LTI Advantage access token to
'Missing access token' from 'Missing Access Token' to better match
convention on other errors.
- Stop trapping system failures during LTI Advantage access token
validation. These aren't actually 'handled errors', so log and
render these 'as usual'. I.e. client will get 500s instead of 401s.
Closes LTIA-39
Test Plan:
- This is primarily a technical refactoring so other than the error
message change noted above, testing would be focused regression
checks.
- Select several negative tests and verify that API error messages
and HTTP status code on the wire are still rendered as expected.
E.g.:
-- Check the response when the NRPS v2 API is invoked without an
access token. Error message should be 'Missing access token' and
status code should be 401.
-- Check the response when the NRPS v2 API is invoked with an
access token with an `exp` in the past. The error message should be
'Access token expired' and status code should be 401.
- For system failures during LTI Advantage access token validation,
try adding `raise StandardError` to the `begin` block in
`AdvantageServices#verify_access_token`. The error response message
will be 'An error occurred' and the status code will be 500.
Change-Id: I0a3eebd03d8ea5d5ff588f81be9187b323470c05
Reviewed-on: https://gerrit.instructure.com/168723
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Bill Smith <bsmith@instructure.com>
closes GRADE-1584
Test Plan
- Go to the account settings for feature flags.
- Verify that there is a setting for Final Grade Override in the
allowed state.
- Go to the course settings for feature flags.
- Verify that the course can toggle the feature flag for Final Grade
Override on or off.
Change-Id: I107671b4f458cb0f9b73db4180d3b101c55cf29e
Reviewed-on: https://gerrit.instructure.com/169729
Tested-by: Jenkins
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Keith Garner <kgarner@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Sidharth Oberoi <soberoi@instructure.com>
This does not remove the old dead code, just all the places
that checked to see if it was enabled.
closes PLAT-3752
Test Plan:
- Regression test on the developer keys page
- Also need to check that creating an oauth 2 token is
not broken (using client credentials)
Change-Id: I89983922a894ff7f20e86c034728d55284c8c668
Reviewed-on: https://gerrit.instructure.com/168271
Tested-by: Jenkins
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Weston Dransfield <wdransfield@instructure.com>
Product-Review: Marc Phillips <mphillips@instructure.com>
The aud value was returning an integer. The spec dictates
that the aud is either a string or an array.
fixes PLAT-3925
Test Plan:
- Install the lti 1.3 test tool in a beta sandbox
- Install the beta instance in the 1.3 test tool
- A launch of the test tool should work
Change-Id: Ibc6021a467a90983adcabdf7f79cc26e38493d52
Reviewed-on: https://gerrit.instructure.com/169837
Tested-by: Jenkins
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Weston Dransfield <wdransfield@instructure.com>
Product-Review: Marc Phillips <mphillips@instructure.com>
closes OUT-2558
test plan:
- create an assignment with aligned outcomes
- as a student, submit to the assignment
- as a teacher, assess the assignment's rubric
- as a student, confirm the results appear under sLMGB
- an an observer to the student, confirm the results
appear under sLMGB
Change-Id: I66118a01822be921e2fd44187143a12c7c13cd6c
Reviewed-on: https://gerrit.instructure.com/169132
Reviewed-by: Neil Gupta <ngupta@instructure.com>
Reviewed-by: Frank Murphy III <fmurphy@instructure.com>
Tested-by: Jenkins
QA-Review: Dariusz Dzien <ddzien@instructure.com>
Product-Review: Augusto Callejas <acallejas@instructure.com>
test plan:
* create a question bank
* create a question with an image
* import the bank question into a quiz
* edit the question in the quiz
* copy the quiz
* students in the copied course should be able
to see the image
closes #ADMIN-1568 #QO-426
Change-Id: Ic44b6cb14e9b74f2fbede4de905f0f11039dcaaa
Reviewed-on: https://gerrit.instructure.com/169731
Tested-by: Jenkins
QA-Review: Clay Diffrient <cdiffrient@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Reviewed-by: Brent Burgoyne <bburgoyne@instructure.com>
closes RECNVS-756
When files from inst-fs were downloaded, they were saved onto
the user's local machine with their name set to the value of
their `filename` column in the canvas database. We want it to
be set to `display_name` instead.
test plan:
* enable inst-fs on your local canvas instance
* upload a file with spaces in its name; e.g. "hello world.png"
* donwload the file to your computer
* verify that the downloaded file is named "hello world.png" instead of
"hello+world.png"
Change-Id: I03776fbd09cf53a574349663ac2fee5e3b4e0b8b
Reviewed-on: https://gerrit.instructure.com/168742
Tested-by: Jenkins
Reviewed-by: Michael Jasper <mjasper@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
Product-Review: Andrew Huff <ahuff@instructure.com>
test plan:
* create a quiz in a blueprint course
with various question types with answers
* copy to an associated course
* in the associated course, take the quiz
as a student
* edit the blueprint quiz description and re-sync
* the quiz statistics page for the student
should not be broken
closes #QO-416 #QO-197
Change-Id: Ib9a2eaf537e66c0729d52ea94c013f64426dfc6a
Reviewed-on: https://gerrit.instructure.com/168223
Tested-by: Jenkins
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
closes GRADE-1439
Test plan:
- Check that the feature flag is available in development mode and
behaves like a normal course feature flag
- (The feature flag won't actually do anything until a later commit)
Change-Id: Id4ce001aa98a5847ed1c61647eac2ed5764caf76
Reviewed-on: https://gerrit.instructure.com/169194
Tested-by: Jenkins
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: Adrian Packel <apackel@instructure.com>
Product-Review: Sidharth Oberoi <soberoi@instructure.com>
closes #CORE-1958
Change-Id: I7604a0cafbf9a7c3e102c1e0d572f33be20095ed
Reviewed-on: https://gerrit.instructure.com/169376
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Fixes OUT-2525
Test Plan:
- Import a CSV with a long (>255 character) vendor_guid.
- Verify that on import, you get an email with an error referencing
the specific line that failed.
Change-Id: I9b71e937666e404ac70d9ff0014d0f81ce541657
Reviewed-on: https://gerrit.instructure.com/168903
Tested-by: Jenkins
Reviewed-by: Neil Gupta <ngupta@instructure.com>
Reviewed-by: Augusto Callejas <acallejas@instructure.com>
QA-Review: Dariusz Dzien <ddzien@instructure.com>
Product-Review: Neil Gupta <ngupta@instructure.com>
Refs MBL-11444
Test Plan:
- Create an assignment
- Create an outcome in the course
- Add a rubric to the assignment
- Attach that outcome to the rubric you created for the assignment
and mark it as not contributing to the score
- hit /api/v1/courses/:courseID/assignment/:assignmentID
- Check that the rubric item for the outcome includes
ignore_for_scoring
Change-Id: I0090c280ccd0dc907731aa31569e59c5740393e7
Reviewed-on: https://gerrit.instructure.com/168999
Reviewed-by: Frank Murphy III <fmurphy@instructure.com>
Tested-by: Jenkins
QA-Review: Nate Armstrong <narmstrong@instructure.com>
Product-Review: Matt Sessions <msessions@instructure.com>
refs AMS-1444
test plan:
- sortable name is available to LTI tools as a
variable expansion
Change-Id: I21b94845a34ff668caa37619de2d5eede4fd0099
Reviewed-on: https://gerrit.instructure.com/167591
Tested-by: Jenkins
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Jared Crystal <jcrystal@instructure.com>
Product-Review: Jesse Poulos <jpoulos@instructure.com>
Closes PLAT-3863
Test Plan:
- Install an LTI 1.3 tool in an account
- Navigate to app cetner of a course in
that account
- Verify that the toggle for the tool installed
in the account is on and disabled
- Turn off the tool in the account
- Verify the toggle is off and enabled in the
course
- Verify the toggle may be turned on and off in
the course
- Enable the tool in the course and the account
- Verify the toggle is enabled and on in both
the account and the course
- Disable the tool in the coures and refresh
the page
- Verify the toggle is on and disabled
Change-Id: Id23c301e874fcf008f7c7d70b5799e2178f8144c
Reviewed-on: https://gerrit.instructure.com/168084
Tested-by: Jenkins
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Marc Phillips <mphillips@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>
Closes ADMIN-1543
To aid in auditing & reducing support week tickets.
Test Plan:
* APIs for `communication_channels`, `groups`,
`pseudonyms`, `roles`, `sections`, and
`users` should return records with the
`created_at` attribute included in the response.
Change-Id: I54776927d439526fa5a16b4249ec847d87bc33f4
Reviewed-on: https://gerrit.instructure.com/168107
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
Product-Review: Dan Minkevitch <dan@instructure.com>
also send the assignment_group_id with the live event because
why not
closes #CNVS-43951
Change-Id: I82b2e8064abc5155b4ca57f3acec17a8849f1a44
Reviewed-on: https://gerrit.instructure.com/167196
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
test plan:
* have an account admin with "SIS data - manage" enabled
but not "account-level settings - manage"
* they should be able to remove enrollments created
through sis imports
closes #ADMIN-1536
Change-Id: I99cb6a0ef3d31fe596261500dc8e4f69d589deec
Reviewed-on: https://gerrit.instructure.com/168005
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jon Willesen <jonw+gerrit@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
fixes OUT-2542
test plan:
1. Create an outcome CSV where calculation_method is omitted
2. Import into a course or account
3. Ensure outcome is created with a "decaying_average" calculation method
Change-Id: I77af71ad0c7b91628e85a0cfa40eb1bb4c383614
Reviewed-on: https://gerrit.instructure.com/167131
Tested-by: Jenkins
Reviewed-by: Augusto Callejas <acallejas@instructure.com>
QA-Review: Brian Watson <bwatson@instructure.com>
Product-Review: Neil Gupta <ngupta@instructure.com>
test plan:
* have an assignment in a course with moderated
and anonymous grading settings checked (may have to enable
features for these to be available)
* copy it into another course (with same features enabled)
* the copied assignment should have the same settings
closes #ADMIN-1489
Change-Id: I2647ce11da3a4cfb3ea370887248ef4b8d79a53a
Reviewed-on: https://gerrit.instructure.com/166114
Tested-by: Jenkins
QA-Review: Jon Willesen <jonw+gerrit@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Reviewed-by: Carl Kibler <ckibler@instructure.com>
Reviewed-by: Mysti Sadler <mysti@instructure.com>
- Effectively adds rendering of the `$Person.sourcedId` custom param
to NRPS v2 responses.
- Required changes to user preloading logic to allow preloading of
pseudonyms even if the current user is not an admin.
- Now memoizes context, enrollment, and user 'decorator' instances,
as well as per-member mapped LTI roles.
- Copy/paste in serialization tests and matchers caused problems
during implementation so those have been DRYed out.
- Some example groups also renamed to remove single quotes which made
them difficult to run in isolation.
Closes LTIA-34
Test Plan:
- Assign a SIS ID to one or more users actively enrolled in a
Course and Group visible to a LTI 1.3 Tool.
- Verify, for both Course and Group contexts, that the NRPS
memberships describing those SIS ID'd users include those SIS
IDs in the `lis_person_sourcedid` field.
Change-Id: I6316f45fe7aa476c5ccd884fa0236dc684241e3b
Reviewed-on: https://gerrit.instructure.com/167720
Tested-by: Jenkins
QA-Review: Samuel Barney <sbarney@instructure.com>
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
closes ADMIN-1505, ADMIN-1513
test plan:
- turn on the assignments 2 feature flag on the account
- navigate to an assignment as a student. you should see an "Assignments
2" button in the upper right.
- clicking on the button should take you to the same page with an
additional url parameter.
- The page will just indicate that it's the assignments 2 student view
- doing the above as a teacher should do the same thing and also show
some of the assignment data.
- with the feature flag off, you shouldn't see the "Assignments 2"
button. It should also ignore the url parameter if you add it
manually.
Change-Id: Icaf108e3baaeea6f85464498ecd02f08c70a8ba2
Reviewed-on: https://gerrit.instructure.com/167226
Reviewed-by: Carl Kibler <ckibler@instructure.com>
Tested-by: Jenkins
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Carl Kibler <ckibler@instructure.com>
- Per recent NRPS vs spec change, the `service_version` LTI 1.3
launch sub-claim is renamed to `service_versions` and its type
changed from a string to an array of strings. (The expressed
version itself is still the same, i.e. it just appears as
["2.0"] instead of an unwrapped "2.0".)
Closes LTIA-35 WIP
Test Plan:
- Verify LTI 1.3 launches to the IMS Reference Implementation include
a https://purl.imsglobal.org/spec/lti-nrps/claim/namesroleservice
claim with a `"service_versions": ["2.0"]` sub-claim
Change-Id: I53801d633166a07c571e106cc60256729059fdcf
Reviewed-on: https://gerrit.instructure.com/167714
QA-Review: Samuel Barney <sbarney@instructure.com>
Tested-by: Jenkins
Product-Review: Karl Lloyd <karl@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
test plan:
- Enable Mastery Paths in the course or account
- Create a course with a Module and add a Page and an Assignment as module items
- Enable MP for Page
- Add Page as a conditional content to an MP assignment
- Login as a student to check that the page is not visible
- Copy course into a new course shell
- Login as a student in the child course
- The page should not be visible before it is unlocked
fixes ADMIN-1461
Change-Id: I4c0d02b33a78862edcb36914dfe531b24caad08f
Reviewed-on: https://gerrit.instructure.com/167010
Tested-by: Jenkins
QA-Review: Mysti Sadler <mysti@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Reviewed-by: Neil Gupta <ngupta@instructure.com>
Reviewed-by: James Williams <jamesw@instructure.com>
- Remove `Lti::Ims::NamesAndRolesController`'s inclusion of
`Lti::Ims::AccessTokenHelper` in favor of a recently
introduced concern: `Lti::Ims::Concerns::AdvantageServices`. This
includes a reimplementation of `Lti::Oauth2::AccessToken` as
`Lti::Ims::Concerns::AdvantageServices::AccessToken`, which has
many of the same capabilities, but none of the LTI 2 baggage.
- Most low-level JWT validation is delegated to `Canvas::Security`
and `Canvas::Security::JwtValidator`. The former provides symmetry
with `Canvas::Oauth::ClientCredentialsProvider` eso w/r/t using
Canvas encryption keys as signing keys. The latter also aligns well
with that same provider, though its `jti` handling may need to
be relaxed in the future.
- ***This commit intentionally avoids extensive modifications to
`Lti::Ims::Concerns::GradebookServices` and related controllers. The
expectation is that `GradebookServices` should just include
`AdvantageServices` and could thus eliminate many of its methods.
But this has been deferred, partly to keep commit size down and
partly to avoid unilateral contract changes to dependent controllers.
- Most of this commit, then, is really just suffling tests around
from `names_and_roles_controller_spec.rb` into
`advantage_services_shared_examples.rb` so other LTI Advantage
controllers can integrate the same validations.
`advantage_services_shared_context.rb` expresses the data setup
needs of `advantage_services_shared_examples.rb`, along with a few
helper methods.
- Eliminates custom tool/account chain walking in
`NamesAndRolesController` in favor of
`ContextExternalTool#all_tools_for`. Latter's use is in
`AdvantageServices`, so should be readily reusable from other
LTI Advantage controllers.
Closes LTIA-28
Test Plan
* This is largely a technical refactoring so testing should focus
primarily on regression checks against NRPS v2 callbacks, especially
to ensure they still require valid access tokens issues via
client credentials.
* Functional differences are limited to newly added validation of
JWT claims embedded in those access tokens. These are difficult to
test since Canvas is responsible for generating the tokens. So
either Canvas code needs to be temporarily modified to generate
variously incorrect access tokens, or an out of band tool needs to
be implemented to generate such tokens. Claims to be validated:
1. `sub` - must be an active developer key's global ID, e.g.
"10000000000020"
2. `aud` - must be the OAuth2 access token issuance URL, i.e.
`<host>/login/oauth2/token`
3. `iat` - must be in the past
4. `exp` - must be in the future
5. `jti` - must be present
6. `scopes` - must include
https://purl.imsglobal.org/spec/lti-nrps/scope/contextmembership.readonly
7. <signature> - Must use `HS256` algorithm and use the Canvas
encryption key as the signing secret.
Change-Id: I14b431e11180f06e9edd4f5dfd3b04ed931ea73e
Reviewed-on: https://gerrit.instructure.com/167361
Tested-by: Jenkins
QA-Review: Samuel Barney <sbarney@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
Product-Review: Karl Lloyd <karl@instructure.com>
- NRPS v2 invocations referencing a `Course` Context now attempt to
resolve a `ContextExternalTool` (CET) given the JWT `AccessToken`
attached to the request. In order to return memberships, that CET
must be active and must either be bound directly to the `Course` or
to an `Account` in the `Course`'s' `Account` chain.
- `AccessToken` must be associated with an active `DeveloperKey`
(DK), and the search for the "operative" CET for the current request
is executed against that DK's list of active CETs.
- `Course`-level CETs are preferred, followed by `Account`-level
CETs.
- LTI 1.3/Advantage features must be turned on at the CET and root
`Account` levels.
- The `AccessToken`'s'JWT signature and security claims are not
themselves validated... that comes later.
- `Group` Context support also comes later.
Closes LTIA-26
Test Plan:
- Via Rails console create a `DeveloperKey` associated with the
public key of a Tool configured in the IMS LTI 1.3/Advantage
Reference Implementation (RI) and the root `Account` in your env
- Via Rails console, create a LTI 1.3-enabled
`ContextExternalTool` with a `course_navigation` placement and
linked to the just-created `DeveloperKey` and its `Account`
- For a `Course` owned by this `Account`, verify that direct
invocations of the NRPS v2 API.
(`GET /api/lti/courses/:course_id/names_and_roles`) fail with
a 401 and a message complaining about a missing access token.
- Navigate to the `Course` and click the newly created nav link,
which should successfully launch the RI.
- Click the 'Request Names and Roles' link in the RI. Verify
course is reported in the NRPS v2 format.
- Deactivate the `DeveloperKey`. Click 'Request Names and Roles'
link in the RI. Verify a (non-descript) on-screen error message.
- Re-enable the `DeveloperKey` and re-verify the same behavior
for a `Course` associated with a sub-`Account`.
- Delete the CET, verify that NRPS v2 invocations from the RI
fail.
- Via Rails console, create a new CET linked to the same
`DeveloperKey`, but now attached to the sub-`Account` `Course`.
- Re-verify NRPS v2 invocation from the RI.
- *Consult JIRA for full acceptance criteria.
Change-Id: Ie9625ea8d6ce5e6f59e3c7ce1d10d0a47291afa4
Reviewed-on: https://gerrit.instructure.com/167183
Tested-by: Jenkins
QA-Review: Samuel Barney <sbarney@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
Product-Review: Karl Lloyd <karl@instructure.com>
fixes CORE-119
Test plan
- Load up the rich content editor
- Ensure you can still select the same
files and embed them into the editor that
you could previously
- Ensure you can see paginated items in folders
- Ensure you can upload a file to folders that
you have permission to upload to
- Ensure you can't access locked folders
- Ensure UI is accessible
Change-Id: Ief8c731e04b1ee2faf07a052720d5d63aab03118
Reviewed-on: https://gerrit.instructure.com/120527
Tested-by: Jenkins
Reviewed-by: Carl Kibler <ckibler@instructure.com>
QA-Review: Carl Kibler <ckibler@instructure.com>
Product-Review: Matthew Goodwin <mattg@instructure.com>
g/167863 will keep the default from getting stored in multicache
but it can still collide in the in-process cache
closes #CORE-1977
Change-Id: Ic29292ad1da69af9684862a934698ed73d001937
Reviewed-on: https://gerrit.instructure.com/167920
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
When importing a gradebook CSV and and analyzing what changes have been
made, map changes to custom columns properly and do not attempt to
include deleted or hidden columns.
fixes GRADE-1616
Test plan:
- In a course with some students, set up four custom columns
(referred to as C1 through C4); for now, don't change any
settings on them
- In new Gradebook, set some values for the columns
- Adjust them as follows:
- C1: deleted
- C2: hidden
- C3: read-only
- C4: [leave as is]
- Export a CSV
- The CSV should contain C3 and C4 but not the other two
columns (this behavior has not changed)
- Twiddle some assignment scores and custom-column values in the CSV
- Import the edited CSV and check the following on the proposed
changes screen:
- Changes to C4 are shown as expected
- Any changes to assignment scores are shown as expected
- Changes to C3 are *not* shown, and the warning message about
preventing changing read-only columns is shown at the bottom
- Otherwise, everything is as you'd expect: the importer isn't
attempting to replace a custom column with the section name
or perpetrate any similar skulduggery
Change-Id: Ib1ebcec6820120288e67c1652cb5496a8b8240a7
Reviewed-on: https://gerrit.instructure.com/167149
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
Tested-by: Jenkins
This commit renames the feature flag to HTML5 Media Recorder in
RCE; it previously included Arc because it was built by the Arc
team but does not directly relate to the Arc product. Also
updated the flag to allow visibility in beta.
REFS: COMMS-1486
Test plan:
- Log in to Canvas as an admin
- Open Account Settings
- In Account feature options, make sure you can view the
HTML5 Media Recorder in RCE feature option with the
development label and that the feature can be toggled on
Change-Id: I47d40263ba3dd91ad0bcca9b14ca7ba83409e323
Reviewed-on: https://gerrit.instructure.com/167716
Tested-by: Jenkins
Reviewed-by: Landon Gilbert-Bland <lbland@instructure.com>
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: KC Naegle <knaegle@instructure.com>
Closes PLAT-3748
Test Plan:
- Create an LTI key with customizations
- Verify the disabled placements are persisted
to the database
- Verify the scopes are persited to the database
- Verify the LTI key flow works as expected
Change-Id: I97217b09cfb10b3732d6ded478b95a8999c6b4e5
Reviewed-on: https://gerrit.instructure.com/166691
Tested-by: Jenkins
Product-Review: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Marc Phillips <mphillips@instructure.com>
refs PLAT-3783
Test Plan:
- Add a lti 1.3 dev key
- Go to the LTI 1.3 app index and toggle
on the tool
- Go to to the installed app index page and note
that it is installed
Change-Id: I8a86ea7eefa64c07dd57db6ab2c2bd7a2e1ecf38
Reviewed-on: https://gerrit.instructure.com/167511
Tested-by: Jenkins
Reviewed-by: Nathan Mills <nathanm@instructure.com>
QA-Review: Marc Phillips <mphillips@instructure.com>
Product-Review: Marc Phillips <mphillips@instructure.com>
and make the other helper use class methods since they're
not even referencing 'self' anyway
should (in theory) work better with the autoloading hacks
inside selenimum
[ci no-cached-dist]
Change-Id: I774f07eb7372eafe5f4d4221a2f15d018d8d13ee
Reviewed-on: https://gerrit.instructure.com/167471
Tested-by: Jenkins
Reviewed-by: James Butters <jbutters@instructure.com>
Reviewed-by: Robert Lamb <rlamb@instructure.com>
QA-Review: Robert Lamb <rlamb@instructure.com>
Product-Review: Robert Lamb <rlamb@instructure.com>
hopefully this helps with the master selenium build
[ci no-cached-dist]
Change-Id: I0a4fde579888edd6bef6d85b5fcbe851bd8bd791
Reviewed-on: https://gerrit.instructure.com/167372
Tested-by: Jenkins
Reviewed-by: Mysti Sadler <mysti@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Change-Id: I11f0e7eeb75504bacb4afd8ab41e58b4e49d2695
Reviewed-on: https://gerrit.instructure.com/167265
Tested-by: Jenkins
Reviewed-by: Tucker Mcknight <tmcknight@instructure.com>
Reviewed-by: Carl Kibler <ckibler@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
closes ADMIN-1111
test plan:
- create calendar events courses a student is enrolled in
- create calendar events on the student's calendar
- create calendar events in a group a student is part of
- go to planner and click on these calendar event titles
- a dialog displaying the calendar information should appear
- in the dialog, clicking the calendar event title should take you to
that event in the calendar
- the dialog should display the proper dates, calendar, location, and
details
Change-Id: Iabb3649c32bdef551dc73740dd2191e301b0de34
Reviewed-on: https://gerrit.instructure.com/166148
Reviewed-by: Carl Kibler <ckibler@instructure.com>
QA-Review: Carl Kibler <ckibler@instructure.com>
Tested-by: Jenkins
Product-Review: Jon Willesen <jonw+gerrit@instructure.com>
In the app center, lists on a tab all the 1.3 tools
avaialable and installed.
closes PLAT-3782
Test Plan:
- Load the appcenter, click the 1.3 tools tab, things
should load
Change-Id: Ib963d6baabdbb9f563c7983f30948990763e428f
Reviewed-on: https://gerrit.instructure.com/166936
Reviewed-by: Nathan Mills <nathanm@instructure.com>
QA-Review: Nathan Mills <nathanm@instructure.com>
Tested-by: Jenkins
Product-Review: Marc Phillips <mphillips@instructure.com>
closes ADMIN-1454
There were 2 issues
1. @columns.each do |col| -> @columns.flatten.each do |col|
this change sees that all the interesting values in the collection's
items are considered in the merge. It also deals with the fact that
@columns looks different when the Plannable::Bookmark is initialized
from users_controller vs. planner_controller.
2. fix a bug in the processing of complex bookmarks like
{submission: {assignment: :due_at}}, :id)
test plan:
- have peer reviews plus other items in a course with due times
on the same day that cause them to interleave with one another
- load the planner
> expect the sorted order of the items to be correct
- you need to look at the api response, because the UI will
correctly sort them for display. This is still a problem because
the code that figures out if planner has found a complete day's
worth of items relies on the items api returning the data sorted
Change-Id: I2753ab3b3425b779dc6bfbea27fe70311e9e7400
Reviewed-on: https://gerrit.instructure.com/165674
QA-Review: Carl Kibler <ckibler@instructure.com>
Tested-by: Jenkins
Product-Review: Carl Kibler <ckibler@instructure.com>
Reviewed-by: Mysti Sadler <mysti@instructure.com>
Reviewed-by: Carl Kibler <ckibler@instructure.com>
The submissions api was not including the submission id when creating
the preview url for docviewer. This brings that up to date with
everywhere else that generates the preview url.
Test Plan
- Create an assignment.
- Have students upload files to that assignment.
- Go to the url for the submissions
`api/v1/courses/:course_id/assignments/:assignment_id/submissions`
- Grab one of the preview_urls for the attachments you just created.
- Launch that preview_url. (you may have to change the "\u0026" back
to an ampersand)
- The document should preview.
Change-Id: Ic57bce12274c34ce0b783ad328ddb209fc43d7d2
Reviewed-on: https://gerrit.instructure.com/167247
Reviewed-by: Keith Garner <kgarner@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>
Tested-by: Jenkins
QA-Review: Gary Mei <gmei@instructure.com>
closes PLAT-3818
Test Plan:
Use of the content item request now includes the oauth_callback
- check that it is the case
Change-Id: I035587458ed75b7dc1c6b2497d60113071e2179e
Reviewed-on: https://gerrit.instructure.com/167206
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Weston Dransfield <wdransfield@instructure.com>
Tested-by: Jenkins
Product-Review: Marc Phillips <mphillips@instructure.com>
closes #CORE-1954
Change-Id: I1546a446230a8e4ab4b4502a4a6ab7d5b778aef6
Reviewed-on: https://gerrit.instructure.com/166568
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
test plan:
* have a user with conversations
* merge the user into one on another shard
* the conversations should remain in the inbox
closes #CORE-1895
Change-Id: I4792c98e3ead122e954ba2b3e0335dcdb99dae69
Reviewed-on: https://gerrit.instructure.com/165371
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Rob Orton