search

fsprojects / pulsar-client-dotnet

Apache Pulsar native client for .NET (C#/F#/VB)
MIT License
301 stars 47 forks source link

Async token supplier / non-file-based oauth2 #253

Closed aeons closed 4 months ago

aeons commented 7 months ago

It would be great if we could either

  1. supply an async token supplier, or
  2. give the oauth2 credentials as configuration instead of a file
Lanayx commented 7 months ago

@aeons Authentication is an abstract class, so you can write your own implementation of it using configuration. As for async version (I suppose you are talking about making GetAuthData method async) - it could be a good idea, but it is initially sync in java, so Pulsar.Client just followed it to simplify understanding and now it would be a breaking change

aeons commented 7 months ago

I get that I can write my own implementation, but since there already is one included, it would be nice to just be able to do 

.Authentication(AuthenticationFactoryOAuth2.ClientCredentials(issuerUrl, audience, clientId, clientSecret))

For the sync vs async, getting an oauth2 token will always be async, so having the method be sync will always introduce blocking, but I do get the reasoning to follow the reference implementation.

chriscameron-vertexinc commented 4 months ago

I was disappointed to find I can't supply a base64 encoded URI in the form: data:application/json;base64,<base64-encoded value>

It looks like the AuthenticationFactoryOAuth2.ClientCredentials is trying to append the URI to the current working directory to find a file, and fails.

Lanayx commented 4 months ago

@chriscameron-vertexinc You are welcome to send a PR with an improvement. Right now only the LocalPath of URI is used , but I think we can extend it with base64 encoded value support.

chriscameron-vertexinc commented 4 months ago

@Lanayx I've opened a completely untested PR here https://github.com/fsprojects/pulsar-client-dotnet/pull/263

I'll try it out tomorrow, but in the meantime I'd appreciate any feedback. I haven't written any F# before!

Lanayx commented 4 months ago

@chriscameron-vertexinc Thanks, good job for the first time F#! I've added some style recommendations and will be ready to merge once you confirm it works

Lanayx commented 4 months ago

Published https://www.nuget.org/packages/Pulsar.Client/3.5.0 with the improvement

Lanayx commented 4 months ago

Closing as completed