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

ClientManager for token-based clients? #98

Open
froodian opened this issue Oct 30, 2017 · 8 comments · May be fixed by #160 or #213
Open

ClientManager for token-based clients? #98

froodian opened this issue Oct 30, 2017 · 8 comments · May be fixed by #160 or #213

Comments

@froodian
Copy link
Contributor

ClientManager is a great class, it would be nice to have a version of this class (TokenClientManager?) which works for JWT/Auth token based clients. It looks like this could be done without too much effort (make the Get method take a *token.Token, change cacheKey to be based on *token.Token instead of tls.Certificate, make Factory take a *token.Token instead of a tls.Certificate).

@sideshow
Copy link
Owner

sideshow commented Nov 3, 2017

cc/- @dwieeb as he created this awesome class.
This would be great. I had overlooked this when merging the token stuff.
Will happily merge a PR if you have any thoughts @froodian / @dwieeb

@imhoffd
Copy link
Contributor

imhoffd commented Nov 5, 2017

The reason I added a PR for ClientManager is that, when given a certificate, Client sets up a TLS connection using that certificate. Each Client has an independent connection by necessity, and managing all those connections was difficult as a user of this library. My impression is that Apple allowed JWT authentication to make things much easier. No reason to manually specify certificates in Client--it will use system certificates to secure the TLS connection.

So it seems to me one could use a single Client, maintaining a single connection, and setting the token to use for each push notification.

I believe this would be possible today in user code with something like this:

mu.Lock()
client.Token = token
client.PushWithContext(...)
mu.Unlock()

@avinassh
Copy link

@dwieeb so, we don't need client manager at all for token clients?

@bdandy bdandy linked a pull request Dec 24, 2019 that will close this issue
@bdandy
Copy link

bdandy commented Dec 24, 2019

@dwieeb you're wrong, we've got an issue with JWT tokens - Apple do not allow to reuse same connection for different tokens. I've made PR to add token support for ClientManager

@lakim
Copy link

lakim commented Nov 16, 2021

Apple documentation clearly states:

APNs does not support authentication tokens from multiple developer accounts over a single connection.

In our case, we are building a system that sends Push Notifications to different apps from different developer accounts, using JWT tokens.
We'll try this PR: #160
@bdandy have you been using your PR in production?

@bdandy
Copy link

bdandy commented Nov 17, 2021

@lakim we've used those changes in production since PR was created, but I don't have any reports about issues

@lakim
Copy link

lakim commented Nov 18, 2021

@bdandy nice, thanks for the feedback. We'll test it out soon.
@sideshow we might eventually take over #160, add tests and submit it back. In a few weeks probably.

@lakim lakim linked a pull request Oct 17, 2022 that will close this issue
@lakim
Copy link

lakim commented Oct 17, 2022

@bdandy @sideshow
FYI, I've created an up to date PR with tests:
#213

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants