and apply new cop 99% Style/SuperArguments
and a couple Layout/EmptyComment and Style/ArgumentsForwarding that
are found by fixes in those cops
Change-Id: Icc0af9c8065f035bca43868b564f73e8776052f2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/348626
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jake Oeding <jake.oeding@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
closes AE-494
[skip-stages=Flakey]
[skip-crystalball]
several things going on here. in general, many of our monkeypatches
have been implemented in upstream, but in a vastly different way, so
we need to enable/configure those things
* `servers` is no longer accepted as a backwards compatible
configuration option; use `url`
* `database` is no longer accepted as a backwards compatible
configuration option; you _can_ use `db`, but preferable to
just use `url`
* no longer merge together redis.yml and cache_store.yml; if one
references the other, simply use the config from the other
* `nil_store` is no longer accepted as a backwards compatible
configuration option; use `null_store` (almost no one should
be explicitly using this anyway, so nbd)
* automatically not-even-trying when redis has previously failed
is now handled by redis-client's circuit breaker. be sure to
configure it in redis.yml/cache_store.yml/dynamic settings
* ignoring redis failures completely is already done by
RedisCacheStore; just rely on that, except for the few cases
where we use redis directly. some of these now take advantage
of a new `failsafe` kwarg (and often in combination with
pipelining), and some just handle it directly
* move logging to a RedisClient middleware
* move Twemproxy disallowed commands to a RedisClient middleware
* simplify Canvas.lookup_cache_store to have far less special casing
(in particular, patching is done automatically now)
* add ability to use Redis::Cluster (configure with `nodes` instead
of `url`)
* still override Redis::Distributed's HashRing, so that we don't
change our ring layout with the new MD5 hashing for servers. but
we got to vastly simplify the new class, due to upstream
refactorings allowing us to simply override a single method rather
than having to re-implement the entire class
* statsd reporting of redis errors is now simply passed as a callback
to RedisCacheStore, breaking CanvasCache's dependency on InstStatsd
Change-Id: I787672677a21994d40ae304dbac0fbf3a960a779
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325641
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>
Build-Review: Cody Cutrer <cody@instructure.com>
This commit squashes two previously seperate commits.
The individual test plans for each commit have been left intact.
Closes CNVS-59219,CNVS-59187
flag=site_admin_service_auth
Test Plan:
- Restart Canvas after pulling change
- Enable the flag
- Make valid requset to the token endponit using a
client_credentials grant. To be valid, the following
must be true:
- The client_id is a usable developer key global ID
- The client_secret is the api_key of the dev key
- The developer key has `internal_service: true`
- The developer key has an associated `service_user`
- Validate an acess token is returned
- Make an API request using the access token. Note
that you will need to set the User-Agent header to
something matching the Instructure service user
agent regexp. For example:
```
inst-service-ninety-nine/1234567890ABCDEF
```
- Tail web logs and validate the client identifier
use by request throttling middleware is:
```
service_user_key:<global developer key ID>"
```
Change-Id: I214823b708fedb3e811f123cb986a955f37b95c0
Allow blocking inst_access tokens by jti claims
Test Plan:
- Restart Canvas after pulling change
- Enable the flag
- Make valid requset to the token endponit using a
client_credentials grant. To be valid, the following
must be true:
- The client_id is a usable developer key global ID
- The client_secret is the api_key of the dev key
- The developer key has `internal_service: true`
- The developer key has an associated `service_user`
- Make an API request using the access token. Note
that you will need to set the User-Agent header to
something matching the Instructure service user
agent regexp. For example:
```
inst-service-ninety-nine/1234567890ABCDEF
```
- Tail web logs and validate the client identifier
use by request throttling middleware is:
```
service_user_key:<global developer key ID>"
```
- Decode the access token and note the `jti` claim value
- In a Canvas Rails console, add that jti value to
the request throttling blocklist:
```
Setting.set("request_throttle.blocklist", <jti value>)
```
- Attempt to use the token again and validate the http
response code is 403
- Attempt to make an API request with a standard access
token and validate throttling middlware does not raise
an error
- Attempt ot make a request with an active session and validate
throttling middleware does not raise an error
Change-Id: Ia8448094b7bf0281268bc3dd2d027bb934aa595c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/323766
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Daniel Matyas Vincze <daniel.vincze@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>
refs AE-380
a lot of these are APM style settings that would be okay to just turn
off while Consul is down. others are things that should just silently
not have any data, instead of failing the request
Change-Id: I34c553b089197f85b2d46029e5079851227090b6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/322239
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@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]
Change-Id: I6abefdfa9fed6dd4525c8786e93efa548b3710f2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/319603
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
Migration-Review: Jacob Burroughs <jburroughs@instructure.com>
refs QUIZ-10507
flag=none
Test plan:
- PreReqs
- Canvas with two shards
- An account with user on each shard
- Create an AccessToken for each user
- at1=AccessToken.create!(developer_key: DeveloperKey.create!(account:
account1)
- at2=AccessToken.create!(developer_key: DeveloperKey.create!(account:
account2))
- Run AccessToken.site_admin?(at1.full_token)
- Observe response is `true`
- Run AccessToken.site_admin?(at2.full_token)
- Observe response is `false`
- Configure an HTTP client with a key on site admin
- Add the key to the approvelist:
- Setting.set("request_throttle.approvelist", "service_user_key:<id>")
- Restart web container and connect to console and run commands:
- `GuardRail.activate!(:deploy)`
- `Rails.cache.redis.keys`
- Confirm no `request_throttling:` keys exist
- Send a Canvas API request from HTTP client
- run `Rails.cache.redis.keys` again
- Observe `request_throttling:service_user_key:<id>` key exists
- Configure HTTP client to use the non-site-admin key (at2)
- Send a Canvas API request from HTTP client
- run `Rails.cache.redis.keys` again
- Observe NO `request_throttling:service_user_key:<id>` key exists
Change-Id: I16e591a9062701849d792fe0293580999ae8a613
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/309604
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
QA-Review: Mark McDermott <mmcdermott@instructure.com>
Product-Review: Stephen Kacsmark <skacsmark@instructure.com>
closes AE-30
flag=none
test plan:
- verify Canvas boots in CD
- verify no influx of new errors in CD
[fsc-timeout=30]
Change-Id: Ifa04bebe1b09f01c6d3b8b2d8f3bb424759730f5
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/308067
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>
Build-Review: James Butters <jbutters@instructure.com>
closes INTEROP-7669
flag=none
Test plan:
- Open a rails console and run
GuardRail.activate!(:deploy)
Rails.cache.redis.keys
To see the list of redis keys.
- Hit an AGS or NRPS endpoint. Look at the list of keys and check that
there is now a key we use throttling redis keys based on the client ID
"request_throttling:lti_advantage:123-" where 123 is the developer key
ID.
- If you test on an MRA install where "shard.database_server_id" is not
empty, you will see that that key actually includes the
database_server_id.
Change-Id: Ied277f948df4885c345c3a2ee9dbfc505feec405
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/307178
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
Some clients calling Canvas are including their own sentry-trace header,
which is a header designed to associate traces from multiple services
together; if that header is present, Sentry decides that the transaction
should be sampled to maintain the association.
This change discards the `sentry-trace` header unless there is also a
`Referrer` header which contains the same host the request is for.
This change also switches to using a proc to define the APM sample rate,
which will allow the sample rate to be updated without restarting app
servers.
flag=none
closes DE-1029, DE-1028
test plan:
- verify that tracing can still be enabled/disabled by modifying the
sampling rate
- verify that calling Canvas with a `sentry-trace` header does not
override the sampling rate, unless a same-origin `Referrer` header is
also present
Change-Id: Iddc80c3234975db0e45fa8defcba8eda1f4e0d24
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/284485
Reviewed-by: Ryan Norton <rnorton@instructure.com>
Product-Review: Ryan Norton <rnorton@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@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>
app except for app/models, config
all manual. many cases I removed the unused argument entirely, and
updated callers to not send it
there were also a few "override this method and raise an exception so
you don't use it cause rails is changed" methods that were for old
rails versions that I just removed completely
Change-Id: I071a5a8266801427c5c7a157fefe14850495e620
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/276446
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>
instead of lower level on the adapter itself, because it causes surprises
when normally uncached methods are suddenly cached
also extend schema caching to jobs nodes
note that we can directly cache SchemaCache objects because it explicitly
implements marshal_dump, which is how Rails intends it to be used (just
with a file, instead of a cache store).
Change-Id: Id61d14ccc1fa9b44435c25f68063bbc5b6ddbaed
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/263004
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 abort the request
test plan:
* add something like `User.connection.execute("SELECT pg_sleep(30)")` to a
controller action like Login::CanvasController#new
* use curl to call it (`curl http://localhost:3000/login/canvas`)
* ctrl-c the curl request
* notice the server immediately "responds" with a 408, and check that
the query is no longer active in the db (with
`SELECT * FROM pg_stat_activity` and checking the `state` column;
the last query will still be `SELECT pg_sleep(30)`)
Change-Id: Iabd3268ef70e4f036ea1c0fc60865697b123a09a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/261781
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
refs FOO-1712
flag=none
- drop bad directories from autoloading (jsx)
- re-inflect some things that zeitwerk doesn't
know by default (InstFS, etc)
- add environment variable trigger for enabling
zeitwerk loading
- move samesite transition cookie to app middleware
so we don't try to reload middleware
- use zeitwerk.rb shim to pre-load things
that are not correctly loaded by their own
gems
TEST PLAN:
1) everything continues to work fine
2) for local development, if you use the
CANVAS_ZEITWERK env var, you get zeitwerk
autoloading (which is probably still broken)
Change-Id: I55a0db84034133240698bf4ff0cc8d225ec181a6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/260674
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
refs FOO-1649
Pull out middleware for request
context to a gem so that other
engines in canvas can use the
generator to look at the current
request for standard attributes
in the same way.
TEST PLAN:
1) requests should keep on getting context ids
2) sessions should keep getting added to the cookie jar
Change-Id: I9245491f722ac29c9544623ee14e0771ae248cd4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/259609
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
refs FOO-1648
flag=none
[fsc-propagate=false]
RequestContextGenerator depends on
the PageView model so that it can
extract these attributes. Pull
that responsibility out into canvas_security
to de-circularize that dependency chain.
TEST PLAN:
1) RequestContext page view token decoding
does exactly what it does right now
2) specs continue to pass
Change-Id: If9dfa923338f2b67490593771d58e9610f514923
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/259613
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
This reverts commit 89a9f28033.
refs FOO-949
Change-Id: Icca2563c670de8c7a0513d709aeb2c2541e37d3e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/248195
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: Jacob Burroughs <jburroughs@instructure.com>
so we know where they came from
Change-Id: I41d58d73c99d1187064f7d83821f361d3d73791f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/249256
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>
Cody Cutrer