search

singer-io / tap-sendgrid

a Singer Tap for SendGrid Core
GNU Affero General Public License v3.0
6 stars 10 forks source link

Sendgrid API no longer supports "marketing_campaigns" scope #3

Closed hjackbeem closed 4 years ago

hjackbeem commented 5 years ago

Description

Trying to configure this tap in Stitch yields the following error:

2019-08-28 18:21:05,094Z   main - INFO Tap exited abnormally with status 1
2019-08-28 18:21:05,096Z   main - INFO Exit status is: Discovery failed with code 1 and error message: "Insufficient authorization, missing for marketing_campaigns.read".

It appears that marketing_campaigns.read is no longer a supported permission for the Sendgrid API, which instead exposes marketing.read and marketing.automation.read. It is worth noting that the Sendgrid docs erroneously still list marketing_campaings.read as viable permission.

Type of change

marketing_campaigns.read has been removed from scopes in streams.py and replaced with marketing.read and marketing.automation.read.

KAllan357 commented 5 years ago

I tried this code locally and get an error: [{'field': None, 'message': 'access forbidden'}] when I try to sync campaigns.

I generated a token with unlimited access. Any thoughts?

hjackbeem commented 5 years ago

Thanks for the quick reply! :) Not sure what's causing that, can you share the properties config you are using?

KAllan357 commented 5 years ago

The only relevant item would be the api_key, I think? {"api_key": "SG..."}.

I see the same error described in this issue, I'm just wondering if whether I could sync Campaigns after this PR is because of the PR or some other issue.

hjackbeem commented 5 years ago

How are you running the tap? Is it something like tap-sendgrid -c config.json --properties properties.json? And if so, what does the content of that properties.json look like?

I'm afraid I'm also a little unclear about the errors you're getting. When you say you see the same error described, do you mean you get the "Insufficient authorization, missing for marketing_campaigns.read" error when using the tap in the Stitch app, or when running the PR code locally?

KAllan357 commented 5 years ago

I was running the tap similar to how it executes within Stitch. With no modifications, I saw the error this PR is fixing. "Insufficient authorization, missing for marketing_campaigns.read".

I figure that the old marketing_campaign.read permission was useful for reading the Campaign stream, so I selected that one and executed the tap with the code in this PR. What I'm ultimately wondering is whether there's an underlying issue with our test Sendgrid account (that we can't query Campaigns) or if that error is a side effect of this change?

As the author of the PR, were you able to read Campaign objects from Sendgrid with the changes here?

hjackbeem commented 5 years ago

Looks like I was getting a false positive on my sync action. Looking into it, I think there is a deeper problem with Sendgrid's API permissions regarding Campaings. I have raised an issue with their support team and am waiting to hear back.

leongersing commented 4 years ago

Hello all! Just popping on to confirm that new stitch integrations using this tap are failing due to the missing marketing_campaign.read scope. It appears they have updated the API without updating the docs.

KAllan357 commented 4 years ago

I merged and released this and have confirmed that it broke in other places. Reverting.

hjackbeem commented 4 years ago

After talking to Sendgrid support about this, it seems the only way to add the marketing_campaign.read scope is by raising a support ticket. They added the scope for me and after that this tap worked correctly, without any of the modification I suggested.

So this tap will be broken for any new Sendgrid users, because they can't pass the permission check without requesting support to add the undocumented permission, but they only need that permission if they want to have access to legacy campaigns.

I'm still very new to the Singer ecosystem so I'm not sure what the best way to handle this scenario. Should "Legacy Campaign" support be a configurable option? Or would it make more sense to have a separate "Sendgrid Legacy Campaigns" tap?

shedd commented 4 years ago

I've been attempting to resolve #6

Per the comment above, I asked for marketing_campaign.read to be added - SendGrid support replied:

The [marketing_campaigns.read] scope you are referencing is a legacy Marketing Campaigns scope. The Legacy Marketing Campaigns feature is no longer available, unfortunately. As such, I wouldn't be able to add that scope on your behalf.

Why was this PR reverted in #5?