This reverts commit b9d45dbc1d.
Reason for revert: breaks prod database.yml because of the to_prepare
Change-Id: I3db4c5cbad77168ede014f9924049f458f05cb3f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/284097
Reviewed-by: Andrea Cirulli <andrea.cirulli@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Andrea Cirulli <andrea.cirulli@instructure.com>
Product-Review: Andrea Cirulli <andrea.cirulli@instructure.com>
qti_migration_tool decided that object ids need to start with a
letter or underscore and are not allowed to start with a digit.
this isn't consistent with non-QTI packages and it can cause
imported quizzes to lose their Canvas metadata (including the
quiz description and due dates)
the fix is in qti_migration_tool but some Canvas specs need to
be adjusted to match the new behavior
test plan: specs
flag=none
refs LS-2980
[pin-commit-qti_migration_tool=fed7c13a5b0cd54a2d43a1085e365b93b9243ac5]
Change-Id: I902c265f200637dfeb45961d94b5e2102f733c6b
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/284571
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Eric Saupe <eric.saupe@instructure.com>
QA-Review: Eric Saupe <eric.saupe@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
fixes LS-2897
flag=none
test plan:
- Have the INST-FS plugin ON
- Have a canvas export package with at least one file
- Import the export package into a fresh course
- Import the same export package again.
- Check that the file wasn't duplicated
- (You could, after the first import, alter the
attachment md5 column and put it's sha512 there,
then theorically you'd be testing this, but Ideally
you want instfs ON)
Change-Id: I0de79032d2ccd64e51cc09dea61b410aad43756f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282779
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Robin Kuss <rkuss@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Luis Oliveira <luis.oliveira@instructure.com>
why:
* to see if this is a problem, since events sent out without a context
will pose a problem to live events consumers
refs INTEROP-7187
flag=none
test plan:
* follow the instructions in doc/live_events.md to set up live events
and print them to stdout logs
* follow the instructions in doc/lti_manual/11_testing.md under
`Stubbing Statsd Calls` to monkey-patch Statsd for easy metric viewing
* (a good place to put that is in the ApplicationController)
* comment out the call to `LiveEvents.set_context` in the
ApplicationController
* trigger any live event - navigating through course pages should be
enough, though creating an assignment is always a good one
* for every event triggered there should be a Statsd call for
`missing_context`
* add back the call to `set_context`
* trigger more live events
* the Statsd calls should not continue
Change-Id: Ib87952e208885ef15b00561731769f808cdcd066
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/284053
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Tucker Mcknight <tmcknight@instructure.com>
Reviewed-by: Evan Battaglia <ebattaglia@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Product-Review: Xander Moffatt <xmoffatt@instructure.com>
test plan:
- create a migration and stage it for commit (no need to
actually commit or run it) that creates a new table
- start without having a root_account_id column or a
replica identity index
- run script/rlint and it should complain that you need
a root_account reference and a replica identity index
and provide example lines for both
- if you create the root_account_id column in any of the
following less-desired ways, it should point out your
error and give you an example of what the line should
look like:
- `t.bigint :root_account_id` instead of
't.references :root_account`
- missing `{ foreign_key: { to_table: :accounts} }`
- missing `index` option or `index` option other than
`index: false` (the replica identity index is
generally sufficient)
- missing `null: false`
(if multiple deficiencies exist, it'll only log one;
it's not possible for one cop to log multiple errors
per line, but it will give an example that checks all
the boxes so following that should avoid back-and-forth)
- create more than one table in the migration and verify
that it detects which table(s) is/are missing the
`add_replica_identity` call
flag=none
closes DE-951
Change-Id: I582608c8792767c2b51f69d90afd777e3c7a0981
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/283076
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Fixes INTEROP-7269
flag=none
The problem was introduced in
https://gerrit.instructure.com/c/canvas-lms/+/275160 which added a UUID,
to the KID. If the UUID happens to have something that looks like a
date, Time.zone.parse picks that up and doesn't return the date in the
first part of the kid. For example:
> Time.parse "2022-01-25T17:46:59Z_d2fe13a4-b3f7-4feb-9"
=> -0009-02-04 17:46:59 UTC
This resulted in a flaky spec. It could also cause rotate_keys to rotate
keys multiple times on the first of the month (which can happen but
we're not sure why, see 89941a1d). It could also cause the keys to not
rotate (if the timestamp is in the future):
> Time.zone.parse "2022-01-25T17:46:59Z_2feb2023-aaaa-aaaa-a"
=> Thu, 02 Feb 2023 17:46:59 UTC +00:00
Test plan:
- run Lti::KeyStorage.retrieve_keys to see the keys; then Lti::KeyStorage.rotate_keys
- retrieve keys again to see they rotated
- run rotate_keys again and make sure they haven't rotated
- class << Lti::KeyStorage; def min_rotation_period; 10.seconds; end; end
- run rotate_keys again and make sure they've rotated
- class << SecureRandom; def uuid; "2feb2023-aaaa-aaaa-a"; end; end
- run rotate_keys again and make sure they only rotate 10 seconds after
the last time and that the kid includes that string
Change-Id: Ia67cafe3d04a650d8c6735835bb170b8dc8980ee
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/283562
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
test plan:
- create a migration that adds an integer column whose name
ends in "_id" and stage it for commit (no need to actually
run the migration)
- `script/rlint` should warn you to use bigint types for
id columns
- using type `:integer` with `limit: 8` is acceptable
as is using type `:bigint`
- no complaints are issued in `down`
flag=none
closes DE-953
Change-Id: Ifd1f4ff1cc74368b99ac2eda42181bcec25b94c8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282668
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
per our migration guidelines, tables, columns, and indexes
should be created in predeploy migrations
test plan:
- create a migration and stage it for commit (no need to
actually commit it or run it) and ensure `script/rlint`
flags issues for a postdeploy migration that:
(1) tries to `create_table`
(I don't bother flagging all the index or column
creation inside this block, since the table is enough)
(2) tries to create columns or indexes inside `change_table`
(e.g., with `t.index`, `t.text`, `t.boolean` etc.)
(2a) a postdeploy migration that uses a change_table
block to remove columns or indexes is ok
(3) tries to `add_column`
(4) tries to `add_index`
(5) tries to `add_reference`
- the above things should not be errors in a predeploy migration
and should not be flagged if they appear in `down` regardless
flag=none
closes DE-949
Change-Id: I209f44289f3a21f80d3d1c063600c009fc406d7c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282588
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: James Butters <jbutters@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
test plan: a call to DataFixup::(Something).run should be flagged
unless the migration is tagged :postdeploy
flag=none
closes DE-948
Change-Id: I5c1deb43267f00d4a042bb3bae9db906a2e3673a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282570
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: James Butters <jbutters@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
per our migration best practices, boolean columns should have
a default value and should not be nullable
test plan:
- create a migration that adds new boolean columns and stage it
for commit (no need to actually commit run the migration)
- run script/rlint and you should be warned if your column
does not have a default type or is nullable
- test various ways of doing this:
- with `add_column`
- with `t.boolean` in a `create_table` or `change_table` block
- with `t.column :foo, :boolean` in one of those blocks
- try both the :boolean and :bool spellings (the latter isn't
documented but is used in a few places anyway)
flag=none
refs DE-948
Change-Id: I305d2d146c1cca8fae80532416e0b8d5ded7e5c2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282302
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: James Butters <jbutters@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This reverts commit 7dd287adea.
Reason for revert: try again
Change-Id: Ie0eededa8465a7fb79bbd221c777646ab2640fa7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282036
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: James Butters <jbutters@instructure.com>
test plan:
- create a database migration and add it to a commit
(no need to push the commit or run the migration))
that tries to add a not-null constraint to a column. e.g.
change_column_null :table, :column, false
- run script/rlint
- you should be warned to use `disable_ddl_transaction!` if
this is not present in the migration class definition
- you should be warned to use DataFixup::BackfillNulls to
backfill null values if this is not present
(note that the linter can't actually tell if your arguments
to BackfillNulls are correct, because it takes an AR class name
instead of a table name and the cop has no way of linking
the two, and also the arguments are dynamic in many places;
it's just looking for the presence of that call)
closes DE-944
Change-Id: I8b79f8d0a44ae1bbf8de86c1a7e89c50cfe2b702
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282137
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
which is nicer because
(1) that's where the `disable_ddl_transaction!` needs to be, and
(2) the message doesn't preempt the `algorithm: :concurrently` note
test plan:
- add a migration to a commit (no need to actually run it or push
the commit) that adds an index to an existing table, and does
not use `algorithm: :concurrently` or `disable_ddl_transaction!`
- run script/rlint and you should see both warnings
(the transaction one at the top, under the class, and the
algorithm one on the index line)
flag=none
refs DE-945
Change-Id: Ibfc55b26387d3520c2de9fde5491869a9033b2c5
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282461
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
test plan:
- create a commit containing a migration that adds an index
to a table with add_index (no need to actually run the migration
or push the commit)
- run script/rlint and verify:
1. it wants `disable_ddl_transaction!` in the migration class
2. it wants `algorithm: :concurrently` in the index line
3. it doesn't care about 1 and 2 if the table was also created
in the same migration
- repeat with `add_reference` instead of `add_index`, except for
part 2, the option is spelled `index: { algorithm: :concurrently }`
and if you put `index: false` it doesn't complain (but it still
does if you put `index: true` or omit the option, as it defaults
to true)
- repeat with a `change_table` block using `t.index`,
e.g. `t.index [:columns], algorithm: :concurrently` is fine,
without the option results in the warning. (but note that
inside `create_table` the option is not needed!)
- repeat with a `change_table` block using `t.references`
(as with `add_reference`, `index: false` is fine, as is
`index: { algorithm: :concurrently }`, but `index: true`
or no `index` option result in the warning.)
flag=none
closes DE-945
Change-Id: If95ce2d930bdf41d0bc863a065f2447ec01d2271
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282227
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
test plan:
- create a migration that renames a table and `git add` it
(no need to actually run the migration or push the commit)
- run script/rlint
- it should tell you that renaming a table requires a multi-
deploy process and link you to our migration best practices page
- if your migration uses the process from that page and drops a view
before renaming the table, no warning is issued
- it doesn't warn you about the `down` half of the above pattern
flag=none
closes DE-947
Change-Id: Ic19807af210940df60e8b72952db297d71ac7dfa
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282292
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit replaces `sentry-raven` with the more modern Sentry SDKs.
Settings related to this change:
- `sentry_disabled`: completely disables Sentry, in case anything goes
sideways (defaults to `false`)
refs DE-922
flag=none
test plan:
- make sure to check out the correct patchset in the
`gems/plugins/uuid_provisioner` directory (see the tag below for ref)
- create a new project in Sentry (it may be easiest to set up Sentry to
run locally; check out `getsentry/onpremise` on GitHub for setup
details; it's not too onerous. note that you will need to add
`VIRTUAL_HOST` and `VIRTUAL_PORT` to the `nginx` container in that
`docker-compose.yml` in order to have your Sentry accessible via Dory
at `sentry.docker`)
- configure `config/sentry.rb` with your DSN
- restart rails and job servers
- confirm that, by default:
- any errors raised in controllers appear in the "Issues" section
- any errors raised in jobs appear in the "Issues" section
- user context associated with issues includes user IP and global ID
- job issues contain context from the job under the "INST-JOBS" section
- set `sentry_disabled` setting to true and restart rails/jobs servers
- confirm that the app/jobs start fine and there are no issues
- also confirm that Sentry is disabled (no errors received)
- remove `config/sentry.yml` and restart rails and jobs servers
- confirm that the app/jobs start fine and there are no issues
[pin-commit-uuid_provisioner=735c2102fb0020ac5aa80734a323bf08322d002d]
Change-Id: Id0fa4b4ee57ab812cd75d21d2e8ab5e21177af1a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279454
Reviewed-by: Ben Rinaca <brinaca@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
This reverts commit 276eb49f8e.
Reason for revert: Something in here is failing deploys
Change-Id: I615a90e6a4edf743ba47a2673bc14483ad141b3d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282032
Reviewed-by: Andrea Cirulli <andrea.cirulli@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: James Butters <jbutters@instructure.com>
Tested-by: James Butters <jbutters@instructure.com>
run rspecq/rspec tests in separate sub-build.
flag = none
refs: DE-929
Test Plan:
-Jenkins passes
-test count is consistent
Change-Id: Ia62730c32d1793d591555c0fd034538254efffc3
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282016
Reviewed-by: Aaron Ogata <aogata@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: James Butters <jbutters@instructure.com>
test plan:
- create a migration that changes a column data type
using change_column (no need to run it)
- `git add` the migration file (no need to actually
commit it)
- run script/rlint and it should tell you that
changing a column type usually requires a multi-
deploy process and link you to our best practices
document
- repeat with `rename_column` and the same warning
should appear
flag=none
closes DE-943
closes DE-946
Change-Id: I21b4c4296441cfe5e7252827341ac4e48a2eec7f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/281349
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
test plan:
- create a migration and add it to a commit (no need to actually
run the migration or push the commit) that performs raw SQL
via `execute` or `connection.execute`
- run `script/rlint` and you should see an "info" message that
raw SQL in migrations must be approved by a migration reviewer
flag=none
closes DE-941
Change-Id: Icfddda67f134aee9922d22c8c092836da09fc29d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/280808
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
test plan:
- create a migration (locally) and add it to a commit
(no need to actually commit, push, or even run the migration)
that adds a foreign key to an existing table, e.g.
add_foreign_key :users, :eportfolios
- run script/rlint, and it should complain that you should
use a non-transactional migration and add `delay_validation: true`
to the arguments
- do those things, run script/rlint again, and it should not
complain
- add a foreign key to a table created in the same migration,
e.g.
create_table :foo do |t|
end
add_foreign_key :foo, :bar
- run script/rlint again, and it shouldn't complain
flag=none
closes DE-940
Change-Id: I3f235cbf1ff4011e20d74bdaa8953b857c7e330a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/280514
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
test plan:
- an `info` message should be logged when a column is removed
reminding the engineer to use ignored_columns
- the message doesn't occur in `down`
flag=none
closes DE-942
Change-Id: If46c2d325f2e2d3b32964f23fed1ba25520efabd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/280381
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
since planned cops may provide different severity levels
depending on circumstances
refs DE-942
flag = none
Change-Id: I7501c13ea350241c47debceb7440cc8ce3d43925
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/280532
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
migrations that disable the DDL transaction should use if_not_exists
when adding a reference to a table
test plan: specs
closes DE-935
Change-Id: I3ccb9786446995c22c9cf2b8b1fa9da9e24e184e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/280182
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Added in the "resource_link_id" migration parameter for resource link
launches. This parameter is included when the LTI 1.3 resource_link_uuid
is different than the LTI 1.1 lti_context_id. It helps tools migrating
from 1.1 to 1.3 map old ids in their databases to the new ids.
flag=none
closes INTEROP-6796
test-plan:
* Install an LTI 1.1 tool and create an assignment that launches that
tool, then launch it. Note the resource_link_uuid that is provided. It
should look like a hash, not a UUID. Xander likely knows of a good LTI
1.1 tool that would work for testing, or a way to make the 1.3 tool
work for this.
* Install the LTI 1.3 Test Tool in at least the assignment submission
and course menu placement. I'm lazy, so I have it installed everywhere :)
* Migrate your previous LTI 1.1 assignment to use the LTI 1.3 Test Tool.
You can do this by updating the assignment's external_tool_tag.url to
the launch url of the LTI 1.3 Test Tool which is typically
http://lti13testtool.docker/launch.
* Launch the assignment. You should see under the
"https://purl.imsglobal.org/spec/lti/claim/lti1p1" a value for
resource_link_id. This should match the hash you saw earlier when you
launched the 1.1 tool.
* Now for some monkey-patching :). Place a byebug breakpoint at the
beginning of the include_lti1p1_claims? method in
lib/lti/messages/resource_link_request.rb. Then, launch the 1.3 tool
from the assignment. When you hit the breakpoint, monkey-patch
Assignment with the following code:
class Assignment < ActiveRecord::Base
def lti_resource_link_id
primary_resource_link.resource_link_uuid
end
end
This makes sure that the code thinks that the LTI 1.1 and 1.3
resource_link_ids are the same, even though that can basically
never happen in real life. This way though, we know that only
include the claim when we absolutely need to.
Continue the launch after monkey-patching and you should see there is
no resource_link_id claim in the lti1p1 claims section.
* Launch the tool from the course menu and make sure that you *don't*
see a key-value pair for resource_link_id. It should only be included
on assignment launches.
Change-Id: I85bbd977f4aa0809b2b031492bf58c0c86fea4bc
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/275459
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Ryan Hawkins <ryan.hawkins@instructure.com>
[skip-stages=Flakey]
manual, mostly by fixing copy/paste name problems,
combining groups that didn't need separating, or creating
new groups to contain contiguous top level groups that can't
be combined due to having different setup
Change-Id: I5b21664eabd8c3a62e22c09a28ba8bf519371aa5
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279428
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>
means we can remove a whole bunch of specific cop configurations that
now inherit the proper settings. cops that still have violations need
to be explicitly named to make them not error. this means that the
list of remaining cops is explicitly in our config now. also combined
the don't-auto-correct lists with the not-yet-fixed lists, so that
it's only a "permanent" section and a "todo" section
the final computed configuration is unchanged by this commit, with one
exception - Bundler/OrderedGems was moved to the Gemfile.d/.rubocop.yml
override, and some of the internal gems violated that so their
gemfiles have been autocorrected
Change-Id: I100884a96dbe9eb9d4bfc0692663c271daa9de16
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279413
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>
we're far enough along that the generated disablings are no longer useful
Change-Id: Iec341384f7a5f6eeb42bad4ae1ea12f433d270a4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279397
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>
[skip-stages=Flakey]
auto-corrected with post review ensuring line breaks continue to
convey original meaning of separated conditions, and cleaning up
some now-duplicated conditions
Change-Id: Ib9b31226de0665a2e4427fe595639d0d91a33f83
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279151
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>
[skip-stages=Flakey]
auto-corrected, with post review looking for opportunities to convert
to heredocs
Change-Id: Ic163882ebb5f4d2d28b5e5fd65188781d8eef3c8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279287
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>
[skip-stages=Flakey]
manual
this isn't a complete fix, but addresses many offenses
note in particular for feature flags the enable_at key
has changed from doing eval to a Date.parse.
Change-Id: I1381a107c238dc7102a815cc0b38df390299e59f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279085
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>
auto-corrected, with a bit of manual massaging afterwards if there were
multiple delegations to the same object to combine them into a single
call
Change-Id: Ic93ccad26b037e1f89874d36f9759e5c34435892
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278965
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>
[skip-stages=Flakey]
auto-corrected
had to rework some instfs to play nice with stubs in its specs
Change-Id: I894831c6503095c6b197154e39fe17e803702d9c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278785
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>
it only exists to add `utc` as an acceptable "zonifying" method, but the
upstream Rails/TimeZone cop already does that
Change-Id: I09e301fa73c0eee106f4da59039488f71962f215
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278962
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>
auto-corrected, with post review looking for important side effects
of the conditional (one found).
Change-Id: I0fb09793a8645fcac8d81974de9d18a167d2b710
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278946
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>
[skip-stages=Flakey]
auto-corrected, with a few instances of reworking things that modify
the hash as they're iterating
Change-Id: I73cef3016e46309c1c2ce906b013288d73976acf
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278781
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>
[skip-stages=Flakey]
auto-corrected, with post review to remove unnecessary empty down methods
in migrations, and change def x(*args); end to just def x(*); end
Change-Id: Ic006bcebb0b073e6c66ed957a561c93c3d368e24
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278893
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>
Migration-Review: Cody Cutrer <cody@instructure.com>
We were previously using an extremely permisive,
restricted scope when getting a token for the
Google Drive plugin (not LTI apps)
This change moves to using the recommended scopes
and should be enough to continue doing collaborations
Test Plan:
This is super hard to test locally, so I recommend
testing in beta or production like this:
- Navigatge to https://tudy.beta.instructure.com/
profile/settings
- If you already have Google Drive listed under
"Registered Services" delete it from that list
- Click on Google Drive in the "Other Services"
list
- Click on "Authorize Google Drive Access"
- Copy the URL of the page you are redirected to
- In an editor, change the "scope" query param
to match the scopes in this commit (URL encoded):
&scope=https%3A%2F%2Fwww.googleapis.com
%2Fauth%2Fdrive.appdata%20https%3A%2F%2
Fwww.googleapis.com%2Fauth%2Fdrive.file
- Paste the modified URL into your browser and
log in to Google
- Grant consent to connect with Canvas
- Verify you are redirected back to Canvas
- Verify Google Drive is now listed under
"registered services"
Now we need to test that the new scopes are
permissive enough. Do this by creating a collaboration
- As a teacher, create a collaboration using the Google
drive plugin
- Add a student to the collaboration
- Log in as that student in an other browser. Verify
you can access the collaboration and linked google
doc
- As the teacher, remove the student from the collab
- As the student, attempt to access the Google doc
- Verify you are asked to request permission
- As a teacher delete the collaboration and associated
document
- As the student, attempt again to access the doc
- Verify you are shown a "not found" message
Change-Id: I6f3c514534fc9e1d161f6b760365685b300887d1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278807
Reviewed-by: Ben Rinaca <brinaca@instructure.com>
Reviewed-by: Tucker Mcknight <tmcknight@instructure.com>
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>
[skip-stages=Flakey]
auto-corrected, and also introduced empty? method on several
file-like classes so that the autocorrect is safe on them
Change-Id: I7c84a39fc3f11cad50bf4ccb3cd97883881c2129
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278756
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>
[skip-stages=Flakey]
auto-corrected
also enable Style/PercentQLiterals forcing uppercase with it, so that
both %q and %Q get changed to bare %
Change-Id: I91389c18d864b3ec638c6cd366c2c74f78c69a57
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278673
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>
apparently sometimes active support wasn't getting required fully here
Change-Id: I3fbe46859e504b58ae7bda7b7d254fb7e30595bb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278832
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>
[skip-stages=Flakey]
auto-corrected, with post-review changing hashes that were meant
to be string keys in the first place
Change-Id: I877a365b9035bb62cea4d3b2f01f641f55b63281
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278676
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>
not really rubocop, but found while doing Style/ExpandPathArguments
Change-Id: Iec2710795be95e7663df5a49de212043459e9823
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278626
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>
auto-corrected, but so many tweaks after to gemspecs it may as well
have been manual
Change-Id: I69aeb6e216894462d6d893ed4c123aa9898fc72f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278516
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>
[skip-stages=Flakey]
manual
also use methods on Pathname while we're auditing these, to simplify
Change-Id: I4ff00329a7de9d39f5797790c22b55f7bb661c05
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278481
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>
auto-corrected, with post-review to look for things that are only
hash-like (one found; fixed by calling `to_h` beforehand)
Change-Id: I1a1f273513ff466197a4da6a7b6f68565a08abe0
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278500
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>
why:
* live events is not plugin-configured anymore like the docs say, so
update docs and code to match reality
* remove env var for stubbing and rely solely on dynamic settings
* other small fixes for local stubbing
refs INTEROP-7143
flag=none
test plan:
* set up live events locally following the doc
* stubbing to stdout should work
* sending to canvas kinesis stream should work
* if you have the publisher set up, sending to the publisher stream
should also work
Change-Id: I4e627590e79bfebd8da3d43a8a479eacd5aa0b9f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277280
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Xander Moffatt <xmoffatt@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Mysti Lilla <mysti@instructure.com>
auto-corrected, with post-review looking for cases where an unqualified
column would now be (incorrectly) qualified (only one found)
Change-Id: I62ef6d40ce9e7bc062db261d9c6fb9383ecd102e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278432
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Migration-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
the form .map { }.sum was autocorrected to .sum {}.
.map {}.inject(:+) and variations was manually corrected
Change-Id: I44b8ef3b8257e8acf70f259aed35f1e16cde9856
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278335
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
auto-corrected for positive cases; negative cases (reject {}.length)
had to be done manually by inverting the block's output
Change-Id: I9e0616319c9d7f93c24d03f928b78072e0959c39
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278326
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
manual (it says it's auto-correct, but it didn't. which is fine cause a few
I simplified in other ways than using Array.new)
Change-Id: I8a4d5cf61deac22ad24e012f5a6f3647c84d3d3c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278312
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
Naming/HeredocDelimiterNaming and Rails/SquishedSQLHeredocs
the former was manual, the latter was automatic. I also changed
some <<- to <<~ to allow for better formatting
I also had to change comments inside squished SQL heredocs to
be block comments (since newlines are removed); searching for those
I found some multi-line strings that are better as heredocs
Change-Id: I6b138f8e32544b97df1e4c56f09ee5316cbdef9d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278184
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
auto-corrected, with review for non-single-argument-callers.
there weren't any, but I did disable it anyway for one file where
it seemed to make the DSL less legible (QtiItems)
Change-Id: I1b4c43ffd899e656902981baac213ca394791b67
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278156
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
auto-corrected for the most part, but I went through specs and used
the have_key matcher when possible
Change-Id: I4a2df1cdd536a94f3b493668386883d1148660c9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278050
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
auto-corrected, with post-review checking for possible nilness
Change-Id: I89c30b92691a2a5f73d98d9c8ac721c50d3a4ba7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278014
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
some auto, some manual if there was a better fitting construct than
an if/elsif/else
Change-Id: Ifee268f80d411b7b4c98a024c209d2014c7d0c9f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277998
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
auto-corrected (with manual post-review looking for possible non-array,
non-string slices that might go bad, with none found)
Change-Id: I00feba96f58f701ed1f668f86928b8871d0d8ef2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277991
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
auto-corrected, with manual review to identify possible nilness
Change-Id: I205436e5c3cb37aae99ea552c7d14e6d1a04ef06
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277893
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
closes OREO-1234
flag = none
here is the problem:
- now some features use graphql endpoint to create canvas elements
- for example, recent discussion redesign use graphql queries
- but graphql requests dont have url path and query params for
app "pageview etl" when deciding page view and participation
here is the solution:
- discussed in OREO standup and detail can be found at OREO-1197
- to cover student activities, we added "operationName" in http
response "X-Canvas-meta"
- later, turbo log parser will pick up the operation name and
pass it to downstream app "pageview etl"
- later, pageview etl will use it to classify student activities
via graphql endpoint
here is the change:
- called "add-meta-header" function when the request parameters
have operation name field in it
- update unit tests accordingly
test plan:
- downloaded the change locally
- bundle update
- bundle exec rails server
- launch http://localhost:3000
- under your local organization, in account settings, enable feature
"discussion redesign"
- select a course
- if you don't have any discussion, act as a teacher and then create
a discussion
- command option i and open browser debugger
- under network tab, type "graphql"
- back to the ui, act as a student and use the student to open the
discussion created by the teacher
- in debugger, click the graphql endpoint entry, verify response header
"X-Canvas-Meta" has on=GetDiscussionQuery in it
- back to the ui, use the same student to reply the discussion
- in debugger, verify "X-Canvas-meta" has on=CreateDiscussionEntry in it
Change-Id: Ia7a208fb30dde18ac4b8413578ee6d1144283a22
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277799
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Kwok Lam <klam@instructure.com>
Product-Review: Kwok Lam <klam@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
to get its no-longer-dependency on nokogumbo
the major version bump was only for dropping ruby 2.5
Change-Id: I5a03053e8c3f800a39bcaa384fb5003fa489a2e1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277905
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>
need to wrap the sub-process execution of bin/rubocop in a
Bundler.with_unbundled_env so that _it_ can also use bundler/inline
to install its dependencies
Change-Id: I08b86288152c156f3704d1e4f9c5bfb6a5cf7867
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277834
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>
[skip-stages=Flakey]
the balance. mostly. Lint/UriEscapeUnescape is put in the pending
block because it's so touchy, and I didn't want to deal with it
right now
all manual
Change-Id: Ibeb81e013f56f160d51f7d237a9bcfe98daa1e53
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277569
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>
all manual
note that for any methods moved/re-indented, I made no other changes
(except possibly a remove of a `self.` on a method call to call another
now-private method) for ease of review
Change-Id: I0cca6644264a0b46a45a1a5c99021c9deb64fca0
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277532
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This moves the inst_access gem to the public repo and references
the now published inst_access gem instead.
Github:
https://github.com/instructure/inst_access
Rubygems.org:
https://rubygems.org/gems/inst_access
Test Plan:
- Specs pass
flag = none
Change-Id: I6002b118723e5a329202085a6c649a857e34d0e3
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277527
Reviewed-by: Michael Ziwisky <mziwisky@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
it's no longer a problem with zeitwerk
Change-Id: I737701d9f3400468d9a8698a898b422f5969e780
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277227
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>
[skip-stages=Flakey]
so we don't have to manually do it, especially with increasingly
complicated relative paths.
and stop manually requiring it in plugins, and also convert any other
require File.expand_path to require_relative
Change-Id: Ifc07149372fe09bcc146ac1b980feb3a9fd62865
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/276816
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: August Thornton <august@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
all manual. a few cascaded to cleaning up leaky consts in specs
Change-Id: Ia77478b92f23e06d1023f4e3cfa02a751593a368
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/276484
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
refs INTEROP-7051
this is a temporary measure to help debug an issue where the user_uuid
doesn't exist on the shard associated with the canvas_domain. these
"debug" fields should not be relied upon by InstAccess Token consumers,
and will be removed when they are no longer needed to debug this issue.
Change-Id: Ibeceb09b8c2cc80226bdbccd9374073c277f0298
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/275998
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
Reviewed-by: Evan Battaglia <ebattaglia@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
It is unused currently internally within instructure and we don't document using
it for anything particular so nothing should break
refs FOO-2410
Change-Id: I5611c80955b6c24df00e65e06015c032bab3111c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/275334
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
refs FOO-2410
test plan:
- in dynamic_settings.yml, add the following block:
```
store:
canvas:
services-jwt:
# these are all the same JWK but with different kid
# to generate a new key, run the following in a Canvas console:
#
# key = OpenSSL::PKey::RSA.generate(2048)
# key.public_key.to_jwk(kid: Time.now.utc.iso8601).to_json
jwk-past.json: "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"n\":\"uX1MpfEMQCBUMcj0sBYI-iFaG5Nodp3C6OlN8uY60fa5zSBd83-iIL3n_qzZ8VCluuTLfB7rrV_tiX727XIEqQ\",\"kid\":\"2018-05-18T22:33:20Z_a\",\"d\":\"pYwR64x-LYFtA13iHIIeEvfPTws50ZutyGfpHN-kIZz3k-xVpun2Hgu0hVKZMxcZJ9DkG8UZPqD-zTDbCmCyLQ\",\"p\":\"6OQ2bi_oY5fE9KfQOcxkmNhxDnIKObKb6TVYqOOz2JM\",\"q\":\"y-UBef95njOrqMAxJH1QPds3ltYWr8QgGgccmcATH1M\",\"dp\":\"Ol_xkL7rZgNFt_lURRiJYpJmDDPjgkDVuafIeFTS4Ic\",\"dq\":\"RtzDY5wXr5TzrwWEztLCpYzfyAuF_PZj1cfs976apsM\",\"qi\":\"XA5wnwIrwe5MwXpaBijZsGhKJoypZProt47aVCtWtPE\"}"
jwk-present.json: "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"n\":\"uX1MpfEMQCBUMcj0sBYI-iFaG5Nodp3C6OlN8uY60fa5zSBd83-iIL3n_qzZ8VCluuTLfB7rrV_tiX727XIEqQ\",\"kid\":\"2018-06-18T22:33:20Z_b\",\"d\":\"pYwR64x-LYFtA13iHIIeEvfPTws50ZutyGfpHN-kIZz3k-xVpun2Hgu0hVKZMxcZJ9DkG8UZPqD-zTDbCmCyLQ\",\"p\":\"6OQ2bi_oY5fE9KfQOcxkmNhxDnIKObKb6TVYqOOz2JM\",\"q\":\"y-UBef95njOrqMAxJH1QPds3ltYWr8QgGgccmcATH1M\",\"dp\":\"Ol_xkL7rZgNFt_lURRiJYpJmDDPjgkDVuafIeFTS4Ic\",\"dq\":\"RtzDY5wXr5TzrwWEztLCpYzfyAuF_PZj1cfs976apsM\",\"qi\":\"XA5wnwIrwe5MwXpaBijZsGhKJoypZProt47aVCtWtPE\"}"
jwk-future.json: "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"n\":\"uX1MpfEMQCBUMcj0sBYI-iFaG5Nodp3C6OlN8uY60fa5zSBd83-iIL3n_qzZ8VCluuTLfB7rrV_tiX727XIEqQ\",\"kid\":\"2018-07-18T22:33:20Z_c\",\"d\":\"pYwR64x-LYFtA13iHIIeEvfPTws50ZutyGfpHN-kIZz3k-xVpun2Hgu0hVKZMxcZJ9DkG8UZPqD-zTDbCmCyLQ\",\"p\":\"6OQ2bi_oY5fE9KfQOcxkmNhxDnIKObKb6TVYqOOz2JM\",\"q\":\"y-UBef95njOrqMAxJH1QPds3ltYWr8QgGgccmcATH1M\",\"dp\":\"Ol_xkL7rZgNFt_lURRiJYpJmDDPjgkDVuafIeFTS4Ic\",\"dq\":\"RtzDY5wXr5TzrwWEztLCpYzfyAuF_PZj1cfs976apsM\",\"qi\":\"XA5wnwIrwe5MwXpaBijZsGhKJoypZProt47aVCtWtPE\"}"
```
- Ensure /internal/services/jwks loads correctly
- In console, ensure `CanvasSecurity::ServicesJwt.decrypt(Base64.decode64(CanvasSecurity::ServicesJwt.for_user('localhost', User.first)))`
and `CanvasSecurity::ServicesJwt.decrypt(Base64.decode64(CanvasSecurity::ServicesJwt.for_user('localhost', User.first, symmetric: true)))`
both work and produce sensible looking output
Change-Id: I13c6c35cc92ed12d03bf97e89e590614e11c6d47
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/275160
QA-Review: August Thornton <august@instructure.com>
Product-Review: August Thornton <august@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
Reviewed-by: Evan Battaglia <ebattaglia@instructure.com>
This reverts commit eb44849fd6.
Reason for revert: apt repo is no longer broken
Change-Id: I9224768d117e1b1a030f881ec03e0295afcb73d9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274305
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
opts (to use the global datacenter) should be used
refs INTEROP-7008
flag=none
Test plan:
- I'm using the updated function to fix up all the regions now, on test,
beta, and soon prod. (Just getting the keys from iad, pushing global,
and for each region resetting the cache.)
Change-Id: I08a0de5d541dc7fc9824fbfc2de4e49a7467dd24
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274905
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
the current description is what the spec was made for, but it generally
catches errors for new reports misconfigured in a plugin, so renaming to
hopefully entice investigation.
Change-Id: I8b8fd217ecac621f37cc7903b6589646587a775a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274944
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Knapp <eknapp@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
closes FOO-2364
flag = none
test plan: configure incoming mail, and it should show up in the deep
health check.
Change-Id: I23b231273c83c214ec05ef89633895722876da2a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274843
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
they may have a different license
Change-Id: I158d52c9964fcfea2e04ae0ceb28d80f89ec079d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274654
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
we no longer have a bajillion variants, so it's easy to look for exactly
what we want
Change-Id: Ib8569d0729b1563f978ab7d8db5b717bf4c38b11
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274591
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
test plan
- run report
- it should work
- existing specs should pass
fixes VICE-2084
flag=none
Change-Id: I786b98576aa303d49c6f58863b9ce6d36ac449f6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/273683
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
it was just too confusing on which one an editor is using, double comments
in jenkins, etc.
this is accomplished by several things:
* required cops are just marked as severe, instead of using a separate
config for them, and failing if anything shows up from that config
* get rid of all the logic to only include certain directories for
certain cops. turns out it's not _that_ ominous to correct errors
across the entire repository before marking a cop as required.
* but still auto-generate config to turn _off_ autocorrect for
non-severe cops. this is important because auto-correct must run
for entire files, and we don't want it auto-correcting optional
things that you didn't touch.
* update gergich to get more details from the parsed comments.
this plus the prior point means we _don't_ have to have heavy mode when
in autocorrecting, but we still display out-of-context lines that were
autocorrected
this also makes it so we can use per-dir .rubocop.yml files again, so
take some of the exceptions out of the root and put them in their own
directory
Change-Id: Ie936d1a9920b68910acd250ba817c7b4a670b958
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274394
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This reverts commit a7f0ea277a.
Reason for revert: the removal broke launching NQ on mobile
Change-Id: I1f719ce9317cfb1f4477098fe25fd7eb558b0d09
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/273543
Reviewed-by: Jeff Largent <jeff.largent@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Mark McDermott <mmcdermott@instructure.com>
Product-Review: Jackson Howe <jackson.howe@instructure.com>
the built-in Style/MutableConstants cop does the same thing
Change-Id: I1b415c46f93c80d42023a6f8ab071c2f7846c2b8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/273640
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>
and remove now-unneccessary uses of db:migrate:predeploy, since
db:migrate will implicitly run predeploys first
Change-Id: I4de20c5a24c8d9125e698b9a650eb7a592b258ff
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/273291
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>
fixes FOO-2263
flag = none
test plan:
- setup pop3 incoming mail processing with secure settings
- it should still work
Change-Id: I2ed6816b0fa791abd5f4cb8e9193af6916af8aa8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272501
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Reviewed-by: August Thornton <august@instructure.com>
fixes MAT-346
flag=none
Test plan 1:
- Navigate to a RCE instance.
- Set HTML content using flexbox properties
Example: flex-direction, flex-wrap, flex-flow, etc.
- Save content.
- Verify that the content was saved with the flexbox
properties.
Test plan 2:
- Navigate to a RCE instance.
- Set HTML content using grid properties
Example: grid-row, grid-column, grid-template, etc.
- Save content.
- Verify that the content was saved with the grid
properties.
Change-Id: Iff2a57ad43d8526c5f477e0575e188aaa76b4f9f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272776
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Juan Chavez <juan.chavez@instructure.com>
It takes string, rather than symbol, keys. This was breaking key
rotation (Lti::KeyStorage and Canvas::Oauth::KeyStorage).
Fixes INTEROP-7008
flag=none
Test plan:
- I copied and pasted the new method definition into beta & prod consoles
and ran rotate_keys (those were scheduled to be done and just failed
running today anyway)
Change-Id: I619406d06d4593b49abd80e0cf9a7dee1a52a442
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272640
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
namely, the graphql subgraph path
refs INTEROP-6704
flag = none
test plan:
- post to the subgraph endpoint with a base64-encoded request id
$ curl 'http://<canvas>/api/graphql/subgraph' \
-X POST \
-H "X-Request-Context-Id: $(echo sweet-request-id | base64)" \
-v
- the response should include this header
< X-Request-Context-Id: sweet-request-id
Change-Id: I46a905b893fd21643d5bf857fce8f54c9ab579e1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272478
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
a configuration value can be set to override redis guards. this
is needed for test queue work to allow multiple unsafe redis
commands to be used in a test environment. see RspecQ:
https://gerrit.instructure.com/c/canvas-lms/+/272130
test-plan:
-set ignore_redis_guards to true in test environment config
-execute `exec` command against test redis instance
-setting ignore_redis_guards when running in dev or production
has no effect
Change-Id: I2aa3a04c9ce6b024a9e052a93d5278b27569dc5c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272559
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: James Butters <jbutters@instructure.com>
QA-Review: Kyle Rosenbaum <krosenbaum@instructure.com>
Product-Review: Kyle Rosenbaum <krosenbaum@instructure.com>
a configuration value can be set to override redis guards. this
is needed for test queue work to allow multiple unsafe redis
commands to be used in a test environment. see RspecQ:
https://gerrit.instructure.com/c/canvas-lms/+/272130
test-plan:
-set ignore_redis_guards to true in test environment config
-execute `exec` command against test redis instance
-setting ignore_redis_guards when running in dev or production
has no effect
Change-Id: Ib165129af16d321f29e4a5512ed4fe0909e34a93
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272541
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: Kyle Rosenbaum <krosenbaum@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: James Butters <jbutters@instructure.com>
so that script/rlint can run as fast as possible locally
Change-Id: Ia081a0cb4292db92b5cc36930a484dc5edeef921
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271958
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>
it's disabled anyway, and because the cop requires active_record itself,
it adds 0.5s to the boot time of rubocop
Change-Id: Ie22944afef9df45fe08ea0e13857a4703d1c53f8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271959
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Reviewed-by: August Thornton <august@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
so that we can reference only that fragment from our binstub
(which is de-springified), making it run significantly faster
Change-Id: I4f602e6c4d1feecf74eccd66e610781c76756ffc
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271957
Tested-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Don't assume logger has been set.
For actual Canvas, it should be, as config/initializers/live_events.rb
has `LiveEvents.logger = Rails.logger` in the
`Rails.configuration.to_prepare` block
flag=none
Change-Id: Ia613aa2e15974f5b53056a4359e9db0bd58f8846
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272141
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
especially because when run as a pre-commit hook, everything is staged
also add env var to bypass rubocop, and actually set a useful exit code
Change-Id: I4d7b0a5c5cdad0b0d9d988b9dd6d134d96576d8e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271956
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>
and rubocop-rails was split out to a dedicated gem
Change-Id: I12b45540a400aab0d57db303457103516ab92df7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271931
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>
Psych has safe_load now, and it's fairly trivial to convert our existing
overrides to use that instead
Change-Id: I2648df8d4574e15fc9072a25882e318d902765c3
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271939
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>
Creating a Kinesis client with no creds causes the AWS gem to reach out
to an AWS-internal IP (169.254.169.254). There is a connect timeout of 1
second but this is disabled in
config/initializers/no_timeouts_debugging.rb. As a result, creating a
Kinesis client can indefinitely hang, making Canvas completely unusable.
The issue is made worse by recent changes which turned Live Events on
with such invalid config for anyone who used config from
dynamic_settings.yml, whereas previously, Live Events had to be manually
turned on via a plugin.
This change prevents invalid configs from reaching the Kinesis client;
except in prod in case anyone is actually using the feature whereby
AWS looks up creds on the internal IP (and to avoid any possible
slowdowns by checking for the settings repeatedly).
Additionally I made NoRaiseTimeoutsWhileDebugging log a message when a
timeout happens, which would have greatly helped with debugging this
issue.
refs INTEROP-7016
flag=none
Test plan:
- Have config in dynamic_settings.yml like the old
dynamic_settings.yml.example, with live-events config but no creds,
e.g.:
live-events:
aws_endpoint: http://kinesis.canvaslms.docker
kinesis_stream_name: live-events
- From a rails dev console run LiveEvents::Client.config. It should
return nil.
- Run Canvas and make sure you can login, view a course, etc. -- just
try anything that emits a live event (almost anything)
- Add aws_access_key_id and aws_secret_access_key_dec from from
dynamic_settings.yml.example into your dynamic_settings.yml
- From a rails dev console run LiveEvents::Client.config. It should
return the config.
- From a rails dev console run `LiveEvents::Client.new`. It should
immediately produce a client.
- Restart Canvas and make sure you can view a course, etc.
Change-Id: I9a325b7f30c8e0203c2903a25a1f0139776b3f1f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271907
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Feature flag new_quizzes_in_module_progression now removed
and assumed on.
Closes LS-2514
flag = none
Test plan:
- check out better module progression patch:
https://gerrit.instructure.com/c/canvas-lms/+/265118
- Should expect the same behavior when replicating
better module progression test plan but without enabling
removed flag
Change-Id: I321ac5de8bc7823df9807e407cb4de940b49a9a3
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271642
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Jackson Howe <jackson.howe@instructure.com>
Reviewed-by: Jackson Howe <jackson.howe@instructure.com>
I forgot to add the file when I added this class
Change-Id: Idc0c2a00ddbba0596063450d214c12f5c482c86f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271734
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
closes FOO-2226
flag = granular_permissions_manage_assignments
Split manage_assignments_add and manage_assignments_delete off from the
main manage_assignments permission.
These permissions control more granularity for adding and deleting the
following types of content:
- Assignments
- Assignment Groups
- Quizzes
- Question Banks
- Questions within Question Banks
- Live Assessments
test plan:
[ with the flag off ]
- Smoke test creating, editing and deleting assignments, assignment
groups, quizzes, and question banks.
- It should all work as before.
[ with the flag on ]
- Create roles that have only the add permission, only the manage
permission, and only the delete permission.
- With those three roles, as well as with a role with all permission,
try creating, editing, and deleting assignments, assignment groups,
quizzes, and quesiton banks.
- It should make sense with the role.
Change-Id: I06505509e55e7ac6c3b5ef1c688ef1353e2045d8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271290
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: August Thornton <august@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: August Thornton <august@instructure.com>
This change adds support for typescript transpilation to our frontend
build via babel, and also updates eslint and canvas_i18nliner to be
able to understand typescript as well. The main goal of this PS is to
enable developers to add typescript code to the Canvas codebase but to
be unopinionated about how type-checking is done; at this stage types
will only be checked by running the new `check:ts` or `check:js`
scripts (which run the typescript compiler directly), or via
integration with an IDE like RubyMine or VS Code.
closes LS-2430
flag = none
Test plan:
BUILD STEPS:
- FE build, i18n build, and tests pass Jenkins
- `bin/rails canvas:compile_assets` still works
- `RAILS_ENV=production bin/rails convas:compile_assets` still works
SPOT CHECKING:
- Starting up rails and run `yarn build:js:watch`
- Click around Canvas and make sure the frontend still loads as
normal
Change-Id: I8bb1a0f065e09496a924708dead6fa4518b59496
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/270401
Reviewed-by: Ahmad Amireh <ahmad@instructure.com>
Reviewed-by: Nate Armstrong <narmstrong@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Ahmad Amireh <ahmad@instructure.com>
Product-Review: Jeff Largent <jeff.largent@instructure.com>
plus miscelaneous improvements to the gql controller specs
refs INTEROP-6900
flag = none
test plan:
- request the subgraph schema w/o providing any auth, e.g.:
$ curl http://localhost:3000/api/graphql/subgraph \
-X POST \
-d query='query { _service { sdl } }'
- this should return the schema
Change-Id: I705f694ffb490fd4c2ba2241291769a6877cee14
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271051
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
just load settings from consul.yml to avoid circular references at boot
Change-Id: I405e8541cc46e58f2447d33c87d99d6906da5ca9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271102
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
so that everyone gets the benefits, not just instfs
also include a new circuit breaker so that if consul is unresponsive for more than the
retry interval, we just let failures through quickly for a while
Change-Id: I9ba757c8529c1011ca771612f592f289c6a844b6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/270789
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Community links should never be added to the codebase itself anymore.
Instead they should be referenced as a translation with a key of
'community.#{key}'. The links should be stored in config/locales/community.csv,
which is a download of the 'canvas-import' tab of the
'Canvas Translated Guides Mapping' sheet managed by the community team.
There are a few links that the community team says should be updated, that
will be done in a followup commit.
test plan:
- Spot check the various types of files (js/handlebars/erb/plain js)
- Try English, Spanish (or another non-english language in community.csv,
and Catalan (or another language not in community.csv)
fixes FOO-1771
Change-Id: I67534204c2236a3ff05642cb2d13b8f546da9498
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/270622
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Charley Kline <ckline@instructure.com>
QA-Review: Charley Kline <ckline@instructure.com>
Product-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-1019
the common case is that no suspensions are (or ever have been) registered,
so avoid the allocation when callbacks are run _twice_ for every object
instantiated from the database
also, remove some very very old rails compatibility code
Change-Id: Ie7c2c75b2ce83a0473fa023b49ef977018a9fe15
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/270031
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
fixes INTEROP-6892
flag = none
test plan:
tokens can still be generated from `POST /api/v1/inst_access_tokens`
Change-Id: I9d17ce794440c4e347c0f1b5d26a9f15828be4e4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/269818
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
flag = none
so as to avoid confusing people who actually know what an Identity Token
is, e.g. in the OpenID Connect sense.
test plan:
i think it's sufficient to see tests pass, since this is just a renaming
and slight refactoring. but for a concrete smoke test, follow the same
test plan described in the commit message of commit 4826df723d, only
instead of hitting the endpoint `/api/v1/inst_ids?unencrypted=1`, hit
`/api/v1/inst_access_tokens?unencrypted=1`
Change-Id: Ie7b646ff80129c094aa44ed46999321bfbcf2851
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/269690
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Ryan Hawkins <ryan.hawkins@instructure.com>
QA-Review: Ryan Hawkins <ryan.hawkins@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
fixes INTEROP-6913, INTEROP-6892, INTEROP-6893, INTEROP-6920
flag = none
This commit introduces the InstID token, a signed and encrypted JWT (aka
JWE) that will soon be usable for Canvas API access (that's "part 2").
If the InstID class is configured with a private signing key and public
encryption key, it will be able to produce encrypted JWTs and validate
and deserialize decrypted JWTs. If it is configured with only a public
signing key, it cannot produce tokens but it can still validate and
deserialize decrypted ones. Therefore this class can be used by the
identity provider (currently Canvas) to produce tokens, but also by any
services that want to use InstID tokens for authentication.
test plan:
1) generate two RSA keypairs. one way to generate a keypair is from a
rails console:
> keypair = Canvas::Security::RSAKeyPair.new
> puts keypair.private_key.to_s
> puts keypair.public_key.to_s
2) choose which one is for signing and which is for encryption, then add
the private signing key and the public encryption key to your rails
credentials:
- run `bin/rails credentials:edit`
- add an entry like the following, and then save and close your
editor:
```
inst_id:
encryption_key: |
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvY1EMlGm1daM87ejGuFX
<...snip...>
/wIDAQAB
-----END PUBLIC KEY-----
signing_key: |
-----BEGIN RSA PRIVATE KEY-----
MIIEpAIBAAKCAQEAnDwED/QOB0f0H6TOZqLmjaPqA7m8c40NDXkAa6u5cK8zCbk3
<...snip...>
QhjPgifBwTrzj21484CfiPfy5oe756Exerj8PIlRrE/hxWRSDwBIOg==
-----END RSA PRIVATE KEY-----
```
3) open a rails console and do:
> id = InstID.for_user('user-uuid')
> id.to_token # make sure this doesn't blow up
> token = id.to_unencrypted_token
> decoded_id = InstID.from_token(token)
> id.jwt_payload == decoded_id.jwt_payload # => true
TODO in followup commits:
- make canvas accept InstID tokens for auth
Change-Id: Ie550c17507c26f9944bd62a747a6a63161e8e770
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/268872
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Michael Ziwisky <mziwisky@instructure.com>
Product-Review: Michael Ziwisky <mziwisky@instructure.com>
refs FOO-2036
basically, refresh tokens took over the meaning of expires_at to mean
when you needed to refresh. another column is needed to mean you
need to refresh vs. the token is expired, period. I opted to add
a new column for permanent_expires_at instead of needs_refresh_at
because the datafixup is less racey this way.
Change-Id: Ia11a2e862e540f211d628aa39c05bcb6930647ac
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/266765
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This has been in prod a while and not going to be disabled
test plan
- specs should pass
flag = none
refs VICE-1647
Change-Id: I762d2be87fdcaf0eb41eb502a385f33579b2606d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/268627
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Omar Soto-Fortuño <omar.soto@instructure.com>
QA-Review: Omar Soto-Fortuño <omar.soto@instructure.com>
Product-Review: Omar Soto-Fortuño <omar.soto@instructure.com>
test plan
- run provisioning report
- it should work and match UI
flag = none
fixes VICE-1647
Change-Id: I834342b2ce31064821b2838406b07ccacb7836a1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/268617
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Omar Soto-Fortuño <omar.soto@instructure.com>
QA-Review: Omar Soto-Fortuño <omar.soto@instructure.com>
Product-Review: Omar Soto-Fortuño <omar.soto@instructure.com>
This reverts commit 2c5c3584ff.
Reason for revert: I'm back in the office and can debug the problems
Change-Id: Ib469fff450a8d51d7ca59cb9d7fa29874d6b6e53
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/268386
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>
test plan
- deleted a pseudonym from a user included in
the eportfolio report results
- run eportfolio report
- it should still work
fixes VICE-1696
flag=none
Change-Id: I8c51ca899e64eab94014a67ba17115b34c4d65be
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/268449
Reviewed-by: Michael Ziwisky <mziwisky@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Jacob Burroughs