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

Incorrect assumption about FCM endpoint #14

Open
arjan opened this issue Aug 16, 2019 · 4 comments
Open

Incorrect assumption about FCM endpoint #14

arjan opened this issue Aug 16, 2019 · 4 comments

Comments

@arjan
Copy link
Contributor

arjan commented Aug 16, 2019

This code asserts that an auth key (GCM sender ID) is required when a push subscription is sent from FCM. However, this is not true because FCM fully implements VAPID, as can be read here: "With VAPID you no longer need to sign up for an account with GCM".

My suggestion therefore is to remove this function clause altogether, as well as this part.

@danhper
Copy link
Owner

danhper commented Aug 16, 2019

Hi, thanks for reporting the issue.
I suppose that was the situation when I first implemented this library but if it not required anymore this should indeed be fixed.
It would be great if you had the time to send a PR with the changes you suggested.
Thanks a lot.

@arjan
Copy link
Contributor Author

arjan commented Aug 16, 2019

Yes, only, this will effectively reverse #13 so I am curious why @splagemann fixed this in the first place...

@splagemann
Copy link
Contributor

I'm pretty sure that I had issues without an auth key. But I can check that again and I'm happy to have that removed.

@arjan
Copy link
Contributor Author

arjan commented Aug 19, 2019

I had issues with an auth key :-)
Anyway I think it's possible to support both code paths. Could you check whether the auth key path works for you?

SlIdE42 added a commit to SlIdE42/elixir-web-push-encryption that referenced this issue Sep 25, 2019
danhper added a commit that referenced this issue Sep 25, 2019
Fix issue #14 and allow a custom TTL
@danhper danhper mentioned this issue Nov 6, 2019
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

No branches or pull requests

3 participants