Fixes: CNVS-38111
Test Plan:
* Disable permission caching
`Setting.set('permissions_cache_enabled', 'false')`
* Start up your web server and visit any page
* You should not see any Redis requests for permissions in your
development logs
Change-Id: Ib91a0b7fc9ca8db121f942984e26f44d19d2400b
Reviewed-on: https://gerrit.instructure.com/118850
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
Tested-by: Jenkins
Fixes: CNVS-38113
By intermediate permissions we mean permissions used to determine if the
directly queried one matches, for example in `app/models/account.rb` we
have a :create_tool_manually permission that checks if the user can
:lti_add_edit. Before this patch both permissions would get written to
the Rails cache, with this patch and the flag set to false :lti_add_edit
won't get written but the top level :create_tool_manually will.
Test Plan:
* Set the new flag to false:
`Setting.set('permissions_cache_intermediate', 'false')`
* Start tailing the development log grepping for setting permissions
`tail -f log/development.log | grep 'setex permissions'`
* Create an LTI1 tool registration using the UI, you should see a
permission named `create_tool_manually` get cached while one named
`lti_add_edit` should not.
Change-Id: I88ba5cc03d1a13030f53194554d6eee3c287c10d
Reviewed-on: https://gerrit.instructure.com/118656
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
Fixes: CNVS-38114
We were just blindly caching everything that could be cached when
calculating permissions, it turns out these aren't actually used very
frequently so we're going to try turning caching of them off.
Test Plan:
* With Redis backed caching enabled
* Tail your development log grepping for setting permission caches:
`tail -f log/development.log | grep 'setex permissions'`
* Visit your main account settings page: `/accounts/1/settings`
* Note that there are many manage, update, and delete permissions
written to redis.
* In a console disable related permissions caching:
`Setting.set('permissions_cache_related', 'false')`
* Reload configuration values for your web process (SIGHUP or restart)
* Reload the account settings page, note that there are only redis
writes for read permissions now instead of many others.
Change-Id: Ic5a73a5124b42405fed79bede37aff70322d0c54
Reviewed-on: https://gerrit.instructure.com/118480
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
Fixes: CNVS-38110
This is the first of a few knobs we'll be adding to give the ability to
tune rails cache traffic coming from adheres_to_policy. We're adding
these because it constitutes a very large portion of the current redis
traffic with a fair bit of that traffic writing keys that never end up
actually being read based on some recent log analysis.
Test Plan:
* Tail your development log grepping for `manage_courses`
`tail -f log/development.log | grep manage_courses`
* Visit an account's courses page: /accounts/self
* The tailed logs should show a Redis get and setex for a permissions
key on accounts.manage_courses
* In a console run
`Setting.set('permissions_cache_blacklist', 'account.manage_courses')`
* Restart your web server
* Visit the account courses page
* The logs should no longer include the Redis traffic
Change-Id: I566f58e70687c4fcffe7369b94f38e62f1318ce8
Reviewed-on: https://gerrit.instructure.com/118378
Tested-by: Jenkins
Reviewed-by: Brent Burgoyne <bburgoyne@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
We weren't tracking this so we couldn't get an idea of how much our
caching was saving us.
Change-Id: I41cc595a34e6cf709a945420a5e75efdcdcbb57d
Reviewed-on: https://gerrit.instructure.com/109494
Tested-by: Jenkins
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
test plan:
see coverage in previous rspec/selenium builds, same numbers as before
Change-Id: I331bf5102914da00a5d350f32b74b4bc9d49c5f8
Reviewed-on: https://gerrit.instructure.com/106895
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
closes CNVS-35293
test plan:
1. Create a moderated assignment.
2. As a user who can edit grades, review and give a provisional grade.
3. As a user with the "Moderate grades" and "Edit grades" and "View all
grades" permissions, visit that assignment's moderation page. Verify
there is a "Post" button and students are listed. Verify you can
post grades.
4. As a user with the "Moderate grades" and "Edit grades" permissions
and without the "View all grades" permission, visit that
assignment's moderation page. Verify there is a "Post" button and
students are listed. Verify you can post grades.
5. As a user with the "Moderate grades" and "View all grades"
permissions and without the "Edit grades" permission, visit that
assignment's moderation page. Verify there is no "Post" button and
students are listed.
6. As a user with the "Moderate grades" permission and without the
"Edit grades" and "View all grades" permissions, visit that
assignment's moderation page. Verify there is no "Post" button and
no students are listed. Also verify there is no checkbox next to the
"Student" header.
7. As a user with the "Moderate grades" and "View all grades"
permissions and without the "Edit Grades" permission, try to publish
a moderated assignment via the API (api/v1/courses/:course_id/
assignments/:id/provisional_grades/publish). Verify the response
comes back as unauthorized.
Change-Id: Iffacc6c4eb314f7d8730786d627dffaf5623bc4a
Reviewed-on: https://gerrit.instructure.com/104659
Reviewed-by: Derek Bender <djbender@instructure.com>
Tested-by: Jenkins
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
fixes CNVS-34170
test plan:
* configure statsd.yml
* use the app
* observe data sent to statsd, and logged, re: permissions
Change-Id: I1ff35f4679fffcac49774e1bad0cf2540b7f17b8
Reviewed-on: https://gerrit.instructure.com/99727
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
as of this commit, all canvas gems should be on rspec 3.5, and pass
without deprecation warnings.
closes CNVS-34040
test plan: specs should pass without deprecation warnings
Change-Id: I556b1a4a5aeb791c6ddd50ee35b51c513e025019
Reviewed-on: https://gerrit.instructure.com/98414
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
Tested-by: Jenkins
test plan:
* create a course with 'institution' visibility
(i.e public to authenticated users)
* visit the course syllabus as a random user
* visit the course home page
* it should not show an unauthorized message
closes #CNVS-32584
Change-Id: I5a85ba83334fb5099af8f3002197ae74af7a1573
Reviewed-on: https://gerrit.instructure.com/93215
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: David Tan <dtan@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Fixes CNVS-29511
Sometimes when posting a comment, the incorrect grade was being
applied. This commit decouples the 2 features and DRY's up the code to
address this issue.
Test plan:
* As a teacher, setup an assignment with a letter grade scheme.
* As a student, submit an assignment.
* As a teacher, go to the submission detail view.
* Start typing a comment but don't submit it.
* Enter a grade of 95. The grade should submit and display as "A", but
the comment should not be submitted. Refresh page to confirm.
* Post a comment. It should post but the grade should still be 95.
Refresh the page to confirm.
Change-Id: I1eb7478e0c693ee28a58578e45dd7369b24baa05
Reviewed-on: https://gerrit.instructure.com/81568
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Tested-by: Jenkins
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: Christi Wruck
rely on test_all_gems.sh to output header and trailer,
and use `set -e` in each test.sh to simplify passing
through errors
Change-Id: I3ba724ad2539ddfe31195394c43f646acfc73920
Reviewed-on: https://gerrit.instructure.com/70469
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
should improve performance in requests when permissions
are checked for users that don't have them
(e.g. student grade summary page)
refs #CNVS-21317
Change-Id: I349a02664bcc81da1808e76bbc023f5d743f90a4
Reviewed-on: https://gerrit.instructure.com/56846
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Fixes CNVS-20926
Test plan:
- Regression test several pieces of Canvas that use adheres_to_policy
- This is almost everything, including calendar, discussions,
accounts, assignments, groups, courses, sections, enrollments,
submissions, quizzes, and wiki pages.
- You'll specifically want to test every situation where a 401
Unauthorized should be sent back to a user or where a user should
otherwise not be permitted to view a certain thing or perform a
certain action and ensure that they continue to enforce the same
restrictions that they used to.
- You'll also want to test situations where a user should be allowed
to view a certain thing or perform a certain action and ensure
that they are still able to.
Change-Id: I643e6462584213adf2d415057fa59277a8627043
Reviewed-on: https://gerrit.instructure.com/55663
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
Tested-by: Jenkins
QA-Review: Steven Shepherd <sshepherd@instructure.com>
Product-Review: Alex Boyd <aboyd@instructure.com>
* add test runner for broadcast_policy and fix a couple of tests that
had been broken by accident
* add test runner for canvas_quiz_statistics and add a reference to
iconv to it's Gemfile to allow tests to run in ruby 2
* add a test runner for canvas_sanitize
* remove leftover cruft from running gem builds in both rails 2 and 3
Change-Id: Iab2a0986f277a82c096e1fff2dab1d3a55b91733
Reviewed-on: https://gerrit.instructure.com/54518
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Change-Id: Ib7db93b7ce5c877d2c0dcf24ee0d748f11ec58cb
Reviewed-on: https://gerrit.instructure.com/44806
Reviewed-by: Adam Ard <aard@instructure.com>
Product-Review: Adam Ard <aard@instructure.com>
QA-Review: Adam Ard <aard@instructure.com>
Tested-by: Adam Ard <aard@instructure.com>
fixes CNVS-13711
test-plan:
- regression tests of:
* permissions of admin with pending invitation to a course
* user that typically would not have access to a portfolio, but
visited the portfolio with a verifier string earlier in the session
(but has no verifier string now)
* access to attachments on the safe file domain
Change-Id: Ie321c77655e6ba5e87fd35a079086a48608f5d0e
Reviewed-on: https://gerrit.instructure.com/38029
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Reviewed-by: Nick Cloward <ncloward@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
fixes: CNVS-13917
Moves the initializer from active record base to an alias method chain
on AdheresToPolicy::Instance methods. The way its including the
instance methods did not allow it to override the
permission_cache_key_for method.
Test Case:
- Flush cache
- Open a Course
- Look at the cache keys and all the ids should be global ids
Change-Id: Iac6a5ed95a800c27bd53fcb757cabd11976aef21
Reviewed-on: https://gerrit.instructure.com/37226
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Nick Cloward <ncloward@instructure.com>
fixes: CNVS-11197
Cleans and simplifies the code to remove the returns and nexts.
The block was short cuirciting the cache methods withe the returns.
Since the caching for permissions is more aggressive a few places in
specs need to clear the cache.
Test Plan:
- Clear cache
- Open a course page
- Check cache to make sure there are permission keys stored for that
course.
Change-Id: Ib7f747242bfb4394a73876377f4b6ba632f8b728
Reviewed-on: https://gerrit.instructure.com/37100
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Nick Cloward <ncloward@instructure.com>
refs CNVS-11425
* be more strict about the DSL
* store policies so that we can avoid linear searches for
an applicable condition block
Change-Id: I68f6414b396e1cb16d744d0719cdd6aa86085784
Reviewed-on: https://gerrit.instructure.com/36222
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Nick Cloward <ncloward@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
fixes: CNVS-11425
This is a performance refactor of the permissions. The
biggest change is the caching. The cache key is now based
on the right so each right will be cached by itself. The
goal is to reduce places where we implement caching for
permissions and let adheres_to_policy handle it.
Another commit is coming to clean up calls to the new
methods created here. g/34280
Test Plan:
- Make sure permissions all work still.
- Make masquerading still works with permissions.
- Make sure switing views such as "student view" for a
course.
Change-Id: I4a30b0aba394cea24c3b60167fc1369a2584f5a4
Reviewed-on: https://gerrit.instructure.com/34278
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
they're likely already installed, and it will save a ton of trips
to rubygems.org
Change-Id: I9ccf2194619a6e8f97d7f21b4e232dac7ff20d3c
Reviewed-on: https://gerrit.instructure.com/35694
Reviewed-by: Bryan Madsen <bryan@instructure.com>
QA-Review: Bryan Madsen <bryan@instructure.com>
Product-Review: Bryan Madsen <bryan@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Change-Id: I0dad5345aae75e552af97f5b54ded10bbfebbe37
Reviewed-on: https://gerrit.instructure.com/33925
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Mike Nomitch <mnomitch@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
fixes: CNVS-12259
test plan:
- run unit tests, run bundle, verify app starts up without rails version
conflicts
Change-Id: Iddd6167bbb8a4cc61ca015fc979236659e24da73
Reviewed-on: https://gerrit.instructure.com/33126
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
also move the RAILS3 sentinel into the config folder, for easier
deploys, and normalize how sub-gems decide on rails 3
Change-Id: I39fda434bb3d94cf72e66774b6b9d11c968a3d5a
Reviewed-on: https://gerrit.instructure.com/31632
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
fixes CNVS-11072
test plan: in a canvas repository without a .git directory (like a deploy) call
a bundle command (like script/console). it should not give you warnings about
missing .git directories
Change-Id: I2daa1371d97c94f7c3db81d3fd2bad387fcabaf8
Reviewed-on: https://gerrit.instructure.com/29947
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Stephan Hagemann <stephan@pivotallabs.com>
Product-Review: Stephan Hagemann <stephan@pivotallabs.com>
QA-Review: Stephan Hagemann <stephan@pivotallabs.com>
* clean up sqlite db after tests run in stringex gem
* test Redcloth correctly in stringex gem
* move database cleanup to test runner in stringex
* keep rails2 Gemfile.lock after running gem test runners
closes CNVS-11020
test plan:
- stringex tests should still pass in rails 2.3 and 3
Change-Id: Idde773ddf3c8158bb0040a2eae9d4ecc8c8c2d15
Reviewed-on: https://gerrit.instructure.com/29781
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Liz Abinante <labinante@instructure.com>
Product-Review: Bryan Madsen <bryan@instructure.com>
QA-Review: Bryan Madsen <bryan@instructure.com>
Reviewed-by: Raphael Weiner <rweiner@pivotallabs.com>
Reviewed-by: Stephan Hagemann <stephan@pivotallabs.com>
Rob Orton