Commit Graph

54 Commits

Author SHA1 Message Date
Ethan Vizitei 7c12fe1cbb wrap network errors to inst-fs in service exception
closes FOO-1467
flag=none

TEST PLAN:
  1) disable inst-fs
  2) network errors should get
    reported as service errors because
    we already have error paths for those.

Change-Id: Ib8cd10d05156b809e78602d24a6a6d92f10465ab
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/256728
Reviewed-by: Simon Williams <simon@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>
2021-01-14 18:17:37 +00:00
Nathan Mills 17dfc3af96 thumbnails: pass original_url, and skipping cache
refs SAS-1610

test plan:

* original_url should be in the thumbnail jwt, and include the no_cache
  param
* using the original_url with the no_cache param should generate a new
  thumbnail url jwt

flag = none

Change-Id: Ie1f5a01649bc6645f9766256d973d04d7ffeba9f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/255421
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Nathan Mills <nathanm@instructure.com>
Product-Review: Nathan Mills <nathanm@instructure.com>
2020-12-17 01:22:45 +00:00
Ethan Vizitei 7d190b3def gracefully handle inst-fs service errors
closes FOO-1327
flag=none

TEST PLAN:
  1) make inst-fs fail with 502
  2) upload something to canvas for SIS import
  3) user gets a 502

Change-Id: Id9c851b3246cd7b413f8a4428243b09d98477066
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/255072
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Charley Kline <ckline@instructure.com>
Product-Review: Charley Kline <ckline@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
2020-12-14 22:07:30 +00:00
Nathan Mills 5f0c9c13b7 add jti and original url to inst-fs thumbnail jwt
refs SAS-1610

test plan:

inst-fs thumbnail jwt tokens should have a "original_url" and "jti" claim

flag = none

Change-Id: I05c59584c8c165d358301d6d101545f4571a36f8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/255005
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Nathan Mills <nathanm@instructure.com>
Product-Review: Nathan Mills <nathanm@instructure.com>
2020-12-14 16:44:27 +00:00
Nathan Mills 4540d77073 add original_url to inst-fs jwt tokens
refs SAS-1610

test plan:

inst-fs jwt tokens should have a "original_url" claim

flag = none

Change-Id: I477d583ac7f9fd71e5e3233e5adc982d66d3c195
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/254239
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Levente Szabo <levente.szabo@instructure.com>
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Nathan Mills <nathanm@instructure.com>
Product-Review: Nathan Mills <nathanm@instructure.com>
2020-12-04 18:21:49 +00:00
Nathan Mills e2e4408995 add jti to the inst-fs tokens
flag = none

refs SAS-1578

test plan:
* inst-fs should still work
* there should be a jti in the verifier token

Change-Id: I8970bd6b4e4f54bb6e27a994250fdd2bd4ff75f7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/253771
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Nathan Mills <nathanm@instructure.com>
Product-Review: Nathan Mills <nathanm@instructure.com>
2020-11-30 21:42:58 +00:00
Ethan Vizitei ca6bebc9e4 inst_fs errors are comingling
refs FOO-1078
flag=none

Track errors with service infra
or behavior (500s) separately
from bad client behavior (400s)
in instfs integration.

Change-Id: I0da856232a89fdb9be002fa11e9c3e4582de4ae0
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/252736
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
2020-11-13 16:03:05 +00:00
Ethan Vizitei a9d9da13ad downgrade several expected error locations
refs FOO-1125
flag=non

TEST PLAN:
  1) stats for things like ImperiumTimeouts should still
     end up in datadog
  2) sentry errors for the target error types should disappear

Change-Id: I6e97c04e3f6fcc3545b10418511934c89f20a419
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251536
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
2020-10-30 16:46:01 +00:00
Cody Cutrer 06763dd519 add # frozen_string_literal: true for lib
Change-Id: I59b751cac52367a89e03f572477f0cf1d607b405
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251155
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>
2020-10-27 20:49:50 +00:00
Ethan Vizitei 0bf387d961 let attachment processing retry consul connection
refs FOO-965

Also, if we fail due to Imperium timeout
let's actually fail and not pass back
a nil value and then fail when trying to
treat it as a URL later.

Change-Id: Ibfaa95d9aa40012a97b10033f86499adc945fd50
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/247859
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
2020-09-17 14:24:57 +00:00
Jacob Fugal f71f1aa81d safari inst-fs service worker
closes SAS-1474, SAS-1452

canvas server-side, when:
  * a canvas request would generate a redirect to an inst-fs file
  * but the request also includes a `X-Canvas-File-Location` header
  * and the request is session authenticated

then the server instead responds with a JSON object with:
  * a `location` field containing the nominal redirect target
  * a `token` field containing a token that can be used as a value for
    an `Authorization: Bearer <token>` header when requesting the file

a service worker is then added that intercepts non-navigation GET
requests to those endpoints. it adds the X-Canvas-File-Location header
to the request, and then issues a followup request for the returned
location with the Authorization header added. in effect, it's following
the redirect but with the Authorization header injected. inst-fs can
then recognize the Authorization header as a means of user
authentication in the absence of cookies.

finally, the service worker is installed only if:
  * the plugin setting controlling it is enabled
  * the browser is Safari 13+

other browsers are able to use simpler work arounds when cookie blocking
is enabled and can thus take advantage otherwise.

test-plan:
 * have canvas and inst-fs both served over https
 * for safari, be using 13.1 or newer with tracking prevention enabled
 * leave the service worker disabled through the inst-fs plugin setting
 * have an image uploaded to inst-fs
 * visit the image's preview page in the files UI:
   - in safari: observe the image fails to load
   - in chrome: observe the image successfully loads
 * enable the service worker via the plugin setting
 * refresh the image's preview page:
   - in safari: observe the image now successfully loads
   - in chrome: the image still successfully loads, but does so without
     involving the service worker

Change-Id: Ie28f2cad40f67549bfbb4c7c6604f215581fbe18
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/237135
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
2020-07-02 22:39:11 +00:00
Jacob Fugal 40908c8c9d export legacy files to inst-fs on-demand
fixes SAS-1443

requires opt-in (ramping up rate in plugin setting, defaults to 0). will
initially opt in on sandbox accounts, then trial accounts, then a
gradual roll out to avoid hammering the service.

when opted in (100%), asking for a redirect target for a file that's not
yet in inst-fs will first try to add it to Inst-FS by posting the
metadata only (a nominally quick transaction). Inst-FS can use this to
create a "foreign" reference to the existing object in canvas' s3 bucket
(which inst-fs should be given access to), at which point it can
immediately start serving the object.

opting in at a level above 0% but below 100% will have the chosen chance
of doing the above per-access.

if successful, Inst-FS returns the new instfs_uuid with which the
attachment is updated, then the redirect continues to inst-fs instead of
s3. if unsuccessful, opted-out, or skipped due to opt-in threshold, the
existing s3 object is redirected to as before.

test-plan:
  (setup)
  - have inst-fs integrated locally
  - be using an s3 backing for non-instfs files
  - give inst-fs access to that s3 backing

  (testing)
  - turn off the inst-fs plugin setting
  - upload a file
  - turn on the inst-fs plugin setting, but leave the migrating flag
    unset
  - access the previously uploaded file, should be served from s3
  - turn on the migrating flag in the plugin setting
  - access the previously uploaded file, should be served (or at least
    attempted, if you haven't allowed inst-fs access to the canvas s3
    bucket) from inst-fs

Change-Id: I2b909d9b3f1c950e5a88f2af6ec7b40c05e71bb6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/234167
Reviewed-by: Michael Jasper <mjasper@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Jacob Fugal <jacob@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2020-04-15 19:43:58 +00:00
Jonathan Featherstone fa92af681d Add metadata url generation code to inst-fs
Inst-fs is beginning to implement object security scanning (malware
scanning). Status of the file scanning is included in the metadata
object. Metadata endpoint to inst-fs has been implemented, this is the
corresponding library integration.

Test Plan:
* Specs

refs SAS-1426

Change-Id: Ia6eb396420b272957842935bace545472aba9190
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/228123
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
Product-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
2020-02-28 18:47:13 +00:00
Jacob Fugal 96362cc600 allow multiple inst-fs secrets for key rotation
refs SAS-1215

test-plan:
- specs confirm existing behavior is still fine
- inst-fs secret can be set to a space separated list of multiple keys
  and:
  - signing still works as before, using the foremost key
  - validation of jwt to /api/v1/files/capture can use any of the keys

Change-Id: I0edc478ec07b44b197b5d9b066b3e0ef552966a8
Reviewed-on: https://gerrit.instructure.com/206932
Tested-by: Jenkins
Reviewed-by: Nathan Mills <nathanm@instructure.com>
QA-Review: Jacob Fugal <jacob@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2019-08-28 22:25:00 +00:00
James Williams e30f505827 fix file deletion and replacement for inst-fs
test plan:
* with inst-fs enabled, should be able to
 run `attachment.destroy_content_and_replace` in a
 rails console successfully
* it should replace the file with the tumbleweed doc
* should be able to run `attachment.resurrect_from_purgatory`
 in a rails console
* the file should be restored

closes #CORE-3166

Change-Id: If12f4e3d7db0ef5a019bcd251c45b91579857521
Reviewed-on: https://gerrit.instructure.com/200098
Tested-by: Jenkins
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
2019-08-01 12:30:48 +00:00
Ryan Shaw 4eb697d6c7 Don’t generate different inst-fs urls every time
Closes: CORE-2913

Right now, in canvas, every inst-fs url we generate will be unique.
This means that, for example, every time you go to your dashboard,
You will have to redownload all the course images for your all your
dashcards.

This is because we include Time.now in the jwt token we add as a query
string parameter on the url, and since time is always changing, so will
that token in the url.

This fix says: if tokens are valid for 24 hours, every time we generate
a url, pick a consistent time between now and 12 hours ago and say
the token was created at that time. And we make sure that the times used
are evenly distributed across that 12 hour window so there is never a
thundering herd of invalidations. (my example uses 24 and 12 hours
because the default expiration time is 24 hours, but the logic is
the same if you say it expires in is 2 hours or 24 hours, it just makes
sure that there is at least half of the availability time left before
it expires)

Note: this fixes the canvas side so we actually use the same url,
we still need to fix things on the inst-fs side so that they send a 
“304 not modified” when you send a request to a url that you already
have in your browser cache.


Test plan:
* once this is applied, in an environment that is set up to use inst-fs:
* as a user enrolled in at least one course that uses an inst-fs image
  as it’s dashcard image
* load the dashboard
* a few seconds later, load it again
* both requests should have used the same url to that image

Change-Id: Id059874ac3e7b51646f28028faad3cb8eda2a816
Reviewed-on: https://gerrit.instructure.com/192174
Tested-by: Jenkins
QA-Review: Brent Burgoyne <bburgoyne@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Brent Burgoyne <bburgoyne@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
Reviewed-by: Brent Burgoyne <bburgoyne@instructure.com>
2019-05-21 15:11:44 +00:00
Michael Jasper 692e3fee50 Inst-fs error messaging improvement
refs #SAS-794

test plan
- use InstFS.direct_upload through the console to either
  - make a bad request to inst-fs or
  - have inst-fs configured locally to always return a 300
- get an error raised that includes the response body (error message)
  from inst-fs

Change-Id: Icf03caba8d35e192e0ad22211103833de60bff04
Reviewed-on: https://gerrit.instructure.com/192707
Tested-by: Jenkins
QA-Review: Michael Jasper <mjasper@instructure.com>
Product-Review: Michael Jasper <mjasper@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
2019-05-09 15:20:19 +00:00
Jacob Fugal 3d2bc63a48 prefer display name over filename, but encode it
fixes SAS-418, SAS-338

inst-fs will read the segment of the path following the UUID and attempt
to use it as the filename in the content-disposition header when it
serves the file. prefer using the display name over the filename for
that, so that when if the file is served as a download (vs. inline) it
is saved according to the display name on the user's machine. but we
need to make sure to URI encode it before putting it in the path, in
case of any special characters in the display name.

test-plan:
 - have inst-fs enabled
 - upload a file
 - rename the file
 - download the file; should save according to the new name, not the
   original name
 - rename the file to have special characters in it (spaces,
   URI-specific symbols like "?" or "/", non-UTF-8 characters, etc.)
 - download the file; should save according to the new name, but the URL
   downloaded from (i.e. check the inst-fs URL redirected to) should
   have those characters escaped

Change-Id: Ic6fb3cbafffb51791605c7a6ba2e29117213576e
Reviewed-on: https://gerrit.instructure.com/174739
Tested-by: Jenkins
Reviewed-by: Michael Jasper <mjasper@instructure.com>
QA-Review: Michael Jasper <mjasper@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-12-11 16:42:08 +00:00
James Williams 03ed1fcbb8 don't show login pixel with pending otp
closes #CORE-2145

Change-Id: I11adbcc0be236325db0ebf4f4e8a94cbe7468d15
Reviewed-on: https://gerrit.instructure.com/172693
Reviewed-by: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
2018-11-19 12:21:31 +00:00
Andrew Huff 10cc439f18 Revert "fix display name url encoding in inst-fs"
refs RECNVS-322
refs RECNVS-756
refs ADMIN-1573

The previous commit fixed the problem with inst-fs (RECNVS-756)
but it broke other stuff; see ADMIN-1573 and
https://zach.beta.instructure.com/error_reports/500071076

We'll fix the inst-fs problem in an upcoming ticket to that project

This reverts commit 121e62f5d3.

Change-Id: I1cd01787052058751e7f57e599273c691e5b53e0
Reviewed-on: https://gerrit.instructure.com/172286
Tested-by: Jenkins
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Andrew Huff <ahuff@instructure.com>
Product-Review: Andrew Huff <ahuff@instructure.com>
2018-11-14 04:16:22 +00:00
Andrew Huff 121e62f5d3 fix display name url encoding in inst-fs
closes RECNVS-756

When files from inst-fs were downloaded, they were saved onto
the user's local machine with their name set to the value of
their `filename` column in the canvas database. We want it to
be set to `display_name` instead.

test plan:
* enable inst-fs on your local canvas instance
* upload a file with spaces in its name; e.g. "hello world.png"
* donwload the file to your computer
* verify that the downloaded file is named "hello world.png" instead of
  "hello+world.png"

Change-Id: I03776fbd09cf53a574349663ac2fee5e3b4e0b8b
Reviewed-on: https://gerrit.instructure.com/168742
Tested-by: Jenkins
Reviewed-by: Michael Jasper <mjasper@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
Product-Review: Andrew Huff <ahuff@instructure.com>
2018-10-24 19:14:10 +00:00
Jacob Fugal 815821a762 restore and fix "stream inst-fs direct uploads"
fixes RECNVS-583, RECNVS-594, RECNVS-595

reverts commit efdb13e9a9, restoring
3da400a75e, but adds fixes for the bugs
that caused the original revert. in particular:

* refactor to avoid `eof?` (RECNVS-594)

  in some situations, the file contents stream doesn't implement `eof?`.
  the only required interface for `IO.copy_stream` is `size` and `read`,
  so we'll restrict our interface to that and only expect no more than
  that interface of the parts.

* keep trying the same stream on partial read (RECNVS-595)

  the IO#read docs indicate that if the read result has fewer than the
  bytes requested, it's because the stream is at EOF. the
  SequencedStream output sticks to that behavior. but in some
  situations, the file contents stream will return a partial read before
  EOF. so we need to keep asking for the remainder until we either get
  as much as we wanted or an explicit EOF (nil returned).

test-plan:
- ensure the following all still work with inst-fs enabled:
  - content exports, particularly of larger courses. don't just verify
    the file is created and can be downloaded from inst-fs, but the
    generated file should also be a valid zip file (file extension is
    imscc, but format is zip) that can be extracted into the expected
    course contents. exercises RECNVS-595
  - content migrations, particularly of larger courses, where you choose
    a source Canvas course when executing "Import Course Content" in a
    destination Canvas course. also exercises RECNVS-595.
  - SIS imports via API
    - create a simple courses.csv with a new course defined in it a la
      https://canvas.instructure.com/doc/api/file.sis_csv.html
    - POST this CSV to the SIS imports API documented at
      https://canvas.instructure.com/doc/api/sis_imports.html#method.sis_imports_api.create
      use the mechanism described under the text "If you decide to do a raw post,
      you can skip the 'attachment' argument..." in the description of
      the attachment parameter. exercises RECNVS-594.

Change-Id: Ie7e7fecff491ac2344a48d97f1c6b27eb24885c8
Reviewed-on: https://gerrit.instructure.com/162584
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
Tested-by: Jenkins
Product-Review: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
2018-08-30 18:37:07 +00:00
Jacob Fugal 0511ebf24a skip inst-fs pixel during other oauth flow
fixes CORE-1744

for example, when the other app's oauth flow is on the confirm page.
they'll have info in session[:oauth2] that would get overwritten during
the pixel's initiated oauth flow. we can wait until their down and
display the pixel on the next page after that.

test-plan:
- have inst-fs enabled
- have another app using oauth against canvas which _isn't_ trusted
  (will show confirm page)
- in an incognito window (so a login will be forced), try and
  authenticate the other app, triggering an oauth flow against canvas
- when the confirm page is displayed for that app, the inst-fs pixel
  should not be included
- when the confirm button is clicked, the oauth of the other app
  concludes successfully
- the next canvas page viewed displays the inst-fs pixel instead

Change-Id: I9b80145dfc79e393a3b6fe17c3b62566e392416c
Reviewed-on: https://gerrit.instructure.com/161567
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: Michael Jasper <mjasper@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-08-28 17:37:33 +00:00
Jonathan Featherstone efdb13e9a9 Revert "stream inst-fs direct uploads"
This reverts commit 3da400a75e.

closes RECNVS-592

Change-Id: I51fb152a1ceef45a51b4434575afd1b25a955aa5
Reviewed-on: https://gerrit.instructure.com/162120
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Tested-by: Jenkins
Product-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
2018-08-25 16:18:04 +00:00
Jacob Fugal 3da400a75e stream inst-fs direct uploads
fixes RECNVS-547

generally, in Multipart::Post, you can get a stream representation of
the body instead of a string, and in CanvasHttp.post you can request
that stream be supplied to the underlying request's body_stream instead
of the string to the underlying body. inst-fs then supplies the
streaming flag during direct upload.

this avoids an error when the body as a single string is too large to
POST.

test-plan:
- confirm general upload-from-canvas functionality is preserved by
  triggering uploads from canvas to another server
  - e.g. inst-fs enabled course export
  - e.g. inst-fs enabled course export
- confirm specific upload-from-canvas functionality with streamed larger
  files
  - i.e. inst-fs enabled course export where the export size is larger
    than 2GB
  - note that there may be a new HTTP 504 error when the upload size
    approaches 3GB. this is a separate issue not related to this commit

Change-Id: I15e22e6688233a77783d1cd86960bbaaa64eb0dc
Reviewed-on: https://gerrit.instructure.com/160105
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: Michael Jasper <mjasper@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-08-20 22:00:35 +00:00
Xander Moffatt 7a4b731cdb reference instfs_uuid from instfs direct upload
refs RECNVS-521

* previously instfs file upload returned only the
instfs uuid as `uuid`
* instfs file upload now returns canvas capture
data, which includes the canvas uuid
* instfs uuid has been renamed to `instfs_uuid`

test plan:
* specs

Change-Id: I1f77f3dd1952fbd275ae67c399ff04ef6533785b
Reviewed-on: https://gerrit.instructure.com/157118
Reviewed-by: Michael Jasper <mjasper@instructure.com>
Tested-by: Jenkins
Product-Review: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
2018-07-12 16:13:06 +00:00
Nick Houle 8cdf400ba7 Implement Inst-fs capture callback for url uploads.
Adds an alert when the student submits stating that the upload is
processing and an email will be sent.

THe FilesController#api_capture called by InstFS, when the upload
processing is finished, uses Services::SubmitHomeworkService#submit
to submit the assignment.

Update the JS to prevent the browser from submitting the url upload
assignment twice. Passes the submit_assignment parameter to provide
backwards compatibilty, so the old JS can still double submit.

fixes: GOOF-449, GOOF-450, GOOF-435

Test Plan:
* Submit a file from the Google Drive, will see a pop up.
* Check the xhr for the request, the submit_assignment param will be true
* InstFS callback will cause the assignment to be submitted.

Change-Id: I5f49b8ef642fe5913bf0c2f282de8d697dcf1ce1
Reviewed-on: https://gerrit.instructure.com/154757
Reviewed-by: Nick Houle <nhoule@instructure.com>
QA-Review: Tyler Belnap <tyler@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Jesse Poulos <jpoulos@instructure.com>
Tested-by: Jenkins
Reviewed-by: Josh Orr <jgorr@instructure.com>
2018-07-10 19:56:27 +00:00
Jacob Fugal 7f301937ea legacy api claims in inst-fs jwts for API clients
fixes RECNVS-471
fixes RECNVS-479

allows API requests from access tokens issued by whitelisted developer
keys to receive additional claims in the JWTs of inst-fs links in the
response. these additional claims are a workaround to cause inst-fs to
accept the link as if authenticated despite the client not having an
inst-fs session or presenting inst-fs with the access token.

updated API clients will need to present their API token when accessing
inst-fs links. once the clients associated with a developer key are
updated, the developer key will be removed from the whitelist. this is
only a temporary workaround.

test-plan:
- have inst-fs configured and enabled with your canvas instance
- generate a new access token for your user
- in the rails console of your canvas instance, set:
  Setting.set('instfs.whitelist_all_developer_keys', 'true')
- using something without a session, like postman, POST to
  /api/v1/courses/:course_id/files with a valid preflight and
  authenticated via the access token (e.g. using the `Authorization`
  header)
  - the `upload_url` in the response should be an inst-fs link
  - the `upload_url` should include a `token` query parameter with a JWT
    as the value
  - decoding the JWT from the `upload_url`, it should include
    `legacy_api_developer_key_id` and a `legacy_api_root_account_id`
    claims
- in the rails console of your canvas instance:
  Setting.remove('instfs.whitelist_all_developer_keys')
- repeat the upload preflight attempt from above
- this time, the JWT should not include the `legacy_api_*` claims

Change-Id: I911d18c031d9ba90de808e260e4644beaef69ff9
Reviewed-on: https://gerrit.instructure.com/151690
Tested-by: Jenkins
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-06-04 17:04:06 +00:00
Michael Jasper ceb0f5ec75 handle missing setting values properly
closes RECNVS-440

test plan
- enable the instfs plugin with canvas, but have missing setting
  values (app-host, secret)
- actions with respect to instfs should not cause errors
  - login, logout, file upload
- canvas should treat instfs as it is not enabled if setting values
  are not set

Change-Id: I2a33a553c6d66e99fd85e8333b8d8a3b60124d10
Reviewed-on: https://gerrit.instructure.com/150575
Reviewed-by: Andrew Huff <ahuff@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Tested-by: Jenkins
Product-Review: Michael Jasper <mjasper@instructure.com>
2018-05-30 19:03:30 +00:00
Xander Moffatt ebc0faa4ca generate instfs JWTs with HS512 algorithm
closes RECNVS-390
refs RECNVS-387

test plan:
* smoke test that instfs still works

Change-Id: Id0a75fa25d293244e26d30c757b80f6c97f907f8
Reviewed-on: https://gerrit.instructure.com/150615
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Tested-by: Jenkins
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Xander Moffatt <xmoffatt@instructure.com>
2018-05-23 16:54:19 +00:00
Brent Burgoyne ce3379a3ac add success include param for upload preflight
refs CORE-20

test plan:
- test file upload api with success_include[]=avatar on the preflight
  request
- the final response on success should include the avatar property
- test with local storage, s3, and InstFS

Change-Id: I974197944d0f84ad0b89a628ab8604f50cdec45e
Reviewed-on: https://gerrit.instructure.com/144456
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Tested-by: Jenkins
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Product-Review: Brent Burgoyne <bburgoyne@instructure.com>
2018-04-04 19:45:50 +00:00
Andrew Huff 5e14d9f097 fix/unify resource claim sent to instfs
refs RECNVS-384

test plan: none (will be tested during an inst-fs ticket)

Change-Id: I75565846743d258a9d1abae8f799e72e8f205600
Reviewed-on: https://gerrit.instructure.com/144737
Tested-by: Jenkins
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Andrew Huff <ahuff@instructure.com>
2018-03-28 17:09:49 +00:00
Michael Jasper 816c4bad2d Upload attachment to inst-fs, eportfolio export
closes RECNVS-362
closes RECNVS-363

Test plan:
- with instfs configured and enabled:
  - log into canvas and navigate to the users eportfolio
  - dowload the eportfolio
  - verify that the eportfolio was
    - created successfully
    - download from the <instfs domain>/files
  - verify that the instfs logs show
    - a post of the eportfolio to instfs
    - a get of the eportfolio from instfs

Change-Id: Ia27ca9811598cea180432e730ae8d5163a720036
Reviewed-on: https://gerrit.instructure.com/143050
Tested-by: Jenkins
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Michael Jasper <mjasper@instructure.com>
2018-03-21 19:45:35 +00:00
Jacob Fugal 2bdca4957e consider request host when choosing oauth host
fixes RECNVS-361

instead of just using Attachment.current_root_account.domain

to facilitate the change, all generation of non-public links to external
file storage go through a FileAuthenticator instance that holds the
necessary information about the current user and oauth host

test-plan:
[locally]
- smoke test file uploads and downloads, there should be no impact on
  behavior, just a refactor
[once on beta]
- enable inst-fs in your sandbox
- logout of canvas
- open network logging in your browser dev tools and log back in to
  canvas
- identify the requests related to the inst-fs login pixel and
  corresponding oauth redirects
- the oauth should have occurred against your beta sandbox, not your
  production sandbox
- attempt to upload a file; should be successful

Change-Id: Ic859b707908baef84f5ee4dba29f18bdd841abcc
Reviewed-on: https://gerrit.instructure.com/143930
Tested-by: Jenkins
Reviewed-by: Michael Jasper <mjasper@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-03-20 04:25:18 +00:00
Andrew Huff c15f938cb7 canvas upload via url
closes RECNVS-329

test plan: (joint with g/142642)

Change-Id: I32df18bdc91451e67bb83fbcc68b3f0629444ad6
Reviewed-on: https://gerrit.instructure.com/142732
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Tested-by: Jenkins
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-03-11 19:37:47 +00:00
Jacob Fugal 14d54e7eb3 export attachments to instfs
closes RECNVS-43

This datafixup finds attachments which do not have an instfs_uuid,
creates `references` for them, posts them to instfs /references
endpoint. The endpoint imports them into instfs and returns an array
of the references with their new uuids. The attachments are updated
to include the uuids from instfs

Test plan
- turn on s3 storage for files in canvas
- with inst-fs off in canvas (deactivate the plugin), upload a file
- Enable the inst-fs plugin
- in the rails console, run
  `DataFixup::ExportAttachmentsToInstfs.run(Account.find(1))`
  (or whatever your account id is)
- verify using instfs /debug/attachments/ls route that the attachment
  was posted
  to instfs
- verify using the rails console that the attachment has an instfs_uuid

Change-Id: I26720c2324b7302a3a843f04932785518ac699e7
Reviewed-on: https://gerrit.instructure.com/142572
Tested-by: Jenkins
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Michael Jasper <mjasper@instructure.com>
Reviewed-by: Michael Jasper <mjasper@instructure.com>
2018-03-07 19:38:50 +00:00
Jacob Fugal 171ef86f81 set Attachment.current_root_account during jobs
and rename that suite of methods to make more sense

test-plan: N/A

Change-Id: Iffc520ea55141ac47da669663838a4d3c3d8712c
Reviewed-on: https://gerrit.instructure.com/142486
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
QA-Review: Jacob Fugal <jacob@instructure.com>
2018-03-07 17:18:24 +00:00
Jacob Fugal e60143748d user real user when available in instfs JWTs
refs RECNVS-264

but still pass the @current_user for access and upload JWTs, so that it
can go in a separate claim if its different

test-plan:
- smoke test an inst-fs upload. should still work

Change-Id: I4be9cd4049c83848e69aae37500ae9f4b96392b4
Reviewed-on: https://gerrit.instructure.com/142334
Tested-by: Jenkins
Reviewed-by: Michael Jasper <mjasper@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-03-06 20:44:27 +00:00
Michael Jasper f92b17b174 Directly upload to inst-fs from canvas
closes RECNVS-324

test plan
- use the `InstFS.direct_upload` method to upload files to canvas through the
  the rails console
  - an example of a rails console command is documented in the method
  - verify that uploads from canvas are available in inst-fs

Change-Id: I20e5d2544a0853fd913ebcba5229e9977825f482
Reviewed-on: https://gerrit.instructure.com/142411
Tested-by: Jenkins
QA-Review: Collin Parrish <cparrish@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-03-06 18:02:10 +00:00
Michael Jasper a891136f20 Add host claim to inst-fs JWT
refs RECNVS-313

test-plan:
- with inst-fs enabled, do the following:
  - login, upload a file, access a file
- in each case the JWT serialized in the url should contain a
  `host` claim which is the oauth host (domain of the root account)

Change-Id: I05e833407bfd6a3d6cb0dcb95d42d446e314d224
Reviewed-on: https://gerrit.instructure.com/141584
Tested-by: Jenkins
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Michael Jasper <mjasper@instructure.com>
2018-02-28 17:32:59 +00:00
Jacob Fugal edcef8d4a8 logout of instfs during explicit canvas logout
fixes RECNVS-248

test-plan:
  [happy path]
  - have instfs configured and enabled
  - login to canvas
  - logout of canvas (do not expect instfs cookie to be removed)
  - attempt to access <instfs>/session/ensure; should get oauth
    redirected to canvas login despite existing cookie
  - login to canvas anew
  - attempt to access <instfs>/session/ensure; should get 204 response

  [no instfs path]
  - have instfs disabled
  - logout of canvas; should not get a page error or other unexpected
    behavior

  [broken instfs path]
  - have instfs configured and enabled in canvas, but service down
  - logout of canvas; should not get a page error or other unexpected
    behavior, but should still see an error report for the failure to
    logout of instfs

Change-Id: Ibb4c846cfe48151df3dab04bf8fde39c2738ee4a
Reviewed-on: https://gerrit.instructure.com/141372
Tested-by: Jenkins
Reviewed-by: Andrew Huff <ahuff@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-02-21 21:11:57 +00:00
Jacob Fugal cfe8da7ee5 only display instfs pixel on enabled accounts
fixes RECNVS-319

test-plan:
- have inst-fs configured with REQUIRE_AUTH=true
- have canvas _configured_ for inst-fs, but inst-fs not enabled
- in safari:
  - delete any inst-fs cookies and log out of canvas
  - log in
    - inst-fs login pixel should not be displayed (verify with network
      request log in browser)
    - no inst-fs cookie should have been created
  - enable inst-fs
  - refresh the page
    - inst-fs login pixel should be displayed
    - inst-fs cookie should have been created

Change-Id: Idecb926e8f17b5c29948b38fd8b5afc6c298d78a
Reviewed-on: https://gerrit.instructure.com/140938
Reviewed-by: Andrew Huff <ahuff@instructure.com>
Tested-by: Jenkins
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
2018-02-15 01:15:44 +00:00
Jacob Fugal bb4cea2f06 instfs login pixel, send cookies on instfs upload
refs RECNVS-106, fixes RECNVS-257

test-plan:
[upload sends inst-fs credentials]
 - set `REQUIRE_AUTH: 'true'` in your inst-fs docker-compose.local.yml
   and ensure your jwt secret and oauth flow are synced between canvas
   and inst-fs
 - turn on inst-fs in canvas
 - delete any cookies on the api.instfs.docker domain
 - login to canvas and upload a file on the user's files page, should
   work
[regression]
 - turn inst-fs back off and configure canvas for S3 storage
 - upload a file on the user's files page, should work
 - configure canvas for local storage, repeat

Change-Id: If646a6630554da5778852d8eb1abc976cd77886d
Reviewed-on: https://gerrit.instructure.com/138539
Reviewed-by: Andrew Huff <ahuff@instructure.com>
Tested-by: Jenkins
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-01-27 00:11:48 +00:00
Jacob Fugal a91a9fbf59 handle uncommon contexts inst-fs uploads
refs RECNVS-257

test-plan:
* with inst-fs enabled and configured
* attempt to import a content export; import job will fail but upload of
  import file should succeed
* attempt to upload a file to a "File Upload" question in a quiz; should
  succeed

Change-Id: I98174652ae25162cf703560090535acc5f815f02
Reviewed-on: https://gerrit.instructure.com/138538
Tested-by: Jenkins
Reviewed-by: Andrew Huff <ahuff@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-01-22 20:55:37 +00:00
Jacob Fugal aa21611939 Revert "instfs login pixel, send cookies on instfs upload"
This reverts commit 554f6cddc7.

Specifying 'withCredentials: true' in the XHRs breaks uploads to S3 in
its current CORS configuration.

Change-Id: Ic9e5571f736f2165ef140fd9da55aac7d01d2502
Reviewed-on: https://gerrit.instructure.com/137560
Reviewed-by: Andrew Huff <ahuff@instructure.com>
Tested-by: Jenkins
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-01-11 21:08:43 +00:00
Jacob Fugal b29eb06383 involve user in generating non-public links
fixes RECNVS-12

and make public links explicit. note that for non-inst-fs file storage,
the user parameter to the existing authenticated_url method is unused.
so for non-inst-fs, the following sets of methods are equivalent:

 * authenticated_url_for_user(*) == public_url == authenticated_url
 * download_url_for_user(*) == public_download_url (was download_url)
 * inline_url_for_user(*) == public_inline_url (was inline_url)

the choice of `public_...` over `..._for_user` methods in the refactoring
should thus be a no-op except when inst-fs is enabled. with inst-fs enabled,
the `public_...` methods produce URLs usable by any user (including
those not logged in!); this matches non-inst-fs behavior. the
`..._for_user` methods produce URLs usable only by the user for whom
they were generated, and should be preferred where public access is not
necessary.

after this refactor, make public links for google doc previews short
lived and consolidate some code around google doc preview links.

test-plan:
 - enable inst-fs
 [per-user inst-fs JWTs]
   - have a student and a teacher in a course with an assignment
   - as the teacher upload an image to the course files, then add the
     image to the course syllabus
   - as the student, attempt to view the course syllabus. should see the
     image in the course syllabus (prior to this commit would fail)
   - copy the inst-fs URL the student's browser was redirected to when
     viewing the file
   - as the teacher attempt to access the image directly using the
     student's inst-fs URL; should fail
 [public inst-fs JWTs]
   - as the teacher, upload a course card image (example of public file)
   - as the teacher, view the course card image and copy the inst-fs URL
     redirected to
   - as the student, attempt to access the course card image directly
     using the copied inst-fs URL; should succeed
 [google docs preview]
   - disable canvadocs on the account if enabled
   - as the teacher, upload a PDF to the course files
   - find the PDF in the course files and preview it
   - preview should be displayed via embedded google docs iframe
   - preview should succeed

Change-Id: I8384cbb89f1522022e2f06579e6381de5ed0076c
Reviewed-on: https://gerrit.instructure.com/133889
Tested-by: Jenkins
Reviewed-by: Andrew Huff <ahuff@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-01-09 22:21:38 +00:00
Andrew Huff 13c2d0b7d2 add root account id to instfs preflight json
closes RECNVS-38

test plan: none
(this commit will be tested as a part of RECNVS-39)

Change-Id: I39c39974c8f9553af5057e159c419cd84397caf8
Reviewed-on: https://gerrit.instructure.com/136940
Tested-by: Jenkins
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Andrew Huff <ahuff@instructure.com>
2018-01-09 20:02:53 +00:00
Jacob Fugal 554f6cddc7 instfs login pixel, send cookies on instfs upload
refs RECNVS-106

test-plan: (combined)

Change-Id: I080133aedca4e34262321790db604a98a2408d40
Reviewed-on: https://gerrit.instructure.com/137161
Reviewed-by: Andrew Huff <ahuff@instructure.com>
Tested-by: Jenkins
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2018-01-09 15:53:25 +00:00
Andrew Huff d2363d7b4d use global ids in InstFS JWTs
refs CNVS-39071

test plan: none

Change-Id: I8402ef121d86c24a626b5cec830e37ede2875522
Reviewed-on: https://gerrit.instructure.com/129522
Tested-by: Jenkins
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Product-Review: Andrew Huff <ahuff@instructure.com>
QA-Review: Andrew Huff <ahuff@instructure.com>
2017-10-16 19:42:20 +00:00
Jacob Fugal 328c1648dd request thumbnails from instfs when enabled
fixes CNVS-39058, CNVS-39061

Canvas doesn't need to store any additional metadata about instfs
thumbnails; the only metadata it uses about its legacy generated
thumbnails is the url (whether local or s3). we can just generate
thumbnail urls direct to instfs and forgo the thumbnail model
altogether.

test-plan:
- enable instfs
- upload an image to Canvas
- the uploaded image should have a thumbnail in the UI same as a
  non-instfs file before this commit, but the thumbnail should be being
  read from instfs as well
- regression test thumbnails with instfs disabled

Change-Id: I03c21be89469238de7efa317753763834d41c86c
Reviewed-on: https://gerrit.instructure.com/128180
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
Tested-by: Jenkins
QA-Review: Collin Parrish <cparrish@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
2017-10-03 20:00:08 +00:00