Page MenuHomePhabricator

[native] Pass CSAT to Blob service when fetching multimedia
ClosedPublic

Authored by bartek on Feb 1 2024, 4:09 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 15, 11:00 AM
Unknown Object (File)
Mon, Nov 11, 9:50 AM
Unknown Object (File)
Sat, Nov 9, 3:21 AM
Unknown Object (File)
Fri, Nov 8, 10:03 PM
Unknown Object (File)
Fri, Nov 8, 7:15 PM
Unknown Object (File)
Fri, Nov 8, 7:15 PM
Unknown Object (File)
Fri, Nov 8, 7:15 PM
Unknown Object (File)
Fri, Nov 8, 7:15 PM
Subscribers

Details

Summary

Modified fetchAndDecryptMedia() to accept authMetadata argument and pass it as Authorization header with Blob service requests.

Depends on D10907, D10908

Test Plan
  • Enabled encrypted media and blob service, sent a multimedia message
  • Modified blob handlers the same way as in D10908 test plan
  • Opened chat on native - the multimedia loads correctly, blob service logs contain auth metadata:
INFO get_blob{blob_hash=xn2ILjHR45XHsbjNf_wRalWNjusWtCNntiutX1eJn0w}: blob::http::handlers::blob: Get blob request auth=UserToken(UserIdentity { user_id: "foo", access_token: "baz", device_id: "9/Dk+21eU27n7fpvFDVWqNrouje2XKGW2jtLPC7vx+A" })
`

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Feb 1 2024, 5:43 AM

Replace convenient hook with imperative calls

lib/utils/services-utils.js
25 ↗(On Diff #36575)

Same as in https://phab.comm.dev/D10908?id=36504#inline-66206
This type shouldn't be readonly

native/media/save-media.js
381 ↗(On Diff #36575)

This place is too deeply nested function so it's not worth passing-through value from identity context 5 functions above. I think it's OK to get it directly from SecureStore here.
We want it for one-off call, so no need to watch for updates etc.

tomek added inline comments.
native/media/save-media.js
381 ↗(On Diff #36575)

You can consider introducing a hook that uses IdentityClientContext and returns a callback saveRemoteMediaToDisk. It might also make sense for fetchAndDecryptMedia because it always requires auth headers.

This revision is now accepted and ready to land.Feb 6 2024, 1:23 AM
native/media/save-media.js
381 ↗(On Diff #36575)

Yes, it makes sense for fetchAndDecryptMedia in other places, but in this case, this won't solve my initial issue - I still need to pass the callback 5 levels down which, in my opinion, is not worth it.

Add hook for fetchAndDecryptMedia