Skip to content

Commit

Permalink
Update vulnerable dependencies
Browse files Browse the repository at this point in the history
Updates dependencies that have outstanding CVEs reported by npm audit.
Specifically:
  - Bumped node-jose from 1.x to 2.x due to critical vuln
  - Replaced request and request-promise-native with
    node-fetch since the former is deprecated
  - Replaced isomorphic-fetch polyfill with node-fetch (recommended by
    unsplash-js and isomorphic-fetch was already using node-fetch)
  - Bumped morgan from 1.9.0 to 1.10.0 due to moderate vuln

Leaving dev dependencies alone for now, except for:
  - Bumped mocha from 5.2.0 to 8.2.1 for better configuration support

Also note that we are now attaching fetch to the global object since
unsplash-js requires it to be there.

closes LS-1523
flag = rce_enhancements

Test plan:
  Ensure that all interactions between both old RCE and new RCE with
  the RCS still work as expected. Specifically:
  - Viewing/adding/editing links to:
    * Pages
    * Assignments
    * Quizzes
    * Announcements
    * Discussions
    * Modules
  - Viewing and uploading files
  - Viewing and uploading images
  - Searching for and inserting images from Flickr (old RCE only)
  - Searching for and inserting images from Unsplash
  - Viewing, recording, and uploading media via Kaltura
  - Searching for and inserting YouTube videos (old RCE only)

Change-Id: I585b86b87ae2e6cdb16dc3d12062959ec78c5565
Reviewed-on: https://gerrit.instructure.com/c/canvas-rce-api/+/254605
Tested-by: Service Cloud Jenkins <[email protected]>
Reviewed-by: Ed Schiebel <[email protected]>
QA-Review: Ed Schiebel <[email protected]>
Product-Review: Jeff Largent <[email protected]>
  • Loading branch information
rojlarge committed Jan 7, 2021
1 parent d0c54cd commit 3304dac
Show file tree
Hide file tree
Showing 12 changed files with 840 additions and 525 deletions.
9 changes: 7 additions & 2 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ NODE_ENV=development
STATSD_HOST=127.0.0.1
STATSD_PORT=8125
STATS_PREFIX=rceapi
ECOSYSTEM_SECRET="astringthatisactually32byteslong"
ECOSYSTEM_KEY="astringthatisactually32byteslong"
CIPHER_PASSWORD=TEMP_PASSWORD
ECOSYSTEM_SECRET=astringthatisactually32byteslong
ECOSYSTEM_KEY=astringthatisactually32byteslong
FLICKR_API_KEY=fake_key
UNSPLASH_APP_NAME=canvas-rce-api-dev
UNSPLASH_APP_ID=fake_app_id
UNSPLASH_SECRET=fake_secret
YOUTUBE_API_KEY=fake_key
3 changes: 3 additions & 0 deletions app.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
"use strict";

require("dotenv").config();
// unsplash-js requires fetch to be available in the global scope
// see: https://github.com/unsplash/unsplash-js#adding-polyfills
global.fetch = require("node-fetch");

const container = require("./app/container");
const _application = require("./app/application");
Expand Down
60 changes: 25 additions & 35 deletions app/api/canvasProxy.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
"use strict";

const request = require("request-promise-native");
const crypto = require("crypto");
const parseLinkHeader = require("parse-link-header");
const sign = require("../utils/sign");
const { parseFetchResponse } = require("../utils/fetch");

function signatureFor(string_to_sign) {
const shared_secret = process.env.ECOSYSTEM_SECRET;
const hmac = crypto.createHmac("sha512", shared_secret);
hmac.write(string_to_sign);
hmac.end();
const hmacString = hmac.read();
return new Buffer(hmacString).toString("base64");
return Buffer.from(hmacString).toString("base64");
}

function requestHeaders(tokenString, req) {
const reqIdSignature = signatureFor(req.id);
return {
Authorization: "Bearer " + tokenString,
"User-Agent": req.get("User-Agent"),
"X-Request-Context-Id": new Buffer(req.id).toString("base64"),
"X-Request-Context-Id": Buffer.from(req.id).toString("base64"),
"X-Request-Context-Signature": reqIdSignature
};
}
Expand All @@ -30,7 +30,7 @@ function parseBookmark(response) {
// request downcases all headers
const header = response.headers.link;
if (header) {
const links = parseLinkHeader(header);
const links = parseLinkHeader(header[0]);
if (links.next) {
response.bookmark = sign.sign(links.next.url);
}
Expand All @@ -55,41 +55,31 @@ function catchStatusCodeError(err) {
}

function fetch(url, req, tokenString) {
const headers = requestHeaders(tokenString, req);
return collectStats(req, () =>
request({
url,
headers,
resolveWithFullResponse: true,
json: true
})
)
.catch(catchStatusCodeError)
.then(parseBookmark);
global
.fetch(url, {
headers: requestHeaders(tokenString, req)
})
.then(parseFetchResponse)
.then(parseBookmark)
.catch(catchStatusCodeError)
);
}

function send(method, url, req, tokenString, body) {
const headers = requestHeaders(tokenString, req);
return collectStats(req, () => {
const params = {
method,
url,
headers,
qsStringifyOptions: { arrayFormat: "brackets" },
resolveWithFullResponse: true
};
if (typeof body === "string") {
params.body = body;
} else {
params.form = body;
}
return request(params);
})
.catch(catchStatusCodeError)
.then(response => {
response.body = JSON.parse(response.body);
return response;
});
return collectStats(req, () =>
global
.fetch(url, {
method,
headers: {
...requestHeaders(tokenString, req),
"Content-Type": "application/json"
},
body: typeof body === "string" ? body : JSON.stringify(body)
})
.then(parseFetchResponse)
.catch(catchStatusCodeError)
);
}

module.exports = { fetch, send };
8 changes: 3 additions & 5 deletions app/api/flickrSearch.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";

const request = require("request-promise-native");
const { parseFetchResponse } = require("../utils/fetch");

const flickrBase = "https://api.flickr.com/services/rest";
// extras=needs_interstitial is required to get undocumented needs_interstitial
Expand All @@ -13,7 +13,7 @@ function getFlickrResults(searchTerm) {
const encodedTerm = encodeURIComponent(searchTerm);
const queryAddendum = `api_key=${flickrKey}&text=${encodedTerm}`;
const url = `${flickrBase}?${flickrQuery}&${queryAddendum}`;
return request.get({ url, resolveWithFullResponse: true, json: true });
return global.fetch(url).then(parseFetchResponse);
}

function transformSearchResults(results) {
Expand All @@ -25,9 +25,7 @@ function transformSearchResults(results) {
// nsfw results come through in the future.
.filter(photo => photo.needs_interstitial != 1)
.map(photo => {
const url = `https://farm${photo.farm}.static.flickr.com/${
photo.server
}/${photo.id}_${photo.secret}.jpg`;
const url = `https://farm${photo.farm}.static.flickr.com/${photo.server}/${photo.id}_${photo.secret}.jpg`;
const link = `https://www.flickr.com/photos/${photo.owner}/${photo.id}`;
return {
id: photo.id,
Expand Down
20 changes: 10 additions & 10 deletions app/api/unsplash.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
"use strict";

require("isomorphic-fetch");
const toJson = require("unsplash-js").toJson;
const request = require("request-promise-native");
const _unsplash = require("../services/unsplash");
const _env = require("../env");

Expand Down Expand Up @@ -73,15 +71,17 @@ class UnsplashController {
// an extra request since the SDK requires doing an initial get request
// prior to then doing a "download" request.
try {
await request({
uri: `https://api.unsplash.com/photos/${imageId}/download`,
headers: {
Authorization: `Client-ID ${this.env.get(
"UNSPLASH_APP_ID",
() => "fake_app_id"
)}`
await global.fetch(
`https://api.unsplash.com/photos/${imageId}/download`,
{
headers: {
Authorization: `Client-ID ${this.env.get(
"UNSPLASH_APP_ID",
() => "fake_app_id"
)}`
}
}
});
);
// The Unsplash API gives back an image URL, but it's unnecessary so
// we just send back an OK response with no body to save a few bytes
// across the wire.
Expand Down
4 changes: 2 additions & 2 deletions app/api/youTubeApi.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";

const request = require("request-promise-native");
const { parseFetchResponse } = require("../utils/fetch");

const ytApiBase = "https://content.googleapis.com/youtube/v3/search";
const ytApiQuery = "part=snippet&maxResults=2";
Expand All @@ -13,7 +13,7 @@ function getYouTubeUrl(vid_id) {

function fetchYouTubeTitle(vid_id) {
const url = getYouTubeUrl(vid_id);
return request.get({ url, resolveWithFullResponse: true, json: true });
return global.fetch(url).then(parseFetchResponse);
}

function parseTitle(vidId, results) {
Expand Down
26 changes: 26 additions & 0 deletions app/utils/fetch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"use strict";

// Translates a fetch-style response into a request-style
// response that the proxied routes expect. Specifically,
// we need to be able to read the body and the headers from
// the same object, which fetch doesn't support by default.
function parseFetchResponse(res) {
return res
.text()
.then(text => {
// Try to parse response body as JSON, if it wasn't JSON
// then default to text representation (including blank).
try {
return JSON.parse(text);
} catch (err) {
return text;
}
})
.then(data => ({
body: data,
headers: res.headers.raw(),
statusCode: res.status
}));
}

module.exports = { parseFetchResponse };
Loading

0 comments on commit 3304dac

Please sign in to comment.