[keyserver] Process and store encrypted media
ClosedPublic
Actions

Authored by bartek on Mar 24 2023, 4:04 AM.

Details

Reviewers

atul
marcin
ashoat

Commits

rCOMM8d55d986b329: [keyserver] Process and store encrypted media

Summary

Part of ENG-3393

Encrypted media can no longer be processed on keyserver. Modified validateAndConvert - added a simple validation based on provided MIME type - it must be provided in order to detect if we have image or video. Then the content is passed through.

Then stored the encryption key in extras column of the uploads table.

Depends on D7166

Test Plan

Modified the uploadMultimedia call in input-state-container to pass some encryptionKey. Uploaded some media. Ensured the encryptionKey is added to the uploads table

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Mar 24 2023, 4:04 AM

bartek held this revision as a draft.

Herald added a subscriber: tomek. · View Herald TranscriptMar 24 2023, 4:04 AM

bartek published this revision for review.Mar 24 2023, 4:19 AM

Harbormaster completed remote builds in B17577: Diff 24065.Mar 24 2023, 4:21 AM

bartek added a child revision: D7171: [keyserver] Compose encrypted media messages.Mar 24 2023, 5:59 AM

ashoat requested changes to this revision.Mar 24 2023, 11:47 AM

ashoat added inline comments.

keyserver/src/uploads/media-utils.js
33 ↗	(On Diff #24065)	It feels like this function should probably take an object instead of a list of parameters, given how many of them there are and that some of them are `boolean` (which makes it unclear what's going on at the callsite)
53–58 ↗	(On Diff #24065)	`mediaConfig` tells us what kinds of files we can accept on `native` from the user, but it doesn't tell us what kind of files we can accept in `keyserver` to distribute to peers. For example, somebody is allowed to upload a `gif`, but it gets transcoded to an `mp4` or `jpeg` (depending on whether it is an animated GIF) before being uploaded to the keyserver. Your code here makes it so we change this behavior and now accept a variety of formats that we might not be able to render on clients. Instead, can we use `serverCanHandleTypes`, like we do elsewhere in this function?

This revision now requires changes to proceed.Mar 24 2023, 11:47 AM

bartek added inline comments.Mar 24 2023, 10:42 PM

keyserver/src/uploads/media-utils.js
33 ↗	(On Diff #24065)	Agreed, will change this
53–58 ↗	(On Diff #24065)	Yeah I looked at the `serverCanHandleTypes` before and it's indeed a subset of `mediaConfig` types. But my understanding was that these are types that can be processed by keyserver, and we're skipping that processing for encrypted files. That's why I used `mediaConfig` directly. Anyway, I can change it back to use `serverCanHandleTypes` because my understanding was wrong.

Updated validateAndConvert to accept a single object argument
Used serverCanHandleTypes to validate MIME type

Harbormaster completed remote builds in B17615: Diff 24109.Mar 24 2023, 11:28 PM

ashoat accepted this revision.Mar 25 2023, 2:54 AM

ashoat added inline comments.

keyserver/src/uploads/media-utils.js
70–72 ↗	(On Diff #24109)	We could consider replacing these lines with this. PS there is a typo here (extraneous `}` character in the invariant text)
53–58 ↗	(On Diff #24065)	That's fair... in retrospect, `serverCanHandleTypes` should probably be called `clientCanHandleTypes`