OneSignal / OneSignal-Flutter-SDK

OneSignal is a free push notification service for mobile apps. This plugin makes it easy to integrate your flutter app with OneSignal
https://www.onesignal.com
Other
625 stars 215 forks source link

chore: add warning for identity verification #901

Closed zvikarp closed 4 months ago

zvikarp commented 5 months ago

Description

One Line Summary

This method is misleading. it is not implemented but the package exposes it. Please remove it or annotate it.

Details

Errors from Android in device logs:

06-12 23:50:13.470 21589 21742 D OneSignal: [DefaultDispatcher-worker-2] HttpClient: Got Response = POST https://api.onesignal.com/apps/<REDUCTED-UUID>/users - FAILED STATUS: 400
06-12 23:50:13.478 21589 21742 W OneSignal: [DefaultDispatcher-worker-2] HttpClient: Got Response = POST - STATUS: 400 - Body: {"errors":[{"title":"uuid: incorrect UUID length 42 in string \"local-<REDUCTED-UUID>\""}]}

Error from iOS in API request:

curl:

curl 'https://api.onesignal.com/apps/<REDUCTED-UUID>/users' \
-X POST \
-H 'Host: api.onesignal.com' \
-H 'Connection: keep-alive' \
-H 'Accept: application/vnd.onesignal.v1+json' \
-H 'SDK-Version: onesignal/ios/050200' \
-H 'Accept-Language: en-GB,en-US;q=0.9,en;q=0.8' \
-H 'User-Agent: stern/77 CFNetwork/1496.0.7 Darwin/23.5.0' \
-H 'Content-Type: application/json' \
--cookie '__cf_bm=<REDUCTED-COOKIE>' \
--data-raw '{"refresh_device_metadata":true,"subscriptions":[{"test_type":1, [...] ":"iOSPush"}],"properties":{"timezone_id":"Asia\/Jerusalem","language":"en-IL"},"identity":{}}' \
--proxy http://localhost:9495

response:

{
    "errors": [
        {
            "code": "auth-1",
            "title": "This operation requires 'Authorization' in the HTTP header"
        }
    ]
}

Documentation that mentions it is not implemented: https://documentation.onesignal.com/docs/identity-verification.

Motivation

I spent hours trying to figure out why I'm getting null for onesignal id, no errors in the code logging but different errors from device logs and apis for ios and android didn't help either.

Affected code checklist

Checklist

Overview

Testing

Final pass


This change is Reviewable

zvikarp commented 5 months ago

this is related to https://github.com/OneSignal/OneSignal-Android-SDK/issues/1913

zvikarp commented 5 months ago

Hi @jennantilla @nan-li or the rest of the team, is there any blocker form merging this?

nan-li commented 5 months ago

Hi @zvikarp thanks for submitting this PR, this method should not have been exposed when the functionality is not yet ready.