decision-labs / fcm

Ruby bindings to Firebase Cloud Messaging (FCM) for Android, iOS or Web
MIT License
517 stars 154 forks source link

Use OAuth2 token instead of server key for v1 #123

Closed erimicel closed 1 month ago

erimicel commented 2 months ago

Added this PR thanks to @aap17 's change https://github.com/decision-labs/fcm/issues/122#issuecomment-2324301928

Apps using the deprecated FCM legacy APIs for HTTP and XMPP should migrate to the HTTP v1 API at the earliest opportunity. Sending messages (including upstream messages) with those APIs was deprecated on June 20, 2023, and shutdown begins on July 22, 2024.

https://firebase.google.com/docs/cloud-messaging/migrate-v1 Before Authorization: key=AIzaSyZ-1u...0GBYzPu7Udno5aA

After Authorization: Bearer ya29.ElqKBGN2Ri_Uz...HnS_uNreA

This is already breaking change for sending messages via legacy api and should be removed;

Breaking Changes

Supported Features

TODO

sabman commented 1 month ago

@erimicel thanks for doing this. Could you also look at the README.md and review if the docs are up-to-date with this PR. We can then cut a new version. Since its not backwards compatible we will cut a new version: 2.0.0

erimicel commented 1 month ago

@erimicel thanks for doing this. Could you also look at the README.md and review if the docs are up-to-date with this PR. We can then cut a new version. Since its not backwards compatible we will cut a new version: 2.0.0

@sabman I've added a more detailed README. The core changes are listed below—could you review and let me know if they look sensible to you?

The PR also has several styling issues flagged. Let me know if you think those need to be addressed.

I performed a real production environment test with the given changes for each method, and all looks fine at the moment.

Breaking Changes

Supported Features

We could leave the duplicated topic subscription methods in place and soft delete the api_key. However, if we release a new version, it might be a good opportunity for cleanup. But removing API_KEY will definitely break client initializers for everybody, so I am not sure.

tiendo1011 commented 1 month ago

@AllanQ can we merge this?

tiendo1011 commented 1 month ago

@sabman can we merge this?

AllanQ commented 1 month ago

@AllanQ can we merge this?

@tiendo1011, I am not authorized to make the decision.

sabman commented 1 month ago

Just running tests will merge shortly.

sabman commented 1 month ago

Thanks to all the awesome contributors!