Token source

[pblazej]

Incarnation of livekit/client-sdk-js#1645

Key differences:

• Mostly leverages Swift-native approach (protocols + default impl)
  • it makes e.g. Sandbox "inherit" from TokenServer etc.
• Token fetch is stateless, caching is opt-in wrapper (which could be extended to Keychain tho) AgentSession(credentials: CachingCredentialsProvider(SandboxTokenServer(id: sandboxId))...
• JWT support is limited e.g. exposing cached token payload
  • we have a 3rd party dependency, but only used for testing - IMO it's not worth to compile it for every single use case, but I'm happy to include it based on feedback

[pblazej]

I haven't pushed the respective proto, it does not provide any kind of "validation" for the payload keys, etc.

[pblazej]

It may be good to provide (later) a simpler overload with agentName for the purpose of agent abstractions.

[1egoman]

Looks pretty good to me! I'm not going to be able to comment on some of the swift specific things being done, hopefully others can weigh in on that stuff.

[1egoman]

A few notes:

• At least in the javascript implementation, I had started in a similar direction as what it looks like you are doing here (passing "options" alongside connect) but @lukasIO had objected and preferred that the request parameters be set directly on the ConnectionCredentials with a method. I'll let him rearticulate his exact argument but I think it was in his mind a cleaner separation of concerns. It's maybe something to consider, idk.

• Just like I did in the javascript version, It might be a good idea to cache the credentialsProvider instance on room in the connect call and then in disconnect call self.credentialsProvider.fetch(...) to regenerate the token if required so that reconnecting after a disconnect could be a little faster since the token would already be valid.

• Github won't let me select the old connect function signature, but is there some way it can officially be deprecated in swift in a similar way as what I did in typescript? Given there's also a ConnectionCredentials.Literal it seems like there's not really an explicit use case for the old signature anymore?

[pblazej]

• Yes, passing that is a little awkward here - but the mutable version is equally awkward to me - what if you don't set a request/options at all and fetch?
• Then the caching part makes less sense to me - if you invalidate on every disconnect, am I right?
• Yes, the deprecation is perfectly possible, buuuut it will appear on the ObjC side as well - and the new method essentially breaks ObjC compat (as AgentSession) does

[pblazej]

@lukasIO making everything stateful by default also creates some challenges, e.g. breaks the composition (who should cache the request - the cache or the underlying provider), I cannot force Sandbox to be initialized with both options and ID, etc.

Essentially, it fits one use case - when you have one concrete (caching) provider and switch the lambda under the hood.

IMO, that's the same problem that exists in Room - you can pass options on both connect() and init().

I think the top-level question we need to answer is how and when to pass these token-related options here:

private let session = AgentSession(credentials: CachingCredentialsProvider(SandboxTokenServer(id: Self.sandboxId)),
                                       // agentName: ...

My initial concept was as above ⬆️ then we create credential "options" under the hood and pass to the room on connect. The session reuses agentName(s) for its internal filtering.

[1egoman]

Yes, passing that is a little awkward here - but the mutable version is equally awkward to me - what if you don't set a request/options at all and fetch?

I'd think in that case it would default to an empty request? That's what the typescript implementation does at least.

Then the caching part makes less sense to me - if you invalidate on every disconnect, am I right?

No, it shouldn't - it should refetch the token only if it is expired, and effectively be a no-op if it isn't expired. Though since the caching logic is opt in maybe the tradeoffs differ a bit and it's a less good idea if there isn't caching in the picture.

Yes, the deprecation is perfectly possible, buuuut it will appear on the ObjC side as well - and the new method essentially breaks ObjC compat (as AgentSession) does

Hmm, I see. Sounds like there's some nuance / not a clear thing to do there...

[pblazej]

There will be some renaming to TokenSource and TokenSource.Endpoint as discussed with JS.

[pblazej]

Moved it to a separate file for visibility.

[pblazej]

Added JWT support (if we really want it 👍)