Add TokenSource token fetching abstraction

[1egoman]

Background

Previously, when connecting to a room, this library required passing an explicit url / token - room.connect(url, token). However, this has some downsides:

• Just one literal set of credentials is being passed in, so when performing reconnects those credentials could be expired.
• There are some opportunities for some potential performance improvements - in particular, credential caching (ie, use the existing credentials as long as their are valid, and get new ones once they are expired) and token refreshing on room disconnect to ensure fresh credentials are present are two that come to mind.

This change introduces a new concept ConnectionCredentials TokenSource which is now accepted by room.connect and room.prepareConnection (importantly, in a backwards compatible way!), and addresses both points which initially operates in isolation of Room, but will be more integrated with Room in a coming change.

There are multiple implementations (TokenSourceLiteral / TokenSourceCustom / TokenSourceSandboxTokenServer) which adapt to different token generation mechanisms as well, which should mean that for most simple cases users can use one of these options. If something more fancy is desired, there are a few abstract base classes (TokenSourceFixed and TokenSourceConfigurable) which can be extended based on desired behavior to allow users to create their own fully custom credential fetching strategies.

Usage example

// Literal matches the current behavior today:
const tokenSource = TokenSource.literal({
  serverUrl: "wss://foo.livekit.cloud",
  participantToken: "xxxxxxxxxxxxx",
});

// NOTE: this seems kinda pointless but this is phase one - `Literal`s will become more important once you can pass a `TokenSource` directly into `Room`
const response = await credentials.fetch();
await room.connect(response.serverUrl, response.participantToken);

// Literal can also be used in an async fashion:
const credentials = TokenSource.literal(async () => ({
  serverUrl: "wss://foo.livekit.cloud",
  participantToken: "xxxxxxxxxxxxx",
}));

const response = await credentials.fetch();
await room.connect(response.serverUrl, response.participantToken);

// Custom allows for async token generation - credentials are cached as well, so the passed function is
// only called when the old token expires:
const credentials = new TokenSource.Custom((request) => {
  // Wait some time
  await new Promise((resolve) => setTimeout(resolve, 2000));

  return {
    serverUrl: "wss://foo.livekit.cloud",
    participantToken: "xxxxxx request data encoded in here xxxxxxx",
  };
});

const response = await credentials.fetch();
await room.connect(response.serverUrl, response.participantToken);

// SandboxTokenServer uses a sandbox token server:
const credentials = TokenSource.sandboxTokenServer("sandbox id here");
const response = await credentials.fetch();
await room.connect(response.serverUrl, response.participantToken);

[changeset-bot]

🦋 Changeset detected

Latest commit: c8b46ae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
livekit-client	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

size-limit report 📦

Path	Size
dist/livekit-client.esm.mjs	82.29 KB (+0.87% 🔺)
dist/livekit-client.umd.js	90.76 KB (+3.49% 🔺)

[pblazej]

@davidliu @hiroshihorie let's start the agent feedback loop here - this PR is not really coupled, but sort of prerequisite for simpler connect API.

For now, I did the minimal thing in Swift here which supports literal + sandbox creds.

Not sure, what do you think of:

• TTL thing - that may be useful, however introduces a dependency on jwt frameworks for invalidation
• persistence - IMO that should be out of scope for the SDK, can create new OWASP reports, keychain misuse, etc.

In both cases, I'd implement it rather as an opt-in wrapper: CachedCredentialsProvider(from: any CredentialsProvider, ttl: TimeInterval) or KeychainCredentialsProvider...

[1egoman]

Renamed from ConnectionCredentials to TokenSource after a decision in a meeting, and rebased to fix merge conflicts

[davidzhao]

looks good! just a few nitpicks

[davidzhao]

seems cleaner to have a factory method instead.

also we should use camelCase in all JS types. that's more native here.

Suggested change

	const tokenSource = new TokenSource.Literal({
	const tokenSource = TokenSource.staticToken({
	url,
	token,
	})

[lukasIO]

Discussed with Ryan that ideally we'd want to use protobuf's toJson internally here which gives us

• single source of truth for clients
• language idiomatic casing styles
• consistent on the wire format (snake case'd JSON as defined by Tobias)

[1egoman]

seems cleaner to have a factory method instead.

The reason why I did it this way over an explicit factory constructor type pattern was that fundamentally it's challenging to model all the TokenSource functionality as one class. There's too much breadth in functionality for a single monolithic class to work in all cases. Given this, it makes sense to split it up across a few levels of abstraction like I have done now. That inevitably means you end up with TokenSourceLiteral, TokenSourceCustom, TokenSourceSandboxTokenServer, etc.

The question becomes then how these should be exposed to a user. What I did here with a namespace I thought was fairly elegant, and seemed reminiscent to me of dot notation in react component names (like <MyContext.Provider value={...}>), but I totally understand it could be seen as maybe a little weird / unconventional.

Assuming the existing pattern I have isn't preferable, I see two other realistic options:

• Option 1: Make the existing TokenSource implementations all private, and add factory static methods similar to what you have proposed. In other words, at a 10k foot view, something like this:

export class TokenSource {
  /* ... implementation code here ... */
  
  /** "literal" doc string here */
  static literal(request: TokenSourceRequest) {
    return new TokenSourceLiteral(request);
  }
  /** "custom" doc string here */
  static custom(fn: (request: TokenSourceRequest) => Promise<TokenSourceResponse>) {
    return new TokenSourceCustom(fn);
  }
  /* ... etc ... */
}

/** copy of "literal" doc string here (ideally) */
class TokenSourceLiteral extends TokenSource { /* ... */ }
/** copy of "custom" doc string here (ideally) */
class TokenSourceCustom extends TokenSource { /* ... */ }
// ... etc ...

// Then, to use it later on:
import { TokenSource } from 'livekit-client';
const tokenSource = TokenSource.literal({ /* ... */ });

The biggest con to be of the above is just all the duplication / boilerplate through all the levels of indirection (especially the doc strings... I guess it's not necessarily required to duplicate them, but it sucks that we wouldn't have those on the internal abstraction levels), but maybe that's worth for the user it if this is really the best interface.

• Option 2: Get rid of the namespace and expose all classes as separate imports all starting with the same prefix. This gets rid of the leading namespace with the dot and embraces fully everything just being a class. So for example:

export class TokenSource {
  /* ... implementation code here ... */
}

/** "literal" doc string here */
export class TokenSourceLiteral extends TokenSource { /* ... */ }
/** "custom" doc string here */
export class TokenSourceCustom extends TokenSource { /* ... */ }
// ... etc ...

// Then, to use it later on:
import { TokenSourceLiteral } from 'livekit-client';
const tokenSource = new TokenSourceLiteral({ /* ... */ });

While it is verbose, the nice thing about this approach is the lack of duplication - no multiple sets of function args to keep up to date, only one doc string, etc. Also another nice thing is anybody else who wanted to make their own token source could just make a class starting with a TokenSource prefix and it matches with the stock implementations.

---

I generally am a fan of what I have now, but I'm curious where you / others like @lukasIO weigh in between what I have currently / option 1 / option 2 given all the pros and cons I outlined.

[bcherry]

I think either what you have already or your option 1 is correct, I wouldn't go for option 2. I don't have much preference between factory vs constructor here, but in swift for instance the factory option is better because you can omit the namespace, something like this:

room.connect(.literal("my-token"))

[davidzhao]

I prefer option 1.. it's verbose to copy the docstring once.. but for a end user it's so much nicer.. I don't have to think about these types to import. For most this could also be typing TokenSource. and seeing the completion on the options on how to create it.. that in itself makes the system a lot more intuitive.

[davidzhao]

I still don't like snake case.. this feels very foreign to someone who's programming in JS.

why are we making it harder for end users (forcing snake_case) in order to satisfy the wire protocol? to me that's something we should be handling internally.. all of the protobuf libs can also parse & generate in both casing.. if that's what we are concerned about

[1egoman]

all of the protobuf libs can also parse & generate in both casing.. if that's what we are concerned about

A few comments up the thread, this was what @lukasIO and I decided as well. Using a protobuf for these means we get the preferred field name casing on the js end without having to write manual mapping logic, plus it's a source of truth for all sdks which seems like a nice property.

Pull request to introduce the protobuf messages: livekit/protocol#1224

Once that gets merged I'll integrate it in here.

[1egoman]

I prefer option 1.

I still generally prefer what I originally wrote (namespace with classes inside), which has all the same benefits of option 1 you outlined (only one type to import, autocomplete behavior, etc).

That being said, I don't know if I have a strong enough opinion to keep going around and around here and given @lukasIO also prefers the static method approach over the class namespace approach, I'll migrate to what I had outlined in option 1 above.

It's worth noting that the existing classes will still likely need to be exported since they will be needed for typescript typing (and also maybe instanceof checks? Though that seems like it would be less important to me), but I'll move over to using the static methods as the preferred constructors.

[1egoman]

Updated to use protobuf like @lukasIO and I had discussed - in effect this means now that all external interfaces are camelcase, except for the parameters passed to a Custom handler function since a user needs to be given the snake case form to pass to a server.

So:

// Camelcase fields used as part of the external interface...
const tokenSourceLiteral = TokenSource.literal({ serverUrl: "...", participantToken: "..." });

const tokenSourceSandboxTokenServer = TokenSource.sandboxTokenServer({
  baseUrl: 'https://xxx.livekit.app',
  sandboxId: '...',
});

// ... except for here, where the snake case forms need to be exposed to meet the standard specification
const tokenSource = TokenSource.custom(async (req) => {
  const requestedRoomConfig = req.room_config;
  // ... etc ...
  return {
    server_url: url,
    participant_token: token,
  };
});

[1egoman]

Updating this thread with some context on some internal discussions over the past week or so:

Add "fixed" and "configurable" subtypes

A new notion of "fixed" and "configurable" token sources now exists (related code here) - "fixed" doesn't take parameters and returns what it is going to return, and "configurable" takes parameters which end up encoded in the token.

Current fixed token source types: literal.
Current configurable token source types: endpoint, custom, sandboxTokenServer.

const fixed = TokenSource.literal({ serverUrl: "...", participantToken: "..." });
fixed.fetch(); // <-- typescript won't let you put an options object in here

const configurable = TokenSource.sandboxTokenServer("sandbox id");
configurable.fetch({}); // <-- typescript REQUIRES that you put an options object in here

Add TokenSourceOptions

A new TokenSourceOptions object has been introduced to abstract TokenSourceRequest, which is now used by "configurable" token sources. This contains a less verbose form of the TokenSourceRequest object fully in camelcase.

We have gone back and forth a bit on this, but I (Ryan) am still not a super big fan of doing this because of the extra layer of abstraction that doesn't (to me) seem to add much value given useSession will be the primary user of this interface and it's an extra layer to thread any schema changes through. But it sounds like I'm in the minority on this view so I think I'm just going to go along with it for now.

// For example: TokenSourceOptions contains agentName as a more abstract form of the
// `roomConfig` token payload. So:
const tokenSource = TokenSource.sandboxTokenServer("sandbox id");
const result = await tokenSource.fetch({ agentName: "my agent to dispatch" });

Change a few subtype function signatures

The TokenSource.custom (a "configurable" token source) handler function contract has changed to (options: TokenSourceOptions) => Promise<TokenSourceResponse>, and the TokenSource.literal (a "fixed" token source) now can take a function and return a computed value in addition to a literal value.

Make TokenSource stateless again

Earlier in their evolution, TokenSource's were fully stateless, but at one point it was decided to make them stateful with a setRequest method rather than passing in the request/options into the credentials fetching function. Due to some comments we've now decided to invert that decision and make them stateless again which resolves some of the challenges of the useSession abstraction trying to declaratively call an imperative api.

const tokenSource = TokenSource.sandboxTokenServer("sandbox id");
// No more setOptions / setRequest call here
const result = await tokenSource.fetch({
  agentName: "my agent to dispatch"
} /* <-- the options get passed in here instead */);

Room integration

We've decided for the time being to NOT integrate TokenSource handling directly into the room.connect call, and to make this a follow up change so we can think a little more about how it should work once some miles have been put on the useSession abstraction.

// Current state, given this decision. 
const tokenSource = TokenSource.literal(async () => {
  // ... make a http request here ...
  return {
    serverUrl: "...",
    participantToken: "...",
  };
});

const results = await tokenSource.fetch();
room.connect(results.serverUrl, results.participantToken);

// Revisit this later - maybe try to pass in `tokenSource` into the `new Room()` constructor in
// a backwards compatible way

[1egoman]

(Rebased on top of latest main to fixed a conflict)

[1egoman]

Some context on this: As far as I can tell, there's no way to get a type of a snake case version of a protobuf type with the current protobuf infrastructure / library. Unfortunately, the protobuf library seems to return an opaque JsonValue type which is used everywhere that a snake case form could have maybe been found.

This is relevant because @lukasIO had brought up a really good point earlier on that it would be great to expose the snake case version so it can be used in the context of a server written in typescript for applying a type to the server request / response body. I agree and would like to be able to do this.

So this leaves us with a few options:

1. Forward along the opaque JsonValue type and don't expose the snake case types here at all.
2. Maintain a duplicate copy of the request / response types here in snake case
3. Somehow convert from the camelcase to snake case forms dynamically

I opted for 3 given it ended up being less complicated than I thought, but I dislike this kind of thing - it adds a fair bit of complexity. But I think out of the options it's the least bad one in my eyes at least.