Page MenuHomePhabricator

[keyserver] Authorize blob service requests
ClosedPublic

Authored by bartek on Feb 1 2024, 1:30 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 8, 10:03 PM
Unknown Object (File)
Fri, Nov 8, 7:14 PM
Unknown Object (File)
Fri, Nov 8, 7:14 PM
Unknown Object (File)
Fri, Nov 8, 7:14 PM
Unknown Object (File)
Fri, Nov 8, 7:14 PM
Unknown Object (File)
Fri, Nov 8, 1:36 PM
Unknown Object (File)
Oct 24 2024, 2:35 AM
Unknown Object (File)
Oct 3 2024, 11:47 PM
Subscribers

Details

Summary

Resolves ENG-6287.
Added Authorization header to blob service requests.

Test Plan

Called all these blob functions in "main" keyserver.js. In blob service handlers, I added the following:

#[instrument(name = "assign_holder", skip(service))]
pub async fn assign_holder_handler(
  // ...
  auth: comm_lib::auth::AuthorizationCredential,
) -> actix_web::Result<HttpResponse> {
  info!(?auth, "Assign holder request");
  // ...

And checked the logs to see that the auth struct was populated with the correct data from the keyserver.

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, 2:08 AM
bartek added inline comments.
keyserver/src/services/blob.js
20 ↗(On Diff #36504)

This is on purpose NOT $ReadOnly - seems like fetch() doesn't like the plus sign 😛

lib/utils/services-utils.js
17 ↗(On Diff #36504)

I really don't like passing this param all the time but haven't got any better idea than possibly this:

  1. Create higher-order-function (curry this one) in lib: const httpAuthorizationHeaderFactoryFn = b64encodeFn => authMetadata => { ... }
  2. For each platform create {keyserver,native,web}/utils/some-utils.js with
    • keyserver: const createHTTPAuthorizationHeader = httpAuthorizationHeaderFactoryFn(the_above_Buffer_from_fn);
    • native: const createHTTPAuthorizationHeader = httpAuthorizationHeaderFactoryFn(some_fns_from_commUtilsModule);
    • web: const createHTTPAuthorizationHeader = httpAuthorizationHeaderFactoryFn(btoa);

But the above is still not perfect. Let's assume I want to add another utility function on top of this one:

function createDefaultHTTPHeaders(authMetadata: AuthMetadata): { [string]: string } {
  const authorizationValue = createHTTPAuthorizationHeader(authMetadata);
  return {
    Authorization: authorizationValue,
  };
}

I'd have to repeat this function 3 times per platform, or curry the same way as the original: const defaultHTTPHeadersFactory = authHeaderFn => authMetadata => { ... }

tomek added inline comments.
lib/utils/services-utils.js
17 ↗(On Diff #36504)

Not sure if we can elegantly solve this. Maybe some platform-specific config might help... not sure if it is worth it.

This revision is now accepted and ready to land.Feb 1 2024, 5:56 AM
lib/utils/services-utils.js
17 ↗(On Diff #36504)

Probably I'll have to add the js-only base-64 dependency to lib.

I worked around passing callback to multimedia endpoints, but reports stuff is mostly lib-only so no good way of doing it.
The library is slower than platform-specific APIs I mentioned above, but Authorization header isn't too much data so performance shouldn't hurt