laravel-notification-channels / twitter

Twitter Notifications Channel for Laravel
http://laravel-notification-channels.com
MIT License
169 stars 44 forks source link

Attaching Media on behalf of users authenticated via oAuth 2.0 PKCE doesn't work #108

Open ivannovak opened 1 month ago

ivannovak commented 1 month ago

... and I think this is fine because the upload-media API requires oauth 1.1, but it opens up what seems to be an unaccounted for error condition. I think handling the error more elegantly would have saved me time and maybe it'd save some time for others as well.

For better context, I was using socialstream with the twitter oauth v2 provider. Users were able to login to the app and in connected accounts they would have a token and refresh_token set without a secret... by design.

When creating new posts from the app's account, there's no issue because the app has it's own provisioned access token and secret generated via the twitter app so it can seamlessly switch between 1.1 and 2.

Also, when creating new text based posts on behalf of other users it works just fine. Using the documentation for Twitter and the source for the twitter notification channel, the routeNotificationsForTwitter ended up looking like this:

public function routeNotificationForTwitter($notification)
{
    $connectedAccount = $this->connectedAccounts->where('provider', Providers::twitterOAuth2())->first();
    return [
        config('services.twitter.consumer_key'),
        config('services.twitter.consumer_secret'),
        null,
        $connectedAccount->token ?? null, // uses the token as the bearer per the oauth 2.0 pkce docs and \Abraham\TwitterOAuth source
    ];
}

However, where it breaks is when attempting to attach media when posting on behalf of the user and using the bearer token generated through oauth 2.0 pkce. Even if the scopes are setup appropriately with tweet.read, tweet.write, and users.read.

The TwitterChannel send method appropriately sets the api version to 1.1 prior to handling media, but breaks in an unhandled way if the credentials being used are not compliant with 1.1.

The errors look like this:

   WARNING  Undefined property: stdClass::$media_id_string in vendor/abraham/twitteroauth/src/TwitterOAuth.php on line 384.

   WARNING  Undefined property: stdClass::$media_id_string in vendor/abraham/twitteroauth/src/TwitterOAuth.php on line 384.

   WARNING  Undefined property: stdClass::$media_id_string in vendor/abraham/twitteroauth/src/TwitterOAuth.php on line 384.

   WARNING  Undefined property: stdClass::$media_id_string in vendor/abraham/twitteroauth/src/TwitterOAuth.php on line 384.

   WARNING  Undefined property: stdClass::$media_id_string in vendor/abraham/twitteroauth/src/TwitterOAuth.php on line 384.

   WARNING  Undefined property: stdClass::$media_id_string in vendor/abraham/twitteroauth/src/TwitterOAuth.php on line 401.

   WARNING  Undefined property: stdClass::$media_id_string in vendor/laravel-notification-channels/twitter/src/TwitterChannel.php on line 105.

   TYPE ERROR  Abraham\TwitterOAuth\TwitterOAuth::mediaStatus(): Argument #1 ($media_id) must be of type string, null given, called in vendor/laravel-notification-channels/twitter/src/TwitterChannel.php on line 105 in phar:///Applications/Tinkerwell.app/Contents/Resources/tinkerwell/tinker.phar/vendor/psy/psysh/src/Exception/TypeErrorException.php on line 20.

Abraham\TwitterOAuth\TwitterOAuth::mediaStatus(): Argument #1 ($media_id) must be of type string, null given, called in ..../vendor/laravel-notification-channels/twitter/src/TwitterChannel.php on line 105
/vendor/abraham/twitteroauth/src/TwitterOAuth.php:58
    /vendor/laravel-notification-channels/twitter/src/TwitterChannel.php:105
         in Abraham\TwitterOAuth\TwitterOAuth::mediaStatus()

I'm pretty sure I exhausted all avenues of application tweaks and twitter/x app configuration here, but I'm all ears if you know of a pathway to use 1.1 media AND 2.0 auth on behalf of other users. I ended up switching back to the twitterOAuth1() provider which generates 1.1 compliant token and secret to post on behalf of users. Good enough workaround.

As for a proposed solution, I'm equally unsure. For my use case, I found that $init was coming back null in \Abraham\TwitterOAuth on line 367... outside the scope of the twitter notification channel package. Seems as though this might be out of scope OR maybe a new exception around the uses of $this->twitter->upload which gets thrown if the return is borked, but even that would still dump out bad references from Abraham's package. Tough one.

Happy to discuss and submit a PR if we find a decent solution.

christophrumpel commented 1 month ago

Thanks, @ivannovak, for all the details, and sorry for the issues you had. Honestly, I currently don't have time to look into this. If you got an idea for a PR I'm open for taking a look at that and help.

ivannovak commented 1 month ago

No worries at all @christophrumpel! I'm sure you're plenty busy elsewhere. Figured, worst case this could help someone else along the way.

christophrumpel commented 1 month ago

Yeah right, thanks a lot fo sharing. I will keep the issue open so people can read it. And I hope there will be a point when we are past those issues 🙏 and when I find some time again.