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

Sourse OneSignal: Authentication of OneSignal API changed to not allow User Auth key for notification requests #23535

Closed
andnig opened this issue Feb 28, 2023 · 1 comment
Assignees
Labels
area/connectors Connector related issues community connectors/source/onesignal python Pull requests that update Python code team/connectors-python type/bug Something isn't working

Comments

@andnig
Copy link
Contributor

andnig commented Feb 28, 2023

Environment

  • Airbyte version: all
  • OS Version / Instance: all
  • Deployment: all
  • Source Connector and version: OneSignal 0.1.2
  • Step where error happened: Sync

Current Behavior

The OneSignal 0.1.2 connector uses the User Auth Key (which is sort of a "global" key) to authenticate the API. This worked fine until 2023-02-21. However, OneSignal changed their authentication schema to only allow app-specific REST API Keys to authenticate. This is one key per OneSignal app - meaning we can't use this "global" key to authenticate ALL operations anymore.

This also means, we can't simply configure one key and sync ALL app notifications. We need to configure one key per app.

This change in authentication was confirmed by OneSignal support.

Expected Behavior

We'd need to have configuration options to set the app we want to sync as well as the respective rest api key. As there are also API operations / airbyte streams which still require the user auth key (like listing all apps), I propose having the following configuration options:

  • user auth key
  • app to sync
  • rest api key

Then we'd need to change the behavior of the connector itself.

  • For app-specific streams: Instead of iterating through all apps and syncing all apps, we'd only need to sync the configured app and use the rest-api-key
  • For "global" streams like listing apps, we keep the behavior as is

Logs

I'll not add the whole logs here, as the issue was confirmed by OneSignal support and the logs simply tell us that we make a bad request to OneSignl.

The main portion of the logs is:

2023-02-27 12:02:18 source > Encountered an exception while reading stream SourceOnesignal
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/airbyte_cdk/sources/abstract_source.py", line 108, in read
    internal_config=internal_config,
  File "/usr/local/lib/python3.7/site-packages/airbyte_cdk/sources/abstract_source.py", line 141, in _read_stream
    for record in record_iterator:
  File "/usr/local/lib/python3.7/site-packages/airbyte_cdk/sources/abstract_source.py", line 213, in _read_full_refresh
    for record in records:
  File "/usr/local/lib/python3.7/site-packages/airbyte_cdk/sources/streams/http/http.py", line 352, in read_records
    response = self._send_request(request, request_kwargs)
  File "/usr/local/lib/python3.7/site-packages/airbyte_cdk/sources/streams/http/http.py", line 319, in _send_request
    return backoff_handler(user_backoff_handler)(request, request_kwargs)
  File "/usr/local/lib/python3.7/site-packages/backoff/_sync.py", line 94, in retry
    ret = target(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/backoff/_sync.py", line 94, in retry
    ret = target(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/airbyte_cdk/sources/streams/http/http.py", line 285, in _send
    response.raise_for_status()
  File "/usr/local/lib/python3.7/site-packages/requests/models.py", line 953, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: https://onesignal.com/api/v1/notifications?app_id=2c1d5775-8fd6-4c1c-bdad-7bfc9cf9fd64&limit=50

Steps to Reproduce

  1. Use the OneSignal source connector
  2. Attempt to sync the notifications stream

Are you willing to submit a PR?

I'm happy to submit a PR - but I'd need the following question answered:

  • As I need to dive deep into the OneSignal connector and remodel most of the more complex app-specific streams would it make sense to create a new connector using the lowcode CDK? This would most probably take less time and make it more robust to maintain in the future.
@andnig andnig added needs-triage type/bug Something isn't working labels Feb 28, 2023
@natalyjazzviolin natalyjazzviolin changed the title Connector OneSignal: Authentication of OneSignal API changed to not allow User Auth key for notification requests Sourse OneSignal: Authentication of OneSignal API changed to not allow User Auth key for notification requests Feb 28, 2023
@natalyjazzviolin natalyjazzviolin added connectors/source/onesignal area/connectors Connector related issues python Pull requests that update Python code team/connectors-python and removed needs-triage team/tse Technical Support Engineers autoteam labels Feb 28, 2023
@artem1205 artem1205 self-assigned this Mar 13, 2023
@artem1205
Copy link
Collaborator

Fixed in #24076

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues community connectors/source/onesignal python Pull requests that update Python code team/connectors-python type/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants