Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Namespaced APIs #563

Closed
wants to merge 2 commits into from
Closed

Conversation

Menecats
Copy link

This PR introduces the concept of namespaced proxies.

With these chenges it's now possible to expose multiple objects from a single worker.

E.g.

worker.js

const authentication = {
  login: function (username, password) {
    return Math.random() < .5;
  }
};
const logger = {
  log: function (message) {
    console.log(message);
  }
};
const obj = {
  counter: 0,
  inc() {
    this.counter++;
  }
};

Comlink.expose(obj);
Comlink.exposeNamespaced(authentication, 'auth-namespace');
Comlink.exposeNamespaced(logger, 'log-namespace');

main.js

const worker = new Worker("worker.js");

const obj = Comlink.wrap(worker);
const auth = Comlink.wrapNamespaced(worker, 'auth-namespace');
const logger = Comlink.wrapNamespaced(worker, 'log-namespace');

if (await auth.login('my-username', 's3cr3t pwd!')) {
  await obj.inc();
  await logger.log(await obj.counter));
} else {
  await logger.log('Invalid credentials');
}

@mhofman
Copy link
Contributor

mhofman commented Sep 23, 2021

Out of curiosity, can't the same thing be accomplished by something like this (sorry haven't looked at Comlink in ages, but the idea is to expose a single root object which contains proxies for your namespaced objects)

// worker.js
Comlink.expose({
  obj: Comlink.proxy(obj),
  authentication: Comlink.proxy(authentication),
  logger: Comlink.proxy(logger)
});

//main.js
const root = Comlink.wrap(worker);
const [obj, auth, logger] = await Promise.all(['obj', 'authentication', 'logger'].map(async (prop) => root[prop]));

@Menecats
Copy link
Author

Menecats commented Sep 24, 2021

Yes currently there's a little difference by using a root object and namespaces, but I've also started working on the issue #444, where you can specify an allowed origin when exposing something with Comlink.

Something like this:
Note: the parameters are not yet final

worker.js

Comlink.expose(obj, /* Comlink.ExposeOptions */ { allowedOrigins: ['*'] /* This is the default */ });
Comlink.exposeNamespaced(authentication, /* Comlink.ExposeOptions */ { allowedOrigins: ['example.com'] });

By exposing a single root object is not currently possible to specify different origin restrictions for each of them.

@surma
Copy link
Collaborator

surma commented Sep 24, 2021

Hey @Menecats,

I appreciate your enthusiasm, but it would be good in the future to open an issue first before implementing a feature so we can talk about it. I don’t think I want to add this to Comlink. As @mhofman points out, you can already do this with normal objects in Comlink right now.

As for the per-namespace origins, can you tell me more about the use-cases? I haven’t encountered a need for that.

That being said, if you wanted to tackle #444 as you hinted at, that’d be awesome!

@Menecats
Copy link
Author

Hey @surma,

Namespaced APIs

Sorry about the missing issue, I've implemented this feature in a fork of comlink that I'm using for a customer and I thought that someone else could have needed it.

My reasons for implementing this API are the following:

Our customer has a web application that orchestrates different web applications loaded inside iframes; the orchestrator (launcher from now on) and the different applications loaded by the launcher (dashboards from now on) communicate already by using Comlink, we need to implement some components that can be loaded dynamically into the dashboards and they also have to communicate with the launcher, the problem is that we can't use the already existing Comlink proxy since it's used by the primary communication protocol between dashboards and the launcher that has been developed by another third party vendor that created the dashboards and the launcher.

That's why I've implemented namespaces, this way the vendor can continue to use wrap and expose exclusively for itself, while I can use a custom namespace to allow my components to communicate between the launcher and the dashboards.

per-namespace origin

TLDR

nevermind, I've noticed that the MessageEvent.origin property is not set in webworkers

Long answer

The idea behind these changes was to restrict access to a particular object in a web worker that I want to make available only to a particular origin.

E.g.
worker.js

var publicApi = {...}
var privateApi = {...}; // This API is should be availabe only if this worker is loaded from 'example.com'

Comlink.expose(publicApi);
Comlink.exposeNamespaced(privateApi, 'my-namespace' /* , Specify somehow the allowed origin*/);

A simple idea using the current API can be to check the page origin before exposing the value

if (isValidOrigin(self.location.origin)) {
  Comlink.expose({privateApi, publicApi});
} else {
  Comlink.expose({publicApi});
}

The problem with that is that both chrome and firefox allow you to redefine the location property and this will expose the privateApi object even if the worker is loaded from an unauthorized origin.

worker-loader.js

Object.defineProperty(self, 'location', {value: {origin: 'https://example.com'}})
importScripts('http://example.com/worker.js');

My idea was to look at the origin property in the event received by the listeners of each exposed object and if it didn't match the allowedOrigins the event would have been discarded.

But since MessageEvent.origin is set only in server-sent events and cross-document messaging as per spec this whole point is currently invalid.

@benjamind
Copy link
Collaborator

Origin filtering has been added to the expose method in #605 (docs to follow in #606).

With regard to the namespaces proposed in this PR. Could the problem described not be solved by passing MessagePorts across the iframe postmessage channel, and thus you can expose separate interfaces for each origin by passing the appropriate port endpoint?

@Menecats
Copy link
Author

Hi @benjamind, yeah that's something I didn't think about when i tackled this issue.
I think that that's a much more straightforward solution than what I've come up with.

@Menecats Menecats closed this Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants